[Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

There is a problem with place_string() which is used as early memory
allocator. It gets memory chunks starting from start symbol and
going down. Sadly this does not work when Xen is loaded using multiboot2
protocol because start lives on 1 MiB address. So, I tried to use
mem_lower address calculated by GRUB2. However, it works only on some
machines. There are machines in the wild (e.g. Dell PowerEdge R820)
which uses first ~640 KiB for boot services code or data... :-(((

In case of multiboot2 protocol we need that place_string() only allocate
memory chunk for EFI memory map. However, I think that it should be fixed
instead of making another function used just in one case. I thought about
two solutions.

1) We could use native EFI allocation functions (e.g. AllocatePool()
   or AllocatePages()) to get memory chunk. However, later (somewhere
   in __start_xen()) we must copy its contents to safe place or reserve
   this in e820 memory map and map it in Xen virtual address space.
   In later case we must also care about conflicts with e.g. crash
   kernel regions which could be quite difficult.

2) We may allocate memory area statically somewhere in Xen code which
   could be used as memory pool for early dynamic allocations. Looks
   quite simple. Additionally, it would not depend on EFI at all and
   could be used on legacy BIOS platforms if we need it. However, we
   must carefully choose size of this pool. We do not want increase
   Xen binary size too much and waste too much memory but also we must fit
   at least memory map on x86 EFI platforms. As I saw on small machine,
   e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more
   than 200 entries. Every entry on x86-64 platform is 40 bytes in size.
   So, it means that we need more than 8 KiB for EFI memory map only.
   Additionally, if we want to use this memory pool for Xen and modules
   command line storage (it would be used when xen.efi is executed as EFI
   application) then we should add, I think, about 1 KiB. In this case,
   to be on safe side, we should assume at least 64 KiB pool for early
   memory allocations, which is about 4 times of our earlier calculations.
   However, during discussion on Xen-devel Jan Beulich suggested that
   just in case we should use 1 MiB memory pool like it was in original
   place_string() implementation. So, let's use 1 MiB as it was proposed.
   If we think that we should not waste unallocated memory in the pool
   on running system then we can mark this region as __initdata and move
   all required data to dynamically allocated places somewhere in __start_xen().

Now solution #2 is implemented but maybe we should consider #1 one day.

Jan Beulich added 1b) Do away with efi_arch_allocate_mmap_buffer() and use
   AllocatePages() uniformly, perhaps with a per-arch specified memory type
   (by means of which you can control whether the memory contents will remain
   preserved until the time you want to look at it). That will eliminate the
   only place_string() you're concerned about, with a patch with better
   diffstat (largely due to the questionable arch hook gone).

However, this solution does not solve conflicts problem described in #1
because EFI memory map is needed during Xen runtime after init phase.
So, finally we would get back to #1. Hmmm... Should I check how Linux
and others cope with that problem?

Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
---
 xen/arch/x86/efi/efi-boot.h |   38 ++++++++++++++++++++++++++++++--------
 xen/arch/x86/setup.c        |    3 +--
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 6dbb14d..84afffa 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -103,9 +103,36 @@ static void __init relocate_trampoline(unsigned long phys)
         *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
 }
 
+#define EBMALLOC_SIZE  MB(1)
+
+static char __initdata ebmalloc_mem[EBMALLOC_SIZE];
+static char __initdata *ebmalloc_free = NULL;
+
+/* EFI boot allocator. */
+static void __init *ebmalloc(size_t size)
+{
+    void *ptr;
+
+    /*
+     * Init ebmalloc_free on runtime. Static initialization
+     * will not work because it puts virtual address there.
+     */
+    if ( ebmalloc_free == NULL )
+        ebmalloc_free = ebmalloc_mem;
+
+    ptr = ebmalloc_free;
+
+    ebmalloc_free += size;
+
+    if ( ebmalloc_free - ebmalloc_mem > sizeof(ebmalloc_mem) )
+        blexit(L"Out of static memory\r\n");
+
+    return ptr;
+}
+
 static void __init place_string(u32 *addr, const char *s)
 {
-    static char *__initdata alloc = start;
+    char *alloc = NULL;
 
     if ( s && *s )
     {
@@ -113,7 +140,7 @@ static void __init place_string(u32 *addr, const char *s)
         const char *old = (char *)(long)*addr;
         size_t len2 = *addr ? strlen(old) + 1 : 0;
 
-        alloc -= len1 + len2;
+        alloc = ebmalloc(len1 + len2);
         /*
          * Insert new string before already existing one. This is needed
          * for options passed on the command line to override options from
@@ -196,12 +223,7 @@ static void __init 
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
 
 static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
 {
-    place_string(&mbi.mem_upper, NULL);
-    mbi.mem_upper -= map_size;
-    mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR);
-    if ( mbi.mem_upper < xen_phys_start )
-        return NULL;
-    return (void *)(long)mbi.mem_upper;
+    return ebmalloc(map_size);
 }
 
 static void __init efi_arch_pre_exit_boot(void)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5d80868..4eb8572 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1057,8 +1057,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     if ( !xen_phys_start )
         panic("Not enough memory to relocate Xen.");
-    reserve_e820_ram(&boot_e820, efi_enabled(EFI_PLATFORM) ? mbi->mem_upper : 
__pa(&_start),
-                     __pa(&_end));
+    reserve_e820_ram(&boot_e820, __pa(&_start), __pa(&_end));
 
     /* Late kexec reservation (dynamic start address). */
     kexec_reserve_area(&boot_e820);
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel