Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, 2013-09-12 at 08:55 +0100, Matt Fleming wrote: > On Tue, 10 Sep, at 10:43:27AM, Darren Hart wrote: > > Josh OK'd this, but as far as I can tell, it hasn't made it upstream > > yet. Matt was there an alternate fixed pushed? > > Nope, this one slipped through the cracks. Thanks for following up! > > Here's the patch I just applied on the 'urgent' branch. > Excellent, thank you Matt. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Tue, 10 Sep, at 10:43:27AM, Darren Hart wrote:
> Josh OK'd this, but as far as I can tell, it hasn't made it upstream
> yet. Matt was there an alternate fixed pushed?
Nope, this one slipped through the cracks. Thanks for following up!
Here's the patch I just applied on the 'urgent' branch.
---
>From e285df5368b33efcc9a0ca5b4ed87b0e0c590218 Mon Sep 17 00:00:00 2001
From: Josh Boyer
Date: Thu, 18 Apr 2013 07:51:34 -0700
Subject: [PATCH] x86, efi: Don't map Boot Services on i386
Add patch to fix 32bit EFI service mapping (rhbz 726701)
Multiple people are reporting hitting the following WARNING on i386,
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:
[] warn_slowpath_common+0x5f/0x80
[] ? __ioremap_caller+0x3d3/0x440
[] ? __ioremap_caller+0x3d3/0x440
[] warn_slowpath_null+0x1d/0x20
[] __ioremap_caller+0x3d3/0x440
[] ? get_usage_chars+0xfb/0x110
[] ? vprintk_emit+0x147/0x480
[] ? efi_enter_virtual_mode+0x1e4/0x3de
[] ioremap_cache+0x1a/0x20
[] ? efi_enter_virtual_mode+0x1e4/0x3de
[] efi_enter_virtual_mode+0x1e4/0x3de
[] start_kernel+0x286/0x2f4
[] ? repair_env_string+0x51/0x51
[] i386_start_kernel+0x12c/0x12f
Due to the workaround described in commit 916f676f8 ("x86, efi: Retain
boot service code until after switching to virtual mode") EFI Boot
Service regions are mapped for a period during boot. Unfortunately, with
the limited size of the i386 direct kernel map it's possible that some
of the Boot Service regions will not be directly accessible, which
causes them to be ioremap()'d, triggering the above warning as the
regions are marked as E820_RAM in the e820 memmap.
There are currently only two situations where we need to map EFI Boot
Service regions,
1. To workaround the firmware bug described in 916f676f8
2. To access the ACPI BGRT image
but since we haven't seen an i386 implementation that requires either,
this simple fix should suffice for now.
[ Added to changelog - Matt ]
Reported-by: Bryan O'Donoghue
Acked-by: Tom Zanussi
Acked-by: Darren Hart
Cc: Josh Triplett
Cc: Matthew Garrett
Cc: H. Peter Anvin
Cc: Ingo Molnar
Cc: Thomas Gleixner
Cc: Greg Kroah-Hartman
Cc:
Signed-off-by: Josh Boyer
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 90f6ed1..c7e22ab 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -912,10 +912,13 @@ 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)
- continue;
+ if (!(md->attribute & EFI_MEMORY_RUNTIME)) {
+#ifdef CONFIG_X86_64
+ if (md->type != EFI_BOOT_SERVICES_CODE &&
+ md->type != EFI_BOOT_SERVICES_DATA)
+#endif
+ continue;
+ }
size = md->num_pages << EFI_PAGE_SHIFT;
end = md->phys_addr + size;
--
1.8.1.4
--
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Fri, 2013-04-19 at 08:50 +0100, Matt Fleming wrote:
> On 04/19/2013 01:18 AM, Darren Hart wrote:
> > On 04/18/2013 09:19 AM, Matt Fleming wrote:
> >>
> >> Could you give it a spin on your MinnowBoard?
> >
> > I've removed the patch I reference above and applied your patch to my
> > 3.8.4 MinnowBoard dev tree. It panics with:
>
> D'oh. OK, at this point I'm inclined to apply Josh Boyer's patch on top
> of my urgent branch which will address the WARNING people are hitting on
> i386. I updated the commit message a little.
>
> Josh (Boyer), are you guys still carrying this patch and have you seen
> any fallout? I notice your SoB isn't on the patch that Darren posted, am
> I OK to add it?
Josh OK'd this, but as far as I can tell, it hasn't made it upstream
yet. Matt was there an alternate fixed pushed?
The patch from Josh we're referring to here I believe is:
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e4a86a6..b9876aa 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -997,10 +997,13 @@ 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)
- continue;
+ if (!(md->attribute & EFI_MEMORY_RUNTIME)) {
+#ifdef CONFIG_X86_64
+ if (md->type != EFI_BOOT_SERVICES_CODE &&
+ md->type != EFI_BOOT_SERVICES_DATA)
+#endif
+ continue;
+ }
size = md->num_pages << EFI_PAGE_SHIFT;
end = md->phys_addr + size;
--
Darren
>
> ---
>
> From 40f053eb6ccb3f0c462ef7a23c44c3264d87a0d4 Mon Sep 17 00:00:00 2001
> From: Josh Boyer
> Date: Thu, 18 Apr 2013 07:51:34 -0700
> Subject: [PATCH] x86, efi: Don't map Boot Services on i386
>
> Add patch to fix 32bit EFI service mapping (rhbz 726701)
>
> Multiple people are reporting hitting the following WARNING on i386,
>
> 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:
>[] warn_slowpath_common+0x5f/0x80
>[] ? __ioremap_caller+0x3d3/0x440
>[] ? __ioremap_caller+0x3d3/0x440
>[] warn_slowpath_null+0x1d/0x20
>[] __ioremap_caller+0x3d3/0x440
>[] ? get_usage_chars+0xfb/0x110
>[] ? vprintk_emit+0x147/0x480
>[] ? efi_enter_virtual_mode+0x1e4/0x3de
>[] ioremap_cache+0x1a/0x20
>[] ? efi_enter_virtual_mode+0x1e4/0x3de
>[] efi_enter_virtual_mode+0x1e4/0x3de
>[] start_kernel+0x286/0x2f4
>[] ? repair_env_string+0x51/0x51
>[] i386_start_kernel+0x12c/0x12f
>
> Due to the workaround described in commit 916f676f8 ("x86, efi: Retain
> boot service code until after switching to virtual mode") EFI Boot
> Service regions are mapped for a period during boot. Unfortunately, with
> the limited size of the i386 direct kernel map it's possible that some
> of the Boot Service regions will not be directly accessible, which
> causes them to be ioremap()'d, triggering the above warning as the
> regions are marked as E820_RAM in the e820 memmap.
>
> There are currently only two situations where we need to map EFI Boot
> Service regions,
>
> 1. To workaround the firmware bug described in 916f676f8
> 2. To access the ACPI BGRT image
>
> but since we haven't seen an i386 implementation that requires either,
> this simple fix should suffice for now.
>
> [ Added to changelog - Matt ]
>
> Reported-by: Bryan O'Donoghue
> Acked-by: Tom Zanussi
> Acked-by: Darren Hart
> Cc: Josh Triplett
> Cc: Matthew Garrett
> Cc: H. Peter Anvin
> Cc: Ingo Molnar
> Cc: Thomas Gleixner
> Cc: Greg Kroah-Hartman
> Cc:
> Signed-off-by: Josh Boyer
> Signed-off-by: Matt Fleming
> ---
> arch/x86/platform/efi/efi.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index e4a86a6..b9876aa 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -997,10 +997,13 @@ 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)
> - continue;
> + if (!(md->attribute & EFI_MEMORY_RUNTIME)) {
> +#ifdef CONFIG_X86_64
> + if (md->type != EFI_BOOT_SERVICES_CODE &&
> + md->type != EFI_BOOT_SERVICES_DATA)
> +#endif
> + continue;
> + }
>
> size = md->num_pages << EFI_PAGE_SHIFT;
>
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Fri, Apr 19, 2013 at 08:50:14AM +0100, Matt Fleming wrote: > On 04/19/2013 01:18 AM, Darren Hart wrote: > > On 04/18/2013 09:19 AM, Matt Fleming wrote: > >> > >> Could you give it a spin on your MinnowBoard? > > > > I've removed the patch I reference above and applied your patch to my > > 3.8.4 MinnowBoard dev tree. It panics with: > > D'oh. OK, at this point I'm inclined to apply Josh Boyer's patch on top > of my urgent branch which will address the WARNING people are hitting on > i386. I updated the commit message a little. > > Josh (Boyer), are you guys still carrying this patch and have you seen > any fallout? I notice your SoB isn't on the patch that Darren posted, am > I OK to add it? Yeah, we're still carrying it. We've had it around since August of 2011 to cover the bug mentioned. I'm not aware of any further fallout it might have caused, but again we don't support 32-bit EFI in Fedora. That's not to say Fedora won't work, we just don't focus on it. I don't actually remember authoring the patch, but it's been a while so: Signed-off-by: Josh Boyer josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode V2
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/19/2013 01:18 AM, Darren Hart wrote:
> On 04/18/2013 09:19 AM, Matt Fleming wrote:
>>
>> Could you give it a spin on your MinnowBoard?
>
> I've removed the patch I reference above and applied your patch to my
> 3.8.4 MinnowBoard dev tree. It panics with:
D'oh. OK, at this point I'm inclined to apply Josh Boyer's patch on top
of my urgent branch which will address the WARNING people are hitting on
i386. I updated the commit message a little.
Josh (Boyer), are you guys still carrying this patch and have you seen
any fallout? I notice your SoB isn't on the patch that Darren posted, am
I OK to add it?
---
>From 40f053eb6ccb3f0c462ef7a23c44c3264d87a0d4 Mon Sep 17 00:00:00 2001
From: Josh Boyer
Date: Thu, 18 Apr 2013 07:51:34 -0700
Subject: [PATCH] x86, efi: Don't map Boot Services on i386
Add patch to fix 32bit EFI service mapping (rhbz 726701)
Multiple people are reporting hitting the following WARNING on i386,
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:
[] warn_slowpath_common+0x5f/0x80
[] ? __ioremap_caller+0x3d3/0x440
[] ? __ioremap_caller+0x3d3/0x440
[] warn_slowpath_null+0x1d/0x20
[] __ioremap_caller+0x3d3/0x440
[] ? get_usage_chars+0xfb/0x110
[] ? vprintk_emit+0x147/0x480
[] ? efi_enter_virtual_mode+0x1e4/0x3de
[] ioremap_cache+0x1a/0x20
[] ? efi_enter_virtual_mode+0x1e4/0x3de
[] efi_enter_virtual_mode+0x1e4/0x3de
[] start_kernel+0x286/0x2f4
[] ? repair_env_string+0x51/0x51
[] i386_start_kernel+0x12c/0x12f
Due to the workaround described in commit 916f676f8 ("x86, efi: Retain
boot service code until after switching to virtual mode") EFI Boot
Service regions are mapped for a period during boot. Unfortunately, with
the limited size of the i386 direct kernel map it's possible that some
of the Boot Service regions will not be directly accessible, which
causes them to be ioremap()'d, triggering the above warning as the
regions are marked as E820_RAM in the e820 memmap.
There are currently only two situations where we need to map EFI Boot
Service regions,
1. To workaround the firmware bug described in 916f676f8
2. To access the ACPI BGRT image
but since we haven't seen an i386 implementation that requires either,
this simple fix should suffice for now.
[ Added to changelog - Matt ]
Reported-by: Bryan O'Donoghue
Acked-by: Tom Zanussi
Acked-by: Darren Hart
Cc: Josh Triplett
Cc: Matthew Garrett
Cc: H. Peter Anvin
Cc: Ingo Molnar
Cc: Thomas Gleixner
Cc: Greg Kroah-Hartman
Cc:
Signed-off-by: Josh Boyer
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e4a86a6..b9876aa 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -997,10 +997,13 @@ 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)
- continue;
+ if (!(md->attribute & EFI_MEMORY_RUNTIME)) {
+#ifdef CONFIG_X86_64
+ if (md->type != EFI_BOOT_SERVICES_CODE &&
+ md->type != EFI_BOOT_SERVICES_DATA)
+#endif
+ continue;
+ }
size = md->num_pages << EFI_PAGE_SHIFT;
end = md->phys_addr + size;
--
1.8.1.4
--
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode V2
Nope, just a vehicle for experiments. Greg KH 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode V2
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode V2
The only real use for command line options is to be able to get users to test something during troubleshooting. Greg KH 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode V2
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 09:19 AM, Matt Fleming wrote: > On 18/04/13 15:51, Darren Hart wrote: >> I don't believe I have seen a 32-bit EFI system with a BGRT, but then >> again, I had to look it up today! That said, I suspect the MinnowBoard >> would benefit from such a thing, so we should have an example of it >> there in the near future. > > That's fine and things will work as-is provided that the BGRT image is > not in highmem. > >> Is this in anyway related to the following patch from Josh Boyer? We're >> carrying this in the Yocto Project trees currently. >> >> commit 6f3e186bc7721c5b24ad90d4a751cccfccd445e6 >> Author: Josh Boyer >> Date: Fri Aug 5 08:47:23 2011 -0400 >> >> Add patch to fix 32bit EFI service mapping (rhbz 726701) >> >> Signed-off-by: Tom Zanussi >> Signed-off-by: Darren Hart > > Yep, it's basically the same fix. My patch just avoids the #ifdef and > prints an error message if we have EFI Boot services regions we can't > access directly. The error message will at least be useful if we do > start seeing BGRT pointers in highmem. > > Could you give it a spin on your MinnowBoard? I've removed the patch I reference above and applied your patch to my 3.8.4 MinnowBoard dev tree. It panics with: [ cut here ] WARNING: at arch/x86/mm/ioremap.c:102 __ioremap_caller+0x2cb/0x2f0() Hardware name: Minnow Modules linked in: Pid: 0, comm: swapper/0 Not tainted 3.8.4-yocto-standard+ #111 Call Trace: [] warn_slowpath_common+0x73/0xa0 [] ? __ioremap_caller+0x2cb/0x2f0 [] ? __ioremap_caller+0x2cb/0x2f0 [] warn_slowpath_null+0x23/0x30 [] __ioremap_caller+0x2cb/0x2f0 [] ? kmem_cache_alloc_trace+0x58/0x120 [] ? sysfs_slab_alias+0x2f/0x80 [] ? add_preempt_count+0x8/0x80 [] ? sub_preempt_count+0x8/0x80 [] ? walk_system_ram_range+0xfa/0x110 [] ioremap_cache+0x19/0x20 [] ? efi_ioremap+0x1b/0x23 [] efi_ioremap+0x1b/0x23 [] efi_enter_virtual_mode+0x195/0x346 [] start_kernel+0x288/0x30b [] ? repair_env_string+0x51/0x51 [] i386_start_kernel+0x78/0x7d ---[ end trace f74542647d553317 ]--- efi: ioremap of 0x3DE3F000 failed! efi: ioremap of 0x3E601000 failed! efi: ioremap of 0x3E602000 failed! efi: ioremap of 0x3E609000 failed! efi: ioremap of 0x3E60C000 failed! efi: ioremap of 0x3E614000 failed! efi: ioremap of 0x3E617000 failed! efi: ioremap of 0x3E621000 failed! efi: ioremap of 0x3E622000 failed! efi: ioremap of 0x3E627000 failed! efi: ioremap of 0x3E628000 failed! efi: ioremap of 0x3E62D000 failed! efi: ioremap of 0x3E62E000 failed! efi: ioremap of 0x3E631000 failed! efi: ioremap of 0x3E632000 failed! efi: ioremap of 0x3E63E000 failed! efi: ioremap of 0x3EA0E000 failed! efi: ioremap of 0x3EA0F000 failed! efi: ioremap of 0x3EA11000 failed! efi: ioremap of 0x3EA1A000 failed! efi: ioremap of 0x3EA1B000 failed! efi: ioremap of 0x3EA1D000 failed! efi: ioremap of 0x3EC0B000 failed! efi: ioremap of 0x3EC0F000 failed! efi: ioremap of 0x3F33F000 failed! efi: ioremap of 0xFED1C000 failed! [ cut here ] kernel BUG at arch/x86/platform/efi/efi.c:933! invalid opcode: [#1] PREEMPT SMP Modules linked in: Pid: 0, comm: swapper/0 Tainted: GW3.8.4-yocto-standard+ #111 Circuitco Minnow/Minnow Board EIP: 0060:[] EFLAGS: 00010246 CPU: 0 EIP is at efi_enter_virtual_mode+0x244/0x346 EAX: ee012930 EBX: 0030 ECX: EDX: 00d0 ESI: ffe190b0 EDI: ee012960 EBP: c18adfc4 ESP: c18adf7c DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 CR0: 8005003b CR2: ffe17000 CR3: 019b4000 CR4: 0690 DR0: DR1: DR2: DR3: DR6: 0ff0 DR7: 0400 Process swapper/0 (pid: 0, ti=c18ac000 task=c18b70e0 task.ti=c18ac000) Stack: 8000 0004 0004 8000 a000 0002effe 0032 ee012000 fffaa000 ffe190b0 c195b3c0 0008a800 c18ae800 c18adfe4 c19208b8 0113 c1920462 Call Trace: [] start_kernel+0x288/0x30b [] ? repair_env_string+0x51/0x51 [] i386_start_kernel+0x78/0x7d Code: 03 45 e4 89 c7 f3 a4 89 5d e0 8b 1d fc d7 9b c1 01 5d f0 8b 75 f0 39 35 f0 d7 9b c1 0f 87 82 fe ff ff 83 3d a0 be 91 c1 00 75 02 <0f> 0b 8b 1d fc d7 9b c1 8b 75 e0 0f af f3 8b 3d f8 d7 9b c1 e8 EIP: [] efi_enter_virtual_mode+0x244/0x346 SS:ESP 0068:c18adf7c ---[ end trace f74542647d553318 ]--- Kernel panic - not syncing: Attempted to kill the idle task! -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 04:01 PM, Josh Triplett wrote: > > I'd much rather see the code do the right thing automatically, rather > than requiring an "unbreak me" command-line parameter. > > But in any case, since BGRT is a "make my system look prettier" feature > rather than core functionality, giving up on it on 32-bit EFI seems > fairly reasonable, especially since it may still work if stored > sufficiently low in memory. > No, it really isn't. Quite frankly, I don't understand why HIGHMEM makes it any harder... in a lot of ways HIGHMEM memory is easier to deal with because it is touched much later in the process. Sure, it has to be mapped before you can copy it out, but that usually isn't a big deal. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
> > "UEI" is Celtic for "We missed DOS so we burned it into your ROMS" > > I love it "maith an fear" > >> There are currently only two situations where we need to map EFI Boot >> Service regions, >> >>1. To workaround the firmware bug described in 916f676f8 >>2. To access the ACPI BGRT image >> >> but since we haven't seen an i386 implementation that requires either, >> this simple fix should suffice for now. Item 2. above does still work on >> i386 provided that the BGRT image is not in highmem. > > Matt, Peter, Josh, Darren. > > Given it's not possible to guarantee someone won't stuff a BGRT into > EFI_BOOT_MEMORY >= highmem eventually (and indeed the axioms of the > universe pretty much guarantee eventually it will be so) - I'd suggest > version 2. > Quite frankly, it is quite likely. > A kernel parameter - rather than a probe for BGRT - since we anticipate > BIOS bugs on the way. Pardon? Kernel parameter for what? -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, Apr 18, 2013 at 11:07:06PM +0100, Bryan O'Donoghue wrote: > > > UEFI stands for "Unified Extensible Firmware Interface", where > > "Firmware" > > is an ancient African word meaning "Why do something right when you can > > do it so wrong that children will weep and brave adults will cower > > before > > you", and "UEI" is Celtic for "We missed DOS so we burned it into your > > ROMs". The UEFI specification provides for runtime services (ie, another > > way for the operating system to be forced to depend on the firmware) and > > we rely on these for certain trivial tasks such as setting up the > > bootloader. But some hardware fails to work if we attempt to use these > > runtime services from physical mode, and so we have to switch into > > virtual > > mode. So far so dreadful. > > > > "UEI" is Celtic for "We missed DOS so we burned it into your ROMS" > > I love it "maith an fear" > > >There are currently only two situations where we need to map EFI Boot > >Service regions, > > > > 1. To workaround the firmware bug described in 916f676f8 > > 2. To access the ACPI BGRT image > > > >but since we haven't seen an i386 implementation that requires either, > >this simple fix should suffice for now. Item 2. above does still work on > >i386 provided that the BGRT image is not in highmem. > > Matt, Peter, Josh, Darren. > > Given it's not possible to guarantee someone won't stuff a BGRT into > EFI_BOOT_MEMORY >= highmem eventually (and indeed the axioms of the > universe pretty much guarantee eventually it will be so) - I'd > suggest version 2. > > A kernel parameter - rather than a probe for BGRT - since we > anticipate BIOS bugs on the way. > > Version 2 of the submitted path introduces an early kernel parameter > "virt_mapboot" - which is true by default (maintaining the current > behavior of mapping EFI_BOOT_MEMORY by default) - but which can be > set to false - if your IA32 BIOS is not buggy. > > Perhaps it would be better to be optimistic. > > Change the behavior of efi_enter_virtual_mode() to do the right > thing re: the standard and require passing of a parameter to switch > on work-arounds for non-standards conformant BIOS. Note: this > approach would break BGRT code - requiring addition of kernel > parameters to existing systems - which from a user-friendliness POV > is probably verboten I'd much rather see the code do the right thing automatically, rather than requiring an "unbreak me" command-line parameter. But in any case, since BGRT is a "make my system look prettier" feature rather than core functionality, giving up on it on 32-bit EFI seems fairly reasonable, especially since it may still work if stored sufficiently low in memory. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
UEFI stands for "Unified Extensible Firmware Interface", where "Firmware" is an ancient African word meaning "Why do something right when you can do it so wrong that children will weep and brave adults will cower before you", and "UEI" is Celtic for "We missed DOS so we burned it into your ROMs". The UEFI specification provides for runtime services (ie, another way for the operating system to be forced to depend on the firmware) and we rely on these for certain trivial tasks such as setting up the bootloader. But some hardware fails to work if we attempt to use these runtime services from physical mode, and so we have to switch into virtual mode. So far so dreadful. "UEI" is Celtic for "We missed DOS so we burned it into your ROMS" I love it "maith an fear" There are currently only two situations where we need to map EFI Boot Service regions, 1. To workaround the firmware bug described in 916f676f8 2. To access the ACPI BGRT image but since we haven't seen an i386 implementation that requires either, this simple fix should suffice for now. Item 2. above does still work on i386 provided that the BGRT image is not in highmem. Matt, Peter, Josh, Darren. Given it's not possible to guarantee someone won't stuff a BGRT into EFI_BOOT_MEMORY >= highmem eventually (and indeed the axioms of the universe pretty much guarantee eventually it will be so) - I'd suggest version 2. A kernel parameter - rather than a probe for BGRT - since we anticipate BIOS bugs on the way. Version 2 of the submitted path introduces an early kernel parameter "virt_mapboot" - which is true by default (maintaining the current behavior of mapping EFI_BOOT_MEMORY by default) - but which can be set to false - if your IA32 BIOS is not buggy. Perhaps it would be better to be optimistic. Change the behavior of efi_enter_virtual_mode() to do the right thing re: the standard and require passing of a parameter to switch on work-arounds for non-standards conformant BIOS. Note: this approach would break BGRT code - requiring addition of kernel parameters to existing systems - which from a user-friendliness POV is probably verboten -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 01:17 PM, Darren Hart wrote: > > Ah, well then I've misunderstood the nature of the problem a bit. Will > have to spend some time understanding this better. > Way, way too many people misunderstand HIGHMEM. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 01:13 PM, H. Peter Anvin wrote: > On 04/18/2013 01:11 PM, Darren Hart wrote: >>> >>> I would expect that if there are any 32-bit UEFI systems that ship with >>> BGRT support (and Darren makes it sound like that's a possibility), >>> there's a realistic chance of the BGRT ending up allocated above the >>> highmem barrier. >> >> I can certainly ensure we sit below that barrier on the MinnowBoard. We >> can also speak with the Intel UEFI firmware development teams to see >> about making that a requirement. I don't know if we'll be successful, >> but Matt, Peter, and I have recently coaxed some changes of that nature in. >> > > No, that is the wrong approach. The HIGHMEM barrier is a Linux kernel > construct and isn't even guaranteed to be the same from one boot *of the > same kernel* to another. The value 896M is merely the default, but it > can be affected by both compile time and command line options. > > -hpa Ah, well then I've misunderstood the nature of the problem a bit. Will have to spend some time understanding this better. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 01:11 PM, Darren Hart wrote: >> >> I would expect that if there are any 32-bit UEFI systems that ship with >> BGRT support (and Darren makes it sound like that's a possibility), >> there's a realistic chance of the BGRT ending up allocated above the >> highmem barrier. > > I can certainly ensure we sit below that barrier on the MinnowBoard. We > can also speak with the Intel UEFI firmware development teams to see > about making that a requirement. I don't know if we'll be successful, > but Matt, Peter, and I have recently coaxed some changes of that nature in. > No, that is the wrong approach. The HIGHMEM barrier is a Linux kernel construct and isn't even guaranteed to be the same from one boot *of the same kernel* to another. The value 896M is merely the default, but it can be affected by both compile time and command line options. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 12:58 PM, Matthew Garrett wrote: > On Thu, 2013-04-18 at 20:55 +0100, Matt Fleming wrote: > >> If there are machines out there that would require us to write support, >> it's probably worth doing now instead of punting. But it sounds like >> there aren't any. > > I would expect that if there are any 32-bit UEFI systems that ship with > BGRT support (and Darren makes it sound like that's a possibility), > there's a realistic chance of the BGRT ending up allocated above the > highmem barrier. I can certainly ensure we sit below that barrier on the MinnowBoard. We can also speak with the Intel UEFI firmware development teams to see about making that a requirement. I don't know if we'll be successful, but Matt, Peter, and I have recently coaxed some changes of that nature in. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, 2013-04-18 at 20:55 +0100, Matt Fleming wrote:
> If there are machines out there that would require us to write support,
> it's probably worth doing now instead of punting. But it sounds like
> there aren't any.
I would expect that if there are any 32-bit UEFI systems that ship with
BGRT support (and Darren makes it sound like that's a possibility),
there's a realistic chance of the BGRT ending up allocated above the
highmem barrier.
--
Matthew Garrett | mj...@srcf.ucam.org
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 12:55 PM, Matt Fleming wrote: > On 04/18/2013 05:44 PM, Josh Triplett wrote: The machine I developed the BGRT changes on kept the image below the 4G mark, inside one of the memory regions reclaimable via ExitBootServices(). >>> >>> Well, highmem is >= ~896M. Do you have a machine with BGRT over the >>> highmem mark? >> >> I don't have the machine in question anymore, and I don't remember. > > Sorry, I should have been more clear - having a BGRT image in highmem > has never worked for the reasons I outlined in my previous mail. What I > was really asking was: is it OK that we now explicitly don't support > that case? I'm working on the assumption that it's pointless writing > support for the BGRT in highmem because no such i386 machines exist. If > the BGRT code works for your i386 right now, the address isn't in highmem. > > If there are machines out there that would require us to write support, > it's probably worth doing now instead of punting. But it sounds like > there aren't any. > I suspect if there aren't any yet there WILL be. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 05:44 PM, Josh Triplett wrote: >>> The machine I developed the BGRT changes on kept the image below the 4G >>> mark, inside one of the memory regions reclaimable via >>> ExitBootServices(). >> >> Well, highmem is >= ~896M. Do you have a machine with BGRT over the >> highmem mark? > > I don't have the machine in question anymore, and I don't remember. Sorry, I should have been more clear - having a BGRT image in highmem has never worked for the reasons I outlined in my previous mail. What I was really asking was: is it OK that we now explicitly don't support that case? I'm working on the assumption that it's pointless writing support for the BGRT in highmem because no such i386 machines exist. If the BGRT code works for your i386 right now, the address isn't in highmem. If there are machines out there that would require us to write support, it's probably worth doing now instead of punting. But it sounds like there aren't any. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, Apr 18, 2013 at 09:38:04AM -0700, H. Peter Anvin wrote: > On 04/18/2013 09:33 AM, Josh Triplett wrote: > > On Thu, Apr 18, 2013 at 12:00:26PM +0100, Matt Fleming wrote: > >>> No, no - we *don't* have a BGRT object at all. > >>> > >>> We have a completely clean memory map - but the BGRT code is causing the > >>> is_ram() failure. > >> > >> You assume that mapping of the Boot Services regions is done purely for > >> the benefit of pulling out the bgrt image - it's not, see the above > >> commit log - and I assumed that you had an ACPI bgrt pointer in your > >> memory map, but you don't. > >> > >> Darren, Josh, have you ever seen an i386 machine with a bgrt pointer? If > >> not, and given that we've never seen an i386 firmware that requires the > >> above workaround from Matthew, combined with the fact that there are so > >> few i386 implementations out there, I'm inclined to apply the patch > >> below, because anything else is a lot more work. We can address this > >> properly if we ever start seeing i386 machines with bgrt pointers that > >> reference highmem. > > > > The machine I developed the BGRT changes on kept the image below the 4G > > mark, inside one of the memory regions reclaimable via > > ExitBootServices(). > > Well, highmem is >= ~896M. Do you have a machine with BGRT over the > highmem mark? I don't have the machine in question anymore, and I don't remember. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 09:33 AM, Josh Triplett wrote: > On Thu, Apr 18, 2013 at 12:00:26PM +0100, Matt Fleming wrote: >>> No, no - we *don't* have a BGRT object at all. >>> >>> We have a completely clean memory map - but the BGRT code is causing the >>> is_ram() failure. >> >> You assume that mapping of the Boot Services regions is done purely for >> the benefit of pulling out the bgrt image - it's not, see the above >> commit log - and I assumed that you had an ACPI bgrt pointer in your >> memory map, but you don't. >> >> Darren, Josh, have you ever seen an i386 machine with a bgrt pointer? If >> not, and given that we've never seen an i386 firmware that requires the >> above workaround from Matthew, combined with the fact that there are so >> few i386 implementations out there, I'm inclined to apply the patch >> below, because anything else is a lot more work. We can address this >> properly if we ever start seeing i386 machines with bgrt pointers that >> reference highmem. > > The machine I developed the BGRT changes on kept the image below the 4G > mark, inside one of the memory regions reclaimable via > ExitBootServices(). > Well, highmem is >= ~896M. Do you have a machine with BGRT over the highmem mark? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, Apr 18, 2013 at 12:00:26PM +0100, Matt Fleming wrote: > > No, no - we *don't* have a BGRT object at all. > > > > We have a completely clean memory map - but the BGRT code is causing the > > is_ram() failure. > > You assume that mapping of the Boot Services regions is done purely for > the benefit of pulling out the bgrt image - it's not, see the above > commit log - and I assumed that you had an ACPI bgrt pointer in your > memory map, but you don't. > > Darren, Josh, have you ever seen an i386 machine with a bgrt pointer? If > not, and given that we've never seen an i386 firmware that requires the > above workaround from Matthew, combined with the fact that there are so > few i386 implementations out there, I'm inclined to apply the patch > below, because anything else is a lot more work. We can address this > properly if we ever start seeing i386 machines with bgrt pointers that > reference highmem. The machine I developed the BGRT changes on kept the image below the 4G mark, inside one of the memory regions reclaimable via ExitBootServices(). - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 18/04/13 15:51, Darren Hart wrote: > I don't believe I have seen a 32-bit EFI system with a BGRT, but then > again, I had to look it up today! That said, I suspect the MinnowBoard > would benefit from such a thing, so we should have an example of it > there in the near future. That's fine and things will work as-is provided that the BGRT image is not in highmem. > Is this in anyway related to the following patch from Josh Boyer? We're > carrying this in the Yocto Project trees currently. > > commit 6f3e186bc7721c5b24ad90d4a751cccfccd445e6 > Author: Josh Boyer > Date: Fri Aug 5 08:47:23 2011 -0400 > > Add patch to fix 32bit EFI service mapping (rhbz 726701) > > Signed-off-by: Tom Zanussi > Signed-off-by: Darren Hart Yep, it's basically the same fix. My patch just avoids the #ifdef and prints an error message if we have EFI Boot services regions we can't access directly. The error message will at least be useful if we do start seeing BGRT pointers in highmem. Could you give it a spin on your MinnowBoard? -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, Apr 18, 2013 at 03:01:12PM +, Matthew Garrett wrote: > On Thu, 2013-04-18 at 09:40 -0400, Josh Boyer wrote: > > > BGRT is a new addition in ACPI 5.0, right? Hopefully with it being > > relatively recent, and new 32-bit firmware being somewhat rare, it won't > > be a problem. > > New 32-bit UEFI firmware is fairly common. Thanks, Intel. See. I said I was the wrong person to weigh in. And I'm now also sad. josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, 2013-04-18 at 09:40 -0400, Josh Boyer wrote:
> BGRT is a new addition in ACPI 5.0, right? Hopefully with it being
> relatively recent, and new 32-bit firmware being somewhat rare, it won't
> be a problem.
New 32-bit UEFI firmware is fairly common. Thanks, Intel.
--
Matthew Garrett | mj...@srcf.ucam.org
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 04/18/2013 04:00 AM, Matt Fleming wrote:
> On 17/04/13 23:00, Bryan O'Donoghue wrote:
>> In my mind the only memory that is relevant to efi_enter_virtual_mode is
>> memory that the BIOS has marked as EFI_RUNTIME_SERVICE
>>
>> md->attribute & 0x8000_
>>
>> I couldn't quite understand why the code in
>>
>> arch/x86/platform/efi/efi.c:enter_virtual_mode() looks like this
>>
>> 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)
>> continue;
>>
>> While the code in
>>
>> arch/ia64/kernel/efi.c:enter_virtual_mode
>>
>> for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
>> md = p;
>> if (md->attribute & EFI_MEMORY_RUNTIME) {
>>
>> The ia64 version is consistent with the standard - but obviously isn't
>> accounting for the work-around implemented to retrieve the BGRT on ia32.
>>
>> Looking at the commit message associated with
>> arch/x86/platform/efi/efi-bgrt.c
>>
>> It's pretty obvious the mapping of boot code/data was done to facilitate
>> BGRT.
>
> No, that's incorrect. The patch that introduced the mapping of
> EFI_BOOT_SERVICES_{CODE,DATA} was committed before support for bgrt
> existed. git blame is a good tool to use when doing one of these
> historical digs, and in this case it shows that the above lines from
> efi_enter_virtual_mode() were introduced in the following commit,
>
> commit 916f676f8dc016103f983c7ec54c18ecdbb6e349
> Author: Matthew Garrett
> Date: Wed May 25 09:53:13 2011 -0400
>
> x86, efi: Retain boot service code until after switching to virtual mode
>
> UEFI stands for "Unified Extensible Firmware Interface", where "Firmware"
> is an ancient African word meaning "Why do something right when you can
> do it so wrong that children will weep and brave adults will cower before
> you", and "UEI" is Celtic for "We missed DOS so we burned it into your
> ROMs". The UEFI specification provides for runtime services (ie, another
> way for the operating system to be forced to depend on the firmware) and
> we rely on these for certain trivial tasks such as setting up the
> bootloader. But some hardware fails to work if we attempt to use these
> runtime services from physical mode, and so we have to switch into virtual
> mode. So far so dreadful.
>
> The specification makes it clear that the operating system is free to do
> whatever it wants with boot services code after ExitBootServices() has
> been
> called. SetVirtualAddressMap() can't be called until ExitBootServices()
> has
> been. So, obviously, a whole bunch of EFI implementations call into boot
> services code when we do that. Since we've been charmingly naive and
> trusted that the specification may be somehow relevant to the real world,
> we've already stuffed a picture of a penguin or something in that address
> space. And just to make things more entertaining, we've also marked it
> non-executable.
>
> This patch allocates the boot services regions during EFI init and makes
> sure that they're executable. Then, after SetVirtualAddressMap(), it
> discards them and everyone lives happily ever after. Except for the ones
> who have to work on EFI, who live sad lives haunted by the knowledge that
> someone's eventually going to write yet another firmware specification.
>
> [ hpa: adding this to urgent with a stable tag since it fixes
> currently-broken
> hardware. However, I do not know what the dependencies are and so I do
> not know which -stable versions this may be a candidate for. ]
>
> Signed-off-by: Matthew Garrett
> Link:
> http://lkml.kernel.org/r/1306331593-28715-1-git-send-email-...@redhat.com
> Signed-off-by: H. Peter Anvin
> Cc: Tony Luck
> Cc:
>
> Yes the bgrt code accesses the Boot Service mappings, but that isn't the
> only reason we want to map those regions.
>
>> That's one solution - you'd need to make the i386::efi_ioremap() aware
>> of the BGRT work-around.
>>
>> Presumably this work-around is also required on ia64 too.
>
> No, we've never seen an ia64 firmware implementation with the "access
> EFI Boot Services Code/Data after ExitBootServices() bug", and it
> doesn't suffer from the same virtual address space limitations that i386
> does.
>
>> No, no - we *don't* have a BGRT object at all.
>>
>> We have a completely clean memory map - but the BGRT code is causing the
>> is_ram() failure.
>
> You assume that mapping of the Boot Services regions is done purely for
> the benefit of pulling out the bgrt image - it's not, see the above
> commit log - and I assumed that you had an ACPI bgrt pointer in your
> memory map, but you don't.
>
> Darren, Josh, have you ever seen a
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On Thu, Apr 18, 2013 at 12:00:26PM +0100, Matt Fleming wrote:
> On 17/04/13 23:00, Bryan O'Donoghue wrote:
> > In my mind the only memory that is relevant to efi_enter_virtual_mode is
> > memory that the BIOS has marked as EFI_RUNTIME_SERVICE
> >
> > md->attribute & 0x8000_
> >
> > I couldn't quite understand why the code in
> >
> > arch/x86/platform/efi/efi.c:enter_virtual_mode() looks like this
> >
> > 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)
> > continue;
> >
> > While the code in
> >
> > arch/ia64/kernel/efi.c:enter_virtual_mode
> >
> > for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
> > md = p;
> > if (md->attribute & EFI_MEMORY_RUNTIME) {
> >
> > The ia64 version is consistent with the standard - but obviously isn't
> > accounting for the work-around implemented to retrieve the BGRT on ia32.
> >
> > Looking at the commit message associated with
> > arch/x86/platform/efi/efi-bgrt.c
> >
> > It's pretty obvious the mapping of boot code/data was done to facilitate
> > BGRT.
>
> No, that's incorrect. The patch that introduced the mapping of
> EFI_BOOT_SERVICES_{CODE,DATA} was committed before support for bgrt
> existed. git blame is a good tool to use when doing one of these
> historical digs, and in this case it shows that the above lines from
> efi_enter_virtual_mode() were introduced in the following commit,
>
> commit 916f676f8dc016103f983c7ec54c18ecdbb6e349
> Author: Matthew Garrett
> Date: Wed May 25 09:53:13 2011 -0400
>
> x86, efi: Retain boot service code until after switching to virtual mode
>
> UEFI stands for "Unified Extensible Firmware Interface", where "Firmware"
> is an ancient African word meaning "Why do something right when you can
> do it so wrong that children will weep and brave adults will cower before
> you", and "UEI" is Celtic for "We missed DOS so we burned it into your
> ROMs". The UEFI specification provides for runtime services (ie, another
> way for the operating system to be forced to depend on the firmware) and
> we rely on these for certain trivial tasks such as setting up the
> bootloader. But some hardware fails to work if we attempt to use these
> runtime services from physical mode, and so we have to switch into virtual
> mode. So far so dreadful.
>
> The specification makes it clear that the operating system is free to do
> whatever it wants with boot services code after ExitBootServices() has
> been
> called. SetVirtualAddressMap() can't be called until ExitBootServices()
> has
> been. So, obviously, a whole bunch of EFI implementations call into boot
> services code when we do that. Since we've been charmingly naive and
> trusted that the specification may be somehow relevant to the real world,
> we've already stuffed a picture of a penguin or something in that address
> space. And just to make things more entertaining, we've also marked it
> non-executable.
>
> This patch allocates the boot services regions during EFI init and makes
> sure that they're executable. Then, after SetVirtualAddressMap(), it
> discards them and everyone lives happily ever after. Except for the ones
> who have to work on EFI, who live sad lives haunted by the knowledge that
> someone's eventually going to write yet another firmware specification.
>
> [ hpa: adding this to urgent with a stable tag since it fixes
> currently-broken
> hardware. However, I do not know what the dependencies are and so I do
> not know which -stable versions this may be a candidate for. ]
>
> Signed-off-by: Matthew Garrett
> Link:
> http://lkml.kernel.org/r/1306331593-28715-1-git-send-email-...@redhat.com
> Signed-off-by: H. Peter Anvin
> Cc: Tony Luck
> Cc:
>
> Yes the bgrt code accesses the Boot Service mappings, but that isn't the
> only reason we want to map those regions.
>
> > That's one solution - you'd need to make the i386::efi_ioremap() aware
> > of the BGRT work-around.
> >
> > Presumably this work-around is also required on ia64 too.
>
> No, we've never seen an ia64 firmware implementation with the "access
> EFI Boot Services Code/Data after ExitBootServices() bug", and it
> doesn't suffer from the same virtual address space limitations that i386
> does.
>
> > No, no - we *don't* have a BGRT object at all.
> >
> > We have a completely clean memory map - but the BGRT code is causing the
> > is_ram() failure.
>
> You assume that mapping of the Boot Services regions is done purely for
> the benefit of pulling out the bgrt image - it's not, see the above
> commit log - and I assumed that you had an ACPI bgrt pointer in you
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 17/04/13 23:00, Bryan O'Donoghue wrote:
> In my mind the only memory that is relevant to efi_enter_virtual_mode is
> memory that the BIOS has marked as EFI_RUNTIME_SERVICE
>
> md->attribute & 0x8000_
>
> I couldn't quite understand why the code in
>
> arch/x86/platform/efi/efi.c:enter_virtual_mode() looks like this
>
> 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)
> continue;
>
> While the code in
>
> arch/ia64/kernel/efi.c:enter_virtual_mode
>
> for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
> md = p;
> if (md->attribute & EFI_MEMORY_RUNTIME) {
>
> The ia64 version is consistent with the standard - but obviously isn't
> accounting for the work-around implemented to retrieve the BGRT on ia32.
>
> Looking at the commit message associated with
> arch/x86/platform/efi/efi-bgrt.c
>
> It's pretty obvious the mapping of boot code/data was done to facilitate
> BGRT.
No, that's incorrect. The patch that introduced the mapping of
EFI_BOOT_SERVICES_{CODE,DATA} was committed before support for bgrt
existed. git blame is a good tool to use when doing one of these
historical digs, and in this case it shows that the above lines from
efi_enter_virtual_mode() were introduced in the following commit,
commit 916f676f8dc016103f983c7ec54c18ecdbb6e349
Author: Matthew Garrett
Date: Wed May 25 09:53:13 2011 -0400
x86, efi: Retain boot service code until after switching to virtual mode
UEFI stands for "Unified Extensible Firmware Interface", where "Firmware"
is an ancient African word meaning "Why do something right when you can
do it so wrong that children will weep and brave adults will cower before
you", and "UEI" is Celtic for "We missed DOS so we burned it into your
ROMs". The UEFI specification provides for runtime services (ie, another
way for the operating system to be forced to depend on the firmware) and
we rely on these for certain trivial tasks such as setting up the
bootloader. But some hardware fails to work if we attempt to use these
runtime services from physical mode, and so we have to switch into virtual
mode. So far so dreadful.
The specification makes it clear that the operating system is free to do
whatever it wants with boot services code after ExitBootServices() has been
called. SetVirtualAddressMap() can't be called until ExitBootServices() has
been. So, obviously, a whole bunch of EFI implementations call into boot
services code when we do that. Since we've been charmingly naive and
trusted that the specification may be somehow relevant to the real world,
we've already stuffed a picture of a penguin or something in that address
space. And just to make things more entertaining, we've also marked it
non-executable.
This patch allocates the boot services regions during EFI init and makes
sure that they're executable. Then, after SetVirtualAddressMap(), it
discards them and everyone lives happily ever after. Except for the ones
who have to work on EFI, who live sad lives haunted by the knowledge that
someone's eventually going to write yet another firmware specification.
[ hpa: adding this to urgent with a stable tag since it fixes
currently-broken
hardware. However, I do not know what the dependencies are and so I do
not know which -stable versions this may be a candidate for. ]
Signed-off-by: Matthew Garrett
Link:
http://lkml.kernel.org/r/1306331593-28715-1-git-send-email-...@redhat.com
Signed-off-by: H. Peter Anvin
Cc: Tony Luck
Cc:
Yes the bgrt code accesses the Boot Service mappings, but that isn't the
only reason we want to map those regions.
> That's one solution - you'd need to make the i386::efi_ioremap() aware
> of the BGRT work-around.
>
> Presumably this work-around is also required on ia64 too.
No, we've never seen an ia64 firmware implementation with the "access
EFI Boot Services Code/Data after ExitBootServices() bug", and it
doesn't suffer from the same virtual address space limitations that i386
does.
> No, no - we *don't* have a BGRT object at all.
>
> We have a completely clean memory map - but the BGRT code is causing the
> is_ram() failure.
You assume that mapping of the Boot Services regions is done purely for
the benefit of pulling out the bgrt image - it's not, see the above
commit log - and I assumed that you had an ACPI bgrt pointer in your
memory map, but you don't.
Darren, Josh, have you ever seen an i386 machine with a bgrt pointer? If
not, and given that we've never seen an i386 firmware that requires the
above workaround from Matthew, combined with the fact that there are so
few i386 implementations out ther
Re: [PATCH] Remove warning in efi_enter_virtual_mode
On 17/04/13 15:06, Matt Fleming wrote:
(Cc'ing some more folks)
On 16/04/13 16:58, Bryan O'Donoghue wrote:
This warning is caused by efi_enter_virtual_mode mapping memory marked
as !EFI_RUNTIME_MEMORY. The reason this memory is being mapped is to
work around buggy code that stores an ACPI object called the Boot
Graphics Resource Table - BGRT in memory of type EfiBootServices*.
[ 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:
[] warn_slowpath_common+0x5f/0x80
[] ? __ioremap_caller+0x3d3/0x440
[] ? __ioremap_caller+0x3d3/0x440
[] warn_slowpath_null+0x1d/0x20
[] __ioremap_caller+0x3d3/0x440
[] ? get_usage_chars+0xfb/0x110
[] ? vprintk_emit+0x147/0x480
[] ? efi_enter_virtual_mode+0x1e4/0x3de
[] ioremap_cache+0x1a/0x20
[] ? efi_enter_virtual_mode+0x1e4/0x3de
[] efi_enter_virtual_mode+0x1e4/0x3de
[] start_kernel+0x286/0x2f4
[] ? repair_env_string+0x51/0x51
[] i386_start_kernel+0x12c/0x12f
---[ end trace 4eaa2a86a8e2da22 ]---
The warning is telling you that someone is trying to ioremap RAM, which
is bad. It's not specifically an issue with the bgrt image, it's just
that said object happens to occupy an EfiBootServicesData region which
isn't mapped by the direct kernel mapping on i386.
I understand that.
In my mind the only memory that is relevant to efi_enter_virtual_mode is
memory that the BIOS has marked as EFI_RUNTIME_SERVICE
md->attribute & 0x8000_
I couldn't quite understand why the code in
arch/x86/platform/efi/efi.c:enter_virtual_mode() looks like this
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)
continue;
While the code in
arch/ia64/kernel/efi.c:enter_virtual_mode
for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
md = p;
if (md->attribute & EFI_MEMORY_RUNTIME) {
The ia64 version is consistent with the standard - but obviously isn't
accounting for the work-around implemented to retrieve the BGRT on ia32.
Looking at the commit message associated with
arch/x86/platform/efi/efi-bgrt.c
It's pretty obvious the mapping of boot code/data was done to facilitate
BGRT.
Since the EFI memory map I'm using is clean - below - there's no reason
for us to map the boot code/data.
Most (all?) boot loaders mark EFI_BOOT_SERVICES_{CODE,DATA} as E820_RAM,
which is why ioremap() complained about you trying to ioremap() a page
of RAM.
But they aught to. It's entirely legitimate for the run-time to reclaim
this memory - since after ExitBootServices() - none of the code/data
inside of EFI_BOOT_SERVICES is of any use.
Caveat being the work-around that was done for BGRT.
They do this because after efi_free_boot_services() we want
these regions to be available for general allocation.
Agree.
On x86-64 you rarely hit the ioremap() path because all RAM regions are
mapped by the kernel direct mapping, e.g __va() works on those
addresses. On i386, with its reduced virtual address space, it's much
more likely that those addresses are not part of the direct mapping, and
so this is the chunk of code that causes problems in
efi_enter_virtual_mode(),
start_pfn = PFN_DOWN(md->phys_addr);
end_pfn = PFN_UP(end);
if (pfn_range_is_mapped(start_pfn, end_pfn)) {
va = __va(md->phys_addr);
if (!(md->attribute& EFI_MEMORY_WB))
efi_memory_uc((u64)(unsigned long)va, size);
} else
va = efi_ioremap(md->phys_addr, size,
md->type, md->attribute);
Yes it fails sanity checking in efi_ioremap::__ioremap_caller on an
"is_ram()" call.
What we probably need is an i386-specific implementation of
efi_ioremap() that checks to see whether we're mapping a RAM region. I
thought of maybe using the kmap_high() functions, but I don't think
repeated calls to the kmap*() functions are guaranteed to provide
consecutive virtual addresses, and I doubt free_bootmem() (called from
efi_free_boot_services()) understands kmap'd addresses.
That's one solution - you'd need to make the i386::efi_ioremap() aware
of the BGRT work-around.
Presumably this work-around is also required on ia64 too.
Maybe we should hot-add the EFI_BOOT_SERVICES_* regions once we've
finished with them and only then mark them as RAM?
x86 folks? Halp?
On systems that do not have a BGRT object, there's no reason to map this
memory in efi_enter_virtual_mode. This patch searches for the BGRT
object and if found - will continue to map the boot services memory,
else only memory with a
Re: [PATCH] Remove warning in efi_enter_virtual_mode
(Cc'ing some more folks)
On 16/04/13 16:58, Bryan O'Donoghue wrote:
> This warning is caused by efi_enter_virtual_mode mapping memory marked
> as !EFI_RUNTIME_MEMORY. The reason this memory is being mapped is to
> work around buggy code that stores an ACPI object called the Boot
> Graphics Resource Table - BGRT in memory of type EfiBootServices*.
>
> [ 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:
> [] warn_slowpath_common+0x5f/0x80
> [] ? __ioremap_caller+0x3d3/0x440
> [] ? __ioremap_caller+0x3d3/0x440
> [] warn_slowpath_null+0x1d/0x20
> [] __ioremap_caller+0x3d3/0x440
> [] ? get_usage_chars+0xfb/0x110
> [] ? vprintk_emit+0x147/0x480
> [] ? efi_enter_virtual_mode+0x1e4/0x3de
> [] ioremap_cache+0x1a/0x20
> [] ? efi_enter_virtual_mode+0x1e4/0x3de
> [] efi_enter_virtual_mode+0x1e4/0x3de
> [] start_kernel+0x286/0x2f4
> [] ? repair_env_string+0x51/0x51
> [] i386_start_kernel+0x12c/0x12f
> ---[ end trace 4eaa2a86a8e2da22 ]---
The warning is telling you that someone is trying to ioremap RAM, which
is bad. It's not specifically an issue with the bgrt image, it's just
that said object happens to occupy an EfiBootServicesData region which
isn't mapped by the direct kernel mapping on i386.
Most (all?) boot loaders mark EFI_BOOT_SERVICES_{CODE,DATA} as E820_RAM,
which is why ioremap() complained about you trying to ioremap() a page
of RAM. They do this because after efi_free_boot_services() we want
these regions to be available for general allocation.
On x86-64 you rarely hit the ioremap() path because all RAM regions are
mapped by the kernel direct mapping, e.g __va() works on those
addresses. On i386, with its reduced virtual address space, it's much
more likely that those addresses are not part of the direct mapping, and
so this is the chunk of code that causes problems in
efi_enter_virtual_mode(),
start_pfn = PFN_DOWN(md->phys_addr);
end_pfn = PFN_UP(end);
if (pfn_range_is_mapped(start_pfn, end_pfn)) {
va = __va(md->phys_addr);
if (!(md->attribute & EFI_MEMORY_WB))
efi_memory_uc((u64)(unsigned long)va, size);
} else
va = efi_ioremap(md->phys_addr, size,
md->type, md->attribute);
What we probably need is an i386-specific implementation of
efi_ioremap() that checks to see whether we're mapping a RAM region. I
thought of maybe using the kmap_high() functions, but I don't think
repeated calls to the kmap*() functions are guaranteed to provide
consecutive virtual addresses, and I doubt free_bootmem() (called from
efi_free_boot_services()) understands kmap'd addresses.
Maybe we should hot-add the EFI_BOOT_SERVICES_* regions once we've
finished with them and only then mark them as RAM?
x86 folks? Halp?
> On systems that do not have a BGRT object, there's no reason to map this
> memory in efi_enter_virtual_mode. This patch searches for the BGRT
> object and if found - will continue to map the boot services memory,
> else only memory with attribute EFI_RUNTIME_MEMORY will be mapped.
Like I said above, it just so happens on your machine that a BGRT object
occupies that chunk of memory, but this might not be the case on every
EFI i386 machine. You may have other useful things in that region that
you want to be mapped. It's also entirely possible that someone with the
same memory map layout as you _will_ want the bgrt image mapped. This
code needs fixing, instead of just working around the problem.
> 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
> ---
> arch/x86/platform/efi/efi-bgrt.c | 20 +++-
> arch/x86/platform/efi/efi.c | 12 +---
> include/linux/efi-bgrt.h |2 ++
> 3 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi-bgrt.c
> b/arch/x86/platform/efi/efi-bgrt.c
> index 7145ec6..3655d62 100644
> --- a/arch/x86/platform/efi/efi-bgrt.c
> +++ b/arch/x86/platform/efi/efi-bgrt.c
> @@ -25,19 +25,29 @@ struct bmp_header {
> u32 size;
> } __packed;
>
> -void __init efi_bgrt_init(void)
> +bool __init efi_bgrt_probe(void)
> {
> acpi_status status;
> - void __iomem *image;
> - bool ioremapped = false;
> - struct bmp_header bmp_header;
>
> if (acpi_disabled)
> - return;
> + return false;
>
> + bgrt_tab = NULL;
> status = acpi_get_table("BGRT", 0,
> (struct acpi_table_header **)&bgrt_tab);
> if (ACPI_FAILURE(status))
> + return false;
> +
> + return true;
> +}
> +
> +void __init efi_bgrt_init(void)
> +{
> + void __iomem *image;
> +
