[PATCH 00/10] Extract bindings in dom0less code

[PATCH 00/10] Extract bindings in dom0less code

[Alejandro Vallejo]

Hi,

pipeline: https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/1941315850

With boot_domain common between architectures, we're now in a position to use
it as the common ground to dump results of dom0less bindings. This series is
largely code motion with a few tweaks to make it simple for x86 to hook into
the dom0less bindings.

The idea is to create a parse_dom0less_node() function, that takes a boot_domain
and depending on the bindings found on the xen,domain node, populates it one way
or the other. parse_dom0less_node() calls internally an
arch_parse_dom0less_node() function to allow arch-specific bindings or checks.

Patch 9: "dom0less: Reinitialise all variables on each loop iteration" is a
         separate bugfix. Patch 10 does code motion and implicitly fixes it
         too, but I added it so it can be backported to stable branches.

Alejandro Vallejo (10):
  static-evtchn: Add missing include
  dt: Add cf_check to device-tree.c function pointers
  dom0less: Move asm-generic/dom0less-build.h -> xen/dom0less-build.h
  bootfdt: Add createdomain arguments to boot_domain
  dom0less: Introduce kernel_info into the domain creation loop
  dom0less: Create llc_color_str field in boot_domain
  dom0less: Turn arch_create_domUs into arch_parse_dom0less_node()
  dom0less: Allow arch_parse_dom0less_node() to be fallible
  dom0less: Reinitialise all variables on each loop iteration
  dom0less: Parse dom0less bindings into createdomain input args

 xen/arch/arm/dom0less-build.c                 |  11 +-
 xen/arch/arm/domain_build.c                   |   2 +-
 xen/arch/arm/include/asm/Makefile             |   1 -
 xen/arch/arm/setup.c                          |   2 +-
 xen/common/device-tree/Makefile               |   3 +-
 xen/common/device-tree/device-tree.c          |  28 +--
 xen/common/device-tree/dom0less-bindings.c    | 145 +++++++++++++
 xen/common/device-tree/dom0less-build.c       | 193 ++++--------------
 xen/common/device-tree/static-evtchn.c        |   1 +
 xen/include/xen/bootfdt.h                     |  11 +
 .../{asm-generic => xen}/dom0less-build.h     |  15 +-
 11 files changed, 228 insertions(+), 184 deletions(-)
 create mode 100644 xen/common/device-tree/dom0less-bindings.c
 rename xen/include/{asm-generic => xen}/dom0less-build.h (90%)


base-commit: e9339514744904061ca01046e5ca6b97cb4f0fb2
-- 
2.43.0

[PATCH 01/10] static-evtchn: Add missing include

[Alejandro Vallejo]

From: Alejandro Vallejo <agarciav@amd.com>

When later on x86 starts using this file in later patches it won't find
device_tree.h because it's only transitively included by arm.

Make it explicit.

Not a functional change.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/common/device-tree/static-evtchn.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/device-tree/static-evtchn.c b/xen/common/device-tree/static-evtchn.c
index 88342b44a1..45cdd664e4 100644
--- a/xen/common/device-tree/static-evtchn.c
+++ b/xen/common/device-tree/static-evtchn.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
 #include <xen/bootfdt.h>
+#include <xen/device_tree.h>
 #include <xen/event.h>
 #include <xen/static-evtchn.h>
 
-- 
2.43.0

[PATCH 02/10] dt: Add cf_check to device-tree.c function pointers

[Alejandro Vallejo]

In preparation for it to be usable on x86 with IBT, tag targets of
function pointers with cf_check

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 xen/common/device-tree/device-tree.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/xen/common/device-tree/device-tree.c b/xen/common/device-tree/device-tree.c
index 84daa3f0ed..0b5375f151 100644
--- a/xen/common/device-tree/device-tree.c
+++ b/xen/common/device-tree/device-tree.c
@@ -538,7 +538,7 @@ int dt_child_n_size_cells(const struct dt_device_node *parent)
 /*
  * Default translator (generic bus)
  */
-static bool dt_bus_default_match(const struct dt_device_node *node)
+static bool cf_check dt_bus_default_match(const struct dt_device_node *node)
 {
     /* Root node doesn't have "ranges" property */
     if ( node->parent == NULL )
@@ -550,8 +550,8 @@ static bool dt_bus_default_match(const struct dt_device_node *node)
     return (dt_get_property(node, "ranges", NULL) != NULL);
 }
 
-static void dt_bus_default_count_cells(const struct dt_device_node *dev,
-                                int *addrc, int *sizec)
+static void cf_check dt_bus_default_count_cells(
+    const struct dt_device_node *dev, int *addrc, int *sizec)
 {
     if ( addrc )
         *addrc = dt_n_addr_cells(dev);
@@ -559,8 +559,8 @@ static void dt_bus_default_count_cells(const struct dt_device_node *dev,
         *sizec = dt_n_size_cells(dev);
 }
 
-static u64 dt_bus_default_map(__be32 *addr, const __be32 *range,
-                              int na, int ns, int pna)
+static u64 cf_check dt_bus_default_map(__be32 *addr, const __be32 *range,
+                                       int na, int ns, int pna)
 {
     u64 cp, s, da;
 
@@ -585,7 +585,7 @@ static u64 dt_bus_default_map(__be32 *addr, const __be32 *range,
     return da - cp;
 }
 
-static int dt_bus_default_translate(__be32 *addr, u64 offset, int na)
+static int cf_check dt_bus_default_translate(__be32 *addr, u64 offset, int na)
 {
     u64 a = dt_read_number(addr, na);
 
@@ -597,7 +597,7 @@ static int dt_bus_default_translate(__be32 *addr, u64 offset, int na)
 
     return 0;
 }
-static unsigned int dt_bus_default_get_flags(const __be32 *addr)
+static unsigned int cf_check dt_bus_default_get_flags(const __be32 *addr)
 {
     return IORESOURCE_MEM;
 }
@@ -616,7 +616,7 @@ static bool dt_node_is_pci(const struct dt_device_node *np)
     return is_pci;
 }
 
-static bool dt_bus_pci_match(const struct dt_device_node *np)
+static bool cf_check dt_bus_pci_match(const struct dt_device_node *np)
 {
     /*
      * "pciex" is PCI Express "vci" is for the /chaos bridge on 1st-gen PCI
@@ -630,8 +630,8 @@ static bool dt_bus_pci_match(const struct dt_device_node *np)
         dt_node_is_pci(np);
 }
 
-static void dt_bus_pci_count_cells(const struct dt_device_node *np,
-				   int *addrc, int *sizec)
+static void cf_check dt_bus_pci_count_cells(const struct dt_device_node *np,
+				                                    int *addrc, int *sizec)
 {
     if (addrc)
         *addrc = 3;
@@ -639,7 +639,7 @@ static void dt_bus_pci_count_cells(const struct dt_device_node *np,
         *sizec = 2;
 }
 
-static unsigned int dt_bus_pci_get_flags(const __be32 *addr)
+static unsigned int cf_check dt_bus_pci_get_flags(const __be32 *addr)
 {
     unsigned int flags = 0;
     u32 w = be32_to_cpu(*addr);
@@ -658,8 +658,8 @@ static unsigned int dt_bus_pci_get_flags(const __be32 *addr)
     return flags;
 }
 
-static u64 dt_bus_pci_map(__be32 *addr, const __be32 *range, int na, int ns,
-		int pna)
+static u64 cf_check dt_bus_pci_map(__be32 *addr, const __be32 *range,
+                                   int na, int ns, int pna)
 {
     u64 cp, s, da;
     unsigned int af, rf;
@@ -685,7 +685,7 @@ static u64 dt_bus_pci_map(__be32 *addr, const __be32 *range, int na, int ns,
     return da - cp;
 }
 
-static int dt_bus_pci_translate(__be32 *addr, u64 offset, int na)
+static int cf_check dt_bus_pci_translate(__be32 *addr, u64 offset, int na)
 {
     return dt_bus_default_translate(addr + 1, offset, na - 1);
 }
-- 
2.43.0

[PATCH 03/10] dom0less: Move asm-generic/dom0less-build.h -> xen/dom0less-build.h

[Alejandro Vallejo]

It's meant to be usable by anyone with CONFIG_DOM0LESS_BOOT.

While moving, replace an inclusion of public/domctl.h by a forward
declaration.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/arm/dom0less-build.c                     | 2 +-
 xen/arch/arm/domain_build.c                       | 2 +-
 xen/arch/arm/include/asm/Makefile                 | 1 -
 xen/arch/arm/setup.c                              | 2 +-
 xen/common/device-tree/dom0less-build.c           | 2 +-
 xen/include/{asm-generic => xen}/dom0less-build.h | 7 +++----
 6 files changed, 7 insertions(+), 9 deletions(-)
 rename xen/include/{asm-generic => xen}/dom0less-build.h (96%)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 69b9ea22ce..286557fafd 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -3,6 +3,7 @@
 #include <xen/domain_page.h>
 #include <xen/fdt-domain-build.h>
 #include <xen/fdt-kernel.h>
+#include <xen/dom0less-build.h>
 #include <xen/err.h>
 #include <xen/event.h>
 #include <xen/grant_table.h>
@@ -20,7 +21,6 @@
 #include <public/io/xs_wire.h>
 
 #include <asm/arm64/sve.h>
-#include <asm/dom0less-build.h>
 #include <asm/domain_build.h>
 #include <asm/grant_table.h>
 #include <asm/setup.h>
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d91a71acfd..463ae4474d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2,6 +2,7 @@
 #include <xen/init.h>
 #include <xen/bootinfo.h>
 #include <xen/compile.h>
+#include <xen/dom0less-build.h>
 #include <xen/fdt-domain-build.h>
 #include <xen/fdt-kernel.h>
 #include <xen/lib.h>
@@ -32,7 +33,6 @@
 #include <asm/setup.h>
 #include <asm/arm64/sve.h>
 #include <asm/cpufeature.h>
-#include <asm/dom0less-build.h>
 #include <asm/domain_build.h>
 #include <xen/event.h>
 
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 87c8821421..4565baca6a 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += altp2m.h
 generic-y += device.h
-generic-y += dom0less-build.h
 generic-y += hardirq.h
 generic-y += iocap.h
 generic-y += irq-dt.h
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 8abc1d641d..27bd3f5a6e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -11,6 +11,7 @@
 #include <xen/bootinfo.h>
 #include <xen/compile.h>
 #include <xen/device_tree.h>
+#include <xen/dom0less-build.h>
 #include <xen/domain_page.h>
 #include <xen/grant_table.h>
 #include <xen/llc-coloring.h>
@@ -39,7 +40,6 @@
 #include <xen/warning.h>
 #include <xen/hypercall.h>
 #include <asm/alternative.h>
-#include <asm/dom0less-build.h>
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/setup.h>
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index efa846da2a..e49213e763 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -2,6 +2,7 @@
 
 #include <xen/bootinfo.h>
 #include <xen/device_tree.h>
+#include <xen/dom0less-build.h>
 #include <xen/domain.h>
 #include <xen/domain_page.h>
 #include <xen/err.h>
@@ -25,7 +26,6 @@
 #include <public/event_channel.h>
 #include <public/io/xs_wire.h>
 
-#include <asm/dom0less-build.h>
 #include <asm/setup.h>
 
 #include <xen/static-memory.h>
diff --git a/xen/include/asm-generic/dom0less-build.h b/xen/include/xen/dom0less-build.h
similarity index 96%
rename from xen/include/asm-generic/dom0less-build.h
rename to xen/include/xen/dom0less-build.h
index e0ad0429ec..34b9884cc7 100644
--- a/xen/include/asm-generic/dom0less-build.h
+++ b/xen/include/xen/dom0less-build.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
-#ifndef __ASM_GENERIC_DOM0LESS_BUILD_H__
-#define __ASM_GENERIC_DOM0LESS_BUILD_H__
+#ifndef XEN_DOM0LESS_BUILD_H
+#define XEN_DOM0LESS_BUILD_H
 
 #include <xen/stdbool.h>
 
@@ -9,8 +9,7 @@ struct domain;
 
 #ifdef CONFIG_DOM0LESS_BOOT
 
-#include <public/domctl.h>
-
+struct xen_domctl_createdomain;
 struct dt_device_node;
 struct kernel_info;
 
-- 
2.43.0

[PATCH 04/10] bootfdt: Add createdomain arguments to boot_domain

[Alejandro Vallejo]

Add the arguments that create_domain() takes to boot_domain. This creates
a consistent place to drop the outputs of the dom0less bindings.

Not a functional change. Later patches use these fields as the outputs of
the dom0less parsing functions.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 xen/include/xen/bootfdt.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 6cf05b5cdd..f107099263 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -11,6 +11,8 @@
 #include <asm/bootfdt.h>
 #endif
 
+#include <public/domctl.h>
+
 #define MIN_FDT_ALIGN 8
 
 /* Default #address and #size cells */
@@ -119,6 +121,10 @@ struct boot_domain {
     struct boot_module *initrd;
 
     const char *cmdline;
+
+    /* Input arguments to create_domain() */
+    struct xen_domctl_createdomain create_cfg;
+    unsigned int create_flags;
 };
 
 #define BOOTMOD_MAX_CMDLINE 1024
-- 
2.43.0

[PATCH 05/10] dom0less: Introduce kernel_info into the domain creation loop

[Alejandro Vallejo]

In later patches boot_domain becomes the common ground for the bindings
to drop the extracted information. In preparation for the bindings
themselves to be in a separate function, introduce kernel_info early in
the domain construction loop.

This simplifies a later diff, turning it into a strict cut-and-paste, rather
than a mutate-on-move.

Lines after domain_create() don't make use of the "d_cfg" and "flags"
variables because those will disappear when the bindings move to a
separate file.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/common/device-tree/dom0less-build.c | 109 ++++++++++++------------
 1 file changed, 54 insertions(+), 55 deletions(-)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index e49213e763..cee666786e 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -732,10 +732,10 @@ static inline int __init domain_p2m_set_allocation(
 }
 #endif /* CONFIG_ARCH_PAGING_MEMPOOL */
 
-static int __init construct_domU(struct domain *d,
+static int __init construct_domU(struct kernel_info *kinfo,
                           const struct dt_device_node *node)
 {
-    struct kernel_info kinfo = KERNEL_INFO_INIT;
+    struct domain *d = kinfo->bd.d;
     const char *dom0less_enhanced;
     int rc;
     u64 mem;
@@ -746,7 +746,7 @@ static int __init construct_domU(struct domain *d,
         printk("Error building DomU: cannot read \"memory\" property\n");
         return -EINVAL;
     }
-    kinfo.unassigned_mem = (paddr_t)mem * SZ_1K;
+    kinfo->unassigned_mem = (paddr_t)mem * SZ_1K;
 
     rc = domain_p2m_set_allocation(d, mem, node);
     if ( rc != 0 )
@@ -761,66 +761,64 @@ static int __init construct_domU(struct domain *d,
          (rc == 0 && !strcmp(dom0less_enhanced, "enabled")) )
     {
         need_xenstore = true;
-        kinfo.dom0less_feature = DOM0LESS_ENHANCED;
+        kinfo->dom0less_feature = DOM0LESS_ENHANCED;
     }
     else if ( rc == 0 && !strcmp(dom0less_enhanced, "legacy") )
     {
         need_xenstore = true;
-        kinfo.dom0less_feature = DOM0LESS_ENHANCED_LEGACY;
+        kinfo->dom0less_feature = DOM0LESS_ENHANCED_LEGACY;
     }
     else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
-        kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
+        kinfo->dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
 
     if ( vcpu_create(d, 0) == NULL )
         return -ENOMEM;
 
     d->max_pages = ((paddr_t)mem * SZ_1K) >> PAGE_SHIFT;
 
-    kinfo.bd.d = d;
-
-    rc = kernel_probe(&kinfo, node);
+    rc = kernel_probe(kinfo, node);
     if ( rc < 0 )
         return rc;
 
-    set_domain_type(d, &kinfo);
+    set_domain_type(d, kinfo);
 
     if ( is_hardware_domain(d) )
     {
-        rc = construct_hwdom(&kinfo, node);
+        rc = construct_hwdom(kinfo, node);
         if ( rc < 0 )
             return rc;
     }
     else
     {
         if ( !dt_find_property(node, "xen,static-mem", NULL) )
-            allocate_memory(d, &kinfo);
+            allocate_memory(d, kinfo);
         else if ( !is_domain_direct_mapped(d) )
-            allocate_static_memory(d, &kinfo, node);
+            allocate_static_memory(d, kinfo, node);
         else
-            assign_static_memory_11(d, &kinfo, node);
+            assign_static_memory_11(d, kinfo, node);
 
-        rc = process_shm(d, &kinfo, node);
+        rc = process_shm(d, kinfo, node);
         if ( rc < 0 )
             return rc;
 
-        rc = init_vuart(d, &kinfo, node);
+        rc = init_vuart(d, kinfo, node);
         if ( rc < 0 )
             return rc;
 
-        rc = prepare_dtb_domU(d, &kinfo);
+        rc = prepare_dtb_domU(d, kinfo);
         if ( rc < 0 )
             return rc;
 
-        rc = construct_domain(d, &kinfo);
+        rc = construct_domain(d, kinfo);
         if ( rc < 0 )
             return rc;
     }
 
     domain_vcpu_affinity(d, node);
 
-    rc = alloc_xenstore_params(&kinfo);
+    rc = alloc_xenstore_params(kinfo);
 
-    rangeset_destroy(kinfo.xen_reg_assigned);
+    rangeset_destroy(kinfo->xen_reg_assigned);
 
     return rc;
 }
@@ -837,9 +835,9 @@ void __init create_domUs(void)
     dt_for_each_child_node(chosen, node)
     {
         const char *llc_colors_str = NULL;
-        struct domain *d;
-        struct xen_domctl_createdomain d_cfg = {0};
-        unsigned int flags = 0U;
+        struct kernel_info ki = KERNEL_INFO_INIT;
+        struct xen_domctl_createdomain *d_cfg = &ki.bd.create_cfg;
+        unsigned int *flags = &ki.bd.create_flags;
         bool has_dtb = false;
         uint32_t val;
         int rc;
@@ -850,10 +848,10 @@ void __init create_domUs(void)
         if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
             panic("No more domain IDs available\n");
 
-        d_cfg.max_evtchn_port = 1023;
-        d_cfg.max_grant_frames = -1;
-        d_cfg.max_maptrack_frames = -1;
-        d_cfg.grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version);
+        d_cfg->max_evtchn_port = 1023;
+        d_cfg->max_grant_frames = -1;
+        d_cfg->max_maptrack_frames = -1;
+        d_cfg->grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version);
 
         if ( dt_property_read_u32(node, "capabilities", &val) )
         {
@@ -861,7 +859,7 @@ void __init create_domUs(void)
                 panic("Invalid capabilities (%"PRIx32")\n", val);
 
             if ( val & DOMAIN_CAPS_CONTROL )
-                flags |= CDF_privileged;
+                *flags |= CDF_privileged;
 
             if ( val & DOMAIN_CAPS_HARDWARE )
             {
@@ -870,17 +868,17 @@ void __init create_domUs(void)
                             hardware_domain);
 
 #ifdef CONFIG_GRANT_TABLE
-                d_cfg.max_grant_frames = gnttab_dom0_frames();
+                d_cfg->max_grant_frames = gnttab_dom0_frames();
 #endif
-                d_cfg.max_evtchn_port = -1;
-                flags |= CDF_hardware;
+                d_cfg->max_evtchn_port = -1;
+                *flags |= CDF_hardware;
                 iommu = true;
             }
 
             if ( val & DOMAIN_CAPS_XENSTORE )
             {
-                d_cfg.flags |= XEN_DOMCTL_CDF_xs_domain;
-                d_cfg.max_evtchn_port = -1;
+                d_cfg->flags |= XEN_DOMCTL_CDF_xs_domain;
+                d_cfg->max_evtchn_port = -1;
             }
         }
 
@@ -889,45 +887,45 @@ void __init create_domUs(void)
             if ( llc_coloring_enabled )
                 panic("LLC coloring and static memory are incompatible\n");
 
-            flags |= CDF_staticmem;
+            *flags |= CDF_staticmem;
         }
 
         if ( dt_property_read_bool(node, "direct-map") )
         {
-            if ( !(flags & CDF_staticmem) )
+            if ( !(*flags & CDF_staticmem) )
                 panic("direct-map is not valid for domain %s without static allocation.\n",
                       dt_node_name(node));
 
-            flags |= CDF_directmap;
+            *flags |= CDF_directmap;
         }
 
-        if ( !dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus) )
+        if ( !dt_property_read_u32(node, "cpus", &d_cfg->max_vcpus) )
             panic("Missing property 'cpus' for domain %s\n",
                   dt_node_name(node));
 
         if ( !dt_property_read_string(node, "passthrough", &dom0less_iommu) )
         {
-            if ( flags & CDF_hardware )
+            if ( *flags & CDF_hardware )
                 panic("Don't specify passthrough for hardware domain\n");
 
             if ( !strcmp(dom0less_iommu, "enabled") )
                 iommu = true;
         }
 
-        if ( (flags & CDF_hardware) && !(flags & CDF_directmap) &&
+        if ( (*flags & CDF_hardware) && !(*flags & CDF_directmap) &&
              !iommu_enabled )
             panic("non-direct mapped hardware domain requires iommu\n");
 
         if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") )
         {
-            if ( flags & CDF_hardware )
+            if ( *flags & CDF_hardware )
                 panic("\"multiboot,device-tree\" incompatible with hardware domain\n");
 
             has_dtb = true;
         }
 
         if ( iommu_enabled && (iommu || has_dtb) )
-            d_cfg.flags |= XEN_DOMCTL_CDF_iommu;
+            d_cfg->flags |= XEN_DOMCTL_CDF_iommu;
 
         /* Get the optional property domain-cpupool */
         cpupool_node = dt_parse_phandle(node, "domain-cpupool", 0);
@@ -937,57 +935,58 @@ void __init create_domUs(void)
             if ( pool_id < 0 )
                 panic("Error getting cpupool id from domain-cpupool (%d)\n",
                       pool_id);
-            d_cfg.cpupool_id = pool_id;
+            d_cfg->cpupool_id = pool_id;
         }
 
         if ( dt_property_read_u32(node, "max_grant_version", &val) )
-            d_cfg.grant_opts = XEN_DOMCTL_GRANT_version(val);
+            d_cfg->grant_opts = XEN_DOMCTL_GRANT_version(val);
 
         if ( dt_property_read_u32(node, "max_grant_frames", &val) )
         {
             if ( val > INT32_MAX )
                 panic("max_grant_frames (%"PRIu32") overflow\n", val);
-            d_cfg.max_grant_frames = val;
+            d_cfg->max_grant_frames = val;
         }
 
         if ( dt_property_read_u32(node, "max_maptrack_frames", &val) )
         {
             if ( val > INT32_MAX )
                 panic("max_maptrack_frames (%"PRIu32") overflow\n", val);
-            d_cfg.max_maptrack_frames = val;
+            d_cfg->max_maptrack_frames = val;
         }
 
         dt_property_read_string(node, "llc-colors", &llc_colors_str);
         if ( !llc_coloring_enabled && llc_colors_str )
             panic("'llc-colors' found, but LLC coloring is disabled\n");
 
-        arch_create_domUs(node, &d_cfg, flags);
+        arch_create_domUs(node, d_cfg, *flags);
 
         /*
          * The variable max_init_domid is initialized with zero, so here it's
          * very important to use the pre-increment operator to call
          * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
          */
-        d = domain_create(++max_init_domid, &d_cfg, flags);
-        if ( IS_ERR(d) )
+        ki.bd.d = domain_create(++max_init_domid,
+                                &ki.bd.create_cfg, ki.bd.create_flags);
+        if ( IS_ERR(ki.bd.d) )
             panic("Error creating domain %s (rc = %ld)\n",
-                  dt_node_name(node), PTR_ERR(d));
+                  dt_node_name(node), PTR_ERR(ki.bd.d));
 
         if ( llc_coloring_enabled &&
-             (rc = domain_set_llc_colors_from_str(d, llc_colors_str)) )
+             (rc = domain_set_llc_colors_from_str(ki.bd.d, llc_colors_str)) )
             panic("Error initializing LLC coloring for domain %s (rc = %d)\n",
                   dt_node_name(node), rc);
 
-        d->is_console = true;
-        dt_device_set_used_by(node, d->domain_id);
+        ki.bd.d->is_console = true;
+        dt_device_set_used_by(node, ki.bd.d->domain_id);
 
-        rc = construct_domU(d, node);
+        rc = construct_domU(&ki, node);
         if ( rc )
             panic("Could not set up domain %s (rc = %d)\n",
                   dt_node_name(node), rc);
 
-        if ( d_cfg.flags & XEN_DOMCTL_CDF_xs_domain )
-            set_xs_domain(d);
+        if ( ki.bd.create_cfg.flags & XEN_DOMCTL_CDF_xs_domain )
+            set_xs_domain(ki.bd.d);
     }
 
     if ( need_xenstore && xs_domid == DOMID_INVALID )
-- 
2.43.0

[PATCH 06/10] dom0less: Create llc_color_str field in boot_domain

[Alejandro Vallejo]

Later patches move the bindings to a separate function and expect the
outputs to land in fields of a boot_domain. Adjust llc_color_str to
live inside boot_domain so it can be parsed later on.

Not a functional change.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/common/device-tree/dom0less-build.c | 12 ++++++++----
 xen/include/xen/bootfdt.h               |  5 +++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index cee666786e..e1d723c796 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -834,7 +834,6 @@ void __init create_domUs(void)
     BUG_ON(chosen == NULL);
     dt_for_each_child_node(chosen, node)
     {
-        const char *llc_colors_str = NULL;
         struct kernel_info ki = KERNEL_INFO_INIT;
         struct xen_domctl_createdomain *d_cfg = &ki.bd.create_cfg;
         unsigned int *flags = &ki.bd.create_flags;
@@ -955,9 +954,11 @@ void __init create_domUs(void)
             d_cfg->max_maptrack_frames = val;
         }
 
-        dt_property_read_string(node, "llc-colors", &llc_colors_str);
-        if ( !llc_coloring_enabled && llc_colors_str )
+#ifdef CONFIG_HAS_LLC_COLORING
+        dt_property_read_string(node, "llc-colors", &ki.bd.llc_colors_str);
+        if ( !llc_coloring_enabled && ki.bd.llc_colors_str )
             panic("'llc-colors' found, but LLC coloring is disabled\n");
+#endif
 
         arch_create_domUs(node, d_cfg, *flags);
 
@@ -972,10 +973,13 @@ void __init create_domUs(void)
             panic("Error creating domain %s (rc = %ld)\n",
                   dt_node_name(node), PTR_ERR(ki.bd.d));
 
+#ifdef CONFIG_HAS_LLC_COLORING
         if ( llc_coloring_enabled &&
-             (rc = domain_set_llc_colors_from_str(ki.bd.d, llc_colors_str)) )
+             (rc = domain_set_llc_colors_from_str(ki.bd.d,
+                                                  ki.bd.llc_colors_str)) )
             panic("Error initializing LLC coloring for domain %s (rc = %d)\n",
                   dt_node_name(node), rc);
+#endif /* CONFIG_HAS_LLC_COLORING */
 
         ki.bd.d->is_console = true;
         dt_device_set_used_by(node, ki.bd.d->domain_id);
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index f107099263..0e82ccea2f 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -125,6 +125,11 @@ struct boot_domain {
     /* Input arguments to create_domain() */
     struct xen_domctl_createdomain create_cfg;
     unsigned int create_flags;
+
+#ifdef CONFIG_HAS_LLC_COLORING
+    /* LLC color selection string */
+    const char *llc_colors_str;
+#endif
 };
 
 #define BOOTMOD_MAX_CMDLINE 1024
-- 
2.43.0

[PATCH 07/10] dom0less: Turn arch_create_domUs into arch_parse_dom0less_node()

[Alejandro Vallejo]

It deals with a single domain, and will be called on a later patch
by a new function parse_dom0less_node(), so the new name is apt.

Also, pass parameters using boot_domain instead as the plan is to use it
as dumping gound for all the extracted information from the bindings.

Not a functional change.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/arm/dom0less-build.c           | 7 ++++---
 xen/common/device-tree/dom0less-build.c | 2 +-
 xen/include/xen/dom0less-build.h        | 5 ++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 286557fafd..3dea56dc76 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -272,10 +272,11 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
     return rc;
 }
 
-void __init arch_create_domUs(struct dt_device_node *node,
-                       struct xen_domctl_createdomain *d_cfg,
-                       unsigned int flags)
+void __init arch_parse_dom0less_node(struct dt_device_node *node,
+                                     struct boot_domain *bd)
 {
+    struct xen_domctl_createdomain *d_cfg = &bd->create_cfg;
+    unsigned int flags = bd->create_flags;
     uint32_t val;
 
     d_cfg->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index e1d723c796..8caceb9f6b 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -960,7 +960,7 @@ void __init create_domUs(void)
             panic("'llc-colors' found, but LLC coloring is disabled\n");
 #endif
 
-        arch_create_domUs(node, d_cfg, *flags);
+        arch_parse_dom0less_node(node, &ki.bd);
 
         /*
          * The variable max_init_domid is initialized with zero, so here it's
diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
index 34b9884cc7..8f3f90ae2a 100644
--- a/xen/include/xen/dom0less-build.h
+++ b/xen/include/xen/dom0less-build.h
@@ -45,9 +45,8 @@ void create_domUs(void);
 bool is_dom0less_mode(void);
 void set_xs_domain(struct domain *d);
 
-void arch_create_domUs(struct dt_device_node *node,
-                       struct xen_domctl_createdomain *d_cfg,
-                       unsigned int flags);
+void arch_parse_dom0less_node(struct dt_device_node *node,
+                              struct boot_domain *bd);
 
 int init_vuart(struct domain *d, struct kernel_info *kinfo,
                const struct dt_device_node *node);
-- 
2.43.0

[PATCH 08/10] dom0less: Allow arch_parse_dom0less_node() to be fallible

[Alejandro Vallejo]

Let the function return an errno, so fallible bindings are not precluded.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/arm/dom0less-build.c           | 6 ++++--
 xen/common/device-tree/dom0less-build.c | 3 ++-
 xen/include/xen/dom0less-build.h        | 6 +++---
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 3dea56dc76..c8d07213e2 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -272,8 +272,8 @@ int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
     return rc;
 }
 
-void __init arch_parse_dom0less_node(struct dt_device_node *node,
-                                     struct boot_domain *bd)
+int __init arch_parse_dom0less_node(struct dt_device_node *node,
+                                    struct boot_domain *bd)
 {
     struct xen_domctl_createdomain *d_cfg = &bd->create_cfg;
     unsigned int flags = bd->create_flags;
@@ -354,6 +354,8 @@ void __init arch_parse_dom0less_node(struct dt_device_node *node,
         if ( !val )
             d_cfg->flags &= ~XEN_DOMCTL_CDF_trap_unmapped_accesses;
     }
+
+    return 0;
 }
 
 int __init init_intc_phandle(struct kernel_info *kinfo, const char *name,
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 8caceb9f6b..ef4b095d97 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -960,7 +960,8 @@ void __init create_domUs(void)
             panic("'llc-colors' found, but LLC coloring is disabled\n");
 #endif
 
-        arch_parse_dom0less_node(node, &ki.bd);
+        if ( (rc = arch_parse_dom0less_node(node, &ki.bd)) )
+            panic("error parsing arch-specific dom0less props (rc=%d)", rc);
 
         /*
          * The variable max_init_domid is initialized with zero, so here it's
diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
index 8f3f90ae2a..72ca8f5e6d 100644
--- a/xen/include/xen/dom0less-build.h
+++ b/xen/include/xen/dom0less-build.h
@@ -9,7 +9,7 @@ struct domain;
 
 #ifdef CONFIG_DOM0LESS_BOOT
 
-struct xen_domctl_createdomain;
+struct boot_domain;
 struct dt_device_node;
 struct kernel_info;
 
@@ -45,8 +45,8 @@ void create_domUs(void);
 bool is_dom0less_mode(void);
 void set_xs_domain(struct domain *d);
 
-void arch_parse_dom0less_node(struct dt_device_node *node,
-                              struct boot_domain *bd);
+int arch_parse_dom0less_node(struct dt_device_node *node,
+                             struct boot_domain *bd);
 
 int init_vuart(struct domain *d, struct kernel_info *kinfo,
                const struct dt_device_node *node);
-- 
2.43.0

[PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Alejandro Vallejo]

Reduce the scope of every variable so they are reinitialised. "iommu",
for instance, isn't being cleared, so the wrong flags may make it to
domains that should not have them.

Fixes: 1d2b4f3049fd("xen/arm, doc: Add a DT property to specify...")
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
This is implicitly fixed in the next patch, but I'm sending this
standalone so it can be backported where relevant.
---
 xen/common/device-tree/dom0less-build.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index ef4b095d97..676a3317cf 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -826,14 +826,14 @@ static int __init construct_domU(struct kernel_info *kinfo,
 void __init create_domUs(void)
 {
     struct dt_device_node *node;
-    const char *dom0less_iommu;
-    bool iommu = false;
-    const struct dt_device_node *cpupool_node,
-                                *chosen = dt_find_node_by_path("/chosen");
+    const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
 
     BUG_ON(chosen == NULL);
     dt_for_each_child_node(chosen, node)
     {
+        const char *dom0less_iommu;
+        bool iommu = false;
+        const struct dt_device_node *cpupool_node;
         struct kernel_info ki = KERNEL_INFO_INIT;
         struct xen_domctl_createdomain *d_cfg = &ki.bd.create_cfg;
         unsigned int *flags = &ki.bd.create_flags;
-- 
2.43.0

[PATCH 10/10] dom0less: Parse dom0less bindings into createdomain input args

[Alejandro Vallejo]

The builder in common code already does this, but it's not callable
independently from a separate location. Create a function x86 can
call to use its own domain builder, using createdomain arguments
as the parsed data.

The bindings are moved on the next patch so it's strict code motion.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/common/device-tree/Makefile            |   3 +-
 xen/common/device-tree/dom0less-bindings.c | 145 +++++++++++++++++++++
 xen/common/device-tree/dom0less-build.c    | 129 +-----------------
 xen/include/xen/dom0less-build.h           |   3 +
 4 files changed, 154 insertions(+), 126 deletions(-)
 create mode 100644 xen/common/device-tree/dom0less-bindings.c

diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
index e399242cdf..9036e455d6 100644
--- a/xen/common/device-tree/Makefile
+++ b/xen/common/device-tree/Makefile
@@ -3,7 +3,8 @@ obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += bootinfo-fdt.init.o
 obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += bootinfo.init.o
 obj-y += device-tree.o
 obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += domain-build.init.o
-obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
+obj-$(filter $(CONFIG_DOM0LESS_BOOT),$(CONFIG_HAS_DEVICE_TREE_DISCOVERY)) += dom0less-build.init.o
+obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-bindings.init.o
 obj-$(CONFIG_OVERLAY_DTB) += dt-overlay.o
 obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += intc.o
 obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += kernel.o
diff --git a/xen/common/device-tree/dom0less-bindings.c b/xen/common/device-tree/dom0less-bindings.c
new file mode 100644
index 0000000000..41d72d0d58
--- /dev/null
+++ b/xen/common/device-tree/dom0less-bindings.c
@@ -0,0 +1,145 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/bootfdt.h>
+#include <xen/device_tree.h>
+#include <xen/dom0less-build.h>
+#include <xen/domain.h>
+#include <xen/grant_table.h>
+#include <xen/llc-coloring.h>
+#include <xen/sched.h>
+
+#include <public/bootfdt.h>
+#include <public/domctl.h>
+
+int __init parse_dom0less_node(struct dt_device_node *node,
+                               struct boot_domain *bd)
+{
+    struct xen_domctl_createdomain *d_cfg = &bd->create_cfg;
+    unsigned int *flags = &bd->create_flags;
+    struct dt_device_node *cpupool_node;
+    uint32_t val;
+    bool has_dtb = false;
+    bool iommu = false;
+    const char *dom0less_iommu = NULL;
+
+    if ( !dt_device_is_compatible(node, "xen,domain") )
+        return -ENOENT;
+
+    *flags = 0;
+    *d_cfg = (struct xen_domctl_createdomain){
+        .max_evtchn_port = 1023,
+        .max_grant_frames = -1,
+        .max_maptrack_frames = -1,
+        .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
+    };
+
+    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);
+
+#ifdef CONFIG_GRANT_TABLE
+            d_cfg->max_grant_frames = gnttab_dom0_frames();
+#endif
+            d_cfg->max_evtchn_port = -1;
+            *flags |= CDF_hardware;
+            iommu = true;
+        }
+
+        if ( val & DOMAIN_CAPS_XENSTORE )
+        {
+            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 )
+            panic("LLC coloring and static memory are incompatible\n");
+
+        *flags |= CDF_staticmem;
+    }
+
+    if ( dt_property_read_bool(node, "direct-map") )
+    {
+        if ( !(*flags & CDF_staticmem) )
+            panic("direct-map is not valid for domain %s without static allocation.\n",
+                  dt_node_name(node));
+
+        *flags |= CDF_directmap;
+    }
+
+    if ( !dt_property_read_u32(node, "cpus", &d_cfg->max_vcpus) )
+        panic("Missing property 'cpus' for domain %s\n",
+              dt_node_name(node));
+
+    if ( !dt_property_read_string(node, "passthrough", &dom0less_iommu) )
+    {
+        if ( *flags & CDF_hardware )
+            panic("Don't specify passthrough for hardware domain\n");
+
+        if ( !strcmp(dom0less_iommu, "enabled") )
+            iommu = true;
+    }
+
+    if ( (*flags & CDF_hardware) && !(*flags & CDF_directmap) &&
+         !iommu_enabled )
+        panic("non-direct mapped hardware domain requires iommu\n");
+
+    if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") )
+    {
+        if ( *flags & CDF_hardware )
+            panic("\"multiboot,device-tree\" incompatible with hardware domain\n");
+
+        has_dtb = true;
+    }
+
+    if ( iommu_enabled && (iommu || has_dtb) )
+        d_cfg->flags |= XEN_DOMCTL_CDF_iommu;
+
+    /* Get the optional property domain-cpupool */
+    cpupool_node = dt_parse_phandle(node, "domain-cpupool", 0);
+    if ( cpupool_node )
+    {
+        int pool_id = btcpupools_get_domain_pool_id(cpupool_node);
+        if ( pool_id < 0 )
+            panic("Error getting cpupool id from domain-cpupool (%d)\n",
+                  pool_id);
+        d_cfg->cpupool_id = pool_id;
+    }
+
+    if ( dt_property_read_u32(node, "max_grant_version", &val) )
+        d_cfg->grant_opts = XEN_DOMCTL_GRANT_version(val);
+
+    if ( dt_property_read_u32(node, "max_grant_frames", &val) )
+    {
+        if ( val > INT32_MAX )
+            panic("max_grant_frames (%"PRIu32") overflow\n", val);
+        d_cfg->max_grant_frames = val;
+    }
+
+    if ( dt_property_read_u32(node, "max_maptrack_frames", &val) )
+    {
+        if ( val > INT32_MAX )
+            panic("max_maptrack_frames (%"PRIu32") overflow\n", val);
+        d_cfg->max_maptrack_frames = val;
+    }
+
+#ifdef CONFIG_HAS_LLC_COLORING
+    dt_property_read_string(node, "llc-colors", &bd->llc_colors_str);
+    if ( !llc_coloring_enabled && bd->llc_colors_str )
+        panic("'llc-colors' found, but LLC coloring is disabled\n");
+#endif
+
+    return arch_parse_dom0less_node(node, bd);
+}
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 676a3317cf..6bb038111d 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -831,138 +831,17 @@ void __init create_domUs(void)
     BUG_ON(chosen == NULL);
     dt_for_each_child_node(chosen, node)
     {
-        const char *dom0less_iommu;
-        bool iommu = false;
-        const struct dt_device_node *cpupool_node;
         struct kernel_info ki = KERNEL_INFO_INIT;
-        struct xen_domctl_createdomain *d_cfg = &ki.bd.create_cfg;
-        unsigned int *flags = &ki.bd.create_flags;
-        bool has_dtb = false;
-        uint32_t val;
-        int rc;
+        int rc = parse_dom0less_node(node, &ki.bd);
 
-        if ( !dt_device_is_compatible(node, "xen,domain") )
+        if ( rc == -ENOENT )
             continue;
+        if ( rc )
+            panic("Malformed DTB: Invalid domain %s\n", dt_node_name(node));
 
         if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
             panic("No more domain IDs available\n");
 
-        d_cfg->max_evtchn_port = 1023;
-        d_cfg->max_grant_frames = -1;
-        d_cfg->max_maptrack_frames = -1;
-        d_cfg->grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version);
-
-        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);
-
-#ifdef CONFIG_GRANT_TABLE
-                d_cfg->max_grant_frames = gnttab_dom0_frames();
-#endif
-                d_cfg->max_evtchn_port = -1;
-                *flags |= CDF_hardware;
-                iommu = true;
-            }
-
-            if ( val & DOMAIN_CAPS_XENSTORE )
-            {
-                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 )
-                panic("LLC coloring and static memory are incompatible\n");
-
-            *flags |= CDF_staticmem;
-        }
-
-        if ( dt_property_read_bool(node, "direct-map") )
-        {
-            if ( !(*flags & CDF_staticmem) )
-                panic("direct-map is not valid for domain %s without static allocation.\n",
-                      dt_node_name(node));
-
-            *flags |= CDF_directmap;
-        }
-
-        if ( !dt_property_read_u32(node, "cpus", &d_cfg->max_vcpus) )
-            panic("Missing property 'cpus' for domain %s\n",
-                  dt_node_name(node));
-
-        if ( !dt_property_read_string(node, "passthrough", &dom0less_iommu) )
-        {
-            if ( *flags & CDF_hardware )
-                panic("Don't specify passthrough for hardware domain\n");
-
-            if ( !strcmp(dom0less_iommu, "enabled") )
-                iommu = true;
-        }
-
-        if ( (*flags & CDF_hardware) && !(*flags & CDF_directmap) &&
-             !iommu_enabled )
-            panic("non-direct mapped hardware domain requires iommu\n");
-
-        if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") )
-        {
-            if ( *flags & CDF_hardware )
-                panic("\"multiboot,device-tree\" incompatible with hardware domain\n");
-
-            has_dtb = true;
-        }
-
-        if ( iommu_enabled && (iommu || has_dtb) )
-            d_cfg->flags |= XEN_DOMCTL_CDF_iommu;
-
-        /* Get the optional property domain-cpupool */
-        cpupool_node = dt_parse_phandle(node, "domain-cpupool", 0);
-        if ( cpupool_node )
-        {
-            int pool_id = btcpupools_get_domain_pool_id(cpupool_node);
-            if ( pool_id < 0 )
-                panic("Error getting cpupool id from domain-cpupool (%d)\n",
-                      pool_id);
-            d_cfg->cpupool_id = pool_id;
-        }
-
-        if ( dt_property_read_u32(node, "max_grant_version", &val) )
-            d_cfg->grant_opts = XEN_DOMCTL_GRANT_version(val);
-
-        if ( dt_property_read_u32(node, "max_grant_frames", &val) )
-        {
-            if ( val > INT32_MAX )
-                panic("max_grant_frames (%"PRIu32") overflow\n", val);
-            d_cfg->max_grant_frames = val;
-        }
-
-        if ( dt_property_read_u32(node, "max_maptrack_frames", &val) )
-        {
-            if ( val > INT32_MAX )
-                panic("max_maptrack_frames (%"PRIu32") overflow\n", val);
-            d_cfg->max_maptrack_frames = val;
-        }
-
-#ifdef CONFIG_HAS_LLC_COLORING
-        dt_property_read_string(node, "llc-colors", &ki.bd.llc_colors_str);
-        if ( !llc_coloring_enabled && ki.bd.llc_colors_str )
-            panic("'llc-colors' found, but LLC coloring is disabled\n");
-#endif
-
-        if ( (rc = arch_parse_dom0less_node(node, &ki.bd)) )
-            panic("error parsing arch-specific dom0less props (rc=%d)", rc);
-
         /*
          * The variable max_init_domid is initialized with zero, so here it's
          * very important to use the pre-increment operator to call
diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
index 72ca8f5e6d..408859e325 100644
--- a/xen/include/xen/dom0less-build.h
+++ b/xen/include/xen/dom0less-build.h
@@ -45,6 +45,9 @@ void create_domUs(void);
 bool is_dom0less_mode(void);
 void set_xs_domain(struct domain *d);
 
+int parse_dom0less_node(struct dt_device_node *node,
+                        struct boot_domain *bd);
+
 int arch_parse_dom0less_node(struct dt_device_node *node,
                              struct boot_domain *bd);
 
-- 
2.43.0

Re: [PATCH 01/10] static-evtchn: Add missing include

[Jan Beulich]

On 22.07.2025 13:59, Alejandro Vallejo wrote:
> From: Alejandro Vallejo <agarciav@amd.com>
> 
> When later on x86 starts using this file in later patches it won't find
> device_tree.h because it's only transitively included by arm.
> 
> Make it explicit.
> 
> Not a functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

I'm certainly happy to trust you, but on a v1 submission I'm a little
surprised to find two tags present already.

Jan

Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Jan Beulich]

On 22.07.2025 13:59, Alejandro Vallejo wrote:
> Reduce the scope of every variable so they are reinitialised. "iommu",
> for instance, isn't being cleared, so the wrong flags may make it to
> domains that should not have them.

Yet "for instance" isn't quite right, is it? "iommu" is the only one where
the (re)init was misplaced. The other two ...

> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -826,14 +826,14 @@ static int __init construct_domU(struct kernel_info *kinfo,
>  void __init create_domUs(void)
>  {
>      struct dt_device_node *node;
> -    const char *dom0less_iommu;
> -    bool iommu = false;
> -    const struct dt_device_node *cpupool_node,
> -                                *chosen = dt_find_node_by_path("/chosen");
> +    const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
>  
>      BUG_ON(chosen == NULL);
>      dt_for_each_child_node(chosen, node)
>      {
> +        const char *dom0less_iommu;
> +        bool iommu = false;
> +        const struct dt_device_node *cpupool_node;

... had no initializer, and also don't gain any. So they must both be
set inside the loop. (Irrespective, the scope reduction is a good thing
imo.)

Jan

Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Alejandro Vallejo]

On Tue Jul 22, 2025 at 2:18 PM CEST, Jan Beulich wrote:
> On 22.07.2025 13:59, Alejandro Vallejo wrote:
>> Reduce the scope of every variable so they are reinitialised. "iommu",
>> for instance, isn't being cleared, so the wrong flags may make it to
>> domains that should not have them.
>
> Yet "for instance" isn't quite right, is it? "iommu" is the only one where
> the (re)init was misplaced. The other two ...

We do strive for minimal scope where possible. But you're right "for instance"
might be misleading in suggesting there's more bugs than one.

I'm happy to have "for instance" removed, leaving the rest as-is, if that works
for you.

>
>> --- a/xen/common/device-tree/dom0less-build.c
>> +++ b/xen/common/device-tree/dom0less-build.c
>> @@ -826,14 +826,14 @@ static int __init construct_domU(struct kernel_info *kinfo,
>>  void __init create_domUs(void)
>>  {
>>      struct dt_device_node *node;
>> -    const char *dom0less_iommu;
>> -    bool iommu = false;
>> -    const struct dt_device_node *cpupool_node,
>> -                                *chosen = dt_find_node_by_path("/chosen");
>> +    const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
>>  
>>      BUG_ON(chosen == NULL);
>>      dt_for_each_child_node(chosen, node)
>>      {
>> +        const char *dom0less_iommu;
>> +        bool iommu = false;
>> +        const struct dt_device_node *cpupool_node;
>
> ... had no initializer, and also don't gain any. So they must both be
> set inside the loop. (Irrespective, the scope reduction is a good thing
> imo.)
>
> Jan

Cheers,
Alejandro

Re: [PATCH 01/10] static-evtchn: Add missing include

[Alejandro Vallejo]

On Tue Jul 22, 2025 at 2:10 PM CEST, Jan Beulich wrote:
> On 22.07.2025 13:59, Alejandro Vallejo wrote:
>> From: Alejandro Vallejo <agarciav@amd.com>
>> 
>> When later on x86 starts using this file in later patches it won't find
>> device_tree.h because it's only transitively included by arm.
>> 
>> Make it explicit.
>> 
>> Not a functional change.
>> 
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
>> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
>
> I'm certainly happy to trust you, but on a v1 submission I'm a little
> surprised to find two tags present already.
>
> Jan

They came out of internal review, and I didn't really want to drop them.

Everyone with tags is in CC, as auto-cc "helpfully" does in the absence of
further arguments (for better or worse), so they can pitch in.

If this is a problem, I'll make a note to drop internal tags before public
submissions in the future, and feel very free to ignore these. I do understand
it's important to keep proper paper trail, which, by doing this, is lost.

Cheers,
Alejandro

Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Jan Beulich]

On 22.07.2025 14:37, Alejandro Vallejo wrote:
> On Tue Jul 22, 2025 at 2:18 PM CEST, Jan Beulich wrote:
>> On 22.07.2025 13:59, Alejandro Vallejo wrote:
>>> Reduce the scope of every variable so they are reinitialised. "iommu",
>>> for instance, isn't being cleared, so the wrong flags may make it to
>>> domains that should not have them.
>>
>> Yet "for instance" isn't quite right, is it? "iommu" is the only one where
>> the (re)init was misplaced. The other two ...
> 
> We do strive for minimal scope where possible. But you're right "for instance"
> might be misleading in suggesting there's more bugs than one.
> 
> I'm happy to have "for instance" removed, leaving the rest as-is, if that works
> for you.

Except that "every" isn't quite right either. Nor is "they".

Jan

>>> --- a/xen/common/device-tree/dom0less-build.c
>>> +++ b/xen/common/device-tree/dom0less-build.c
>>> @@ -826,14 +826,14 @@ static int __init construct_domU(struct kernel_info *kinfo,
>>>  void __init create_domUs(void)
>>>  {
>>>      struct dt_device_node *node;
>>> -    const char *dom0less_iommu;
>>> -    bool iommu = false;
>>> -    const struct dt_device_node *cpupool_node,
>>> -                                *chosen = dt_find_node_by_path("/chosen");
>>> +    const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
>>>  
>>>      BUG_ON(chosen == NULL);
>>>      dt_for_each_child_node(chosen, node)
>>>      {
>>> +        const char *dom0less_iommu;
>>> +        bool iommu = false;
>>> +        const struct dt_device_node *cpupool_node;
>>
>> ... had no initializer, and also don't gain any. So they must both be
>> set inside the loop. (Irrespective, the scope reduction is a good thing
>> imo.)
>>
>> Jan
> 
> Cheers,
> Alejandro

Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Alejandro Vallejo]

On Tue Jul 22, 2025 at 2:57 PM CEST, Jan Beulich wrote:
> On 22.07.2025 14:37, Alejandro Vallejo wrote:
>> On Tue Jul 22, 2025 at 2:18 PM CEST, Jan Beulich wrote:
>>> On 22.07.2025 13:59, Alejandro Vallejo wrote:
>>>> Reduce the scope of every variable so they are reinitialised. "iommu",
>>>> for instance, isn't being cleared, so the wrong flags may make it to
>>>> domains that should not have them.
>>>
>>> Yet "for instance" isn't quite right, is it? "iommu" is the only one where
>>> the (re)init was misplaced. The other two ...
>> 
>> We do strive for minimal scope where possible. But you're right "for instance"
>> might be misleading in suggesting there's more bugs than one.
>> 
>> I'm happy to have "for instance" removed, leaving the rest as-is, if that works
>> for you.
>
> Except that "every" isn't quite right either. Nor is "they".
>
> Jan

Ok, take 3:

	Reduce the scope of dom0less_iommu, iommu and cpupool_node. iommu, in
	particular, wasn't being cleared, so the wrong flags may make it to
	domains that should not have them.

Cheers,
Alejandro

Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Jan Beulich]

On 22.07.2025 15:31, Alejandro Vallejo wrote:
> On Tue Jul 22, 2025 at 2:57 PM CEST, Jan Beulich wrote:
>> On 22.07.2025 14:37, Alejandro Vallejo wrote:
>>> On Tue Jul 22, 2025 at 2:18 PM CEST, Jan Beulich wrote:
>>>> On 22.07.2025 13:59, Alejandro Vallejo wrote:
>>>>> Reduce the scope of every variable so they are reinitialised. "iommu",
>>>>> for instance, isn't being cleared, so the wrong flags may make it to
>>>>> domains that should not have them.
>>>>
>>>> Yet "for instance" isn't quite right, is it? "iommu" is the only one where
>>>> the (re)init was misplaced. The other two ...
>>>
>>> We do strive for minimal scope where possible. But you're right "for instance"
>>> might be misleading in suggesting there's more bugs than one.
>>>
>>> I'm happy to have "for instance" removed, leaving the rest as-is, if that works
>>> for you.
>>
>> Except that "every" isn't quite right either. Nor is "they".
> 
> Ok, take 3:
> 
> 	Reduce the scope of dom0less_iommu, iommu and cpupool_node. iommu, in
> 	particular, wasn't being cleared, so the wrong flags may make it to
> 	domains that should not have them.

Fine with me, thanks.

Jan

Re: [PATCH 01/10] static-evtchn: Add missing include

[Stefano Stabellini]

On Tue, 22 Jul 2025, Alejandro Vallejo wrote:
> On Tue Jul 22, 2025 at 2:10 PM CEST, Jan Beulich wrote:
> > On 22.07.2025 13:59, Alejandro Vallejo wrote:
> >> From: Alejandro Vallejo <agarciav@amd.com>
> >> 
> >> When later on x86 starts using this file in later patches it won't find
> >> device_tree.h because it's only transitively included by arm.
> >> 
> >> Make it explicit.
> >> 
> >> Not a functional change.
> >> 
> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> >> Acked-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
> >
> > I'm certainly happy to trust you, but on a v1 submission I'm a little
> > surprised to find two tags present already.
> >
> > Jan
> 
> They came out of internal review, and I didn't really want to drop them.

I confirm

Re: [PATCH 03/10] dom0less: Move asm-generic/dom0less-build.h -> xen/dom0less-build.h

[Stefano Stabellini]

On Tue, 22 Jul 2025, Alejandro Vallejo wrote:
> It's meant to be usable by anyone with CONFIG_DOM0LESS_BOOT.
> 
> While moving, replace an inclusion of public/domctl.h by a forward
> declaration.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [PATCH 06/10] dom0less: Create llc_color_str field in boot_domain

[Stefano Stabellini]

On Tue, 22 Jul 2025, Alejandro Vallejo wrote:
> Later patches move the bindings to a separate function and expect the
> outputs to land in fields of a boot_domain. Adjust llc_color_str to
> live inside boot_domain so it can be parsed later on.
> 
> Not a functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/common/device-tree/dom0less-build.c | 12 ++++++++----
>  xen/include/xen/bootfdt.h               |  5 +++++
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
> index cee666786e..e1d723c796 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -834,7 +834,6 @@ void __init create_domUs(void)
>      BUG_ON(chosen == NULL);
>      dt_for_each_child_node(chosen, node)
>      {
> -        const char *llc_colors_str = NULL;
>          struct kernel_info ki = KERNEL_INFO_INIT;
>          struct xen_domctl_createdomain *d_cfg = &ki.bd.create_cfg;
>          unsigned int *flags = &ki.bd.create_flags;
> @@ -955,9 +954,11 @@ void __init create_domUs(void)
>              d_cfg->max_maptrack_frames = val;
>          }
>  
> -        dt_property_read_string(node, "llc-colors", &llc_colors_str);
> -        if ( !llc_coloring_enabled && llc_colors_str )
> +#ifdef CONFIG_HAS_LLC_COLORING
> +        dt_property_read_string(node, "llc-colors", &ki.bd.llc_colors_str);
> +        if ( !llc_coloring_enabled && ki.bd.llc_colors_str )
>              panic("'llc-colors' found, but LLC coloring is disabled\n");
> +#endif
>  
>          arch_create_domUs(node, d_cfg, *flags);
>  
> @@ -972,10 +973,13 @@ void __init create_domUs(void)
>              panic("Error creating domain %s (rc = %ld)\n",
>                    dt_node_name(node), PTR_ERR(ki.bd.d));
>  
> +#ifdef CONFIG_HAS_LLC_COLORING
>          if ( llc_coloring_enabled &&
> -             (rc = domain_set_llc_colors_from_str(ki.bd.d, llc_colors_str)) )
> +             (rc = domain_set_llc_colors_from_str(ki.bd.d,
> +                                                  ki.bd.llc_colors_str)) )
>              panic("Error initializing LLC coloring for domain %s (rc = %d)\n",
>                    dt_node_name(node), rc);
> +#endif /* CONFIG_HAS_LLC_COLORING */
>  
>          ki.bd.d->is_console = true;
>          dt_device_set_used_by(node, ki.bd.d->domain_id);
> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> index f107099263..0e82ccea2f 100644
> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -125,6 +125,11 @@ struct boot_domain {
>      /* Input arguments to create_domain() */
>      struct xen_domctl_createdomain create_cfg;
>      unsigned int create_flags;
> +
> +#ifdef CONFIG_HAS_LLC_COLORING
> +    /* LLC color selection string */
> +    const char *llc_colors_str;
> +#endif
>  };
>  
>  #define BOOTMOD_MAX_CMDLINE 1024
> -- 
> 2.43.0
> 

Re: [PATCH 08/10] dom0less: Allow arch_parse_dom0less_node() to be fallible

[Stefano Stabellini]

On Tue, 22 Jul 2025, Alejandro Vallejo wrote:
> Let the function return an errno, so fallible bindings are not precluded.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [PATCH 07/10] dom0less: Turn arch_create_domUs into arch_parse_dom0less_node()

[Stefano Stabellini]

On Tue, 22 Jul 2025, Alejandro Vallejo wrote:
> It deals with a single domain, and will be called on a later patch
> by a new function parse_dom0less_node(), so the new name is apt.
> 
> Also, pass parameters using boot_domain instead as the plan is to use it
> as dumping gound for all the extracted information from the bindings.
> 
> Not a functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Stefano Stabellini]

On Tue, 22 Jul 2025, Jan Beulich wrote:
> On 22.07.2025 15:31, Alejandro Vallejo wrote:
> > On Tue Jul 22, 2025 at 2:57 PM CEST, Jan Beulich wrote:
> >> On 22.07.2025 14:37, Alejandro Vallejo wrote:
> >>> On Tue Jul 22, 2025 at 2:18 PM CEST, Jan Beulich wrote:
> >>>> On 22.07.2025 13:59, Alejandro Vallejo wrote:
> >>>>> Reduce the scope of every variable so they are reinitialised. "iommu",
> >>>>> for instance, isn't being cleared, so the wrong flags may make it to
> >>>>> domains that should not have them.
> >>>>
> >>>> Yet "for instance" isn't quite right, is it? "iommu" is the only one where
> >>>> the (re)init was misplaced. The other two ...
> >>>
> >>> We do strive for minimal scope where possible. But you're right "for instance"
> >>> might be misleading in suggesting there's more bugs than one.
> >>>
> >>> I'm happy to have "for instance" removed, leaving the rest as-is, if that works
> >>> for you.
> >>
> >> Except that "every" isn't quite right either. Nor is "they".
> > 
> > Ok, take 3:
> > 
> > 	Reduce the scope of dom0less_iommu, iommu and cpupool_node. iommu, in
> > 	particular, wasn't being cleared, so the wrong flags may make it to
> > 	domains that should not have them.
> 
> Fine with me, thanks.

With Jan's suggestion:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>