Hi Chritophe,

christophe lombard [off-list ref] writes:

Le 20/12/2017 =C3=A0 07:31, Vaibhav Jain a =C3=A9crit=C2=A0:

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

+	/* Assign a unique TIDR (thread id) for the current thread */
+	if (work.flags & CXL_START_WORK_TID) {
+		rc =3D 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.

quoted

quoted

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

should update the tracing here to also report the tidr
=20

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

Thanks

--=20
Vaibhav Jain [off-list ref]
Linux Technology Center, IBM India Pvt. Ltd.