Re: [PATCH v2 6/6] xen/arm: Add capabilities to dom0less

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 31 Mar 2025, Jason Andryuk wrote:
> Add capabilities property to dom0less to allow building a
> disaggregated system.  Only a single hardware domain and single xenstore
> domain can be specified.  Multiple control domains are possible.
> 
> Introduce bootfdt.h to contain these constants.
> 
> When using the hardware or xenstore capabilities, adjust the grant and
> event channel limits similar to dom0.
> 
> For a hardware domain, require an IOMMU and disallow specifying a vpl011
> console or nr_spis.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> ---
> v2:
> Fix comment style
> Make DOMAIN_CAPS_* unsigned
> Remove forced directmap & iommu
> Require iommu with use of hardware domain
> Limit to a single xenstore domain
> 
> 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.  Hardware domain
> receiving all hardware can only have a single domain.
> 
> For Xenstore, the logic latches the single xs_domid and uses that for
> all domains.  Also, only a single domain can register for VIRQ_DOM_EXC.
> ---
>  docs/misc/arm/device-tree/booting.txt | 11 ++++++++
>  xen/arch/arm/dom0less-build.c         | 39 +++++++++++++++++++++++++++
>  xen/arch/arm/domain.c                 |  3 ++-
>  xen/include/public/bootfdt.h          | 31 +++++++++++++++++++++
>  4 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 xen/include/public/bootfdt.h
> 
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index ac781c9cc8..490c792ddf 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -167,6 +167,17 @@ with the following properties:
>      Refer to docs/misc/cache_coloring.rst for syntax. This option is 
> applicable
>      only to Arm64 guests.
>  
> +- capabilities
> +    Optional.  A bit field of domain capabilities for a disaggregated

Please reword as:

Optional.  A 32-bit integer representing a bit field...

That is because there is no native bit field type in device tree.



> +    system.  A traditional dom0 has all all of these capabilities, and a
> +    domU has none of them.
> +
> +    0x1 DOMAIN_CAPS_CONTROL  - A privileged, control domain
> +    0x2 DOMAIN_CAPS_HARDWARE - The hardware domain - there can be only 1
> +    0x4 DOMAIN_CAPS_XENSTORE - The xenstore domain - there can be only 1
> +
> +    The default is no capabilities.
> +
>  - vpl011
>  
>      An empty property to enable/disable a virtual pl011 for the guest to
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index fc515c9852..1cb6c170a7 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>
> @@ -906,6 +907,8 @@ static int __init construct_domU(struct domain *d,
>             d->max_vcpus, mem);
>  
>      kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
> +    if ( kinfo.vpl011 && is_hardware_domain(d) )
> +        panic("hardware domain cannot specify vpl011\n");
>  
>      rc = dt_property_read_string(node, "xen,enhanced", &dom0less_enhanced);
>      if ( rc == -EILSEQ ||
> @@ -1020,6 +1023,40 @@ 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);
> +
> +                if ( !iommu_enabled )
> +                    panic("iommu required for dom0less hardware domain\n");

The panic is OK if "direct-map" is not specified. We need to check for
direct-map before panic'ing.

Other than these two comments it looks good to me.


> +
> +                d_cfg.max_grant_frames = gnttab_dom0_frames();
> +                d_cfg.max_evtchn_port = -1;
> +                flags |= CDF_hardware;
> +                iommu = true;
> +            }
> +
> +            if ( val & DOMAIN_CAPS_XENSTORE )
> +            {
> +                if ( xs_domid != DOMID_INVALID )
> +                    panic("Only 1 xenstore domain can be specified! (%u)\n",
> +                          xs_domid);
> +
> +                d_cfg.flags |= XEN_DOMCTL_CDF_xs_domain;
> +                d_cfg.max_evtchn_port = -1;
> +            }
> +        }
> +
>          if ( dt_find_property(node, "xen,static-mem", NULL) )
>          {
>              if ( llc_coloring_enabled )
> @@ -1082,6 +1119,8 @@ void __init create_domUs(void)
>                  d_cfg.arch.nr_spis = MAX(d_cfg.arch.nr_spis,
>                                           vpl011_virq - 32 + 1);
>          }
> +        else if ( flags & CDF_hardware )
> +            panic("nr_spis cannot be specified for hardware domain\n");
>  
>          /* Get the optional property domain-cpupool */
>          cpupool_node = dt_parse_phandle(node, "domain-cpupool", 0);
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 3ba959f866..dc4b4e84c1 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -608,7 +608,8 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  {
>      unsigned int max_vcpus;
>      unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
> -    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpmu);
> +    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpmu |
> +                                   XEN_DOMCTL_CDF_xs_domain );
>      unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
>  
>      if ( (config->flags & ~flags_optional) != flags_required )
> diff --git a/xen/include/public/bootfdt.h b/xen/include/public/bootfdt.h
> new file mode 100644
> index 0000000000..86c46b42a9
> --- /dev/null
> +++ b/xen/include/public/bootfdt.h
> @@ -0,0 +1,31 @@
> +/* 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.
> + */
> +
> +/* Control/Privileged domain capable of affecting other domains. */
> +#define DOMAIN_CAPS_CONTROL  (1U << 0)
> +/*
> + * Hardware domain controlling physical hardware.  Typically providing
> + * backends to other domains.
> + */
> +#define DOMAIN_CAPS_HARDWARE (1U << 1)
> +/* Xenstore domain. */
> +#define DOMAIN_CAPS_XENSTORE (1U << 2)
> +#define DOMAIN_CAPS_MASK     (DOMAIN_CAPS_CONTROL | DOMAIN_CAPS_HARDWARE | \
> +                              DOMAIN_CAPS_XENSTORE)
> +
> +#endif /* __XEN_PUBLIC_BOOTFDT_H__ */
> -- 
> 2.49.0
>