* [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls
@ 2018-01-25 10:31 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8
Cc: joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A, Ard Biesheuvel
UEFI doesn't keep any secrets, so it is mostly unaffected by Meltdown and
Spectre. At boot time, when UEFI is in charge of everything, there is
simply no point in inferring things from the cache state if you can simply
read the data. At runtime, when UEFI runs in the execution context of the
OS, it is really up to the OS to ensure that lower privilege levels cannot
access OS data structures unpermitted.
This applies equally to Spectre variant 2: UEFI runtime services are not
entered directly from userland but always via the kernel, which carries
out any required branch predictor maintenance when switching between the
various tasks (and the UEFI runtime execution context may be considered
a separate task in this sense)
Spectre variant 1 is a different matter though. It requires code changes
and software rebuilds with updated compilers to be fully hardened against
it, although nobody seems to know exactly what that means at the moment.
Given the poor track record of vendors when it comes to keeping UEFI
firmware up to date, it is highly likely that vulnerable versions will
still be in circulation long after we fixed the OS.
Since UEFI interacts with data structures that the OS may consider opaque
(capsule images, authenticated variable updates), it is not really up to
the OS to reason about which invocation is safe and which one isn't. The
only solution really is to simply unmap the entire kernel during UEFI
runtime services invocations, so that there are no secrets to steal to
begin with.
Patch #1 is included for completeness. I sent it out before, and it is a
dependency for this series, but it is otherwise unrelated.
Patch #2 creates a separate stack for UEFI and puts it in the EFI page
tables, along with the asm wrapper that invokes the UEFI runtime services
and a vector table that is activated before and deactivated after the
service is called.
Patch #3 implements marshalling of all byref arguments taken by UEFI
runtime services. This is necessary because they will refer to memory
that is going to be unmapped.
Patch #4 implements the actual unmap/remap sequences, by setting/clearing
the EPD1 bit in TCR_EL1, and doing a local TLB flush.
Note that capsule update has been omitted. This is a bit involved, and
I'd rather get some feedback before burning too many cycles on that. All
other services should be functional, with the caveat that EFI variable
names are now limited to 1024 [wide] characters, and UEFI variables
themselves to 64 KB.
Ard Biesheuvel (4):
efi: arm64: Check whether x18 is preserved by runtime services calls
efi/arm64: map the stack and entry wrapper into the UEFI page tables
efi/arm64: marshall runtime services arguments via buffer in TTBR0
efi/arm64: unmap the kernel while executing UEFI services
arch/arm/include/asm/efi.h | 5 +
arch/arm64/Kconfig | 1 -
arch/arm64/include/asm/efi.h | 30 +-
arch/arm64/include/asm/stacktrace.h | 4 +
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/efi-rt-wrapper.S | 112 +++++
arch/arm64/kernel/efi.c | 505 +++++++++++++++++++-
arch/arm64/kernel/entry.S | 1 +
drivers/firmware/efi/arm-runtime.c | 2 +
9 files changed, 658 insertions(+), 5 deletions(-)
create mode 100644 arch/arm64/kernel/efi-rt-wrapper.S
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls
@ 2018-01-25 10:31 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-arm-kernel
UEFI doesn't keep any secrets, so it is mostly unaffected by Meltdown and
Spectre. At boot time, when UEFI is in charge of everything, there is
simply no point in inferring things from the cache state if you can simply
read the data. At runtime, when UEFI runs in the execution context of the
OS, it is really up to the OS to ensure that lower privilege levels cannot
access OS data structures unpermitted.
This applies equally to Spectre variant 2: UEFI runtime services are not
entered directly from userland but always via the kernel, which carries
out any required branch predictor maintenance when switching between the
various tasks (and the UEFI runtime execution context may be considered
a separate task in this sense)
Spectre variant 1 is a different matter though. It requires code changes
and software rebuilds with updated compilers to be fully hardened against
it, although nobody seems to know exactly what that means at the moment.
Given the poor track record of vendors when it comes to keeping UEFI
firmware up to date, it is highly likely that vulnerable versions will
still be in circulation long after we fixed the OS.
Since UEFI interacts with data structures that the OS may consider opaque
(capsule images, authenticated variable updates), it is not really up to
the OS to reason about which invocation is safe and which one isn't. The
only solution really is to simply unmap the entire kernel during UEFI
runtime services invocations, so that there are no secrets to steal to
begin with.
Patch #1 is included for completeness. I sent it out before, and it is a
dependency for this series, but it is otherwise unrelated.
Patch #2 creates a separate stack for UEFI and puts it in the EFI page
tables, along with the asm wrapper that invokes the UEFI runtime services
and a vector table that is activated before and deactivated after the
service is called.
Patch #3 implements marshalling of all byref arguments taken by UEFI
runtime services. This is necessary because they will refer to memory
that is going to be unmapped.
Patch #4 implements the actual unmap/remap sequences, by setting/clearing
the EPD1 bit in TCR_EL1, and doing a local TLB flush.
Note that capsule update has been omitted. This is a bit involved, and
I'd rather get some feedback before burning too many cycles on that. All
other services should be functional, with the caveat that EFI variable
names are now limited to 1024 [wide] characters, and UEFI variables
themselves to 64 KB.
Ard Biesheuvel (4):
efi: arm64: Check whether x18 is preserved by runtime services calls
efi/arm64: map the stack and entry wrapper into the UEFI page tables
efi/arm64: marshall runtime services arguments via buffer in TTBR0
efi/arm64: unmap the kernel while executing UEFI services
arch/arm/include/asm/efi.h | 5 +
arch/arm64/Kconfig | 1 -
arch/arm64/include/asm/efi.h | 30 +-
arch/arm64/include/asm/stacktrace.h | 4 +
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/efi-rt-wrapper.S | 112 +++++
arch/arm64/kernel/efi.c | 505 +++++++++++++++++++-
arch/arm64/kernel/entry.S | 1 +
drivers/firmware/efi/arm-runtime.c | 2 +
9 files changed, 658 insertions(+), 5 deletions(-)
create mode 100644 arch/arm64/kernel/efi-rt-wrapper.S
--
2.11.0
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/4] efi: arm64: Check whether x18 is preserved by runtime services calls
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-25 10:31 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8
Cc: joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A, Ard Biesheuvel
Whether or not we will ever decide to start using x18 as a platform
register in Linux is uncertain, but by that time, we will need to
ensure that UEFI runtime services calls don't corrupt it. So let's
start issuing warnings now for this, and increase the likelihood that
these firmware images have all been replaced by that time.
This has been fixed on the EDK2 side in commit 6d73863b5464
("BaseTools/tools_def AARCH64: mark register x18 as reserved").,
dated July 13, 2017.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm64/include/asm/efi.h | 4 +-
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/efi-rt-wrapper.S | 41 ++++++++++++++++++++
arch/arm64/kernel/efi.c | 6 +++
4 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 8389050328bb..192d791f1103 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -31,7 +31,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
({ \
efi_##f##_t *__f; \
__f = p->f; \
- __f(args); \
+ __efi_rt_asm_wrapper(__f, #f, args); \
})
#define arch_efi_call_virt_teardown() \
@@ -40,6 +40,8 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
efi_virtmap_unload(); \
})
+efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
+
#define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
/* arch specific definitions used by the stub code */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index b87541360f43..6a4bd80c75bd 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -38,7 +38,8 @@ arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o
arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o
arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
arm64-obj-$(CONFIG_KGDB) += kgdb.o
-arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o
+arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o \
+ efi-rt-wrapper.o
arm64-obj-$(CONFIG_PCI) += pci.o
arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
arm64-obj-$(CONFIG_ACPI) += acpi.o
diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
new file mode 100644
index 000000000000..05235ebb336d
--- /dev/null
+++ b/arch/arm64/kernel/efi-rt-wrapper.S
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2018 Linaro Ltd <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+
+ENTRY(__efi_rt_asm_wrapper)
+ stp x29, x30, [sp, #-32]!
+ mov x29, sp
+
+ /*
+ * Register x18 is designated as the 'platform' register by the AAPCS,
+ * which means firmware running at the same exception level as the OS
+ * (such as UEFI) should never touch it.
+ */
+ stp x1, x18, [sp, #16]
+
+ /*
+ * We are lucky enough that no EFI runtime services take more than
+ * 5 arguments, so all are passed in registers rather than via the
+ * stack.
+ */
+ mov x8, x0
+ mov x0, x2
+ mov x1, x3
+ mov x2, x4
+ mov x3, x5
+ mov x4, x6
+ blr x8
+
+ ldp x1, x2, [sp, #16]
+ cmp x2, x18
+ ldp x29, x30, [sp], #32
+ b.ne 0f
+ ret
+0: b efi_handle_corrupted_x18 // tail call
+ENDPROC(__efi_rt_asm_wrapper)
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 82cd07592519..af4f943cffac 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -124,3 +124,9 @@ bool efi_poweroff_required(void)
{
return efi_enabled(EFI_RUNTIME_SERVICES);
}
+
+asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
+{
+ pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
+ return s;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/4] efi: arm64: Check whether x18 is preserved by runtime services calls
@ 2018-01-25 10:31 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-arm-kernel
Whether or not we will ever decide to start using x18 as a platform
register in Linux is uncertain, but by that time, we will need to
ensure that UEFI runtime services calls don't corrupt it. So let's
start issuing warnings now for this, and increase the likelihood that
these firmware images have all been replaced by that time.
This has been fixed on the EDK2 side in commit 6d73863b5464
("BaseTools/tools_def AARCH64: mark register x18 as reserved").,
dated July 13, 2017.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/efi.h | 4 +-
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/efi-rt-wrapper.S | 41 ++++++++++++++++++++
arch/arm64/kernel/efi.c | 6 +++
4 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 8389050328bb..192d791f1103 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -31,7 +31,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
({ \
efi_##f##_t *__f; \
__f = p->f; \
- __f(args); \
+ __efi_rt_asm_wrapper(__f, #f, args); \
})
#define arch_efi_call_virt_teardown() \
@@ -40,6 +40,8 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
efi_virtmap_unload(); \
})
+efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
+
#define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
/* arch specific definitions used by the stub code */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index b87541360f43..6a4bd80c75bd 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -38,7 +38,8 @@ arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o
arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o
arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
arm64-obj-$(CONFIG_KGDB) += kgdb.o
-arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o
+arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o \
+ efi-rt-wrapper.o
arm64-obj-$(CONFIG_PCI) += pci.o
arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
arm64-obj-$(CONFIG_ACPI) += acpi.o
diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
new file mode 100644
index 000000000000..05235ebb336d
--- /dev/null
+++ b/arch/arm64/kernel/efi-rt-wrapper.S
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2018 Linaro Ltd <ard.biesheuvel@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+
+ENTRY(__efi_rt_asm_wrapper)
+ stp x29, x30, [sp, #-32]!
+ mov x29, sp
+
+ /*
+ * Register x18 is designated as the 'platform' register by the AAPCS,
+ * which means firmware running at the same exception level as the OS
+ * (such as UEFI) should never touch it.
+ */
+ stp x1, x18, [sp, #16]
+
+ /*
+ * We are lucky enough that no EFI runtime services take more than
+ * 5 arguments, so all are passed in registers rather than via the
+ * stack.
+ */
+ mov x8, x0
+ mov x0, x2
+ mov x1, x3
+ mov x2, x4
+ mov x3, x5
+ mov x4, x6
+ blr x8
+
+ ldp x1, x2, [sp, #16]
+ cmp x2, x18
+ ldp x29, x30, [sp], #32
+ b.ne 0f
+ ret
+0: b efi_handle_corrupted_x18 // tail call
+ENDPROC(__efi_rt_asm_wrapper)
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 82cd07592519..af4f943cffac 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -124,3 +124,9 @@ bool efi_poweroff_required(void)
{
return efi_enabled(EFI_RUNTIME_SERVICES);
}
+
+asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
+{
+ pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
+ return s;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-25 10:31 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8
Cc: joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A, Ard Biesheuvel
As a preparatory step towards unmapping the kernel entirely while
executing UEFI runtime services, move the stack and the entry
wrapper routine mappings into the EFI page tables. Also, create a
vector table that overrides the main one while executing in the
firmware so we will be able to remap/unmap the kernel while taking
interrupts.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm/include/asm/efi.h | 5 ++
arch/arm64/include/asm/efi.h | 23 ++++++++-
arch/arm64/include/asm/stacktrace.h | 4 ++
arch/arm64/kernel/efi-rt-wrapper.S | 51 +++++++++++++++++++-
arch/arm64/kernel/efi.c | 24 +++++++++
arch/arm64/kernel/entry.S | 1 +
drivers/firmware/efi/arm-runtime.c | 2 +
7 files changed, 107 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 17f1f1a814ff..3a63e7cc1dfa 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -99,4 +99,9 @@ static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
return dram_base + SZ_512M;
}
+static inline int efi_allocate_runtime_regions(struct mm_struct *mm)
+{
+ return 0;
+}
+
#endif /* _ASM_ARM_EFI_H */
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 192d791f1103..b9b09a734719 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -2,11 +2,13 @@
#ifndef _ASM_EFI_H
#define _ASM_EFI_H
+#include <asm/memory.h>
+
+#ifndef __ASSEMBLY__
#include <asm/boot.h>
#include <asm/cpufeature.h>
#include <asm/fpsimd.h>
#include <asm/io.h>
-#include <asm/memory.h>
#include <asm/mmu_context.h>
#include <asm/neon.h>
#include <asm/ptrace.h>
@@ -30,8 +32,9 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
#define arch_efi_call_virt(p, f, args...) \
({ \
efi_##f##_t *__f; \
+ typeof(__efi_rt_asm_wrapper) *__wrap = (void *)EFI_CODE_BASE; \
__f = p->f; \
- __efi_rt_asm_wrapper(__f, #f, args); \
+ __wrap(__f, #f, args); \
})
#define arch_efi_call_virt_teardown() \
@@ -146,4 +149,20 @@ static inline void efi_set_pgd(struct mm_struct *mm)
void efi_virtmap_load(void);
void efi_virtmap_unload(void);
+int __init efi_allocate_runtime_regions(struct mm_struct *mm);
+
+#endif /* __ASSEMBLY__ */
+
+/*
+ * When running with vmap'ed stacks, we need the base of the stack to be aligned
+ * appropriately, where the exact alignment depends on the page size. Let's just
+ * put the stack at address 0x0, which is guaranteed to be free and aligned.
+ */
+#define EFI_STACK_BASE 0x0
+#define EFI_STACK_SIZE THREAD_SIZE
+
+/* where to map the pivot code in the UEFI page tables */
+#define EFI_CODE_BASE 0x200000
+#define EFI_CODE_SIZE PAGE_SIZE
+
#endif /* _ASM_EFI_H */
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index 472ef944e932..b1212b3b3df5 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -72,6 +72,8 @@ static inline bool on_overflow_stack(unsigned long sp)
static inline bool on_overflow_stack(unsigned long sp) { return false; }
#endif
+bool on_efi_stack(unsigned long sp);
+
/*
* We can only safely access per-cpu stacks from current in a non-preemptible
* context.
@@ -88,6 +90,8 @@ static inline bool on_accessible_stack(struct task_struct *tsk, unsigned long sp
return true;
if (on_sdei_stack(sp))
return true;
+ if (on_efi_stack(sp))
+ return true;
return false;
}
diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
index 05235ebb336d..09e77e5edd94 100644
--- a/arch/arm64/kernel/efi-rt-wrapper.S
+++ b/arch/arm64/kernel/efi-rt-wrapper.S
@@ -7,7 +7,10 @@
*/
#include <linux/linkage.h>
+#include <asm/efi.h>
+ .section ".rodata", "a"
+ .align PAGE_SHIFT
ENTRY(__efi_rt_asm_wrapper)
stp x29, x30, [sp, #-32]!
mov x29, sp
@@ -19,6 +22,12 @@ ENTRY(__efi_rt_asm_wrapper)
*/
stp x1, x18, [sp, #16]
+ /* switch to the EFI runtime stack and vector table */
+ mov sp, #EFI_STACK_BASE + EFI_STACK_SIZE
+ adr x1, __efi_rt_vectors
+ msr vbar_el1, x1
+ isb
+
/*
* We are lucky enough that no EFI runtime services take more than
* 5 arguments, so all are passed in registers rather than via the
@@ -32,10 +41,50 @@ ENTRY(__efi_rt_asm_wrapper)
mov x4, x6
blr x8
+ /* switch back to the task stack and primary vector table */
+ mov sp, x29
+ ldr x1, 2f
+ msr vbar_el1, x1
+ isb
+
ldp x1, x2, [sp, #16]
cmp x2, x18
ldp x29, x30, [sp], #32
b.ne 0f
ret
-0: b efi_handle_corrupted_x18 // tail call
+0: ldr x8, 1f
+ br x8 // tail call
ENDPROC(__efi_rt_asm_wrapper)
+ .align 3
+1: .quad efi_handle_corrupted_x18
+2: .quad vectors
+
+ .macro ventry
+ .align 7
+.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
+ mrs x29, elr_el1 // preserve ELR
+ adr x30, .Lret // take return address
+ msr elr_el1, x30 // set ELR to return address
+ ldr x30, 2b // take address of 'vectors'
+ msr vbar_el1, x30 // set VBAR to 'vectors'
+ isb
+ add x30, x30, #.Lv\@ - __efi_rt_vectors
+ br x30
+ .endm
+
+.Lret: msr elr_el1, x29
+ adr x30, __efi_rt_vectors
+ msr vbar_el1, x30
+ isb
+ ldp x29, x30, [sp], #16
+ eret
+
+ .align 11
+__efi_rt_vectors:
+ .rept 8
+ ventry
+ .endr
+ /*
+ * EFI runtime services never drop to EL0, so the
+ * remaining vector table entries are not needed.
+ */
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index af4f943cffac..68c920b2f4f0 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
return s;
}
+
+bool on_efi_stack(unsigned long sp)
+{
+ return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
+}
+
+int __init efi_allocate_runtime_regions(struct mm_struct *mm)
+{
+ static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
+
+ /* map the stack */
+ create_pgd_mapping(mm, __pa_symbol(stack),
+ EFI_STACK_BASE, EFI_STACK_SIZE,
+ __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+ false);
+
+ /* map the runtime wrapper pivot function */
+ create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
+ EFI_CODE_BASE, EFI_CODE_SIZE,
+ __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
+ false);
+
+ return 0;
+}
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index b34e717d7597..3bab6c60a12b 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
alternative_else_nop_endif
.if \el != 0
+ tbz x21, #63, 1f // skip if TTBR0 covers the stack
mrs x21, ttbr0_el1
tst x21, #TTBR_ASID_MASK // Check for the reserved ASID
orr x23, x23, #PSR_PAN_BIT // Set the emulated PAN in the saved SPSR
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 1cc41c3d6315..e84f4d961de2 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -107,6 +107,8 @@ static bool __init efi_virtmap_init(void)
if (efi_memattr_apply_permissions(&efi_mm, efi_set_mapping_permissions))
return false;
+ efi_allocate_runtime_regions(&efi_mm);
+
return true;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
@ 2018-01-25 10:31 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-arm-kernel
As a preparatory step towards unmapping the kernel entirely while
executing UEFI runtime services, move the stack and the entry
wrapper routine mappings into the EFI page tables. Also, create a
vector table that overrides the main one while executing in the
firmware so we will be able to remap/unmap the kernel while taking
interrupts.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/include/asm/efi.h | 5 ++
arch/arm64/include/asm/efi.h | 23 ++++++++-
arch/arm64/include/asm/stacktrace.h | 4 ++
arch/arm64/kernel/efi-rt-wrapper.S | 51 +++++++++++++++++++-
arch/arm64/kernel/efi.c | 24 +++++++++
arch/arm64/kernel/entry.S | 1 +
drivers/firmware/efi/arm-runtime.c | 2 +
7 files changed, 107 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 17f1f1a814ff..3a63e7cc1dfa 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -99,4 +99,9 @@ static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
return dram_base + SZ_512M;
}
+static inline int efi_allocate_runtime_regions(struct mm_struct *mm)
+{
+ return 0;
+}
+
#endif /* _ASM_ARM_EFI_H */
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 192d791f1103..b9b09a734719 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -2,11 +2,13 @@
#ifndef _ASM_EFI_H
#define _ASM_EFI_H
+#include <asm/memory.h>
+
+#ifndef __ASSEMBLY__
#include <asm/boot.h>
#include <asm/cpufeature.h>
#include <asm/fpsimd.h>
#include <asm/io.h>
-#include <asm/memory.h>
#include <asm/mmu_context.h>
#include <asm/neon.h>
#include <asm/ptrace.h>
@@ -30,8 +32,9 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
#define arch_efi_call_virt(p, f, args...) \
({ \
efi_##f##_t *__f; \
+ typeof(__efi_rt_asm_wrapper) *__wrap = (void *)EFI_CODE_BASE; \
__f = p->f; \
- __efi_rt_asm_wrapper(__f, #f, args); \
+ __wrap(__f, #f, args); \
})
#define arch_efi_call_virt_teardown() \
@@ -146,4 +149,20 @@ static inline void efi_set_pgd(struct mm_struct *mm)
void efi_virtmap_load(void);
void efi_virtmap_unload(void);
+int __init efi_allocate_runtime_regions(struct mm_struct *mm);
+
+#endif /* __ASSEMBLY__ */
+
+/*
+ * When running with vmap'ed stacks, we need the base of the stack to be aligned
+ * appropriately, where the exact alignment depends on the page size. Let's just
+ * put the stack at address 0x0, which is guaranteed to be free and aligned.
+ */
+#define EFI_STACK_BASE 0x0
+#define EFI_STACK_SIZE THREAD_SIZE
+
+/* where to map the pivot code in the UEFI page tables */
+#define EFI_CODE_BASE 0x200000
+#define EFI_CODE_SIZE PAGE_SIZE
+
#endif /* _ASM_EFI_H */
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h
index 472ef944e932..b1212b3b3df5 100644
--- a/arch/arm64/include/asm/stacktrace.h
+++ b/arch/arm64/include/asm/stacktrace.h
@@ -72,6 +72,8 @@ static inline bool on_overflow_stack(unsigned long sp)
static inline bool on_overflow_stack(unsigned long sp) { return false; }
#endif
+bool on_efi_stack(unsigned long sp);
+
/*
* We can only safely access per-cpu stacks from current in a non-preemptible
* context.
@@ -88,6 +90,8 @@ static inline bool on_accessible_stack(struct task_struct *tsk, unsigned long sp
return true;
if (on_sdei_stack(sp))
return true;
+ if (on_efi_stack(sp))
+ return true;
return false;
}
diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
index 05235ebb336d..09e77e5edd94 100644
--- a/arch/arm64/kernel/efi-rt-wrapper.S
+++ b/arch/arm64/kernel/efi-rt-wrapper.S
@@ -7,7 +7,10 @@
*/
#include <linux/linkage.h>
+#include <asm/efi.h>
+ .section ".rodata", "a"
+ .align PAGE_SHIFT
ENTRY(__efi_rt_asm_wrapper)
stp x29, x30, [sp, #-32]!
mov x29, sp
@@ -19,6 +22,12 @@ ENTRY(__efi_rt_asm_wrapper)
*/
stp x1, x18, [sp, #16]
+ /* switch to the EFI runtime stack and vector table */
+ mov sp, #EFI_STACK_BASE + EFI_STACK_SIZE
+ adr x1, __efi_rt_vectors
+ msr vbar_el1, x1
+ isb
+
/*
* We are lucky enough that no EFI runtime services take more than
* 5 arguments, so all are passed in registers rather than via the
@@ -32,10 +41,50 @@ ENTRY(__efi_rt_asm_wrapper)
mov x4, x6
blr x8
+ /* switch back to the task stack and primary vector table */
+ mov sp, x29
+ ldr x1, 2f
+ msr vbar_el1, x1
+ isb
+
ldp x1, x2, [sp, #16]
cmp x2, x18
ldp x29, x30, [sp], #32
b.ne 0f
ret
-0: b efi_handle_corrupted_x18 // tail call
+0: ldr x8, 1f
+ br x8 // tail call
ENDPROC(__efi_rt_asm_wrapper)
+ .align 3
+1: .quad efi_handle_corrupted_x18
+2: .quad vectors
+
+ .macro ventry
+ .align 7
+.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
+ mrs x29, elr_el1 // preserve ELR
+ adr x30, .Lret // take return address
+ msr elr_el1, x30 // set ELR to return address
+ ldr x30, 2b // take address of 'vectors'
+ msr vbar_el1, x30 // set VBAR to 'vectors'
+ isb
+ add x30, x30, #.Lv\@ - __efi_rt_vectors
+ br x30
+ .endm
+
+.Lret: msr elr_el1, x29
+ adr x30, __efi_rt_vectors
+ msr vbar_el1, x30
+ isb
+ ldp x29, x30, [sp], #16
+ eret
+
+ .align 11
+__efi_rt_vectors:
+ .rept 8
+ ventry
+ .endr
+ /*
+ * EFI runtime services never drop to EL0, so the
+ * remaining vector table entries are not needed.
+ */
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index af4f943cffac..68c920b2f4f0 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
return s;
}
+
+bool on_efi_stack(unsigned long sp)
+{
+ return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
+}
+
+int __init efi_allocate_runtime_regions(struct mm_struct *mm)
+{
+ static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
+
+ /* map the stack */
+ create_pgd_mapping(mm, __pa_symbol(stack),
+ EFI_STACK_BASE, EFI_STACK_SIZE,
+ __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+ false);
+
+ /* map the runtime wrapper pivot function */
+ create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
+ EFI_CODE_BASE, EFI_CODE_SIZE,
+ __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
+ false);
+
+ return 0;
+}
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index b34e717d7597..3bab6c60a12b 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
alternative_else_nop_endif
.if \el != 0
+ tbz x21, #63, 1f // skip if TTBR0 covers the stack
mrs x21, ttbr0_el1
tst x21, #TTBR_ASID_MASK // Check for the reserved ASID
orr x23, x23, #PSR_PAN_BIT // Set the emulated PAN in the saved SPSR
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 1cc41c3d6315..e84f4d961de2 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -107,6 +107,8 @@ static bool __init efi_virtmap_init(void)
if (efi_memattr_apply_permissions(&efi_mm, efi_set_mapping_permissions))
return false;
+ efi_allocate_runtime_regions(&efi_mm);
+
return true;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/4] efi/arm64: marshall runtime services arguments via buffer in TTBR0
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-25 10:31 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8
Cc: joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A, Ard Biesheuvel
If we want to unmap the kernel while executing UEFI runtime services,
we need to ensure that all arguments passed to those services are
valid during that time, and this includes pointer arguments to
string buffers and other data. So we have to do the rather tedious
job of replacing each generic runtime wrapper with one that takes the
above into account.
Fortunately, UEFI runtime services are not reentrant, so we can create
a static buffer with fields for each runtime service, and a separate
buffer for get/set_variable. This does limit variable names to 1024
characters and the variables themselves to 64 KB, but this should not
be a limitation in practice.
Note that capsule update is omitted for now - this will be addressed
in a subsequent patch.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm64/Kconfig | 1 -
arch/arm64/include/asm/efi.h | 5 +
arch/arm64/kernel/efi.c | 475 +++++++++++++++++++-
3 files changed, 479 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index de83b53c7e19..f2224566cc75 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1196,7 +1196,6 @@ config EFI
select LIBFDT
select UCS2_STRING
select EFI_PARAMS_FROM_FDT
- select EFI_RUNTIME_WRAPPERS
select EFI_STUB
select EFI_ARMSTUB
default y
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index b9b09a734719..b96bc4836c37 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -165,4 +165,9 @@ int __init efi_allocate_runtime_regions(struct mm_struct *mm);
#define EFI_CODE_BASE 0x200000
#define EFI_CODE_SIZE PAGE_SIZE
+#define EFI_VARBUFFER_BASE 0x210000
+#define EFI_VARBUFFER_SIZE SZ_64K
+
+#define EFI_DATA_BASE 0x220000
+
#endif /* _ASM_EFI_H */
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 68c920b2f4f0..cd23c20fba39 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -11,9 +11,15 @@
*
*/
+#define pr_fmt(fmt) "efi: " fmt
+
+#include <linux/bug.h>
#include <linux/efi.h>
#include <linux/init.h>
-
+#include <linux/irqflags.h>
+#include <linux/mutex.h>
+#include <linux/semaphore.h>
+#include <linux/stringify.h>
#include <asm/efi.h>
/*
@@ -136,6 +142,63 @@ bool on_efi_stack(unsigned long sp)
return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
}
+#define MAX_CAPSULES 16
+#define MAX_VARNAME_LEN 1024
+
+#define EFI_ADDR(arg) (&((typeof(__efi_rt_marshall_data)*)EFI_DATA_BASE)->arg)
+#define EFI_DATA(arg) (__efi_rt_marshall_data.arg)
+
+static union {
+ struct {
+ efi_time_t tm;
+ efi_time_cap_t tc;
+ } get_time;
+
+ struct {
+ efi_time_t tm;
+ } set_time;
+
+ struct {
+ efi_bool_t enabled;
+ efi_bool_t pending;
+ efi_time_t tm;
+ } get_wakeup_time;
+
+ struct {
+ efi_char16_t name[MAX_VARNAME_LEN];
+ efi_guid_t vendor;
+ u32 attr;
+ unsigned long data_size;
+ void *data;
+ } get_variable;
+
+ struct {
+ unsigned long name_size;
+ efi_char16_t name[MAX_VARNAME_LEN];
+ efi_guid_t vendor;
+ } get_next_variable;
+
+ struct {
+ u64 storage_space;
+ u64 remaining_space;
+ u64 max_variable_size;
+ } query_variable_info;
+
+ struct {
+ u32 count;
+ } get_next_high_mono_count;
+
+ struct {
+ efi_capsule_header_t capsules[MAX_CAPSULES];
+ efi_capsule_header_t *capsule_headers[MAX_CAPSULES];
+ u64 max_size;
+ int reset_type;
+ } query_capsule_caps;
+
+} __efi_rt_marshall_data __page_aligned_bss;
+
+static u8 __efi_rt_varbuffer[EFI_VARBUFFER_SIZE] __page_aligned_bss;
+
int __init efi_allocate_runtime_regions(struct mm_struct *mm)
{
static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
@@ -152,5 +215,415 @@ int __init efi_allocate_runtime_regions(struct mm_struct *mm)
__pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
false);
+ /* map the marshall data struct */
+ create_pgd_mapping(mm, __pa_symbol(&__efi_rt_marshall_data),
+ EFI_DATA_BASE, sizeof(__efi_rt_marshall_data),
+ __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+ false);
+
+ /* map the varbuffer */
+ create_pgd_mapping(mm, __pa_symbol(__efi_rt_varbuffer),
+ EFI_VARBUFFER_BASE, EFI_VARBUFFER_SIZE,
+ __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+ false);
+
return 0;
}
+
+/*
+ * Wrap around the new efi_call_virt_generic() macros so that the
+ * code doesn't get too cluttered:
+ */
+#define efi_call_virt(f, args...) \
+ efi_call_virt_pointer(efi.systab->runtime, f, args)
+#define __efi_call_virt(f, args...) \
+ __efi_call_virt_pointer(efi.systab->runtime, f, args)
+
+void efi_call_virt_check_flags(unsigned long flags, const char *call)
+{
+ unsigned long cur_flags, mismatch;
+
+ local_save_flags(cur_flags);
+
+ mismatch = flags ^ cur_flags;
+ if (!WARN_ON_ONCE(mismatch & ARCH_EFI_IRQ_FLAGS_MASK))
+ return;
+
+ add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_NOW_UNRELIABLE);
+ pr_err_ratelimited(FW_BUG "IRQ flags corrupted (0x%08lx=>0x%08lx) by EFI %s\n",
+ flags, cur_flags, call);
+ local_irq_restore(flags);
+}
+
+static int strlen16(efi_char16_t const *name)
+{
+ int ret = 0;
+
+ while (*name++)
+ ret++;
+
+ return ret;
+}
+
+static DEFINE_SEMAPHORE(efi_runtime_lock);
+
+static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
+{
+ efi_status_t status;
+
+ if (!tm)
+ return EFI_INVALID_PARAMETER;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+ status = efi_call_virt(get_time,
+ EFI_ADDR(get_time.tm),
+ tc ? EFI_ADDR(get_time.tc): NULL);
+
+ *tm = EFI_DATA(get_time.tm);
+ if (tc)
+ *tc = EFI_DATA(get_time.tc);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_set_time(efi_time_t *tm)
+{
+ efi_status_t status;
+
+ if (!tm)
+ return EFI_INVALID_PARAMETER;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ EFI_DATA(set_time.tm) = *tm;
+
+ status = efi_call_virt(set_time,
+ EFI_ADDR(set_time.tm));
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
+ efi_bool_t *pending,
+ efi_time_t *tm)
+{
+ efi_status_t status;
+
+ if (!enabled || !pending || !tm)
+ return EFI_INVALID_PARAMETER;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+ status = efi_call_virt(get_wakeup_time,
+ EFI_ADDR(get_wakeup_time.enabled),
+ EFI_ADDR(get_wakeup_time.pending),
+ EFI_ADDR(get_wakeup_time.tm));
+
+ *enabled = EFI_DATA(get_wakeup_time.enabled);
+ *pending = EFI_DATA(get_wakeup_time.pending);
+ *tm = EFI_DATA(get_wakeup_time.tm);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
+{
+ efi_status_t status;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ EFI_DATA(set_time.tm) = *tm;
+
+ status = efi_call_virt(set_wakeup_time, enabled,
+ EFI_ADDR(set_time.tm));
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_variable(efi_char16_t *name,
+ efi_guid_t *vendor,
+ u32 *attr,
+ unsigned long *data_size,
+ void *data)
+{
+ efi_status_t status;
+ int l = strlen16(name) + 1;
+
+ if (!name || !vendor || !data_size)
+ return EFI_INVALID_PARAMETER;
+
+ if (*data_size > EFI_VARBUFFER_SIZE || l > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ memcpy(EFI_DATA(get_variable.name), name, l * sizeof(efi_char16_t));
+ EFI_DATA(get_variable.vendor) = *vendor;
+ EFI_DATA(get_variable.data_size) = *data_size;
+
+ status = efi_call_virt(get_variable,
+ EFI_ADDR(get_variable.name),
+ EFI_ADDR(get_variable.vendor),
+ attr ? EFI_ADDR(get_variable.attr) : NULL,
+ EFI_ADDR(get_variable.data_size),
+ data ? (void *)EFI_VARBUFFER_BASE : NULL);
+
+ if (attr)
+ *attr = EFI_DATA(get_variable.attr);
+ *data_size = EFI_DATA(get_variable.data_size);
+ if (data)
+ memcpy(data, __efi_rt_varbuffer, *data_size);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
+ efi_char16_t *name,
+ efi_guid_t *vendor)
+{
+ efi_status_t status;
+
+ if (*name_size > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ EFI_DATA(get_next_variable.name_size) = *name_size;
+ memcpy(EFI_DATA(get_next_variable.name), name, *name_size);
+ EFI_DATA(get_next_variable.vendor) = *vendor;
+
+ status = efi_call_virt(get_next_variable,
+ EFI_ADDR(get_next_variable.name_size),
+ EFI_ADDR(get_next_variable.name),
+ EFI_ADDR(get_next_variable.vendor));
+
+ *name_size = EFI_DATA(get_next_variable.name_size);
+ memcpy(name, EFI_DATA(get_next_variable.name), *name_size);
+ *vendor = EFI_DATA(get_next_variable.vendor);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_set_variable(efi_char16_t *name,
+ efi_guid_t *vendor,
+ u32 attr,
+ unsigned long data_size,
+ void *data)
+{
+ efi_status_t status;
+ int l = strlen16(name) + 1;
+
+ if (!name || !vendor)
+ return EFI_INVALID_PARAMETER;
+
+ if (data_size > EFI_VARBUFFER_SIZE || l > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ memcpy(EFI_DATA(get_variable.name), name, l * sizeof(efi_char16_t));
+ EFI_DATA(get_variable.vendor) = *vendor;
+ if (data)
+ memcpy(__efi_rt_varbuffer, data, data_size);
+
+ status = efi_call_virt(set_variable,
+ EFI_ADDR(get_variable.name),
+ EFI_ADDR(get_variable.vendor),
+ attr,
+ data_size,
+ data ? (void *)EFI_VARBUFFER_BASE : NULL);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t
+virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor,
+ u32 attr, unsigned long data_size,
+ void *data)
+{
+ efi_status_t status;
+ int l = strlen16(name) + 1;
+
+ if (!name || !vendor)
+ return EFI_INVALID_PARAMETER;
+
+ if (data_size > EFI_VARBUFFER_SIZE || l > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_trylock(&efi_runtime_lock))
+ return EFI_NOT_READY;
+
+ memcpy(EFI_DATA(get_variable.name), name, l * sizeof(efi_char16_t));
+ EFI_DATA(get_variable.vendor) = *vendor;
+ if (data)
+ memcpy(__efi_rt_varbuffer, data, data_size);
+
+ status = efi_call_virt(set_variable,
+ EFI_ADDR(get_variable.name),
+ EFI_ADDR(get_variable.vendor),
+ attr,
+ data_size,
+ data ? (void *)EFI_VARBUFFER_BASE : NULL);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+
+static efi_status_t virt_efi_query_variable_info(u32 attr,
+ u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size)
+{
+ efi_status_t status;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ status = efi_call_virt(query_variable_info,
+ attr,
+ EFI_ADDR(query_variable_info.storage_space),
+ EFI_ADDR(query_variable_info.remaining_space),
+ EFI_ADDR(query_variable_info.max_variable_size));
+
+ *storage_space = EFI_DATA(query_variable_info.storage_space);
+ *remaining_space = EFI_DATA(query_variable_info.remaining_space);
+ *max_variable_size = EFI_DATA(query_variable_info.max_variable_size);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t
+virt_efi_query_variable_info_nonblocking(u32 attr,
+ u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size)
+{
+ efi_status_t status;
+
+ if (down_trylock(&efi_runtime_lock))
+ return EFI_NOT_READY;
+
+ status = efi_call_virt(query_variable_info,
+ attr,
+ EFI_ADDR(query_variable_info.storage_space),
+ EFI_ADDR(query_variable_info.remaining_space),
+ EFI_ADDR(query_variable_info.max_variable_size));
+
+ *storage_space = EFI_DATA(query_variable_info.storage_space);
+ *remaining_space = EFI_DATA(query_variable_info.remaining_space);
+ *max_variable_size = EFI_DATA(query_variable_info.max_variable_size);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
+{
+ efi_status_t status;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+ status = efi_call_virt(get_next_high_mono_count,
+ EFI_ADDR(get_next_high_mono_count.count));
+
+ *count = EFI_DATA(get_next_high_mono_count.count);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static void virt_efi_reset_system(int reset_type,
+ efi_status_t status,
+ unsigned long data_size,
+ efi_char16_t *data)
+{
+ /* we don't use 'data' in the kernel */
+ pr_warn("efi_reset_system: ignoring 'data' argument\n");
+
+ if (down_interruptible(&efi_runtime_lock)) {
+ pr_warn("failed to invoke the reset_system() runtime service:\n"
+ "could not get exclusive access to the firmware\n");
+ return;
+ }
+ __efi_call_virt(reset_system, reset_type, status, 0, NULL);
+ up(&efi_runtime_lock);
+}
+
+static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
+ unsigned long count,
+ unsigned long sg_list)
+{
+// efi_status_t status;
+
+ return EFI_NOT_READY;
+
+// if (down_interruptible(&efi_runtime_lock))
+// return EFI_ABORTED;
+// status = efi_call_virt(update_capsule, capsules, count, sg_list);
+// up(&efi_runtime_lock);
+// return status;
+}
+
+static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules,
+ unsigned long count,
+ u64 *max_size,
+ int *reset_type)
+{
+ efi_status_t status;
+ int i;
+
+ if (count > MAX_CAPSULES)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ for (i = 0; i < count; i++) {
+ EFI_DATA(query_capsule_caps.capsules[i]) = *capsules[i];
+ EFI_DATA(query_capsule_caps.capsule_headers[i]) =
+ EFI_ADDR(query_capsule_caps.capsules[i]);
+ }
+
+ status = efi_call_virt(query_capsule_caps,
+ EFI_ADDR(query_capsule_caps.capsule_headers),
+ count,
+ EFI_ADDR(query_capsule_caps.max_size),
+ EFI_ADDR(query_capsule_caps.reset_type));
+
+ *max_size = EFI_DATA(query_capsule_caps.max_size);
+ *reset_type = EFI_DATA(query_capsule_caps.reset_type);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+void efi_native_runtime_setup(void)
+{
+ efi.get_time = virt_efi_get_time;
+ efi.set_time = virt_efi_set_time;
+ efi.get_wakeup_time = virt_efi_get_wakeup_time;
+ efi.set_wakeup_time = virt_efi_set_wakeup_time;
+ efi.get_variable = virt_efi_get_variable;
+ efi.get_next_variable = virt_efi_get_next_variable;
+ efi.set_variable = virt_efi_set_variable;
+ efi.set_variable_nonblocking = virt_efi_set_variable_nonblocking;
+ efi.get_next_high_mono_count = virt_efi_get_next_high_mono_count;
+ efi.reset_system = virt_efi_reset_system;
+ efi.query_variable_info = virt_efi_query_variable_info;
+ efi.query_variable_info_nonblocking = virt_efi_query_variable_info_nonblocking;
+ efi.update_capsule = virt_efi_update_capsule;
+ efi.query_capsule_caps = virt_efi_query_capsule_caps;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/4] efi/arm64: marshall runtime services arguments via buffer in TTBR0
@ 2018-01-25 10:31 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-arm-kernel
If we want to unmap the kernel while executing UEFI runtime services,
we need to ensure that all arguments passed to those services are
valid during that time, and this includes pointer arguments to
string buffers and other data. So we have to do the rather tedious
job of replacing each generic runtime wrapper with one that takes the
above into account.
Fortunately, UEFI runtime services are not reentrant, so we can create
a static buffer with fields for each runtime service, and a separate
buffer for get/set_variable. This does limit variable names to 1024
characters and the variables themselves to 64 KB, but this should not
be a limitation in practice.
Note that capsule update is omitted for now - this will be addressed
in a subsequent patch.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/Kconfig | 1 -
arch/arm64/include/asm/efi.h | 5 +
arch/arm64/kernel/efi.c | 475 +++++++++++++++++++-
3 files changed, 479 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index de83b53c7e19..f2224566cc75 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1196,7 +1196,6 @@ config EFI
select LIBFDT
select UCS2_STRING
select EFI_PARAMS_FROM_FDT
- select EFI_RUNTIME_WRAPPERS
select EFI_STUB
select EFI_ARMSTUB
default y
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index b9b09a734719..b96bc4836c37 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -165,4 +165,9 @@ int __init efi_allocate_runtime_regions(struct mm_struct *mm);
#define EFI_CODE_BASE 0x200000
#define EFI_CODE_SIZE PAGE_SIZE
+#define EFI_VARBUFFER_BASE 0x210000
+#define EFI_VARBUFFER_SIZE SZ_64K
+
+#define EFI_DATA_BASE 0x220000
+
#endif /* _ASM_EFI_H */
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 68c920b2f4f0..cd23c20fba39 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -11,9 +11,15 @@
*
*/
+#define pr_fmt(fmt) "efi: " fmt
+
+#include <linux/bug.h>
#include <linux/efi.h>
#include <linux/init.h>
-
+#include <linux/irqflags.h>
+#include <linux/mutex.h>
+#include <linux/semaphore.h>
+#include <linux/stringify.h>
#include <asm/efi.h>
/*
@@ -136,6 +142,63 @@ bool on_efi_stack(unsigned long sp)
return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
}
+#define MAX_CAPSULES 16
+#define MAX_VARNAME_LEN 1024
+
+#define EFI_ADDR(arg) (&((typeof(__efi_rt_marshall_data)*)EFI_DATA_BASE)->arg)
+#define EFI_DATA(arg) (__efi_rt_marshall_data.arg)
+
+static union {
+ struct {
+ efi_time_t tm;
+ efi_time_cap_t tc;
+ } get_time;
+
+ struct {
+ efi_time_t tm;
+ } set_time;
+
+ struct {
+ efi_bool_t enabled;
+ efi_bool_t pending;
+ efi_time_t tm;
+ } get_wakeup_time;
+
+ struct {
+ efi_char16_t name[MAX_VARNAME_LEN];
+ efi_guid_t vendor;
+ u32 attr;
+ unsigned long data_size;
+ void *data;
+ } get_variable;
+
+ struct {
+ unsigned long name_size;
+ efi_char16_t name[MAX_VARNAME_LEN];
+ efi_guid_t vendor;
+ } get_next_variable;
+
+ struct {
+ u64 storage_space;
+ u64 remaining_space;
+ u64 max_variable_size;
+ } query_variable_info;
+
+ struct {
+ u32 count;
+ } get_next_high_mono_count;
+
+ struct {
+ efi_capsule_header_t capsules[MAX_CAPSULES];
+ efi_capsule_header_t *capsule_headers[MAX_CAPSULES];
+ u64 max_size;
+ int reset_type;
+ } query_capsule_caps;
+
+} __efi_rt_marshall_data __page_aligned_bss;
+
+static u8 __efi_rt_varbuffer[EFI_VARBUFFER_SIZE] __page_aligned_bss;
+
int __init efi_allocate_runtime_regions(struct mm_struct *mm)
{
static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
@@ -152,5 +215,415 @@ int __init efi_allocate_runtime_regions(struct mm_struct *mm)
__pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
false);
+ /* map the marshall data struct */
+ create_pgd_mapping(mm, __pa_symbol(&__efi_rt_marshall_data),
+ EFI_DATA_BASE, sizeof(__efi_rt_marshall_data),
+ __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+ false);
+
+ /* map the varbuffer */
+ create_pgd_mapping(mm, __pa_symbol(__efi_rt_varbuffer),
+ EFI_VARBUFFER_BASE, EFI_VARBUFFER_SIZE,
+ __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+ false);
+
return 0;
}
+
+/*
+ * Wrap around the new efi_call_virt_generic() macros so that the
+ * code doesn't get too cluttered:
+ */
+#define efi_call_virt(f, args...) \
+ efi_call_virt_pointer(efi.systab->runtime, f, args)
+#define __efi_call_virt(f, args...) \
+ __efi_call_virt_pointer(efi.systab->runtime, f, args)
+
+void efi_call_virt_check_flags(unsigned long flags, const char *call)
+{
+ unsigned long cur_flags, mismatch;
+
+ local_save_flags(cur_flags);
+
+ mismatch = flags ^ cur_flags;
+ if (!WARN_ON_ONCE(mismatch & ARCH_EFI_IRQ_FLAGS_MASK))
+ return;
+
+ add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_NOW_UNRELIABLE);
+ pr_err_ratelimited(FW_BUG "IRQ flags corrupted (0x%08lx=>0x%08lx) by EFI %s\n",
+ flags, cur_flags, call);
+ local_irq_restore(flags);
+}
+
+static int strlen16(efi_char16_t const *name)
+{
+ int ret = 0;
+
+ while (*name++)
+ ret++;
+
+ return ret;
+}
+
+static DEFINE_SEMAPHORE(efi_runtime_lock);
+
+static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
+{
+ efi_status_t status;
+
+ if (!tm)
+ return EFI_INVALID_PARAMETER;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+ status = efi_call_virt(get_time,
+ EFI_ADDR(get_time.tm),
+ tc ? EFI_ADDR(get_time.tc): NULL);
+
+ *tm = EFI_DATA(get_time.tm);
+ if (tc)
+ *tc = EFI_DATA(get_time.tc);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_set_time(efi_time_t *tm)
+{
+ efi_status_t status;
+
+ if (!tm)
+ return EFI_INVALID_PARAMETER;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ EFI_DATA(set_time.tm) = *tm;
+
+ status = efi_call_virt(set_time,
+ EFI_ADDR(set_time.tm));
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
+ efi_bool_t *pending,
+ efi_time_t *tm)
+{
+ efi_status_t status;
+
+ if (!enabled || !pending || !tm)
+ return EFI_INVALID_PARAMETER;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+ status = efi_call_virt(get_wakeup_time,
+ EFI_ADDR(get_wakeup_time.enabled),
+ EFI_ADDR(get_wakeup_time.pending),
+ EFI_ADDR(get_wakeup_time.tm));
+
+ *enabled = EFI_DATA(get_wakeup_time.enabled);
+ *pending = EFI_DATA(get_wakeup_time.pending);
+ *tm = EFI_DATA(get_wakeup_time.tm);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
+{
+ efi_status_t status;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ EFI_DATA(set_time.tm) = *tm;
+
+ status = efi_call_virt(set_wakeup_time, enabled,
+ EFI_ADDR(set_time.tm));
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_variable(efi_char16_t *name,
+ efi_guid_t *vendor,
+ u32 *attr,
+ unsigned long *data_size,
+ void *data)
+{
+ efi_status_t status;
+ int l = strlen16(name) + 1;
+
+ if (!name || !vendor || !data_size)
+ return EFI_INVALID_PARAMETER;
+
+ if (*data_size > EFI_VARBUFFER_SIZE || l > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ memcpy(EFI_DATA(get_variable.name), name, l * sizeof(efi_char16_t));
+ EFI_DATA(get_variable.vendor) = *vendor;
+ EFI_DATA(get_variable.data_size) = *data_size;
+
+ status = efi_call_virt(get_variable,
+ EFI_ADDR(get_variable.name),
+ EFI_ADDR(get_variable.vendor),
+ attr ? EFI_ADDR(get_variable.attr) : NULL,
+ EFI_ADDR(get_variable.data_size),
+ data ? (void *)EFI_VARBUFFER_BASE : NULL);
+
+ if (attr)
+ *attr = EFI_DATA(get_variable.attr);
+ *data_size = EFI_DATA(get_variable.data_size);
+ if (data)
+ memcpy(data, __efi_rt_varbuffer, *data_size);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
+ efi_char16_t *name,
+ efi_guid_t *vendor)
+{
+ efi_status_t status;
+
+ if (*name_size > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ EFI_DATA(get_next_variable.name_size) = *name_size;
+ memcpy(EFI_DATA(get_next_variable.name), name, *name_size);
+ EFI_DATA(get_next_variable.vendor) = *vendor;
+
+ status = efi_call_virt(get_next_variable,
+ EFI_ADDR(get_next_variable.name_size),
+ EFI_ADDR(get_next_variable.name),
+ EFI_ADDR(get_next_variable.vendor));
+
+ *name_size = EFI_DATA(get_next_variable.name_size);
+ memcpy(name, EFI_DATA(get_next_variable.name), *name_size);
+ *vendor = EFI_DATA(get_next_variable.vendor);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_set_variable(efi_char16_t *name,
+ efi_guid_t *vendor,
+ u32 attr,
+ unsigned long data_size,
+ void *data)
+{
+ efi_status_t status;
+ int l = strlen16(name) + 1;
+
+ if (!name || !vendor)
+ return EFI_INVALID_PARAMETER;
+
+ if (data_size > EFI_VARBUFFER_SIZE || l > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ memcpy(EFI_DATA(get_variable.name), name, l * sizeof(efi_char16_t));
+ EFI_DATA(get_variable.vendor) = *vendor;
+ if (data)
+ memcpy(__efi_rt_varbuffer, data, data_size);
+
+ status = efi_call_virt(set_variable,
+ EFI_ADDR(get_variable.name),
+ EFI_ADDR(get_variable.vendor),
+ attr,
+ data_size,
+ data ? (void *)EFI_VARBUFFER_BASE : NULL);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t
+virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor,
+ u32 attr, unsigned long data_size,
+ void *data)
+{
+ efi_status_t status;
+ int l = strlen16(name) + 1;
+
+ if (!name || !vendor)
+ return EFI_INVALID_PARAMETER;
+
+ if (data_size > EFI_VARBUFFER_SIZE || l > MAX_VARNAME_LEN)
+ return EFI_NOT_READY;
+
+ if (down_trylock(&efi_runtime_lock))
+ return EFI_NOT_READY;
+
+ memcpy(EFI_DATA(get_variable.name), name, l * sizeof(efi_char16_t));
+ EFI_DATA(get_variable.vendor) = *vendor;
+ if (data)
+ memcpy(__efi_rt_varbuffer, data, data_size);
+
+ status = efi_call_virt(set_variable,
+ EFI_ADDR(get_variable.name),
+ EFI_ADDR(get_variable.vendor),
+ attr,
+ data_size,
+ data ? (void *)EFI_VARBUFFER_BASE : NULL);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+
+static efi_status_t virt_efi_query_variable_info(u32 attr,
+ u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size)
+{
+ efi_status_t status;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ status = efi_call_virt(query_variable_info,
+ attr,
+ EFI_ADDR(query_variable_info.storage_space),
+ EFI_ADDR(query_variable_info.remaining_space),
+ EFI_ADDR(query_variable_info.max_variable_size));
+
+ *storage_space = EFI_DATA(query_variable_info.storage_space);
+ *remaining_space = EFI_DATA(query_variable_info.remaining_space);
+ *max_variable_size = EFI_DATA(query_variable_info.max_variable_size);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t
+virt_efi_query_variable_info_nonblocking(u32 attr,
+ u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size)
+{
+ efi_status_t status;
+
+ if (down_trylock(&efi_runtime_lock))
+ return EFI_NOT_READY;
+
+ status = efi_call_virt(query_variable_info,
+ attr,
+ EFI_ADDR(query_variable_info.storage_space),
+ EFI_ADDR(query_variable_info.remaining_space),
+ EFI_ADDR(query_variable_info.max_variable_size));
+
+ *storage_space = EFI_DATA(query_variable_info.storage_space);
+ *remaining_space = EFI_DATA(query_variable_info.remaining_space);
+ *max_variable_size = EFI_DATA(query_variable_info.max_variable_size);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
+{
+ efi_status_t status;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+ status = efi_call_virt(get_next_high_mono_count,
+ EFI_ADDR(get_next_high_mono_count.count));
+
+ *count = EFI_DATA(get_next_high_mono_count.count);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+static void virt_efi_reset_system(int reset_type,
+ efi_status_t status,
+ unsigned long data_size,
+ efi_char16_t *data)
+{
+ /* we don't use 'data' in the kernel */
+ pr_warn("efi_reset_system: ignoring 'data' argument\n");
+
+ if (down_interruptible(&efi_runtime_lock)) {
+ pr_warn("failed to invoke the reset_system() runtime service:\n"
+ "could not get exclusive access to the firmware\n");
+ return;
+ }
+ __efi_call_virt(reset_system, reset_type, status, 0, NULL);
+ up(&efi_runtime_lock);
+}
+
+static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
+ unsigned long count,
+ unsigned long sg_list)
+{
+// efi_status_t status;
+
+ return EFI_NOT_READY;
+
+// if (down_interruptible(&efi_runtime_lock))
+// return EFI_ABORTED;
+// status = efi_call_virt(update_capsule, capsules, count, sg_list);
+// up(&efi_runtime_lock);
+// return status;
+}
+
+static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules,
+ unsigned long count,
+ u64 *max_size,
+ int *reset_type)
+{
+ efi_status_t status;
+ int i;
+
+ if (count > MAX_CAPSULES)
+ return EFI_NOT_READY;
+
+ if (down_interruptible(&efi_runtime_lock))
+ return EFI_ABORTED;
+
+ for (i = 0; i < count; i++) {
+ EFI_DATA(query_capsule_caps.capsules[i]) = *capsules[i];
+ EFI_DATA(query_capsule_caps.capsule_headers[i]) =
+ EFI_ADDR(query_capsule_caps.capsules[i]);
+ }
+
+ status = efi_call_virt(query_capsule_caps,
+ EFI_ADDR(query_capsule_caps.capsule_headers),
+ count,
+ EFI_ADDR(query_capsule_caps.max_size),
+ EFI_ADDR(query_capsule_caps.reset_type));
+
+ *max_size = EFI_DATA(query_capsule_caps.max_size);
+ *reset_type = EFI_DATA(query_capsule_caps.reset_type);
+
+ up(&efi_runtime_lock);
+ return status;
+}
+
+void efi_native_runtime_setup(void)
+{
+ efi.get_time = virt_efi_get_time;
+ efi.set_time = virt_efi_set_time;
+ efi.get_wakeup_time = virt_efi_get_wakeup_time;
+ efi.set_wakeup_time = virt_efi_set_wakeup_time;
+ efi.get_variable = virt_efi_get_variable;
+ efi.get_next_variable = virt_efi_get_next_variable;
+ efi.set_variable = virt_efi_set_variable;
+ efi.set_variable_nonblocking = virt_efi_set_variable_nonblocking;
+ efi.get_next_high_mono_count = virt_efi_get_next_high_mono_count;
+ efi.reset_system = virt_efi_reset_system;
+ efi.query_variable_info = virt_efi_query_variable_info;
+ efi.query_variable_info_nonblocking = virt_efi_query_variable_info_nonblocking;
+ efi.update_capsule = virt_efi_update_capsule;
+ efi.query_capsule_caps = virt_efi_query_capsule_caps;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-25 10:31 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8
Cc: joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A, Ard Biesheuvel
Now that all UEFI runtime service wrappers ensure that byref
arguments are moved into the UEFI marshalling buffer (which
is not part of the kernel mapping), we can proceed and unmap
the kernel while UEFI runtime service calls are in progress.
This is done by setting the EPD1 bit and flushing the TLB of
the local CPU. This makes it independent of KPTI or whether
non-global mappings are being used.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm64/kernel/efi-rt-wrapper.S | 22 ++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
index 09e77e5edd94..70af90ef914c 100644
--- a/arch/arm64/kernel/efi-rt-wrapper.S
+++ b/arch/arm64/kernel/efi-rt-wrapper.S
@@ -9,6 +9,24 @@
#include <linux/linkage.h>
#include <asm/efi.h>
+ .macro sepd1, reg
+ mrs \reg, tcr_el1 // read Translation Control Reg
+ orr \reg, \reg, #1 << 23 // set EPD1 bit
+ msr tcr_el1, \reg // write back TCR
+ isb
+ tlbi vmalle1
+ dsb nsh
+ .endm
+
+ .macro cepd1, reg
+ mrs \reg, tcr_el1 // read Translation Control Reg
+ bic \reg, \reg, #1 << 23 // clear EPD1 bit
+ msr tcr_el1, \reg // write back TCR
+ isb
+ tlbi vmalle1
+ dsb nsh
+ .endm
+
.section ".rodata", "a"
.align PAGE_SHIFT
ENTRY(__efi_rt_asm_wrapper)
@@ -27,6 +45,7 @@ ENTRY(__efi_rt_asm_wrapper)
adr x1, __efi_rt_vectors
msr vbar_el1, x1
isb
+ sepd1 x1
/*
* We are lucky enough that no EFI runtime services take more than
@@ -46,6 +65,7 @@ ENTRY(__efi_rt_asm_wrapper)
ldr x1, 2f
msr vbar_el1, x1
isb
+ cepd1 x1
ldp x1, x2, [sp, #16]
cmp x2, x18
@@ -63,6 +83,7 @@ ENDPROC(__efi_rt_asm_wrapper)
.align 7
.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
mrs x29, elr_el1 // preserve ELR
+ cepd1 x30
adr x30, .Lret // take return address
msr elr_el1, x30 // set ELR to return address
ldr x30, 2b // take address of 'vectors'
@@ -76,6 +97,7 @@ ENDPROC(__efi_rt_asm_wrapper)
adr x30, __efi_rt_vectors
msr vbar_el1, x30
isb
+ sepd1 x30
ldp x29, x30, [sp], #16
eret
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
@ 2018-01-25 10:31 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-25 10:31 UTC (permalink / raw)
To: linux-arm-kernel
Now that all UEFI runtime service wrappers ensure that byref
arguments are moved into the UEFI marshalling buffer (which
is not part of the kernel mapping), we can proceed and unmap
the kernel while UEFI runtime service calls are in progress.
This is done by setting the EPD1 bit and flushing the TLB of
the local CPU. This makes it independent of KPTI or whether
non-global mappings are being used.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/efi-rt-wrapper.S | 22 ++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
index 09e77e5edd94..70af90ef914c 100644
--- a/arch/arm64/kernel/efi-rt-wrapper.S
+++ b/arch/arm64/kernel/efi-rt-wrapper.S
@@ -9,6 +9,24 @@
#include <linux/linkage.h>
#include <asm/efi.h>
+ .macro sepd1, reg
+ mrs \reg, tcr_el1 // read Translation Control Reg
+ orr \reg, \reg, #1 << 23 // set EPD1 bit
+ msr tcr_el1, \reg // write back TCR
+ isb
+ tlbi vmalle1
+ dsb nsh
+ .endm
+
+ .macro cepd1, reg
+ mrs \reg, tcr_el1 // read Translation Control Reg
+ bic \reg, \reg, #1 << 23 // clear EPD1 bit
+ msr tcr_el1, \reg // write back TCR
+ isb
+ tlbi vmalle1
+ dsb nsh
+ .endm
+
.section ".rodata", "a"
.align PAGE_SHIFT
ENTRY(__efi_rt_asm_wrapper)
@@ -27,6 +45,7 @@ ENTRY(__efi_rt_asm_wrapper)
adr x1, __efi_rt_vectors
msr vbar_el1, x1
isb
+ sepd1 x1
/*
* We are lucky enough that no EFI runtime services take more than
@@ -46,6 +65,7 @@ ENTRY(__efi_rt_asm_wrapper)
ldr x1, 2f
msr vbar_el1, x1
isb
+ cepd1 x1
ldp x1, x2, [sp, #16]
cmp x2, x18
@@ -63,6 +83,7 @@ ENDPROC(__efi_rt_asm_wrapper)
.align 7
.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
mrs x29, elr_el1 // preserve ELR
+ cepd1 x30
adr x30, .Lret // take return address
msr elr_el1, x30 // set ELR to return address
ldr x30, 2b // take address of 'vectors'
@@ -76,6 +97,7 @@ ENDPROC(__efi_rt_asm_wrapper)
adr x30, __efi_rt_vectors
msr vbar_el1, x30
isb
+ sepd1 x30
ldp x29, x30, [sp], #16
eret
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/4] efi: arm64: Check whether x18 is preserved by runtime services calls
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-26 15:05 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 15:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
marc.zyngier-5wv7dgnIgG8, joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A
On Thu, Jan 25, 2018 at 10:31:28AM +0000, Ard Biesheuvel wrote:
> Whether or not we will ever decide to start using x18 as a platform
> register in Linux is uncertain, but by that time, we will need to
> ensure that UEFI runtime services calls don't corrupt it. So let's
> start issuing warnings now for this, and increase the likelihood that
> these firmware images have all been replaced by that time.
>
> This has been fixed on the EDK2 side in commit 6d73863b5464
> ("BaseTools/tools_def AARCH64: mark register x18 as reserved").,
> dated July 13, 2017.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> arch/arm64/include/asm/efi.h | 4 +-
> arch/arm64/kernel/Makefile | 3 +-
> arch/arm64/kernel/efi-rt-wrapper.S | 41 ++++++++++++++++++++
> arch/arm64/kernel/efi.c | 6 +++
> 4 files changed, 52 insertions(+), 2 deletions(-)
Looks good to me:
Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Will
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 8389050328bb..192d791f1103 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -31,7 +31,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> ({ \
> efi_##f##_t *__f; \
> __f = p->f; \
> - __f(args); \
> + __efi_rt_asm_wrapper(__f, #f, args); \
> })
>
> #define arch_efi_call_virt_teardown() \
> @@ -40,6 +40,8 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> efi_virtmap_unload(); \
> })
>
> +efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
> +
> #define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
>
> /* arch specific definitions used by the stub code */
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index b87541360f43..6a4bd80c75bd 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -38,7 +38,8 @@ arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o
> arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> arm64-obj-$(CONFIG_KGDB) += kgdb.o
> -arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o
> +arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o \
> + efi-rt-wrapper.o
> arm64-obj-$(CONFIG_PCI) += pci.o
> arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
> arm64-obj-$(CONFIG_ACPI) += acpi.o
> diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
> new file mode 100644
> index 000000000000..05235ebb336d
> --- /dev/null
> +++ b/arch/arm64/kernel/efi-rt-wrapper.S
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (C) 2018 Linaro Ltd <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/linkage.h>
> +
> +ENTRY(__efi_rt_asm_wrapper)
> + stp x29, x30, [sp, #-32]!
> + mov x29, sp
> +
> + /*
> + * Register x18 is designated as the 'platform' register by the AAPCS,
> + * which means firmware running at the same exception level as the OS
> + * (such as UEFI) should never touch it.
> + */
> + stp x1, x18, [sp, #16]
> +
> + /*
> + * We are lucky enough that no EFI runtime services take more than
> + * 5 arguments, so all are passed in registers rather than via the
> + * stack.
> + */
> + mov x8, x0
> + mov x0, x2
> + mov x1, x3
> + mov x2, x4
> + mov x3, x5
> + mov x4, x6
> + blr x8
> +
> + ldp x1, x2, [sp, #16]
> + cmp x2, x18
> + ldp x29, x30, [sp], #32
> + b.ne 0f
> + ret
> +0: b efi_handle_corrupted_x18 // tail call
> +ENDPROC(__efi_rt_asm_wrapper)
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 82cd07592519..af4f943cffac 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -124,3 +124,9 @@ bool efi_poweroff_required(void)
> {
> return efi_enabled(EFI_RUNTIME_SERVICES);
> }
> +
> +asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> +{
> + pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
> + return s;
> +}
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/4] efi: arm64: Check whether x18 is preserved by runtime services calls
@ 2018-01-26 15:05 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 15:05 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 25, 2018 at 10:31:28AM +0000, Ard Biesheuvel wrote:
> Whether or not we will ever decide to start using x18 as a platform
> register in Linux is uncertain, but by that time, we will need to
> ensure that UEFI runtime services calls don't corrupt it. So let's
> start issuing warnings now for this, and increase the likelihood that
> these firmware images have all been replaced by that time.
>
> This has been fixed on the EDK2 side in commit 6d73863b5464
> ("BaseTools/tools_def AARCH64: mark register x18 as reserved").,
> dated July 13, 2017.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/include/asm/efi.h | 4 +-
> arch/arm64/kernel/Makefile | 3 +-
> arch/arm64/kernel/efi-rt-wrapper.S | 41 ++++++++++++++++++++
> arch/arm64/kernel/efi.c | 6 +++
> 4 files changed, 52 insertions(+), 2 deletions(-)
Looks good to me:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 8389050328bb..192d791f1103 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -31,7 +31,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> ({ \
> efi_##f##_t *__f; \
> __f = p->f; \
> - __f(args); \
> + __efi_rt_asm_wrapper(__f, #f, args); \
> })
>
> #define arch_efi_call_virt_teardown() \
> @@ -40,6 +40,8 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> efi_virtmap_unload(); \
> })
>
> +efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
> +
> #define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
>
> /* arch specific definitions used by the stub code */
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index b87541360f43..6a4bd80c75bd 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -38,7 +38,8 @@ arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o
> arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> arm64-obj-$(CONFIG_KGDB) += kgdb.o
> -arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o
> +arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o \
> + efi-rt-wrapper.o
> arm64-obj-$(CONFIG_PCI) += pci.o
> arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
> arm64-obj-$(CONFIG_ACPI) += acpi.o
> diff --git a/arch/arm64/kernel/efi-rt-wrapper.S b/arch/arm64/kernel/efi-rt-wrapper.S
> new file mode 100644
> index 000000000000..05235ebb336d
> --- /dev/null
> +++ b/arch/arm64/kernel/efi-rt-wrapper.S
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (C) 2018 Linaro Ltd <ard.biesheuvel@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/linkage.h>
> +
> +ENTRY(__efi_rt_asm_wrapper)
> + stp x29, x30, [sp, #-32]!
> + mov x29, sp
> +
> + /*
> + * Register x18 is designated as the 'platform' register by the AAPCS,
> + * which means firmware running at the same exception level as the OS
> + * (such as UEFI) should never touch it.
> + */
> + stp x1, x18, [sp, #16]
> +
> + /*
> + * We are lucky enough that no EFI runtime services take more than
> + * 5 arguments, so all are passed in registers rather than via the
> + * stack.
> + */
> + mov x8, x0
> + mov x0, x2
> + mov x1, x3
> + mov x2, x4
> + mov x3, x5
> + mov x4, x6
> + blr x8
> +
> + ldp x1, x2, [sp, #16]
> + cmp x2, x18
> + ldp x29, x30, [sp], #32
> + b.ne 0f
> + ret
> +0: b efi_handle_corrupted_x18 // tail call
> +ENDPROC(__efi_rt_asm_wrapper)
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 82cd07592519..af4f943cffac 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -124,3 +124,9 @@ bool efi_poweroff_required(void)
> {
> return efi_enabled(EFI_RUNTIME_SERVICES);
> }
> +
> +asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> +{
> + pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
> + return s;
> +}
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-26 16:57 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 16:57 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
marc.zyngier-5wv7dgnIgG8, joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A
Hi Ard,
On Thu, Jan 25, 2018 at 10:31:29AM +0000, Ard Biesheuvel wrote:
> As a preparatory step towards unmapping the kernel entirely while
> executing UEFI runtime services, move the stack and the entry
> wrapper routine mappings into the EFI page tables. Also, create a
> vector table that overrides the main one while executing in the
> firmware so we will be able to remap/unmap the kernel while taking
> interrupts.
[...]
> + .macro ventry
> + .align 7
> +.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
> + mrs x29, elr_el1 // preserve ELR
> + adr x30, .Lret // take return address
> + msr elr_el1, x30 // set ELR to return address
Oh man, are you really doing two ERETs for a single exception, or am I
missing something?
> + ldr x30, 2b // take address of 'vectors'
> + msr vbar_el1, x30 // set VBAR to 'vectors'
> + isb
> + add x30, x30, #.Lv\@ - __efi_rt_vectors
> + br x30
> + .endm
> +
> +.Lret: msr elr_el1, x29
If you take an IRQ here, aren't you toast?
> + adr x30, __efi_rt_vectors
> + msr vbar_el1, x30
> + isb
> + ldp x29, x30, [sp], #16
> + eret
> +
> + .align 11
> +__efi_rt_vectors:
> + .rept 8
> + ventry
> + .endr
Have you thought about SDEI at all? I can't see any code here to handle
that.
> + /*
> + * EFI runtime services never drop to EL0, so the
> + * remaining vector table entries are not needed.
> + */
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index af4f943cffac..68c920b2f4f0 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
> return s;
> }
> +
> +bool on_efi_stack(unsigned long sp)
> +{
> + return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
> +}
> +
> +int __init efi_allocate_runtime_regions(struct mm_struct *mm)
> +{
> + static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
Probably just me, but the use of a function static variable in a function
annotated with __init just makes me feel uneasy. Could we move it out into
wider scope?
> +
> + /* map the stack */
> + create_pgd_mapping(mm, __pa_symbol(stack),
> + EFI_STACK_BASE, EFI_STACK_SIZE,
> + __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
> + false);
> +
> + /* map the runtime wrapper pivot function */
> + create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
> + EFI_CODE_BASE, EFI_CODE_SIZE,
> + __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
> + false);
> +
> + return 0;
> +}
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index b34e717d7597..3bab6c60a12b 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
> alternative_else_nop_endif
>
> .if \el != 0
> + tbz x21, #63, 1f // skip if TTBR0 covers the stack
So this is really a "detect EFI" check, right? Maybe comment it as such.
Also, probably want to check bit 55 just in case tagging ever takes off.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
@ 2018-01-26 16:57 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 16:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On Thu, Jan 25, 2018 at 10:31:29AM +0000, Ard Biesheuvel wrote:
> As a preparatory step towards unmapping the kernel entirely while
> executing UEFI runtime services, move the stack and the entry
> wrapper routine mappings into the EFI page tables. Also, create a
> vector table that overrides the main one while executing in the
> firmware so we will be able to remap/unmap the kernel while taking
> interrupts.
[...]
> + .macro ventry
> + .align 7
> +.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
> + mrs x29, elr_el1 // preserve ELR
> + adr x30, .Lret // take return address
> + msr elr_el1, x30 // set ELR to return address
Oh man, are you really doing two ERETs for a single exception, or am I
missing something?
> + ldr x30, 2b // take address of 'vectors'
> + msr vbar_el1, x30 // set VBAR to 'vectors'
> + isb
> + add x30, x30, #.Lv\@ - __efi_rt_vectors
> + br x30
> + .endm
> +
> +.Lret: msr elr_el1, x29
If you take an IRQ here, aren't you toast?
> + adr x30, __efi_rt_vectors
> + msr vbar_el1, x30
> + isb
> + ldp x29, x30, [sp], #16
> + eret
> +
> + .align 11
> +__efi_rt_vectors:
> + .rept 8
> + ventry
> + .endr
Have you thought about SDEI at all? I can't see any code here to handle
that.
> + /*
> + * EFI runtime services never drop to EL0, so the
> + * remaining vector table entries are not needed.
> + */
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index af4f943cffac..68c920b2f4f0 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
> return s;
> }
> +
> +bool on_efi_stack(unsigned long sp)
> +{
> + return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
> +}
> +
> +int __init efi_allocate_runtime_regions(struct mm_struct *mm)
> +{
> + static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
Probably just me, but the use of a function static variable in a function
annotated with __init just makes me feel uneasy. Could we move it out into
wider scope?
> +
> + /* map the stack */
> + create_pgd_mapping(mm, __pa_symbol(stack),
> + EFI_STACK_BASE, EFI_STACK_SIZE,
> + __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
> + false);
> +
> + /* map the runtime wrapper pivot function */
> + create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
> + EFI_CODE_BASE, EFI_CODE_SIZE,
> + __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
> + false);
> +
> + return 0;
> +}
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index b34e717d7597..3bab6c60a12b 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
> alternative_else_nop_endif
>
> .if \el != 0
> + tbz x21, #63, 1f // skip if TTBR0 covers the stack
So this is really a "detect EFI" check, right? Maybe comment it as such.
Also, probably want to check bit 55 just in case tagging ever takes off.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
2018-01-26 16:57 ` Will Deacon
@ 2018-01-26 17:03 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-26 17:03 UTC (permalink / raw)
To: Will Deacon
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
Catalin Marinas, Mark Rutland, Marc Zyngier, Joakim Bech,
Leif Lindholm, Graeme Gregory
On 26 January 2018 at 16:57, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi Ard,
>
> On Thu, Jan 25, 2018 at 10:31:29AM +0000, Ard Biesheuvel wrote:
>> As a preparatory step towards unmapping the kernel entirely while
>> executing UEFI runtime services, move the stack and the entry
>> wrapper routine mappings into the EFI page tables. Also, create a
>> vector table that overrides the main one while executing in the
>> firmware so we will be able to remap/unmap the kernel while taking
>> interrupts.
>
> [...]
>
>> + .macro ventry
>> + .align 7
>> +.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
>> + mrs x29, elr_el1 // preserve ELR
>> + adr x30, .Lret // take return address
>> + msr elr_el1, x30 // set ELR to return address
>
> Oh man, are you really doing two ERETs for a single exception, or am I
> missing something?
>
Yes. I told you it was poetry.
>> + ldr x30, 2b // take address of 'vectors'
>> + msr vbar_el1, x30 // set VBAR to 'vectors'
>> + isb
>> + add x30, x30, #.Lv\@ - __efi_rt_vectors
>> + br x30
>> + .endm
>> +
>> +.Lret: msr elr_el1, x29
>
> If you take an IRQ here, aren't you toast?
>
Yep. So we need to switch this with setting the VBAR below then.
>> + adr x30, __efi_rt_vectors
>> + msr vbar_el1, x30
>> + isb
>> + ldp x29, x30, [sp], #16
>> + eret
>> +
>> + .align 11
>> +__efi_rt_vectors:
>> + .rept 8
>> + ventry
>> + .endr
>
> Have you thought about SDEI at all? I can't see any code here to handle
> that.
>
Nope
>> + /*
>> + * EFI runtime services never drop to EL0, so the
>> + * remaining vector table entries are not needed.
>> + */
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index af4f943cffac..68c920b2f4f0 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
>> pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
>> return s;
>> }
>> +
>> +bool on_efi_stack(unsigned long sp)
>> +{
>> + return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
>> +}
>> +
>> +int __init efi_allocate_runtime_regions(struct mm_struct *mm)
>> +{
>> + static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
>
> Probably just me, but the use of a function static variable in a function
> annotated with __init just makes me feel uneasy. Could we move it out into
> wider scope?
>
Should be fine, but I don't mind moving it out.
>> +
>> + /* map the stack */
>> + create_pgd_mapping(mm, __pa_symbol(stack),
>> + EFI_STACK_BASE, EFI_STACK_SIZE,
>> + __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
>> + false);
>> +
>> + /* map the runtime wrapper pivot function */
>> + create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
>> + EFI_CODE_BASE, EFI_CODE_SIZE,
>> + __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
>> + false);
>> +
>> + return 0;
>> +}
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index b34e717d7597..3bab6c60a12b 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
>> alternative_else_nop_endif
>>
>> .if \el != 0
>> + tbz x21, #63, 1f // skip if TTBR0 covers the stack
>
> So this is really a "detect EFI" check, right? Maybe comment it as such.
> Also, probably want to check bit 55 just in case tagging ever takes off.
>
Right. So just bit #55 should be sufficient then, right?
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
@ 2018-01-26 17:03 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-26 17:03 UTC (permalink / raw)
To: linux-arm-kernel
On 26 January 2018 at 16:57, Will Deacon <will.deacon@arm.com> wrote:
> Hi Ard,
>
> On Thu, Jan 25, 2018 at 10:31:29AM +0000, Ard Biesheuvel wrote:
>> As a preparatory step towards unmapping the kernel entirely while
>> executing UEFI runtime services, move the stack and the entry
>> wrapper routine mappings into the EFI page tables. Also, create a
>> vector table that overrides the main one while executing in the
>> firmware so we will be able to remap/unmap the kernel while taking
>> interrupts.
>
> [...]
>
>> + .macro ventry
>> + .align 7
>> +.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
>> + mrs x29, elr_el1 // preserve ELR
>> + adr x30, .Lret // take return address
>> + msr elr_el1, x30 // set ELR to return address
>
> Oh man, are you really doing two ERETs for a single exception, or am I
> missing something?
>
Yes. I told you it was poetry.
>> + ldr x30, 2b // take address of 'vectors'
>> + msr vbar_el1, x30 // set VBAR to 'vectors'
>> + isb
>> + add x30, x30, #.Lv\@ - __efi_rt_vectors
>> + br x30
>> + .endm
>> +
>> +.Lret: msr elr_el1, x29
>
> If you take an IRQ here, aren't you toast?
>
Yep. So we need to switch this with setting the VBAR below then.
>> + adr x30, __efi_rt_vectors
>> + msr vbar_el1, x30
>> + isb
>> + ldp x29, x30, [sp], #16
>> + eret
>> +
>> + .align 11
>> +__efi_rt_vectors:
>> + .rept 8
>> + ventry
>> + .endr
>
> Have you thought about SDEI at all? I can't see any code here to handle
> that.
>
Nope
>> + /*
>> + * EFI runtime services never drop to EL0, so the
>> + * remaining vector table entries are not needed.
>> + */
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index af4f943cffac..68c920b2f4f0 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
>> pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
>> return s;
>> }
>> +
>> +bool on_efi_stack(unsigned long sp)
>> +{
>> + return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
>> +}
>> +
>> +int __init efi_allocate_runtime_regions(struct mm_struct *mm)
>> +{
>> + static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;
>
> Probably just me, but the use of a function static variable in a function
> annotated with __init just makes me feel uneasy. Could we move it out into
> wider scope?
>
Should be fine, but I don't mind moving it out.
>> +
>> + /* map the stack */
>> + create_pgd_mapping(mm, __pa_symbol(stack),
>> + EFI_STACK_BASE, EFI_STACK_SIZE,
>> + __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
>> + false);
>> +
>> + /* map the runtime wrapper pivot function */
>> + create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
>> + EFI_CODE_BASE, EFI_CODE_SIZE,
>> + __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
>> + false);
>> +
>> + return 0;
>> +}
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index b34e717d7597..3bab6c60a12b 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
>> alternative_else_nop_endif
>>
>> .if \el != 0
>> + tbz x21, #63, 1f // skip if TTBR0 covers the stack
>
> So this is really a "detect EFI" check, right? Maybe comment it as such.
> Also, probably want to check bit 55 just in case tagging ever takes off.
>
Right. So just bit #55 should be sufficient then, right?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-26 17:05 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 17:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
marc.zyngier-5wv7dgnIgG8, joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A
On Thu, Jan 25, 2018 at 10:31:31AM +0000, Ard Biesheuvel wrote:
> Now that all UEFI runtime service wrappers ensure that byref
> arguments are moved into the UEFI marshalling buffer (which
> is not part of the kernel mapping), we can proceed and unmap
> the kernel while UEFI runtime service calls are in progress.
>
> This is done by setting the EPD1 bit and flushing the TLB of
> the local CPU. This makes it independent of KPTI or whether
> non-global mappings are being used.
One snag with this is that it will break SPE, so I'd prefer this behaviour
to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check
in drivers/perf/arm_spe_pmu.c remains valid.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
@ 2018-01-26 17:05 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 17:05 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 25, 2018 at 10:31:31AM +0000, Ard Biesheuvel wrote:
> Now that all UEFI runtime service wrappers ensure that byref
> arguments are moved into the UEFI marshalling buffer (which
> is not part of the kernel mapping), we can proceed and unmap
> the kernel while UEFI runtime service calls are in progress.
>
> This is done by setting the EPD1 bit and flushing the TLB of
> the local CPU. This makes it independent of KPTI or whether
> non-global mappings are being used.
One snag with this is that it will break SPE, so I'd prefer this behaviour
to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check
in drivers/perf/arm_spe_pmu.c remains valid.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
2018-01-26 17:05 ` Will Deacon
@ 2018-01-26 17:06 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-26 17:06 UTC (permalink / raw)
To: Will Deacon
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
Catalin Marinas, Mark Rutland, Marc Zyngier, Joakim Bech,
Leif Lindholm, Graeme Gregory
On 26 January 2018 at 17:05, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Thu, Jan 25, 2018 at 10:31:31AM +0000, Ard Biesheuvel wrote:
>> Now that all UEFI runtime service wrappers ensure that byref
>> arguments are moved into the UEFI marshalling buffer (which
>> is not part of the kernel mapping), we can proceed and unmap
>> the kernel while UEFI runtime service calls are in progress.
>>
>> This is done by setting the EPD1 bit and flushing the TLB of
>> the local CPU. This makes it independent of KPTI or whether
>> non-global mappings are being used.
>
> One snag with this is that it will break SPE, so I'd prefer this behaviour
> to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check
> in drivers/perf/arm_spe_pmu.c remains valid.
>
The problem with that is that they serve two different purposes: kpti
protects against meltdown, this protects against Spectre variant 1.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
@ 2018-01-26 17:06 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-26 17:06 UTC (permalink / raw)
To: linux-arm-kernel
On 26 January 2018 at 17:05, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Jan 25, 2018 at 10:31:31AM +0000, Ard Biesheuvel wrote:
>> Now that all UEFI runtime service wrappers ensure that byref
>> arguments are moved into the UEFI marshalling buffer (which
>> is not part of the kernel mapping), we can proceed and unmap
>> the kernel while UEFI runtime service calls are in progress.
>>
>> This is done by setting the EPD1 bit and flushing the TLB of
>> the local CPU. This makes it independent of KPTI or whether
>> non-global mappings are being used.
>
> One snag with this is that it will break SPE, so I'd prefer this behaviour
> to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check
> in drivers/perf/arm_spe_pmu.c remains valid.
>
The problem with that is that they serve two different purposes: kpti
protects against meltdown, this protects against Spectre variant 1.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
2018-01-26 17:03 ` Ard Biesheuvel
@ 2018-01-26 17:09 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 17:09 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
Catalin Marinas, Mark Rutland, Marc Zyngier, Joakim Bech,
Leif Lindholm, Graeme Gregory
On Fri, Jan 26, 2018 at 05:03:29PM +0000, Ard Biesheuvel wrote:
> On 26 January 2018 at 16:57, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Thu, Jan 25, 2018 at 10:31:29AM +0000, Ard Biesheuvel wrote:
> >> As a preparatory step towards unmapping the kernel entirely while
> >> executing UEFI runtime services, move the stack and the entry
> >> wrapper routine mappings into the EFI page tables. Also, create a
> >> vector table that overrides the main one while executing in the
> >> firmware so we will be able to remap/unmap the kernel while taking
> >> interrupts.
> >
> > [...]
> >
> >> + .macro ventry
> >> + .align 7
> >> +.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
> >> + mrs x29, elr_el1 // preserve ELR
> >> + adr x30, .Lret // take return address
> >> + msr elr_el1, x30 // set ELR to return address
> >
> > Oh man, are you really doing two ERETs for a single exception, or am I
> > missing something?
> >
>
> Yes. I told you it was poetry.
We should organise a recital.
> >> + ldr x30, 2b // take address of 'vectors'
> >> + msr vbar_el1, x30 // set VBAR to 'vectors'
> >> + isb
> >> + add x30, x30, #.Lv\@ - __efi_rt_vectors
> >> + br x30
> >> + .endm
> >> +
> >> +.Lret: msr elr_el1, x29
> >
> > If you take an IRQ here, aren't you toast?
> >
>
> Yep. So we need to switch this with setting the VBAR below then.
Hmm, but the ELR will still be clobbered by an IRQ, so I don't see how you
can make this safe unless you hack SPSR before entering the kernel vectors
on the entry side.
> >> +__efi_rt_vectors:
> >> + .rept 8
> >> + ventry
> >> + .endr
> >
> > Have you thought about SDEI at all? I can't see any code here to handle
> > that.
> >
>
> Nope
Add JamesM to cc ;)
> >> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> >> index b34e717d7597..3bab6c60a12b 100644
> >> --- a/arch/arm64/kernel/entry.S
> >> +++ b/arch/arm64/kernel/entry.S
> >> @@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
> >> alternative_else_nop_endif
> >>
> >> .if \el != 0
> >> + tbz x21, #63, 1f // skip if TTBR0 covers the stack
> >
> > So this is really a "detect EFI" check, right? Maybe comment it as such.
> > Also, probably want to check bit 55 just in case tagging ever takes off.
> >
>
> Right. So just bit #55 should be sufficient then, right?
Yes, I think so.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables
@ 2018-01-26 17:09 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 17:09 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 26, 2018 at 05:03:29PM +0000, Ard Biesheuvel wrote:
> On 26 January 2018 at 16:57, Will Deacon <will.deacon@arm.com> wrote:
> > On Thu, Jan 25, 2018 at 10:31:29AM +0000, Ard Biesheuvel wrote:
> >> As a preparatory step towards unmapping the kernel entirely while
> >> executing UEFI runtime services, move the stack and the entry
> >> wrapper routine mappings into the EFI page tables. Also, create a
> >> vector table that overrides the main one while executing in the
> >> firmware so we will be able to remap/unmap the kernel while taking
> >> interrupts.
> >
> > [...]
> >
> >> + .macro ventry
> >> + .align 7
> >> +.Lv\@ : stp x29, x30, [sp, #-16]! // preserve x29 and x30
> >> + mrs x29, elr_el1 // preserve ELR
> >> + adr x30, .Lret // take return address
> >> + msr elr_el1, x30 // set ELR to return address
> >
> > Oh man, are you really doing two ERETs for a single exception, or am I
> > missing something?
> >
>
> Yes. I told you it was poetry.
We should organise a recital.
> >> + ldr x30, 2b // take address of 'vectors'
> >> + msr vbar_el1, x30 // set VBAR to 'vectors'
> >> + isb
> >> + add x30, x30, #.Lv\@ - __efi_rt_vectors
> >> + br x30
> >> + .endm
> >> +
> >> +.Lret: msr elr_el1, x29
> >
> > If you take an IRQ here, aren't you toast?
> >
>
> Yep. So we need to switch this with setting the VBAR below then.
Hmm, but the ELR will still be clobbered by an IRQ, so I don't see how you
can make this safe unless you hack SPSR before entering the kernel vectors
on the entry side.
> >> +__efi_rt_vectors:
> >> + .rept 8
> >> + ventry
> >> + .endr
> >
> > Have you thought about SDEI at all? I can't see any code here to handle
> > that.
> >
>
> Nope
Add JamesM to cc ;)
> >> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> >> index b34e717d7597..3bab6c60a12b 100644
> >> --- a/arch/arm64/kernel/entry.S
> >> +++ b/arch/arm64/kernel/entry.S
> >> @@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
> >> alternative_else_nop_endif
> >>
> >> .if \el != 0
> >> + tbz x21, #63, 1f // skip if TTBR0 covers the stack
> >
> > So this is really a "detect EFI" check, right? Maybe comment it as such.
> > Also, probably want to check bit 55 just in case tagging ever takes off.
> >
>
> Right. So just bit #55 should be sufficient then, right?
Yes, I think so.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
2018-01-26 17:06 ` Ard Biesheuvel
@ 2018-01-26 17:10 ` Will Deacon
-1 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 17:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
Catalin Marinas, Mark Rutland, Marc Zyngier, Joakim Bech,
Leif Lindholm, Graeme Gregory
On Fri, Jan 26, 2018 at 05:06:42PM +0000, Ard Biesheuvel wrote:
> On 26 January 2018 at 17:05, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Thu, Jan 25, 2018 at 10:31:31AM +0000, Ard Biesheuvel wrote:
> >> Now that all UEFI runtime service wrappers ensure that byref
> >> arguments are moved into the UEFI marshalling buffer (which
> >> is not part of the kernel mapping), we can proceed and unmap
> >> the kernel while UEFI runtime service calls are in progress.
> >>
> >> This is done by setting the EPD1 bit and flushing the TLB of
> >> the local CPU. This makes it independent of KPTI or whether
> >> non-global mappings are being used.
> >
> > One snag with this is that it will break SPE, so I'd prefer this behaviour
> > to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check
> > in drivers/perf/arm_spe_pmu.c remains valid.
> >
>
> The problem with that is that they serve two different purposes: kpti
> protects against meltdown, this protects against Spectre variant 1.
Fair enough, but we should do something because it renders SPE unusable
and it can be a really handy profiling feature. Having the new EFI behaviour
optional in some way would be my preference.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services
@ 2018-01-26 17:10 ` Will Deacon
0 siblings, 0 replies; 28+ messages in thread
From: Will Deacon @ 2018-01-26 17:10 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 26, 2018 at 05:06:42PM +0000, Ard Biesheuvel wrote:
> On 26 January 2018 at 17:05, Will Deacon <will.deacon@arm.com> wrote:
> > On Thu, Jan 25, 2018 at 10:31:31AM +0000, Ard Biesheuvel wrote:
> >> Now that all UEFI runtime service wrappers ensure that byref
> >> arguments are moved into the UEFI marshalling buffer (which
> >> is not part of the kernel mapping), we can proceed and unmap
> >> the kernel while UEFI runtime service calls are in progress.
> >>
> >> This is done by setting the EPD1 bit and flushing the TLB of
> >> the local CPU. This makes it independent of KPTI or whether
> >> non-global mappings are being used.
> >
> > One snag with this is that it will break SPE, so I'd prefer this behaviour
> > to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check
> > in drivers/perf/arm_spe_pmu.c remains valid.
> >
>
> The problem with that is that they serve two different purposes: kpti
> protects against meltdown, this protects against Spectre variant 1.
Fair enough, but we should do something because it renders SPE unusable
and it can be a really handy profiling feature. Having the new EFI behaviour
optional in some way would be my preference.
Will
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls
2018-01-25 10:31 ` Ard Biesheuvel
@ 2018-01-29 14:51 ` Jeffrey Hugo
-1 siblings, 0 replies; 28+ messages in thread
From: Jeffrey Hugo @ 2018-01-29 14:51 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8
Cc: joakim.bech-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A
On 1/25/2018 3:31 AM, Ard Biesheuvel wrote:
> Note that capsule update has been omitted. This is a bit involved, and
> I'd rather get some feedback before burning too many cycles on that. All
> other services should be functional, with the caveat that EFI variable
> names are now limited to 1024 [wide] characters, and UEFI variables
> themselves to 64 KB.
>
I'm curious, why these limitations?
We don't have any concern about the name length limitation at this time,
but the variable size one is concerning.
We have a feature which creates a variable greater than 64KB that we
want to utilize from Linux. The size limitation would seem to prevent that.
--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls
@ 2018-01-29 14:51 ` Jeffrey Hugo
0 siblings, 0 replies; 28+ messages in thread
From: Jeffrey Hugo @ 2018-01-29 14:51 UTC (permalink / raw)
To: linux-arm-kernel
On 1/25/2018 3:31 AM, Ard Biesheuvel wrote:
> Note that capsule update has been omitted. This is a bit involved, and
> I'd rather get some feedback before burning too many cycles on that. All
> other services should be functional, with the caveat that EFI variable
> names are now limited to 1024 [wide] characters, and UEFI variables
> themselves to 64 KB.
>
I'm curious, why these limitations?
We don't have any concern about the name length limitation at this time,
but the variable size one is concerning.
We have a feature which creates a variable greater than 64KB that we
want to utilize from Linux. The size limitation would seem to prevent that.
--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls
2018-01-29 14:51 ` Jeffrey Hugo
@ 2018-01-29 14:55 ` Ard Biesheuvel
-1 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-29 14:55 UTC (permalink / raw)
To: Jeffrey Hugo
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, Will Deacon,
Catalin Marinas, Mark Rutland, Marc Zyngier, Joakim Bech,
Leif Lindholm, Graeme Gregory
On 29 January 2018 at 14:51, Jeffrey Hugo <jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 1/25/2018 3:31 AM, Ard Biesheuvel wrote:
>
>> Note that capsule update has been omitted. This is a bit involved, and
>> I'd rather get some feedback before burning too many cycles on that. All
>> other services should be functional, with the caveat that EFI variable
>> names are now limited to 1024 [wide] characters, and UEFI variables
>> themselves to 64 KB.
>>
>
> I'm curious, why these limitations?
>
Because mapping and unmapping things on the fly in the UEFI page
tables is a bit complicated, although I need to do something of the
kind for capsules anyway, so once I solve that, we can use the same
approach for variables.
> We don't have any concern about the name length limitation at this time, but
> the variable size one is concerning.
>
> We have a feature which creates a variable greater than 64KB that we want to
> utilize from Linux. The size limitation would seem to prevent that.
>
Well, the limit itself is rather arbitrary, and putting >64 KB into
the fault tolerant, write ahead logging NOR flash seems mildly
inappropriate to me. But you are right, the spec currently has no such
limitations, so ideally, nor should the OS.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls
@ 2018-01-29 14:55 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-01-29 14:55 UTC (permalink / raw)
To: linux-arm-kernel
On 29 January 2018 at 14:51, Jeffrey Hugo <jhugo@codeaurora.org> wrote:
> On 1/25/2018 3:31 AM, Ard Biesheuvel wrote:
>
>> Note that capsule update has been omitted. This is a bit involved, and
>> I'd rather get some feedback before burning too many cycles on that. All
>> other services should be functional, with the caveat that EFI variable
>> names are now limited to 1024 [wide] characters, and UEFI variables
>> themselves to 64 KB.
>>
>
> I'm curious, why these limitations?
>
Because mapping and unmapping things on the fly in the UEFI page
tables is a bit complicated, although I need to do something of the
kind for capsules anyway, so once I solve that, we can use the same
approach for variables.
> We don't have any concern about the name length limitation at this time, but
> the variable size one is concerning.
>
> We have a feature which creates a variable greater than 64KB that we want to
> utilize from Linux. The size limitation would seem to prevent that.
>
Well, the limit itself is rather arbitrary, and putting >64 KB into
the fault tolerant, write ahead logging NOR flash seems mildly
inappropriate to me. But you are right, the spec currently has no such
limitations, so ideally, nor should the OS.
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2018-01-29 14:55 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 10:31 [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls Ard Biesheuvel
2018-01-25 10:31 ` Ard Biesheuvel
[not found] ` <20180125103131.19168-1-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-25 10:31 ` [PATCH 1/4] efi: arm64: Check whether x18 is preserved by runtime services calls Ard Biesheuvel
2018-01-25 10:31 ` Ard Biesheuvel
[not found] ` <20180125103131.19168-2-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-26 15:05 ` Will Deacon
2018-01-26 15:05 ` Will Deacon
2018-01-25 10:31 ` [PATCH 2/4] efi/arm64: map the stack and entry wrapper into the UEFI page tables Ard Biesheuvel
2018-01-25 10:31 ` Ard Biesheuvel
[not found] ` <20180125103131.19168-3-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-26 16:57 ` Will Deacon
2018-01-26 16:57 ` Will Deacon
[not found] ` <20180126165744.GD16008-5wv7dgnIgG8@public.gmane.org>
2018-01-26 17:03 ` Ard Biesheuvel
2018-01-26 17:03 ` Ard Biesheuvel
[not found] ` <CAKv+Gu8CvZXZvxp75x5JL0dNMvEau5XuXxAObsJURs6ALU+nFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-26 17:09 ` Will Deacon
2018-01-26 17:09 ` Will Deacon
2018-01-25 10:31 ` [PATCH 3/4] efi/arm64: marshall runtime services arguments via buffer in TTBR0 Ard Biesheuvel
2018-01-25 10:31 ` Ard Biesheuvel
2018-01-25 10:31 ` [PATCH 4/4] efi/arm64: unmap the kernel while executing UEFI services Ard Biesheuvel
2018-01-25 10:31 ` Ard Biesheuvel
[not found] ` <20180125103131.19168-5-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-26 17:05 ` Will Deacon
2018-01-26 17:05 ` Will Deacon
[not found] ` <20180126170505.GE16008-5wv7dgnIgG8@public.gmane.org>
2018-01-26 17:06 ` Ard Biesheuvel
2018-01-26 17:06 ` Ard Biesheuvel
[not found] ` <CAKv+Gu84Dk1ZRicHNWr2SLn1GEpaj1YmekzgjvK4aC5t05TgSw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-26 17:10 ` Will Deacon
2018-01-26 17:10 ` Will Deacon
2018-01-29 14:51 ` [PATCH 0/4] efi/arm64: unmap the kernel during runtime service calls Jeffrey Hugo
2018-01-29 14:51 ` Jeffrey Hugo
[not found] ` <bdba264b-6619-6e9f-2849-d9d82a820ab0-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-01-29 14:55 ` Ard Biesheuvel
2018-01-29 14:55 ` Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.