Commit a14593e3995a ("xen/device-tree: Allow region overlapping with
/memreserve/ ranges") introduced a type in the 'struct membanks_hdr'
but forgot to update the 'struct kernel_info' initialiser, while
it doesn't lead to failures because the field is not currently
used while managing kernel_info structures, it's good to have it
for completeness.

There are other instance of structures using 'struct membanks_hdr'
that are dynamically allocated and don't fully initialise these
fields, provide a static inline helper for that.

Fixes: a14593e3995a ("xen/device-tree: Allow region overlapping with /memreserve/ ranges")
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes from v2:
 - Banks created by Xen should be MEMORY type. (Michal)
 - Add R-by Michal
Changes from v1:
 - Changed commit title and body msg
 - initialised max_banks and type for all structures using 'struct membanks_hdr'
---
---
 xen/arch/arm/domain_build.c       | 13 ++++---------
 xen/arch/arm/include/asm/kernel.h |  5 ++++-
 xen/arch/arm/static-shmem.c       |  3 ++-
 xen/include/xen/bootfdt.h         | 16 ++++++++++++++++
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b072a16249fe..7b47abade196 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1039,7 +1039,7 @@ void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
      */
     if ( is_hardware_domain(d) )
     {
-        struct membanks *gnttab = xzalloc_flex_struct(struct membanks, bank, 1);
+        struct membanks *gnttab = membanks_xzalloc(1, MEMORY);
         /*
          * Exclude the following regions:
          * 1) Remove reserved memory
@@ -1057,13 +1057,10 @@ void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
         gnttab->bank[0].start = kinfo->gnttab_start;
         gnttab->bank[0].size = kinfo->gnttab_size;
 
-        hwdom_free_mem = xzalloc_flex_struct(struct membanks, bank,
-                                             NR_MEM_BANKS);
+        hwdom_free_mem = membanks_xzalloc(NR_MEM_BANKS, MEMORY);
         if ( !hwdom_free_mem )
             goto fail;
 
-        hwdom_free_mem->max_banks = NR_MEM_BANKS;
-
         if ( find_unallocated_memory(kinfo, mem_banks, ARRAY_SIZE(mem_banks),
                                      hwdom_free_mem, add_hwdom_free_regions) )
             goto fail;
@@ -1293,7 +1290,7 @@ static int __init find_host_extended_regions(const struct kernel_info *kinfo,
                                              struct membanks *ext_regions)
 {
     int res;
-    struct membanks *gnttab = xzalloc_flex_struct(struct membanks, bank, 1);
+    struct membanks *gnttab = membanks_xzalloc(1, MEMORY);
 
     /*
      * Exclude the following regions:
@@ -1374,12 +1371,10 @@ int __init make_hypervisor_node(struct domain *d,
     }
     else
     {
-        ext_regions = xzalloc_flex_struct(struct membanks, bank, NR_MEM_BANKS);
+        ext_regions = membanks_xzalloc(NR_MEM_BANKS, MEMORY);
         if ( !ext_regions )
             return -ENOMEM;
 
-        ext_regions->max_banks = NR_MEM_BANKS;
-
         if ( domain_use_host_layout(d) )
         {
             if ( !is_iommu_enabled(d) )
diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
index 7e6e3c82a477..de3f945ae54c 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -92,7 +92,9 @@ kernel_info_get_mem_const(const struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_STATIC_SHM
-#define KERNEL_INFO_SHM_MEM_INIT .shm_mem.common.max_banks = NR_SHMEM_BANKS,
+#define KERNEL_INFO_SHM_MEM_INIT                \
+    .shm_mem.common.max_banks = NR_SHMEM_BANKS, \
+    .shm_mem.common.type = STATIC_SHARED_MEMORY,
 #else
 #define KERNEL_INFO_SHM_MEM_INIT
 #endif
@@ -100,6 +102,7 @@ kernel_info_get_mem_const(const struct kernel_info *kinfo)
 #define KERNEL_INFO_INIT                        \
 {                                               \
     .mem.common.max_banks = NR_MEM_BANKS,       \
+    .mem.common.type = MEMORY,                  \
     KERNEL_INFO_SHM_MEM_INIT                    \
 }
 
diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
index 66088a426785..8f87154c3587 100644
--- a/xen/arch/arm/static-shmem.c
+++ b/xen/arch/arm/static-shmem.c
@@ -20,7 +20,8 @@ static struct {
     struct membanks_hdr common;
     struct membank bank[NR_SHMEM_BANKS];
 } shm_heap_banks __initdata = {
-    .common.max_banks = NR_SHMEM_BANKS
+    .common.max_banks = NR_SHMEM_BANKS,
+    .common.type = STATIC_SHARED_MEMORY
 };
 
 static inline struct membanks *get_shmem_heap_banks(void)
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index c8bbfd8979b2..80a90e53c001 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -4,6 +4,7 @@
 #include <xen/types.h>
 #include <xen/kernel.h>
 #include <xen/macros.h>
+#include <xen/xmalloc.h>
 
 #define MIN_FDT_ALIGN 8
 
@@ -219,4 +220,19 @@ static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
 }
 #endif
 
+static inline struct membanks *membanks_xzalloc(unsigned int nr,
+                                                enum region_type type)
+{
+    struct membanks *banks = xzalloc_flex_struct(struct membanks, bank, nr);
+
+    if ( !banks )
+        goto out;
+
+    banks->max_banks = nr;
+    banks->type = type;
+
+ out:
+    return banks;
+}
+
 #endif /* XEN_BOOTFDT_H */
-- 
2.34.1

On 1/9/25 2:02 PM, Luca Fancellu wrote:
> Commit a14593e3995a ("xen/device-tree: Allow region overlapping with
> /memreserve/ ranges") introduced a type in the 'struct membanks_hdr'
> but forgot to update the 'struct kernel_info' initialiser, while
> it doesn't lead to failures because the field is not currently
> used while managing kernel_info structures, it's good to have it
> for completeness.
>
> There are other instance of structures using 'struct membanks_hdr'
> that are dynamically allocated and don't fully initialise these
> fields, provide a static inline helper for that.
>
> Fixes: a14593e3995a ("xen/device-tree: Allow region overlapping with /memreserve/ ranges")
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

> ---
> Changes from v2:
>   - Banks created by Xen should be MEMORY type. (Michal)
>   - Add R-by Michal
> Changes from v1:
>   - Changed commit title and body msg
>   - initialised max_banks and type for all structures using 'struct membanks_hdr'
> ---
> ---
>   xen/arch/arm/domain_build.c       | 13 ++++---------
>   xen/arch/arm/include/asm/kernel.h |  5 ++++-
>   xen/arch/arm/static-shmem.c       |  3 ++-
>   xen/include/xen/bootfdt.h         | 16 ++++++++++++++++
>   4 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index b072a16249fe..7b47abade196 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1039,7 +1039,7 @@ void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
>        */
>       if ( is_hardware_domain(d) )
>       {
> -        struct membanks *gnttab = xzalloc_flex_struct(struct membanks, bank, 1);
> +        struct membanks *gnttab = membanks_xzalloc(1, MEMORY);
>           /*
>            * Exclude the following regions:
>            * 1) Remove reserved memory
> @@ -1057,13 +1057,10 @@ void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
>           gnttab->bank[0].start = kinfo->gnttab_start;
>           gnttab->bank[0].size = kinfo->gnttab_size;
>   
> -        hwdom_free_mem = xzalloc_flex_struct(struct membanks, bank,
> -                                             NR_MEM_BANKS);
> +        hwdom_free_mem = membanks_xzalloc(NR_MEM_BANKS, MEMORY);
>           if ( !hwdom_free_mem )
>               goto fail;
>   
> -        hwdom_free_mem->max_banks = NR_MEM_BANKS;
> -
>           if ( find_unallocated_memory(kinfo, mem_banks, ARRAY_SIZE(mem_banks),
>                                        hwdom_free_mem, add_hwdom_free_regions) )
>               goto fail;
> @@ -1293,7 +1290,7 @@ static int __init find_host_extended_regions(const struct kernel_info *kinfo,
>                                                struct membanks *ext_regions)
>   {
>       int res;
> -    struct membanks *gnttab = xzalloc_flex_struct(struct membanks, bank, 1);
> +    struct membanks *gnttab = membanks_xzalloc(1, MEMORY);
>   
>       /*
>        * Exclude the following regions:
> @@ -1374,12 +1371,10 @@ int __init make_hypervisor_node(struct domain *d,
>       }
>       else
>       {
> -        ext_regions = xzalloc_flex_struct(struct membanks, bank, NR_MEM_BANKS);
> +        ext_regions = membanks_xzalloc(NR_MEM_BANKS, MEMORY);
>           if ( !ext_regions )
>               return -ENOMEM;
>   
> -        ext_regions->max_banks = NR_MEM_BANKS;
> -
>           if ( domain_use_host_layout(d) )
>           {
>               if ( !is_iommu_enabled(d) )
> diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
> index 7e6e3c82a477..de3f945ae54c 100644
> --- a/xen/arch/arm/include/asm/kernel.h
> +++ b/xen/arch/arm/include/asm/kernel.h
> @@ -92,7 +92,9 @@ kernel_info_get_mem_const(const struct kernel_info *kinfo)
>   }
>   
>   #ifdef CONFIG_STATIC_SHM
> -#define KERNEL_INFO_SHM_MEM_INIT .shm_mem.common.max_banks = NR_SHMEM_BANKS,
> +#define KERNEL_INFO_SHM_MEM_INIT                \
> +    .shm_mem.common.max_banks = NR_SHMEM_BANKS, \
> +    .shm_mem.common.type = STATIC_SHARED_MEMORY,
>   #else
>   #define KERNEL_INFO_SHM_MEM_INIT
>   #endif
> @@ -100,6 +102,7 @@ kernel_info_get_mem_const(const struct kernel_info *kinfo)
>   #define KERNEL_INFO_INIT                        \
>   {                                               \
>       .mem.common.max_banks = NR_MEM_BANKS,       \
> +    .mem.common.type = MEMORY,                  \
>       KERNEL_INFO_SHM_MEM_INIT                    \
>   }
>   
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
> index 66088a426785..8f87154c3587 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -20,7 +20,8 @@ static struct {
>       struct membanks_hdr common;
>       struct membank bank[NR_SHMEM_BANKS];
>   } shm_heap_banks __initdata = {
> -    .common.max_banks = NR_SHMEM_BANKS
> +    .common.max_banks = NR_SHMEM_BANKS,
> +    .common.type = STATIC_SHARED_MEMORY
>   };
>   
>   static inline struct membanks *get_shmem_heap_banks(void)
> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> index c8bbfd8979b2..80a90e53c001 100644
> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -4,6 +4,7 @@
>   #include <xen/types.h>
>   #include <xen/kernel.h>
>   #include <xen/macros.h>
> +#include <xen/xmalloc.h>
>   
>   #define MIN_FDT_ALIGN 8
>   
> @@ -219,4 +220,19 @@ static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
>   }
>   #endif
>   
> +static inline struct membanks *membanks_xzalloc(unsigned int nr,
> +                                                enum region_type type)
> +{
> +    struct membanks *banks = xzalloc_flex_struct(struct membanks, bank, nr);
> +
> +    if ( !banks )
> +        goto out;
> +
> +    banks->max_banks = nr;
> +    banks->type = type;
> +
> + out:
> +    return banks;
> +}
> +
>   #endif /* XEN_BOOTFDT_H */