Skip to content

Commit fe49f80

Browse files
Maxim Levitskysean-jc
authored andcommitted
KVM: selftests: Support multiple write retires in dirty_log_test
If dirty_log_test is run nested, it is possible for entries in the emulated PML log to appear before the actual memory write is committed to the RAM, due to the way KVM retries memory writes as a response to a MMU fault. In addition to that in some very rare cases retry can happen more than once, which will lead to the test failure because once the write is finally committed it may have a very outdated iteration value. Detect and avoid this case. Cc: Peter Xu <peterx@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Link: https://lore.kernel.org/r/20250111003004.1235645-2-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 89ea56a commit fe49f80

File tree

1 file changed

+50
-2
lines changed

1 file changed

+50
-2
lines changed

tools/testing/selftests/kvm/dirty_log_test.c

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ static atomic_t vcpu_sync_stop_requested;
152152
* sem_vcpu_stop and before vcpu continues to run.
153153
*/
154154
static bool dirty_ring_vcpu_ring_full;
155+
155156
/*
156157
* This is only used for verifying the dirty pages. Dirty ring has a very
157158
* tricky case when the ring just got full, kvm will do userspace exit due to
@@ -166,7 +167,51 @@ static bool dirty_ring_vcpu_ring_full;
166167
* dirty gfn we've collected, so that if a mismatch of data found later in the
167168
* verifying process, we let it pass.
168169
*/
169-
static uint64_t dirty_ring_last_page;
170+
static uint64_t dirty_ring_last_page = -1ULL;
171+
172+
/*
173+
* In addition to the above, it is possible (especially if this
174+
* test is run nested) for the above scenario to repeat multiple times:
175+
*
176+
* The following can happen:
177+
*
178+
* - L1 vCPU: Memory write is logged to PML but not committed.
179+
*
180+
* - L1 test thread: Ignores the write because its last dirty ring entry
181+
* Resets the dirty ring which:
182+
* - Resets the A/D bits in EPT
183+
* - Issues tlb flush (invept), which is intercepted by L0
184+
*
185+
* - L0: frees the whole nested ept mmu root as the response to invept,
186+
* and thus ensures that when memory write is retried, it will fault again
187+
*
188+
* - L1 vCPU: Same memory write is logged to the PML but not committed again.
189+
*
190+
* - L1 test thread: Ignores the write because its last dirty ring entry (again)
191+
* Resets the dirty ring which:
192+
* - Resets the A/D bits in EPT (again)
193+
* - Issues tlb flush (again) which is intercepted by L0
194+
*
195+
* ...
196+
*
197+
* N times
198+
*
199+
* - L1 vCPU: Memory write is logged in the PML and then committed.
200+
* Lots of other memory writes are logged and committed.
201+
* ...
202+
*
203+
* - L1 test thread: Sees the memory write along with other memory writes
204+
* in the dirty ring, and since the write is usually not
205+
* the last entry in the dirty-ring and has a very outdated
206+
* iteration, the test fails.
207+
*
208+
*
209+
* Note that this is only possible when the write was the last log entry
210+
* write during iteration N-1, thus remember last iteration last log entry
211+
* and also don't fail when it is reported in the next iteration, together with
212+
* an outdated iteration count.
213+
*/
214+
static uint64_t dirty_ring_prev_iteration_last_page;
170215

171216
enum log_mode_t {
172217
/* Only use KVM_GET_DIRTY_LOG for logging */
@@ -316,6 +361,8 @@ static uint32_t dirty_ring_collect_one(struct kvm_dirty_gfn *dirty_gfns,
316361
struct kvm_dirty_gfn *cur;
317362
uint32_t count = 0;
318363

364+
dirty_ring_prev_iteration_last_page = dirty_ring_last_page;
365+
319366
while (true) {
320367
cur = &dirty_gfns[*fetch_index % test_dirty_ring_count];
321368
if (!dirty_gfn_is_dirtied(cur))
@@ -613,7 +660,8 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap)
613660
*/
614661
min_iter = iteration - 1;
615662
continue;
616-
} else if (page == dirty_ring_last_page) {
663+
} else if (page == dirty_ring_last_page ||
664+
page == dirty_ring_prev_iteration_last_page) {
617665
/*
618666
* Please refer to comments in
619667
* dirty_ring_last_page.

0 commit comments

Comments
 (0)