[PATCH v1 0/3] dom0less: various updates

[PATCH v1 0/3] dom0less: various updates

[Oleksii Kurochko]

This patch series introduces a new field to track not-yet-used phandles as there
are some use cases where RISC-V needs to know which phandle number could
be used for generating a device tree node.

For example, on the RISC-V side in make_cpus_node() [1] it is necessary to know
which phandle number is unused to use it for device tree node generation.

Here is an example of generated guest DTB:
    cpus {
    ...
      cpu@0 {
    ...
        interrupt-controller {
          compatible = "riscv,cpu-intc";
          #interrupt-cells = <0x1>;
          interrupt-controller;
          phandle = <0xfdea>;
        };
      };
    };

    /soc/imsics@28000000 {

      interrupts-extended = <0xfdea 0x9 >;

      phandle = <0xfdeb>;
    };

    /soc/aplic@d000000 {
    ...
      msi-parent = <0xfdeb>;
      phandle = <0x1>;
    };

Note that phandles for imsic and riscv,cpu-intc are generated in this example
not by get_next_free_phandle(), that is why they have such big numbers.

For non-RISC-V people, APLIC is an interrupt controller (something like GIC in
Arm), IMSIC is an interrupt controller that provides MSI and connects to
each CPU.

So (based on the DTS above) for APLIC, kinfo->phandle_intc is reused, which
will also be re-used for the device node's interrupt property. For all others, I
just introduced GUEST_PHANDLE_LAST [2] and used it for generation [3]. But I expect
that it could be useful for other architectures too so I just moved it to common
and re-use pfdt to understand what the maximum used phandle is.

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/interrupt-controller/riscv%2Ccpu-intc.txt
[2] https://lore.kernel.org/xen-devel/ccd6d21b224b478c88ca5f2fdd2d1dd507671510.1773157782.git.oleksii.kurochko@gmail.com/
[3] https://lore.kernel.org/xen-devel/fd64b8526a23e9d7775b9b48c5a933b0673c4fba.1773157782.git.oleksii.kurochko@gmail.com/
           *************************************

Another thing introduced in this patch series is moving domain type to common
code as several architectures (ARM and RISC-V for now) use them and it
looks pretty architecture-independent. Also, is_64bit_domain() is used by
dom0less common code, so I found it useful also to move is_{32,64}bit_domain
macros to common code.

           *************************************

And the last thing is changing the prototype of make_cpus_node() to be aligned
with other make_*_node() and since RISC-V will need access to the free_phandle field
(even if it will be moved to kinfo->arch.free_phandle) and for the reason that
this ->free_phandle is updated in make_*_node(), the kinfo argument is passed as
non-const.

CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2457632124

Oleksii Kurochko (3):
  xen/dom0less: introduce free_phandle in struct kernel_info
  xen/dom0less: pass kernel_info struct instead of fdt to
    make_cpus_node()
  xen: introduce CONFIG_HAS_DOMAIN_TYPE

 xen/arch/Kconfig                        |  3 ++
 xen/arch/arm/Kconfig                    |  1 +
 xen/arch/arm/arm64/domctl.c             |  4 +-
 xen/arch/arm/dom0less-build.c           | 14 -------
 xen/arch/arm/domain_build.c             | 11 +++---
 xen/arch/arm/include/asm/domain.h       | 12 ------
 xen/arch/arm/include/asm/kernel.h       |  4 --
 xen/arch/arm/kernel.c                   | 10 ++---
 xen/common/device-tree/dom0less-build.c | 51 ++++++++++++++++++-------
 xen/include/xen/dom0less-build.h        |  2 -
 xen/include/xen/domain.h                | 13 +++++++
 xen/include/xen/fdt-domain-build.h      |  8 +++-
 xen/include/xen/fdt-kernel.h            |  8 ++++
 xen/include/xen/sched.h                 |  4 ++
 14 files changed, 86 insertions(+), 59 deletions(-)

-- 
2.53.0

[PATCH v1 1/3] xen/dom0less: introduce free_phandle in struct kernel_info

[Oleksii Kurochko]

There are cases where it is necessary to know the next available phandle
number in order to generate phandles for guest device nodes.

When a partial FDT (pfdt) is provided, special care is needed during
initialization of free_phandle, as the pfdt may already contain a dummy
interrupt controller node with a phandle assigned to it.  free_phandle
must therefore be initialized to one past the highest phandle already
present in the pfdt, to avoid collisions.

Since free_phandle may be needed for the very first guest node generated,
domain_handle_dtb_boot_module() is moved earlier in prepare_dtb_domU().
The new call site also aligns better with the existing comment stating
that domain_handle_dtb_boot_module() must be called before the rest of
the device tree is generated.

Introduce get_next_free_phandle() to ensure that phandles allocated for
guest nodes do not overlap the Xen-reserved phandle range.  This helper
will be used by subsequent patches (by RISC-V at the moment).

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/common/device-tree/dom0less-build.c | 44 ++++++++++++++++++-------
 xen/include/xen/fdt-domain-build.h      |  6 ++++
 xen/include/xen/fdt-kernel.h            |  3 ++
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 840d14419da2..9205f01f0a49 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -389,6 +389,24 @@ static int __init domain_handle_dtb_boot_module(struct domain *d,
     if ( res < 0 )
         goto out;
 
+    /*
+     * Find the highest phandle in the partial FDT so free_phandle starts
+     * above it, avoiding collisions with pfdt's own phandle assignments.
+     */
+    res = fdt_generate_phandle(pfdt, &kinfo->free_phandle);
+    if ( res )
+    {
+        res = (res == -FDT_ERR_NOPHANDLES) ? -EOVERFLOW : -EINVAL;
+        goto out;
+    }
+
+    if ( kinfo->free_phandle >= GUEST_PHANDLE_GIC )
+    {
+        dprintk(XENLOG_ERR, "Phandle allocation overlaps Xen reserved range\n");
+        res = -EOVERFLOW;
+        goto out;
+    }
+
     for ( node_next = fdt_first_subnode(pfdt, 0);
           node_next > 0;
           node_next = fdt_next_subnode(pfdt, node_next) )
@@ -459,6 +477,8 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
     BUILD_BUG_ON(DOMU_DTB_SIZE > SZ_2M);
 
     kinfo->phandle_intc = GUEST_PHANDLE_GIC;
+    kinfo->free_phandle = 1;
+    BUILD_BUG_ON(GUEST_PHANDLE_GIC == 1);
 
 #ifdef CONFIG_GRANT_TABLE
     kinfo->gnttab_start = GUEST_GNTTAB_BASE;
@@ -499,6 +519,18 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
     if ( ret )
         goto err;
 
+    /*
+     * domain_handle_dtb_boot_module() must be called before the rest of the
+     * device tree is generated because it sets phandle_intc and free_phandle,
+     * which subsequent node generation depends on.
+     */
+    if ( kinfo->dtb )
+    {
+        ret = domain_handle_dtb_boot_module(d, kinfo);
+        if ( ret )
+            goto err;
+    }
+
     ret = make_chosen_node(kinfo);
     if ( ret )
         goto err;
@@ -516,18 +548,6 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
     if ( ret )
         goto err;
 
-    /*
-     * domain_handle_dtb_boot_module has to be called before the rest of
-     * the device tree is generated because it depends on the value of
-     * the field phandle_intc.
-     */
-    if ( kinfo->dtb )
-    {
-        ret = domain_handle_dtb_boot_module(d, kinfo);
-        if ( ret )
-            goto err;
-    }
-
     ret = make_intc_domU_node(kinfo);
     if ( ret )
         goto err;
diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
index 1d9e77df0eb3..220ae46ddbe1 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -63,6 +63,12 @@ int find_unallocated_memory(const struct kernel_info *kinfo,
                                       unsigned long e_gfn,
                                       void *data));
 
+/* Return 0 (invalid phandle) if the Xen-reserved range has been reached */
+static inline uint32_t get_next_free_phandle(struct kernel_info *kinfo)
+{
+    return kinfo->free_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->free_phandle++;
+}
+
 #endif /* __XEN_FDT_DOMAIN_BUILD_H__ */
 
 /*
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index aa977a50f4fc..3527934b2a00 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -44,6 +44,9 @@ struct kernel_info {
     /* Interrupt controller phandle */
     uint32_t phandle_intc;
 
+    /* Next free phandle available for assigning to guest device nodes */
+    uint32_t free_phandle;
+
     /* loader to use for this kernel */
     void (*load)(struct kernel_info *info);
 
-- 
2.53.0

[PATCH v1 2/3] xen/dom0less: pass kernel_info struct instead of fdt to make_cpus_node()

[Oleksii Kurochko]

There are two reasons of this change:
1. Align prototype with what other make_*_node() are passed.
2. A follow-up RISC-V patch will call get_next_free_phandle() inside
   make_cpus_node(), requiring mutable access to kinfo->free_phandle.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/arm/domain_build.c             | 5 +++--
 xen/common/device-tree/dom0less-build.c | 2 +-
 xen/include/xen/fdt-domain-build.h      | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 60a7cbf915a5..6740da3d324b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1164,7 +1164,7 @@ int __init make_psci_node(void *fdt)
     return res;
 }
 
-int __init make_cpus_node(const struct domain *d, void *fdt)
+int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
 {
     int res;
     const struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
@@ -1178,6 +1178,7 @@ int __init make_cpus_node(const struct domain *d, void *fdt)
     /* Keep the compiler happy with -Og */
     bool clock_valid = false;
     uint64_t mpidr_aff;
+    void *fdt = kinfo;
 
     dt_dprintk("Create cpus node\n");
 
@@ -1626,7 +1627,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
         if ( res )
             return res;
 
-        res = make_cpus_node(d, kinfo->fdt);
+        res = make_cpus_node(d, kinfo);
         if ( res )
             return res;
 
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 9205f01f0a49..64b12d6aec62 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -535,7 +535,7 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
     if ( ret )
         goto err;
 
-    ret = make_cpus_node(d, kinfo->fdt);
+    ret = make_cpus_node(d, kinfo);
     if ( ret )
         goto err;
 
diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
index 220ae46ddbe1..2349baba3427 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -25,7 +25,7 @@ int construct_domain(struct domain *d, struct kernel_info *kinfo);
 int construct_hwdom(struct kernel_info *kinfo,
                     const struct dt_device_node *node);
 int make_chosen_node(const struct kernel_info *kinfo);
-int make_cpus_node(const struct domain *d, void *fdt);
+int make_cpus_node(const struct domain *d, struct kernel_info *kinfo);
 int make_hypervisor_node(struct domain *d, const struct kernel_info *kinfo,
                          int addrcells, int sizecells);
 int make_memory_node(const struct kernel_info *kinfo, int addrcells,
-- 
2.53.0

[PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Oleksii Kurochko]

As domain type is part of common code now there is no any reason
to have architecture-specific set_domain_type() functions so
it is dropped.

The CONFIG_ARM_64 guard in xen/arch/arm/domain_build.c is intentionally
left unchanged: it is ARM-specific code, and ARM_64 selects
HAS_DOMAIN_TYPE, so the semantics are identical.

x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. For x86_32
it is fine to follow generic is_{32,64}_bit_domain() implementations.

Generally it probably would be better to introduce CONFIG_32BIT
and use it to define is_{32,64}bit_domain() as if one day
CONFIG_128BIT will be introduced is_{32,64}bit_domain() will be
defined incorrectly but considering that CONFIG_64BIT is maximum
supported bitness it is fine to define in this way.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/Kconfig                        |  3 +++
 xen/arch/arm/Kconfig                    |  1 +
 xen/arch/arm/arm64/domctl.c             |  4 ++--
 xen/arch/arm/dom0less-build.c           | 14 --------------
 xen/arch/arm/domain_build.c             |  8 ++++----
 xen/arch/arm/include/asm/domain.h       | 12 ------------
 xen/arch/arm/include/asm/kernel.h       |  4 ----
 xen/arch/arm/kernel.c                   | 10 +++++-----
 xen/common/device-tree/dom0less-build.c |  5 ++++-
 xen/include/xen/dom0less-build.h        |  2 --
 xen/include/xen/domain.h                | 13 +++++++++++++
 xen/include/xen/fdt-kernel.h            |  5 +++++
 xen/include/xen/sched.h                 |  4 ++++
 13 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
index f9230bfaf872..6c53149f7ee1 100644
--- a/xen/arch/Kconfig
+++ b/xen/arch/Kconfig
@@ -1,6 +1,9 @@
 config 64BIT
 	bool
 
+config HAS_DOMAIN_TYPE
+	bool
+
 config PHYS_ADDR_T_32
 	bool
 
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2f2b501fdac4..79622b46a10d 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -7,6 +7,7 @@ config ARM_64
 	def_bool y
 	depends on !ARM_32
 	select 64BIT
+	select HAS_DOMAIN_TYPE
 	select HAS_FAST_MULTIPLY
 	select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
 
diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index 8720d126c97d..9e9a29eb1e78 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -21,10 +21,10 @@ static long switch_mode(struct domain *d, enum domain_type type)
         return -EINVAL;
     if ( domain_tot_pages(d) != 0 )
         return -EBUSY;
-    if ( d->arch.type == type )
+    if ( d->type == type )
         return 0;
 
-    d->arch.type = type;
+    d->type = type;
 
     if ( is_64bit_domain(d) )
         for_each_vcpu(d, v)
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 4181c105389a..6f73c65e5151 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -237,20 +237,6 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
     return 0;
 }
 
-/* TODO: make arch.type generic ? */
-#ifdef CONFIG_ARM_64
-void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
-{
-    /* type must be set before allocate memory */
-    d->arch.type = kinfo->arch.type;
-}
-#else
-void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
-{
-    /* Nothing to do */
-}
-#endif
-
 int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
                       const struct dt_device_node *node)
 {
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6740da3d324b..2fd89b3a1345 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1178,7 +1178,7 @@ int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
     /* Keep the compiler happy with -Og */
     bool clock_valid = false;
     uint64_t mpidr_aff;
-    void *fdt = kinfo;
+    void *fdt = kinfo->fdt;
 
     dt_dprintk("Create cpus node\n");
 
@@ -1774,13 +1774,13 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
 
 #ifdef CONFIG_ARM_64
     /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
-    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
+    if ( !(cpu_has_el1_32) && kinfo->type == DOMAIN_32BIT )
     {
         printk("Platform does not support 32-bit domain\n");
         return -EINVAL;
     }
 
-    if ( is_sve_domain(d) && (kinfo->arch.type == DOMAIN_32BIT) )
+    if ( is_sve_domain(d) && (kinfo->type == DOMAIN_32BIT) )
     {
         printk("SVE is not available for 32-bit domain\n");
         return -EINVAL;
@@ -1896,7 +1896,7 @@ int __init construct_hwdom(struct kernel_info *kinfo,
 
 #ifdef CONFIG_ARM_64
     /* type must be set before allocate_memory */
-    d->arch.type = kinfo->arch.type;
+    d->type = kinfo->type;
 #endif
     find_gnttab_region(d, kinfo);
     if ( is_domain_direct_mapped(d) )
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index f95ad1285e6e..2d4c1bdecb66 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -18,18 +18,6 @@ struct hvm_domain
     uint64_t              params[HVM_NR_PARAMS];
 };
 
-#ifdef CONFIG_ARM_64
-enum domain_type {
-    DOMAIN_32BIT,
-    DOMAIN_64BIT,
-};
-#define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
-#define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
-#else
-#define is_32bit_domain(d) (1)
-#define is_64bit_domain(d) (0)
-#endif
-
 struct vtimer {
     struct vcpu *v;
     int irq;
diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
index 7c3b7fde5b64..21f4273fa1b5 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -10,10 +10,6 @@
 
 struct arch_kernel_info
 {
-#ifdef CONFIG_ARM_64
-    enum domain_type type;
-#endif
-
     /* Enable pl011 emulation */
     bool vpl011;
 };
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 3c613cdb233f..bc9aad3f5f45 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -101,7 +101,7 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
     paddr_t load_addr;
 
 #ifdef CONFIG_ARM_64
-    if ( (info->arch.type == DOMAIN_64BIT) && (info->image.start == 0) )
+    if ( (info->type == DOMAIN_64BIT) && (info->image.start == 0) )
         return mem->bank[0].start + info->image.text_offset;
 #endif
 
@@ -272,10 +272,10 @@ int __init kernel_uimage_probe(struct kernel_info *info,
     switch ( uimage.arch )
     {
     case IH_ARCH_ARM:
-        info->arch.type = DOMAIN_32BIT;
+        info->type = DOMAIN_32BIT;
         break;
     case IH_ARCH_ARM64:
-        info->arch.type = DOMAIN_64BIT;
+        info->type = DOMAIN_64BIT;
         break;
     default:
         printk(XENLOG_ERR "Unsupported uImage arch type %d\n", uimage.arch);
@@ -345,7 +345,7 @@ static int __init kernel_zimage64_probe(struct kernel_info *info,
 
     info->load = kernel_zimage_load;
 
-    info->arch.type = DOMAIN_64BIT;
+    info->type = DOMAIN_64BIT;
 
     return 0;
 }
@@ -397,7 +397,7 @@ static int __init kernel_zimage32_probe(struct kernel_info *info,
     info->load = kernel_zimage_load;
 
 #ifdef CONFIG_ARM_64
-    info->arch.type = DOMAIN_32BIT;
+    info->type = DOMAIN_32BIT;
 #endif
 
     return 0;
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 64b12d6aec62..418282a6a549 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -809,7 +809,10 @@ static int __init construct_domU(struct kernel_info *kinfo,
     if ( rc < 0 )
         return rc;
 
-    set_domain_type(d, kinfo);
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    /* type must be set before allocate memory */
+    d->type = kinfo->type;
+#endif
 
     if ( is_hardware_domain(d) )
     {
diff --git a/xen/include/xen/dom0less-build.h b/xen/include/xen/dom0less-build.h
index faaf660424b2..4118dec76c0a 100644
--- a/xen/include/xen/dom0less-build.h
+++ b/xen/include/xen/dom0less-build.h
@@ -57,8 +57,6 @@ int init_vuart(struct domain *d, struct kernel_info *kinfo,
 int make_intc_domU_node(struct kernel_info *kinfo);
 int make_arch_nodes(struct kernel_info *kinfo);
 
-void set_domain_type(struct domain *d, struct kernel_info *kinfo);
-
 int init_intc_phandle(struct kernel_info *kinfo, const char *name,
                       const int node_next, const void *pfdt);
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 93c0fd00c1d7..5951e7e6928f 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -13,6 +13,19 @@ struct guest_area {
     void *map;
 };
 
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+enum domain_type {
+    DOMAIN_32BIT,
+    DOMAIN_64BIT,
+};
+#define is_32bit_domain(d) ((d)->type == DOMAIN_32BIT)
+#define is_64bit_domain(d) ((d)->type == DOMAIN_64BIT)
+#elif !defined(CONFIG_64BIT)
+/* On 32-bit-only platforms all domains are 32-bit. */
+#define is_32bit_domain(d) (true)
+#define is_64bit_domain(d) (false)
+#endif
+
 #include <asm/domain.h>
 
 typedef union {
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index 3527934b2a00..2c9bf4d4ef4f 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -9,6 +9,7 @@
 
 #include <xen/bootinfo.h>
 #include <xen/device_tree.h>
+#include <xen/domain.h>
 #include <xen/types.h>
 
 #if __has_include(<asm/kernel.h>)
@@ -69,6 +70,10 @@ struct kernel_info {
         } image;
     };
 
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    enum domain_type type;
+#endif
+
 #if __has_include(<asm/kernel.h>)
     struct arch_kernel_info arch;
 #endif
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 212c7d765c3e..4314bd195457 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -668,6 +668,10 @@ struct domain
     struct page_info *pending_scrub;
     unsigned int pending_scrub_order;
     unsigned int pending_scrub_index;
+
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    enum domain_type type;
+#endif
 } __aligned(PAGE_SIZE);
 
 static inline struct page_list_head *page_to_list(
-- 
2.53.0

Re: [PATCH v1 1/3] xen/dom0less: introduce free_phandle in struct kernel_info

[Orzel, Michal]

On 16/04/2026 16:20, Oleksii Kurochko wrote:
> There are cases where it is necessary to know the next available phandle
> number in order to generate phandles for guest device nodes.
> 
> When a partial FDT (pfdt) is provided, special care is needed during
> initialization of free_phandle, as the pfdt may already contain a dummy
> interrupt controller node with a phandle assigned to it.  free_phandle
> must therefore be initialized to one past the highest phandle already
> present in the pfdt, to avoid collisions.
> 
> Since free_phandle may be needed for the very first guest node generated,
I would prefer next_phandle rather than free_phandle given that it always holds
the *next* phandle to be allocated. Free is implied.

> domain_handle_dtb_boot_module() is moved earlier in prepare_dtb_domU().
> The new call site also aligns better with the existing comment stating
> that domain_handle_dtb_boot_module() must be called before the rest of
> the device tree is generated.
> 
> Introduce get_next_free_phandle() to ensure that phandles allocated for
The name is a bit confusing. It reads as "get the next one after the current
free phandle" but it actually returns the current value of free_phandle and post
increments it. Let's call it alloc_phandle(). This will also make it clear to
use the return value of this function instead of directly kinfo value.

Other than that, all good.

~Michal

Re: [PATCH v1 2/3] xen/dom0less: pass kernel_info struct instead of fdt to make_cpus_node()

[Orzel, Michal]

On 16/04/2026 16:20, Oleksii Kurochko wrote:
> There are two reasons of this change:
> 1. Align prototype with what other make_*_node() are passed.
> 2. A follow-up RISC-V patch will call get_next_free_phandle() inside
>    make_cpus_node(), requiring mutable access to kinfo->free_phandle.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
>  xen/arch/arm/domain_build.c             | 5 +++--
>  xen/common/device-tree/dom0less-build.c | 2 +-
>  xen/include/xen/fdt-domain-build.h      | 2 +-
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60a7cbf915a5..6740da3d324b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1164,7 +1164,7 @@ int __init make_psci_node(void *fdt)
>      return res;
>  }
>  
> -int __init make_cpus_node(const struct domain *d, void *fdt)
> +int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
>  {
>      int res;
>      const struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
> @@ -1178,6 +1178,7 @@ int __init make_cpus_node(const struct domain *d, void *fdt)
>      /* Keep the compiler happy with -Og */
>      bool clock_valid = false;
>      uint64_t mpidr_aff;
> +    void *fdt = kinfo;
You're assigning kinfo to fdt pointer which is a bug.
Instead, you should do:
void *fdt = kinfo->fdt

~Michal

Re: [PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Orzel, Michal]

On 16/04/2026 16:21, Oleksii Kurochko wrote:
> As domain type is part of common code now there is no any reason
> to have architecture-specific set_domain_type() functions so
> it is dropped.
> 
> The CONFIG_ARM_64 guard in xen/arch/arm/domain_build.c is intentionally
> left unchanged: it is ARM-specific code, and ARM_64 selects
> HAS_DOMAIN_TYPE, so the semantics are identical.
> 
> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. For x86_32
> it is fine to follow generic is_{32,64}_bit_domain() implementations.
I thought x86_32 Xen no longer builds. This means the fallback is only for arm32.

> 
> Generally it probably would be better to introduce CONFIG_32BIT
> and use it to define is_{32,64}bit_domain() as if one day
> CONFIG_128BIT will be introduced is_{32,64}bit_domain() will be
> defined incorrectly but considering that CONFIG_64BIT is maximum
> supported bitness it is fine to define in this way.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
>  xen/arch/Kconfig                        |  3 +++
>  xen/arch/arm/Kconfig                    |  1 +
>  xen/arch/arm/arm64/domctl.c             |  4 ++--
>  xen/arch/arm/dom0less-build.c           | 14 --------------
>  xen/arch/arm/domain_build.c             |  8 ++++----
>  xen/arch/arm/include/asm/domain.h       | 12 ------------
>  xen/arch/arm/include/asm/kernel.h       |  4 ----
>  xen/arch/arm/kernel.c                   | 10 +++++-----
>  xen/common/device-tree/dom0less-build.c |  5 ++++-
>  xen/include/xen/dom0less-build.h        |  2 --
>  xen/include/xen/domain.h                | 13 +++++++++++++
>  xen/include/xen/fdt-kernel.h            |  5 +++++
>  xen/include/xen/sched.h                 |  4 ++++
>  13 files changed, 41 insertions(+), 44 deletions(-)
> 
> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
> index f9230bfaf872..6c53149f7ee1 100644
> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -1,6 +1,9 @@
>  config 64BIT
>  	bool
>  
> +config HAS_DOMAIN_TYPE
> +	bool
> +
>  config PHYS_ADDR_T_32
>  	bool
>  
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2f2b501fdac4..79622b46a10d 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -7,6 +7,7 @@ config ARM_64
>  	def_bool y
>  	depends on !ARM_32
>  	select 64BIT
> +	select HAS_DOMAIN_TYPE
>  	select HAS_FAST_MULTIPLY
>  	select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
>  
> diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
> index 8720d126c97d..9e9a29eb1e78 100644
> --- a/xen/arch/arm/arm64/domctl.c
> +++ b/xen/arch/arm/arm64/domctl.c
> @@ -21,10 +21,10 @@ static long switch_mode(struct domain *d, enum domain_type type)
>          return -EINVAL;
>      if ( domain_tot_pages(d) != 0 )
>          return -EBUSY;
> -    if ( d->arch.type == type )
> +    if ( d->type == type )
>          return 0;
>  
> -    d->arch.type = type;
> +    d->type = type;
>  
>      if ( is_64bit_domain(d) )
>          for_each_vcpu(d, v)
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index 4181c105389a..6f73c65e5151 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -237,20 +237,6 @@ int __init make_arch_nodes(struct kernel_info *kinfo)
>      return 0;
>  }
>  
> -/* TODO: make arch.type generic ? */
> -#ifdef CONFIG_ARM_64
> -void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
> -{
> -    /* type must be set before allocate memory */
> -    d->arch.type = kinfo->arch.type;
> -}
> -#else
> -void __init set_domain_type(struct domain *d, struct kernel_info *kinfo)
> -{
> -    /* Nothing to do */
> -}
> -#endif
> -
>  int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
>                        const struct dt_device_node *node)
>  {
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 6740da3d324b..2fd89b3a1345 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1178,7 +1178,7 @@ int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
>      /* Keep the compiler happy with -Og */
>      bool clock_valid = false;
>      uint64_t mpidr_aff;
> -    void *fdt = kinfo;
> +    void *fdt = kinfo->fdt;
Here you are fixing a regression introduced in patch 2/3.

I'm ok with this patch. but since you touch common code too you need to wait for
other's opinion. For now, provided you fix the commit msg and regression remark:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Jan Beulich]

On 16.04.2026 16:21, Oleksii Kurochko wrote:
> As domain type is part of common code now there is no any reason
> to have architecture-specific set_domain_type() functions so
> it is dropped.
> 
> The CONFIG_ARM_64 guard in xen/arch/arm/domain_build.c is intentionally
> left unchanged: it is ARM-specific code, and ARM_64 selects
> HAS_DOMAIN_TYPE, so the semantics are identical.

Still I think it would better be updated, for consistency's sake.

> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. For x86_32
> it is fine to follow generic is_{32,64}_bit_domain() implementations.

As Michal has also alluded to, the use of x86_32 here needs some further
qualification, as Xen hasn't been possible to be built for that target
for many years. Quite possibly you mean guest type there, but then it's
unclear why you would restrict the explanation to 32-bit guests.

> Generally it probably would be better to introduce CONFIG_32BIT
> and use it to define is_{32,64}bit_domain() as if one day
> CONFIG_128BIT will be introduced is_{32,64}bit_domain() will be
> defined incorrectly but considering that CONFIG_64BIT is maximum
> supported bitness it is fine to define in this way.

Many things will need adjustment for a future CONFIG_128BIT. Imo this
kind of statement doesn't belong in a patch description; it could be a
post-commit-message remark.

> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -1,6 +1,9 @@
>  config 64BIT
>  	bool
>  
> +config HAS_DOMAIN_TYPE
> +	bool
> +
>  config PHYS_ADDR_T_32
>  	bool

Why here rather than where the bulk of the other HAS_* are?

> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1178,7 +1178,7 @@ int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
>      /* Keep the compiler happy with -Og */
>      bool clock_valid = false;
>      uint64_t mpidr_aff;
> -    void *fdt = kinfo;
> +    void *fdt = kinfo->fdt;
>  
>      dt_dprintk("Create cpus node\n");
>  
> @@ -1774,13 +1774,13 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
>  
>  #ifdef CONFIG_ARM_64
>      /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
> -    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
> +    if ( !(cpu_has_el1_32) && kinfo->type == DOMAIN_32BIT )

I'm not an Arm maintainer; if I was, I'd ask for the stray parentheses to be
dropped on this occasion.

> @@ -1896,7 +1896,7 @@ int __init construct_hwdom(struct kernel_info *kinfo,
>  
>  #ifdef CONFIG_ARM_64
>      /* type must be set before allocate_memory */
> -    d->arch.type = kinfo->arch.type;
> +    d->type = kinfo->type;
>  #endif

I wonder why this doesn't use set_domain_type() (see also below).

> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -809,7 +809,10 @@ static int __init construct_domU(struct kernel_info *kinfo,
>      if ( rc < 0 )
>          return rc;
>  
> -    set_domain_type(d, kinfo);
> +#ifdef CONFIG_HAS_DOMAIN_TYPE
> +    /* type must be set before allocate memory */

Nit: Comment style.

> +    d->type = kinfo->type;
> +#endif

Imo it would be preferable to retain set_domain_type(), to avoid #ifdef-ary
here and in construct_hwdom(). Of course its implementation would need to
move. Couldn't it be ...

> --- a/xen/include/xen/dom0less-build.h
> +++ b/xen/include/xen/dom0less-build.h
> @@ -57,8 +57,6 @@ int init_vuart(struct domain *d, struct kernel_info *kinfo,
>  int make_intc_domU_node(struct kernel_info *kinfo);
>  int make_arch_nodes(struct kernel_info *kinfo);
>  
> -void set_domain_type(struct domain *d, struct kernel_info *kinfo);

... an inline function here?

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -668,6 +668,10 @@ struct domain
>      struct page_info *pending_scrub;
>      unsigned int pending_scrub_order;
>      unsigned int pending_scrub_index;
> +
> +#ifdef CONFIG_HAS_DOMAIN_TYPE
> +    enum domain_type type;
> +#endif
>  } __aligned(PAGE_SIZE);

I'm not quite happy with all new fields getting put at the bottom, when
better options may exist. If the enum was a packed one, it could go next
to domain_id (where 16 bits of padding presently exist). The five *_pages
fields also have a padding field following them (unless MEM_SHARING !=
MEM_PAGING).

Jan

Re: [PATCH v1 1/3] xen/dom0less: introduce free_phandle in struct kernel_info

[Oleksii Kurochko]

On 4/20/26 9:26 AM, Orzel, Michal wrote:
> On 16/04/2026 16:20, Oleksii Kurochko wrote:
>> There are cases where it is necessary to know the next available phandle
>> number in order to generate phandles for guest device nodes.
>>
>> When a partial FDT (pfdt) is provided, special care is needed during
>> initialization of free_phandle, as the pfdt may already contain a dummy
>> interrupt controller node with a phandle assigned to it.  free_phandle
>> must therefore be initialized to one past the highest phandle already
>> present in the pfdt, to avoid collisions.
>>
>> Since free_phandle may be needed for the very first guest node generated,
> I would prefer next_phandle rather than free_phandle given that it always holds
> the *next* phandle to be allocated. Free is implied.

I am okay with such naming as I had it before and decided to change at 
last minute.

> 
>> domain_handle_dtb_boot_module() is moved earlier in prepare_dtb_domU().
>> The new call site also aligns better with the existing comment stating
>> that domain_handle_dtb_boot_module() must be called before the rest of
>> the device tree is generated.
>>
>> Introduce get_next_free_phandle() to ensure that phandles allocated for
> The name is a bit confusing. It reads as "get the next one after the current
> free phandle" but it actually returns the current value of free_phandle and post
> increments it. Let's call it alloc_phandle(). This will also make it clear to
> use the return value of this function instead of directly kinfo value.

alloc_phandle() sounds good to me.

Thanks.

~ Oleksii

Re: [PATCH v1 2/3] xen/dom0less: pass kernel_info struct instead of fdt to make_cpus_node()

[Oleksii Kurochko]

On 4/20/26 9:29 AM, Orzel, Michal wrote:
> 
> 
> On 16/04/2026 16:20, Oleksii Kurochko wrote:
>> There are two reasons of this change:
>> 1. Align prototype with what other make_*_node() are passed.
>> 2. A follow-up RISC-V patch will call get_next_free_phandle() inside
>>     make_cpus_node(), requiring mutable access to kinfo->free_phandle.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> ---
>>   xen/arch/arm/domain_build.c             | 5 +++--
>>   xen/common/device-tree/dom0less-build.c | 2 +-
>>   xen/include/xen/fdt-domain-build.h      | 2 +-
>>   3 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 60a7cbf915a5..6740da3d324b 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1164,7 +1164,7 @@ int __init make_psci_node(void *fdt)
>>       return res;
>>   }
>>   
>> -int __init make_cpus_node(const struct domain *d, void *fdt)
>> +int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
>>   {
>>       int res;
>>       const struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
>> @@ -1178,6 +1178,7 @@ int __init make_cpus_node(const struct domain *d, void *fdt)
>>       /* Keep the compiler happy with -Og */
>>       bool clock_valid = false;
>>       uint64_t mpidr_aff;
>> +    void *fdt = kinfo;
> You're assigning kinfo to fdt pointer which is a bug.
> Instead, you should do:
> void *fdt = kinfo->fdt

Oh, right. Missed apply changes properly. As you noticed it was fixed in 
the next patch.

Thanks.

~ Oleksii

Re: [PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Oleksii Kurochko]

On 4/20/26 10:03 AM, Orzel, Michal wrote:
> 
> 
> On 16/04/2026 16:21, Oleksii Kurochko wrote:
>> As domain type is part of common code now there is no any reason
>> to have architecture-specific set_domain_type() functions so
>> it is dropped.
>>
>> The CONFIG_ARM_64 guard in xen/arch/arm/domain_build.c is intentionally
>> left unchanged: it is ARM-specific code, and ARM_64 selects
>> HAS_DOMAIN_TYPE, so the semantics are identical.
>>
>> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
>> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. For x86_32
>> it is fine to follow generic is_{32,64}_bit_domain() implementations.
> I thought x86_32 Xen no longer builds. This means the fallback is only for arm32.

Will you be okay with
   Since x86_32 Xen no longer builds, the fallback is currently only 
relevant for arm32.

?

Re: [PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Oleksii Kurochko]

On 4/20/26 10:22 AM, Jan Beulich wrote:
> On 16.04.2026 16:21, Oleksii Kurochko wrote:
>> As domain type is part of common code now there is no any reason
>> to have architecture-specific set_domain_type() functions so
>> it is dropped.
>>
>> The CONFIG_ARM_64 guard in xen/arch/arm/domain_build.c is intentionally
>> left unchanged: it is ARM-specific code, and ARM_64 selects
>> HAS_DOMAIN_TYPE, so the semantics are identical.
> 
> Still I think it would better be updated, for consistency's sake.

I can update that. Just tried to keep changes smaller.

> 
>> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
>> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. For x86_32
>> it is fine to follow generic is_{32,64}_bit_domain() implementations.
> 
> As Michal has also alluded to, the use of x86_32 here needs some further
> qualification, as Xen hasn't been possible to be built for that target
> for many years. Quite possibly you mean guest type there, but then it's
> unclear why you would restrict the explanation to 32-bit guests.

I suggested to rephrase that part to: "Since x86_32 Xen no longer 
builds, the fallback is currently only relevant for arm32."

> 
>> Generally it probably would be better to introduce CONFIG_32BIT
>> and use it to define is_{32,64}bit_domain() as if one day
>> CONFIG_128BIT will be introduced is_{32,64}bit_domain() will be
>> defined incorrectly but considering that CONFIG_64BIT is maximum
>> supported bitness it is fine to define in this way.
> 
> Many things will need adjustment for a future CONFIG_128BIT. Imo this
> kind of statement doesn't belong in a patch description; it could be a
> post-commit-message remark.

I will drop that part of the message.

> 
>> --- a/xen/arch/Kconfig
>> +++ b/xen/arch/Kconfig
>> @@ -1,6 +1,9 @@
>>   config 64BIT
>>   	bool
>>   
>> +config HAS_DOMAIN_TYPE
>> +	bool
>> +
>>   config PHYS_ADDR_T_32
>>   	bool
> 
> Why here rather than where the bulk of the other HAS_* are?

Because it is a little arch-specific now as not all arch-s support it.

I can move it to xen/common/Kconfig.

> 
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1178,7 +1178,7 @@ int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
>>       /* Keep the compiler happy with -Og */
>>       bool clock_valid = false;
>>       uint64_t mpidr_aff;
>> -    void *fdt = kinfo;
>> +    void *fdt = kinfo->fdt;
>>   
>>       dt_dprintk("Create cpus node\n");
>>   
>> @@ -1774,13 +1774,13 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
>>   
>>   #ifdef CONFIG_ARM_64
>>       /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
>> -    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
>> +    if ( !(cpu_has_el1_32) && kinfo->type == DOMAIN_32BIT )
> 
> I'm not an Arm maintainer; if I was, I'd ask for the stray parentheses to be
> dropped on this occasion.

They could be dropped. Should be then it mentioned in commit message?

> 
>> @@ -1896,7 +1896,7 @@ int __init construct_hwdom(struct kernel_info *kinfo,
>>   
>>   #ifdef CONFIG_ARM_64
>>       /* type must be set before allocate_memory */
>> -    d->arch.type = kinfo->arch.type;
>> +    d->type = kinfo->type;
>>   #endif
> 
> I wonder why this doesn't use set_domain_type() (see also below).

I assume that domain_build.c could be used for both dom0 and dom0less 
but set_domain_type() was used for dom0less. But generally I think it 
shouldn't be a problem to re-use it in construct_hwdom().

If it is fine to ...


> 
>> --- a/xen/common/device-tree/dom0less-build.c
>> +++ b/xen/common/device-tree/dom0less-build.c
>> @@ -809,7 +809,10 @@ static int __init construct_domU(struct kernel_info *kinfo,
>>       if ( rc < 0 )
>>           return rc;
>>   
>> -    set_domain_type(d, kinfo);
>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>> +    /* type must be set before allocate memory */
> 
> Nit: Comment style.
> 
>> +    d->type = kinfo->type;
>> +#endif
> 
> Imo it would be preferable to retain set_domain_type(), to avoid #ifdef-ary
> here and in construct_hwdom(). Of course its implementation would need to
> move. Couldn't it be ...
> 
>> --- a/xen/include/xen/dom0less-build.h
>> +++ b/xen/include/xen/dom0less-build.h
>> @@ -57,8 +57,6 @@ int init_vuart(struct domain *d, struct kernel_info *kinfo,
>>   int make_intc_domU_node(struct kernel_info *kinfo);
>>   int make_arch_nodes(struct kernel_info *kinfo);
>>   
>> -void set_domain_type(struct domain *d, struct kernel_info *kinfo);
> 
> ... an inline function here?

... move set_domain_type() to <xen/fdt-domain-build.h> then it would be 
fine for me to have the following implementation:

static void inline set_domain_type(struct domain *d, struct kernel_info 
*kinfo)
{
#ifdef CONFIG_HAS_DOMAIN
     /* type must be set before allocate memory */
     d->arch.type = kinfo->arch.type;
#endif
}

and re-use it everywhere it is needed.

> 
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -668,6 +668,10 @@ struct domain
>>       struct page_info *pending_scrub;
>>       unsigned int pending_scrub_order;
>>       unsigned int pending_scrub_index;
>> +
>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>> +    enum domain_type type;
>> +#endif
>>   } __aligned(PAGE_SIZE);
> 
> I'm not quite happy with all new fields getting put at the bottom, when
> better options may exist. If the enum was a packed one, it could go next
> to domain_id (where 16 bits of padding presently exist). The five *_pages
> fields also have a padding field following them (unless MEM_SHARING !=
> MEM_PAGING).

Just to be sure that I understand correctly what you meant:

enum __attribute__((packed)) domain_type {
     DOMAIN_32BIT,
     DOMAIN_64BIT,
};

struct domain
{
     domid_t          domain_id;

#ifdef CONFIG_HAS_DOMAIN_TYPE
     enum domain_type type;
#endif
  ....

It is what you suggested?

I thought that it make sense only for struct and unions to remove 
padding between members.

Maybe do you mean just declare type member as uint16_t or even uint8_t, 
place it after domain_id and and keep the enum as symbolic constants?

Thanks.

~ Oleksii

Re: [PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Orzel, Michal]

On 21/04/2026 13:06, Oleksii Kurochko wrote:
> 
> 
> On 4/20/26 10:03 AM, Orzel, Michal wrote:
>>
>>
>> On 16/04/2026 16:21, Oleksii Kurochko wrote:
>>> As domain type is part of common code now there is no any reason
>>> to have architecture-specific set_domain_type() functions so
>>> it is dropped.
>>>
>>> The CONFIG_ARM_64 guard in xen/arch/arm/domain_build.c is intentionally
>>> left unchanged: it is ARM-specific code, and ARM_64 selects
>>> HAS_DOMAIN_TYPE, so the semantics are identical.
>>>
>>> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
>>> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. For x86_32
>>> it is fine to follow generic is_{32,64}_bit_domain() implementations.
>> I thought x86_32 Xen no longer builds. This means the fallback is only for arm32.
> 
> Will you be okay with
>    Since x86_32 Xen no longer builds, the fallback is currently only 
> relevant for arm32.
Yes. I would.

~Michal

Re: [PATCH v1 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Jan Beulich]

On 21.04.2026 13:53, Oleksii Kurochko wrote:
> On 4/20/26 10:22 AM, Jan Beulich wrote:
>> On 16.04.2026 16:21, Oleksii Kurochko wrote:
>>> --- a/xen/arch/Kconfig
>>> +++ b/xen/arch/Kconfig
>>> @@ -1,6 +1,9 @@
>>>   config 64BIT
>>>   	bool
>>>   
>>> +config HAS_DOMAIN_TYPE
>>> +	bool
>>> +
>>>   config PHYS_ADDR_T_32
>>>   	bool
>>
>> Why here rather than where the bulk of the other HAS_* are?
> 
> Because it is a little arch-specific now as not all arch-s support it.

Most HAS_* are there to deal with per-arch differences.

> I can move it to xen/common/Kconfig.

Please do.

>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -1178,7 +1178,7 @@ int __init make_cpus_node(const struct domain *d, struct kernel_info *kinfo)
>>>       /* Keep the compiler happy with -Og */
>>>       bool clock_valid = false;
>>>       uint64_t mpidr_aff;
>>> -    void *fdt = kinfo;
>>> +    void *fdt = kinfo->fdt;
>>>   
>>>       dt_dprintk("Create cpus node\n");
>>>   
>>> @@ -1774,13 +1774,13 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
>>>   
>>>   #ifdef CONFIG_ARM_64
>>>       /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
>>> -    if ( !(cpu_has_el1_32) && kinfo->arch.type == DOMAIN_32BIT )
>>> +    if ( !(cpu_has_el1_32) && kinfo->type == DOMAIN_32BIT )
>>
>> I'm not an Arm maintainer; if I was, I'd ask for the stray parentheses to be
>> dropped on this occasion.
> 
> They could be dropped. Should be then it mentioned in commit message?

For something this small I wouldn't insist. But recall that I'm not the one
to ack this part of the change.

>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -668,6 +668,10 @@ struct domain
>>>       struct page_info *pending_scrub;
>>>       unsigned int pending_scrub_order;
>>>       unsigned int pending_scrub_index;
>>> +
>>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>>> +    enum domain_type type;
>>> +#endif
>>>   } __aligned(PAGE_SIZE);
>>
>> I'm not quite happy with all new fields getting put at the bottom, when
>> better options may exist. If the enum was a packed one, it could go next
>> to domain_id (where 16 bits of padding presently exist). The five *_pages
>> fields also have a padding field following them (unless MEM_SHARING !=
>> MEM_PAGING).
> 
> Just to be sure that I understand correctly what you meant:
> 
> enum __attribute__((packed)) domain_type {
>      DOMAIN_32BIT,
>      DOMAIN_64BIT,
> };
> 
> struct domain
> {
>      domid_t          domain_id;
> 
> #ifdef CONFIG_HAS_DOMAIN_TYPE
>      enum domain_type type;
> #endif
>   ....
> 
> It is what you suggested?

Yes, just without open-coding of __packed.

> I thought that it make sense only for struct and unions to remove 
> padding between members.

That's the attribute's effect on struct / union, yes. The effect is
different for enum.

> Maybe do you mean just declare type member as uint16_t or even uint8_t, 
> place it after domain_id and and keep the enum as symbolic constants?

Preferably use the real type for the field.

Jan