Re: [Xen-devel] [PATCH v8 2/6] domain: introduce XEN_DOMCTL_CDF_iommu flag

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
[snip]
> > diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> > index 35958b94d5..bdf3f2e395 100644
> > --- a/tools/ocaml/libs/xc/xenctrl.ml
> > +++ b/tools/ocaml/libs/xc/xenctrl.ml
> > @@ -56,7 +56,13 @@ type arch_domainconfig =
> >     | ARM of xen_arm_arch_domainconfig
> >     | X86 of xen_x86_arch_domainconfig
> >
> > -type domain_create_flag = CDF_HVM | CDF_HAP
> > +type domain_create_flag =
> > +   | CDF_HVM
> > +   | CDF_HAP
> > +   | CDF_S3_INTEGRITY
> > +   | CDF_OOS_OFF
> > +   | CDF_XS_DOMAIN
> > +   | CDF_IOMMU
> 
> This patch is only adding the last flag, but you are adding more here. I
> understand that you are just re-syncing the type with Xen. IHMO, this
> should belong in a separate patch as we may want to backport it.
> 
> If backporting is not necessary, then the change should at least be
> mentioned in the commit message as this seems a bit off-topic.
> 

The apparently extraneous flag additions are because of the way that the code at

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/ocaml/libs/xc/xenctrl_stubs.c;hb=HEAD#l149

works. TL;DR the index of the item in the list needs to match the bit position 
in the flags field.

Since you queried it I guess it'd better have a comment in the code.

  Paul

> Cheers,
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2019-08/msg01974.html
> 
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel