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

[Julien Grall <julien@xxxxxxx>]

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 0000000000..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 0000000000..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 0000000000..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 <xen/types.h>
> +
> +#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