Le 20/12/2017 à 09:46, Vaibhav Jain a écrit :

Hi Chritophe,

christophe lombard [off-list ref] writes:

quoted

Le 20/12/2017 à 07:31, Vaibhav Jain a écrit :

quoted

EINVAL might be a better return value instead of ENODEV in this case.

This return code has been already discussed (with mpe) on the first
version of the patch. "Either ENODEV or ENXIO would be best that
can be distinguished and interpreted correctly by userspace"

Agreed. Please ignore the review comment.

quoted

quoted

quoted

+	/* Assign a unique TIDR (thread id) for the current thread */
+	if (work.flags & CXL_START_WORK_TID) {
+		rc = cxl_context_thread_tidr(ctx);
+		if (rc)
+			goto out;

May need to copy the cxl_ioctl_start_work struct back to userspace with
the value of tidr allocated.

In fact, it does not matter. I don't know what the userspace could do
with this value.

Without libcxl knowing the tidr value, it cannot enforce the condition
that only threads that have called attach can issue 'wait' on the right
context.

Also AFU can selectively ask PSL to issue asb_notify to a specific
thread via the PSL interface. Without userspace knowing the tidr value
it might not be easy for it to give this value to AFU through a Problem
State Area register.

Don't forget that The ASB_Notify will use LPID:PID:TID tuple found
in the Process Element Entry.
The AFU may optionally provide a TID on AxH_CEA[40:55] (AxH_CEA[39]
must be set to indicate an AFU provided TID)
If AxH_CEA[39] == 1’b0 then Process Element information
(LPID:PID:TID) is used to generate the PCIe address.
If AxH_CEA[39] == 1’b1then the LPID:PID are taken from the PEE
while the TID is taken from AxH_-CEA[40:55]

quoted

quoted

quoted

+	}
+
   	trace_cxl_attach(ctx, work.work_element_descriptor,
work.num_interrupts, amr);

should update the tracing here to also report the tidr

yep. I will provide a new patch to include this update.

Thanks