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

[Andrew Cooper <Andrew.Cooper3@citrix.com>]

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.

~Andrew