]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
KVM: arm64: Eagerly switch ZCR_EL{1,2}
authorMark Rutland <mark.rutland@arm.com>
Fri, 4 Apr 2025 13:23:45 +0000 (14:23 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 25 Apr 2025 08:44:00 +0000 (10:44 +0200)
[ Upstream commit 59419f10045bc955d2229819c7cf7a8b0b9c5b59 ]

In non-protected KVM modes, while the guest FPSIMD/SVE/SME state is live on the
CPU, the host's active SVE VL may differ from the guest's maximum SVE VL:

* For VHE hosts, when a VM uses NV, ZCR_EL2 contains a value constrained
  by the guest hypervisor, which may be less than or equal to that
  guest's maximum VL.

  Note: in this case the value of ZCR_EL1 is immaterial due to E2H.

* For nVHE/hVHE hosts, ZCR_EL1 contains a value written by the guest,
  which may be less than or greater than the guest's maximum VL.

  Note: in this case hyp code traps host SVE usage and lazily restores
  ZCR_EL2 to the host's maximum VL, which may be greater than the
  guest's maximum VL.

This can be the case between exiting a guest and kvm_arch_vcpu_put_fp().
If a softirq is taken during this period and the softirq handler tries
to use kernel-mode NEON, then the kernel will fail to save the guest's
FPSIMD/SVE state, and will pend a SIGKILL for the current thread.

This happens because kvm_arch_vcpu_ctxsync_fp() binds the guest's live
FPSIMD/SVE state with the guest's maximum SVE VL, and
fpsimd_save_user_state() verifies that the live SVE VL is as expected
before attempting to save the register state:

| if (WARN_ON(sve_get_vl() != vl)) {
|         force_signal_inject(SIGKILL, SI_KERNEL, 0, 0);
|         return;
| }

Fix this and make this a bit easier to reason about by always eagerly
switching ZCR_EL{1,2} at hyp during guest<->host transitions. With this
happening, there's no need to trap host SVE usage, and the nVHE/nVHE
__deactivate_cptr_traps() logic can be simplified to enable host access
to all present FPSIMD/SVE/SME features.

In protected nVHE/hVHE modes, the host's state is always saved/restored
by hyp, and the guest's state is saved prior to exit to the host, so
from the host's PoV the guest never has live FPSIMD/SVE/SME state, and
the host's ZCR_EL1 is never clobbered by hyp.

Fixes: 8c8010d69c132273 ("KVM: arm64: Save/restore SVE state for nVHE")
Fixes: 2e3cf82063a00ea0 ("KVM: arm64: nv: Ensure correct VL is loaded before saving SVE state")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Tested-by: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Fuad Tabba <tabba@google.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
Link: https://lore.kernel.org/r/20250210195226.1215254-9-mark.rutland@arm.com
Signed-off-by: Marc Zyngier <maz@kernel.org>
[ v6.6 lacks pKVM saving of host SVE state, pull in discovery of maximum
  host VL separately -- broonie ]
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
arch/arm64/include/asm/kvm_host.h
arch/arm64/include/asm/kvm_hyp.h
arch/arm64/kvm/fpsimd.c
arch/arm64/kvm/hyp/entry.S
arch/arm64/kvm/hyp/include/hyp/switch.h
arch/arm64/kvm/hyp/nvhe/hyp-main.c
arch/arm64/kvm/hyp/nvhe/pkvm.c
arch/arm64/kvm/hyp/nvhe/switch.c
arch/arm64/kvm/hyp/vhe/switch.c
arch/arm64/kvm/reset.c

index c13a0d5907e8756cbbf458847403bab78de7947c..0935f9849510471ab29c1bcc5fa584557a852dfe 100644 (file)
@@ -67,6 +67,7 @@ enum kvm_mode kvm_get_mode(void);
 DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
 
 extern unsigned int kvm_sve_max_vl;
+extern unsigned int kvm_host_sve_max_vl;
 int kvm_arm_init_sve(void);
 
 u32 __attribute_const__ kvm_target_cpu(void);
index aa7fa2a08f0604af5b25f2eb0f334f4a716b4431..1d0bb7624a1c073b5deffff7d851e581a6433c45 100644 (file)
@@ -122,5 +122,6 @@ extern u64 kvm_nvhe_sym(id_aa64isar2_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64mmfr0_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64mmfr1_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64mmfr2_el1_sys_val);
+extern unsigned int kvm_nvhe_sym(kvm_host_sve_max_vl);
 
 #endif /* __ARM64_KVM_HYP_H__ */
index df050e4d3562d2ef36b7c27602a6feaa431dfa93..3fd86b71ee379e6e4d9b53e0634bea6f37d3be99 100644 (file)
@@ -148,15 +148,16 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
        local_irq_save(flags);
 
        if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
-               if (vcpu_has_sve(vcpu)) {
-                       __vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
-
-                       /* Restore the VL that was saved when bound to the CPU */
-                       if (!has_vhe())
-                               sve_cond_update_zcr_vq(vcpu_sve_max_vq(vcpu) - 1,
-                                                      SYS_ZCR_EL1);
-               }
-
+               /*
+                * Flush (save and invalidate) the fpsimd/sve state so that if
+                * the host tries to use fpsimd/sve, it's not using stale data
+                * from the guest.
+                *
+                * Flushing the state sets the TIF_FOREIGN_FPSTATE bit for the
+                * context unconditionally, in both nVHE and VHE. This allows
+                * the kernel to restore the fpsimd/sve state, including ZCR_EL1
+                * when needed.
+                */
                fpsimd_save_and_flush_cpu_state();
        }
 
index 435346ea1504e158f7877499514118d39400f247..d8c94c45cb2f2f815d0f5e9e58f9fd4e6eb572f2 100644 (file)
@@ -44,6 +44,11 @@ alternative_if ARM64_HAS_RAS_EXTN
 alternative_else_nop_endif
        mrs     x1, isr_el1
        cbz     x1,  1f
+
+       // Ensure that __guest_enter() always provides a context
+       // synchronization event so that callers don't need ISBs for anything
+       // that would usually be synchonized by the ERET.
+       isb
        mov     x0, #ARM_EXCEPTION_IRQ
        ret
 
index 0db90cb47308453caafa214c3ee337ce990a76e2..275176e61d748811e8e6b55b9756409c4bf2d719 100644 (file)
@@ -167,6 +167,61 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)
        write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR);
 }
 
+static inline void fpsimd_lazy_switch_to_guest(struct kvm_vcpu *vcpu)
+{
+       u64 zcr_el1, zcr_el2;
+
+       if (!guest_owns_fp_regs(vcpu))
+               return;
+
+       if (vcpu_has_sve(vcpu)) {
+               zcr_el2 = vcpu_sve_max_vq(vcpu) - 1;
+
+               write_sysreg_el2(zcr_el2, SYS_ZCR);
+
+               zcr_el1 = __vcpu_sys_reg(vcpu, ZCR_EL1);
+               write_sysreg_el1(zcr_el1, SYS_ZCR);
+       }
+}
+
+static inline void fpsimd_lazy_switch_to_host(struct kvm_vcpu *vcpu)
+{
+       u64 zcr_el1, zcr_el2;
+
+       if (!guest_owns_fp_regs(vcpu))
+               return;
+
+       /*
+        * When the guest owns the FP regs, we know that guest+hyp traps for
+        * any FPSIMD/SVE/SME features exposed to the guest have been disabled
+        * by either fpsimd_lazy_switch_to_guest() or kvm_hyp_handle_fpsimd()
+        * prior to __guest_entry(). As __guest_entry() guarantees a context
+        * synchronization event, we don't need an ISB here to avoid taking
+        * traps for anything that was exposed to the guest.
+        */
+       if (vcpu_has_sve(vcpu)) {
+               zcr_el1 = read_sysreg_el1(SYS_ZCR);
+               __vcpu_sys_reg(vcpu, ZCR_EL1) = zcr_el1;
+
+               /*
+                * The guest's state is always saved using the guest's max VL.
+                * Ensure that the host has the guest's max VL active such that
+                * the host can save the guest's state lazily, but don't
+                * artificially restrict the host to the guest's max VL.
+                */
+               if (has_vhe()) {
+                       zcr_el2 = vcpu_sve_max_vq(vcpu) - 1;
+                       write_sysreg_el2(zcr_el2, SYS_ZCR);
+               } else {
+                       zcr_el2 = sve_vq_from_vl(kvm_host_sve_max_vl) - 1;
+                       write_sysreg_el2(zcr_el2, SYS_ZCR);
+
+                       zcr_el1 = vcpu_sve_max_vq(vcpu) - 1;
+                       write_sysreg_el1(zcr_el1, SYS_ZCR);
+               }
+       }
+}
+
 /*
  * We trap the first access to the FP/SIMD to save the host context and
  * restore the guest context lazily.
index 3cea4b6ac23ec114c037348fee1363ad25edeff6..b183cc866404633cd17f6603a5d4626574e56021 100644 (file)
@@ -5,6 +5,7 @@
  */
 
 #include <hyp/adjust_pc.h>
+#include <hyp/switch.h>
 
 #include <asm/pgtable-types.h>
 #include <asm/kvm_asm.h>
@@ -25,7 +26,9 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 {
        DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
 
+       fpsimd_lazy_switch_to_guest(kern_hyp_va(vcpu));
        cpu_reg(host_ctxt, 1) =  __kvm_vcpu_run(kern_hyp_va(vcpu));
+       fpsimd_lazy_switch_to_host(kern_hyp_va(vcpu));
 }
 
 static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt)
@@ -285,11 +288,6 @@ void handle_trap(struct kvm_cpu_context *host_ctxt)
        case ESR_ELx_EC_SMC64:
                handle_host_smc(host_ctxt);
                break;
-       case ESR_ELx_EC_SVE:
-               sysreg_clear_set(cptr_el2, CPTR_EL2_TZ, 0);
-               isb();
-               sve_cond_update_zcr_vq(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);
-               break;
        case ESR_ELx_EC_IABT_LOW:
        case ESR_ELx_EC_DABT_LOW:
                handle_host_mem_abort(host_ctxt);
index 93586bf80ec9f2bab44625253fed71654e22c87c..6042cdd3d887709888d997dd6d4ff888fdf9f715 100644 (file)
@@ -9,6 +9,8 @@
 #include <nvhe/fixed_config.h>
 #include <nvhe/trap_handler.h>
 
+unsigned int kvm_host_sve_max_vl;
+
 /*
  * Set trap register values based on features in ID_AA64PFR0.
  */
index 58171926f9ba23844997ff02406518a312eb8bb7..47c7f3a675aec8c140aec24732fa46ebbadbf2af 100644 (file)
@@ -40,6 +40,9 @@ static void __activate_cptr_traps(struct kvm_vcpu *vcpu)
 {
        u64 val = CPTR_EL2_TAM; /* Same bit irrespective of E2H */
 
+       if (!guest_owns_fp_regs(vcpu))
+               __activate_traps_fpsimd32(vcpu);
+
        /* !hVHE case upstream */
        if (1) {
                val |= CPTR_EL2_TTA | CPTR_NVHE_EL2_RES1;
@@ -55,12 +58,24 @@ static void __activate_cptr_traps(struct kvm_vcpu *vcpu)
 
                if (!guest_owns_fp_regs(vcpu))
                        val |= CPTR_EL2_TFP;
+
+               write_sysreg(val, cptr_el2);
        }
+}
 
-       if (!guest_owns_fp_regs(vcpu))
-               __activate_traps_fpsimd32(vcpu);
+static void __deactivate_cptr_traps(struct kvm_vcpu *vcpu)
+{
+       /* !hVHE case upstream */
+       if (1) {
+               u64 val = CPTR_NVHE_EL2_RES1;
 
-       write_sysreg(val, cptr_el2);
+               if (!cpus_have_final_cap(ARM64_SVE))
+                       val |= CPTR_EL2_TZ;
+               if (!cpus_have_final_cap(ARM64_SME))
+                       val |= CPTR_EL2_TSM;
+
+               write_sysreg(val, cptr_el2);
+       }
 }
 
 static void __activate_traps(struct kvm_vcpu *vcpu)
@@ -89,7 +104,6 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 static void __deactivate_traps(struct kvm_vcpu *vcpu)
 {
        extern char __kvm_hyp_host_vector[];
-       u64 cptr;
 
        ___deactivate_traps(vcpu);
 
@@ -114,13 +128,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 
        write_sysreg(this_cpu_ptr(&kvm_init_params)->hcr_el2, hcr_el2);
 
-       cptr = CPTR_EL2_DEFAULT;
-       if (vcpu_has_sve(vcpu) && (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED))
-               cptr |= CPTR_EL2_TZ;
-       if (cpus_have_final_cap(ARM64_SME))
-               cptr &= ~CPTR_EL2_TSM;
-
-       write_sysreg(cptr, cptr_el2);
+       __deactivate_cptr_traps(vcpu);
        write_sysreg(__kvm_hyp_host_vector, vbar_el2);
 }
 
index f24569ac26c22978fc991221a9f5435e64cf4db2..179152bb9e4286c4398c0439feede87e755bc26f 100644 (file)
@@ -134,6 +134,8 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
        sysreg_save_host_state_vhe(host_ctxt);
 
+       fpsimd_lazy_switch_to_guest(vcpu);
+
        /*
         * ARM erratum 1165522 requires us to configure both stage 1 and
         * stage 2 translation for the guest context before we clear
@@ -164,6 +166,8 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
        __deactivate_traps(vcpu);
 
+       fpsimd_lazy_switch_to_host(vcpu);
+
        sysreg_restore_host_state_vhe(host_ctxt);
 
        if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
index f9d070473614e5d8d52c0ff9b90f48c7a9100791..54e00ee631a052fa60979f70ebab47f1faeacbde 100644 (file)
@@ -42,11 +42,14 @@ static u32 kvm_ipa_limit;
                                 PSR_AA32_I_BIT | PSR_AA32_F_BIT)
 
 unsigned int kvm_sve_max_vl;
+unsigned int kvm_host_sve_max_vl;
 
 int kvm_arm_init_sve(void)
 {
        if (system_supports_sve()) {
                kvm_sve_max_vl = sve_max_virtualisable_vl();
+               kvm_host_sve_max_vl = sve_max_vl();
+               kvm_nvhe_sym(kvm_host_sve_max_vl) = kvm_host_sve_max_vl;
 
                /*
                 * The get_sve_reg()/set_sve_reg() ioctl interface will need