From patchwork Fri Apr 15 12:33:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Kiper X-Patchwork-Id: 8850771 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 850A6C0553 for ; Fri, 15 Apr 2016 12:36:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6FAED202DD for ; Fri, 15 Apr 2016 12:36:33 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6243320380 for ; Fri, 15 Apr 2016 12:36:32 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ar2xO-0005g9-8w; Fri, 15 Apr 2016 12:34:42 +0000 Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ar2xM-0005dT-Hd for xen-devel@lists.xenproject.org; Fri, 15 Apr 2016 12:34:40 +0000 Received: from [85.158.143.35] by server-1.bemta-6.messagelabs.com id 8B/9C-18833-0EFD0175; Fri, 15 Apr 2016 12:34:40 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrFLMWRWlGSWpSXmKPExsXSO6nOVff+fYF wg72XjC2+b5nM5MDocfjDFZYAxijWzLyk/IoE1ow5LyaxFTw0qOia28/UwPhDrYuRk0NIoI1J orOHtYuRC8j+xiixfPJSFghnA6PEth132CCciYwSS2f9ZQJpYRPQkbj45SE7iC0ioCRxb9VkJ pAiZoHzTBLTOi8DtXNwCAt4SVx/zQlSwyKgKtE56ywziM0r4CFxcBLIBk4OCQFFie5nE9hAbE 6g+OrOJrBWIQF3iX9TRSBKDCVOP9zGOIGRbwEjwypG9eLUorLUIl1jvaSizPSMktzEzBxdQwM zvdzU4uLE9NScxKRiveT83E2MwDBhAIIdjB3/nA4xSnIwKYnyzt0rEC7El5SfUpmRWJwRX1Sa k1p8iFGGg0NJgvfJPaCcYFFqempFWmYOMGBh0hIcPEoivC9B0rzFBYm5xZnpEKlTjIpS4rzvQ RICIImM0jy4NliUXGKUlRLmZQQ6RIinILUoN7MEVf4VozgHo5Iw72uQKTyZeSVw018BLWYCWl z2jhdkcUkiQkqqgXHKthUs4uKf7kTbaaR4RJ6Tf39oBrtDzI37zxfkbpzlKfNhlTav0jm56V0 fKhbOmfTFVtt1ToXzDc0262VxO3YubjDNjf5hoNbyLK77fuA39z9K9VdmrRO7KnXxyHxu5pY1 xT89HJ208zOaza39RISLObbfWfHkPp/xvHlV/x4+2vRGqZ9VzVqJpTgj0VCLuag4EQBOjdnnj QIAAA== X-Env-Sender: daniel.kiper@oracle.com X-Msg-Ref: server-15.tower-21.messagelabs.com!1460723678!9526084!1 X-Originating-IP: [141.146.126.69] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTQxLjE0Ni4xMjYuNjkgPT4gMjc3MjE4\n X-StarScan-Received: X-StarScan-Version: 8.28; banners=-,-,- X-VirusChecked: Checked Received: (qmail 14126 invoked from network); 15 Apr 2016 12:34:39 -0000 Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by server-15.tower-21.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 15 Apr 2016 12:34:39 -0000 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u3FCYEwP027433 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 15 Apr 2016 12:34:14 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0021.oracle.com (8.13.8/8.13.8) with ESMTP id u3FCYDBl017009 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 15 Apr 2016 12:34:13 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u3FCYDTo016547; Fri, 15 Apr 2016 12:34:13 GMT Received: from olila.local.net-space.pl (/10.175.160.106) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 15 Apr 2016 05:34:12 -0700 From: Daniel Kiper To: xen-devel@lists.xenproject.org Date: Fri, 15 Apr 2016 14:33:12 +0200 Message-Id: <1460723596-13261-13-git-send-email-daniel.kiper@oracle.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1460723596-13261-1-git-send-email-daniel.kiper@oracle.com> References: <1460723596-13261-1-git-send-email-daniel.kiper@oracle.com> X-Source-IP: userv0021.oracle.com [156.151.31.71] Cc: jgross@suse.com, andrew.cooper3@citrix.com, stefano.stabellini@eu.citrix.com, cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com, david.vrabel@citrix.com, jbeulich@suse.com, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org Subject: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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);