On 2020-11-13 11:38, Will Deacon wrote:
The EL2 vectors installed when a guest is running point at one of the
following configurations for a given CPU:
- Straight at __kvm_hyp_vector
- A trampoline containing an SMC sequence to mitigate Spectre-v2 and
then a direct branch to __kvm_hyp_vector
- A dynamically-allocated trampoline which has an indirect branch to
__kvm_hyp_vector
- A dynamically-allocated trampoline containing an SMC sequence to
mitigate Spectre-v2 and then an indirect branch to __kvm_hyp_vector
The indirect branches mean that VA randomization at EL2 isn't trivially
bypassable using Spectre-v3a (where the vector base is readable by the
guest).
Rather than populate these vectors dynamically, configure everything
statically and use an enumerated type to identify the vector "slot"
corresponding to one of the configurations above. This both simplifies
the code, but also makes it much easier to implement at EL2 later on.
Cc: Marc Zyngier <maz@xxxxxxxxxx>
Cc: Quentin Perret <qperret@xxxxxxxxxx>
Signed-off-by: Will Deacon <will@xxxxxxxxxx>
---
arch/arm64/include/asm/kvm_asm.h | 5 --
arch/arm64/include/asm/spectre.h | 36 +++++++++++++-
arch/arm64/kernel/cpu_errata.c | 2 +
arch/arm64/kernel/proton-pack.c | 63 +++++-------------------
arch/arm64/kvm/arm.c | 82 +++++++++++++-------------------
arch/arm64/kvm/hyp/Makefile | 2 +-
arch/arm64/kvm/hyp/hyp-entry.S | 72 ++++++++++++++++------------
arch/arm64/kvm/hyp/smccc_wa.S | 32 -------------
arch/arm64/kvm/va_layout.c | 11 +----
9 files changed, 126 insertions(+), 179 deletions(-)
delete mode 100644 arch/arm64/kvm/hyp/smccc_wa.S
I haven't had a chance to test this series yet, but I may have spotted
another small nit, see below:
[...]
+static int kvm_init_vector_slots(void)
+{
+ int err;
+ void *base;
+
+ base = kern_hyp_va(kvm_ksym_ref(__kvm_hyp_vector));
+ kvm_init_vector_slot(base, HYP_VECTOR_DIRECT);
+
+ base = kern_hyp_va(kvm_ksym_ref(__bp_harden_hyp_vecs));
+ kvm_init_vector_slot(base, HYP_VECTOR_SPECTRE_DIRECT);
- /*
- * SV2 = ARM64_SPECTRE_V2
- * HEL2 = ARM64_HARDEN_EL2_VECTORS
- *
- * !SV2 + !HEL2 -> use direct vectors
- * SV2 + !HEL2 -> use hardened vectors in place
- * !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_HARDEN_EL2_VECTORS))
return 0;
- /*
- * Always allocate a spare vector slot, as we don't know yet which
CPUs
- * have a BP hardening slot that we can reuse.
- */
- slot = atomic_inc_return(&arm64_el2_vector_last_slot);
- BUG_ON(slot >= BP_HARDEN_EL2_SLOTS);
- __kvm_harden_el2_vector_slot = slot;
+ if (!has_vhe()) {
+ err = create_hyp_exec_mappings(__pa_symbol(__bp_harden_hyp_vecs),
+ __BP_HARDEN_HYP_VECS_SZ, &base);
+ if (err)
+ return err;
+ }
- return create_hyp_exec_mappings(__pa_symbol(__bp_harden_hyp_vecs),
- __BP_HARDEN_HYP_VECS_SZ,
- &__kvm_bp_vect_base);
+ kvm_init_vector_slot(base, HYP_VECTOR_INDIRECT);
+ kvm_init_vector_slot(base, HYP_VECTOR_SPECTRE_INDIRECT);
+ return 0;
}
static void cpu_init_hyp_mode(void)
@@ -1406,24 +1403,9 @@ static void cpu_hyp_reset(void)
static void cpu_set_hyp_vector(void)
{
struct bp_hardening_data *data = this_cpu_ptr(&bp_hardening_data);
- void *vect = kern_hyp_va(kvm_ksym_ref(__kvm_hyp_vector));
- int slot = -1;
-
- if (cpus_have_const_cap(ARM64_SPECTRE_V2) && data->fn) {
- vect = kern_hyp_va(kvm_ksym_ref(__bp_harden_hyp_vecs));
- slot = data->hyp_vectors_slot;
- }
+ void *vector = hyp_spectre_vector_selector[data->slot];
- if (this_cpu_has_cap(ARM64_HARDEN_EL2_VECTORS) && !has_vhe()) {
- vect = __kvm_bp_vect_base;
- if (slot == -1)
- slot = __kvm_harden_el2_vector_slot;
- }
-
- if (slot != -1)
- vect += slot * SZ_2K;
-
- *this_cpu_ptr_hyp_sym(kvm_hyp_vector) = (unsigned long)vect;
+ *this_cpu_ptr_hyp_sym(kvm_hyp_vector) = (unsigned long)vector;
}
static void cpu_hyp_reinit(void)
@@ -1661,9 +1643,9 @@ static int init_hyp_mode(void)
goto out_err;
}
- err = kvm_map_vectors();
+ err = kvm_init_vector_slots();
if (err) {
- kvm_err("Cannot map vectors\n");
+ kvm_err("Cannot initialise vector slots\n");
goto out_err;
}
@@ -1810,6 +1792,10 @@ int kvm_arch_init(void *opaque)
goto out_err;
}
+ err = kvm_init_vector_slots();
+ if (err)
+ goto out_err;
Don't you end-up calling kvm_init_vector_slots() twice on nVHE?
It's probably harmless, but I think we can have a single call here,
and drop the call from init_hyp_mode().
What do you think? If you agree, I can perform the change when queuing
the series.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
