On 12/12/2024 10:30, Luca Fancellu wrote:
>
>
> From: Penny Zheng <[email protected]>
>
> If the Xen heap is statically configured in Device Tree, its size is
> definite, so only the defined memory shall be given to the boot
> allocator. Have a check where init_domheap_pages() is called
> which takes into account if static heap feature is used.
>
> Extract static_heap flag from init data bootinfo, as it will be needed
> after destroying the init data section, rename it to using_static_heap
> and use it to tell whether the Xen static heap feature is enabled.
>
> Signed-off-by: Penny Zheng <[email protected]>
> Signed-off-by: Wei Chen <[email protected]>
> Signed-off-by: Luca Fancellu <[email protected]>
> ---
> Changes from v5:
> - Drop Jan R-by due to the code changes
> - Static heap is not dependent on static memory, so delete #ifdefs
> for CONFIG_STATIC_MEMORY
> Changes from v4:
> - Add R-by Jan
> - Changed code to reduce nesting in discard_initial_modules (Julien)
> Changes from v3:
> - Removed helper using_static_heap(), renamed static_heap variable
> to using_static_heap and simplified #ifdef-ary (Jan suggestion)
> Changes from v2:
> - Change xen_is_using_staticheap() to using_static_heap()
> - Move declaration of static_heap to xen/mm.h and import that in
> bootfdt.h
> - Reprased first part of the commit message
> Changes from v1:
> - moved static_heap to common/page_alloc.c
> - protect static_heap access with CONFIG_STATIC_MEMORY
> - update comment in arm/kernel.c kernel_decompress()
> ---
> ---
> xen/arch/arm/arm32/mmu/mm.c | 4 ++--
> xen/arch/arm/kernel.c | 7 ++++---
> xen/arch/arm/mmu/setup.c | 8 ++++++--
> xen/arch/arm/setup.c | 3 +++
> xen/common/device-tree/bootfdt.c | 2 +-
> xen/common/device-tree/bootinfo.c | 2 +-
> xen/common/page_alloc.c | 3 +++
> xen/include/xen/bootfdt.h | 1 -
> xen/include/xen/mm.h | 2 ++
> 9 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
> index 063611412be0..0824d61323b5 100644
> --- a/xen/arch/arm/arm32/mmu/mm.c
> +++ b/xen/arch/arm/arm32/mmu/mm.c
> @@ -199,7 +199,7 @@ void __init setup_mm(void)
>
> total_pages = ram_size >> PAGE_SHIFT;
>
> - if ( bootinfo.static_heap )
> + if ( using_static_heap )
> {
> const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
>
> @@ -246,7 +246,7 @@ void __init setup_mm(void)
>
> do
> {
> - e = bootinfo.static_heap ?
> + e = using_static_heap ?
> fit_xenheap_in_static_heap(pfn_to_paddr(xenheap_pages), MB(32)) :
> consider_modules(ram_start, ram_end,
> pfn_to_paddr(xenheap_pages),
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 293d7efaed9c..8270684246ea 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -244,10 +244,11 @@ static __init int kernel_decompress(struct bootmodule
> *mod, uint32_t offset)
> size += offset;
>
> /*
> - * Free the original kernel, update the pointers to the
> - * decompressed kernel
> + * In case Xen is not using the static heap feature, free the original
> + * kernel, update the pointers to the decompressed kernel
> */
> - fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0);
> + if ( !using_static_heap )
You should get out of this function even earlier, before calculating addr and
size that are only
used in fw_unreserved_regions.
> + fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0);
>
> return 0;
> }
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index 9664e85ee6c0..8c87649bc88e 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -341,8 +341,12 @@ void free_init_memory(void)
> if ( rc )
> panic("Unable to remove the init section (rc = %d)\n", rc);
>
> - init_domheap_pages(pa, pa + len);
> - printk("Freed %ldkB init memory.\n",
> (long)(__init_end-__init_begin)>>10);
> + if ( !using_static_heap )
So here, you allow the init region mappings to be destroyed (above), yet ...
> + {
> + init_domheap_pages(pa, pa + len);
> + printk("Freed %ldkB init memory.\n",
> + (long)(__init_end-__init_begin) >> 10);
> + }
> }
>
> /**
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 2e27af4560a5..85f743a2c6ad 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -206,6 +206,9 @@ void __init discard_initial_modules(void)
> struct bootmodules *mi = &bootinfo.modules;
> int i;
>
> + if ( using_static_heap )
> + return;
... here you would get out without calling remove_early_mappings() that today
destroys early FDT
mappings. IMO you should allow to call remove_early_mappings().
With the remarks addressed:
Reviewed-by: Michal Orzel <[email protected]>
~Michal
