linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] efi: Retain boot service code until after switching to virtual mode
@ 2011-05-19 17:32 Matthew Garrett
  2011-05-19 20:26 ` Yinghai Lu
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Garrett @ 2011-05-19 17:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, hpa, Matthew Garrett

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.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 arch/x86/platform/efi/efi.c    |   51 +++++++++++++++++++++++++++++++++++++++-
 arch/x86/platform/efi/efi_64.c |    5 ++-
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index b30aa26..8237c6e 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -304,6 +304,40 @@ static void __init print_efi_memmap(void)
 }
 #endif  /*  EFI_DEBUG  */
 
+static void __init reserve_efi_boot_services(void)
+{
+	void *p;
+
+	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;
+		unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
+
+		if (md->type != EFI_BOOT_SERVICES_CODE &&
+		    md->type != EFI_BOOT_SERVICES_DATA)
+			continue;
+
+		memblock_x86_reserve_range(start, start + size, "EFI Boot");
+	}
+}
+
+static void __init free_efi_boot_services(void)
+{
+	void *p;
+
+	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;
+		unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
+
+		if (md->type != EFI_BOOT_SERVICES_CODE &&
+		    md->type != EFI_BOOT_SERVICES_DATA)
+			continue;
+
+		memblock_x86_free_range(start, start + size);
+	}
+}
+
 void __init efi_init(void)
 {
 	efi_config_table_t *config_tables;
@@ -441,6 +475,12 @@ void __init efi_init(void)
 		printk(KERN_WARNING
 		  "Kernel-defined memdesc doesn't match the one from EFI!\n");
 
+	/*
+	 * The EFI specification says that boot service code won't be called
+	 * after ExitBootServices(). This is, in fact, a lie.
+	 */
+	reserve_efi_boot_services();
+
 	if (add_efi_memmap)
 		do_add_efi_memmap();
 
@@ -536,7 +576,9 @@ void __init efi_enter_virtual_mode(void)
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
-		if (!(md->attribute & EFI_MEMORY_RUNTIME))
+		if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
+		    md->type != EFI_BOOT_SERVICES_CODE &&
+		    md->type != EFI_BOOT_SERVICES_DATA)
 			continue;
 
 		size = md->num_pages << EFI_PAGE_SHIFT;
@@ -593,6 +635,13 @@ 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
+	 */
+	free_efi_boot_services();
+
+	/*
 	 * Now that EFI is in virtual mode, update the function
 	 * pointers in the runtime service table to the new virtual addresses.
 	 *
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 2649426..ac3aa54 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -49,10 +49,11 @@ static void __init early_code_mapping_set_exec(int executable)
 	if (!(__supported_pte_mask & _PAGE_NX))
 		return;
 
-	/* Make EFI runtime service code area executable */
+	/* Make EFI service code area executable */
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
-		if (md->type == EFI_RUNTIME_SERVICES_CODE)
+		if (md->type == EFI_RUNTIME_SERVICES_CODE ||
+		    md->type == EFI_BOOT_SERVICES_CODE)
 			efi_set_executable(md, executable);
 	}
 }
-- 
1.7.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] efi: Retain boot service code until after switching to virtual mode
  2011-05-19 17:32 [PATCH] efi: Retain boot service code until after switching to virtual mode Matthew Garrett
@ 2011-05-19 20:26 ` Yinghai Lu
  2011-05-19 20:33   ` Matthew Garrett
  0 siblings, 1 reply; 3+ messages in thread
From: Yinghai Lu @ 2011-05-19 20:26 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-kernel, x86, hpa, Benjamin Herrenschmidt, Ingo Molnar

On Thu, May 19, 2011 at 10:32 AM, Matthew Garrett <mjg@redhat.com> 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.
>
> 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.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  arch/x86/platform/efi/efi.c    |   51 +++++++++++++++++++++++++++++++++++++++-
>  arch/x86/platform/efi/efi_64.c |    5 ++-
>  2 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index b30aa26..8237c6e 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -304,6 +304,40 @@ static void __init print_efi_memmap(void)
>  }
>  #endif  /*  EFI_DEBUG  */
>
> +static void __init reserve_efi_boot_services(void)
> +{
> +       void *p;
> +
> +       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;
> +               unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
> +
> +               if (md->type != EFI_BOOT_SERVICES_CODE &&
> +                   md->type != EFI_BOOT_SERVICES_DATA)
> +                       continue;
> +
> +               memblock_x86_reserve_range(start, start + size, "EFI Boot");
> +       }
> +}
> +
> +static void __init free_efi_boot_services(void)
> +{
> +       void *p;
> +
> +       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;
> +               unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
> +
> +               if (md->type != EFI_BOOT_SERVICES_CODE &&
> +                   md->type != EFI_BOOT_SERVICES_DATA)
> +                       continue;
> +
> +               memblock_x86_free_range(start, start + size);
> +       }
> +}
> +
>  void __init efi_init(void)
>  {
>        efi_config_table_t *config_tables;
> @@ -441,6 +475,12 @@ void __init efi_init(void)
>                printk(KERN_WARNING
>                  "Kernel-defined memdesc doesn't match the one from EFI!\n");
>
> +       /*
> +        * The EFI specification says that boot service code won't be called
> +        * after ExitBootServices(). This is, in fact, a lie.
> +        */
> +       reserve_efi_boot_services();
> +
>        if (add_efi_memmap)
>                do_add_efi_memmap();
>

how many entries are listed in memmap?

Did you test this system that have lots of those entries ? like > 128?

because efi_init() is called before memblock_x86_fill(), that means
memblock array for reserved can not be doubled yet. --- no usable
entries in memblock array for ram.

> @@ -536,7 +576,9 @@ void __init efi_enter_virtual_mode(void)
>
>        for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
>                md = p;
> -               if (!(md->attribute & EFI_MEMORY_RUNTIME))
> +               if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> +                   md->type != EFI_BOOT_SERVICES_CODE &&
> +                   md->type != EFI_BOOT_SERVICES_DATA)
>                        continue;
>
>                size = md->num_pages << EFI_PAGE_SHIFT;
> @@ -593,6 +635,13 @@ 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
> +        */
> +       free_efi_boot_services();
> +

No, at that point memblock is not used any more.  after mm_init()/mem_init()

need to use free_bootmem_late() in free_efi_boot_services

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] efi: Retain boot service code until after switching to virtual mode
  2011-05-19 20:26 ` Yinghai Lu
@ 2011-05-19 20:33   ` Matthew Garrett
  0 siblings, 0 replies; 3+ messages in thread
From: Matthew Garrett @ 2011-05-19 20:33 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: linux-kernel, x86, hpa, Benjamin Herrenschmidt, Ingo Molnar

On Thu, May 19, 2011 at 01:26:27PM -0700, Yinghai Lu wrote:

> how many entries are listed in memmap?

So far I've been seeing about 20 boot services entries at most.

> Did you test this system that have lots of those entries ? like > 128?

Do any exist?

> because efi_init() is called before memblock_x86_fill(), that means
> memblock array for reserved can not be doubled yet. --- no usable
> entries in memblock array for ram.

Mm. I guess this can be shifted later.

> > +        * get rid of it all at this point
> > +        */
> > +       free_efi_boot_services();
> > +
> 
> No, at that point memblock is not used any more.  after mm_init()/mem_init()
> 
> need to use free_bootmem_late() in free_efi_boot_services

Ah, ok.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-05-19 20:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-19 17:32 [PATCH] efi: Retain boot service code until after switching to virtual mode Matthew Garrett
2011-05-19 20:26 ` Yinghai Lu
2011-05-19 20:33   ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).