Re: [PATCH 05/23] xen/arm: Add capabilities to dom0less

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-03-06 20:40, Stefano Stabellini wrote:
> On Thu, 6 Mar 2025, Jason Andryuk wrote:
> > Add capabilities property to dom0less to allow building a
> > disaggregated system.
> >
> > Introduce bootfdt.h to contain these constants.
> >
> > When using the hardware or xenstore capabilities, adjust the grant and
> > event channel limits similar to dom0.
> >
> > Also for the hardware domain, set directmap and iommu.  This brings its
> > configuration in line with a dom0.
> >
> > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > ---
> > There is overlap with hyperlaunch.  The numeric values are the same.
> > Hyperlaunch doesn't expose the values in a public header as done here.
> > Is this to be expected for dom0less?  It seems most of dom0less isn't in
> > a header, but just in docs.
> >
> > Hyperlaunch uses BUILD_CAPS_, but I chose DOMAIN_CAPS_ since there are
> > domain-level capabilities.
> >
> > Only a single xenstore and hardware domain make sense.  A check to limit
> > to only a single hardware domain is in place - building two breaks.  But
> > nothing prevents the dom0less configuration from only having multiple
> > xenstore domains.  Each xenstore domain would have slightly more
> > permissions, but only the last one would be used.
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 5a7871939b..068bf99294 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -12,6 +12,7 @@
> >   #include <xen/sizes.h>
> >   #include <xen/vmap.h>
> >   
> > +#include <public/bootfdt.h>
> >   #include <public/io/xs_wire.h>
> >   
> >   #include <asm/arm64/sve.h>
> > @@ -994,6 +995,34 @@ void __init create_domUs(void)
> >           if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
> >               panic("No more domain IDs available\n");
> >   
> > +        if ( dt_property_read_u32(node, "capabilities", &val) )
> > +        {
> > +            if ( val & ~DOMAIN_CAPS_MASK )
> > +                panic("Invalid capabilities (%"PRIx32")\n", val);
> > +
> > +            if ( val & DOMAIN_CAPS_CONTROL )
> > +                flags |= CDF_privileged;
> > +
> > +            if ( val & DOMAIN_CAPS_HARDWARE )
> > +            {
> > +                if ( hardware_domain )
> > +                    panic("Only 1 hardware domain can be specified! (%pd)\n",
> > +                           hardware_domain);
> > +
> > +                d_cfg.max_grant_frames = gnttab_dom0_frames();
> > +                d_cfg.max_evtchn_port = -1;
> max_maptrack_frames = -1 ?
Yes.

> > +                flags |= CDF_hardware;
> > +                flags |= CDF_directmap;
> > +                iommu = true;
> > +            }
> > +
> > +            if ( val & DOMAIN_CAPS_XENSTORE )
> > +            {
> > +                d_cfg.flags |= XEN_DOMCTL_CDF_xs_domain;
> shouldn't we take the opportunity to also set XEN_DOMCTL_CDF_xs_domain
> in xen/arch/arm/domain_build.c:create_dom0 ?
We could.  It isn't normally used except for a standalone xenstore 
domain, which is why I didn't add it.
> > +                d_cfg.max_evtchn_port = -1;
> Why this one?
It mirrors what is done in init-xenstore for a xenstore stubdom.  It's 
to remove the limit so the xenstore domain can create as many as 
necessary for however many domains.
> > +            }
> > +        }
> > +
> >           if ( dt_find_property(node, "xen,static-mem", NULL) )
> >           {
> >               if ( llc_coloring_enabled )
> > diff --git a/xen/include/public/bootfdt.h b/xen/include/public/bootfdt.h
> > new file mode 100644
> > index 0000000000..4e87aca8ac
> > --- /dev/null
> > +++ b/xen/include/public/bootfdt.h
> > @@ -0,0 +1,27 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Xen Device Tree boot information
> > + *
> > + * Information for configuring Xen domains created at boot time.
> > + */
> > +
> > +#ifndef __XEN_PUBLIC_BOOTFDT_H__
> > +#define __XEN_PUBLIC_BOOTFDT_H__
> > +
> > +/* Domain Capabilities specified in the "capabilities" property.  Use of
> > + * this property allows splitting up the monolithic dom0 into separate,
> > + * less privileged components.  A regular domU has no capabilities
> > + * (which is the default if nothing is specified).  A traditional dom0
> > + * has all three capabilities.*/
> The multiline comment coding style is:
>
> /*
>   * comment
>   * comment
>   */
Ok.

Thanks,
Jason