Skip to content

Commit a11fbd7

Browse files
committed
Merge: KVM: arm64: Unconditionally save+flush host FPSIMD/SVE/SME state
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/6397 JIRA: https://issues.redhat.com/browse/RHEL-68997 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: 8383741 ("KVM: arm64: Get rid of host SVE tracking/saving") ... and so this eager save+flush probably needs to be backported to ALL stable trees. Signed-off-by: Eric Auger <[email protected]> Approved-by: Cornelia Huck <[email protected]> Approved-by: Gavin Shan <[email protected]> Approved-by: Sebastian Ott <[email protected]> Approved-by: Donald Dutile <[email protected]> Approved-by: CKI KWF Bot <[email protected]> Merged-by: Augusto Caringi <[email protected]>
2 parents 3099640 + 0f7c491 commit a11fbd7

File tree

2 files changed

+9
-49
lines changed

2 files changed

+9
-49
lines changed

arch/arm64/kernel/fpsimd.c

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,31 +1696,6 @@ void fpsimd_signal_preserve_current_state(void)
16961696
sve_to_fpsimd(current);
16971697
}
16981698

1699-
/*
1700-
* Called by KVM when entering the guest.
1701-
*/
1702-
void fpsimd_kvm_prepare(void)
1703-
{
1704-
if (!system_supports_sve())
1705-
return;
1706-
1707-
/*
1708-
* KVM does not save host SVE state since we can only enter
1709-
* the guest from a syscall so the ABI means that only the
1710-
* non-saved SVE state needs to be saved. If we have left
1711-
* SVE enabled for performance reasons then update the task
1712-
* state to be FPSIMD only.
1713-
*/
1714-
get_cpu_fpsimd_context();
1715-
1716-
if (test_and_clear_thread_flag(TIF_SVE)) {
1717-
sve_to_fpsimd(current);
1718-
current->thread.fp_type = FP_STATE_FPSIMD;
1719-
}
1720-
1721-
put_cpu_fpsimd_context();
1722-
}
1723-
17241699
/*
17251700
* Associate current's FPSIMD context with this cpu
17261701
* The caller must have ownership of the cpu FPSIMD context before calling

arch/arm64/kvm/fpsimd.c

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,17 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
5454
if (!system_supports_fpsimd())
5555
return;
5656

57-
fpsimd_kvm_prepare();
58-
5957
/*
60-
* We will check TIF_FOREIGN_FPSTATE just before entering the
61-
* guest in kvm_arch_vcpu_ctxflush_fp() and override this to
62-
* FP_STATE_FREE if the flag set.
58+
* Ensure that any host FPSIMD/SVE/SME state is saved and unbound such
59+
* that the host kernel is responsible for restoring this state upon
60+
* return to userspace, and the hyp code doesn't need to save anything.
61+
*
62+
* When the host may use SME, fpsimd_save_and_flush_cpu_state() ensures
63+
* that PSTATE.{SM,ZA} == {0,0}.
6364
*/
64-
*host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
65-
*host_data_ptr(fpsimd_state) = kern_hyp_va(&current->thread.uw.fpsimd_state);
65+
fpsimd_save_and_flush_cpu_state();
66+
*host_data_ptr(fp_owner) = FP_STATE_FREE;
67+
*host_data_ptr(fpsimd_state) = NULL;
6668

6769
vcpu_clear_flag(vcpu, HOST_SVE_ENABLED);
6870
if (read_sysreg(cpacr_el1) & CPACR_EL1_ZEN_EL0EN)
@@ -72,23 +74,6 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
7274
vcpu_clear_flag(vcpu, HOST_SME_ENABLED);
7375
if (read_sysreg(cpacr_el1) & CPACR_EL1_SMEN_EL0EN)
7476
vcpu_set_flag(vcpu, HOST_SME_ENABLED);
75-
76-
/*
77-
* If PSTATE.SM is enabled then save any pending FP
78-
* state and disable PSTATE.SM. If we leave PSTATE.SM
79-
* enabled and the guest does not enable SME via
80-
* CPACR_EL1.SMEN then operations that should be valid
81-
* may generate SME traps from EL1 to EL1 which we
82-
* can't intercept and which would confuse the guest.
83-
*
84-
* Do the same for PSTATE.ZA in the case where there
85-
* is state in the registers which has not already
86-
* been saved, this is very unlikely to happen.
87-
*/
88-
if (read_sysreg_s(SYS_SVCR) & (SVCR_SM_MASK | SVCR_ZA_MASK)) {
89-
*host_data_ptr(fp_owner) = FP_STATE_FREE;
90-
fpsimd_save_and_flush_cpu_state();
91-
}
9277
}
9378

9479
/*

0 commit comments

Comments
 (0)