On Thu, Sep 15, 2022, Shivam Kumar wrote:
@@ -542,6 +545,21 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
return cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE);
}
+static inline int kvm_vcpu_check_dirty_quota(struct kvm_vcpu *vcpu)
+{
+ struct kvm_run *run = vcpu->run;
+ u64 dirty_quota = READ_ONCE(run->dirty_quota);
+ u64 pages_dirtied = vcpu->stat.generic.pages_dirtied;
+
+ if (!dirty_quota || (pages_dirtied < dirty_quota))
+ return 1;
+
+ run->exit_reason = KVM_EXIT_DIRTY_QUOTA_EXHAUSTED;
+ run->dirty_quota_exit.count = pages_dirtied;
+ run->dirty_quota_exit.quota = dirty_quota;
+ return 0;
Dead code.
+}
...
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 584a5bab3af3..f315af50037d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3298,18 +3298,36 @@ int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len)
}
EXPORT_SYMBOL_GPL(kvm_clear_guest);
+static void kvm_vcpu_is_dirty_quota_exhausted(struct kvm_vcpu *vcpu)
Ouch, sorry. I suspect you got this name from me[*]. That was a goof on my end,
I'm 99% certain I copy-pasted stale code, i.e. didn't intended to suggest a
rename.
Let's keep kvm_vcpu_check_dirty_quota(), IMO that's still the least awful name.
[*] https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_Yo-2B82LjHSOdyxKzT-40google.com&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=4hVFP4-J13xyn-OcN0apTCh8iKZRosf5OJTQePXBMB8&m=IIED7C8bZGKU5CadTcY5IR4yLs79Rsv2HVHCn5FIL8TQpiAbpjEBs97euDE3FFZf&s=WyikNnoMe8QzP5dTKKectMN-uxc_fTby1FlKVAaS7zI&e=
+{
+ u64 dirty_quota = READ_ONCE(vcpu->run->dirty_quota);
+
+ if (!dirty_quota || (vcpu->stat.generic.pages_dirtied < dirty_quota))
+ return;
+
+ /*
+ * Snapshot the quota to report it to userspace. The dirty count will be
+ * captured when the request is processed.
+ */
+ vcpu->dirty_quota = dirty_quota;
+ kvm_make_request(KVM_REQ_DIRTY_QUOTA_EXIT, vcpu);
Making the request needs to be guarded with an arch opt-in. Pending requests
prevent KVM from entering the guest, and so making a request that an arch isn't
aware of will effectively hang the vCPU. Obviously userspace would be shooting
itself in the foot by setting run->dirty_quota in this case, but KVM shouldn't
hand userspace a loaded gun and help them aim.
My suggestion from v1[*] about not forcing architectures to opt-in was in the
context of a request-less implementation where dirty_quota was a nop until the
arch took action.
And regardless of arch opt-in, I think this needs a capability so that userspace
can detect KVM support.
I don't see any reason to wrap the request or vcpu_run field, e.g. something like
this should suffice:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ea5847d22aff..93362441215b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3298,8 +3298,9 @@ int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len)
}
EXPORT_SYMBOL_GPL(kvm_clear_guest);
-static void kvm_vcpu_is_dirty_quota_exhausted(struct kvm_vcpu *vcpu)
+static void kvm_vcpu_check_dirty_quota(struct kvm_vcpu *vcpu)
{
+#ifdef CONFIG_HAVE_KVM_DIRTY_QUOTA
u64 dirty_quota = READ_ONCE(vcpu->run->dirty_quota);
if (!dirty_quota || (vcpu->stat.generic.pages_dirtied < dirty_quota))
@@ -3311,6 +3312,7 @@ static void kvm_vcpu_is_dirty_quota_exhausted(struct kvm_vcpu *vcpu)
*/
vcpu->dirty_quota = dirty_quota;
kvm_make_request(KVM_REQ_DIRTY_QUOTA_EXIT, vcpu);
+#endif
}
void mark_page_dirty_in_slot(struct kvm *kvm,
@@ -4507,6 +4509,8 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
case KVM_CAP_BINARY_STATS_FD:
case KVM_CAP_SYSTEM_EVENT_DATA:
return 1;
+ case KVM_CAP_DIRTY_QUOTA:
+ return !!IS_ENABLED(CONFIG_HAVE_KVM_DIRTY_QUOTA);
default:
break;
}
[*] https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_YZaUENi0ZyQi-252F9M0-40google.com&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=4hVFP4-J13xyn-OcN0apTCh8iKZRosf5OJTQePXBMB8&m=IIED7C8bZGKU5CadTcY5IR4yLs79Rsv2HVHCn5FIL8TQpiAbpjEBs97euDE3FFZf&s=kbNeQcRDxAxx4SEPIe_tL-_1dYMa9zx-REEozKvO0w0&e=
