[PATCH v4 0/3] dom0less: various updates

[PATCH v4 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: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2482499536

---
Changes in v4:
 - Rebase on top of staging.
 - Add Review-by for patch 1 and 3.
 - Address the comments.
---
Changes in v3:
 - Rebase on top of staging.
 - Address the comments.
---
Changes in v2:
 - Address the comments from ML.
---

Oleksii Kurochko (3):
  xen/dom0less: introduce next_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/arm/Kconfig                    |  1 +
 xen/arch/arm/arm64/domctl.c             |  4 +--
 xen/arch/arm/dom0less-build.c           | 14 --------
 xen/arch/arm/domain_build.c             | 17 +++++-----
 xen/arch/arm/include/asm/domain.h       | 16 ---------
 xen/arch/arm/include/asm/kernel.h       |  4 ---
 xen/arch/arm/kernel.c                   | 16 ++++-----
 xen/common/Kconfig                      |  3 ++
 xen/common/device-tree/dom0less-build.c | 45 ++++++++++++++++++-------
 xen/include/xen/dom0less-build.h        |  2 --
 xen/include/xen/domain.h                | 13 +++++++
 xen/include/xen/fdt-domain-build.h      | 17 +++++++++-
 xen/include/xen/fdt-kernel.h            | 11 ++++++
 xen/include/xen/sched.h                 |  4 +++
 14 files changed, 98 insertions(+), 69 deletions(-)

-- 
2.53.0

[PATCH v4 1/3] xen/dom0less: introduce next_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 next_phandle, as the pfdt may already contain a dummy
interrupt controller node with a phandle assigned to it. next_phandle
must therefore be initialized to one past the highest phandle already
present in the pfdt, to avoid collisions.

Since next_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 alloc_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>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
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 phandle is generated in this example not by get_next_free_phandle().

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

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/interrupt-controller/riscv%2Ccpu-intc.txt
---
Changes in v4:
 - Add Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v3:
 - Drop BUG_ON(GUEST_PHANDLE_GIC == 1).
 - Update the comment above declaration of next_phandle field of srtuct
   kernel_info.
---
Changes in v2:
 - s/free_phandle/next_phandle.
 - s/get_next_free_phandle/alloc_phandle.
---
 xen/common/device-tree/dom0less-build.c | 43 ++++++++++++++++++-------
 xen/include/xen/fdt-domain-build.h      |  6 ++++
 xen/include/xen/fdt-kernel.h            |  6 ++++
 3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index 840d14419da2..9787ee264975 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 next_phandle starts
+     * above it, avoiding collisions with pfdt's own phandle assignments.
+     */
+    res = fdt_generate_phandle(pfdt, &kinfo->next_phandle);
+    if ( res )
+    {
+        res = (res == -FDT_ERR_NOPHANDLES) ? -EOVERFLOW : -EINVAL;
+        goto out;
+    }
+
+    if ( kinfo->next_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,7 @@ 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->next_phandle = 1;
 
 #ifdef CONFIG_GRANT_TABLE
     kinfo->gnttab_start = GUEST_GNTTAB_BASE;
@@ -499,6 +518,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 next_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 +547,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 886a85381651..fd2ba01ff0f4 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 alloc_phandle(struct kernel_info *kinfo)
+{
+    return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
+}
+
 #endif /* __XEN_FDT_DOMAIN_BUILD_H__ */
 
 /*
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index 33a60597bb4d..4d0467bb396a 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -44,6 +44,12 @@ struct kernel_info {
     /* Interrupt controller phandle */
     uint32_t phandle_intc;
 
+    /*
+     * Next free phandle for guest device nodes; do not access directly, use
+     * alloc_phandle().
+     */
+    uint32_t next_phandle;
+
     /* loader to use for this kernel */
     void (*load)(struct kernel_info *info);
 
-- 
2.53.0

[PATCH v4 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>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Look at the footer to the prev. patch to understand why phandle is needed
inside cpu node.
---
Changes in v4:
 - Nothing changed. Only rebase.
---
Changes in v3:
 - Add Reviewed-by: Michal Orzel <michal.orzel@amd.com>.
---
Changes in v2:
 - Properly initialize local variable fdt in Arm's
   make_cpus_node().
---
 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 6c17a84b2633..a3ff70102376 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1163,7 +1163,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");
@@ -1177,6 +1177,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->fdt;
 
     dt_dprintk("Create cpus node\n");
 
@@ -1625,7 +1626,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 9787ee264975..6d6882a34b5a 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -534,7 +534,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 fd2ba01ff0f4..0d40d8cfa105 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 v4 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.

Change the guard around access of kinfo->type to CONFIG_HAS_DOMAIN_TYPE
for consistency. Also, drop and add some parentheses to be aligned
with the similar if() below.

x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. Since x86_32 Xen no
longer builds, the fallback is currently only relevant for arm32.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v4:
 - Drop enum domain_type type; from Arm's struct arch_domain.
 - Add Reviewed-by: Michal Orzel <michal.orzel@amd.com>.
---
Changes in v3:
 - Sort properly HAS_DOMAIN_TYPE in xen/common/Kconfig.
 - Update the comment above is_*bit_domain() macros.
---
Changes in v2:
 - Update the commit message.
 - Move HAS_DOMAIN_TYPE to xen/common/Kconfig.
 - Return set_domain_type(), move it to xen/fdt-domain-build.h and make it
   static inline.
 - s/CONFIG_ARM_64/CONFIG_HAS_DOMAIN for the places where kinfo->type is used.
 - Drop parantethes around cpu_has_el1_32 in if() and add around
   (kinfo->type == DOMAIN_32BIT) to be consistent with the similar check below.
 - Fix comment code style.
 - Add __packed to definition of enum domain_type
---
 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        | 12 +++++-------
 xen/arch/arm/include/asm/domain.h  | 16 ----------------
 xen/arch/arm/include/asm/kernel.h  |  4 ----
 xen/arch/arm/kernel.c              | 16 ++++++++--------
 xen/common/Kconfig                 |  3 +++
 xen/include/xen/dom0less-build.h   |  2 --
 xen/include/xen/domain.h           | 13 +++++++++++++
 xen/include/xen/fdt-domain-build.h |  9 +++++++++
 xen/include/xen/fdt-kernel.h       |  5 +++++
 xen/include/xen/sched.h            |  4 ++++
 13 files changed, 50 insertions(+), 53 deletions(-)

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 a3ff70102376..ad665cd3c045 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1771,15 +1771,15 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
     BUG_ON(d->vcpu[0] == NULL);
     BUG_ON(v->is_initialised);
 
-#ifdef CONFIG_ARM_64
+#ifdef CONFIG_HAS_DOMAIN_TYPE
     /* 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;
@@ -1893,10 +1893,8 @@ int __init construct_hwdom(struct kernel_info *kinfo,
 
     iommu_hwdom_init(d);
 
-#ifdef CONFIG_ARM_64
-    /* type must be set before allocate_memory */
-    d->arch.type = kinfo->arch.type;
-#endif
+    set_domain_type(d, kinfo);
+
     find_gnttab_region(d, kinfo);
     if ( is_domain_direct_mapped(d) )
         allocate_memory_11(d, kinfo);
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index ffe5d0d9f0a6..b24f02d269be 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
-
 /*
  * Is the domain using the host memory layout?
  *
@@ -62,10 +50,6 @@ struct paging_domain {
 
 struct arch_domain
 {
-#ifdef CONFIG_ARM_64
-    enum domain_type type;
-#endif
-
 #ifdef CONFIG_ARM64_SVE
     /* max SVE encoded vector length */
     uint8_t sve_vl;
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 7544fd50a20f..9395b5af8745 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -100,8 +100,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
     const struct membanks *mem = kernel_info_get_mem(info);
     paddr_t load_addr;
 
-#ifdef CONFIG_ARM_64
-    if ( (info->arch.type == DOMAIN_64BIT) && (info->zimage.start == 0) )
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    if ( (info->type == DOMAIN_64BIT) && (info->zimage.start == 0) )
         return mem->bank[0].start + info->zimage.text_offset;
 #endif
 
@@ -268,14 +268,14 @@ int __init kernel_uimage_probe(struct kernel_info *info,
 
     info->load = kernel_zimage_load;
 
-#ifdef CONFIG_ARM_64
+#ifdef CONFIG_HAS_DOMAIN_TYPE
     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;
 }
@@ -396,8 +396,8 @@ static int __init kernel_zimage32_probe(struct kernel_info *info,
 
     info->load = kernel_zimage_load;
 
-#ifdef CONFIG_ARM_64
-    info->arch.type = DOMAIN_32BIT;
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    info->type = DOMAIN_32BIT;
 #endif
 
     return 0;
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0a20aa0a1249..5ff71480eebe 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -128,6 +128,9 @@ config HAS_DEVICE_TREE_DISCOVERY
 config HAS_DOM0LESS
 	bool
 
+config HAS_DOMAIN_TYPE
+	bool
+
 config HAS_DIT # Data Independent Timing
 	bool
 
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..8c898afe74fc 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 __packed 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)
+/* At the moment 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-domain-build.h b/xen/include/xen/fdt-domain-build.h
index 0d40d8cfa105..bc7744270c8f 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -7,6 +7,7 @@
 #include <xen/device_tree.h>
 #include <xen/fdt-kernel.h>
 #include <xen/mm.h>
+#include <xen/sched.h>
 #include <xen/types.h>
 
 struct domain;
@@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info *kinfo)
     return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
 }
 
+static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)
+{
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    /* Type must be set before allocate memory */
+    d->type = kinfo->type;
+#endif
+}
+
 #endif /* __XEN_FDT_DOMAIN_BUILD_H__ */
 
 /*
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index 4d0467bb396a..86a37a13048b 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>)
@@ -65,6 +66,10 @@ struct kernel_info {
         } zimage;
     };
 
+#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..00db1da12f21 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -386,6 +386,10 @@ struct domain
 {
     domid_t          domain_id;
 
+#ifdef CONFIG_HAS_DOMAIN_TYPE
+    enum domain_type type;
+#endif
+
     unsigned int     max_vcpus;
 
     uint64_t         unique_id;       /* Unique domain identifier */
-- 
2.53.0

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

[Jan Beulich]

On 27.04.2026 17:34, 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.
> 
> Change the guard around access of kinfo->type to CONFIG_HAS_DOMAIN_TYPE
> for consistency. Also, drop and add some parentheses to be aligned
> with the similar if() below.
> 
> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. Since x86_32 Xen no
> longer builds, the fallback is currently only relevant for arm32.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

In principle:
Acked-by: Jan Beulich <jbeulich@suse.com>

However, still a few remarks:

> --- 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 __packed 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)
> +/* At the moment on 32-bit-only platforms all domains are 32-bit. */
> +#define is_32bit_domain(d) (true)
> +#define is_64bit_domain(d) (false)

I think it would be nice if the excess parentheses were dropped from here.

> --- a/xen/include/xen/fdt-domain-build.h
> +++ b/xen/include/xen/fdt-domain-build.h
> @@ -7,6 +7,7 @@
>  #include <xen/device_tree.h>
>  #include <xen/fdt-kernel.h>
>  #include <xen/mm.h>
> +#include <xen/sched.h>
>  #include <xen/types.h>
>  
>  struct domain;
> @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info *kinfo)
>      return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
>  }
>  
> +static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)

Pointer-to-const for the 2nd parameter?

> +{
> +#ifdef CONFIG_HAS_DOMAIN_TYPE
> +    /* Type must be set before allocate memory */

This comment would be more prominent if it lived outside of the #ifdef,
perhaps (read on) ahead of the function. I wonder though why it's only
a comment, and not e.g. an assertion. If an assertion was possible to
add, the comment would want to live next to it. Without an assertion
putting it ahead of the function may be better.

Depending on how far to go, changes could be made while committing, or a
proper v5 may want submitting.

Jan

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

[Oleksii Kurochko]

On 5/4/26 2:21 PM, Jan Beulich wrote:
> On 27.04.2026 17:34, 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.
>>
>> Change the guard around access of kinfo->type to CONFIG_HAS_DOMAIN_TYPE
>> for consistency. Also, drop and add some parentheses to be aligned
>> with the similar if() below.
>>
>> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
>> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. Since x86_32 Xen no
>> longer builds, the fallback is currently only relevant for arm32.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> In principle:
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> 
> However, still a few remarks:
> 
>> --- 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 __packed 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)
>> +/* At the moment on 32-bit-only platforms all domains are 32-bit. */
>> +#define is_32bit_domain(d) (true)
>> +#define is_64bit_domain(d) (false)
> 
> I think it would be nice if the excess parentheses were dropped from here.
> 
>> --- a/xen/include/xen/fdt-domain-build.h
>> +++ b/xen/include/xen/fdt-domain-build.h
>> @@ -7,6 +7,7 @@
>>   #include <xen/device_tree.h>
>>   #include <xen/fdt-kernel.h>
>>   #include <xen/mm.h>
>> +#include <xen/sched.h>
>>   #include <xen/types.h>
>>   
>>   struct domain;
>> @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info *kinfo)
>>       return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
>>   }
>>   
>> +static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)
> 
> Pointer-to-const for the 2nd parameter?

I will apply this comment and comment above.

> 
>> +{
>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>> +    /* Type must be set before allocate memory */
> 
> This comment would be more prominent if it lived outside of the #ifdef,
> perhaps (read on) ahead of the function. I wonder though why it's only
> a comment, and not e.g. an assertion. If an assertion was possible to
> add, the comment would want to live next to it. Without an assertion
> putting it ahead of the function may be better.
> 
> Depending on how far to go, changes could be made while committing, or a
> proper v5 may want submitting.

I think that instead of comment or just after comment the following 
could be added:
   ASSERT(!domain_tot_pages(d));

Jan, Michal, do you see any concern with that ASSERT() or I could add it 
and keep your Ack-by and R-by.

Thanks.

~ Oleksii

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

[Jan Beulich]

On 06.05.2026 09:44, Oleksii Kurochko wrote:
> On 5/4/26 2:21 PM, Jan Beulich wrote:
>> On 27.04.2026 17:34, Oleksii Kurochko wrote:
>>> @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info *kinfo)
>>>       return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
>>>   }
>>>   
>>> +static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)
>>
>> Pointer-to-const for the 2nd parameter?
> 
> I will apply this comment and comment above.
> 
>>
>>> +{
>>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>>> +    /* Type must be set before allocate memory */
>>
>> This comment would be more prominent if it lived outside of the #ifdef,
>> perhaps (read on) ahead of the function. I wonder though why it's only
>> a comment, and not e.g. an assertion. If an assertion was possible to
>> add, the comment would want to live next to it. Without an assertion
>> putting it ahead of the function may be better.
>>
>> Depending on how far to go, changes could be made while committing, or a
>> proper v5 may want submitting.
> 
> I think that instead of comment or just after comment the following 
> could be added:
>    ASSERT(!domain_tot_pages(d));
> 
> Jan, Michal, do you see any concern with that ASSERT() or I could add it 
> and keep your Ack-by and R-by.

I'm okay with it being added, as long as you have made sure that it is
legitimate to have. IOW (as pointed out numerous times before) you may
not assert on state that's user/admin controlled, and that isn't covered
by another, earlier check. In such a case an error would need returning
instead.

Jan

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

[Oleksii Kurochko]

On 5/6/26 10:10 AM, Jan Beulich wrote:
> On 06.05.2026 09:44, Oleksii Kurochko wrote:
>> On 5/4/26 2:21 PM, Jan Beulich wrote:
>>> On 27.04.2026 17:34, Oleksii Kurochko wrote:
>>>> @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info *kinfo)
>>>>        return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
>>>>    }
>>>>    
>>>> +static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)
>>>
>>> Pointer-to-const for the 2nd parameter?
>>
>> I will apply this comment and comment above.
>>
>>>
>>>> +{
>>>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>>>> +    /* Type must be set before allocate memory */
>>>
>>> This comment would be more prominent if it lived outside of the #ifdef,
>>> perhaps (read on) ahead of the function. I wonder though why it's only
>>> a comment, and not e.g. an assertion. If an assertion was possible to
>>> add, the comment would want to live next to it. Without an assertion
>>> putting it ahead of the function may be better.
>>>
>>> Depending on how far to go, changes could be made while committing, or a
>>> proper v5 may want submitting.
>>
>> I think that instead of comment or just after comment the following
>> could be added:
>>     ASSERT(!domain_tot_pages(d));
>>
>> Jan, Michal, do you see any concern with that ASSERT() or I could add it
>> and keep your Ack-by and R-by.
> 
> I'm okay with it being added, as long as you have made sure that it is
> legitimate to have. IOW (as pointed out numerous times before) you may
> not assert on state that's user/admin controlled, and that isn't covered
> by another, earlier check. In such a case an error would need returning
> instead.

All callers of set_domain_type() are in the domain build path, before 
any memory allocation, so domain_tot_pages(d) being zero at this point 
is an internal code invariant, not user-controlled.

Also, I checked CI and it looks okay except ARM64 randcondig I mentioned 
in xen-devel matrix channel:
https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2503536959

~ Oleksii

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

[Orzel, Michal]

On 06-May-26 09:44, Oleksii Kurochko wrote:
> 
> 
> On 5/4/26 2:21 PM, Jan Beulich wrote:
>> On 27.04.2026 17:34, 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.
>>>
>>> Change the guard around access of kinfo->type to CONFIG_HAS_DOMAIN_TYPE
>>> for consistency. Also, drop and add some parentheses to be aligned
>>> with the similar if() below.
>>>
>>> x86 with CONFIG_64BIT=y shouldn't use is_{32,64}bit_domain() as
>>> x86 doesn't have support of CONFIG_HAS_DOMAIN_TYPE. Since x86_32 Xen no
>>> longer builds, the fallback is currently only relevant for arm32.
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>>
>> In principle:
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Thanks.
> 
>>
>> However, still a few remarks:
>>
>>> --- 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 __packed 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)
>>> +/* At the moment on 32-bit-only platforms all domains are 32-bit. */
>>> +#define is_32bit_domain(d) (true)
>>> +#define is_64bit_domain(d) (false)
>>
>> I think it would be nice if the excess parentheses were dropped from here.
>>
>>> --- a/xen/include/xen/fdt-domain-build.h
>>> +++ b/xen/include/xen/fdt-domain-build.h
>>> @@ -7,6 +7,7 @@
>>>   #include <xen/device_tree.h>
>>>   #include <xen/fdt-kernel.h>
>>>   #include <xen/mm.h>
>>> +#include <xen/sched.h>
>>>   #include <xen/types.h>
>>>   
>>>   struct domain;
>>> @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info *kinfo)
>>>       return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : kinfo->next_phandle++;
>>>   }
>>>   
>>> +static inline void set_domain_type(struct domain *d, struct kernel_info *kinfo)
>>
>> Pointer-to-const for the 2nd parameter?
> 
> I will apply this comment and comment above.
> 
>>
>>> +{
>>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>>> +    /* Type must be set before allocate memory */
>>
>> This comment would be more prominent if it lived outside of the #ifdef,
>> perhaps (read on) ahead of the function. I wonder though why it's only
>> a comment, and not e.g. an assertion. If an assertion was possible to
>> add, the comment would want to live next to it. Without an assertion
>> putting it ahead of the function may be better.
>>
>> Depending on how far to go, changes could be made while committing, or a
>> proper v5 may want submitting.
> 
> I think that instead of comment or just after comment the following 
> could be added:
>    ASSERT(!domain_tot_pages(d));
> 
> Jan, Michal, do you see any concern with that ASSERT() or I could add it 
> and keep your Ack-by and R-by.
I don't see any issues with it. You can send a v5 and I'll commit it later on.

~Michal

Re: [PATCH v4 0/3] dom0less: various updates

[Orzel, Michal]

I committed the first two patches. I decided to wait with the third patch for
Jan as he was the one with the most remarks.

~Michal

On 27-Apr-26 17:34, Oleksii Kurochko wrote:
> 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: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2482499536
> 
> ---
> Changes in v4:
>  - Rebase on top of staging.
>  - Add Review-by for patch 1 and 3.
>  - Address the comments.
> ---
> Changes in v3:
>  - Rebase on top of staging.
>  - Address the comments.
> ---
> Changes in v2:
>  - Address the comments from ML.
> ---
> 
> Oleksii Kurochko (3):
>   xen/dom0less: introduce next_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/arm/Kconfig                    |  1 +
>  xen/arch/arm/arm64/domctl.c             |  4 +--
>  xen/arch/arm/dom0less-build.c           | 14 --------
>  xen/arch/arm/domain_build.c             | 17 +++++-----
>  xen/arch/arm/include/asm/domain.h       | 16 ---------
>  xen/arch/arm/include/asm/kernel.h       |  4 ---
>  xen/arch/arm/kernel.c                   | 16 ++++-----
>  xen/common/Kconfig                      |  3 ++
>  xen/common/device-tree/dom0less-build.c | 45 ++++++++++++++++++-------
>  xen/include/xen/dom0less-build.h        |  2 --
>  xen/include/xen/domain.h                | 13 +++++++
>  xen/include/xen/fdt-domain-build.h      | 17 +++++++++-
>  xen/include/xen/fdt-kernel.h            | 11 ++++++
>  xen/include/xen/sched.h                 |  4 +++
>  14 files changed, 98 insertions(+), 69 deletions(-)
>