From: Mark Rutland <mark.rutland@arm.com>
To: linux-arm-kernel@lists.infradead.org
Cc: broonie@kernel.org, catalin.marinas@arm.com, eauger@redhat.com,
eric.auger@redhat.com, fweimer@redhat.com, jeremy.linton@arm.com,
mark.rutland@arm.com, maz@kernel.org, oliver.upton@linux.dev,
pbonzini@redhat.com, stable@vger.kernel.org, tabba@google.com,
wilco.dijkstra@arm.com, will@kernel.org
Subject: [PATCH v2 1/8] KVM: arm64: Unconditionally save+flush host FPSIMD/SVE/SME state
Date: Thu, 6 Feb 2025 14:10:55 +0000 [thread overview]
Message-ID: <20250206141102.954688-2-mark.rutland@arm.com> (raw)
In-Reply-To: <20250206141102.954688-1-mark.rutland@arm.com>
There are several problems with the way hyp code lazily saves the host's
FPSIMD/SVE state, including:
* Host SVE being discarded unexpectedly due to inconsistent
configuration of TIF_SVE and CPACR_ELx.ZEN. This has been seen to
result in QEMU crashes where SVE is used by memmove(), as reported by
Eric Auger:
https://issues.redhat.com/browse/RHEL-68997
* Host SVE state is discarded *after* modification by ptrace, which was an
unintentional ptrace ABI change introduced with lazy discarding of SVE state.
* The host FPMR value can be discarded when running a non-protected VM,
where FPMR support is not exposed to a VM, and that VM uses
FPSIMD/SVE. In these cases the hyp code does not save the host's FPMR
before unbinding the host's FPSIMD/SVE/SME state, leaving a stale
value in memory.
Avoid these by eagerly saving and "flushing" the host's FPSIMD/SVE/SME
state when loading a vCPU such that KVM does not need to save any of the
host's FPSIMD/SVE/SME state. For clarity, fpsimd_kvm_prepare() is
removed and the necessary call to fpsimd_save_and_flush_cpu_state() is
placed in kvm_arch_vcpu_load_fp(). As 'fpsimd_state' and 'fpmr_ptr'
should not be used, they are set to NULL; all uses of these will be
removed in subsequent patches.
Historical problems go back at least as far as v5.17, e.g. erroneous
assumptions about TIF_SVE being clear in commit:
8383741ab2e773a9 ("KVM: arm64: Get rid of host SVE tracking/saving")
... and so this eager save+flush probably needs to be backported to ALL
stable trees.
Fixes: 93ae6b01bafee8fa ("KVM: arm64: Discard any SVE state when entering KVM guests")
Fixes: 8c845e2731041f0f ("arm64/sve: Leave SVE enabled on syscall if we don't context switch")
Fixes: ef3be86021c3bdf3 ("KVM: arm64: Add save/restore support for FPMR")
Reported-by: Eric Auger <eauger@redhat.com>
Reported-by: Wilco Dijkstra <wilco.dijkstra@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Tested-by: Eric Auger <eric.auger@redhat.com>
Cc: stable@vger.kernel.org
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Fuad Tabba <tabba@google.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/kernel/fpsimd.c | 25 -------------------------
arch/arm64/kvm/fpsimd.c | 35 ++++++++++-------------------------
2 files changed, 10 insertions(+), 50 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 2b601d88762d4..8370d55f03533 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1694,31 +1694,6 @@ void fpsimd_signal_preserve_current_state(void)
sve_to_fpsimd(current);
}
-/*
- * Called by KVM when entering the guest.
- */
-void fpsimd_kvm_prepare(void)
-{
- if (!system_supports_sve())
- return;
-
- /*
- * KVM does not save host SVE state since we can only enter
- * the guest from a syscall so the ABI means that only the
- * non-saved SVE state needs to be saved. If we have left
- * SVE enabled for performance reasons then update the task
- * state to be FPSIMD only.
- */
- get_cpu_fpsimd_context();
-
- if (test_and_clear_thread_flag(TIF_SVE)) {
- sve_to_fpsimd(current);
- current->thread.fp_type = FP_STATE_FPSIMD;
- }
-
- put_cpu_fpsimd_context();
-}
-
/*
* Associate current's FPSIMD context with this cpu
* The caller must have ownership of the cpu FPSIMD context before calling
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index 4d3d1a2eb1570..ceeb0a4893aa7 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -54,16 +54,18 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
if (!system_supports_fpsimd())
return;
- fpsimd_kvm_prepare();
-
/*
- * We will check TIF_FOREIGN_FPSTATE just before entering the
- * guest in kvm_arch_vcpu_ctxflush_fp() and override this to
- * FP_STATE_FREE if the flag set.
+ * Ensure that any host FPSIMD/SVE/SME state is saved and unbound such
+ * that the host kernel is responsible for restoring this state upon
+ * return to userspace, and the hyp code doesn't need to save anything.
+ *
+ * When the host may use SME, fpsimd_save_and_flush_cpu_state() ensures
+ * that PSTATE.{SM,ZA} == {0,0}.
*/
- *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
- *host_data_ptr(fpsimd_state) = kern_hyp_va(¤t->thread.uw.fpsimd_state);
- *host_data_ptr(fpmr_ptr) = kern_hyp_va(¤t->thread.uw.fpmr);
+ fpsimd_save_and_flush_cpu_state();
+ *host_data_ptr(fp_owner) = FP_STATE_FREE;
+ *host_data_ptr(fpsimd_state) = NULL;
+ *host_data_ptr(fpmr_ptr) = NULL;
host_data_clear_flag(HOST_SVE_ENABLED);
if (read_sysreg(cpacr_el1) & CPACR_EL1_ZEN_EL0EN)
@@ -73,23 +75,6 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
host_data_clear_flag(HOST_SME_ENABLED);
if (read_sysreg(cpacr_el1) & CPACR_EL1_SMEN_EL0EN)
host_data_set_flag(HOST_SME_ENABLED);
-
- /*
- * If PSTATE.SM is enabled then save any pending FP
- * state and disable PSTATE.SM. If we leave PSTATE.SM
- * enabled and the guest does not enable SME via
- * CPACR_EL1.SMEN then operations that should be valid
- * may generate SME traps from EL1 to EL1 which we
- * can't intercept and which would confuse the guest.
- *
- * Do the same for PSTATE.ZA in the case where there
- * is state in the registers which has not already
- * been saved, this is very unlikely to happen.
- */
- if (read_sysreg_s(SYS_SVCR) & (SVCR_SM_MASK | SVCR_ZA_MASK)) {
- *host_data_ptr(fp_owner) = FP_STATE_FREE;
- fpsimd_save_and_flush_cpu_state();
- }
}
/*
--
2.30.2
next prev parent reply other threads:[~2025-02-06 14:17 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-06 14:10 [PATCH v2 0/8] KVM: arm64: FPSIMD/SVE/SME fixes Mark Rutland
2025-02-06 14:10 ` Mark Rutland [this message]
2025-02-07 12:27 ` [PATCH v2 1/8] KVM: arm64: Unconditionally save+flush host FPSIMD/SVE/SME state Will Deacon
2025-02-07 13:21 ` Mark Rutland
2025-02-10 10:53 ` Marc Zyngier
2025-02-10 15:05 ` Will Deacon
2025-02-06 14:10 ` [PATCH v2 2/8] KVM: arm64: Remove host FPSIMD saving for non-protected KVM Mark Rutland
2025-02-10 16:12 ` Will Deacon
2025-02-10 16:59 ` Mark Rutland
2025-02-10 18:06 ` Will Deacon
2025-02-10 20:03 ` Mark Rutland
2025-02-11 19:08 ` Mark Rutland
2025-02-06 14:10 ` [PATCH v2 3/8] KVM: arm64: Remove VHE host restore of CPACR_EL1.ZEN Mark Rutland
2025-02-10 16:14 ` Will Deacon
2025-02-06 14:10 ` [PATCH v2 4/8] KVM: arm64: Remove VHE host restore of CPACR_EL1.SMEN Mark Rutland
2025-02-10 16:16 ` Will Deacon
2025-02-06 14:10 ` [PATCH v2 5/8] KVM: arm64: Refactor CPTR trap deactivation Mark Rutland
2025-02-10 16:34 ` Will Deacon
2025-02-06 14:11 ` [PATCH v2 6/8] KVM: arm64: Refactor exit handlers Mark Rutland
2025-02-10 16:37 ` Will Deacon
2025-02-06 14:11 ` [PATCH v2 7/8] KVM: arm64: Mark some header functions as inline Mark Rutland
2025-02-10 16:39 ` Will Deacon
2025-02-06 14:11 ` [PATCH v2 8/8] KVM: arm64: Eagerly switch ZCR_EL{1,2} Mark Rutland
2025-02-06 19:12 ` Mark Brown
2025-02-07 9:34 ` Mark Rutland
2025-02-10 16:53 ` Will Deacon
2025-02-10 17:21 ` Mark Rutland
2025-02-10 18:20 ` Will Deacon
2025-02-10 18:56 ` Mark Rutland
2025-02-11 10:29 ` Will Deacon
2025-02-08 0:27 ` [PATCH v2 0/8] KVM: arm64: FPSIMD/SVE/SME fixes Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250206141102.954688-2-mark.rutland@arm.com \
--to=mark.rutland@arm.com \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=eauger@redhat.com \
--cc=eric.auger@redhat.com \
--cc=fweimer@redhat.com \
--cc=jeremy.linton@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=pbonzini@redhat.com \
--cc=stable@vger.kernel.org \
--cc=tabba@google.com \
--cc=wilco.dijkstra@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.