Re: [Xen-devel] [PATCH v3 20/24] xen/passthrough: Extend XEN_DOMCTL_assign_device to support DT device

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 29 Jan 2015, Julien Grall wrote:
> On 29/01/15 12:09, Stefano Stabellini wrote:
> > On Thu, 29 Jan 2015, Julien Grall wrote:
> >> Hi Stefano,
> >>
> >> On 29/01/15 10:29, Stefano Stabellini wrote:
> >>>> +static bool_t iommu_dt_device_is_assigned(const struct dt_device_node 
> >>>> *dev)
> >>>> +{
> >>>> +    bool_t assigned = 0;
> >>>> +
> >>>> +    if ( !dt_device_is_protected(dev) )
> >>>> +        return 1;
> >>>
> >>> Why return true here?
> >>
> >> Because any device not protected cannot be assigned to another guest.
> >> This could be used by the toolstack to know whether the device is
> >> assigned or not.
> > 
> > I understand that much.
> > 
> > 
> >> IHMO, returning 0 would be a false negative.
> > 
> > Why? Returning 0 means that the device is not assigned, that would be
> > correct. From this statement I think that actually you are thinking as
> > if this function actually returned whether a given device is assignable.
> 
> 0 means the device is not assigned and therefore can be used for
> passthrough.

I disagree on the "therefore".
0 means (or should mean that) the device is not assigned and stop there.


> This function is used in the DOMCTL test_assign_device. That would mean
> we will return 0 for non-protected device. That doesn't sound right as a
> such device can never be assigned.

assigned -> passed through to a VM
assignable -> can potentially be assigned

A non-assignable device cannot be assigned by definition. It does not
make sense to returned true for it, if the question is "is it
assigned?".


> > In that case you should rename the function to
> > 
> > iommu_dt_device_is_assignable
> 
> dt_device_is_assignable is not more clear.

I disagree.


> Return 0 here would mean the
> device cannot be assigned in the sense it's not protected.
> 
> Moreover, this name would not be in sync with the DOMCTL test_assign_device.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel