diff mbox

EFI_STUB fails to boot non-EFI on arm64

Message ID 20140523131656.GR4179@bivouac.eciton.net
State Accepted
Commit 74bcc2499291d38b6253f9dbd6af33a195222208
Headers show

Commit Message

Leif Lindholm May 23, 2014, 1:16 p.m. UTC 
On Fri, May 23, 2014 at 10:45:13AM +0100, Catalin Marinas wrote:
> As the EFI_STUB for arm64 got into tip and soon into -next, I thought
> about giving it a try (tip/arm64/efi). Using my boot-wrapper (non-EFI)
> is still supposed to work but I get the trace below. It looks like
> memmap.map is NULL but the code still assumes the kernel was started as
> an EFI app.
> 
> Have you guys tested these patches properly?

Apparently not sufficuently...
Sorry about that.

Fix appended:

From 98433920394730d835f0061474832909c0740f29 Mon Sep 17 00:00:00 2001
From: Leif Lindholm <leif.lindholm@linaro.org>
Date: Fri, 23 May 2014 11:23:23 +0100
Subject: [PATCH] arm64: efi: only attempt efi map setup if booting via EFI

Booting a kernel with CONFIG_EFI enabled on a non-EFI system caused
an oops with the current UEFI support code.
Add the required test to prevent this.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 arch/arm64/kernel/efi.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Matt Fleming May 23, 2014, 1:24 p.m. UTC | #1 
On Fri, 23 May, at 02:16:56PM, Leif Lindholm wrote:
> On Fri, May 23, 2014 at 10:45:13AM +0100, Catalin Marinas wrote:
> > As the EFI_STUB for arm64 got into tip and soon into -next, I thought
> > about giving it a try (tip/arm64/efi). Using my boot-wrapper (non-EFI)
> > is still supposed to work but I get the trace below. It looks like
> > memmap.map is NULL but the code still assumes the kernel was started as
> > an EFI app.
> > 
> > Have you guys tested these patches properly?
> 
> Apparently not sufficuently...
> Sorry about that.
> 
> Fix appended:

This looks pretty straight forward. I've picked it up and shoved it on
my arm64-efi branch.
Catalin Marinas May 23, 2014, 1:47 p.m. UTC | #2 
On Fri, May 23, 2014 at 02:16:56PM +0100, Leif Lindholm wrote:
> Subject: [PATCH] arm64: efi: only attempt efi map setup if booting via EFI
> 
> Booting a kernel with CONFIG_EFI enabled on a non-EFI system caused
> an oops with the current UEFI support code.
> Add the required test to prevent this.
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  arch/arm64/kernel/efi.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 7bfd650..14db1f6 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -333,6 +333,9 @@ void __init efi_init(void)
>  
>  void __init efi_idmap_init(void)
>  {
> +	if (!efi_enabled(EFI_BOOT))
> +		return;
> +

That's a first (possibly temporary) step and I think it's fine:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

But we need some further tweaking to the way we call efi_init().
Currently it doesn't matter whether Linux booted as an EFI application
or not and efi_init() is always called, causing some pr_err() in
fdt_find_uefi_params(). It's not really an error as we support the same
image booting non-EFI as well.

Can we add another of detecting whether it's an EFI application and
avoid calling efi_init()? I can see x86 sets some efi_loader_signature
string in exit_boot() and checks against it later when calling
efi_init().
diff mbox

Patch

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 7bfd650..14db1f6 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -333,6 +333,9 @@  void __init efi_init(void)
 
 void __init efi_idmap_init(void)
 {
+	if (!efi_enabled(EFI_BOOT))
+		return;
+
 	/* boot time idmap_pg_dir is incomplete, so fill in missing parts */
 	efi_setup_idmap();
 }