Re: [PATCH 2/2] KVM: arm64: nv: Don't save/restore FP register during a nested ERET or exception

[Marc Zyngier <maz@kernel.org>]

Hi Mark,

Thanks for looking into this.

On Wed, 13 May 2026 13:28:56 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Tue, May 12, 2026 at 03:07:55PM +0100, Marc Zyngier wrote:
> > When switching between L1 and L2, we diligently use a non-preemptible
> > put/load sequence in order to make sure that the old state is saved,
> > while the new state is brought in. Crucially, this includes the FP
> > registers.
> > 
> > However, this is a bit silly. The FP registers are completely shared
> > between the various ELs (just like the GPRs, really), and eagerly
> > save/restoring those in a non-preemptible section is just overhead.
> > Not to mention that the next access will end-up trapping, something
> > that becomes exponentially expensive as we nest deeper.
> > 
> > The temptation is therefore to completely drop this save/restore thing.
> > Why is it valid to do so? By analogy, the hypervisor doesn't try to
> > poloce things between EL1 and EL0, or between EL2 and EL0. Why should
> > it do so between EL2 and EL1 (or EL2 and L2 EL0)?
> >
> > Once you admit that the FP (and by extension SVE) registers are EL-agnostic,
> > the things that matter are:
> 
> s/poloce/police/ ?

That.

> 
> The above is a bit flowery; it would be nice to remove the rhetorical
> questions and just state that (aside from some control registers) the
> FPSIMD/SVE/SME state is shared between exception levels and doesn't need
> to be saved/restored.
> 
> How about:
> 
>   When switching between L1 and L2, we save the old state using
>   kvm_arch_vcpu_put(), mutate the state in memory, then load the new
>   state using kvm_arch_vcpu_load(). Any live FPSIMD/SVE state is saved
>   and unbound, such that it can be lazily restored on a subsequent trap.
> 
>   The FPSIMD/SVE state is shared by exception levels, and only a handful
>   of related control registers need to be changed when transitioning
>   between L1 and L2. The save/restore of the common state is needless
>   overhead, especially as trapping becomes exponentially more expensive
>   with nesting.
> 
>   Avoid this overhead by leaving the common FPSIMD/SVE state live on the
>   CPU, and only switching the state that is distinct for L1 and L2:
>

Sold. Do you offer a CMAAS (Commit Message As A Service)? Asking for a
friend... ;-)

> > - the trap controls: the effective values are recomputed on each entry
> >   into the guest to take the EL into account and merge the L0 and L1
> >   configuration if in a nested context, or directly use the L0 configuration
> >   in non-nested context (see __activate_traps()).
> > 
> > - the VL settings: the effective values are are also recomputed on each
> >   entry into the guest (see fpsimd_lazy_switch_to_guest()).
> 
> This is true for FPSIMD+SVE today. For SME, SMCR_ELx also contains other
> controls, and will need to be dealt with similarly. It might be worth
> noting that (and that ZCR_ELx could gain new controls in future).
>

Yeah. I tried not to worry too much about SME, but given that it is on
people's radar, I'll drop a comment here.

> > Since we appear to cover all bases, use the vcpu flags indicating the
> > handling of a nested ERET or exception delivery to avoid the whole FP
> > save/restore shenanigans.
> > 
> > For an EL1 L3 guest where L1 and L2 have this optimisation, this
> > results in at least a 10% wall clock reduction when running an I/O
> > heavy workload, generating a high rate of nested exceptions.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/fpsimd.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> > index 15e17aca1dec0..73eda0f46b127 100644
> > --- a/arch/arm64/kvm/fpsimd.c
> > +++ b/arch/arm64/kvm/fpsimd.c
> > @@ -28,6 +28,10 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> >  	if (!system_supports_fpsimd())
> >  		return;
> >  
> > +	if (vcpu_get_flag(vcpu, IN_NESTED_ERET) ||
> > +	    vcpu_get_flag(vcpu, IN_NESTED_EXCEPTION))
> > +		return;
> > +
> 
> I think we need a comment as to why this is safe, with some other detail
> from the commit message. It would also be good to have asserts here to
> catch if something goes wrong.
> 
> How about:
> 
> 	/*
> 	 * Avoid needless save/restore of the guest's common
> 	 * FPSIMD/SVE/SME regs during transitions between L1/L2.
> 	 *
> 	 * These transitions only happens in a non-preemptible context
> 	 * where the host regs have already been saved and unbound. The
> 	 * live registers are either free or owned by the guest.
> 	 */
> 	if (vcpu_get_flag(vcpu, IN_NESTED_ERET) ||
> 	    vcpu_get_flag(vcpu, IN_NESTED_EXCEPTION) {
> 		WARN_ON_ONCE(host_owns_fp_regs());
> 		return;
> 	}
> 
> ... ?
> 
> Note: I didn't add WARN_ON_ONCE(preemptible()), since
> kvm_arch_vcpu_load_fp() should *never* be called in a preemptible
> context.
> 
> >  	/*
> >  	 * Ensure that any host FPSIMD/SVE/SME state is saved and unbound such
> >  	 * that the host kernel is responsible for restoring this state upon
> > @@ -102,6 +106,10 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
> >  {
> >  	unsigned long flags;
> >  
> > +	if (vcpu_get_flag(vcpu, IN_NESTED_ERET) ||
> > +	    vcpu_get_flag(vcpu, IN_NESTED_EXCEPTION))
> > +		return;
> 
> Likewise here, but we can reduce the comment, e.g.
> 
> 	/*
> 	 * See comment in kvm_arch_vcpu_load_fp().
> 	 */
> 	if (vcpu_get_flag(vcpu, IN_NESTED_ERET) ||
> 	    vcpu_get_flag(vcpu, IN_NESTED_EXCEPTION) {
> 		WARN_ON_ONCE(host_owns_fp_regs());
> 		return;
> 	}

Yup, that all looks good to me. I'll repost that next week with these
changes.

Thanks again,

	M.

-- 
Without deviation from the norm, progress is not possible.