[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/5] xen/arm: do not give memory back to static heap



Hi Luca,

On 15/11/2024 10:50, Luca Fancellu wrote:
From: Penny Zheng <Penny.Zheng@xxxxxxx>

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@xxxxxxx>

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@xxxxxxx/
---
  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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.