[PATCH v2 3/9] arm64: factor out EL1 SSBS emulation hook

Mark Rutland
2022-10-19T15:04:52Z

Currently call_undef_hook() is used to handle UNDEFINED exceptions from
EL0 and EL1. As support for deprecated instructions may be enabled
independently, the handlers for individual instructions are organised as
a linked list of struct undef_hook which can be manipulated dynamically.
As this can be manipulated dynamically, the list is protected with a
raw_spinlock which must be acquired when handling UNDEFINED exceptions
or when manipulating the list of handlers.

This locking is unfortunate as it serialises handling of UNDEFINED
exceptions, and requires RCU to be enabled for lockdep, requiring the
use of RCU_NONIDLE() in resume path of cpu_suspend() since commit:

a2c42bbabbe260b7 ("arm64: spectre: Prevent lockdep splat on v4 mitigation enable path")

The list of UNDEFINED handlers largely consist of handlers for
exceptions taken from EL0, and the only handler for exceptions taken
from EL1 handles `MSR SSBS, #imm` on CPUs which feature PSTATE.SSBS but
lack the corresponding MSR (Immediate) instruction. Other than this we
never expect to take an UNDEFINED exception from EL1 in normal
operation.

This patch reworks do_el0_undef() to invoke the EL1 SSBS handler
directly, relegating call_undef_hook() to only handle EL0 UNDEFs. This
removes redundant work to iterate the list for EL1 UNDEFs, and removes
the need for locking, permitting EL1 UNDEFs to be handled in parallel
without contention.

The RCU_NONIDLE() call in cpu_suspend() will be removed in a subsequent
patch, as there are other potential issues with the use of
instrumentable code and RCU in the CPU suspend code.

I've tested this by forcing the detection of SSBS on a CPU that doesn't
have it, and verifying that the try_emulate_el1_ssbs() callback is
invoked.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/spectre.h | 2 ++
arch/arm64/kernel/proton-pack.c | 26 +++++++-------------------
arch/arm64/kernel/traps.c | 15 ++++++++-------
3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/arch/arm64/include/asm/spectre.h b/arch/arm64/include/asm/spectre.h
index aa3d3607d5c8..db7b371b367c 100644
--- a/arch/arm64/include/asm/spectre.h
+++ b/arch/arm64/include/asm/spectre.h
@@ -26,6 +26,7 @@ enum mitigation_state {
SPECTRE_VULNERABLE,
};

+struct pt_regs;
struct task_struct;

/*
@@ -98,5 +99,6 @@ enum mitigation_state arm64_get_spectre_bhb_state(void);
bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry, int scope);
u8 spectre_bhb_loop_affected(int scope);
void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *__unused);
+bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr);
#endif /* __ASSEMBLY__ */
#endif /* __ASM_SPECTRE_H */
diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
index bfce41c2a53b..fca9cc6f5581 100644
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -521,10 +521,13 @@ bool has_spectre_v4(const struct arm64_cpu_capabilities *cap, int scope)
return state != SPECTRE_UNAFFECTED;
}

-static int ssbs_emulation_handler(struct pt_regs *regs, u32 instr)
+bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr)
{
- if (user_mode(regs))
- return 1;
+ const u32 instr_mask = ~(1U << PSTATE_Imm_shift);
+ const u32 instr_val = 0xd500401f | PSTATE_SSBS;
+
+ if ((instr & instr_mask) != instr_val)
+ return false;

if (instr & BIT(PSTATE_Imm_shift))
regs->pstate |= PSR_SSBS_BIT;
@@ -532,19 +535,11 @@ static int ssbs_emulation_handler(struct pt_regs *regs, u32 instr)
regs->pstate &= ~PSR_SSBS_BIT;

arm64_skip_faulting_instruction(regs, 4);
- return 0;
+ return true;
}

-static struct undef_hook ssbs_emulation_hook = {
- .instr_mask = ~(1U << PSTATE_Imm_shift),
- .instr_val = 0xd500401f | PSTATE_SSBS,
- .fn = ssbs_emulation_handler,
-};
-
static enum mitigation_state spectre_v4_enable_hw_mitigation(void)
{
- static bool undef_hook_registered = false;
- static DEFINE_RAW_SPINLOCK(hook_lock);
enum mitigation_state state;

/*
@@ -555,13 +550,6 @@ static enum mitigation_state spectre_v4_enable_hw_mitigation(void)
if (state != SPECTRE_MITIGATED || !this_cpu_has_cap(ARM64_SSBS))
return state;

- raw_spin_lock(&hook_lock);
- if (!undef_hook_registered) {
- register_undef_hook(&ssbs_emulation_hook);
- undef_hook_registered = true;
- }
- raw_spin_unlock(&hook_lock);
-
if (spectre_v4_mitigations_off()) {
sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_DSSBS);
set_pstate_ssbs(1);
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 678720103388..ccd7d773e5cd 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -402,12 +402,7 @@ static int call_undef_hook(struct pt_regs *regs)
int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
unsigned long pc = instruction_pointer(regs);

- if (!user_mode(regs)) {
- __le32 instr_le;
- if (get_kernel_nofault(instr_le, (__le32 *)pc))
- goto exit;
- instr = le32_to_cpu(instr_le);
- } else if (compat_thumb_mode(regs)) {
+ if (compat_thumb_mode(regs)) {
/* 16-bit Thumb instruction */
__le16 instr_le;
if (get_user(instr_le, (__le16 __user *)pc))
@@ -500,9 +495,15 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)

void do_el1_undef(struct pt_regs *regs, unsigned long esr)
{
- if (call_undef_hook(regs) == 0)
+ u32 insn;
+
+ if (aarch64_insn_read((void *)regs->pc, &insn))
+ goto out_err;
+
+ if (try_emulate_el1_ssbs(regs, insn))
return;

+out_err:
die("Oops - Undefined instruction", regs, esr);