Hi Chritophe,
christophe lombard <clomb...@linux.vnet.ibm.com> writes:
> Le 20/12/2017 à 07:31, Vaibhav Jain a écrit :
>> 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.
>>> + /* 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.
>>> + }
>>> +
>>> 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
--
Vaibhav Jain <vaib...@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.
