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

[Shivam Kumar <shivam.kumar1@nutanix.com>]

On 08/10/22 1:00 am, Sean Christopherson wrote:
> On Thu, Sep 15, 2022, Shivam Kumar wrote:
>> index c9b49a09e6b5..140a5511ed45 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -5749,6 +5749,9 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
>>   		 */
>>   		if (__xfer_to_guest_mode_work_pending())
>>   			return 1;
>> +
>> +		if (!kvm_vcpu_check_dirty_quota(vcpu))
> 
> A dedicated helper is no longer necessary, this can _test_ the event, a la
> KVM_REQ_EVENT above:
> 
> 		if (kvm_test_request(KVM_REQ_DIRTY_QUOTA_EXIT, vcpu))
> 			return 1;
Sure, thanks.
> 
>> +			return 0;
>>   	}
>>   
>>   	return 1;
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 43a6a7efc6ec..8447e70b26f5 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -10379,6 +10379,15 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>   			r = 0;
>>   			goto out;
>>   		}
>> +		if (kvm_check_request(KVM_REQ_DIRTY_QUOTA_EXIT, 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 = vcpu->dirty_quota;
> 
> As mentioned in patch 1, the code code snippet I suggested is bad.  With a request,
> there's no need to snapshot the quota prior to making the request.  If userspace
> changes the quota, then using the new quota is perfectly ok since KVM is still
> providing sane data.  If userspace lowers the quota, an exit will still ocurr as
> expected.  If userspace disables or increases the quota to the point where no exit
> is necessary, then userspace can't expect and exit and won't even be aware that KVM
> temporarily detected an exhausted quota.
> 
> And all of this belongs in a common KVM helper.  Name isn't pefect, but it aligns
> with kvm_check_request().
> 
> #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;
> 		}