Re: [XEN][PATCH] xen/domctl: add XEN_DOMCTL_CDF_ALL mask

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

On 19.12.25 18:10, Teddy Astie wrote:
> Le 18/12/2025 à 16:21, Grygorii Strashko a écrit :
> > On 11.11.25 22:16, Grygorii Strashko wrote:
> > > From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> > >
> > > Every XEN_DOMCTL_CDF_x flag:
> > > - is defined in public/domctl.h
> > > - explicitly listed in sanitise_domain_config() (common/domain.c) to form
> > > mask containing all supported DOMCTL_CDF flags for "Unknown CDF flags"
> > > check.
> > >
> > > So, it is required to modify two files every time XEN_DOMCTL_CDF_x flags
> > > set is modified.
> > >
> > > Simplify the things by introducing XEN_DOMCTL_CDF_ALL mask, so
> > > sanitise_domain_config() no need to be modified any more.
> > >
> > > Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> > > ---
> > Seems nobody interested, sad, dropping from backlog.
> Sorry, I missed this one (I guess others have too).
>
> > >    xen/common/domain.c         | 7 +------
> > >    xen/include/public/domctl.h | 7 +++++++
> > >    2 files changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > > index 775c33928585..4f91316ad93e 100644
> > > --- a/xen/common/domain.c
> > > +++ b/xen/common/domain.c
> > > @@ -730,12 +730,7 @@ static int sanitise_domain_config(struct
> > > xen_domctl_createdomain *config)
> > >        bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
> > >        bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
> > > -    if ( config->flags &
> > > -         ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> > > -           XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
> > > -           XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> > > -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> > > -           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
> > > +    if ( config->flags & ~XEN_DOMCTL_CDF_ALL )
> > >        {
> > >            dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
> > >            return -EINVAL;
> > > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > > index 8f6708c0a7cd..94a8e3042cbf 100644
> > > --- a/xen/include/public/domctl.h
> > > +++ b/xen/include/public/domctl.h
> > > @@ -72,6 +72,13 @@ struct xen_domctl_createdomain {
> > >    /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> > >    #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
> > > +#define
> > > XEN_DOMCTL_CDF_ALL                                                     \
> > > +        (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> > >                               \
> > > +         XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
> > >                  \
> > > +         XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> > >                       \
> > > +         XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> > >                      \
> > > +         XEN_DOMCTL_CDF_trap_unmapped_accesses)
> > > +
> > >        uint32_t flags;
> > >    #define _XEN_DOMCTL_IOMMU_no_sharept  0
> Given that we already have XEN_DOMCTL_CDF_MAX, we could base the check
> using it instead.
> Something like checking fls(config->flags) > fls(XEN_DOMCTL_CDF_MAX) ?
That's what I've tried first.
The problem is that there is possibility of CDF_X to be removed/deprecated, 
small, but still it is.
In such case fls() can't be used.
So using define which list allowed flags is safer from long term point of view.

--
Best regards,
-grygorii