Re: [PATCH v4 2/6] xen/device-tree: Move Arm's setup.c bootinfo functions to common

2024-04-24 Thread Julien Grall

Hi Shawn,

On 12/04/2024 04:55, Shawn Anastasio wrote:

diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
new file mode 100644
index 00..947bad979c
--- /dev/null
+++ b/xen/common/device-tree/Makefile
@@ -0,0 +1 @@
+obj-y += bootinfo.init.o
diff --git a/xen/common/device-tree/bootinfo.c 
b/xen/common/device-tree/bootinfo.c
new file mode 100644
index 00..914f876d29
--- /dev/null
+++ b/xen/common/device-tree/bootinfo.c
@@ -0,0 +1,446 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Derived from $xen/arch/arm/setup.c.
+ *
+ * Early device tree parsing and bookkeeping routines.


Based on the discussion in v3, I think this is only containing 
bookkepping routines. So you probably want to remove "Early device tree 
parsing".


This would also make a bit clearer the difference bettwen bootfdt.c and 
bootinfo.c.


[...]


+/*
+ * Populate the boot allocator.
+ * If a static heap was not provided by the admin, all the RAM but the
+ * following regions will be added:
+ *  - Modules (e.g., Xen, Kernel)
+ *  - Reserved regions
+ *  - Xenheap (CONFIG_SEPARATE_XENHEAP only)
+ * If a static heap was provided by the admin, populate the boot
+ * allocator with the corresponding regions only, but with Xenheap excluded
+ * on arm32.


Now this is moved in common code, I think we want to 
s/arm32/CONFIG_SEPARATE_XENHEAP/. This could be done on a follow-up commit.


[...]


diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
new file mode 100644
index 00..b0487bdbbd
--- /dev/null
+++ b/xen/include/xen/bootfdt.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __XEN_BOOTFDT_H__
+#define __XEN_BOOTFDT_H__
+
+#include 
+
+#define MIN_FDT_ALIGN 8
+#define MAX_FDT_SIZE SZ_2M
+
+#define NR_MEM_BANKS 256
+
+#define MAX_MODULES 32 /* Current maximum useful modules */
+
+typedef enum {
+BOOTMOD_XEN,
+BOOTMOD_FDT,
+BOOTMOD_KERNEL,
+BOOTMOD_RAMDISK,
+BOOTMOD_XSM,
+BOOTMOD_GUEST_DTB,
+BOOTMOD_UNKNOWN
+} bootmodule_kind;
+
+enum membank_type {
+/*
+ * The MEMBANK_DEFAULT type refers to either reserved memory for the
+ * device/firmware (when the bank is in 'reserved_mem') or any RAM (when
+ * the bank is in 'mem').
+ */
+MEMBANK_DEFAULT,
+/*
+ * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory
+ * bank is bound to a static Xen domain. It is only valid when the bank
+ * is in reserved_mem.
+ */
+MEMBANK_STATIC_DOMAIN,
+/*
+ * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory
+ * bank is reserved as static heap. It is only valid when the bank is
+ * in reserved_mem.
+ */
+MEMBANK_STATIC_HEAP,
+};
+
+/* Indicates the maximum number of characters(\0 included) for shm_id */
+#define MAX_SHM_ID_LENGTH 16
+
+struct membank {
+paddr_t start;
+paddr_t size;
+enum membank_type type;
+#ifdef CONFIG_STATIC_SHM
+char shm_id[MAX_SHM_ID_LENGTH];
+unsigned int nr_shm_borrowers;
+#endif
+};
+
+struct meminfo {
+unsigned int nr_banks;
+struct membank bank[NR_MEM_BANKS];
+};


I have just committed some code which is modifying membank/meminfo. I 
think this may need to be rebased. I have CCed the author of the series 
(Luca) who may be able to advise/help.


Cheers,

--
Julien Grall



Re: [PATCH v4 2/6] xen/device-tree: Move Arm's setup.c bootinfo functions to common

2024-04-18 Thread Jan Beulich
On 12.04.2024 05:55, Shawn Anastasio wrote:
> Arm's setup.c contains a collection of functions for parsing memory map
> and other boot information from a device tree. Since these routines are
> generally useful on any architecture that supports device tree booting,
> move them into xen/common/device-tree.
> 
> Suggested-by: Julien Grall 
> Signed-off-by: Shawn Anastasio 
> ---
> Changes in v4:
>   - create new xen/include/bootinfo.h rather than relying on arch's
> asm/setup.h to provide required definitions for bootinfo.c
>   - build bootinfo.c as .init.o
>   - clean up and sort bootinfo.c's #includes
>   - use CONFIG_SEPARATE_XENHEAP rather than CONFIG_ARM_32 to guard
> xenheap-specific behavior of populate_boot_allocator
>   - (MAINTAINERS) include all of common/device-tree rather than just
> bootinfo.c
> 
>  MAINTAINERS   |   1 +
>  xen/arch/arm/include/asm/setup.h  | 109 +---
>  xen/arch/arm/setup.c  | 419 
>  xen/common/Makefile   |   1 +
>  xen/common/device-tree/Makefile   |   1 +
>  xen/common/device-tree/bootinfo.c | 446 ++
>  xen/include/xen/bootfdt.h | 116 
>  7 files changed, 566 insertions(+), 527 deletions(-)
>  create mode 100644 xen/common/device-tree/Makefile
>  create mode 100644 xen/common/device-tree/bootinfo.c
>  create mode 100644 xen/include/xen/bootfdt.h

Should this further new file ...

> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -295,6 +295,7 @@ M:Stefano Stabellini 
>  M:   Julien Grall 
>  S:   Supported
>  F:   xen/common/libfdt/
> +F:   xen/common/device-tree/
>  F:   xen/common/device_tree.c
>  F:   xen/common/dt-overlay.c
>  F:   xen/include/xen/libfdt/

... perhaps also be added here?

Jan