Patchwork [quantal] x86: efi: Turn off efi_enabled after setup on mixed fw/kernel

login
register
mail settings
Submitter Chris J Arges
Date Nov. 27, 2012, 3:13 p.m.
Message ID <50B4D89A.6060909@canonical.com>
Download mbox | patch
Permalink /patch/202250/
State New
Headers show

Comments

Chris J Arges - Nov. 27, 2012, 3:13 p.m.
SRU Justification:

Impact: Booting i386 kernels on 64-bit efi firmware does not work on
quantal.

Fix: Upstream commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c can be
cherry-picked into quantal. This commit is already present in raring.

Testcase: Properly boot an i386 quantal kernel on a device that has
64-bit EFI firmware.

BugLink: http://launchpad.net/bugs/1082059
Tim Gardner - Nov. 27, 2012, 3:44 p.m.
On 11/27/2012 08:13 AM, Chris J Arges wrote:
> SRU Justification:
> 
> Impact: Booting i386 kernels on 64-bit efi firmware does not work on
> quantal.
> 
> Fix: Upstream commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c can be
> cherry-picked into quantal. This commit is already present in raring.
> 
> Testcase: Properly boot an i386 quantal kernel on a device that has
> 64-bit EFI firmware.
> 
> BugLink: http://launchpad.net/bugs/1082059
> 
> 
> 

Note that this is a backport and not a clean cherry-pick.

Herton - stable material ?
Seth Forshee - Nov. 27, 2012, 3:50 p.m.
On Tue, Nov 27, 2012 at 09:13:30AM -0600, Chris J Arges wrote:
> SRU Justification:
> 
> Impact: Booting i386 kernels on 64-bit efi firmware does not work on
> quantal.
> 
> Fix: Upstream commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c can be
> cherry-picked into quantal. This commit is already present in raring.
> 
> Testcase: Properly boot an i386 quantal kernel on a device that has
> 64-bit EFI firmware.
> 
> BugLink: http://launchpad.net/bugs/1082059
> 

> From 8d0ea8f86b6c45ccb14a3f1acf312726cbfc04ad Mon Sep 17 00:00:00 2001
> From: Olof Johansson <olof@lixom.net>
> Date: Wed, 24 Oct 2012 10:00:44 -0700
> Subject: [PATCH] x86: efi: Turn off efi_enabled after setup on mixed
>  fw/kernel
> 
> When 32-bit EFI is used with 64-bit kernel (or vice versa), turn off
> efi_enabled once setup is done. Beyond setup, it is normally used to
> determine if runtime services are available and we will have none.
> 
> This will resolve issues stemming from efivars modprobe panicking on a
> 32/64-bit setup, as well as some reboot issues on similar setups.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=45991
> 
> Reported-by: Marko Kohtala <marko.kohtala@gmail.com>
> Reported-by: Maxim Kammerer <mk@dee.su>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> Acked-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> Cc: stable@kernel.org # 3.4 - 3.6

If this was Cc stable, why don't we have it already? Seems like it
should be in all the upstream stable trees.

> Cc: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> (cherry picked from commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c)
> 
> Conflicts:
> 
> 	arch/x86/kernel/setup.c
> 	arch/x86/platform/efi/efi.c
> 
> BugLink: http://launchpad.net/bugs/1082059
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
> ---
>  arch/x86/include/asm/efi.h  |    1 +
>  arch/x86/kernel/setup.c     |   12 ++++++++++++
>  arch/x86/platform/efi/efi.c |   31 +++++++++++++++++++++++--------
>  3 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index c9dcc18..029189d 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -98,6 +98,7 @@ extern void efi_set_executable(efi_memory_desc_t *md, bool executable);
>  extern int efi_memblock_x86_reserve_range(void);
>  extern void efi_call_phys_prelog(void);
>  extern void efi_call_phys_epilog(void);
> +extern void efi_unmap_memmap(void);
>  
>  #ifndef CONFIG_EFI
>  /*
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 16be6dc..cea0aef 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1036,6 +1036,18 @@ void __init setup_arch(char **cmdline_p)
>  	mcheck_init();
>  
>  	arch_init_ideal_nops();
> +
> +#ifdef CONFIG_EFI
> +	/* Once setup is done above, disable efi_enabled on mismatched
> +	 * firmware/kernel archtectures since there is no support for
> +	 * runtime services.
> +	 */
> +	if (efi_enabled && IS_ENABLED(CONFIG_X86_64) != efi_64bit) {
> +		pr_info("efi: Setup done, disabling due to 32/64-bit mismatch\n");
> +		efi_unmap_memmap();
> +		efi_enabled = 0;
> +	}
> +#endif
>  }
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index f55a4ce..f9bcbac 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -69,11 +69,15 @@ EXPORT_SYMBOL(efi);
>  struct efi_memory_map memmap;
>  
>  bool efi_64bit;
> -static bool efi_native;
>  
>  static struct efi efi_phys __initdata;
>  static efi_system_table_t efi_systab __initdata;
>  
> +static inline bool efi_is_native(void)
> +{
> +	return IS_ENABLED(CONFIG_X86_64) == efi_64bit;
> +}
> +
>  static int __init setup_noefi(char *arg)
>  {
>  	efi_enabled = 0;
> @@ -419,10 +423,21 @@ void __init efi_reserve_boot_services(void)
>  	}
>  }
>  
> -static void __init efi_free_boot_services(void)
> +void __init efi_unmap_memmap(void)
> +{
> +	if (memmap.map) {
> +		early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size);
> +		memmap.map = NULL;
> +	}
> +}
> +
> +void __init efi_free_boot_services(void)

These changes were already present in the original commit. Does it make
sense to backport the patch(es) that originally adds these instead of
folding it into this backport?

>  {
>  	void *p;
>  
> +	if (!efi_is_native())
> +		return;
> +
>  	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
>  		efi_memory_desc_t *md = p;
>  		unsigned long long start = md->phys_addr;
> @@ -670,12 +685,10 @@ void __init efi_init(void)
>  		return;
>  	}
>  	efi_phys.systab = (efi_system_table_t *)boot_params.efi_info.efi_systab;
> -	efi_native = !efi_64bit;
>  #else
>  	efi_phys.systab = (efi_system_table_t *)
>  			  (boot_params.efi_info.efi_systab |
>  			  ((__u64)boot_params.efi_info.efi_systab_hi<<32));
> -	efi_native = efi_64bit;
>  #endif
>  
>  	if (efi_systab_init(efi_phys.systab)) {
> @@ -709,7 +722,7 @@ void __init efi_init(void)
>  	 * that doesn't match the kernel 32/64-bit mode.
>  	 */
>  
> -	if (!efi_native)
> +	if (!efi_is_native())
>  		pr_info("No EFI runtime due to 32/64-bit mismatch with kernel\n");
>  	else if (efi_runtime_init()) {
>  		efi_enabled = 0;
> @@ -721,7 +734,7 @@ void __init efi_init(void)
>  		return;
>  	}
>  #ifdef CONFIG_X86_32
> -	if (efi_native) {
> +	if (efi_is_native()) {
>  		x86_platform.get_wallclock = efi_get_time;
>  		x86_platform.set_wallclock = efi_set_rtc_mmss;
>  	}
> @@ -787,8 +800,10 @@ void __init efi_enter_virtual_mode(void)
>  	 * non-native EFI
>  	 */
>  
> -	if (!efi_native)
> -		goto out;
> +	if (!efi_is_native()) {
> +		efi_unmap_memmap();
> +		return;
> +	}

Again, some of the changes here appear to be from an earlier commit, so
I have the same question as above.

>  
>  	/* Merge contiguous regions of the same type and attribute */
>  	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
> -- 
> 1.7.9.5
> 

> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Herton Ronaldo Krzesinski - Nov. 27, 2012, 4:32 p.m.
On Tue, Nov 27, 2012 at 08:44:27AM -0700, Tim Gardner wrote:
> On 11/27/2012 08:13 AM, Chris J Arges wrote:
> > SRU Justification:
> > 
> > Impact: Booting i386 kernels on 64-bit efi firmware does not work on
> > quantal.
> > 
> > Fix: Upstream commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c can be
> > cherry-picked into quantal. This commit is already present in raring.
> > 
> > Testcase: Properly boot an i386 quantal kernel on a device that has
> > 64-bit EFI firmware.
> > 
> > BugLink: http://launchpad.net/bugs/1082059
> > 
> > 
> > 
> 
> Note that this is a backport and not a clean cherry-pick.
> 
> Herton - stable material ?

Yeah queued already, will be released on 3.5.7.1

I'll handle this one.

> 
> -- 
> Tim Gardner tim.gardner@canonical.com
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Tim Gardner - Nov. 28, 2012, 12:40 p.m.
On 11/27/2012 09:32 AM, Herton Ronaldo Krzesinski wrote:
> On Tue, Nov 27, 2012 at 08:44:27AM -0700, Tim Gardner wrote:
>> On 11/27/2012 08:13 AM, Chris J Arges wrote:
>>> SRU Justification:
>>>
>>> Impact: Booting i386 kernels on 64-bit efi firmware does not work on
>>> quantal.
>>>
>>> Fix: Upstream commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c can be
>>> cherry-picked into quantal. This commit is already present in raring.
>>>
>>> Testcase: Properly boot an i386 quantal kernel on a device that has
>>> 64-bit EFI firmware.
>>>
>>> BugLink: http://launchpad.net/bugs/1082059
>>>
>>>
>>>
>>
>> Note that this is a backport and not a clean cherry-pick.
>>
>> Herton - stable material ?
>
> Yeah queued already, will be released on 3.5.7.1
>
> I'll handle this one.
>

You gonna leave the original BugLink in so that it closes out #1082059 ?

rtg
Herton Ronaldo Krzesinski - Nov. 28, 2012, 1:07 p.m.
On Wed, Nov 28, 2012 at 05:40:50AM -0700, Tim Gardner wrote:
> On 11/27/2012 09:32 AM, Herton Ronaldo Krzesinski wrote:
> >On Tue, Nov 27, 2012 at 08:44:27AM -0700, Tim Gardner wrote:
> >>On 11/27/2012 08:13 AM, Chris J Arges wrote:
> >>>SRU Justification:
> >>>
> >>>Impact: Booting i386 kernels on 64-bit efi firmware does not work on
> >>>quantal.
> >>>
> >>>Fix: Upstream commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c can be
> >>>cherry-picked into quantal. This commit is already present in raring.
> >>>
> >>>Testcase: Properly boot an i386 quantal kernel on a device that has
> >>>64-bit EFI firmware.
> >>>
> >>>BugLink: http://launchpad.net/bugs/1082059
> >>>
> >>>
> >>>
> >>
> >>Note that this is a backport and not a clean cherry-pick.
> >>
> >>Herton - stable material ?
> >
> >Yeah queued already, will be released on 3.5.7.1
> >
> >I'll handle this one.
> >
> 
> You gonna leave the original BugLink in so that it closes out #1082059 ?

Yes, I'll remember about it when applying in Quantal.

> 
> rtg
> -- 
> Tim Gardner tim.gardner@canonical.com
>

Patch

From 8d0ea8f86b6c45ccb14a3f1acf312726cbfc04ad Mon Sep 17 00:00:00 2001
From: Olof Johansson <olof@lixom.net>
Date: Wed, 24 Oct 2012 10:00:44 -0700
Subject: [PATCH] x86: efi: Turn off efi_enabled after setup on mixed
 fw/kernel

When 32-bit EFI is used with 64-bit kernel (or vice versa), turn off
efi_enabled once setup is done. Beyond setup, it is normally used to
determine if runtime services are available and we will have none.

This will resolve issues stemming from efivars modprobe panicking on a
32/64-bit setup, as well as some reboot issues on similar setups.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=45991

Reported-by: Marko Kohtala <marko.kohtala@gmail.com>
Reported-by: Maxim Kammerer <mk@dee.su>
Signed-off-by: Olof Johansson <olof@lixom.net>
Acked-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: stable@kernel.org # 3.4 - 3.6
Cc: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
(cherry picked from commit 5189c2a7c7769ee9d037d76c1a7b8550ccf3481c)

Conflicts:

	arch/x86/kernel/setup.c
	arch/x86/platform/efi/efi.c

BugLink: http://launchpad.net/bugs/1082059

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 arch/x86/include/asm/efi.h  |    1 +
 arch/x86/kernel/setup.c     |   12 ++++++++++++
 arch/x86/platform/efi/efi.c |   31 +++++++++++++++++++++++--------
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index c9dcc18..029189d 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -98,6 +98,7 @@  extern void efi_set_executable(efi_memory_desc_t *md, bool executable);
 extern int efi_memblock_x86_reserve_range(void);
 extern void efi_call_phys_prelog(void);
 extern void efi_call_phys_epilog(void);
+extern void efi_unmap_memmap(void);
 
 #ifndef CONFIG_EFI
 /*
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 16be6dc..cea0aef 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1036,6 +1036,18 @@  void __init setup_arch(char **cmdline_p)
 	mcheck_init();
 
 	arch_init_ideal_nops();
+
+#ifdef CONFIG_EFI
+	/* Once setup is done above, disable efi_enabled on mismatched
+	 * firmware/kernel archtectures since there is no support for
+	 * runtime services.
+	 */
+	if (efi_enabled && IS_ENABLED(CONFIG_X86_64) != efi_64bit) {
+		pr_info("efi: Setup done, disabling due to 32/64-bit mismatch\n");
+		efi_unmap_memmap();
+		efi_enabled = 0;
+	}
+#endif
 }
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index f55a4ce..f9bcbac 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -69,11 +69,15 @@  EXPORT_SYMBOL(efi);
 struct efi_memory_map memmap;
 
 bool efi_64bit;
-static bool efi_native;
 
 static struct efi efi_phys __initdata;
 static efi_system_table_t efi_systab __initdata;
 
+static inline bool efi_is_native(void)
+{
+	return IS_ENABLED(CONFIG_X86_64) == efi_64bit;
+}
+
 static int __init setup_noefi(char *arg)
 {
 	efi_enabled = 0;
@@ -419,10 +423,21 @@  void __init efi_reserve_boot_services(void)
 	}
 }
 
-static void __init efi_free_boot_services(void)
+void __init efi_unmap_memmap(void)
+{
+	if (memmap.map) {
+		early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size);
+		memmap.map = NULL;
+	}
+}
+
+void __init efi_free_boot_services(void)
 {
 	void *p;
 
+	if (!efi_is_native())
+		return;
+
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		efi_memory_desc_t *md = p;
 		unsigned long long start = md->phys_addr;
@@ -670,12 +685,10 @@  void __init efi_init(void)
 		return;
 	}
 	efi_phys.systab = (efi_system_table_t *)boot_params.efi_info.efi_systab;
-	efi_native = !efi_64bit;
 #else
 	efi_phys.systab = (efi_system_table_t *)
 			  (boot_params.efi_info.efi_systab |
 			  ((__u64)boot_params.efi_info.efi_systab_hi<<32));
-	efi_native = efi_64bit;
 #endif
 
 	if (efi_systab_init(efi_phys.systab)) {
@@ -709,7 +722,7 @@  void __init efi_init(void)
 	 * that doesn't match the kernel 32/64-bit mode.
 	 */
 
-	if (!efi_native)
+	if (!efi_is_native())
 		pr_info("No EFI runtime due to 32/64-bit mismatch with kernel\n");
 	else if (efi_runtime_init()) {
 		efi_enabled = 0;
@@ -721,7 +734,7 @@  void __init efi_init(void)
 		return;
 	}
 #ifdef CONFIG_X86_32
-	if (efi_native) {
+	if (efi_is_native()) {
 		x86_platform.get_wallclock = efi_get_time;
 		x86_platform.set_wallclock = efi_set_rtc_mmss;
 	}
@@ -787,8 +800,10 @@  void __init efi_enter_virtual_mode(void)
 	 * non-native EFI
 	 */
 
-	if (!efi_native)
-		goto out;
+	if (!efi_is_native()) {
+		efi_unmap_memmap();
+		return;
+	}
 
 	/* Merge contiguous regions of the same type and attribute */
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
-- 
1.7.9.5