ARM Spectre variant 2 fixes

This series addresses the Spectre variant 2 issues on ARM Cortex and Broadcom Brahma B15 CPUs. Due to the complexity of the bug, it is not possible to verify that this series fixes any of the bugs, since it has not been able to reproduce these exact scenarios using test programs.

I believe that this covers the entire extent of the Spectre variant 2 issues, with the exception of Cortex A53 and Cortex A72 processors as these require a substantially more complex solution (except where the workaround is implemented in PSCI firmware.)

Spectre variant 1 is not covered by this series.

The patch series is based partly on Marc Zyngier's work from February - two of the KVM patches are from Marc's work.

The main differences are:
- Inclusion of more processors as per current ARM Ltd security update documentation.
- Extension of "bugs" infrastructure to detect Cortex A8 and Cortex A15 CPUs missing out on the IBE bit being set on (re-)entry to the kernel through all paths.
- Handle all suspect userspace-touching-kernelspace aborts irrespective of mapping type.

The first patch will trivially conflict with the Broadcom Brahma updates already in arm-soc - it has been necessary to independently add the ID definitions for the B15 CPU.

Having worked through this series, I'm of the opinion that the define_processor_functions macro in proc-v7 are probably more hassle than they're worth - here, we don't need the global equivalent symbols, because we never refer to them from the kernel code for any V7 processor (MULTI_CPU is always defined.)

This series is currently in my "spectre" branch (along with some Spectre variant 1 patches.)

Please carefully review.

 arch/arm/include/asm/bugs.h        |   6 +-
 arch/arm/include/asm/cp15.h        |   3 +
 arch/arm/include/asm/cputype.h     |   5 ++
 arch/arm/include/asm/kvm_asm.h     |   2 -
 arch/arm/include/asm/kvm_host.h    |  14 +++-
 arch/arm/include/asm/kvm_mmu.h     |  23 +++++-
 arch/arm/include/asm/proc-fns.h    |   4 +
 arch/arm/include/asm/system_misc.h |   8 ++
 arch/arm/kernel/Makefile           |   1 +
 arch/arm/kernel/bugs.c             |  18 +++++
 arch/arm/kernel/smp.c              |   4 +
 arch/arm/kernel/suspend.c          |   2 +
 arch/arm/kvm/hyp/hyp-entry.S       | 108 ++++++++++++++++++++++++-
 arch/arm/mm/Kconfig                |  23 ++++++
 arch/arm/mm/Makefile               |   2 +-
 arch/arm/mm/fault.c                |   3 +
 arch/arm/mm/proc-macros.S          |   3 +-
 arch/arm/mm/proc-v7-2level.S       |   6 --
 arch/arm/mm/proc-v7-bugs.c         | 130 ++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              | 158 +++++++++++++++++++++++++++++--------
 20 files changed, 471 insertions(+), 52 deletions(-)

Signature · 8 lines

RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Check for CPU bugs when secondary processors are being brought online, and also when CPUs are resuming from a low power mode. This gives an opportunity to check that processor specific bug workarounds are correctly enabled for all paths that a CPU re-enters the kernel.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

---
 arch/arm/include/asm/bugs.h | 2 ++
 arch/arm/kernel/bugs.c      | 5 +++++
 arch/arm/kernel/smp.c       | 4 ++++
 arch/arm/kernel/suspend.c   | 2 ++
 4 files changed, 13 insertions(+)

arch/arm/include/asm/bugs.h +2 -0

index ed122d294f3f..73a99c72a930 100644
@@ -14,8 +14,10 @@ extern void check_writebuffer_bugs(void);
 
 #ifdef CONFIG_MMU
 extern void check_bugs(void);
+extern void check_other_bugs(void);
 #else
 #define check_bugs() do { } while (0)
+#define check_other_bugs() do { } while (0)
 #endif
 
 #endif

arch/arm/kernel/bugs.c +5 -0

index 88024028bb70..16e7ba2a9cc4 100644
@@ -3,7 +3,12 @@
 #include <asm/bugs.h>
 #include <asm/proc-fns.h>
 
+void check_other_bugs(void)
+{
+}
+
 void __init check_bugs(void)
 {
 	check_writebuffer_bugs();
+	check_other_bugs();
 }

arch/arm/kernel/smp.c +4 -0

index 2da087926ebe..5ad0b67b9e33 100644
@@ -31,6 +31,7 @@
 #include <linux/irq_work.h>
 
 #include <linux/atomic.h>
+#include <asm/bugs.h>
 #include <asm/smp.h>
 #include <asm/cacheflush.h>
 #include <asm/cpu.h>

@@ -405,6 +406,9 @@ asmlinkage void secondary_start_kernel(void)
 	 * before we continue - which happens after __cpu_up returns.
 	 */
 	set_cpu_online(cpu, true);
+
+	check_other_bugs();
+
 	complete(&cpu_running);
 
 	local_irq_enable();

arch/arm/kernel/suspend.c +2 -0

index a40ebb7c0896..d08099269e35 100644
@@ -3,6 +3,7 @@
 #include <linux/slab.h>
 #include <linux/mm_types.h>
 
+#include <asm/bugs.h>
 #include <asm/cacheflush.h>
 #include <asm/idmap.h>
 #include <asm/pgalloc.h>

@@ -36,6 +37,7 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 		cpu_switch_mm(mm->pgd, mm);
 		local_flush_bp_all();
 		local_flush_tlb_all();
+		check_other_bugs();
 	}
 
 	return ret;

Quoted 7 lines · Russell King

Check for CPU bugs when secondary processors are being brought online, and also when CPUs are resuming from a low power mode. This gives an opportunity to check that processor specific bug workarounds are correctly enabled for all paths that a CPU re-enters the kernel.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

Something I missed, is that this correctly warns about e.g: missing the IBE bit for secondary cores, but it seems to be missing it for the boot CPU:

Quoted 6 lines · Florian Fainelli

Quoted 5 lines · Russell King

Check for CPU bugs when secondary processors are being brought online, and also when CPUs are resuming from a low power mode. This gives an opportunity to check that processor specific bug workarounds are correctly enabled for all paths that a CPU re-enters the kernel.

Quoted 5 lines

Quoted 2 lines

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

Something I missed, is that this correctly warns about e.g: missing the IBE bit for secondary cores, but it seems to be missing it for the boot CPU:

Are you sure that the boot CPU has the IBE bit clear?

Harden the branch predictor against Spectre v2 attacks on context switches for ARMv7 and later CPUs. We do this by:

Cortex A9, A12, A17, A73, A75: invalidating the BTB. Cortex A15, Brahma B15: invalidating the instruction cache.

Cortex A57 and Cortex A72 are not addressed in this patch.

Cortex R7 and Cortex R8 are also not addressed as we do not enforce memory protection on these cores.

Signed-off-by Russell King <[email protected]>

Quoted 6 lines · Russell King

Harden the branch predictor against Spectre v2 attacks on context switches for ARMv7 and later CPUs. We do this by:

Cortex A9, A12, A17, A73, A75: invalidating the BTB. Cortex A15, Brahma B15: invalidating the instruction cache.

Quoted 6 lines

Cortex A57 and Cortex A72 are not addressed in this patch.

Cortex R7 and Cortex R8 are also not addressed as we do not enforce memory protection on these cores.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

Harden the branch predictor against Spectre v2 attacks on context switches for ARMv7 and later CPUs. We do this by:

Cortex A9, A12, A17, A73, A75: invalidating the BTB. Cortex A15, Brahma B15: invalidating the instruction cache.

Cortex A57 and Cortex A72 are not addressed in this patch.

Cortex R7 and Cortex R8 are also not addressed as we do not enforce memory protection on these cores.

When the branch predictor hardening is enabled, firmware must have set the IBE bit in the auxiliary control register. If this bit has not been set, the Spectre workarounds will not be functional.

Add validation that this bit is set, and print a warning at alert level if this is not the case.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

In order to prevent aliasing attacks on the branch predictor, invalidate the BTB or instruction cache on CPUs that are known to be affected when taking an abort on a address that is outside of a user task limit:

Cortex A8, A9, A12, A17, A73, A75: flush BTB.
Cortex A15, Brahma B15: invalidate icache.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

---
 arch/arm/include/asm/cp15.h        |  3 +++
 arch/arm/include/asm/system_misc.h |  8 ++++++
 arch/arm/mm/fault.c                |  3 +++
 arch/arm/mm/proc-v7-bugs.c         | 51 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              |  8 +++---
 5 files changed, 70 insertions(+), 3 deletions(-)

arch/arm/include/asm/cp15.h +3 -0

index 4c9fa72b59f5..07e27f212dc7 100644
@@ -65,6 +65,9 @@
 #define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
 #define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
 
+#define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
+#define ICIALLU				__ACCESS_CP15(c7, 0, c5, 0)
+
 extern unsigned long cr_alignment;	/* defined in entry-armv.S */
 
 static inline unsigned long get_cr(void)

arch/arm/include/asm/system_misc.h +8 -0

index 78f6db114faf..3cfe010c5734 100644
@@ -15,6 +15,14 @@ void soft_restart(unsigned long);
 extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 extern void (*arm_pm_idle)(void);
 
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+extern void (*harden_branch_predictor)(void);
+#define harden_branch_predictor() \
+	do { if (harden_branch_predictor) harden_branch_predictor(); } while (0)
+#else
+#define harden_branch_predictor() do { } while (0)
+#endif
+
 #define UDBG_UNDEFINED	(1 << 0)
 #define UDBG_SYSCALL	(1 << 1)
 #define UDBG_BADABORT	(1 << 2)

arch/arm/mm/fault.c +3 -0

index b75eada23d0a..3b1ba003c4f9 100644
@@ -163,6 +163,9 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 {
 	struct siginfo si;
 
+	if (addr > TASK_SIZE)
+		harden_branch_predictor();
+
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {

arch/arm/mm/proc-v7-bugs.c +19 -0

index a32ce13479d9..65a9b8141f86 100644
@@ -2,6 +2,12 @@
 #include <linux/kernel.h>
 #include <linux/smp.h>
 
+#include <asm/cp15.h>
+#include <asm/cputype.h>
+#include <asm/system_misc.h>
+
+void cpu_v7_bugs_init(void);
+
 static __maybe_unused void cpu_v7_check_auxcr_set(u32 mask, const char *msg)
 {
 	u32 aux_cr;

@@ -21,9 +27,54 @@ static void check_spectre_auxcr(u32 bit)
 void cpu_v7_ca8_ibe(void)
 {
 	check_spectre_auxcr(BIT(6));
+	cpu_v7_bugs_init();
 }
 
 void cpu_v7_ca15_ibe(void)
 {
 	check_spectre_auxcr(BIT(0));
+	cpu_v7_bugs_init();
+}
+
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+void (*harden_branch_predictor)(void);
+
+static void harden_branch_predictor_bpiall(void)
+{
+	write_sysreg(0, BPIALL);
+}
+
+static void harden_branch_predictor_iciallu(void)

arch/arm/mm/proc-v7-bugs.c +11 -0

+
+	case ARM_CPU_PART_CORTEX_A15:
+	case ARM_CPU_PART_BRAHMA_B15:
+		harden_branch_predictor = harden_branch_predictor_iciallu;
+		spectre_v2_method = "ICIALLU";
+		break;
+	}
+	if (spectre_v2_method)
+		pr_info("CPU: Spectre v2: using %s workaround\n",
+			spectre_v2_method);
 }
+#endif

arch/arm/mm/proc-v7.S +4 -2

index fa9214036fb3..79510011e7eb 100644
@@ -532,8 +532,10 @@ ENDPROC(__v7_setup)
 
 	__INITDATA
 
+	.weak cpu_v7_bugs_init
+
 	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
-	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1, bugs=cpu_v7_bugs_init
 
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 	@ generic v7 bpiall on context switch

@@ -548,7 +550,7 @@ ENDPROC(__v7_setup)
 	globl_equ	cpu_v7_bpiall_do_suspend,	cpu_v7_do_suspend
 	globl_equ	cpu_v7_bpiall_do_resume,	cpu_v7_do_resume
 #endif
-	define_processor_functions v7_bpiall, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+	define_processor_functions v7_bpiall, dabort=v7_early_abort, pabort=v7_pabort, suspend=1, bugs=cpu_v7_bugs_init
 
 #define HARDENED_BPIALL_PROCESSOR_FUNCTIONS v7_bpiall_processor_functions
 #else

Quoted 111 lines · Russell King

In order to prevent aliasing attacks on the branch predictor, invalidate the BTB or instruction cache on CPUs that are known to be affected when taking an abort on a address that is outside of a user task limit:

Cortex A8, A9, A12, A17, A73, A75: flush BTB.
Cortex A15, Brahma B15: invalidate icache.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

---
 arch/arm/include/asm/cp15.h        |  3 +++
 arch/arm/include/asm/system_misc.h |  8 ++++++
 arch/arm/mm/fault.c                |  3 +++
 arch/arm/mm/proc-v7-bugs.c         | 51 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              |  8 +++---
 5 files changed, 70 insertions(+), 3 deletions(-)

arch/arm/include/asm/cp15.h +3 -0

index 4c9fa72b59f5..07e27f212dc7 100644
@@ -65,6 +65,9 @@
 #define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
 #define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
 
+#define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
+#define ICIALLU				__ACCESS_CP15(c7, 0, c5, 0)
+
 extern unsigned long cr_alignment;	/* defined in entry-armv.S */
 
 static inline unsigned long get_cr(void)

arch/arm/include/asm/system_misc.h +8 -0

index 78f6db114faf..3cfe010c5734 100644
@@ -15,6 +15,14 @@ void soft_restart(unsigned long);
 extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 extern void (*arm_pm_idle)(void);
 
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+extern void (*harden_branch_predictor)(void);
+#define harden_branch_predictor() \
+	do { if (harden_branch_predictor) harden_branch_predictor(); } while (0)
+#else
+#define harden_branch_predictor() do { } while (0)
+#endif
+
 #define UDBG_UNDEFINED	(1 << 0)
 #define UDBG_SYSCALL	(1 << 1)
 #define UDBG_BADABORT	(1 << 2)

arch/arm/mm/fault.c +3 -0

index b75eada23d0a..3b1ba003c4f9 100644
@@ -163,6 +163,9 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 {
 	struct siginfo si;
 
+	if (addr > TASK_SIZE)
+		harden_branch_predictor();
+
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {

arch/arm/mm/proc-v7-bugs.c +26 -0

index a32ce13479d9..65a9b8141f86 100644
@@ -2,6 +2,12 @@
 #include <linux/kernel.h>
 #include <linux/smp.h>
 
+#include <asm/cp15.h>
+#include <asm/cputype.h>
+#include <asm/system_misc.h>
+
+void cpu_v7_bugs_init(void);
+
 static __maybe_unused void cpu_v7_check_auxcr_set(u32 mask, const char *msg)
 {
 	u32 aux_cr;

@@ -21,9 +27,54 @@ static void check_spectre_auxcr(u32 bit)
 void cpu_v7_ca8_ibe(void)
 {
 	check_spectre_auxcr(BIT(6));
+	cpu_v7_bugs_init();
 }
 
 void cpu_v7_ca15_ibe(void)
 {
 	check_spectre_auxcr(BIT(0));
+	cpu_v7_bugs_init();
+}
+
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+void (*harden_branch_predictor)(void);
+
+static void harden_branch_predictor_bpiall(void)
+{
+	write_sysreg(0, BPIALL);
+}
+
+static void harden_branch_predictor_iciallu(void)
+{
+	write_sysreg(0, ICIALLU);
+}
+
+void cpu_v7_bugs_init(void)
+{
+	const char *spectre_v2_method = NULL;

arch/arm/mm/proc-v7-bugs.c static void check_spectre_auxcr(u32 bit) · line 57

Quoted 3 lines

arch/arm/mm/proc-v7-bugs.c +3 -0

+
+	if (harden_branch_predictor)
+		return;

How does it work on a big-little systems where two CPUs have diverging mitigation methods? Let's say an hypothetical A15/A17 system? Or even a more common A15/A7 system, where the small core doesn't require the mitigation?

Quoted 112 lines · Marc Zyngier

Quoted 111 lines · Russell King

In order to prevent aliasing attacks on the branch predictor, invalidate the BTB or instruction cache on CPUs that are known to be affected when taking an abort on a address that is outside of a user task limit:

Cortex A8, A9, A12, A17, A73, A75: flush BTB.
Cortex A15, Brahma B15: invalidate icache.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

---
 arch/arm/include/asm/cp15.h        |  3 +++
 arch/arm/include/asm/system_misc.h |  8 ++++++
 arch/arm/mm/fault.c                |  3 +++
 arch/arm/mm/proc-v7-bugs.c         | 51 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              |  8 +++---
 5 files changed, 70 insertions(+), 3 deletions(-)

arch/arm/include/asm/cp15.h +3 -0

index 4c9fa72b59f5..07e27f212dc7 100644
@@ -65,6 +65,9 @@
 #define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
 #define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
 
+#define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
+#define ICIALLU				__ACCESS_CP15(c7, 0, c5, 0)
+
 extern unsigned long cr_alignment;	/* defined in entry-armv.S */
 
 static inline unsigned long get_cr(void)

arch/arm/include/asm/system_misc.h +8 -0

index 78f6db114faf..3cfe010c5734 100644
@@ -15,6 +15,14 @@ void soft_restart(unsigned long);
 extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 extern void (*arm_pm_idle)(void);
 
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+extern void (*harden_branch_predictor)(void);
+#define harden_branch_predictor() \
+	do { if (harden_branch_predictor) harden_branch_predictor(); } while (0)
+#else
+#define harden_branch_predictor() do { } while (0)
+#endif
+
 #define UDBG_UNDEFINED	(1 << 0)
 #define UDBG_SYSCALL	(1 << 1)
 #define UDBG_BADABORT	(1 << 2)

arch/arm/mm/fault.c +3 -0

index b75eada23d0a..3b1ba003c4f9 100644
@@ -163,6 +163,9 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 {
 	struct siginfo si;
 
+	if (addr > TASK_SIZE)
+		harden_branch_predictor();
+
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {

arch/arm/mm/proc-v7-bugs.c +26 -0

index a32ce13479d9..65a9b8141f86 100644
@@ -2,6 +2,12 @@
 #include <linux/kernel.h>
 #include <linux/smp.h>
 
+#include <asm/cp15.h>
+#include <asm/cputype.h>
+#include <asm/system_misc.h>
+
+void cpu_v7_bugs_init(void);
+
 static __maybe_unused void cpu_v7_check_auxcr_set(u32 mask, const char *msg)
 {
 	u32 aux_cr;

@@ -21,9 +27,54 @@ static void check_spectre_auxcr(u32 bit)
 void cpu_v7_ca8_ibe(void)
 {
 	check_spectre_auxcr(BIT(6));
+	cpu_v7_bugs_init();
 }
 
 void cpu_v7_ca15_ibe(void)
 {
 	check_spectre_auxcr(BIT(0));
+	cpu_v7_bugs_init();
+}
+
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+void (*harden_branch_predictor)(void);
+
+static void harden_branch_predictor_bpiall(void)
+{
+	write_sysreg(0, BPIALL);
+}
+
+static void harden_branch_predictor_iciallu(void)
+{
+	write_sysreg(0, ICIALLU);
+}
+
+void cpu_v7_bugs_init(void)
+{
+	const char *spectre_v2_method = NULL;

Quoted 8 lines

arch/arm/mm/proc-v7-bugs.c static void check_spectre_auxcr(u32 bit) · line 57

Quoted 3 lines

arch/arm/mm/proc-v7-bugs.c +3 -0

+
+	if (harden_branch_predictor)
+		return;

How does it work on a big-little systems where two CPUs have diverging mitigation methods? Let's say an hypothetical A15/A17 system? Or even a more common A15/A7 system, where the small core doesn't require the mitigation?

Hmm, I'd forgotten about those, because I don't have them.

We don't have the ability to mitigate this on such systems at all at present, it would require a per-CPU cpu_switch_mm() implementation, and the code has no structure to support that at present without considerable rewrite of the CPU glue support.

I'm not even sure it could without checking deeper - I think there's some situations where we call this before we're sufficiently setup.

Quoted 124 lines · Russell King - ARM Linux

Quoted 112 lines · Marc Zyngier

Quoted 111 lines · Russell King

In order to prevent aliasing attacks on the branch predictor, invalidate the BTB or instruction cache on CPUs that are known to be affected when taking an abort on a address that is outside of a user task limit:

Cortex A8, A9, A12, A17, A73, A75: flush BTB.
Cortex A15, Brahma B15: invalidate icache.

Signed-off-by Russell King <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

---
 arch/arm/include/asm/cp15.h        |  3 +++
 arch/arm/include/asm/system_misc.h |  8 ++++++
 arch/arm/mm/fault.c                |  3 +++
 arch/arm/mm/proc-v7-bugs.c         | 51 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              |  8 +++---
 5 files changed, 70 insertions(+), 3 deletions(-)

arch/arm/include/asm/cp15.h +3 -0

index 4c9fa72b59f5..07e27f212dc7 100644
@@ -65,6 +65,9 @@
 #define __write_sysreg(v, r, w, c, t)	asm volatile(w " " c : : "r" ((t)(v)))
 #define write_sysreg(v, ...)		__write_sysreg(v, __VA_ARGS__)
 
+#define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
+#define ICIALLU				__ACCESS_CP15(c7, 0, c5, 0)
+
 extern unsigned long cr_alignment;	/* defined in entry-armv.S */
 
 static inline unsigned long get_cr(void)

arch/arm/include/asm/system_misc.h +8 -0

index 78f6db114faf..3cfe010c5734 100644
@@ -15,6 +15,14 @@ void soft_restart(unsigned long);
 extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 extern void (*arm_pm_idle)(void);
 
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+extern void (*harden_branch_predictor)(void);
+#define harden_branch_predictor() \
+	do { if (harden_branch_predictor) harden_branch_predictor(); } while (0)
+#else
+#define harden_branch_predictor() do { } while (0)
+#endif
+
 #define UDBG_UNDEFINED	(1 << 0)
 #define UDBG_SYSCALL	(1 << 1)
 #define UDBG_BADABORT	(1 << 2)

arch/arm/mm/fault.c +3 -0

index b75eada23d0a..3b1ba003c4f9 100644
@@ -163,6 +163,9 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 {
 	struct siginfo si;
 
+	if (addr > TASK_SIZE)
+		harden_branch_predictor();
+
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {

arch/arm/mm/proc-v7-bugs.c +26 -0

index a32ce13479d9..65a9b8141f86 100644
@@ -2,6 +2,12 @@
 #include <linux/kernel.h>
 #include <linux/smp.h>
 
+#include <asm/cp15.h>
+#include <asm/cputype.h>
+#include <asm/system_misc.h>
+
+void cpu_v7_bugs_init(void);
+
 static __maybe_unused void cpu_v7_check_auxcr_set(u32 mask, const char *msg)
 {
 	u32 aux_cr;

@@ -21,9 +27,54 @@ static void check_spectre_auxcr(u32 bit)
 void cpu_v7_ca8_ibe(void)
 {
 	check_spectre_auxcr(BIT(6));
+	cpu_v7_bugs_init();
 }
 
 void cpu_v7_ca15_ibe(void)
 {
 	check_spectre_auxcr(BIT(0));
+	cpu_v7_bugs_init();
+}
+
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+void (*harden_branch_predictor)(void);
+
+static void harden_branch_predictor_bpiall(void)
+{
+	write_sysreg(0, BPIALL);
+}
+
+static void harden_branch_predictor_iciallu(void)
+{
+	write_sysreg(0, ICIALLU);
+}
+
+void cpu_v7_bugs_init(void)
+{
+	const char *spectre_v2_method = NULL;

Quoted 8 lines

arch/arm/mm/proc-v7-bugs.c static void check_spectre_auxcr(u32 bit) · line 57

Quoted 3 lines

arch/arm/mm/proc-v7-bugs.c +3 -0

+
+	if (harden_branch_predictor)
+		return;

How does it work on a big-little systems where two CPUs have diverging mitigation methods? Let's say an hypothetical A15/A17 system? Or even a more common A15/A7 system, where the small core doesn't require the mitigation?

Hmm, I'd forgotten about those, because I don't have them.

Quoted 7 lines

We don't have the ability to mitigate this on such systems at all at present, it would require a per-CPU cpu_switch_mm() implementation, and the code has no structure to support that at present without considerable rewrite of the CPU glue support.

I'm not even sure it could without checking deeper - I think there's some situations where we call this before we're sufficiently setup.

Confirmed. We can't access per_cpu variables via cpu_switch_mm() because it is used prior to the per_cpu offset being initialised in the CPU. Eg,

secondary_start_kernel
{
...

	cpu_switch_mm(mm->pgd, mm);
...
        cpu_init(); /* <== per cpu setup */
}

However, we can change harden_branch_predictor() to be a per-cpu function pointer to solve some of your concern, but it's still insufficient.

arch/arm/mm/proc-v7-bugs.c static void check_spectre_auxcr(u32 bit) · line 68

Quoted 10 lines · Russell King

arch/arm/mm/proc-v7-bugs.c +10 -0

+	switch (read_cpuid_part()) {
+	case ARM_CPU_PART_CORTEX_A8:
+	case ARM_CPU_PART_CORTEX_A9:
+	case ARM_CPU_PART_CORTEX_A12:
+	case ARM_CPU_PART_CORTEX_A17:
+	case ARM_CPU_PART_CORTEX_A73:
+	case ARM_CPU_PART_CORTEX_A75:
+		harden_branch_predictor = harden_branch_predictor_bpiall;
+		spectre_v2_method = "BPIALL";
+		break;

You don't seem to take into account the PFR0.CSV2 field which indicates that the CPU has a branch predictor that is immune to Spectre-v2.

See for example the Cortex-A75 r3p0 TRM[1].

Add PSCI based hardening for cores that require more complex handling in firmware.

Signed-off-by Russell King <[email protected]>

Acked-by Marc Zyngier <[email protected]>

---
 arch/arm/mm/proc-v7-bugs.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S      | 21 +++++++++++++++++++
 2 files changed, 71 insertions(+)

arch/arm/mm/proc-v7-bugs.c +19 -0

index 65a9b8141f86..0c37e6a2830d 100644
@@ -1,9 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/arm-smccc.h>
 #include <linux/kernel.h>
+#include <linux/psci.h>
 #include <linux/smp.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
+#include <asm/proc-fns.h>
 #include <asm/system_misc.h>
 
 void cpu_v7_bugs_init(void);

@@ -39,6 +42,9 @@ void cpu_v7_ca15_ibe(void)
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 void (*harden_branch_predictor)(void);
 
+extern void cpu_v7_smc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+extern void cpu_v7_hvc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+
 static void harden_branch_predictor_bpiall(void)
 {
 	write_sysreg(0, BPIALL);

@@ -49,6 +55,18 @@ static void harden_branch_predictor_iciallu(void)
 	write_sysreg(0, ICIALLU);
 }
 
+#ifdef CONFIG_ARM_PSCI
+static void call_smc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
+static void call_hvc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+#endif
+
 void cpu_v7_bugs_init(void)
 {
 	const char *spectre_v2_method = NULL;

@@ -73,6 +91,38 @@ void cpu_v7_bugs_init(void)
 		spectre_v2_method = "ICIALLU";
 		break;
 	}
+

Quoted 61 lines · Russell King

Add PSCI based hardening for cores that require more complex handling in firmware.

Signed-off-by Russell King <[email protected]>

Acked-by Marc Zyngier <[email protected]>

---
 arch/arm/mm/proc-v7-bugs.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S      | 21 +++++++++++++++++++
 2 files changed, 71 insertions(+)

arch/arm/mm/proc-v7-bugs.c +19 -0

index 65a9b8141f86..0c37e6a2830d 100644
@@ -1,9 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/arm-smccc.h>
 #include <linux/kernel.h>
+#include <linux/psci.h>
 #include <linux/smp.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
+#include <asm/proc-fns.h>
 #include <asm/system_misc.h>
 
 void cpu_v7_bugs_init(void);

@@ -39,6 +42,9 @@ void cpu_v7_ca15_ibe(void)
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 void (*harden_branch_predictor)(void);
 
+extern void cpu_v7_smc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+extern void cpu_v7_hvc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+
 static void harden_branch_predictor_bpiall(void)
 {
 	write_sysreg(0, BPIALL);

@@ -49,6 +55,18 @@ static void harden_branch_predictor_iciallu(void)
 	write_sysreg(0, ICIALLU);
 }
 
+#ifdef CONFIG_ARM_PSCI
+static void call_smc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
+static void call_hvc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+#endif
+
 void cpu_v7_bugs_init(void)
 {
 	const char *spectre_v2_method = NULL;

@@ -73,6 +91,38 @@ void cpu_v7_bugs_init(void)
 		spectre_v2_method = "ICIALLU";
 		break;
 	}
+

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

But that's probably moot if you don't support BL systems.

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 117

Quoted 13 lines

arch/arm/mm/proc-v7-bugs.c +13 -0

+			harden_branch_predictor = call_hvc_arch_workaround_1;
+			processor.switch_mm = cpu_v7_hvc_switch_mm;
+			spectre_v2_method = "hypervisor";
+			break;
+
+		case PSCI_CONDUIT_SMC:
+			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;
+			harden_branch_predictor = call_smc_arch_workaround_1;
+			processor.switch_mm = cpu_v7_smc_switch_mm;
+			spectre_v2_method = "firmware PSCI";

My previous remark still stands: this is not really PSCI.

Quoted 83 lines · Marc Zyngier

Quoted 61 lines · Russell King

Add PSCI based hardening for cores that require more complex handling in firmware.

Signed-off-by Russell King <[email protected]>

Acked-by Marc Zyngier <[email protected]>

---
 arch/arm/mm/proc-v7-bugs.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S      | 21 +++++++++++++++++++
 2 files changed, 71 insertions(+)

arch/arm/mm/proc-v7-bugs.c +19 -0

index 65a9b8141f86..0c37e6a2830d 100644
@@ -1,9 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/arm-smccc.h>
 #include <linux/kernel.h>
+#include <linux/psci.h>
 #include <linux/smp.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
+#include <asm/proc-fns.h>
 #include <asm/system_misc.h>
 
 void cpu_v7_bugs_init(void);

@@ -39,6 +42,9 @@ void cpu_v7_ca15_ibe(void)
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 void (*harden_branch_predictor)(void);
 
+extern void cpu_v7_smc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+extern void cpu_v7_hvc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+
 static void harden_branch_predictor_bpiall(void)
 {
 	write_sysreg(0, BPIALL);

@@ -49,6 +55,18 @@ static void harden_branch_predictor_iciallu(void)
 	write_sysreg(0, ICIALLU);
 }
 
+#ifdef CONFIG_ARM_PSCI
+static void call_smc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
+static void call_hvc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+#endif
+
 void cpu_v7_bugs_init(void)
 {
 	const char *spectre_v2_method = NULL;

@@ -73,6 +91,38 @@ void cpu_v7_bugs_init(void)
 		spectre_v2_method = "ICIALLU";
 		break;
 	}
+

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

But that's probably moot if you don't support BL systems.

Quoted 15 lines

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 117

Quoted 13 lines

arch/arm/mm/proc-v7-bugs.c +13 -0

+			harden_branch_predictor = call_hvc_arch_workaround_1;
+			processor.switch_mm = cpu_v7_hvc_switch_mm;
+			spectre_v2_method = "hypervisor";
+			break;
+
+		case PSCI_CONDUIT_SMC:
+			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;
+			harden_branch_predictor = call_smc_arch_workaround_1;
+			processor.switch_mm = cpu_v7_smc_switch_mm;
+			spectre_v2_method = "firmware PSCI";

My previous remark still stands: this is not really PSCI.

Sorry, no. Your comment was for the HVC call, not the SMC. You said nothing about this one.

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines · Russell King

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

But that's probably moot if you don't support BL systems.

Quoted 12 lines · Marc Zyngier

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines · Russell King

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

Quoted 7 lines

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

Okay, so digging further into the documentation seems to suggest that we only need to check the firmware for A72 and A57 CPUs, and given this statement:

"Arm recommends that the caller only call this on PEs for which a
firmware based mitigation of CVE-2017-5715 is required, or where a local workaround is infeasible."

it seems that the right answer is to ignore the PSCI based methods when we have anything but these CPUs. Do you agree?

Quoted 1 line

But that's probably moot if you don't support BL systems.

Any bL systems with A72 or A57?

Quoted 29 lines · Russell King - ARM Linux

Quoted 12 lines · Marc Zyngier

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines · Russell King

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

Quoted 7 lines

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

Okay, so digging further into the documentation seems to suggest that we only need to check the firmware for A72 and A57 CPUs, and given this statement:

"Arm recommends that the caller only call this on PEs for which a
firmware based mitigation of CVE-2017-5715 is required, or where a local workaround is infeasible."

Quoted 2 lines

it seems that the right answer is to ignore the PSCI based methods when we have anything but these CPUs. Do you agree?

For CPUs that are produced by ARM, I agree. I don't know about CPUs produced by ARM licensees though, so I'd rather use the opposite logic:
Use the firmware unless the CPU is one of those that can be easily mitigated at EL1 (or isn't affected).

Quoted 33 lines · Marc Zyngier

Quoted 31 lines · Russell King - ARM Linux

Quoted 12 lines · Marc Zyngier

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines · Russell King

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

Quoted 7 lines

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

Okay, so digging further into the documentation seems to suggest that we only need to check the firmware for A72 and A57 CPUs, and given this statement:

"Arm recommends that the caller only call this on PEs for which a
firmware based mitigation of CVE-2017-5715 is required, or where a local workaround is infeasible."

it seems that the right answer is to ignore the PSCI based methods when we have anything but these CPUs. Do you agree?

Quoted 4 lines

For CPUs that are produced by ARM, I agree. I don't know about CPUs produced by ARM licensees though, so I'd rather use the opposite logic:
Use the firmware unless the CPU is one of those that can be easily mitigated at EL1 (or isn't affected).

The "or isn't affected" is the difficult bit - I guess we could match on the CPU vendor field though, and just reject all ARM CPUs that aren't explicitly listed as having a problem.

Quoted 39 lines · Russell King - ARM Linux

Quoted 37 lines · Marc Zyngier

Quoted 25 lines · Russell King - ARM Linux

Quoted 12 lines · Marc Zyngier

arch/arm/mm/proc-v7-bugs.c void cpu_v7_bugs_init(void) · line 104

Quoted 10 lines · Russell King

arch/arm/mm/proc-v7-bugs.c +10 -0

+#ifdef CONFIG_ARM_PSCI
+	if (psci_ops.smccc_version != SMCCC_VERSION_1_0) {
+		struct arm_smccc_res res;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 < 0)
+				break;

Quoted 7 lines

I just realised that there is a small, but significant difference between this and the arm64 version: On arm64, we have a table of vulnerable implementations, and we try the mitigation on a per-cpu basis. Here, you entirely rely on the firmware to discover whether the CPU needs mitigation or not. You then need to check for a return value of 1, which indicates that although the mitigation is implemented, it is not required on this particular CPU.

Okay, so digging further into the documentation seems to suggest that we only need to check the firmware for A72 and A57 CPUs, and given this statement:

Quoted 6 lines

"Arm recommends that the caller only call this on PEs for which a
firmware based mitigation of CVE-2017-5715 is required, or where a local workaround is infeasible."

it seems that the right answer is to ignore the PSCI based methods when we have anything but these CPUs. Do you agree?

For CPUs that are produced by ARM, I agree. I don't know about CPUs produced by ARM licensees though, so I'd rather use the opposite logic:
Use the firmware unless the CPU is one of those that can be easily mitigated at EL1 (or isn't affected).

Quoted 3 lines

The "or isn't affected" is the difficult bit - I guess we could match on the CPU vendor field though, and just reject all ARM CPUs that aren't explicitly listed as having a problem.

That seems sensible. ARM has published an exhaustive status for all its cores, which we can trust. For architecture licensees, I'm not aware of such a list, but I'd expect them to communicate one if they were affected.

Include Brahma B15 in the Spectre v2 KVM workarounds.

Signed-off-by Russell King <[email protected]>

We want SMCCC_ARCH_WORKAROUND_1 to be fast. As fast as possible. So let's intercept it as early as we can by testing for the function call number as soon as we've identified a HVC call coming from the guest.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kvm/hyp/hyp-entry.S | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

arch/arm/kvm/hyp/hyp-entry.S +2 -1

index 918a05dd2d63..67de45685e29 100644
@@ -16,6 +16,7 @@
  * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+#include <linux/arm-smccc.h>
 #include <linux/linkage.h>
 #include <asm/kvm_arm.h>
 #include <asm/kvm_asm.h>

@@ -202,7 +203,7 @@ ENDPROC(__hyp_do_panic)
 	lsr     r2, r2, #16
 	and     r2, r2, #0xff
 	cmp     r2, #0
-	bne	guest_trap		@ Guest called HVC
+	bne	guest_hvc_trap		@ Guest called HVC
 
 	/*
 	 * Getting here means host called HVC, we shift parameters and branch

This is the second posting - the original cover note is below. Comments from previous series addresesd:
- Drop R7 and R8 changes.
- Remove "PSCI" from the hypervisor version of the workaround.

 arch/arm/include/asm/bugs.h        |   6 +-
 arch/arm/include/asm/cp15.h        |   3 +
 arch/arm/include/asm/cputype.h     |   5 ++
 arch/arm/include/asm/kvm_asm.h     |   2 -
 arch/arm/include/asm/kvm_host.h    |  14 +++-
 arch/arm/include/asm/kvm_mmu.h     |  23 +++++-
 arch/arm/include/asm/proc-fns.h    |   4 +
 arch/arm/include/asm/system_misc.h |   8 ++
 arch/arm/kernel/Makefile           |   1 +
 arch/arm/kernel/bugs.c             |  18 +++++
 arch/arm/kernel/smp.c              |   4 +
 arch/arm/kernel/suspend.c          |   2 +
 arch/arm/kvm/hyp/hyp-entry.S       | 108 +++++++++++++++++++++++++-
 arch/arm/mm/Kconfig                |  23 ++++++
 arch/arm/mm/Makefile               |   2 +-

 arch/arm/mm/fault.c                |   3 +
 arch/arm/mm/proc-macros.S          |   3 +-
 arch/arm/mm/proc-v7-2level.S       |   6 --
 arch/arm/mm/proc-v7-bugs.c         | 130 +++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              | 154 +++++++++++++++++++++++++++++--------
 20 files changed, 469 insertions(+), 50 deletions(-)
 create mode 100644 arch/arm/kernel/bugs.c
 create mode 100644 arch/arm/mm/proc-v7-bugs.c

Quoted 71 lines

This series addresses the Spectre variant 2 issues on ARM Cortex and Broadcom Brahma B15 CPUs. Due to the complexity of the bug, it is not possible to verify that this series fixes any of the bugs, since it has not been able to reproduce these exact scenarios using test programs.

I believe that this covers the entire extent of the Spectre variant 2 issues, with the exception of Cortex A53 and Cortex A72 processors as these require a substantially more complex solution (except where the workaround is implemented in PSCI firmware.)

Spectre variant 1 is not covered by this series.

The patch series is based partly on Marc Zyngier's work from February - two of the KVM patches are from Marc's work.

The main differences are:
- Inclusion of more processors as per current ARM Ltd security update documentation.
- Extension of "bugs" infrastructure to detect Cortex A8 and Cortex A15 CPUs missing out on the IBE bit being set on (re-)entry to the kernel through all paths.
- Handle all suspect userspace-touching-kernelspace aborts irrespective of mapping type.

The first patch will trivially conflict with the Broadcom Brahma updates already in arm-soc - it has been necessary to independently add the ID definitions for the B15 CPU.

Having worked through this series, I'm of the opinion that the define_processor_functions macro in proc-v7 are probably more hassle than they're worth - here, we don't need the global equivalent symbols, because we never refer to them from the kernel code for any V7 processor (MULTI_CPU is always defined.)

This series is currently in my "spectre" branch (along with some Spectre variant 1 patches.)

Please carefully review.

 arch/arm/include/asm/bugs.h        |   6 +-
 arch/arm/include/asm/cp15.h        |   3 +
 arch/arm/include/asm/cputype.h     |   5 ++
 arch/arm/include/asm/kvm_asm.h     |   2 -
 arch/arm/include/asm/kvm_host.h    |  14 +++-
 arch/arm/include/asm/kvm_mmu.h     |  23 +++++-
 arch/arm/include/asm/proc-fns.h    |   4 +
 arch/arm/include/asm/system_misc.h |   8 ++
 arch/arm/kernel/Makefile           |   1 +
 arch/arm/kernel/bugs.c             |  18 +++++
 arch/arm/kernel/smp.c              |   4 +
 arch/arm/kernel/suspend.c          |   2 +
 arch/arm/kvm/hyp/hyp-entry.S       | 108 ++++++++++++++++++++++++-
 arch/arm/mm/Kconfig                |  23 ++++++
 arch/arm/mm/Makefile               |   2 +-
 arch/arm/mm/fault.c                |   3 +
 arch/arm/mm/proc-macros.S          |   3 +-
 arch/arm/mm/proc-v7-2level.S       |   6 --
 arch/arm/mm/proc-v7-bugs.c         | 130 ++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              | 158 +++++++++++++++++++++++++++++--------
 20 files changed, 471 insertions(+), 52 deletions(-)

Signature · 8 lines

RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 20 lines · Russell King - ARM Linux

This is the second posting - the original cover note is below. Comments from previous series addresesd:
- Drop R7 and R8 changes.
- Remove "PSCI" from the hypervisor version of the workaround.

 arch/arm/include/asm/bugs.h        |   6 +-
 arch/arm/include/asm/cp15.h        |   3 +
 arch/arm/include/asm/cputype.h     |   5 ++
 arch/arm/include/asm/kvm_asm.h     |   2 -
 arch/arm/include/asm/kvm_host.h    |  14 +++-
 arch/arm/include/asm/kvm_mmu.h     |  23 +++++-
 arch/arm/include/asm/proc-fns.h    |   4 +
 arch/arm/include/asm/system_misc.h |   8 ++
 arch/arm/kernel/Makefile           |   1 +
 arch/arm/kernel/bugs.c             |  18 +++++
 arch/arm/kernel/smp.c              |   4 +
 arch/arm/kernel/suspend.c          |   2 +
 arch/arm/kvm/hyp/hyp-entry.S       | 108 +++++++++++++++++++++++++-
 arch/arm/mm/Kconfig                |  23 ++++++
 arch/arm/mm/Makefile               |   2 +-

Quoted 8 lines

 arch/arm/mm/fault.c                |   3 +
 arch/arm/mm/proc-macros.S          |   3 +-
 arch/arm/mm/proc-v7-2level.S       |   6 --
 arch/arm/mm/proc-v7-bugs.c         | 130 +++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S              | 154 +++++++++++++++++++++++++++++--------
 20 files changed, 469 insertions(+), 50 deletions(-)
 create mode 100644 arch/arm/kernel/bugs.c
 create mode 100644 arch/arm/mm/proc-v7-bugs.c

Since there appears to be more work needed in the PSCI/KVM changes (patches 9 through 14), how about we go with the "bare-metal" parts:
patches 1-8 first and try to get those included ASAP?

The rationale being that a lot of affected people have Linux running on ARMv7-A Cortex-A, typically A9, A15, Brahma-B15, and are in need of those patches but do not necessarily require KVM fixes right now, and even less so PSCI infrastructure to mitigate ARMv8-A running in AArch32.

In terms of backporting to -stable, and because the spectre variant 1 patches have not been submitted yet, it is not like we can lump everything in one go anyway, so we are not making the lives of the -stable maintainers any worse than it currently is?