Fix ACPI BGRT support for images located in EFI boot services memory
cover Fix ACPI BGRT support for images located in EFI boot services memory
The ACPI BGRT lets the OS access the BIOS logo image and its position on the screen at boot time, allowing it to maintain that image on the screen until ready to display something else, making boot more seamless. This series fixes support for accessing the boot logo image via the BGRT when the BIOS stores it in EFI boot services memory, as recommended by the ACPI 5.0 spec. Linux needs to copy the image out of boot services memory before reclaiming boot services memory.
The first patch refactors EFI initialization to defer freeing boot services memory until later in the boot process, after we have ACPI available. The second patch adds a helper function to look up existing EFI boot services mappings, to avoid re-mapping them. The third patch moves BGRT initialization to before the reclamation of boot services memory, copies the logo at that point, and reworks the existing BGRT driver to use that existing copy.
Josh Triplett (3):
efi: Defer freeing boot services memory until after ACPI init
efi: Add a function to look up existing IO memory mappings
efi: Fix the ACPI BGRT driver for images located in EFI boot services memory
arch/x86/platform/efi/Makefile | 1 + arch/x86/platform/efi/efi-bgrt.c | 76 ++++++++++++++++++++++++++++++++++++++ arch/x86/platform/efi/efi.c | 65 +++++++++++++++++++++++++------- drivers/acpi/Kconfig | 4 +- drivers/acpi/bgrt.c | 76 +++++--------------------------------- include/linux/efi-bgrt.h | 21 +++++++++++ include/linux/efi.h | 3 ++ init/main.c | 7 ++++ 8 files changed, 171 insertions(+), 82 deletions(-) create mode 100644 arch/x86/platform/efi/efi-bgrt.c create mode 100644 include/linux/efi-bgrt.h
Signature · 1 line
1.7.10.4
PATCH 1/3 efi: Defer freeing boot services memory until after ACPI init
Some new ACPI 5.0 tables reference resources stored in boot services memory, so keep that memory around until we have ACPI and can extract data from it.
--- arch/x86/platform/efi/efi.c | 31 ++++++++++++++++++------------- include/linux/efi.h | 1 + init/main.c | 5 +++++ 3 files changed, 24 insertions(+), 13 deletions(-)
arch/x86/platform/efi/efi.c +18 -13
@@ -419,10 +419,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_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;
@@ -438,6 +449,8 @@ static void __init efi_free_boot_services(void)free_bootmem_late (start, size); } + +efi_unmap_memmap (); }static int __init efi_systab_init (void *phys)
@@ -787,8 +800,10 @@ void __init efi_enter_virtual_mode(void)* non-native EFI */ -if (!efi_native) - goto out; +if (!efi_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) {
@@ -878,13 +893,6 @@ void __init efi_enter_virtual_mode(void) }/* -* Thankfully, it does seem that no runtime services other than -* SetVirtualAddressMap() will touch boot services code, so we can -* get rid of it all at this point -*/ -efi_free_boot_services (); - -/* * Now that EFI is in virtual mode, update the function * pointers in the runtime service table to the new virtual addresses. *
@@ -906,9 +914,6 @@ void __init efi_enter_virtual_mode(void)if (__supported_pte_mask & _PAGE_NX)runtime_code_page_mkexec (); -out: -early_iounmap (memmap.map, memmap.nr_map * memmap.desc_size); - memmap.map =NULL ;kfree (new_memmap); }
include/linux/efi.h +1 -0
@@ -496,6 +496,7 @@ extern void efi_map_pal_code (void);extern void efi_memmap_walk (efi_freemem_callback_t callback,void *arg);extern void efi_gettimeofday (struct timespec *ts);extern void efi_enter_virtual_mode (void );/* switch EFI to virtual mode, if possible */ +extern void efi_free_boot_services (void );extern u64 efi_get_iobase (void );extern u32 efi_mem_type (unsigned long phys_addr);extern u64 efi_mem_attributes (unsigned long phys_addr);
init/main.c +5 -0
@@ -631,6 +631,11 @@ asmlinkage void __init start_kernel(void)acpi_early_init ();/* before LAPIC and SMP init */ sfi_init_late (); +#ifdef CONFIG_X86 +if (efi_enabled) +efi_free_boot_services (); +#endif +ftrace_init ();/* Do the rest non-__init'ed, we're now alive */
Signature · 1 line
1.7.10.4
PATCH 2/3 efi: Add a function to look up existing IO memory mappings
In reply to Josh TriplettThe EFI initialization creates virtual mappings for EFI boot services memory, so if a driver wants to access EFI boot services memory, it cannot call ioremap itself; doing so will trip the WARN about mapping RAM twice. Thus, a driver accessing EFI boot services memory must do so via the existing mapping already created during EFI intiialization. Since the EFI code already maintains a memory map for that memory, add a function efi_lookup_mapped_addr to look up mappings in that memory map.
--- arch/x86/platform/efi/efi.c | 28 ++++++++++++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 29 insertions(+)
arch/x86/platform/efi/efi.c +28 -0
@@ -777,6 +777,34 @@ static void __init runtime_code_page_mkexec(void) }/* +* We can't ioremap data in EFI boot services RAM, because we've already mapped +* it as RAM. So, look it up in the existing EFI memory map instead. Only +* callable after efi_enter_virtual_mode and before efi_free_boot_services. +*/ +void __iomem *efi_lookup_mapped_addr (u64 phys_addr) +{ +void *p; +if (WARN_ON (!memmap.map)) +return NULL ; +for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) { +efi_memory_desc_t *md = p; +u64 size = md->num_pages <<EFI_PAGE_SHIFT ; +u64 end = md->phys_addr + size; +if (!(md->attribute &EFI_MEMORY_RUNTIME ) && + md->type !=EFI_BOOT_SERVICES_CODE && + md->type !=EFI_BOOT_SERVICES_DATA ) +continue ; +if (!md->virt_addr) +continue ; +if (phys_addr >= md->phys_addr && phys_addr < end) { + phys_addr += md->virt_addr - md->phys_addr; +return (__force void __iomem *)phys_addr; + } + } +return NULL ; +} + +/* * This function will switch the EFI runtime services to virtual mode. * Essentially, look through the EFI memmap and map every region that * has the runtime attribute bit set in its memory descriptor and update
include/linux/efi.h +1 -0
@@ -496,6 +496,7 @@ extern void efi_map_pal_code (void);extern void efi_memmap_walk (efi_freemem_callback_t callback,void *arg);extern void efi_gettimeofday (struct timespec *ts);extern void efi_enter_virtual_mode (void );/* switch EFI to virtual mode, if possible */ +extern void __iomem *efi_lookup_mapped_addr (u64 phys_addr);extern void efi_free_boot_services (void );extern u64 efi_get_iobase (void );extern u32 efi_mem_type (unsigned long phys_addr);
Signature · 1 line
1.7.10.4
PATCH 3/3 efi: Fix the ACPI BGRT driver for images located in EFI boot services memory
In reply to Josh TriplettThe ACPI BGRT driver accesses the BIOS logo image when it initializes. However, ACPI 5.0 (which introduces the BGRT) recommends putting the logo image in EFI boot services memory, so that the OS can reclaim that memory. Production systems follow this recommendation, breaking the ACPI BGRT driver.
Move the bulk of the BGRT code to run during a new EFI late initialization phase, which occurs after switching EFI to virtual mode, and after initializing ACPI, but before freeing boot services memory. Copy the BIOS logo image to kernel memory at that point, and make it accessible to the BGRT driver. Rework the existing ACPI BGRT driver to act as a simple wrapper exposing that image (and the properties from the BGRT) via sysfs.
--- arch/x86/platform/efi/Makefile | 1 + arch/x86/platform/efi/efi-bgrt.c | 76 ++++++++++++++++++++++++++++++++++++++ arch/x86/platform/efi/efi.c | 6 +++ drivers/acpi/Kconfig | 4 +- drivers/acpi/bgrt.c | 76 +++++--------------------------------- include/linux/efi-bgrt.h | 21 +++++++++++ include/linux/efi.h | 1 + init/main.c | 4 +- 8 files changed, 119 insertions(+), 70 deletions(-) create mode 100644 arch/x86/platform/efi/efi-bgrt.c create mode 100644 include/linux/efi-bgrt.h
arch/x86/platform/efi/Makefile +1 -0
@@ -1 +1,2 @@ obj-$(CONFIG_EFI) += efi.o efi_$(BITS).o efi_stub_$(BITS).o +obj-$(CONFIG_ACPI_BGRT) += efi-bgrt.o
arch/x86/platform/efi/efi-bgrt.c new file +76 -0
76 lines not on this page · raw message
arch/x86/platform/efi/efi.c +6 -0
@@ -31,6 +31,7 @@#include <linux/kernel.h> #include <linux/init.h> #include <linux/efi.h> +#include <linux/efi-bgrt.h> #include <linux/export.h> #include <linux/bootmem.h> #include <linux/memblock.h>
@@ -745,6 +746,11 @@ void __init efi_init(void)#endif } +void __init efi_late_init (void ) +{ +efi_bgrt_init (); +} +void __init efi_set_executable (efi_memory_desc_t *md,bool executable) {u64 addr, npages;
drivers/acpi/Kconfig +2 -2
@@ -385,8 +385,8 @@ config ACPI_CUSTOM_METHOD to override that restriction). config ACPI_BGRT - tristate "Boottime Graphics Resource Table support" - default n + bool "Boottime Graphics Resource Table support" + depends on EFI help This driver adds support for exposing the ACPI Boottime Graphics Resource Table, which allows the operating system to obtain
drivers/acpi/bgrt.c +9 -67
119 lines not on this page · raw message
include/linux/efi-bgrt.h new file +21 -0
@@ -0,0 +1,21 @@ +#ifndef _LINUX_EFI_BGRT_H +#define _LINUX_EFI_BGRT_H + +#ifdef CONFIG_ACPI_BGRT + +#include <linux/acpi.h> + +void efi_bgrt_init (void ); + +/* The BGRT data itself; only valid if bgrt_image != NULL. */ +extern void *bgrt_image; +extern size_t bgrt_image_size; +extern struct acpi_table_bgrt *bgrt_tab; + +#else /* !CONFIG_ACPI_BGRT */ + +static inline void efi_bgrt_init (void ) {} + +#endif /* !CONFIG_ACPI_BGRT */ + +#endif /* _LINUX_EFI_BGRT_H */
include/linux/efi.h +1 -0
@@ -496,6 +496,7 @@ extern void efi_map_pal_code (void);extern void efi_memmap_walk (efi_freemem_callback_t callback,void *arg);extern void efi_gettimeofday (struct timespec *ts);extern void efi_enter_virtual_mode (void );/* switch EFI to virtual mode, if possible */ +extern void efi_late_init (void );extern void __iomem *efi_lookup_mapped_addr (u64 phys_addr);extern void efi_free_boot_services (void );extern u64 efi_get_iobase (void );
init/main.c +3 -1
@@ -632,8 +632,10 @@ asmlinkage void __init start_kernel(void)sfi_init_late ();#ifdef CONFIG_X86 -if (efi_enabled) +if (efi_enabled) { +efi_late_init ();efi_free_boot_services (); + }#endif ftrace_init ();
Signature · 1 line
1.7.10.4
Quoted 8 lines · Josh Triplett
The ACPI BGRT lets the OS access the BIOS logo image and its position on the screen at boot time, allowing it to maintain that image on the screen until ready to display something else, making boot more seamless. This series fixes support for accessing the boot logo image via the BGRT when the BIOS stores it in EFI boot services memory, as recommended by the ACPI 5.0 spec. Linux needs to copy the image out of boot services memory before reclaiming boot services memory.
Quoted 6 lines
The first patch refactors EFI initialization to defer freeing boot services memory until later in the boot process, after we have ACPI available. The second patch adds a helper function to look up existing EFI boot services mappings, to avoid re-mapping them. The third patch moves BGRT initialization to before the reclamation of boot services memory, copies the logo at that point, and reworks the existing BGRT driver to use that existing copy.
Since we always end up doing a copy anyway, is there no way we could just copy the boot logo *without* deferring freeing the boot services code, e.g. move the copy before we do SetVirtualAddressMap()? I wouldn't be surprised if some implementations got really cranky if we accessed boot services data after we installed a new virtual memory map.
Besides, if we can avoid moving the efi_free_boot_services() call we can avoid littering init/main.c with more #ifdef CONFIG_X86 blocks.
Quoted 16 lines · Matt Fleming
Quoted 8 lines · Josh Triplett
The ACPI BGRT lets the OS access the BIOS logo image and its position on the screen at boot time, allowing it to maintain that image on the screen until ready to display something else, making boot more seamless. This series fixes support for accessing the boot logo image via the BGRT when the BIOS stores it in EFI boot services memory, as recommended by the ACPI 5.0 spec. Linux needs to copy the image out of boot services memory before reclaiming boot services memory.
Quoted 6 lines
The first patch refactors EFI initialization to defer freeing boot services memory until later in the boot process, after we have ACPI available. The second patch adds a helper function to look up existing EFI boot services mappings, to avoid re-mapping them. The third patch moves BGRT initialization to before the reclamation of boot services memory, copies the logo at that point, and reworks the existing BGRT driver to use that existing copy.
Quoted 3 lines
Since we always end up doing a copy anyway, is there no way we could just copy the boot logo *without* deferring freeing the boot services code, e.g. move the copy before we do
SetVirtualAddressMap()?
Unfortunately not. We need enough of ACPI available to go read the BGRT to know what to copy, so we need to defer freeing boot services code until after we initialize ACPI (and thus everything ACPI needs, which includes EFI since ACPI looks for root tables there).
Quoted 3 lines
I wouldn't be surprised if some implementations got really cranky if we accessed boot services data after we installed a new virtual memory map.
Note that I've carefully accessed the boot services data *through* the new virtual memory map, which should work fine.
Quoted 2 lines
Besides, if we can avoid moving the
efi_free_boot_services()call we can avoid litteringinit/main.cwith more #ifdefCONFIG_X86blocks.
Those seem easy enough to convert into appropriate always-available stubs, if you'd like. And I could move efi_free_boot_services() inside efi_late_init(), too, keeping it an internal implementation detail of EFI initialization. Would that help?
- Josh Triplett
H. Peter Anvin
reviewerQuoted 6 lines · Josh Triplett
Unfortunately not. We need enough of ACPI available to go read the BGRT to know what to copy, so we need to defer freeing boot services code until after we initialize ACPI (and thus everything ACPI needs, which includes EFI since ACPI looks for root tables there).
Quoted 7 lines
Quoted 3 lines
I wouldn't be surprised if some implementations got really cranky if we accessed boot services data after we installed a new virtual memory map.
Note that I've carefully accessed the boot services data *through* the new virtual memory map, which should work fine.
There are some platforms which have bugs in this area, so there are other reasons to defer freeing up boot memory until as late in the boot process as we can possibly get away with.
free_initmem() is presuambly the place that makes most sense. This is EFI-specific but not x86-specific, let's not commingle those concepts, please...
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Quoted 15 lines · H. Peter Anvin
Quoted 6 lines · Josh Triplett
Unfortunately not. We need enough of ACPI available to go read the BGRT to know what to copy, so we need to defer freeing boot services code until after we initialize ACPI (and thus everything ACPI needs, which includes EFI since ACPI looks for root tables there).
Quoted 7 lines
Quoted 3 lines
I wouldn't be surprised if some implementations got really cranky if we accessed boot services data after we installed a new virtual memory map.
Note that I've carefully accessed the boot services data *through* the new virtual memory map, which should work fine.
Quoted 5 lines
There are some platforms which have bugs in this area, so there are other reasons to defer freeing up boot memory until as late in the boot process as we can possibly get away with.
free_initmem()is presuambly the place that makes most sense.
You're suggesting a call from free_initmem() to efi_free_boot_services()? Or, from init_post() right before the call to free_initmem()?
Quoted 3 lines
This
is EFI-specific but not x86-specific, let's not commingle those concepts, please...
init/main.c already calls the x86-specific efi_enter_virtual_mode (defined in arch/x86/platform/efi/efi.c), and I split the call to the x86-specific efi_free_boot_services out of that. Neither of those functions exists on non-x86 platforms, and thus I mirrored the #ifdef currently wrapped around efi_enter_virtual_mode for the new call to efi_free_boot_services. While it might make sense for that code to exist on non-x86 EFI platforms, it currently doesn't. At best, I could add static inline stubs to linux/efi.h for those functions to avoid the ifdefs, but as far as I can tell the same issue applies to quite a few more functions in efi.h.
Would you like me to add the static inline stubs for the couple of functions called from init/main.c, or leave the #ifdefs?
- Josh Triplett
H. Peter Anvin
reviewerQuoted 11 lines · Josh Triplett
Quoted 6 lines
There are some platforms which have bugs in this area, so there are other reasons to defer freeing up boot memory until as late in the boot process as we can possibly get away with.
free_initmem()is presuambly the place that makes most sense.You're suggesting a call from
free_initmem()toefi_free_boot_services()? Or, frominit_post()right before the call tofree_initmem()?
free_initmem() is arch-specific, so probably the latter.
Quoted 15 lines
Quoted 3 lines
This
is EFI-specific but not x86-specific, let's not commingle those concepts, please...
init/main.calready calls the x86-specificefi_enter_virtual_mode(defined inarch/x86/platform/efi/efi.c), and I split the call to the x86-specificefi_free_boot_servicesout of that. Neither of those functions exists on non-x86 platforms, and thus I mirrored the #ifdef currently wrapped aroundefi_enter_virtual_modefor the new call toefi_free_boot_services. While it might make sense for that code to exist on non-x86 EFI platforms, it currently doesn't. At best, I could add static inline stubs tolinux/efi.hfor those functions to avoid the ifdefs, but as far as I can tell the same issue applies to quite a few more functions inefi.h.
Quoted 3 lines
Would you like me to add the static inline stubs for the couple of functions called from
init/main.c, or leave the #ifdefs?
I think that would really help clean things up.
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Quoted 6 lines · H. Peter Anvin
Quoted 5 lines · Josh Triplett
Quoted 5 lines
There are some platforms which have bugs in this area, so there are other reasons to defer freeing up boot memory until as late in the boot process as we can possibly get away with.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
free_initmem()is presuambly the place that makes most sense.You're suggesting a call from
free_initmem()toefi_free_boot_services()? Or, frominit_post()right before the call tofree_initmem()?
free_initmem()is arch-specific, so probably the latter.
OK, will do.
Quoted 15 lines
Quoted 15 lines
Quoted 3 lines
This
is EFI-specific but not x86-specific, let's not commingle those concepts, please...
init/main.calready calls the x86-specificefi_enter_virtual_mode(defined inarch/x86/platform/efi/efi.c), and I split the call to the x86-specificefi_free_boot_servicesout of that. Neither of those functions exists on non-x86 platforms, and thus I mirrored the #ifdef currently wrapped aroundefi_enter_virtual_modefor the new call toefi_free_boot_services. While it might make sense for that code to exist on non-x86 EFI platforms, it currently doesn't. At best, I could add static inline stubs tolinux/efi.hfor those functions to avoid the ifdefs, but as far as I can tell the same issue applies to quite a few more functions inefi.h.
Quoted 5 lines
Quoted 3 lines
Would you like me to add the static inline stubs for the couple of functions called from
init/main.c, or leave the #ifdefs?I think that would really help clean things up.
Fair enough. I'll send v2 shortly.
- Josh Triplett
Quoted 17 lines · Josh Triplett
Quoted 16 lines · Matt Fleming
Quoted 14 lines · Josh Triplett
The ACPI BGRT lets the OS access the BIOS logo image and its position on the screen at boot time, allowing it to maintain that image on the screen until ready to display something else, making boot more seamless. This series fixes support for accessing the boot logo image via the BGRT when the BIOS stores it in EFI boot services memory, as recommended by the ACPI 5.0 spec. Linux needs to copy the image out of boot services memory before reclaiming boot services memory.
The first patch refactors EFI initialization to defer freeing boot services memory until later in the boot process, after we have ACPI available. The second patch adds a helper function to look up existing EFI boot services mappings, to avoid re-mapping them. The third patch moves BGRT initialization to before the reclamation of boot services memory, copies the logo at that point, and reworks the existing BGRT driver to use that existing copy.
Quoted 8 lines
Quoted 3 lines
Since we always end up doing a copy anyway, is there no way we could just copy the boot logo *without* deferring freeing the boot services code, e.g. move the copy before we do
SetVirtualAddressMap()?Unfortunately not. We need enough of ACPI available to go read the BGRT to know what to copy, so we need to defer freeing boot services code until after we initialize ACPI (and thus everything ACPI needs, which includes EFI since ACPI looks for root tables there).
Ah, right. It was also pointed out to me offline that some drivers have been known to access boot services data even after SetVirtualAddressMap(), so this deferring shouldn't be a problem.
Quoted 7 lines
Quoted 3 lines
I wouldn't be surprised if some implementations got really cranky if we accessed boot services data after we installed a new virtual memory map.
Note that I've carefully accessed the boot services data *through* the new virtual memory map, which should work fine.
Quoted 7 lines
Quoted 2 lines
Besides, if we can avoid moving the
efi_free_boot_services()call we can avoid litteringinit/main.cwith more #ifdefCONFIG_X86blocks.Those seem easy enough to convert into appropriate always-available stubs, if you'd like. And I could move
efi_free_boot_services()insideefi_late_init(), too, keeping it an internal implementation detail of EFI initialization. Would that help?
Yeah, that would seem like a good solution.