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

[Julien Grall <julien.grall@linaro.org>]

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.

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.

> In that case you should rename the function to
> 
> iommu_dt_device_is_assignable

dt_device_is_assignable is not more clear. 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.


Regards,

-- 
Julien Grall