Re: [PATCH v4] xen/iommu: cleanup iommu related domctl handling

[Jan Beulich <jbeulich@suse.com>]

On 21.04.2022 19:47, Andrew Cooper wrote:
> On 20/04/2022 07:27, Jan Beulich wrote:
>> On 20.04.2022 08:22, Juergen Gross wrote:
>>> On 20.04.22 08:11, Jan Beulich wrote:
>>>> On 20.04.2022 07:57, Juergen Gross wrote:
>>>>> --- a/xen/include/xen/iommu.h
>>>>> +++ b/xen/include/xen/iommu.h
>>>>> @@ -341,8 +341,17 @@ struct domain_iommu {
>>>>>   /* Does the IOMMU pagetable need to be kept synchronized with the P2M */
>>>>>   #ifdef CONFIG_HAS_PASSTHROUGH
>>>>>   #define need_iommu_pt_sync(d)     (dom_iommu(d)->need_sync)
>>>>> +
>>>>> +int iommu_do_domctl(struct xen_domctl *domctl, struct domain *d,
>>>>> +                    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
>>>>>   #else
>>>>>   #define need_iommu_pt_sync(d)     ({ (void)(d); false; })
>>>>> +
>>>>> +static inline int iommu_do_domctl(struct xen_domctl *domctl, struct domain *d,
>>>>> +                                  XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>>>> +{
>>>>> +    return -ENOSYS;
>>>>> +}
>>>> As said in reply to Andrew as well as in a number or earlier occasions,
>>>> I firmly think that this wants to be -EOPNOTSUPP, not -ENOSYS. Views
>>> In libxl there is an explicit check for ENOSYS being returned for
>>> assigning/deassigning a device, same in the xc python bindings.
>> Urgh.
> 
> Honestly, I wasn't particularly happy with your push to swap ENOSYS out
> for EOPNOTSUPP.  This shows plainly why it's a bad move.
> 
> An end user doesn't give two hoots about the distinction between
> hypercall not supported and subops not supported; they care about
> whether Xen can perform the requested action or not.  ENOSYS is the more
> common way of signalling this, and having only one errno value to check
> is better for everyone involved.

End users are of little interest here. Code looking for ENOSYS is what
is of interest, when the meaning of ENOSYS is quite well defined as
"system call not implemented" (which we merely extend to "hypercall").
Anything smaller scope than a major hypercall cannot possibly be
"hypercall not implemented". Code caring about knowing one vs the
other should not be misguided, and code looking for one when the other
is meant is simply flawed.

Jan