Re: [RFC][v2][PATCH 07/14] xen/passthrough: extend hypercall to support rdm reservation policy

[Julien Grall <julien.grall@citrix.com>]

Hi,

On 25/05/2015 12:50, Chen, Tiejun wrote:
>
> On 2015/5/25 18:02, Julien Grall wrote:
>>
>>
>> On 25/05/2015 04:09, Chen, Tiejun wrote:
>>> On 2015/5/22 18:33, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> On 22/05/2015 10:35, Tiejun Chen wrote:
>>>>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>>>>> index 0c0ea4a..203c80e 100644
>>>>> --- a/xen/include/public/domctl.h
>>>>> +++ b/xen/include/public/domctl.h
>>>>> @@ -499,6 +499,11 @@ struct xen_domctl_assign_device {
>>>>>               XEN_GUEST_HANDLE_64(char) path; /* path to the device
>>>>> tree node */
>>>>>           } dt;
>>>>>       } u;
>>>>> +    /* IN */
>>>>> +#define XEN_DOMCTL_DEV_NO_RDM           0
>>>>> +#define XEN_DOMCTL_DEV_RDM_RELAXED      1
>>>>> +#define XEN_DOMCTL_DEV_RDM_STRICT       2
>>>>> +    uint32_t  flag;   /* flag of assigned device */
>>>>
>>>> You don't plumb this value for DT neither in the toolstack (see
>>>> xc_assign_dt_device) and Xen. Please add a comment saying it's only
>>>> used
>>>
>>> I think we should do this,
>>>
>>> @@ -1801,6 +1801,8 @@ int xc_assign_dt_device(
>>>
>>>       domctl.u.assign_device.dev = XEN_DOMCTL_DEV_DT;
>>>       domctl.u.assign_device.u.dt.size = size;
>>> +    /* DT doesn't own any RDM. */
>>> +    domctl.u.assign_device.flag = XEN_DOMCTL_DEV_NO_RDM;
>>>       set_xen_guest_handle(domctl.u.assign_device.u.dt.path, path);
>>>
>>>       rc = do_domctl(xch, &domctl);
>>
>> I would be fine with plumbing in drivers/passthrough/device_tree.c and a
>> check that the value is not different.
>>
>
> Are you saying something like this?
>
> @@ -149,6 +149,14 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl,
> struct domain *d,
>           if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
>               break;
>
> +        if ( domctl->u.assign_device.dev == XEN_DOMCTL_DEV_NO_RDM )

wrong field here. Other than that it looks good to me.

> +        {
> +            printk(XENLOG_G_ERR "XEN_DOMCTL_assign_device: assign \"%s\""
> +                   " to dom%u failed (%d) since we don't support RDM.\n",
> +                   dt_node_full_name(dev), d->domain_id, ret);
> +            break;
> +        }
> +
>           if ( unlikely(d->is_dying) )
>           {
>               ret = -EINVAL;

Regards,

-- 
Julien Grall