| Message ID | b43b4792675d89aea36cbb3974cb29eaab6f2365.1484335095.git-series.cardoe@cardoe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: > From: Daniel Kiper <daniel.kiper@oracle.com> > > There is a problem with place_string() which is used as early memory > allocator. It gets memory chunks starting from start symbol and goes > down. Sadly this does not work when Xen is loaded using multiboot2 > protocol because then the start lives on 1 MiB address and we should > not allocate a memory from below of it. So, I tried to use mem_lower > address calculated by GRUB2. However, this solution 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... :-((( > Hence, we need new memory allocator for Xen EFI boot code which is > quite simple and generic and could be used by place_string() and > efi_arch_allocate_mmap_buffer(). I think about following 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 > it in e820 memory map and map it in Xen virtual address space. This > means that the code referring to Xen command line, loaded modules and > EFI memory map, mostly in __start_xen(), will be further complicated > and diverge from legacy BIOS cases. Additionally, both former things > have to be placed below 4 GiB because their addresses are stored in > multiboot_info_t structure which has 32-bit relevant members. > > 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 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 is 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(). > > 2a) We could put memory pool into .bss.page_aligned section. Then allocate > memory chunks starting from the lowest address. After init phase we can > free unused portion of the memory pool as in case of .init.text or .init.data > sections. This way we do not need to allocate any space in image file and > freeing of unused area in the memory pool is very simple. > > Now #2a solution is implemented because it is quite simple and requires > limited number of changes, especially in __start_xen(). > > The new allocator is quite generic and can be used on ARM platforms too. > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > Reviewed-by: Doug Goldstein <cardoe@cardoe.com> You've lost (at least) Julien's and my ack here. Jan
On 1/16/17 6:52 AM, Jan Beulich wrote: >>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: >> From: Daniel Kiper <daniel.kiper@oracle.com> >> >> There is a problem with place_string() which is used as early memory >> allocator. It gets memory chunks starting from start symbol and goes >> down. Sadly this does not work when Xen is loaded using multiboot2 >> protocol because then the start lives on 1 MiB address and we should >> not allocate a memory from below of it. So, I tried to use mem_lower >> address calculated by GRUB2. However, this solution 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... :-((( >> Hence, we need new memory allocator for Xen EFI boot code which is >> quite simple and generic and could be used by place_string() and >> efi_arch_allocate_mmap_buffer(). I think about following 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 >> it in e820 memory map and map it in Xen virtual address space. This >> means that the code referring to Xen command line, loaded modules and >> EFI memory map, mostly in __start_xen(), will be further complicated >> and diverge from legacy BIOS cases. Additionally, both former things >> have to be placed below 4 GiB because their addresses are stored in >> multiboot_info_t structure which has 32-bit relevant members. >> >> 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 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 is 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(). >> >> 2a) We could put memory pool into .bss.page_aligned section. Then allocate >> memory chunks starting from the lowest address. After init phase we can >> free unused portion of the memory pool as in case of .init.text or .init.data >> sections. This way we do not need to allocate any space in image file and >> freeing of unused area in the memory pool is very simple. >> >> Now #2a solution is implemented because it is quite simple and requires >> limited number of changes, especially in __start_xen(). >> >> The new allocator is quite generic and can be used on ARM platforms too. >> >> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> >> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> > > You've lost (at least) Julien's and my ack here. > > Jan > Yes. My mistake. I pulled in the patch wholesale without adding the acks for this patch since they were replies.
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 388c4ea..62c010e 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -114,7 +114,7 @@ static void __init relocate_trampoline(unsigned long phys) static void __init place_string(u32 *addr, const char *s) { - static char *__initdata alloc = start; + char *alloc = NULL; if ( s && *s ) { @@ -122,7 +122,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 @@ -205,12 +205,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 0ccef1d..d4b7d25 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1124,8 +1124,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_LOADER) ? 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); diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index df8c702..36dbb71 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -98,6 +98,54 @@ static CHAR16 __initdata newline[] = L"\r\n"; #define PrintStr(s) StdOut->OutputString(StdOut, s) #define PrintErr(s) StdErr->OutputString(StdErr, s) +#ifdef CONFIG_ARM +/* + * TODO: Enable EFI boot allocator on ARM. + * This code can be common for x86 and ARM. + * Things TODO on ARM before enabling ebmalloc: + * - estimate required EBMALLOC_SIZE value, + * - where (in which section) ebmalloc_mem[] should live; if in + * .bss.page_aligned, as it is right now, then whole BSS zeroing + * have to be disabled in xen/arch/arm/arm64/head.S; though BSS + * should be initialized somehow before use of variables living there, + * - use ebmalloc() in ARM/common EFI boot code, + * - call free_ebmalloc_unused_mem() somewhere in init code. + */ +#define EBMALLOC_SIZE MB(0) +#else +#define EBMALLOC_SIZE MB(1) +#endif + +static char __section(".bss.page_aligned") __aligned(PAGE_SIZE) + ebmalloc_mem[EBMALLOC_SIZE]; +static unsigned long __initdata ebmalloc_allocated; + +/* EFI boot allocator. */ +static void __init __maybe_unused *ebmalloc(size_t size) +{ + void *ptr = ebmalloc_mem + ebmalloc_allocated; + + ebmalloc_allocated += (size + sizeof(void *) - 1) & ~(sizeof(void *) - 1); + + if ( ebmalloc_allocated > sizeof(ebmalloc_mem) ) + blexit(L"Out of static memory\r\n"); + + return ptr; +} + +static void __init __maybe_unused free_ebmalloc_unused_mem(void) +{ + unsigned long start, end; + + start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated); + end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem); + + destroy_xen_mappings(start, end); + init_xenheap_pages(__pa(start), __pa(end)); + + printk(XENLOG_INFO "Freed %lukB unused BSS memory\n", (end - start) >> 10); +} + /* * Include architecture specific implementation here, which references the * static globals defined above. @@ -1251,6 +1299,8 @@ void __init efi_init_memory(void) } *extra, *extra_head = NULL; #endif + free_ebmalloc_unused_mem(); + if ( !efi_enabled(EFI_BOOT) ) return;