Re: [PATCH v4 2/4] KVM: arm64: Dirty quota-based throttling of vcpus

[Sean Christopherson <seanjc@google.com>]

On Thu, May 26, 2022, Shivam Kumar wrote:
> 
> On 24/05/22 12:44 pm, Marc Zyngier wrote:
> > On Sat, 21 May 2022 21:29:38 +0100,
> > Shivam Kumar <shivam.kumar1@nutanix.com> wrote:
> > > Exit to userspace whenever the dirty quota is exhausted (i.e. dirty count
> > > equals/exceeds dirty quota) to request more dirty quota.
> > > 
> > > Suggested-by: Shaju Abraham <shaju.abraham@nutanix.com>
> > > Suggested-by: Manish Mishra <manish.mishra@nutanix.com>
> > > Co-developed-by: Anurag Madnawat <anurag.madnawat@nutanix.com>
> > > Signed-off-by: Anurag Madnawat <anurag.madnawat@nutanix.com>
> > > Signed-off-by: Shivam Kumar <shivam.kumar1@nutanix.com>
> > > ---
> > >   arch/arm64/kvm/arm.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > > index ecc5958e27fe..5b6a239b83a5 100644
> > > --- a/arch/arm64/kvm/arm.c
> > > +++ b/arch/arm64/kvm/arm.c
> > > @@ -848,6 +848,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > >   	ret = 1;
> > >   	run->exit_reason = KVM_EXIT_UNKNOWN;
> > >   	while (ret > 0) {
> > > +		ret = kvm_vcpu_check_dirty_quota(vcpu);
> > > +		if (!ret)
> > > +			break;
> > >   		/*
> > >   		 * Check conditions before entering the guest
> > >   		 */
> > Why do we need yet another check on the fast path? It seems to me that
> > this is what requests are for, so I'm definitely not keen on this
> > approach. I certainly do not want any extra overhead for something
> > that is only used on migration. If anything, it is the migration path
> > that should incur the overhead.
> > 
> > 	M.
> I'll try implementing this with requests. Thanks.

I've no objection to using a request, avoiding the extra checks is quite nice,
and x86's fastpath loop would Just Work (though VMX's handle_invalid_guest_state()
still needs manually checking).

The only gotchas are that it'll likely require a small amount of #ifdeffery, and
the quota will need to be "temporarily" captured in 'struct kvm_vcpu' because
there's no guarantee that the next exit to userspace will be due to the dirty
quota, i.e. something else might ovewrirte the exit union in vcpu->run.