From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Fleming Subject: Re: [PATCH] x86/efi: delay efi_esrt_init to efi_late_init Date: Thu, 11 Aug 2016 11:29:55 +0100 Message-ID: <20160811102955.GB30909@codeblueprint.co.uk> References: <1470694104-9278-1-git-send-email-ricardo.neri-calderon@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1470694104-9278-1-git-send-email-ricardo.neri-calderon-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Ricardo Neri Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Peter Jones , Ard Biesheuvel List-Id: linux-efi@vger.kernel.org On Mon, 08 Aug, at 03:08:24PM, Ricardo Neri wrote: > Commit 7b02d53e7852 introduced a new efi_mem_reserve to reserve the boot > services memory regions forever. This reservation involves allocating a new > EFI memory range descriptor. However, allocation can only succeed if there > is memory available for the allocation. Otherwise, error such as the > following may occur: > > esrt: Reserving ESRT space from 0x000000003dd6a000 to 0x000000003dd6a010. > Kernel panic - not syncing: ERROR: Failed to allocate 0x9f0 bytes below 0x0. > CPU: 0 PID: 0 Comm: swapper Not tainted 4.7.0-rc5+ #503 > 0000000000000000 ffffffff81e03ce0 ffffffff8131dae8 ffffffff81bb6c50 > ffffffff81e03d70 ffffffff81e03d60 ffffffff8111f4df 0000000000000018 > ffffffff81e03d70 ffffffff81e03d08 00000000000009f0 00000000000009f0 > Call Trace: > [] dump_stack+0x4d/0x65 > [] panic+0xc5/0x206 > [] memblock_alloc_base+0x29/0x2e > [] memblock_alloc+0xb/0xd > [] efi_arch_mem_reserve+0xbc/0x134 > [] efi_mem_reserve+0x2c/0x31 > [] ? efi_mem_reserve+0x2c/0x31 > [] efi_esrt_init+0x19e/0x1b4 > [] efi_init+0x398/0x44a > [] setup_arch+0x415/0xc30 > [] start_kernel+0x5b/0x3ef > [] x86_64_start_reservations+0x2f/0x31 > [] x86_64_start_kernel+0xea/0xed > ---[ end Kernel panic - not syncing: ERROR: Failed to allocate 0x9f0 > bytes below 0x0. > > An inspection of the memblock configuration reveals that there is no memory > available for the allocation: > > MEMBLOCK configuration: > memory size = 0x0 reserved size = 0x4f339c0 > memory.cnt = 0x1 > memory[0x0] [0x00000000000000-0xffffffffffffffff], 0x0 bytes on node 0 flags: 0x0 > reserved.cnt = 0x4 > reserved[0x0] [0x0000000008c000-0x0000000008c9bf], 0x9c0 bytes flags: 0x0 > reserved[0x1] [0x0000000009f000-0x000000000fffff], 0x61000 bytes flags: 0x0 > reserved[0x2] [0x00000002800000-0x0000000394bfff], 0x114c000 bytes flags: 0x0 > reserved[0x3] [0x000000304e4000-0x00000034269fff], 0x3d86000 bytes flags: 0x0 > > efi_esrt_init is called from efi_init, which in x86 happens before > memblock_x86_fill is called. Since ESRT is not critical to boot but to > runtime, its initialization can be safely delayed to efi_late_init. Ouch. This looks like the right thing to do now that we no longer need to reserve the ESRT data before efi_reserve_boot_services() runs. Ard, could you confirm that this isn't does not exist for ARM/arm64? It looks like reserve_regions() takes care of adding all RAM from the EFI memmap, so efi_esrt_init() is free to make allocations. Actually Ricardo, there's a slight hiccup with this patch in that efi_esrt_init() is now being called at very different points during boot. ARM/arm64 call it at efi_init() time and the early_memremap() calls in the ESRT driver make sense, but those calls don't make sense when invoked from efi_late_init() on x86. Look how the ACPI BGRT driver makes use of the standard memremap() API. Instead, could you fold this call into the same location as efi_find_mirror() and co. in x86's setup_arch()?