From: James Morse <[email protected]>
commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
CPUs vulnerable to Spectre-BHB either need to make an SMC-CC firmware
call from the vectors, or run a sequence of branches. This gets added
to the hyp vectors. If there is no support for arch-workaround-1 in
firmware, the indirect vector will be used.
kvm_init_vector_slots() only initialises the two indirect slots if
the platform is vulnerable to Spectre-v3a. pKVM's hyp_map_vectors()
only initialises __hyp_bp_vect_base if the platform is vulnerable to
Spectre-v3a.
As there are about to more users of the indirect vectors, ensure
their entries in hyp_spectre_vector_selector[] are always initialised,
and __hyp_bp_vect_base defaults to the regular VA mapping.
The Spectre-v3a check is moved to a helper
kvm_system_needs_idmapped_vectors(), and merged with the code
that creates the hyp mappings.
Reviewed-by: Catalin Marinas <[email protected]>
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
arch/arm64/include/asm/cpucaps.h | 3 +
arch/arm64/include/asm/kvm_asm.h | 6 +++
arch/arm64/include/asm/kvm_mmu.h | 3 +
arch/arm64/include/asm/mmu.h | 6 +++
arch/arm64/kernel/proton-pack.c | 47 +++++++++++++++++++++++++++
arch/arm64/kvm/arm.c | 3 +
arch/arm64/kvm/hyp/smccc_wa.S | 66 +++++++++++++++++++++++++++++++++++++++
7 files changed, 130 insertions(+), 4 deletions(-)
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -66,7 +66,8 @@
#define ARM64_HAS_TLB_RANGE 56
#define ARM64_MTE 57
#define ARM64_WORKAROUND_1508412 58
+#define ARM64_SPECTRE_BHB 59
-#define ARM64_NCAPS 59
+#define ARM64_NCAPS 60
#endif /* __ASM_CPUCAPS_H */
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -35,6 +35,8 @@
#define KVM_VECTOR_PREAMBLE (2 * AARCH64_INSN_SIZE)
#define __SMCCC_WORKAROUND_1_SMC_SZ 36
+#define __SMCCC_WORKAROUND_3_SMC_SZ 36
+#define __SPECTRE_BHB_LOOP_SZ 44
#define KVM_HOST_SMCCC_ID(id) \
ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
@@ -199,6 +201,10 @@ extern void __vgic_v3_init_lrs(void);
extern u32 __kvm_get_mdcr_el2(void);
extern char __smccc_workaround_1_smc[__SMCCC_WORKAROUND_1_SMC_SZ];
+extern char __smccc_workaround_3_smc[__SMCCC_WORKAROUND_3_SMC_SZ];
+extern char __spectre_bhb_loop_k8[__SPECTRE_BHB_LOOP_SZ];
+extern char __spectre_bhb_loop_k24[__SPECTRE_BHB_LOOP_SZ];
+extern char __spectre_bhb_loop_k32[__SPECTRE_BHB_LOOP_SZ];
/*
* Obtain the PC-relative address of a kernel symbol
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -237,7 +237,8 @@ static inline void *kvm_get_hyp_vector(v
void *vect = kern_hyp_va(kvm_ksym_ref(__kvm_hyp_vector));
int slot = -1;
- if (cpus_have_const_cap(ARM64_SPECTRE_V2) && data->fn) {
+ if ((cpus_have_const_cap(ARM64_SPECTRE_V2) ||
+ cpus_have_const_cap(ARM64_SPECTRE_BHB)) && data->template_start) {
vect = kern_hyp_va(kvm_ksym_ref(__bp_harden_hyp_vecs));
slot = data->hyp_vectors_slot;
}
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -67,6 +67,12 @@ typedef void (*bp_hardening_cb_t)(void);
struct bp_hardening_data {
int hyp_vectors_slot;
bp_hardening_cb_t fn;
+
+ /*
+ * template_start is only used by the BHB mitigation to identify the
+ * hyp_vectors_slot sequence.
+ */
+ const char *template_start;
};
DECLARE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data);
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -220,9 +220,9 @@ static void __copy_hyp_vect_bpi(int slot
__flush_icache_range((uintptr_t)dst, (uintptr_t)dst + SZ_2K);
}
+static DEFINE_RAW_SPINLOCK(bp_lock);
static void install_bp_hardening_cb(bp_hardening_cb_t fn)
{
- static DEFINE_RAW_SPINLOCK(bp_lock);
int cpu, slot = -1;
const char *hyp_vecs_start = __smccc_workaround_1_smc;
const char *hyp_vecs_end = __smccc_workaround_1_smc +
@@ -253,6 +253,7 @@ static void install_bp_hardening_cb(bp_h
__this_cpu_write(bp_hardening_data.hyp_vectors_slot, slot);
__this_cpu_write(bp_hardening_data.fn, fn);
+ __this_cpu_write(bp_hardening_data.template_start, hyp_vecs_start);
raw_spin_unlock(&bp_lock);
}
#else
@@ -819,3 +820,47 @@ enum mitigation_state arm64_get_spectre_
{
return spectre_bhb_state;
}
+
+static int kvm_bhb_get_vecs_size(const char *start)
+{
+ if (start == __smccc_workaround_3_smc)
+ return __SMCCC_WORKAROUND_3_SMC_SZ;
+ else if (start == __spectre_bhb_loop_k8 ||
+ start == __spectre_bhb_loop_k24 ||
+ start == __spectre_bhb_loop_k32)
+ return __SPECTRE_BHB_LOOP_SZ;
+
+ return 0;
+}
+
+void kvm_setup_bhb_slot(const char *hyp_vecs_start)
+{
+ int cpu, slot = -1, size;
+ const char *hyp_vecs_end;
+
+ if (!IS_ENABLED(CONFIG_KVM) || !is_hyp_mode_available())
+ return;
+
+ size = kvm_bhb_get_vecs_size(hyp_vecs_start);
+ if (WARN_ON_ONCE(!hyp_vecs_start || !size))
+ return;
+ hyp_vecs_end = hyp_vecs_start + size;
+
+ raw_spin_lock(&bp_lock);
+ for_each_possible_cpu(cpu) {
+ if (per_cpu(bp_hardening_data.template_start, cpu) == hyp_vecs_start) {
+ slot = per_cpu(bp_hardening_data.hyp_vectors_slot, cpu);
+ break;
+ }
+ }
+
+ if (slot == -1) {
+ slot = atomic_inc_return(&arm64_el2_vector_last_slot);
+ BUG_ON(slot >= BP_HARDEN_EL2_SLOTS);
+ __copy_hyp_vect_bpi(slot, hyp_vecs_start, hyp_vecs_end);
+ }
+
+ __this_cpu_write(bp_hardening_data.hyp_vectors_slot, slot);
+ __this_cpu_write(bp_hardening_data.template_start, hyp_vecs_start);
+ raw_spin_unlock(&bp_lock);
+}
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1337,7 +1337,8 @@ static int kvm_map_vectors(void)
* !SV2 + HEL2 -> allocate one vector slot and use exec mapping
* SV2 + HEL2 -> use hardened vectors and use exec mapping
*/
- if (cpus_have_const_cap(ARM64_SPECTRE_V2)) {
+ if (cpus_have_const_cap(ARM64_SPECTRE_V2) ||
+ cpus_have_const_cap(ARM64_SPECTRE_BHB)) {
__kvm_bp_vect_base = kvm_ksym_ref(__bp_harden_hyp_vecs);
__kvm_bp_vect_base = kern_hyp_va(__kvm_bp_vect_base);
}
--- a/arch/arm64/kvm/hyp/smccc_wa.S
+++ b/arch/arm64/kvm/hyp/smccc_wa.S
@@ -30,3 +30,69 @@ SYM_DATA_START(__smccc_workaround_1_smc)
1: .org __smccc_workaround_1_smc + __SMCCC_WORKAROUND_1_SMC_SZ
.org 1b
SYM_DATA_END(__smccc_workaround_1_smc)
+
+ .global __smccc_workaround_3_smc
+SYM_DATA_START(__smccc_workaround_3_smc)
+ esb
+ sub sp, sp, #(8 * 4)
+ stp x2, x3, [sp, #(8 * 0)]
+ stp x0, x1, [sp, #(8 * 2)]
+ mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
+ smc #0
+ ldp x2, x3, [sp, #(8 * 0)]
+ ldp x0, x1, [sp, #(8 * 2)]
+ add sp, sp, #(8 * 4)
+1: .org __smccc_workaround_3_smc + __SMCCC_WORKAROUND_3_SMC_SZ
+ .org 1b
+SYM_DATA_END(__smccc_workaround_3_smc)
+
+ .global __spectre_bhb_loop_k8
+SYM_DATA_START(__spectre_bhb_loop_k8)
+ esb
+ sub sp, sp, #(8 * 2)
+ stp x0, x1, [sp, #(8 * 0)]
+ mov x0, #8
+2: b . + 4
+ subs x0, x0, #1
+ b.ne 2b
+ dsb nsh
+ isb
+ ldp x0, x1, [sp, #(8 * 0)]
+ add sp, sp, #(8 * 2)
+1: .org __spectre_bhb_loop_k8 + __SPECTRE_BHB_LOOP_SZ
+ .org 1b
+SYM_DATA_END(__spectre_bhb_loop_k8)
+
+ .global __spectre_bhb_loop_k24
+SYM_DATA_START(__spectre_bhb_loop_k24)
+ esb
+ sub sp, sp, #(8 * 2)
+ stp x0, x1, [sp, #(8 * 0)]
+ mov x0, #8
+2: b . + 4
+ subs x0, x0, #1
+ b.ne 2b
+ dsb nsh
+ isb
+ ldp x0, x1, [sp, #(8 * 0)]
+ add sp, sp, #(8 * 2)
+1: .org __spectre_bhb_loop_k24 + __SPECTRE_BHB_LOOP_SZ
+ .org 1b
+SYM_DATA_END(__spectre_bhb_loop_k24)
+
+ .global __spectre_bhb_loop_k32
+SYM_DATA_START(__spectre_bhb_loop_k32)
+ esb
+ sub sp, sp, #(8 * 2)
+ stp x0, x1, [sp, #(8 * 0)]
+ mov x0, #8
+2: b . + 4
+ subs x0, x0, #1
+ b.ne 2b
+ dsb nsh
+ isb
+ ldp x0, x1, [sp, #(8 * 0)]
+ add sp, sp, #(8 * 2)
+1: .org __spectre_bhb_loop_k32 + __SPECTRE_BHB_LOOP_SZ
+ .org 1b
+SYM_DATA_END(__spectre_bhb_loop_k32)
Hi!
What is going on here?
> commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
Upstream commit 5bdf is very different from this. In particular,
> arch/arm64/kvm/hyp/smccc_wa.S | 66 +++++++++++++++++++++++++++++++++++++++
I can't find smccc_wa.S, neither in mainline, nor in -next. And it
looks buggy. I suspect loop_k24 should loop 24 times, but it does 8
loops AFAICT. Same problem with loop_k32.
Best regards,
Pavel
> --- a/arch/arm64/kvm/hyp/smccc_wa.S
> +++ b/arch/arm64/kvm/hyp/smccc_wa.S
> +
> + .global __spectre_bhb_loop_k24
> +SYM_DATA_START(__spectre_bhb_loop_k24)
> + esb
> + sub sp, sp, #(8 * 2)
> + stp x0, x1, [sp, #(8 * 0)]
> + mov x0, #8
> +2: b . + 4
> + subs x0, x0, #1
> + b.ne 2b
> + dsb nsh
> + isb
> + ldp x0, x1, [sp, #(8 * 0)]
> + add sp, sp, #(8 * 2)
> +1: .org __spectre_bhb_loop_k24 + __SPECTRE_BHB_LOOP_SZ
> + .org 1b
> +SYM_DATA_END(__spectre_bhb_loop_k24)
> +
> + .global __spectre_bhb_loop_k32
> +SYM_DATA_START(__spectre_bhb_loop_k32)
> + esb
> + sub sp, sp, #(8 * 2)
> + stp x0, x1, [sp, #(8 * 0)]
> + mov x0, #8
> +2: b . + 4
> + subs x0, x0, #1
> + b.ne 2b
> + dsb nsh
> + isb
> + ldp x0, x1, [sp, #(8 * 0)]
> + add sp, sp, #(8 * 2)
> +1: .org __spectre_bhb_loop_k32 + __SPECTRE_BHB_LOOP_SZ
> + .org 1b
> +SYM_DATA_END(__spectre_bhb_loop_k32)
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
On Fri, Mar 11, 2022 at 12:48:59AM +0100, Pavel Machek wrote:
> Hi!
>
> What is going on here?
>
> > commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
>
> Upstream commit 5bdf is very different from this. In particular,
>
> > arch/arm64/kvm/hyp/smccc_wa.S | 66 +++++++++++++++++++++++++++++++++++++++
>
> I can't find smccc_wa.S, neither in mainline, nor in -next. And it
> looks buggy. I suspect loop_k24 should loop 24 times, but it does 8
> loops AFAICT. Same problem with loop_k32.
The kvm portion of these patches is the "trickiest" portions. I'll let
James explain them, as he did so to me when sending the backports.
thanks,
greg k-h
On Tue, Mar 15, 2022 at 12:27:29PM +0000, James Morse wrote:
> On 3/15/22 12:20 PM, James Morse wrote:
> > On 3/11/22 6:42 AM, Greg Kroah-Hartman wrote:
> > > On Fri, Mar 11, 2022 at 12:48:59AM +0100, Pavel Machek wrote:
> > > > What is going on here?
> > > >
> > > > > commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
> > > >
> > > > Upstream commit 5bdf is very different from this. In particular,
> > > >
> > > > > ? arch/arm64/kvm/hyp/smccc_wa.S??? |?? 66 +++++++++++++++++++++++++++++++++++++++
> > > >
> > > > I can't find smccc_wa.S, neither in mainline, nor in -next. And it
> > > > looks buggy. I suspect loop_k24 should loop 24 times, but it does 8
> > > > loops AFAICT. Same problem with loop_k32.
> >
> > Yup, that's a bug. Thanks for spotting it!
> > I'll post a replacement for this patch.
>
> Looking more closely at this: when I originally did this the 'new' code for stable was
> in a separate patch to make it clear it was new code. Here it looks like Greg has merged
> it into this patch.
I did? I don't recall doing that at all, sorry. But getting the 5.10
arm64 tree into the stable queue was not easy from what I recall.
> I'm not sure what the 'rules' are for this sort of thing, obviously Greg gets the last say.
>
> I'll try and restructure the other backports to look like this, it is certainly simpler
> than trrying to pull all the prerequisites for all the upstream patches in.
I tried to also take a lot of prerequisite patches for the cpu id stuff,
to make those merges easier, so I have no problem with taking what is in
Linus's tree to make backports simpler. But it's a balencing act,
especially for tougher stuff like this :(
thanks,
greg k-h
On 3/15/22 12:20 PM, James Morse wrote:
> On 3/11/22 6:42 AM, Greg Kroah-Hartman wrote:
>> On Fri, Mar 11, 2022 at 12:48:59AM +0100, Pavel Machek wrote:
>>> What is going on here?
>>>
>>>> commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
>>>
>>> Upstream commit 5bdf is very different from this. In particular,
>>>
>>>> arch/arm64/kvm/hyp/smccc_wa.S | 66 +++++++++++++++++++++++++++++++++++++++
>>>
>>> I can't find smccc_wa.S, neither in mainline, nor in -next. And it
>>> looks buggy. I suspect loop_k24 should loop 24 times, but it does 8
>>> loops AFAICT. Same problem with loop_k32.
>
> Yup, that's a bug. Thanks for spotting it!
> I'll post a replacement for this patch.
Looking more closely at this: when I originally did this the 'new' code for stable was
in a separate patch to make it clear it was new code. Here it looks like Greg has merged
it into this patch.
I'm not sure what the 'rules' are for this sort of thing, obviously Greg gets the last say.
I'll try and restructure the other backports to look like this, it is certainly simpler
than trrying to pull all the prerequisites for all the upstream patches in.
Thanks,
James
Hi guys,
On 3/11/22 6:42 AM, Greg Kroah-Hartman wrote:
> On Fri, Mar 11, 2022 at 12:48:59AM +0100, Pavel Machek wrote:
>> What is going on here?
>>
>>> commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
>>
>> Upstream commit 5bdf is very different from this. In particular,
>>
>>> arch/arm64/kvm/hyp/smccc_wa.S | 66 +++++++++++++++++++++++++++++++++++++++
>>
>> I can't find smccc_wa.S, neither in mainline, nor in -next. And it
>> looks buggy. I suspect loop_k24 should loop 24 times, but it does 8
>> loops AFAICT. Same problem with loop_k32.
Yup, that's a bug. Thanks for spotting it!
I'll post a replacement for this patch.
I only have A57 I can test this on, guess what its K value is.
> The kvm portion of these patches is the "trickiest" portions. I'll let
> James explain them, as he did so to me when sending the backports.
KVM gets re-written fairly frequently. Earlier kernels don't have any of the infrastructure
for generating the vectors at compile time and selecting a pre-built vector at boot. Instead,
kernels of this vintage have bunch of empty vectors, and some templates they use to create
the appropriate vector at boot. See commit b881cdce77b4.
I've looked at backporting all that - its about 60 patches. I don't think its a good idea.
Thanks,
James
On Tue, Mar 15, 2022 at 12:20:31PM +0000, James Morse wrote:
> Hi guys,
>
> On 3/11/22 6:42 AM, Greg Kroah-Hartman wrote:
> > On Fri, Mar 11, 2022 at 12:48:59AM +0100, Pavel Machek wrote:
> > > What is going on here?
> > >
> > > > commit 5bdf3437603d4af87f9c7f424b0c8aeed2420745 upstream.
> > >
> > > Upstream commit 5bdf is very different from this. In particular,
> > >
> > > > arch/arm64/kvm/hyp/smccc_wa.S | 66 +++++++++++++++++++++++++++++++++++++++
> > >
> > > I can't find smccc_wa.S, neither in mainline, nor in -next. And it
> > > looks buggy. I suspect loop_k24 should loop 24 times, but it does 8
> > > loops AFAICT. Same problem with loop_k32.
>
> Yup, that's a bug. Thanks for spotting it!
> I'll post a replacement for this patch.
That's going to be hard as this is now in a merged tree :(
Can you just send a fixup?
thanks,
greg k-h