Re: [Xen-devel] [PATCH RFC 08/31] tools/stubs: Expose host featureset to userspace

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/01/16 16:09, Ian Campbell wrote:
> On Tue, 2016-01-05 at 15:59 +0000, Andrew Cooper wrote:
>> On 05/01/16 15:36, Ian Campbell wrote:
>>> On Wed, 2015-12-16 at 21:24 +0000, Andrew Cooper wrote:
>>>> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
>>>> index c613545..4d7af3d 100644
>>>> --- a/tools/libxc/xc_misc.c
>>>> +++ b/tools/libxc/xc_misc.c
>>>> @@ -718,6 +718,33 @@ int xc_hvm_inject_trap(
>>>>      return rc;
>>>>  }
>>>>  
>>>> +int xc_get_featureset(xc_interface *xch, uint32_t index,
>>>> +                      uint32_t *nr_features, uint32_t *featureset)
>>> This looks like a valid binding to the hypercall iface, so once that is
>>> agreed this LGTM.
>>>
>>>> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
>>>> b/tools/ocaml/libs/xc/xenctrl_stubs.c
>>>> index b7de615..a47473b 100644
>>>> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
>>>> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
>>>> @@ -1220,6 +1220,41 @@ CAMLprim value
>>>> stub_xc_domain_deassign_device(value xch, value domid, value desc
>>>>    CAMLreturn(Val_unit);
>>>>  }
>>>>  
>>>> +CAMLprim value stub_xc_get_featureset(value xch, value idx)
>>>> +{
>>>> +  CAMLparam2(xch, idx);
>>>> +  CAMLlocal1(bitmap_val);
>>>> +
>>>> +  /* Safe, because of the global ocaml lock. */
>>>> +  static uint32_t fs_len;
>>>> +
>>>> +  if (fs_len == 0)
>>>> +  {
>>>> +          int ret = xc_get_featureset(_H(xch), 0, &fs_len,
>>>> NULL);
>>>> +
>>>> +          if (ret || (fs_len == 0))
>>>> +                  failwith_xc(_H(xch));
>>>> +  }
>>> This confuses me because I had thought when reading the previous patch
>>> that
>>> the output nr_features would depend on the specific index, is that not
>>> the
>>> case? Maybe this is just a hypercall docs fix?
>> nr_features is compile-time-constant in Xen, and every featureset handed
>> back will be of that length.
> IOW if the featureset given to the hypercall is NULL then index is ignored?
> Worth documenting.

From the previous patch,

+struct xen_sysctl_featureset {
+#define XEN_SYSCTL_featureset_host      0
+    uint32_t index;       /* IN: Which featureset to query? */
+    uint32_t nr_features; /* IN/OUT: Number of entries in/written to
+                           * 'features', or the maximum number of features if
+                           * the guest handle is NULL. */
+    XEN_GUEST_HANDLE_64(uint32) features; /* OUT: */


This is the usual documentation for this behaviour.  Is that enough?

>
>> It is expected to grow given new changes to Xen, but won't change at
>> runtime.
> And we never expect any index to need a different length to all the others?

All bitmaps returned use bits from the same numberspace, which are going
to have the same maximum potential length.

~Andrew

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