On 23 October 2014 18:19, Mark Salter <msalter@xxxxxxxxxx> wrote: > On Thu, 2014-10-23 at 16:54 +0100, Mark Rutland wrote: >> On Wed, Oct 22, 2014 at 06:06:56PM +0100, Mark Salter wrote: >> > On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote: >> > > On systems that boot via UEFI, all memory nodes are deleted from the >> > > device tree, and instead, the size and location of system RAM is derived >> > > from the UEFI memory map. This is handled by reserve_regions, which not only >> > > reserves parts of memory that UEFI declares as reserved, but also installs >> > > the memblocks that cover the remaining usable memory. >> > > >> > > Currently, reserve_regions() is only called if uefi_init() succeeds. >> > > However, it does not actually depend on anything that uefi_init() does, >> > > and not calling reserve_regions() results in a broken boot, so it is >> > > better to just call it unconditionally. >> > > >> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >> > > --- >> > > arch/arm64/kernel/efi.c | 11 ++++------- >> > > 1 file changed, 4 insertions(+), 7 deletions(-) >> > > >> > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c >> > > index 51522ab0c6da..4cec21b1ecdd 100644 >> > > --- a/arch/arm64/kernel/efi.c >> > > +++ b/arch/arm64/kernel/efi.c >> > > @@ -313,8 +313,7 @@ void __init efi_init(void) >> > > memmap.desc_size = params.desc_size; >> > > memmap.desc_version = params.desc_ver; >> > > >> > > - if (uefi_init() < 0) >> > > - return; >> > > + WARN_ON(uefi_init() < 0); >> > > >> > > reserve_regions(); >> > > } >> > >> > It also looks like EFI_BOOT flag will be set even if uefi_init fails. >> > If uefi_init fails, we only need reserve_regions() for the purpose >> > of adding memblocks. Otherwise, we end up wasting a lot of memory. >> >> What memory are we wasting in that case? Surely the only items that we >> could choose to throw away if we failed uefi_init are >> EFI_ACPI_RECLAIM_MEMORY and EFI_MEMORY_RUNTIME? >> >> We might want to keep those around so we can kexec into a kernel where >> we can make use of them. Surely they shouldn't take up a significant >> proportion of the available memory? > > In addition, reserve_regions() also reserves BOOT_SERVICES (which gets > freed up later after set_virtual_address_map(). That won't happen if > uefi_init() fails. In any case, if uefi_init() fails, we won't be able > to find the ACPI tables kexec kernel won't be able to either. The > BOOT_SERVICES stuff is the big chunk. There was some discussion of > not reserving that but no one has gotten around to writing the patch > to clean out the code that does it. > I am working on some patches to move the call to SetVirtualAddressMap() to the stub, allowing us to handle the first boot and subsequent kexec boots uniformly. This also means there will be no need to reserve the BOOT_SERVICES regions anymore. Perhaps we should drop this patch from this series, and I can revisit it as part of the kexec series. That can wait for 3.20, I suppose, as I don't see kexec being merged for 3.19 -- Ard. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
