Re: [PATCH v7 08/16] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing

[Dave Martin <Dave.Martin@arm.com>]

On Tue, May 15, 2018 at 12:55:23PM +0200, Christoffer Dall wrote:
> On Wed, May 09, 2018 at 05:12:57PM +0100, Dave Martin wrote:
> > This patch refactors KVM to align the host and guest FPSIMD
> > save/restore logic with each other for arm64.  This reduces the
> > number of redundant save/restore operations that must occur, and
> > reduces the common-case IRQ blackout time during guest exit storms
> > by saving the host state lazily and optimising away the need to
> > restore the host state before returning to the run loop.

[...]

> > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c

[...]

> > +/*
> > + * Prepare vcpu for saving the host's FPSIMD state and loading the guest's.
> > + * The actual loading is done by the FPSIMD access trap taken to hyp.
> > + *
> > + * Here, we just set the correct metadata to indicate that the FPSIMD
> > + * state in the cpu regs (if any) belongs to current on the host.
> > + *
> > + * TIF_SVE is backed up here, since it may get clobbered with guest state.
> > + * This flag is restored by kvm_arch_vcpu_put_fp(vcpu).
> > + */
> > +void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> > +{
> > +	BUG_ON(system_supports_sve());
> > +	BUG_ON(!current->mm);
> > +
> > +	vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
> > +	vcpu->arch.flags |= KVM_ARM64_FP_HOST;
> > +	if (test_thread_flag(TIF_SVE))
> > +		vcpu->arch.flags |= KVM_ARM64_HOST_SVE_IN_USE;
> > +}
> > +
> > +/*
> > + * If the guest FPSIMD state was loaded, update the host's context
> > + * tracking data mark the CPU FPSIMD regs as dirty for vcpu so that they
> 
> nit: by marking the CPU FPSIMD regs as dirty and belonging to the
> vcpu... ?

Changed locally.

Marc, let me know if you can pick this up or want me to repost.  There
are a couple of pending FWIWs now.

There is also the option to include the separate update_thread_flags()
patches that now have maintainer acks, though I will need to confirm
with the respective maintainers that they're OK with this.

> Otherwise:
> 
> Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>

[...]

Thanks
---Dave