Re: [PATCH v3 5/8] asm-generic: move some parts of Arm's domain_build.h to common

[Oleksii Kurochko]

On 5/2/25 10:55 PM, Stefano Stabellini wrote:

On Fri, 2 May 2025, Oleksii Kurochko wrote:

Nothing changed. Only some functions declaration are moved to xen/include/
headers as they are expected to be used by common code of domain builing
or dom0less.

Signed-off-by: Oleksii Kurochko<oleksii.kuroc...@gmail.com>
---
  Chnages in v3:
  - Drop inclusion of <asm/domain_build.h> from xen/fdt-domain-build.h.
  - Add empty line after license tag in xen/fdt-domain-build.h.
---
  Chnages in v2:
   - Add missed declaration of construct_hwdom().
   - Drop unnessary blank line.
   - Introduce xen/fdt-domain-build.h and move parts of Arm's domain_build.h to
     it.
   - Update the commit message.
---
  xen/arch/arm/acpi/domain_build.c        |  1 +
  xen/arch/arm/dom0less-build.c           |  1 +
  xen/arch/arm/domain_build.c             |  1 +
  xen/arch/arm/include/asm/domain_build.h | 21 ++----------
  xen/arch/arm/kernel.c                   |  1 +
  xen/arch/arm/static-shmem.c             |  1 +
  xen/include/xen/fdt-domain-build.h      | 43 +++++++++++++++++++++++++
  7 files changed, 51 insertions(+), 18 deletions(-)
  create mode 100644 xen/include/xen/fdt-domain-build.h

diff --git a/xen/arch/arm/acpi/domain_build.c b/xen/arch/arm/acpi/domain_build.c
index f9ca8b47e5..1c3555d814 100644
--- a/xen/arch/arm/acpi/domain_build.c
+++ b/xen/arch/arm/acpi/domain_build.c
@@ -10,6 +10,7 @@
   */

#include <xen/compile.h>

+#include <xen/fdt-domain-build.h>
  #include <xen/fdt-kernel.h>
  #include <xen/mm.h>
  #include <xen/sched.h>
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 7eecd06d44..0310579863 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -1,6 +1,7 @@
  /* SPDX-License-Identifier: GPL-2.0-only */
  #include <xen/device_tree.h>
  #include <xen/domain_page.h>
+#include <xen/fdt-domain-build.h>
  #include <xen/fdt-kernel.h>
  #include <xen/err.h>
  #include <xen/event.h>
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 8c7a054718..9d649b06b3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1,6 +1,7 @@
  /* SPDX-License-Identifier: GPL-2.0-only */
  #include <xen/init.h>
  #include <xen/compile.h>
+#include <xen/fdt-domain-build.h>
  #include <xen/fdt-kernel.h>
  #include <xen/lib.h>
  #include <xen/llc-coloring.h>
diff --git a/xen/arch/arm/include/asm/domain_build.h 
b/xen/arch/arm/include/asm/domain_build.h
index df1c0fe301..397e408a1f 100644
--- a/xen/arch/arm/include/asm/domain_build.h
+++ b/xen/arch/arm/include/asm/domain_build.h
@@ -5,28 +5,13 @@
  #include <xen/sched.h>

typedef __be32 gic_interrupt_t[3];

-typedef bool (*alloc_domheap_mem_cb)(struct domain *d, struct page_info *pg,
-                                     unsigned int order, void *extra);
-bool allocate_domheap_memory(struct domain *d, paddr_t tot_size,
-                             alloc_domheap_mem_cb cb, void *extra);
-bool allocate_bank_memory(struct kernel_info *kinfo, gfn_t sgfn,
-                          paddr_t tot_size);
-void allocate_memory(struct domain *d, struct kernel_info *kinfo);
-int construct_domain(struct domain *d, struct kernel_info *kinfo);
-int construct_hwdom(struct kernel_info *kinfo,
-                    const struct dt_device_node *node);
+
  int domain_fdt_begin_node(void *fdt, const char *name, uint64_t unit);
-int make_chosen_node(const struct kernel_info *kinfo);
-int make_cpus_node(const struct domain *d, void *fdt);
-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,
-                     int sizecells, const struct membanks *mem);
  int make_psci_node(void *fdt);
-int make_timer_node(const struct kernel_info *kinfo);
  void evtchn_allocate(struct domain *d);

-unsigned int get_allocation_size(paddr_t size);

+int construct_hwdom(struct kernel_info *kinfo,
+                    const struct dt_device_node *node);

At the end of the series construct_hwdom is only called from within
xen/arch/arm/domain_build.c, so it could be made static and removed from
here. However, one of my review comments was that I think we should
still call construct_hwdom from xen/common/device-tree/dom0less-build.c.
So I think we should keep it.

I will move this change to the patch where this function will be really used.

  /*
   * Helper to write an interrupts with the GIC format
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index f00fc388db..5759a3470a 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -7,6 +7,7 @@
  #include <xen/byteorder.h>
  #include <xen/domain_page.h>
  #include <xen/errno.h>
+#include <xen/fdt-domain-build.h>
  #include <xen/fdt-kernel.h>
  #include <xen/guest_access.h>
  #include <xen/gunzip.h>
diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
index 14ae48fb1e..1f8441d920 100644
--- a/xen/arch/arm/static-shmem.c
+++ b/xen/arch/arm/static-shmem.c
@@ -1,6 +1,7 @@
  /* SPDX-License-Identifier: GPL-2.0-only */

#include <xen/device_tree.h>

+#include <xen/fdt-domain-build.h>
  #include <xen/libfdt/libfdt.h>
  #include <xen/rangeset.h>
  #include <xen/sched.h>
diff --git a/xen/include/xen/fdt-domain-build.h 
b/xen/include/xen/fdt-domain-build.h
new file mode 100644
index 0000000000..b79e9fabfe
--- /dev/null
+++ b/xen/include/xen/fdt-domain-build.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __XEN_FDT_DOMAIN_BUILD_H__
+#define __XEN_FDT_DOMAIN_BUILD_H__
+
+#include <xen/bootfdt.h>
+#include <xen/device_tree.h>
+#include <xen/fdt-kernel.h>
+#include <xen/types.h>
+
+struct domain;
+struct page_info;
+struct membanks;
+
+typedef bool (*alloc_domheap_mem_cb)(struct domain *d, struct page_info *pg,
+                                     unsigned int order, void *extra);
+bool allocate_domheap_memory(struct domain *d, paddr_t tot_size,
+                             alloc_domheap_mem_cb cb, void *extra);
+
+bool allocate_bank_memory(struct kernel_info *kinfo, gfn_t sgfn,
+                          paddr_t tot_size);
+void allocate_memory(struct domain *d, struct kernel_info *kinfo);
+int construct_domain(struct domain *d, struct kernel_info *kinfo);
+int make_chosen_node(const struct kernel_info *kinfo);
+int make_cpus_node(const struct domain *d, void *fdt);
+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,
+                     int sizecells, const struct membanks *mem);
+int make_timer_node(const struct kernel_info *kinfo);
+
+unsigned int get_allocation_size(paddr_t size);

Many of these functions are not actually moved until later patches. It
would be best to move the declaration at the time the function is also
moved. But if that is difficult for any reason, this is also OK.

I'll move then allocate_*(), to the patch where defintions
for these functions are introduced.

~ Oleksii

+#endif /* __XEN_FDT_DOMAIN_BUILD_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.49.0