From: Penny Zheng <Penny.Zheng@arm.com>
If Xenheap is statically configured in Device Tree, its size is
definite. So, the memory shall not be given back into static heap, like
it's normally done in free_init_memory, etc, once the initialization
is finished.
Extract static_heap flag from init data bootinfo, as it will be needed
after destroying the init data section.
Introduce a new helper xen_is_using_staticheap() to tell whether Xenheap
is statically configured in the Device Tree.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
This is a rebase of this one: https://patchwork.kernel.org/project/xen-devel/patch/20230626033443.2943270-18-Penny.Zheng@arm.com/
---
xen/arch/arm/arm32/mmu/mm.c | 4 ++--
xen/arch/arm/kernel.c | 3 ++-
xen/arch/arm/mmu/setup.c | 8 ++++++--
xen/arch/arm/setup.c | 27 ++++++++++++++-------------
xen/common/device-tree/bootfdt.c | 2 +-
xen/common/device-tree/bootinfo.c | 2 +-
xen/common/device-tree/device-tree.c | 3 +++
xen/include/xen/bootfdt.h | 11 ++++++++++-
8 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
index 063611412be0..b7ca7c94c9ca 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 ( xen_is_using_staticheap() )
{
const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
@@ -246,7 +246,7 @@ void __init setup_mm(void)
do
{
- e = bootinfo.static_heap ?
+ e = xen_is_using_staticheap() ?
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..a4a99607b668 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -247,7 +247,8 @@ static __init int kernel_decompress(struct bootmodule *mod, uint32_t offset)
* Free the original kernel, update the pointers to the
* decompressed kernel
*/
- fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0);
+ if ( !xen_is_using_staticheap() )
+ 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 1b1d302c8788..d0775793f4b4 100644
--- a/xen/arch/arm/mmu/setup.c
+++ b/xen/arch/arm/mmu/setup.c
@@ -343,8 +343,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 ( !xen_is_using_staticheap() )
+ {
+ 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 71ebaa77ca94..91340d5dc201 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -206,24 +206,25 @@ void __init discard_initial_modules(void)
struct bootmodules *mi = &bootinfo.modules;
int i;
- for ( i = 0; i < mi->nr_mods; i++ )
+ if ( !xen_is_using_staticheap() )
{
- paddr_t s = mi->module[i].start;
- paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
-
- if ( mi->module[i].kind == BOOTMOD_XEN )
- continue;
+ for ( i = 0; i < mi->nr_mods; i++ )
+ {
+ paddr_t s = mi->module[i].start;
+ paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
- if ( !mfn_valid(maddr_to_mfn(s)) ||
- !mfn_valid(maddr_to_mfn(e)) )
- continue;
+ if ( mi->module[i].kind == BOOTMOD_XEN )
+ continue;
- fw_unreserved_regions(s, e, init_domheap_pages, 0);
- }
+ if ( !mfn_valid(maddr_to_mfn(s)) ||
+ !mfn_valid(maddr_to_mfn(e)) )
+ continue;
- mi->nr_mods = 0;
+ fw_unreserved_regions(s, e, init_domheap_pages, 0);
+ }
- remove_early_mappings();
+ mi->nr_mods = 0;
+ }
}
/* Relocate the FDT in Xen heap */
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index 927f59c64b0d..ccb150b34a63 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -403,7 +403,7 @@ static int __init process_chosen_node(const void *fdt, int node,
if ( rc )
return rc;
- bootinfo.static_heap = true;
+ static_heap = true;
}
printk("Checking for initrd in /chosen\n");
diff --git a/xen/common/device-tree/bootinfo.c b/xen/common/device-tree/bootinfo.c
index f2e6a1145b7c..1e83d5172938 100644
--- a/xen/common/device-tree/bootinfo.c
+++ b/xen/common/device-tree/bootinfo.c
@@ -386,7 +386,7 @@ void __init populate_boot_allocator(void)
const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
paddr_t s, e;
- if ( bootinfo.static_heap )
+ if ( xen_is_using_staticheap() )
{
for ( i = 0 ; i < reserved_mem->nr_banks; i++ )
{
diff --git a/xen/common/device-tree/device-tree.c b/xen/common/device-tree/device-tree.c
index d0528c582565..22b69c49171b 100644
--- a/xen/common/device-tree/device-tree.c
+++ b/xen/common/device-tree/device-tree.c
@@ -25,6 +25,9 @@
#include <asm/setup.h>
#include <xen/err.h>
+/* Flag saved when Xen is using the static heap feature (xen,static-heap) */
+bool __read_mostly static_heap;
+
const void *device_tree_flattened;
dt_irq_xlate_func dt_irq_xlate;
/* Host device tree */
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 16fa05f38f38..0015a4babde7 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -132,7 +132,6 @@ struct bootinfo {
#ifdef CONFIG_STATIC_SHM
struct shared_meminfo shmem;
#endif
- bool static_heap;
};
#ifdef CONFIG_ACPI
@@ -156,6 +155,7 @@ struct bootinfo {
}
extern struct bootinfo bootinfo;
+extern bool static_heap;
bool check_reserved_regions_overlap(paddr_t region_start,
paddr_t region_size);
@@ -206,4 +206,13 @@ static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
}
#endif
+static inline bool xen_is_using_staticheap(void)
+{
+#ifdef CONFIG_STATIC_MEMORY
+ return static_heap;
+#else
+ return false;
+#endif
+}
+
#endif /* XEN_BOOTFDT_H */
--
2.34.1Hi Luca,
On 15/11/2024 10:50, Luca Fancellu wrote:
> From: Penny Zheng <Penny.Zheng@arm.com>
>
> If Xenheap is statically configured in Device Tree, its size is
> definite. So, the memory shall not be given back into static heap, like
> it's normally done in free_init_memory, etc, once the initialization
> is finished.
>
> Extract static_heap flag from init data bootinfo, as it will be needed
> after destroying the init data section.
> Introduce a new helper xen_is_using_staticheap() to tell whether Xenheap
> is statically configured in the Device Tree.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Similar to a previous patch, why wre Penny and Wei's Signed-off-by removed?
> ---
> This is a rebase of this one: https://patchwork.kernel.org/project/xen-devel/patch/20230626033443.2943270-18-Penny.Zheng@arm.com/
> ---
> xen/arch/arm/arm32/mmu/mm.c | 4 ++--
> xen/arch/arm/kernel.c | 3 ++-
> xen/arch/arm/mmu/setup.c | 8 ++++++--
> xen/arch/arm/setup.c | 27 ++++++++++++++-------------
> xen/common/device-tree/bootfdt.c | 2 +-
> xen/common/device-tree/bootinfo.c | 2 +-
> xen/common/device-tree/device-tree.c | 3 +++
> xen/include/xen/bootfdt.h | 11 ++++++++++-
> 8 files changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
> index 063611412be0..b7ca7c94c9ca 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 ( xen_is_using_staticheap() )
> {
> const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
>
> @@ -246,7 +246,7 @@ void __init setup_mm(void)
>
> do
> {
> - e = bootinfo.static_heap ?
> + e = xen_is_using_staticheap() ?
> 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..a4a99607b668 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -247,7 +247,8 @@ static __init int kernel_decompress(struct bootmodule *mod, uint32_t offset)
> * Free the original kernel, update the pointers to the
> * decompressed kernel
> */
> - fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0);
> + if ( !xen_is_using_staticheap() )
The comment on top needs to be updated.
> + 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 1b1d302c8788..d0775793f4b4 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -343,8 +343,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 ( !xen_is_using_staticheap() )
> + {
> + 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 71ebaa77ca94..91340d5dc201 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -206,24 +206,25 @@ void __init discard_initial_modules(void)
> struct bootmodules *mi = &bootinfo.modules;
> int i;
>
> - for ( i = 0; i < mi->nr_mods; i++ )
> + if ( !xen_is_using_staticheap() )
> {
> - paddr_t s = mi->module[i].start;
> - paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
> -
> - if ( mi->module[i].kind == BOOTMOD_XEN )
> - continue;
> + for ( i = 0; i < mi->nr_mods; i++ )
> + {
> + paddr_t s = mi->module[i].start;
> + paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
>
> - if ( !mfn_valid(maddr_to_mfn(s)) ||
> - !mfn_valid(maddr_to_mfn(e)) )
> - continue;
> + if ( mi->module[i].kind == BOOTMOD_XEN )
> + continue;
>
> - fw_unreserved_regions(s, e, init_domheap_pages, 0);
> - }
> + if ( !mfn_valid(maddr_to_mfn(s)) ||
> + !mfn_valid(maddr_to_mfn(e)) )
> + continue;
>
> - mi->nr_mods = 0;
> + fw_unreserved_regions(s, e, init_domheap_pages, 0);
> + }
>
> - remove_early_mappings();
> + mi->nr_mods = 0;
> + }
> }
>
> /* Relocate the FDT in Xen heap */
> diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
> index 927f59c64b0d..ccb150b34a63 100644
> --- a/xen/common/device-tree/bootfdt.c
> +++ b/xen/common/device-tree/bootfdt.c
> @@ -403,7 +403,7 @@ static int __init process_chosen_node(const void *fdt, int node,
> if ( rc )
> return rc;
>
> - bootinfo.static_heap = true;
> + static_heap = true;
> }
>
> printk("Checking for initrd in /chosen\n");
> diff --git a/xen/common/device-tree/bootinfo.c b/xen/common/device-tree/bootinfo.c
> index f2e6a1145b7c..1e83d5172938 100644
> --- a/xen/common/device-tree/bootinfo.c
> +++ b/xen/common/device-tree/bootinfo.c
> @@ -386,7 +386,7 @@ void __init populate_boot_allocator(void)
> const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
> paddr_t s, e;
>
> - if ( bootinfo.static_heap )
> + if ( xen_is_using_staticheap() )
> {
> for ( i = 0 ; i < reserved_mem->nr_banks; i++ )
> {
> diff --git a/xen/common/device-tree/device-tree.c b/xen/common/device-tree/device-tree.c
> index d0528c582565..22b69c49171b 100644
> --- a/xen/common/device-tree/device-tree.c
> +++ b/xen/common/device-tree/device-tree.c
> @@ -25,6 +25,9 @@
> #include <asm/setup.h>
> #include <xen/err.h>
>
> +/* Flag saved when Xen is using the static heap feature (xen,static-heap) */
> +bool __read_mostly static_heap;
Strictly speaking, static_heap could be used with ACPI (even though
there is not binding today). So I think it should not belong to
device-tree.c. I think page_alloc.c may be more suitable. Also, I think
static_heap will not be touched after init. So this likely wants to be
__ro_after_init.
Lastly, shouldn't this be protected by #ifdef? Otherwise...
> +
> const void *device_tree_flattened;
> dt_irq_xlate_func dt_irq_xlate;
> /* Host device tree */
> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
> index 16fa05f38f38..0015a4babde7 100644
> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -132,7 +132,6 @@ struct bootinfo {
> #ifdef CONFIG_STATIC_SHM
> struct shared_meminfo shmem;
> #endif
> - bool static_heap;
> };
>
> #ifdef CONFIG_ACPI
> @@ -156,6 +155,7 @@ struct bootinfo {
> }
>
> extern struct bootinfo bootinfo;
> +extern bool static_heap;
>
> bool check_reserved_regions_overlap(paddr_t region_start,
> paddr_t region_size);
> @@ -206,4 +206,13 @@ static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void)
> }
> #endif
>
> +static inline bool xen_is_using_staticheap(void)
... we could have xen_is_using_staticheap() != static_heap.
> +{
> +#ifdef CONFIG_STATIC_MEMORY
> + return static_heap;
> +#else
> + return false;
> +#endif
> +}
> +
> #endif /* XEN_BOOTFDT_H */
Cheers,
--
Julien Grall
Hi Julien, >> - fw_unreserved_regions(addr, addr + size, init_domheap_pages, 0); >> + if ( !xen_is_using_staticheap() ) > > The comment on top needs to be updated. I’ll update, is this ok: /* * In case Xen is not using the static heap feature, free the original * kernel, update the pointers to the decompressed kernel */ >> >> diff --git a/xen/common/device-tree/device-tree.c b/xen/common/device-tree/device-tree.c >> index d0528c582565..22b69c49171b 100644 >> --- a/xen/common/device-tree/device-tree.c >> +++ b/xen/common/device-tree/device-tree.c >> @@ -25,6 +25,9 @@ >> #include <asm/setup.h> >> #include <xen/err.h> >> +/* Flag saved when Xen is using the static heap feature (xen,static-heap) */ >> +bool __read_mostly static_heap; > > Strictly speaking, static_heap could be used with ACPI (even though there is not binding today). So I think it should not belong to device-tree.c. I think page_alloc.c may be more suitable. Also, I think static_heap will not be touched after init. So this likely wants to be __ro_after_init. Sure, I’ll do the modifications and I’ll move to common/page_alloc.c > > Lastly, shouldn't this be protected by #ifdef? Otherwise... Could you clarify if I understood correctly? If I protect static_heap with CONFIG_STATIC_MEMORY, then I have to protect also the code in https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/device-tree/bootfdt.c;h=927f59c64b0d64842e2a0fd09562ac919c204e6e;hb=refs/heads/staging#l393, is this what you are expecting? And in that case, should it be only to protect the access to the variable or the all block? For example now if CONFIG_STATIC_MEMORY is not set, I think we parse anyway the xen,static-mem, so this tends me to think I should protect only the variable. Cheers, Luca
© 2016 - 2025 Red Hat, Inc.