Re: [v3][PATCH 10/16] tools: extend xc_assign_device() to support rdm reservation policy

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2015/6/11 18:02, Tian, Kevin wrote:
>> From: Chen, Tiejun
>> Sent: Thursday, June 11, 2015 9:15 AM
>>
>> This patch passes rdm reservation policy to xc_assign_device() so the policy
>> is checked when assigning devices to a VM.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   tools/libxc/include/xenctrl.h       |  3 ++-
>>   tools/libxc/xc_domain.c             |  6 +++++-
>>   tools/libxl/libxl_pci.c             |  3 ++-
>>   tools/ocaml/libs/xc/xenctrl_stubs.c | 18 ++++++++++++++----
>>   tools/python/xen/lowlevel/xc/xc.c   | 29 +++++++++++++++++++----------
>>   5 files changed, 42 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index 6c01362..7fd60d5 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -2078,7 +2078,8 @@ int xc_hvm_destroy_ioreq_server(xc_interface *xch,
>>   /* HVM guest pass-through */
>>   int xc_assign_device(xc_interface *xch,
>>                        uint32_t domid,
>> -                     uint32_t machine_sbdf);
>> +                     uint32_t machine_sbdf,
>> +                     uint32_t flag);
>>
>>   int xc_get_device_group(xc_interface *xch,
>>                        uint32_t domid,
>> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
>> index 4f96e1b..19127ec 100644
>> --- a/tools/libxc/xc_domain.c
>> +++ b/tools/libxc/xc_domain.c
>> @@ -1697,7 +1697,8 @@ int xc_domain_setdebugging(xc_interface *xch,
>>   int xc_assign_device(
>>       xc_interface *xch,
>>       uint32_t domid,
>> -    uint32_t machine_sbdf)
>> +    uint32_t machine_sbdf,
>> +    uint32_t flag)
>>   {
>>       DECLARE_DOMCTL;
>>
>> @@ -1705,6 +1706,7 @@ int xc_assign_device(
>>       domctl.domain = domid;
>>       domctl.u.assign_device.dev = XEN_DOMCTL_DEV_PCI;
>>       domctl.u.assign_device.u.pci.machine_sbdf = machine_sbdf;
>> +    domctl.u.assign_device.flag = flag;
>>
>>       return do_domctl(xch, &domctl);
>>   }
>> @@ -1792,6 +1794,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;
>
> still not clear about this NO_RDM flag. If a device-tree device doesn't
> own RDM, the hypervisor will know it. Why do we require toolstack
> to tell hypervisor not use it?

I think an explicit flag can make this sort of case identified, right? 
And other flags brings easily some confusions, or even a potential risk 
in the future.

Thanks
Tiejun

>
>>       set_xen_guest_handle(domctl.u.assign_device.u.dt.path, path);
>>
>>       rc = do_domctl(xch, &domctl);
>> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
>> index e0743f8..632c15e 100644
>> --- a/tools/libxl/libxl_pci.c
>> +++ b/tools/libxl/libxl_pci.c
>> @@ -894,6 +894,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
>> libxl_device_pci *pcidev, i
>>       FILE *f;
>>       unsigned long long start, end, flags, size;
>>       int irq, i, rc, hvm = 0;
>> +    uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED;
>>
>>       if (type == LIBXL_DOMAIN_TYPE_INVALID)
>>           return ERROR_FAIL;
>> @@ -987,7 +988,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
>> libxl_device_pci *pcidev, i
>>
>>   out:
>>       if (!libxl_is_stubdom(ctx, domid, NULL)) {
>> -        rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev));
>> +        rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev), flag);
>>           if (rc < 0 && (hvm || errno != ENOSYS)) {
>>               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_assign_device failed");
>>               return ERROR_FAIL;
>> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> index 64f1137..317bf75 100644
>> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
>> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
>> @@ -1172,12 +1172,19 @@ CAMLprim value stub_xc_domain_test_assign_device(value
>> xch, value domid, value d
>>   	CAMLreturn(Val_bool(ret == 0));
>>   }
>>
>> -CAMLprim value stub_xc_domain_assign_device(value xch, value domid, value desc)
>> +static int domain_assign_device_rdm_flag_table[] = {
>> +    XEN_DOMCTL_DEV_NO_RDM,
>> +    XEN_DOMCTL_DEV_RDM_RELAXED,
>> +    XEN_DOMCTL_DEV_RDM_STRICT,
>> +};
>> +
>> +CAMLprim value stub_xc_domain_assign_device(value xch, value domid, value desc,
>> +                                            value rflag)
>>   {
>> -	CAMLparam3(xch, domid, desc);
>> +	CAMLparam4(xch, domid, desc, rflag);
>>   	int ret;
>>   	int domain, bus, dev, func;
>> -	uint32_t sbdf;
>> +	uint32_t sbdf, flag;
>>
>>   	domain = Int_val(Field(desc, 0));
>>   	bus = Int_val(Field(desc, 1));
>> @@ -1185,7 +1192,10 @@ CAMLprim value stub_xc_domain_assign_device(value xch,
>> value domid, value desc)
>>   	func = Int_val(Field(desc, 3));
>>   	sbdf = encode_sbdf(domain, bus, dev, func);
>>
>> -	ret = xc_assign_device(_H(xch), _D(domid), sbdf);
>> +	ret = Int_val(Field(rflag, 0));
>> +	flag = domain_assign_device_rdm_flag_table[ret];
>> +
>> +	ret = xc_assign_device(_H(xch), _D(domid), sbdf, flag);
>>
>>   	if (ret < 0)
>>   		failwith_xc(_H(xch));
>> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
>> index c77e15b..172bdf0 100644
>> --- a/tools/python/xen/lowlevel/xc/xc.c
>> +++ b/tools/python/xen/lowlevel/xc/xc.c
>> @@ -592,7 +592,8 @@ static int token_value(char *token)
>>       return strtol(token, NULL, 16);
>>   }
>>
>> -static int next_bdf(char **str, int *seg, int *bus, int *dev, int *func)
>> +static int next_bdf(char **str, int *seg, int *bus, int *dev, int *func,
>> +                    int *flag)
>>   {
>>       char *token;
>>
>> @@ -607,8 +608,16 @@ static int next_bdf(char **str, int *seg, int *bus, int *dev, int
>> *func)
>>       *dev  = token_value(token);
>>       token = strchr(token, ',') + 1;
>>       *func  = token_value(token);
>> -    token = strchr(token, ',');
>> -    *str = token ? token + 1 : NULL;
>> +    token = strchr(token, ',') + 1;
>> +    if ( token ) {
>> +        *flag = token_value(token);
>> +        *str = token + 1;
>> +    }
>> +    else
>> +    {
>> +        *flag = XEN_DOMCTL_DEV_RDM_STRICT;
>> +        *str = NULL;
>> +    }
>>
>>       return 1;
>>   }
>> @@ -620,14 +629,14 @@ static PyObject *pyxc_test_assign_device(XcObject *self,
>>       uint32_t dom;
>>       char *pci_str;
>>       int32_t sbdf = 0;
>> -    int seg, bus, dev, func;
>> +    int seg, bus, dev, func, flag;
>>
>>       static char *kwd_list[] = { "domid", "pci", NULL };
>>       if ( !PyArg_ParseTupleAndKeywords(args, kwds, "is", kwd_list,
>>                                         &dom, &pci_str) )
>>           return NULL;
>>
>> -    while ( next_bdf(&pci_str, &seg, &bus, &dev, &func) )
>> +    while ( next_bdf(&pci_str, &seg, &bus, &dev, &func, &flag) )
>>       {
>>           sbdf = seg << 16;
>>           sbdf |= (bus & 0xff) << 8;
>> @@ -653,21 +662,21 @@ static PyObject *pyxc_assign_device(XcObject *self,
>>       uint32_t dom;
>>       char *pci_str;
>>       int32_t sbdf = 0;
>> -    int seg, bus, dev, func;
>> +    int seg, bus, dev, func, flag;
>>
>>       static char *kwd_list[] = { "domid", "pci", NULL };
>>       if ( !PyArg_ParseTupleAndKeywords(args, kwds, "is", kwd_list,
>>                                         &dom, &pci_str) )
>>           return NULL;
>>
>> -    while ( next_bdf(&pci_str, &seg, &bus, &dev, &func) )
>> +    while ( next_bdf(&pci_str, &seg, &bus, &dev, &func, &flag) )
>>       {
>>           sbdf = seg << 16;
>>           sbdf |= (bus & 0xff) << 8;
>>           sbdf |= (dev & 0x1f) << 3;
>>           sbdf |= (func & 0x7);
>>
>> -        if ( xc_assign_device(self->xc_handle, dom, sbdf) != 0 )
>> +        if ( xc_assign_device(self->xc_handle, dom, sbdf, flag) != 0 )
>>           {
>>               if (errno == ENOSYS)
>>                   sbdf = -1;
>> @@ -686,14 +695,14 @@ static PyObject *pyxc_deassign_device(XcObject *self,
>>       uint32_t dom;
>>       char *pci_str;
>>       int32_t sbdf = 0;
>> -    int seg, bus, dev, func;
>> +    int seg, bus, dev, func, flag;
>>
>>       static char *kwd_list[] = { "domid", "pci", NULL };
>>       if ( !PyArg_ParseTupleAndKeywords(args, kwds, "is", kwd_list,
>>                                         &dom, &pci_str) )
>>           return NULL;
>>
>> -    while ( next_bdf(&pci_str, &seg, &bus, &dev, &func) )
>> +    while ( next_bdf(&pci_str, &seg, &bus, &dev, &func, &flag) )
>>       {
>>           sbdf = seg << 16;
>>           sbdf |= (bus & 0xff) << 8;
>> --
>> 1.9.1
>
>
>