On Tue, Jan 21, 2025 at 11:20:04AM +0000, Marc Zyngier wrote:
Mark Rutland mark.rutland@arm.com wrote:
/*
* The KVM hyp code doesn't set fp_type when saving the host's
* FPSIMD state. Set fp_type here in case the hyp code saves
* the host state.
Should KVM do that? The comment seems to indicate that this is papering over yet another bug...
It should, we need to record what format the data we saved was in so that if the saved state is loaded we load what we actually saved.
* If hyp code does not save the host state, then the host
* state remains live on the CPU and saved fp_type is
* irrelevant until it is overwritten by a later call to
* fpsimd_save_user_state().
I'm not sure I understand this. If fp_type is irrelevant, surely it is *forever* irrelevant, not until something else happens. Or am I missing something?
The confusion stems from the fact that the setting of fp_type is done separately to the actual state save since we don't map fp_type to the hypervisor. Either we don't save in KVM, in which case any later save in the host kernel will write both state and fp_type, or we save in KVM in which case this results in the correct fp_type being set. In both cases we have live state in the registers and whatever is currently in memory should be considered stale and it's not the end of the world if it's corrupt. It would be a lot more straightforward if we wrote fp_type from the hypervisor as part of the save.
I want to rework the way we represent the state so that we don't have the state spread around like this, avoiding the need to map individual parts of it separately to the hypervisor. We need something like that to sensibly implement in kernel SVE/SME which we will need at some point.