[PATCH] Remove warning in efi_enter_virtual_mode V2

[PATCH] Remove warning in efi_enter_virtual_mode V2

[Bryan O'Donoghue]

Some EFI BIOS stores BGRT data in the wrong place and some EFI based
BIOS also requires mapping of boot code/data when doing
efi_enter_virtual_mode.

Current code in efi_enter_virtual_mode maps both EFI_RUNTIME_MEMORY and
BIOS boot code/data.

This patch gives the option to switch off that behavior - if your BIOS
has neither BGRT - nor bugs that require mapping of EFI boot code/data

Removes the following warning when booting Linux with an EFI BIOS that
is bug free on IA32

------------[ cut here ]------------
WARNING: at arch/x86/mm/ioremap.c:102 __ioremap_caller+0x3d3/0x440()
Modules linked in:
Pid: 0, comm: swapper Not tainted 3.9.0-rc7+ #95
Call Trace:
 [<c102b6af>] warn_slowpath_common+0x5f/0x80
 [<c1023fb3>] ? __ioremap_caller+0x3d3/0x440
 [<c1023fb3>] ? __ioremap_caller+0x3d3/0x440
 [<c102b6ed>] warn_slowpath_null+0x1d/0x20
 [<c1023fb3>] __ioremap_caller+0x3d3/0x440
 [<c106007b>] ? get_usage_chars+0xfb/0x110
 [<c102d937>] ? vprintk_emit+0x147/0x480
 [<c1418593>] ? efi_enter_virtual_mode+0x1e4/0x3de
 [<c102406a>] ioremap_cache+0x1a/0x20
 [<c1418593>] ? efi_enter_virtual_mode+0x1e4/0x3de
 [<c1418593>] efi_enter_virtual_mode+0x1e4/0x3de
 [<c1407984>] start_kernel+0x286/0x2f4
 [<c1407535>] ? repair_env_string+0x51/0x51
 [<c1407362>] i386_start_kernel+0x12c/0x12f
---[ end trace 4eaa2a86a8e2da22 ]---

Mapping only memory EFI_RUNTIME_MEMORY is is consistent with the code in
arch/ia64/kernel/efi.c:efi_enter_virtual_mode();

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue.lkml-SyKdqv6vbfZdzvEItQ6vdLNAH6kLmebB@public.gmane.org>
---
 Documentation/kernel-parameters.txt |    4 ++++
 arch/x86/platform/efi/efi.c         |   14 +++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 4609e81..faaa5cb 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1856,6 +1856,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	noefi		[X86] Disable EFI runtime services support.
 
+	noefi_virt_mapboot
+			[X86] Do not map EFI boot code/data when calling
+			efi_enter_virtual_mode().
+
 	noexec		[IA-64]
 
 	noexec		[X86]
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 5f2ecaf..64f98cd 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -88,6 +88,14 @@ static int __init setup_noefi(char *arg)
 }
 early_param("noefi", setup_noefi);
 
+static bool __initdata virt_mapboot = true;
+static int __init setup_noefi_virt_mapboot(char *arg)
+{
+	virt_mapboot = false;
+	return 0;
+}
+early_param("noefi_virt_mapboot", setup_noefi_virt_mapboot);
+
 int add_efi_memmap;
 EXPORT_SYMBOL(add_efi_memmap);
 
@@ -884,9 +892,9 @@ void __init efi_enter_virtual_mode(void)
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
-		if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
-		    md->type != EFI_BOOT_SERVICES_CODE &&
-		    md->type != EFI_BOOT_SERVICES_DATA)
+		if (!((md->attribute & EFI_MEMORY_RUNTIME) || (virt_mapboot &&
+			(md->type == EFI_BOOT_SERVICES_CODE ||
+			 md->type == EFI_BOOT_SERVICES_DATA))))
 			continue;
 
 		size = md->num_pages << EFI_PAGE_SHIFT;
-- 
1.7.10.4

[PATCH] Remove warning in efi_enter_virtual_mode V2

[Bryan O'Donoghue]

Some EFI BIOS stores BGRT data in the wrong place and some EFI based
BIOS also requires mapping of boot code/data when doing
efi_enter_virtual_mode.

Current code in efi_enter_virtual_mode maps both EFI_RUNTIME_MEMORY and
BIOS boot code/data.

This patch gives the option to switch off that behavior - if your BIOS
has neither BGRT - nor bugs that require mapping of EFI boot code/data

Removes the following warning when booting Linux with an EFI BIOS that
is bug free on IA32

------------[ cut here ]------------
WARNING: at arch/x86/mm/ioremap.c:102 __ioremap_caller+0x3d3/0x440()
Modules linked in:
Pid: 0, comm: swapper Not tainted 3.9.0-rc7+ #95
Call Trace:
 [<c102b6af>] warn_slowpath_common+0x5f/0x80
 [<c1023fb3>] ? __ioremap_caller+0x3d3/0x440
 [<c1023fb3>] ? __ioremap_caller+0x3d3/0x440
 [<c102b6ed>] warn_slowpath_null+0x1d/0x20
 [<c1023fb3>] __ioremap_caller+0x3d3/0x440
 [<c106007b>] ? get_usage_chars+0xfb/0x110
 [<c102d937>] ? vprintk_emit+0x147/0x480
 [<c1418593>] ? efi_enter_virtual_mode+0x1e4/0x3de
 [<c102406a>] ioremap_cache+0x1a/0x20
 [<c1418593>] ? efi_enter_virtual_mode+0x1e4/0x3de
 [<c1418593>] efi_enter_virtual_mode+0x1e4/0x3de
 [<c1407984>] start_kernel+0x286/0x2f4
 [<c1407535>] ? repair_env_string+0x51/0x51
 [<c1407362>] i386_start_kernel+0x12c/0x12f
---[ end trace 4eaa2a86a8e2da22 ]---

Mapping only memory EFI_RUNTIME_MEMORY is is consistent with the code in
arch/ia64/kernel/efi.c:efi_enter_virtual_mode();

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue.lkml@nexus-software.ie>
---
 Documentation/kernel-parameters.txt |    4 ++++
 arch/x86/platform/efi/efi.c         |   14 +++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 4609e81..faaa5cb 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1856,6 +1856,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	noefi		[X86] Disable EFI runtime services support.
 
+	noefi_virt_mapboot
+			[X86] Do not map EFI boot code/data when calling
+			efi_enter_virtual_mode().
+
 	noexec		[IA-64]
 
 	noexec		[X86]
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 5f2ecaf..64f98cd 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -88,6 +88,14 @@ static int __init setup_noefi(char *arg)
 }
 early_param("noefi", setup_noefi);
 
+static bool __initdata virt_mapboot = true;
+static int __init setup_noefi_virt_mapboot(char *arg)
+{
+	virt_mapboot = false;
+	return 0;
+}
+early_param("noefi_virt_mapboot", setup_noefi_virt_mapboot);
+
 int add_efi_memmap;
 EXPORT_SYMBOL(add_efi_memmap);
 
@@ -884,9 +892,9 @@ void __init efi_enter_virtual_mode(void)
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
-		if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
-		    md->type != EFI_BOOT_SERVICES_CODE &&
-		    md->type != EFI_BOOT_SERVICES_DATA)
+		if (!((md->attribute & EFI_MEMORY_RUNTIME) || (virt_mapboot &&
+			(md->type == EFI_BOOT_SERVICES_CODE ||
+			 md->type == EFI_BOOT_SERVICES_DATA))))
 			continue;
 
 		size = md->num_pages << EFI_PAGE_SHIFT;
-- 
1.7.10.4

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[Greg KH]

On Wed, Apr 17, 2013 at 11:05:41PM +0100, Bryan O'Donoghue wrote:
> Some EFI BIOS stores BGRT data in the wrong place and some EFI based
> BIOS also requires mapping of boot code/data when doing
> efi_enter_virtual_mode.
> 
> Current code in efi_enter_virtual_mode maps both EFI_RUNTIME_MEMORY and
> BIOS boot code/data.
> 
> This patch gives the option to switch off that behavior - if your BIOS
> has neither BGRT - nor bugs that require mapping of EFI boot code/data

No, never add new boot options, no users, or distros, know to set them.
Isn't there some way we can dynamically determine this instead?

thanks,

greg k-h

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[Greg KH]

On Wed, Apr 17, 2013 at 11:05:41PM +0100, Bryan O'Donoghue wrote:
> Some EFI BIOS stores BGRT data in the wrong place and some EFI based
> BIOS also requires mapping of boot code/data when doing
> efi_enter_virtual_mode.
> 
> Current code in efi_enter_virtual_mode maps both EFI_RUNTIME_MEMORY and
> BIOS boot code/data.
> 
> This patch gives the option to switch off that behavior - if your BIOS
> has neither BGRT - nor bugs that require mapping of EFI boot code/data

No, never add new boot options, no users, or distros, know to set them.
Isn't there some way we can dynamically determine this instead?

thanks,

greg k-h

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[H. Peter Anvin]

The only real use for command line options is to be able to get users to test something during troubleshooting.

Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:

>On Wed, Apr 17, 2013 at 11:05:41PM +0100, Bryan O'Donoghue wrote:
>> Some EFI BIOS stores BGRT data in the wrong place and some EFI based
>> BIOS also requires mapping of boot code/data when doing
>> efi_enter_virtual_mode.
>> 
>> Current code in efi_enter_virtual_mode maps both EFI_RUNTIME_MEMORY
>and
>> BIOS boot code/data.
>> 
>> This patch gives the option to switch off that behavior - if your
>BIOS
>> has neither BGRT - nor bugs that require mapping of EFI boot
>code/data
>
>No, never add new boot options, no users, or distros, know to set them.
>Isn't there some way we can dynamically determine this instead?
>
>thanks,
>
>greg k-h

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[H. Peter Anvin]

The only real use for command line options is to be able to get users to test something during troubleshooting.

Greg KH <gregkh@linuxfoundation.org> wrote:

>On Wed, Apr 17, 2013 at 11:05:41PM +0100, Bryan O'Donoghue wrote:
>> Some EFI BIOS stores BGRT data in the wrong place and some EFI based
>> BIOS also requires mapping of boot code/data when doing
>> efi_enter_virtual_mode.
>> 
>> Current code in efi_enter_virtual_mode maps both EFI_RUNTIME_MEMORY
>and
>> BIOS boot code/data.
>> 
>> This patch gives the option to switch off that behavior - if your
>BIOS
>> has neither BGRT - nor bugs that require mapping of EFI boot
>code/data
>
>No, never add new boot options, no users, or distros, know to set them.
>Isn't there some way we can dynamically determine this instead?
>
>thanks,
>
>greg k-h

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[Greg KH]

On Thu, Apr 18, 2013 at 11:00:27PM -0700, H. Peter Anvin wrote:
> The only real use for command line options is to be able to get users
> to test something during troubleshooting.

Ok, but the option shouldn't be used to "solve" the problem.

greg k-h

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[Greg KH]

On Thu, Apr 18, 2013 at 11:00:27PM -0700, H. Peter Anvin wrote:
> The only real use for command line options is to be able to get users
> to test something during troubleshooting.

Ok, but the option shouldn't be used to "solve" the problem.

greg k-h

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[H. Peter Anvin]

Nope, just a vehicle for experiments.

Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:

>On Thu, Apr 18, 2013 at 11:00:27PM -0700, H. Peter Anvin wrote:
>> The only real use for command line options is to be able to get users
>> to test something during troubleshooting.
>
>Ok, but the option shouldn't be used to "solve" the problem.
>
>greg k-h

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[H. Peter Anvin]

Nope, just a vehicle for experiments.

Greg KH <gregkh@linuxfoundation.org> wrote:

>On Thu, Apr 18, 2013 at 11:00:27PM -0700, H. Peter Anvin wrote:
>> The only real use for command line options is to be able to get users
>> to test something during troubleshooting.
>
>Ok, but the option shouldn't be used to "solve" the problem.
>
>greg k-h

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[Bryan O'Donoghue]

On 19/04/13 06:58, Greg KH wrote:
>> This patch gives the option to switch off that behavior - if your BIOS
>> has neither BGRT - nor bugs that require mapping of EFI boot code/data
>
> No, never add new boot options, no users, or distros, know to set them.
> Isn't there some way we can dynamically determine this instead?

Peter, Greg.

There are three issues to consider here

1: Some UEFI BIOS is buggy and the EFI_RUNTIME_SERVICES code - actually 
touches EFI_BOOT_MEMORY. Boot memory should be completely untouched 
after an entity calls ExitBootServices() - typically done by an EFI 
aware bootloader before handing off to Linux.

2: Existing code maps EFI_BOOT_MEMORY in arch/x86/platform/efi/efi.c.
Initially it looked to me as though you could probe for ACPI::BGRT - 
look for an ACPI object sometimes stored in EFI_BOOT_MEMORY and use that 
to determine if EFI_BOOT_MEMORY should be mapped. I wasn't aware #1 
above was also a concern. So just probing for something - doesn't appear 
to fly

3: Standards compliant EFI BIOS - like the reference EFI 2.3.1 code we 
have on my project, has neither of the two above problems to work around

So we can.

1. Just silently map EFI_BOOT_MEMORY - even on unbuggy platforms like #3 
- or we can

2. Introduce some sort of intelligence - a parameter somewhere to tell 
the efi_enter_virtual mode if/when to map EFI_BOOT_MEMORY.

3. Just go the suggested route from Josh
#ifdef CONFIG_X86_64
			if (md->type != EFI_BOOT_SERVICES_CODE &&
			    md->type != EFI_BOOT_SERVICES_DATA)
#endif

Option #3 - so long as it doesn't break ia32::BGRT systems works for me.

Re: [PATCH] Remove warning in efi_enter_virtual_mode V2

[Bryan O'Donoghue]

On 19/04/13 06:58, Greg KH wrote:
>> This patch gives the option to switch off that behavior - if your BIOS
>> has neither BGRT - nor bugs that require mapping of EFI boot code/data
>
> No, never add new boot options, no users, or distros, know to set them.
> Isn't there some way we can dynamically determine this instead?

Peter, Greg.

There are three issues to consider here

1: Some UEFI BIOS is buggy and the EFI_RUNTIME_SERVICES code - actually 
touches EFI_BOOT_MEMORY. Boot memory should be completely untouched 
after an entity calls ExitBootServices() - typically done by an EFI 
aware bootloader before handing off to Linux.

2: Existing code maps EFI_BOOT_MEMORY in arch/x86/platform/efi/efi.c.
Initially it looked to me as though you could probe for ACPI::BGRT - 
look for an ACPI object sometimes stored in EFI_BOOT_MEMORY and use that 
to determine if EFI_BOOT_MEMORY should be mapped. I wasn't aware #1 
above was also a concern. So just probing for something - doesn't appear 
to fly

3: Standards compliant EFI BIOS - like the reference EFI 2.3.1 code we 
have on my project, has neither of the two above problems to work around

So we can.

1. Just silently map EFI_BOOT_MEMORY - even on unbuggy platforms like #3 
- or we can

2. Introduce some sort of intelligence - a parameter somewhere to tell 
the efi_enter_virtual mode if/when to map EFI_BOOT_MEMORY.

3. Just go the suggested route from Josh
#ifdef CONFIG_X86_64
			if (md->type != EFI_BOOT_SERVICES_CODE &&
			    md->type != EFI_BOOT_SERVICES_DATA)
#endif

Option #3 - so long as it doesn't break ia32::BGRT systems works for me.