Re: [PATCH v3 3/3] KVM: selftests: Add selftests for dirty quota throttling

[Sean Christopherson <seanjc@google.com>]

On Mon, Apr 18, 2022, Shivam Kumar wrote:
> > > +void vcpu_handle_dirty_quota_exit(struct kvm_run *run,
> > > +            uint64_t test_dirty_quota_increment)
> > > +{
> > > +    uint64_t quota = run->dirty_quota_exit.quota;
> > > +    uint64_t count = run->dirty_quota_exit.count;
> > > +
> > > +    /*
> > > +     * Due to PML, number of pages dirtied by the vcpu can exceed its dirty
> > > +     * quota by PML buffer size.
> > > +     */
> > > +    TEST_ASSERT(count <= quota + PML_BUFFER_SIZE, "Invalid number of pages
> > > +        dirtied: count=%"PRIu64", quota=%"PRIu64"\n", count, quota);
> Sean, I don't think this would be valid anymore because as you mentioned, the
> vcpu can dirty multiple pages in one vmexit. I could use your help here.

TL;DR: Should be fine, but s390 likely needs an exception.

Practically speaking the 512 entry fuzziness is all but guaranteed to prevent
false failures.

But, unconditionally allowing for overflow of 512 entries also means the test is
unlikely to ever detect violations.  So to provide meaningful coverage, this needs
to allow overflow if and only if PML is enabled.

And that brings us back to false failures due to _legitimate_ scenarios where a vCPU
can dirty multiple pages.  Emphasis on legitimate, because except for an s390 edge
case, I don't think this test's guest code does anything that would dirty multiple
pages in a single instruction, e.g. there's no emulation, shouldn't be any descriptor
table side effects, etc...  So unless I'm missing something, KVM should be able to
precisely handle the core run loop.

s390 does appear to have a caveat:

	/*
	 * On s390x, all pages of a 1M segment are initially marked as dirty
	 * when a page of the segment is written to for the very first time.
	 * To compensate this specialty in this test, we need to touch all
	 * pages during the first iteration.
	 */
	for (i = 0; i < guest_num_pages; i++) {
		addr = guest_test_virt_mem + i * guest_page_size;
		*(uint64_t *)addr = READ_ONCE(iteration);
	}

IIUC, subsequent iterations will be ok, but the first iteration needs to allow
for overflow of 256 (AFAIK the test only uses 4kb pages on s390).