Re: [PATCH v6 2/5] KVM: x86: Dirty quota-based throttling of vcpus

[Sean Christopherson <seanjc@google.com>]

On Tue, Oct 18, 2022, Shivam Kumar wrote:
> > > #ifdef CONFIG_HAVE_KVM_DIRTY_QUOTA
> > > static inline bool kvm_check_dirty_quota_request(struct kvm_vcpu *vcpu)
> > > {
> > >     struct kvm_run *run = vcpu->run;
> > > 
> > >     run->exit_reason = KVM_EXIT_DIRTY_QUOTA_EXHAUSTED;
> > >     run->dirty_quota_exit.count = vcpu->stat.generic.pages_dirtied;
> > >     run->dirty_quota_exit.quota = READ_ONCE(run->dirty_quota);
> > > 
> > >     /*
> > >      * Re-check the quota and exit if and only if the vCPU still
> > > exceeds its
> > >      * quota.  If userspace increases (or disables entirely) the
> > > quota, then
> > >      * no exit is required as KVM is still honoring its ABI, e.g.
> > > userspace
> > >      * won't even be aware that KVM temporarily detected an
> > > exhausted quota.
> > >      */
> > >     return run->dirty_quota_exit.count >= run->dirty_quota_exit.quota;
> > > }
> > > #endif
> > > 
> > > And then arch usage is simply something like:
> > > 
> > >         if (kvm_check_dirty_quota_request(vcpu)) {
> > >             r = 0;
> > >             goto out;
> > >         }
> > If we are not even checking for request KVM_REQ_DIRTY_QUOTA_EXIT, what's
> > the use of kvm_make_request in patch 1?
> Ok, so we don't need to explicitely check for request here because we are
> checking the quota directly but we do need to make the request when the
> quota is exhausted so as to guarantee that we enter the if block "if
> (kvm_request_pending(vcpu))".
> 
> Please let me know if my understanding is correct or if I am missing
> something.

The request needs to be explicitly checked, I simply unintentionally omitted that
code in the above snippet.  kvm_check_request() also _clears_ the request, which
is very important as not clearing the request will prevent KVM from entering the
guest.

The intent is to check KVM_REQ_DIRTY_QUOTA_EXIT in kvm_check_dirty_quota_request().

> 
> Also, should I add ifdef here:
> 
> 	#ifdef CONFIG_HAVE_KVM_DIRTY_QUOTA
> 	if (kvm_check_dirty_quota_request(vcpu)) {
> 		r = 0;
> 		goto out;
> 	}
> 	#endif

No need for an #ifdef in the caller, the call is from arch code and architectures
that don't "select HAVE_KVM_DIRTY_QUOTA" shouldn't be checking for a dirty quota
exit.