ACK/wcmt: [PATCH] efi/arm/arm64: Allow SetVirtualAddressMap() to be omitted
Colin Ian King
colin.king at canonical.com
Wed Feb 20 14:57:45 UTC 2019
On 20/02/2019 14:51, Paolo Pisati wrote:
> From: Ard Biesheuvel <ard.biesheuvel at 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 at codeaurora.org>
> Tested-by: Bjorn Andersson <bjorn.andersson at linaro.org>
> Tested-by: Lee Jones <lee.jones at linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> Cc: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Cc: Alexander Graf <agraf at suse.de>
> Cc: Borislav Petkov <bp at alien8.de>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Leif Lindholm <leif.lindholm at linaro.org>
> Cc: Linus Torvalds <torvalds at linux-foundation.org>
> Cc: Matt Fleming <matt at codeblueprint.co.uk>
> Cc: Peter Jones <pjones at redhat.com>
> Cc: Peter Zijlstra <peterz at infradead.org>
> Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya at intel.com>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: linux-efi at vger.kernel.org
> Link: http://lkml.kernel.org/r/20190202094119.13230-8-ard.biesheuvel@linaro.org
> Signed-off-by: Ingo Molnar <mingo at kernel.org>
> (cherry picked from commit 4e46c2a956215482418d7b315749fb1b6c6bc224)
> Signed-off-by: Paolo Pisati <paolo.pisati at 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 at canonical.com>
More information about the kernel-team
mailing list