Re: [Xen-devel] [PATCH V5 7/8] iommu/arm: Introduce iommu_add_dt_device API

[Oleksandr <olekstysh@xxxxxxxxx>]

On 24.09.19 20:21, Julien Grall wrote:
> Hi Oleksandr,
Hi Julien.


> > > [...]
> > >
> > > >   int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
> > > > XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> > > >   {
> > > > @@ -177,6 +241,13 @@ int iommu_do_dt_domctl(struct xen_domctl 
> > > > *domctl, struct domain *d,
> > > >               break;
> > > >           }
> > > >   +        iommu_add_dt_device(dev);
> > > Same here.
> > Yes, I think, we don't need to check for return value, because the 
> > only one positive result "here" is the fact that "device is 
> > protected" (which is checked below).
> > What is more, if we add a check for the return value to be strictly 
> > 0, we will get an error after guest's reboot (since 
> > iommu_add_dt_device() will return -EEXIST).
> > So, I will add a comment explaining why we don't check. What do you 
> > think?
> Why don't you do the following code?
>
> if ( ret < 0 && ret != -EEXIST )
>
> This would allow the code to return the corrrect error to the upper 
> layer. A suitable comment on top explaing the check would also be useful.
Being honest, I was thinking about the similar, but rejected this. I 
thought, all what we wanted to know "here" was whether the particular 
device protected or not. But, I agree now, the upper layer should be 
informed about the exact error reason.
--
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel