On 07/10/22 11:59 pm, Sean Christopherson wrote:
On Thu, Sep 15, 2022, Shivam Kumar wrote:
#endif /* SELFTEST_KVM_UTIL_BASE_H */
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 9889fe0d8919..4c7bd9807d0b 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -18,6 +18,7 @@
#include <linux/kernel.h>
#define KVM_UTIL_MIN_PFN 2
+#define PML_BUFFER_SIZE 512
...
+ /*
+ * Due to PML, number of pages dirtied by the vcpu can exceed its dirty
+ * quota by PML buffer size.
+ */
Buffering for PML is very Intel centric, and even then it's not guaranteed. In
x86, PML can and should be detected programmatically:
bool kvm_is_pml_enabled(void)
{
return is_intel_cpu() && get_kvm_intel_param_bool("pml");
}
Yes, I was looking for something like this. Thanks.
Not sure if it's worth a generic arch hook to get the CPU buffer size, e.g. the
test could do:
I can't think of any usecase for a custom buffer as of now. We are
working on a proposal for dynamically adjusting the PML buffer size in
order to throttle effectively with dirty quota. A potential usecase.
/*
* Allow ??? pages of overrun, KVM is allowed to dirty multiple pages
* before exiting to userspace, e.g. when emulating an instruction that
* performs multiple memory accesses.
*/
uint64_t buffer = ???;
/*
* When Intel's Page-Modification Logging (PML) is enabled, the CPU may
* dirty up to 512 pages (number of entries in the PML buffer) without
* exiting, thus KVM may effectively dirty that many pages before
* enforcing the dirty quota.
*/
#ifdef __x86_64__
if (kvm_is_pml_enabled(void)
buffer = PML_BUFFER_SIZE;
#endif
+ TEST_ASSERT(count <= quota + PML_BUFFER_SIZE, "Invalid number of pages
Clarify _why_ the count is invalid.
In the worst case, the vcpu will be able to dirty 512 more pages than
its dirty quota due to PML.
+ dirtied: count=%"PRIu64", quota=%"PRIu64"\n", count, quota);
Don't split strings unless it's truly necessary. This is one of those cases where
running over the 80 char soft limit is preferable to wrapping. And no need to use
PRIu64, selftests are 64-bit only. E.g.
Got it, thanks.
TEST_ASSERT(count <= (quota + buffer),
"KVM dirtied too many pages: count = %lx, quota = %lx, buffer = %lx",
count, quota, buffer);
+
+ TEST_ASSERT(count >= quota, "Dirty quota exit happened with quota yet to
+ be exhausted: count=%"PRIu64", quota=%"PRIu64"\n", count, quota);
Similar comments here.
+
+ if (count > quota)
+ pr_info("Dirty quota exit with unequal quota and count:
+ count=%"PRIu64", quota=%"PRIu64"\n", count, quota);
Not sure I'd bother with this. Realistically, is anyone ever going to be able to
do anything useful with this information? E.g. is this just going to be a "PML is
enabled!" message?
I thought this information could help detect anomalies when PML is
disabled. If PML is disabled, I am expecting that the exit would be case
when the quota equals count and count wouldn't ever exceed quota.
Now, when I think of, I think I should move this statement inside an
'if' block and make it too an assertion.
if (!kvm_is_pml_enabled(void))
TEST_ASSERT(count == quota, "Dirty quota exit with unequal quota and
count.");
+
+ run->dirty_quota = count + test_dirty_quota_increment;
+}
--
2.22.3
