Re: [PATCH 1/2] UEFI arm64: add noefi boot param

[Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>]

On Wed, 06 Aug, at 02:29:41PM, Leif Lindholm wrote:
> On Wed, Aug 06, 2014 at 02:20:21PM +0100, Matt Fleming wrote:
> > > Since this is really turning an x86-specific feature into a generic
> > > one, could it be moved to core code?
> > > Maybe an efi.mask, reusing the efi_enabled defines, with an
> > > efi_disabled macro?
> >  
> > Why the new efi_disabled() and efi.mask? This is all achievable with
> > efi_enabled() and efi.flags, in fact, this kind of thing is exactly why
> > they were invented.
> 
> Because this flag is indicating which facilities we should not try to
> enable, rather than which facilities we have enabled.
> 
> The EFI_RUNTIME_SERVICES flag is set after successful call to
> set_virtual_address_map. The apparent intent of "noefi" is to prevent
> that call from being made in the first place.
> 
> Anyway, it was just a suggestion - main point was it would make sense
> to share the code.

Definitely.

Dave, below is the kind of thing that I had in mind. Please Cc the Xen
and SGI folks when you submit your next version because they're likely
to be hit the hardest by any changes to EFI_RUNTIME_SERVICES and
"noefi".

Obviously the addition of "efi=noruntime" is needed on top, but that's
about 6 lines of code.

---
 arch/arm64/kernel/efi.c     |  4 ++--
 arch/x86/kernel/setup.c     |  4 +++-
 arch/x86/platform/efi/efi.c | 33 +++++++++------------------------
 drivers/firmware/efi/efi.c  |  7 +++++++
 4 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index e72f3100958f..9fdedb721a4e 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -83,6 +83,7 @@ static int __init uefi_init(void)
 
 	set_bit(EFI_BOOT, &efi.flags);
 	set_bit(EFI_64BIT, &efi.flags);
+	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
 	/*
 	 * Verify the EFI Table
@@ -386,7 +387,7 @@ static int __init arm64_enter_virtual_mode(void)
 	int count = 0;
 	unsigned long flags;
 
-	if (!efi_enabled(EFI_BOOT)) {
+	if (!efi_enabled(EFI_BOOT) || !efi_enabled(EFI_RUNTIME_SERVICES)) {
 		pr_info("EFI services will not be available.\n");
 		return -1;
 	}
@@ -461,7 +462,6 @@ static int __init arm64_enter_virtual_mode(void)
 	/* Set up runtime services function pointers */
 	runtime = efi.systab->runtime;
 	efi_native_runtime_setup();
-	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
 	return 0;
 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 41ead8d3bc0b..3e2f820b6007 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -932,8 +932,10 @@ void __init setup_arch(char **cmdline_p)
 		set_bit(EFI_64BIT, &efi.flags);
 	}
 
-	if (efi_enabled(EFI_BOOT))
+	if (efi_enabled(EFI_BOOT)) {
 		efi_memblock_x86_reserve_range();
+		set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+	}
 #endif
 
 	x86_init.oem.arch_setup();
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index a1f745b0bf1d..bac12cae2a80 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -70,14 +70,6 @@ static efi_config_table_type_t arch_tables[] __initdata = {
 
 u64 efi_setup;		/* efi setup_data physical address */
 
-static bool disable_runtime __initdata = false;
-static int __init setup_noefi(char *arg)
-{
-	disable_runtime = true;
-	return 0;
-}
-early_param("noefi", setup_noefi);
-
 int add_efi_memmap;
 EXPORT_SYMBOL(add_efi_memmap);
 
@@ -397,20 +389,12 @@ static int __init efi_runtime_init(void)
 	 * hypercall which executes relevant EFI functions) and that is why
 	 * they are always enabled.
 	 */
+	if (efi_enabled(EFI_64BIT))
+		rv = efi_runtime_init64();
+	else
+		rv = efi_runtime_init32();
 
-	if (!efi_enabled(EFI_PARAVIRT)) {
-		if (efi_enabled(EFI_64BIT))
-			rv = efi_runtime_init64();
-		else
-			rv = efi_runtime_init32();
-
-		if (rv)
-			return rv;
-	}
-
-	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-
-	return 0;
+	return rv;
 }
 
 static int __init efi_memmap_init(void)
@@ -489,10 +473,11 @@ void __init efi_init(void)
 	 * that doesn't match the kernel 32/64-bit mode.
 	 */
 
-	if (!efi_runtime_supported())
+	if (!efi_runtime_supported()) {
 		pr_info("No EFI runtime due to 32/64-bit mismatch with kernel\n");
-	else {
-		if (disable_runtime || efi_runtime_init())
+		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+	} else {
+		if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_runtime_init())
 			return;
 	}
 	if (efi_memmap_init())
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 36ffa1747e84..e7584ac22be1 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -415,3 +415,10 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
 	return of_scan_flat_dt(fdt_find_uefi_params, &info);
 }
 #endif /* CONFIG_EFI_PARAMS_FROM_FDT */
+
+static int __init setup_noefi(char *arg)
+{
+	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+	return 0;
+}
+early_param("noefi", setup_noefi);
-- 
1.9.0

-- 
Matt Fleming, Intel Open Source Technology Center