Re: [PATCH v2 2/8] KVM: arm64: Remove host FPSIMD saving for non-protected KVM

On Mon, Feb 10, 2025 at 06:06:38PM +0000, Will Deacon wrote:

On Mon, Feb 10, 2025 at 04:59:56PM +0000, Mark Rutland wrote:

quoted

On Mon, Feb 10, 2025 at 04:12:43PM +0000, Will Deacon wrote:

quoted

On Thu, Feb 06, 2025 at 02:10:56PM +0000, Mark Rutland wrote:

| static inline bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
| {
| 	...
| 
| 	/* Valid trap */
| 
| 	/*
| 	 * Enable everything EL2 might need to save/restore state.
| 	 * Maybe each of the bits should depend on system_has_xxx()
| 	 */
| 	cpacr_clear_set(0, CPACR_EL1_FPEN | CPACR_EL1_ZEN | CPACR_EL1_SMEN */
| 	isb();
| 
| 	...
| 
| 	/* Write out the host state if it's in the registers */
| 	if (is_protected_kvm_enabled() && host_owns_fp_regs())
| 		kvm_hyp_save_fpsimd_host(vcpu);
| 	
| 	/* Restore guest state */
| 
| 	...
| 
| 	/*
| 	 * Enable traps for the VCPU. The ERET will cause the traps to
| 	 * take effect in the guest, so no ISB is necessary.
| 	 */
| 	cpacr_guest = CPACR_EL1_FPEN;
| 	if (vcpu_has_sve(vcpu))
| 		cpacr_guest |= CPACR_EL1_ZEN;
| 	if (vcpu_has_sme(vcpu))			// whenever we add this
| 		cpacr_guest |= CPACR_EL1_SMEN;
| 	cpacr_clear_set(CPACR_EL1_FPEN | CPACR_EL1_ZEN | CPACR_EL1_SMEN,
| 			cpacr_guest);
| 
| 	return true;
| }

... where we'd still have the CPACR write to re-enable traps, but it'd
be unconditional, and wouldn't need an extra ISB.

If that makes sense to you, I can go spin that as a subsequent cleanup
atop this series.

That looks very clean, yes please! Don't forget to drop the part from
kvm_hyp_save_fpsimd_host() too.

Yep, that was the idea!

To avoid confusion: I've sent out v3 of this series *without* the
change, and I'll prepare that as a follow-up.

Mark.