[PATCH 1/7] efi: move noefi early param code out of x86 arch code

[PATCH 1/7] efi: move noefi early param code out of x86 arch code

[Dave Young]

noefi param can be used for arches other than X86 later, thus move it out of
x86 platform code.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 Documentation/kernel-parameters.txt |  2 +-
 arch/x86/platform/efi/efi.c         | 10 +---------
 drivers/firmware/efi/efi.c          | 13 +++++++++++++
 include/linux/efi.h                 |  1 +
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7c5fc8e..31c50c1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2132,7 +2132,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	nodsp		[SH] Disable hardware DSP at boot time.
 
-	noefi		[X86] Disable EFI runtime services support.
+	noefi		Disable EFI runtime services support.
 
 	noexec		[IA-64]
 
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index a1f745b..c90d3cd 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);
 
@@ -492,7 +484,7 @@ void __init efi_init(void)
 	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())
+		if (efi_runtime_disabled() || efi_runtime_init())
 			return;
 	}
 	if (efi_memmap_init())
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 36ffa17..1f396eb 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -41,6 +41,19 @@ struct efi __read_mostly efi = {
 };
 EXPORT_SYMBOL(efi);
 
+static bool disable_runtime;
+static int __init setup_noefi(char *arg)
+{
+	disable_runtime = true;
+	return 0;
+}
+early_param("noefi", setup_noefi);
+
+bool efi_runtime_disabled(void)
+{
+	return disable_runtime;
+}
+
 static struct kobject *efi_kobj;
 static struct kobject *efivars_kobj;
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 0d37d33..833329f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1210,4 +1210,5 @@ efi_status_t handle_cmdline_files(efi_system_table_t *sys_table_arg,
 
 efi_status_t efi_parse_options(char *cmdline);
 
+bool efi_runtime_disabled(void);
 #endif /* _LINUX_EFI_H */
-- 
1.8.3.1

[PATCH 2/7] Add a generic cmdline parse function parse_option_str

[Dave Young]

There should be a generic function to parse params like a=b,c
Adding parse_option_str in lib/cmdline.c which will return true
if there's specified option set in the params.

Also updated efi=old_map parsing code to use the new function

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/platform/efi/efi.c | 22 ++--------------------
 include/linux/kernel.h      |  1 +
 lib/cmdline.c               | 29 +++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c90d3cd..c73a7df5 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -932,26 +932,8 @@ u64 efi_mem_attributes(unsigned long phys_addr)
 
 static int __init parse_efi_cmdline(char *str)
 {
-	if (*str == '=')
-		str++;
-
-	while (*str) {
-		if (!strncmp(str, "old_map", 7)) {
-			set_bit(EFI_OLD_MEMMAP, &efi.flags);
-			str += strlen("old_map");
-		}
-
-		/*
-		 * Skip any options we don't understand. Presumably
-		 * they apply to the EFI boot stub.
-		 */
-		while (*str && *str != ',')
-			str++;
-
-		/* If we hit a delimiter, skip it */
-		if (*str == ',')
-			str++;
-	}
+	if (parse_option_str(str, "old_map"))
+		set_bit(EFI_OLD_MEMMAP, &efi.flags);
 
 	return 0;
 }
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c52907..d612718 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -407,6 +407,7 @@ int vsscanf(const char *, const char *, va_list);
 extern int get_option(char **str, int *pint);
 extern char *get_options(const char *str, int nints, int *ints);
 extern unsigned long long memparse(const char *ptr, char **retptr);
+extern bool parse_option_str(const char *str, const char *option);
 
 extern int core_kernel_text(unsigned long addr);
 extern int core_kernel_data(unsigned long addr);
diff --git a/lib/cmdline.c b/lib/cmdline.c
index d4932f7..7f1a182 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -155,3 +155,32 @@ unsigned long long memparse(const char *ptr, char **retptr)
 	return ret;
 }
 EXPORT_SYMBOL(memparse);
+
+/**
+ *	parse_option_str - Parse a string and check an option is set or not
+ *	@str: String to be parsed
+ *	@option: option name
+ *
+ *	This function parses a string containing a comma-separated list of
+ *	strings like a=b,c.
+ *
+ *	Return true if there's such option in the string, or return false.
+ */
+bool parse_option_str(const char *str, const char *option)
+{
+	while (*str) {
+		if (!strncmp(str, option, strlen(option))) {
+			str += strlen(option);
+			if (!*str || *str == ',')
+				return true;
+		}
+
+		while (*str && *str != ',')
+			str++;
+
+		if (*str == ',')
+			str++;
+	}
+
+	return false;
+}
-- 
1.8.3.1

[PATCH 3/7] efi: add kernel param efi=noruntime

[Dave Young]

noefi kernel param means actually disabling efi runtime, Per suggestion from
Leif Lindholm efi=noruntime should be better. But since noefi is already used
in X86 thus just adding another param efi=noruntime for same purpose.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 Documentation/kernel-parameters.txt | 3 ++-
 arch/x86/platform/efi/efi.c         | 4 ++--
 drivers/firmware/efi/efi.c          | 9 +++++++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 31c50c1..7afbf6a 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -981,13 +981,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Format: {"off" | "on" | "skip[mbr]"}
 
 	efi=		[EFI]
-			Format: { "old_map", "nochunk" }
+			Format: { "old_map", "nochunk", "noruntime" }
 			old_map [X86-64]: switch to the old ioremap-based EFI
 			runtime services mapping. 32-bit still uses this one by
 			default.
 			nochunk: disable reading files in "chunks" in the EFI
 			boot stub, as chunking can cause problems with some
 			firmware implementations.
+			noruntime : disable EFI runtime services support
 
 	efi_no_storage_paranoia [EFI; X86]
 			Using this parameter you can use more than 50% of
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c73a7df5..00f4cc5 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -930,11 +930,11 @@ u64 efi_mem_attributes(unsigned long phys_addr)
 	return 0;
 }
 
-static int __init parse_efi_cmdline(char *str)
+static int __init arch_parse_efi_cmdline(char *str)
 {
 	if (parse_option_str(str, "old_map"))
 		set_bit(EFI_OLD_MEMMAP, &efi.flags);
 
 	return 0;
 }
-early_param("efi", parse_efi_cmdline);
+early_param("efi", arch_parse_efi_cmdline);
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 1f396eb..49eb18b 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -54,6 +54,15 @@ bool efi_runtime_disabled(void)
 	return disable_runtime;
 }
 
+static int __init parse_efi_cmdline(char *str)
+{
+	if (parse_option_str(str, "noruntime"))
+		disable_runtime = true;
+
+	return 0;
+}
+early_param("efi", parse_efi_cmdline);
+
 static struct kobject *efi_kobj;
 static struct kobject *efivars_kobj;
 
-- 
1.8.3.1

[PATCH 4/7] arm64/efi: uefi_init error handling fix

[Dave Young]

There's one early memmap leak in uefi_init error path, fix it and slightly tune
the error handling code.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/arm64/kernel/efi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index e72f310..6ed0362 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -89,7 +89,8 @@ static int __init uefi_init(void)
 	 */
 	if (efi.systab->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) {
 		pr_err("System table signature incorrect\n");
-		return -EINVAL;
+		retval = -EINVAL;
+		goto out;
 	}
 	if ((efi.systab->hdr.revision >> 16) < 2)
 		pr_warn("Warning: EFI system table version %d.%02d, expected 2.00 or greater\n",
@@ -103,6 +104,7 @@ static int __init uefi_init(void)
 		for (i = 0; i < (int) sizeof(vendor) - 1 && *c16; ++i)
 			vendor[i] = c16[i];
 		vendor[i] = '\0';
+		early_memunmap(c16, sizeof(vendor));
 	}
 
 	pr_info("EFI v%u.%.02u by %s\n",
@@ -113,9 +115,8 @@ static int __init uefi_init(void)
 	if (retval == 0)
 		set_bit(EFI_CONFIG_TABLES, &efi.flags);
 
-	early_memunmap(c16, sizeof(vendor));
+out:
 	early_memunmap(efi.systab,  sizeof(efi_system_table_t));
-
 	return retval;
 }
 
-- 
1.8.3.1

Re: [PATCH 4/7] arm64/efi: uefi_init error handling fix

[Mark Salter]

On Thu, 2014-08-14 at 17:15 +0800, Dave Young wrote:
> There's one early memmap leak in uefi_init error path, fix it and slightly tune
> the error handling code.
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  arch/arm64/kernel/efi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Acked-by: Mark Salter <msalter@redhat.com>

Re: [PATCH 4/7] arm64/efi: uefi_init error handling fix

[Will Deacon]

On Thu, Aug 14, 2014 at 10:15:29AM +0100, Dave Young wrote:
> There's one early memmap leak in uefi_init error path, fix it and slightly tune
> the error handling code.
> 
> Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Reported-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Thanks for fixing this.

Will

> ---
>  arch/arm64/kernel/efi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e72f310..6ed0362 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -89,7 +89,8 @@ static int __init uefi_init(void)
>  	 */
>  	if (efi.systab->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) {
>  		pr_err("System table signature incorrect\n");
> -		return -EINVAL;
> +		retval = -EINVAL;
> +		goto out;
>  	}
>  	if ((efi.systab->hdr.revision >> 16) < 2)
>  		pr_warn("Warning: EFI system table version %d.%02d, expected 2.00 or greater\n",
> @@ -103,6 +104,7 @@ static int __init uefi_init(void)
>  		for (i = 0; i < (int) sizeof(vendor) - 1 && *c16; ++i)
>  			vendor[i] = c16[i];
>  		vendor[i] = '\0';
> +		early_memunmap(c16, sizeof(vendor));
>  	}
>  
>  	pr_info("EFI v%u.%.02u by %s\n",
> @@ -113,9 +115,8 @@ static int __init uefi_init(void)
>  	if (retval == 0)
>  		set_bit(EFI_CONFIG_TABLES, &efi.flags);
>  
> -	early_memunmap(c16, sizeof(vendor));
> +out:
>  	early_memunmap(efi.systab,  sizeof(efi_system_table_t));
> -
>  	return retval;
>  }
>  
> -- 
> 1.8.3.1
> 
> 

[PATCH 5/7] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Dave Young]

In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode
should error out.

At the same time move early_memunmap(memmap.map, mapsize) to the beginning of
the function or it will leak early mem.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/arm64/kernel/efi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 6ed0362..309fab1 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -392,11 +392,16 @@ static int __init arm64_enter_virtual_mode(void)
 		return -1;
 	}
 
+	mapsize = memmap.map_end - memmap.map;
+	if (efi_runtime_disabled()) {
+		early_memunmap(memmap.map, mapsize);
+		pr_info("EFI runtime services will be disabled.\n");
+		return -1;
+	}
+
 	pr_info("Remapping and enabling EFI services.\n");
 
 	/* replace early memmap mapping with permanent mapping */
-	mapsize = memmap.map_end - memmap.map;
-	early_memunmap(memmap.map, mapsize);
 	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
 						   mapsize);
 	memmap.map_end = memmap.map + mapsize;
-- 
1.8.3.1

Re: [PATCH 5/7] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Will Deacon]

On Thu, Aug 14, 2014 at 10:15:30AM +0100, Dave Young wrote:
> In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode
> should error out.
> 
> At the same time move early_memunmap(memmap.map, mapsize) to the beginning of
> the function or it will leak early mem.
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  arch/arm64/kernel/efi.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 6ed0362..309fab1 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -392,11 +392,16 @@ static int __init arm64_enter_virtual_mode(void)
>  		return -1;
>  	}
>  
> +	mapsize = memmap.map_end - memmap.map;
> +	if (efi_runtime_disabled()) {
> +		early_memunmap(memmap.map, mapsize);

Should this early_memunmap really be conditional? With this change, we no
longer unmap it before setting up the permanent mapping below.

Will

> +		pr_info("EFI runtime services will be disabled.\n");
> +		return -1;
> +	}
> +
>  	pr_info("Remapping and enabling EFI services.\n");
>  
>  	/* replace early memmap mapping with permanent mapping */
> -	mapsize = memmap.map_end - memmap.map;
> -	early_memunmap(memmap.map, mapsize);
>  	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
>  						   mapsize);
>  	memmap.map_end = memmap.map + mapsize;
> -- 
> 1.8.3.1
> 
> 

Re: [PATCH 5/7] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Dave Young]

On 08/15/14 at 04:09pm, Will Deacon wrote:
> On Thu, Aug 14, 2014 at 10:15:30AM +0100, Dave Young wrote:
> > In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode
> > should error out.
> > 
> > At the same time move early_memunmap(memmap.map, mapsize) to the beginning of
> > the function or it will leak early mem.
> > 
> > Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/arm64/kernel/efi.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 6ed0362..309fab1 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -392,11 +392,16 @@ static int __init arm64_enter_virtual_mode(void)
> >  		return -1;
> >  	}
> >  
> > +	mapsize = memmap.map_end - memmap.map;
> > +	if (efi_runtime_disabled()) {
> > +		early_memunmap(memmap.map, mapsize);
> 
> Should this early_memunmap really be conditional? With this change, we no
> longer unmap it before setting up the permanent mapping below.

Ooops, I tested the right version but sent a wrong version for this arm64 patch.

Thanks for catch.

> 
> Will
> 
> > +		pr_info("EFI runtime services will be disabled.\n");
> > +		return -1;
> > +	}
> > +
> >  	pr_info("Remapping and enabling EFI services.\n");
> >  
> >  	/* replace early memmap mapping with permanent mapping */
> > -	mapsize = memmap.map_end - memmap.map;
> > -	early_memunmap(memmap.map, mapsize);
> >  	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
> >  						   mapsize);
> >  	memmap.map_end = memmap.map + mapsize;
> > -- 
> > 1.8.3.1
> > 
> > 

[PATCH 5/7 update] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Dave Young]

In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode
should error out.

At the same time move early_memunmap(memmap.map, mapsize) to the beginning of
the function or it will leak early mem.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/arm64/kernel/efi.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 6ed0362..8f5db4a 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -392,11 +392,16 @@ static int __init arm64_enter_virtual_mode(void)
 		return -1;
 	}
 
-	pr_info("Remapping and enabling EFI services.\n");
-
-	/* replace early memmap mapping with permanent mapping */
 	mapsize = memmap.map_end - memmap.map;
 	early_memunmap(memmap.map, mapsize);
+
+	if (efi_runtime_disabled()) {
+		pr_info("EFI runtime services will be disabled.\n");
+		return -1;
+	}
+
+	pr_info("Remapping and enabling EFI services.\n");
+	/* replace early memmap mapping with permanent mapping */
 	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
 						   mapsize);
 	memmap.map_end = memmap.map + mapsize;
-- 
1.8.3.1

Re: [PATCH 5/7 update] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Will Deacon]

On Mon, Aug 18, 2014 at 02:30:07AM +0100, Dave Young wrote:
> In case efi runtime disabled via noefi kernel cmdline arm64_enter_virtual_mode
> should error out.
> 
> At the same time move early_memunmap(memmap.map, mapsize) to the beginning of
> the function or it will leak early mem.
> 
> Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 6ed0362..8f5db4a 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -392,11 +392,16 @@ static int __init arm64_enter_virtual_mode(void)
>  		return -1;
>  	}
>  
> -	pr_info("Remapping and enabling EFI services.\n");
> -
> -	/* replace early memmap mapping with permanent mapping */
>  	mapsize = memmap.map_end - memmap.map;
>  	early_memunmap(memmap.map, mapsize);
> +
> +	if (efi_runtime_disabled()) {
> +		pr_info("EFI runtime services will be disabled.\n");
> +		return -1;
> +	}
> +
> +	pr_info("Remapping and enabling EFI services.\n");
> +	/* replace early memmap mapping with permanent mapping */
>  	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
>  						   mapsize);
>  	memmap.map_end = memmap.map + mapsize;

This looks better, thanks:

  Reviewed-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Are you sending these via the efi tree as part of this series, or do you
need me to pick anything up into the arm64 tree?

Will

Re: [PATCH 5/7 update] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Matt Fleming]

On Fri, 22 Aug, at 04:10:07PM, Will Deacon wrote:
> 
> Are you sending these via the efi tree as part of this series, or do you
> need me to pick anything up into the arm64 tree?

I was planning on taking the entire series through the EFI tree (once
I've had chance to review it).

Does that work?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 5/7 update] arm64/efi: do not enter virtual mode in case booting with efi=noruntime or noefi

[Will Deacon]

On Fri, Aug 22, 2014 at 04:47:19PM +0100, Matt Fleming wrote:
> On Fri, 22 Aug, at 04:10:07PM, Will Deacon wrote:
> > 
> > Are you sending these via the efi tree as part of this series, or do you
> > need me to pick anything up into the arm64 tree?
> 
> I was planning on taking the entire series through the EFI tree (once
> I've had chance to review it).
> 
> Does that work?

Fine by me; I think I've acked/reviewed all the arm64 bits. If you find an
arm64 patch with a missing ack, just ping me and I'll take a look.

Cheers,

Will

[PATCH 6/7] x86/efi: clear EFI_RUNTIME_SERVICES bit in case failures while entering virtual mode

[Dave Young]

If enter virtual mode failed due to some reason other than the efi call
the EFI_RUNTIME_SERVICES bit in efi.flags should be cleared thus users
of efi runtime services can check the bit and handle the case instead of
assume efi runtime is ok.

Per Matt, if efi call SetVirtualAddressMap fails we will be not sure it's safe
to make any assumptions about the state of the system. so kernel panics instead
of clears EFI_RUNTIME_SERVICES bit.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/platform/efi/efi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 00f4cc5..d3096c0 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -724,6 +724,7 @@ static void __init kexec_enter_virtual_mode(void)
 	 */
 	if (!efi_is_native()) {
 		efi_unmap_memmap();
+		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 		return;
 	}
 
@@ -797,6 +798,7 @@ static void __init __efi_enter_virtual_mode(void)
 	new_memmap = efi_map_regions(&count, &pg_shift);
 	if (!new_memmap) {
 		pr_err("Error reallocating memory, EFI runtime non-functional!\n");
+		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 		return;
 	}
 
@@ -804,8 +806,10 @@ static void __init __efi_enter_virtual_mode(void)
 
 	BUG_ON(!efi.systab);
 
-	if (efi_setup_page_tables(__pa(new_memmap), 1 << pg_shift))
+	if (efi_setup_page_tables(__pa(new_memmap), 1 << pg_shift)) {
+		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 		return;
+	}
 
 	efi_sync_low_kernel_mappings();
 	efi_dump_pagetable();
-- 
1.8.3.1

[PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Dave Young]

efi rtc depends on efi runtime services, so if efi runtime services are not
usable it should error out.

Without this patch rtc-efi will panic with 'noefi' boot

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 drivers/rtc/rtc-efi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index c4c3843..ca2487b 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev)
 {
 	struct rtc_device *rtc;
 
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+		return -ENODEV;
+
 	rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops,
 					THIS_MODULE);
 	if (IS_ERR(rtc))
-- 
1.8.3.1

Re: [PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Mark Salter]

On Thu, 2014-08-14 at 17:19 +0800, Dave Young wrote:
> efi rtc depends on efi runtime services, so if efi runtime services are not
> usable it should error out.
> 
> Without this patch rtc-efi will panic with 'noefi' boot

Is this still needed after commit da167ad7638759:

  rtc: ia64: allow other architectures to use EFI RTC

That should keep efi_rtc_probe from being called if runtime services
are not available.

> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  drivers/rtc/rtc-efi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index c4c3843..ca2487b 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev)
>  {
>  	struct rtc_device *rtc;
>  
> +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> +		return -ENODEV;
> +
>  	rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops,
>  					THIS_MODULE);
>  	if (IS_ERR(rtc))

Re: [PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Dave Young]

Hi, Mark

On 09/22/14 at 12:33pm, Mark Salter wrote:
> On Thu, 2014-08-14 at 17:19 +0800, Dave Young wrote:
> > efi rtc depends on efi runtime services, so if efi runtime services are not
> > usable it should error out.
> > 
> > Without this patch rtc-efi will panic with 'noefi' boot
> 
> Is this still needed after commit da167ad7638759:
> 
>   rtc: ia64: allow other architectures to use EFI RTC
> 
> That should keep efi_rtc_probe from being called if runtime services
> are not available.

If that's the case then this patch should be unnecessary, I have no machine
to test it currently though.

> 
> > 
> > Signed-off-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/rtc/rtc-efi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> > index c4c3843..ca2487b 100644
> > --- a/drivers/rtc/rtc-efi.c
> > +++ b/drivers/rtc/rtc-efi.c
> > @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> >  {
> >  	struct rtc_device *rtc;
> >  
> > +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > +		return -ENODEV;
> > +
> >  	rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops,
> >  					THIS_MODULE);
> >  	if (IS_ERR(rtc))
> 
> 

Re: [PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Matt Fleming]

On Tue, 23 Sep, at 03:24:26PM, Dave Young wrote:
> 
> If that's the case then this patch should be unnecessary, I have no
> machine to test it currently though.

Thanks, I've dropped this from the v3.18 queue.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Matt Fleming]

On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote:
> 
> Is this still needed after commit da167ad7638759:
> 
>   rtc: ia64: allow other architectures to use EFI RTC

Umm... we should really look at disabling this for x86, since the EFI
time functions don't work on a bunch of machines I've got here.

And it looks like someone enabled this driver and triggered an oops on
their 32-bit EFI tablet,

  https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21

Re: [PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Mark Salter]

On Thu, 2014-09-25 at 22:18 +0100, Matt Fleming wrote:
> On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote:
> > 
> > Is this still needed after commit da167ad7638759:
> > 
> >   rtc: ia64: allow other architectures to use EFI RTC
> 
> Umm... we should really look at disabling this for x86, since the EFI
> time functions don't work on a bunch of machines I've got here.
> 
> And it looks like someone enabled this driver and triggered an oops on
> their 32-bit EFI tablet,
> 
>   https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21

Previously, it depended on IA64. If it's not worth the trouble for
x86 then I'd say have it depend on !X86 also. Newer EFI-supporting
architectures are unlikely to have the same legacy issues as x86
so I don't think it'll end up being an ever growing list.

Re: [PATCH 7/7] efi_rtc: probe function error out in case no efi runtime enabled

[Matt Fleming]

(Argh, this was sat in my outbox for over a week)

On Thu, 25 Sep, at 05:38:50PM, Mark Salter wrote:
> On Thu, 2014-09-25 at 22:18 +0100, Matt Fleming wrote:
> > On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote:
> > > 
> > > Is this still needed after commit da167ad7638759:
> > > 
> > >   rtc: ia64: allow other architectures to use EFI RTC
> > 
> > Umm... we should really look at disabling this for x86, since the EFI
> > time functions don't work on a bunch of machines I've got here.
> > 
> > And it looks like someone enabled this driver and triggered an oops on
> > their 32-bit EFI tablet,
> > 
> >   https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21
> 
> Previously, it depended on IA64. If it's not worth the trouble for
> x86 then I'd say have it depend on !X86 also. Newer EFI-supporting
> architectures are unlikely to have the same legacy issues as x86
> so I don't think it'll end up being an ever growing list.
 
Yeah that's what I was thinking. For x86, it's the age old "Windows
doesn't use it, so it was never validated" issue.

I'll send an update.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 1/7] efi: move noefi early param code out of x86 arch code

[Dave Young]

Hi,

I addressed several comments from Matt, Randy and Will in this post.

Also rewrote the patch for efi=noruntime with using a generic param handling
function.

CCed Xen and SGI people, removed rtc list in cc for [1-6]/7 patches

Thanks
Dave

Re: [PATCH 1/7] efi: move noefi early param code out of x86 arch code

[Matt Fleming]

On Thu, 14 Aug, at 05:26:38PM, Dave Young wrote:
> Hi,
> 
> I addressed several comments from Matt, Randy and Will in this post.
> 
> Also rewrote the patch for efi=noruntime with using a generic param handling
> function.
> 
> CCed Xen and SGI people, removed rtc list in cc for [1-6]/7 patches

Thanks Dave, I picked all of these patches up into the 'next' branch.

-- 
Matt Fleming, Intel Open Source Technology Center