efi/arm/arm64: Allow SetVirtualAddressMap() to be omitted
diff mbox series

Message ID 1550674294-10473-2-git-send-email-paolo.pisati@canonical.com
State New
Headers show
Series
  • efi/arm/arm64: Allow SetVirtualAddressMap() to be omitted
Related show

Commit Message

Paolo Pisati Feb. 20, 2019, 2:51 p.m. UTC
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The UEFI spec revision 2.7 errata A section 8.4 has the following to
say about the virtual memory runtime services:

  "This section contains function definitions for the virtual memory
  support that may be optionally used by an operating system at runtime.
  If an operating system chooses to make EFI runtime service calls in a
  virtual addressing mode instead of the flat physical mode, then the
  operating system must use the services in this section to switch the
  EFI runtime services from flat physical addressing to virtual
  addressing."

So it is pretty clear that calling SetVirtualAddressMap() is entirely
optional, and so there is no point in doing so unless it achieves
anything useful for us.

This is not the case for 64-bit ARM. The identity mapping used by the
firmware is arbitrarily converted into another permutation of userland
addresses (i.e., bits [63:48] cleared), and the runtime code could easily
deal with the original layout in exactly the same way as it deals with
the converted layout. However, due to constraints related to page size
differences if the OS is not running with 4k pages, and related to
systems that may expose the individual sections of PE/COFF runtime
modules as different memory regions, creating the virtual layout is a
bit fiddly, and requires us to sort the memory map and reason about
adjacent regions with identical memory types etc etc.

So the obvious fix is to stop calling SetVirtualAddressMap() altogether
on arm64 systems. However, to avoid surprises, which are notoriously
hard to diagnose when it comes to OS<->firmware interactions, let's
start by making it an opt-out feature, and implement support for the
'efi=novamap' kernel command line parameter on ARM and arm64 systems.

( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
  used, given that the physical memory map and the kernel virtual address
  map are not guaranteed to be non-overlapping like on arm64. However,
  having support for efi=novamap,noruntime on 32-bit ARM, combined with
  the recently proposed support for earlycon=efifb, is likely to be useful
  to diagnose boot issues on such systems if they have no accessible serial
  port. )

Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Alexander Graf <agraf@suse.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Jones <pjones@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
(cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 drivers/firmware/efi/libstub/arm-stub.c        |  5 +++++
 drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++++++++++
 drivers/firmware/efi/libstub/efistub.h         |  1 +
 drivers/firmware/efi/libstub/fdt.c             |  3 +++
 4 files changed, 19 insertions(+)

Comments

Colin King Feb. 20, 2019, 2:57 p.m. UTC | #1
On 20/02/2019 14:51, Paolo Pisati wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The UEFI spec revision 2.7 errata A section 8.4 has the following to
> say about the virtual memory runtime services:
> 
>   "This section contains function definitions for the virtual memory
>   support that may be optionally used by an operating system at runtime.
>   If an operating system chooses to make EFI runtime service calls in a
>   virtual addressing mode instead of the flat physical mode, then the
>   operating system must use the services in this section to switch the
>   EFI runtime services from flat physical addressing to virtual
>   addressing."
> 
> So it is pretty clear that calling SetVirtualAddressMap() is entirely
> optional, and so there is no point in doing so unless it achieves
> anything useful for us.
> 
> This is not the case for 64-bit ARM. The identity mapping used by the
> firmware is arbitrarily converted into another permutation of userland
> addresses (i.e., bits [63:48] cleared), and the runtime code could easily
> deal with the original layout in exactly the same way as it deals with
> the converted layout. However, due to constraints related to page size
> differences if the OS is not running with 4k pages, and related to
> systems that may expose the individual sections of PE/COFF runtime
> modules as different memory regions, creating the virtual layout is a
> bit fiddly, and requires us to sort the memory map and reason about
> adjacent regions with identical memory types etc etc.
> 
> So the obvious fix is to stop calling SetVirtualAddressMap() altogether
> on arm64 systems. However, to avoid surprises, which are notoriously
> hard to diagnose when it comes to OS<->firmware interactions, let's
> start by making it an opt-out feature, and implement support for the
> 'efi=novamap' kernel command line parameter on ARM and arm64 systems.
> 
> ( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
>   used, given that the physical memory map and the kernel virtual address
>   map are not guaranteed to be non-overlapping like on arm64. However,
>   having support for efi=novamap,noruntime on 32-bit ARM, combined with
>   the recently proposed support for earlycon=efifb, is likely to be useful
>   to diagnose boot issues on such systems if they have no accessible serial
>   port. )
> 
> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
> Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-efi@vger.kernel.org
> Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
> ---
>  drivers/firmware/efi/libstub/arm-stub.c        |  5 +++++
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++++++++++
>  drivers/firmware/efi/libstub/efistub.h         |  1 +
>  drivers/firmware/efi/libstub/fdt.c             |  3 +++
>  4 files changed, 19 insertions(+)
> 
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 01a9d78..3b1e1dc 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -364,6 +364,11 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
>  		paddr = in->phys_addr;
>  		size = in->num_pages * EFI_PAGE_SIZE;
>  
> +		if (novamap()) {
> +			in->virt_addr = in->phys_addr;
> +			continue;
> +		}
> +
>  		/*
>  		 * Make the mapping compatible with 64k pages: this allows
>  		 * a 4k page size kernel to kexec a 64k page size kernel and
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 50a9cab..39f87e6 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -34,6 +34,7 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
>  
>  static int __section(.data) __nokaslr;
>  static int __section(.data) __quiet;
> +static int __section(.data) __novamap;
>  
>  int __pure nokaslr(void)
>  {
> @@ -43,6 +44,10 @@ int __pure is_quiet(void)
>  {
>  	return __quiet;
>  }
> +int __pure novamap(void)
> +{
> +	return __novamap;
> +}
>  
>  #define EFI_MMAP_NR_SLACK_SLOTS	8
>  
> @@ -454,6 +459,11 @@ efi_status_t efi_parse_options(char const *cmdline)
>  			__chunk_size = -1UL;
>  		}
>  
> +		if (!strncmp(str, "novamap", 7)) {
> +			str += strlen("novamap");
> +			__novamap = 1;
> +		}
> +
>  		/* Group words together, delimited by "," */
>  		while (*str && *str != ' ' && *str != ',')
>  			str++;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index f59564b..2adde22 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -27,6 +27,7 @@
>  
>  extern int __pure nokaslr(void);
>  extern int __pure is_quiet(void);
> +extern int __pure novamap(void);
>  
>  #define pr_efi(sys_table, msg)		do {				\
>  	if (!is_quiet()) efi_printk(sys_table, "EFI stub: "msg);	\
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 8830fa6..94e7115 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -323,6 +323,9 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
>  	if (status == EFI_SUCCESS) {
>  		efi_set_virtual_address_map_t *svam;
>  
> +		if (novamap())
> +			return EFI_SUCCESS;
> +
>  		/* Install the new virtual address map */
>  		svam = sys_table->runtime->set_virtual_address_map;
>  		status = svam(runtime_entry_count * desc_size, desc_size,
> 

Missing buglink in the commit.

Upstream patch, has had regression testing upstream and is an opt-out
option so regression potential is low.

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader Feb. 22, 2019, 10:06 a.m. UTC | #2
On 20.02.19 15:51, Paolo Pisati wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

BugLink: https://bugs.launchpad.net/bugs/1814982
> 
> The UEFI spec revision 2.7 errata A section 8.4 has the following to
> say about the virtual memory runtime services:
> 
>   "This section contains function definitions for the virtual memory
>   support that may be optionally used by an operating system at runtime.
>   If an operating system chooses to make EFI runtime service calls in a
>   virtual addressing mode instead of the flat physical mode, then the
>   operating system must use the services in this section to switch the
>   EFI runtime services from flat physical addressing to virtual
>   addressing."
> 
> So it is pretty clear that calling SetVirtualAddressMap() is entirely
> optional, and so there is no point in doing so unless it achieves
> anything useful for us.
> 
> This is not the case for 64-bit ARM. The identity mapping used by the
> firmware is arbitrarily converted into another permutation of userland
> addresses (i.e., bits [63:48] cleared), and the runtime code could easily
> deal with the original layout in exactly the same way as it deals with
> the converted layout. However, due to constraints related to page size
> differences if the OS is not running with 4k pages, and related to
> systems that may expose the individual sections of PE/COFF runtime
> modules as different memory regions, creating the virtual layout is a
> bit fiddly, and requires us to sort the memory map and reason about
> adjacent regions with identical memory types etc etc.
> 
> So the obvious fix is to stop calling SetVirtualAddressMap() altogether
> on arm64 systems. However, to avoid surprises, which are notoriously
> hard to diagnose when it comes to OS<->firmware interactions, let's
> start by making it an opt-out feature, and implement support for the
> 'efi=novamap' kernel command line parameter on ARM and arm64 systems.
> 
> ( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
>   used, given that the physical memory map and the kernel virtual address
>   map are not guaranteed to be non-overlapping like on arm64. However,
>   having support for efi=novamap,noruntime on 32-bit ARM, combined with
>   the recently proposed support for earlycon=efifb, is likely to be useful
>   to diagnose boot issues on such systems if they have no accessible serial
>   port. )
> 
> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
> Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-efi@vger.kernel.org
> Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

With BugLink added as opt-out and testable.

>  drivers/firmware/efi/libstub/arm-stub.c        |  5 +++++
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++++++++++
>  drivers/firmware/efi/libstub/efistub.h         |  1 +
>  drivers/firmware/efi/libstub/fdt.c             |  3 +++
>  4 files changed, 19 insertions(+)
> 
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 01a9d78..3b1e1dc 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -364,6 +364,11 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
>  		paddr = in->phys_addr;
>  		size = in->num_pages * EFI_PAGE_SIZE;
>  
> +		if (novamap()) {
> +			in->virt_addr = in->phys_addr;
> +			continue;
> +		}
> +
>  		/*
>  		 * Make the mapping compatible with 64k pages: this allows
>  		 * a 4k page size kernel to kexec a 64k page size kernel and
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 50a9cab..39f87e6 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -34,6 +34,7 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
>  
>  static int __section(.data) __nokaslr;
>  static int __section(.data) __quiet;
> +static int __section(.data) __novamap;
>  
>  int __pure nokaslr(void)
>  {
> @@ -43,6 +44,10 @@ int __pure is_quiet(void)
>  {
>  	return __quiet;
>  }
> +int __pure novamap(void)
> +{
> +	return __novamap;
> +}
>  
>  #define EFI_MMAP_NR_SLACK_SLOTS	8
>  
> @@ -454,6 +459,11 @@ efi_status_t efi_parse_options(char const *cmdline)
>  			__chunk_size = -1UL;
>  		}
>  
> +		if (!strncmp(str, "novamap", 7)) {
> +			str += strlen("novamap");
> +			__novamap = 1;
> +		}
> +
>  		/* Group words together, delimited by "," */
>  		while (*str && *str != ' ' && *str != ',')
>  			str++;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index f59564b..2adde22 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -27,6 +27,7 @@
>  
>  extern int __pure nokaslr(void);
>  extern int __pure is_quiet(void);
> +extern int __pure novamap(void);
>  
>  #define pr_efi(sys_table, msg)		do {				\
>  	if (!is_quiet()) efi_printk(sys_table, "EFI stub: "msg);	\
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 8830fa6..94e7115 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -323,6 +323,9 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
>  	if (status == EFI_SUCCESS) {
>  		efi_set_virtual_address_map_t *svam;
>  
> +		if (novamap())
> +			return EFI_SUCCESS;
> +
>  		/* Install the new virtual address map */
>  		svam = sys_table->runtime->set_virtual_address_map;
>  		status = svam(runtime_entry_count * desc_size, desc_size,
>
Seth Forshee Feb. 22, 2019, 10:09 a.m. UTC | #3
On Wed, Feb 20, 2019 at 03:51:34PM +0100, Paolo Pisati wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The UEFI spec revision 2.7 errata A section 8.4 has the following to
> say about the virtual memory runtime services:
> 
>   "This section contains function definitions for the virtual memory
>   support that may be optionally used by an operating system at runtime.
>   If an operating system chooses to make EFI runtime service calls in a
>   virtual addressing mode instead of the flat physical mode, then the
>   operating system must use the services in this section to switch the
>   EFI runtime services from flat physical addressing to virtual
>   addressing."
> 
> So it is pretty clear that calling SetVirtualAddressMap() is entirely
> optional, and so there is no point in doing so unless it achieves
> anything useful for us.
> 
> This is not the case for 64-bit ARM. The identity mapping used by the
> firmware is arbitrarily converted into another permutation of userland
> addresses (i.e., bits [63:48] cleared), and the runtime code could easily
> deal with the original layout in exactly the same way as it deals with
> the converted layout. However, due to constraints related to page size
> differences if the OS is not running with 4k pages, and related to
> systems that may expose the individual sections of PE/COFF runtime
> modules as different memory regions, creating the virtual layout is a
> bit fiddly, and requires us to sort the memory map and reason about
> adjacent regions with identical memory types etc etc.
> 
> So the obvious fix is to stop calling SetVirtualAddressMap() altogether
> on arm64 systems. However, to avoid surprises, which are notoriously
> hard to diagnose when it comes to OS<->firmware interactions, let's
> start by making it an opt-out feature, and implement support for the
> 'efi=novamap' kernel command line parameter on ARM and arm64 systems.
> 
> ( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
>   used, given that the physical memory map and the kernel virtual address
>   map are not guaranteed to be non-overlapping like on arm64. However,
>   having support for efi=novamap,noruntime on 32-bit ARM, combined with
>   the recently proposed support for earlycon=efifb, is likely to be useful
>   to diagnose boot issues on such systems if they have no accessible serial
>   port. )
> 
> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
> Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-efi@vger.kernel.org
> Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)

This commit doesn't appear to be in Linus' tree. Where did it come from?
Paolo Pisati Feb. 22, 2019, 10:29 a.m. UTC | #4
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=4e46c2a956215482418d7b315749fb1b6c6bc224

On Fri, Feb 22, 2019 at 11:09 AM Seth Forshee
<seth.forshee@canonical.com> wrote:
>
> On Wed, Feb 20, 2019 at 03:51:34PM +0100, Paolo Pisati wrote:
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > The UEFI spec revision 2.7 errata A section 8.4 has the following to
> > say about the virtual memory runtime services:
> >
> >   "This section contains function definitions for the virtual memory
> >   support that may be optionally used by an operating system at runtime.
> >   If an operating system chooses to make EFI runtime service calls in a
> >   virtual addressing mode instead of the flat physical mode, then the
> >   operating system must use the services in this section to switch the
> >   EFI runtime services from flat physical addressing to virtual
> >   addressing."
> >
> > So it is pretty clear that calling SetVirtualAddressMap() is entirely
> > optional, and so there is no point in doing so unless it achieves
> > anything useful for us.
> >
> > This is not the case for 64-bit ARM. The identity mapping used by the
> > firmware is arbitrarily converted into another permutation of userland
> > addresses (i.e., bits [63:48] cleared), and the runtime code could easily
> > deal with the original layout in exactly the same way as it deals with
> > the converted layout. However, due to constraints related to page size
> > differences if the OS is not running with 4k pages, and related to
> > systems that may expose the individual sections of PE/COFF runtime
> > modules as different memory regions, creating the virtual layout is a
> > bit fiddly, and requires us to sort the memory map and reason about
> > adjacent regions with identical memory types etc etc.
> >
> > So the obvious fix is to stop calling SetVirtualAddressMap() altogether
> > on arm64 systems. However, to avoid surprises, which are notoriously
> > hard to diagnose when it comes to OS<->firmware interactions, let's
> > start by making it an opt-out feature, and implement support for the
> > 'efi=novamap' kernel command line parameter on ARM and arm64 systems.
> >
> > ( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
> >   used, given that the physical memory map and the kernel virtual address
> >   map are not guaranteed to be non-overlapping like on arm64. However,
> >   having support for efi=novamap,noruntime on 32-bit ARM, combined with
> >   the recently proposed support for earlycon=efifb, is likely to be useful
> >   to diagnose boot issues on such systems if they have no accessible serial
> >   port. )
> >
> > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
> > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Tested-by: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Cc: Alexander Graf <agraf@suse.de>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: linux-efi@vger.kernel.org
> > Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > (cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)
>
> This commit doesn't appear to be in Linus' tree. Where did it come from?
>
Seth Forshee Feb. 22, 2019, 10:36 a.m. UTC | #5
On Fri, Feb 22, 2019 at 11:29:19AM +0100, Paolo Pisati wrote:
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=4e46c2a956215482418d7b315749fb1b6c6bc224

Ok, in the future you should include that information like this:

(cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224
 https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git)

Otherwise it is assumed the sha1 is from Linus' tree.

> 
> On Fri, Feb 22, 2019 at 11:09 AM Seth Forshee
> <seth.forshee@canonical.com> wrote:
> >
> > On Wed, Feb 20, 2019 at 03:51:34PM +0100, Paolo Pisati wrote:
> > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >
> > > The UEFI spec revision 2.7 errata A section 8.4 has the following to
> > > say about the virtual memory runtime services:
> > >
> > >   "This section contains function definitions for the virtual memory
> > >   support that may be optionally used by an operating system at runtime.
> > >   If an operating system chooses to make EFI runtime service calls in a
> > >   virtual addressing mode instead of the flat physical mode, then the
> > >   operating system must use the services in this section to switch the
> > >   EFI runtime services from flat physical addressing to virtual
> > >   addressing."
> > >
> > > So it is pretty clear that calling SetVirtualAddressMap() is entirely
> > > optional, and so there is no point in doing so unless it achieves
> > > anything useful for us.
> > >
> > > This is not the case for 64-bit ARM. The identity mapping used by the
> > > firmware is arbitrarily converted into another permutation of userland
> > > addresses (i.e., bits [63:48] cleared), and the runtime code could easily
> > > deal with the original layout in exactly the same way as it deals with
> > > the converted layout. However, due to constraints related to page size
> > > differences if the OS is not running with 4k pages, and related to
> > > systems that may expose the individual sections of PE/COFF runtime
> > > modules as different memory regions, creating the virtual layout is a
> > > bit fiddly, and requires us to sort the memory map and reason about
> > > adjacent regions with identical memory types etc etc.
> > >
> > > So the obvious fix is to stop calling SetVirtualAddressMap() altogether
> > > on arm64 systems. However, to avoid surprises, which are notoriously
> > > hard to diagnose when it comes to OS<->firmware interactions, let's
> > > start by making it an opt-out feature, and implement support for the
> > > 'efi=novamap' kernel command line parameter on ARM and arm64 systems.
> > >
> > > ( Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
> > >   used, given that the physical memory map and the kernel virtual address
> > >   map are not guaranteed to be non-overlapping like on arm64. However,
> > >   having support for efi=novamap,noruntime on 32-bit ARM, combined with
> > >   the recently proposed support for earlycon=efifb, is likely to be useful
> > >   to diagnose boot issues on such systems if they have no accessible serial
> > >   port. )
> > >
> > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
> > > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > Tested-by: Lee Jones <lee.jones@linaro.org>
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Cc: Alexander Graf <agraf@suse.de>
> > > Cc: Borislav Petkov <bp@alien8.de>
> > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > > Cc: Peter Jones <pjones@redhat.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: linux-efi@vger.kernel.org
> > > Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
> > > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > > (cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)
> >
> > This commit doesn't appear to be in Linus' tree. Where did it come from?
> >
> 
> 
> -- 
> bye,
> p.

Patch
diff mbox series

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 01a9d78..3b1e1dc 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -364,6 +364,11 @@  void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
 		paddr = in->phys_addr;
 		size = in->num_pages * EFI_PAGE_SIZE;
 
+		if (novamap()) {
+			in->virt_addr = in->phys_addr;
+			continue;
+		}
+
 		/*
 		 * Make the mapping compatible with 64k pages: this allows
 		 * a 4k page size kernel to kexec a 64k page size kernel and
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 50a9cab..39f87e6 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -34,6 +34,7 @@  static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
 
 static int __section(.data) __nokaslr;
 static int __section(.data) __quiet;
+static int __section(.data) __novamap;
 
 int __pure nokaslr(void)
 {
@@ -43,6 +44,10 @@  int __pure is_quiet(void)
 {
 	return __quiet;
 }
+int __pure novamap(void)
+{
+	return __novamap;
+}
 
 #define EFI_MMAP_NR_SLACK_SLOTS	8
 
@@ -454,6 +459,11 @@  efi_status_t efi_parse_options(char const *cmdline)
 			__chunk_size = -1UL;
 		}
 
+		if (!strncmp(str, "novamap", 7)) {
+			str += strlen("novamap");
+			__novamap = 1;
+		}
+
 		/* Group words together, delimited by "," */
 		while (*str && *str != ' ' && *str != ',')
 			str++;
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index f59564b..2adde22 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -27,6 +27,7 @@ 
 
 extern int __pure nokaslr(void);
 extern int __pure is_quiet(void);
+extern int __pure novamap(void);
 
 #define pr_efi(sys_table, msg)		do {				\
 	if (!is_quiet()) efi_printk(sys_table, "EFI stub: "msg);	\
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 8830fa6..94e7115 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -323,6 +323,9 @@  efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 	if (status == EFI_SUCCESS) {
 		efi_set_virtual_address_map_t *svam;
 
+		if (novamap())
+			return EFI_SUCCESS;
+
 		/* Install the new virtual address map */
 		svam = sys_table->runtime->set_virtual_address_map;
 		status = svam(runtime_entry_count * desc_size, desc_size,