| Message ID | 20250109092816.1619834-1-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen/arm: Fully initialise struct membanks_hdr fields | expand |
Hi Luca, Is this patch for 4.20? I would say so, therefore it should have "for-4.20" tag and Oleksii as release manager should be CCed. Doing it now. On 09/01/2025 10:28, 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> > --- > Changes from v1: > - Changed commit title and body msg > - initialised max_banks and type for all structures using 'struct membanks_hdr' > > I didn't get why the fixes tag is wrong, but please feel free to > correct it on commit or suggest the good one It is ok. > --- > --- > 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..9e3132fb21d8 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, RESERVED_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, RESERVED_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, RESERVED_MEMORY); I'm a bit confused what is the expectations behind using different types of enum region_type, mostly because it can point to different address spaces depending on the context. Above, you marked gnttab as RESERVED_MEMORY (I guess because this region has already been found - but in fact it is still unused) and hwdom_free_mem as MEMORY. So I would at least expect ext_regions to be of MEMORY type as well. After all both hwdom_free_mem and ext_regions contain banks of unused/free memory (although former lists host memory while latter can also contain guest physical memory). Could you please clarify the intended use? ~Michal
Hi Michal, > On 9 Jan 2025, at 10:00, Michal Orzel <michal.orzel@amd.com> wrote: > > Hi Luca, > > Is this patch for 4.20? I would say so, therefore it should have "for-4.20" tag and Oleksii as release manager > should be CCed. Doing it now. Thanks, I forgot the procedure > >> --- >> --- >> 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..9e3132fb21d8 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, RESERVED_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, RESERVED_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, RESERVED_MEMORY); > I'm a bit confused what is the expectations behind using different types of enum region_type, mostly because it can point to > different address spaces depending on the context. Above, you marked gnttab as RESERVED_MEMORY (I guess because this > region has already been found - but in fact it is still unused) and hwdom_free_mem as MEMORY. So I would at least expect > ext_regions to be of MEMORY type as well. After all both hwdom_free_mem and ext_regions contain > banks of unused/free memory (although former lists host memory while latter can also contain guest physical > memory). Could you please clarify the intended use? You are right, that should be MEMORY, my bad! Could it be something addressable on commit or should I send another one? Cheers, Luca
On 09/01/2025 11:09, Luca Fancellu wrote: > > > Hi Michal, > >> On 9 Jan 2025, at 10:00, Michal Orzel <michal.orzel@amd.com> wrote: >> >> Hi Luca, >> >> Is this patch for 4.20? I would say so, therefore it should have "for-4.20" tag and Oleksii as release manager >> should be CCed. Doing it now. > > Thanks, I forgot the procedure > >> >>> --- >>> --- >>> 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..9e3132fb21d8 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, RESERVED_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, RESERVED_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, RESERVED_MEMORY); >> I'm a bit confused what is the expectations behind using different types of enum region_type, mostly because it can point to >> different address spaces depending on the context. Above, you marked gnttab as RESERVED_MEMORY (I guess because this >> region has already been found - but in fact it is still unused) and hwdom_free_mem as MEMORY. So I would at least expect >> ext_regions to be of MEMORY type as well. After all both hwdom_free_mem and ext_regions contain >> banks of unused/free memory (although former lists host memory while latter can also contain guest physical >> memory). Could you please clarify the intended use? > > You are right, that should be MEMORY, my bad! Could it be something addressable on commit or should I send another one? I can do that on commit but first, can you please answer what is the intended use of enum region_type? At first I was expecting gnttab to be MEMORY as well. I don't see a difference between a region prepared by Xen for domain to store gnttab vs extended regions. Both specify regions of unused address space. It's just that the region for gnttab must always be present but extended regions don't have to. ~Michal
>> >>> >>>> --- >>>> --- >>>> 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..9e3132fb21d8 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, RESERVED_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, RESERVED_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, RESERVED_MEMORY); >>> I'm a bit confused what is the expectations behind using different types of enum region_type, mostly because it can point to >>> different address spaces depending on the context. Above, you marked gnttab as RESERVED_MEMORY (I guess because this >>> region has already been found - but in fact it is still unused) and hwdom_free_mem as MEMORY. So I would at least expect >>> ext_regions to be of MEMORY type as well. After all both hwdom_free_mem and ext_regions contain >>> banks of unused/free memory (although former lists host memory while latter can also contain guest physical >>> memory). Could you please clarify the intended use? >> >> You are right, that should be MEMORY, my bad! Could it be something addressable on commit or should I send another one? > I can do that on commit but first, can you please answer what is the intended use of enum region_type? > At first I was expecting gnttab to be MEMORY as well. I don't see a difference between a region prepared by Xen > for domain to store gnttab vs extended regions. Both specify regions of unused address space. It's just that the region > for gnttab must always be present but extended regions don't have to. Right, at first I thought gnttab could be reserved memory, but now that you pointed out it’s not the right thing to do, I remember now that these type reflects the device tree grouping for the memory banks, so RESERVED_MEMORY is only for these regions in the /reserved-memory tree, STATIC_SHARED_MEMORY is for the 'xen,domain-shared-memory-v1’ comaptible node and MEMORY is for /memory regions. Now I would say that we could use MEMORY also for regions prepared by Xen as long as we don’t need to differentiate them in a different way from /memory regions. Please let me know your thoughts. Cheers, Luca
On 09/01/2025 11:27, Luca Fancellu wrote: > > >>> >>>> >>>>> --- >>>>> --- >>>>> 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..9e3132fb21d8 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, RESERVED_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, RESERVED_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, RESERVED_MEMORY); >>>> I'm a bit confused what is the expectations behind using different types of enum region_type, mostly because it can point to >>>> different address spaces depending on the context. Above, you marked gnttab as RESERVED_MEMORY (I guess because this >>>> region has already been found - but in fact it is still unused) and hwdom_free_mem as MEMORY. So I would at least expect >>>> ext_regions to be of MEMORY type as well. After all both hwdom_free_mem and ext_regions contain >>>> banks of unused/free memory (although former lists host memory while latter can also contain guest physical >>>> memory). Could you please clarify the intended use? >>> >>> You are right, that should be MEMORY, my bad! Could it be something addressable on commit or should I send another one? >> I can do that on commit but first, can you please answer what is the intended use of enum region_type? >> At first I was expecting gnttab to be MEMORY as well. I don't see a difference between a region prepared by Xen >> for domain to store gnttab vs extended regions. Both specify regions of unused address space. It's just that the region >> for gnttab must always be present but extended regions don't have to. > > Right, at first I thought gnttab could be reserved memory, but now that you pointed out it’s not the right thing to do, I remember > now that these type reflects the device tree grouping for the memory banks, so RESERVED_MEMORY is only for these regions > in the /reserved-memory tree, STATIC_SHARED_MEMORY is for the 'xen,domain-shared-memory-v1’ comaptible node and > MEMORY is for /memory regions. > > Now I would say that we could use MEMORY also for regions prepared by Xen as long as we don’t need to differentiate them in > a different way from /memory regions. > > Please let me know your thoughts. Yes, this is in line with my understanding. Please send a v3 with proper types as discusses. With that: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index b072a16249fe..9e3132fb21d8 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, RESERVED_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, RESERVED_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, RESERVED_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 */
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> --- Changes from v1: - Changed commit title and body msg - initialised max_banks and type for all structures using 'struct membanks_hdr' I didn't get why the fixes tag is wrong, but please feel free to correct it on commit or suggest the good one --- --- 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(-)