Re: [PATCH v5] KVM: selftests: Test READ=>WRITE dirty logging behavior for shadow MMU

[Yosry Ahmed]

On Thu, Jan 15, 2026 at 09:21:54AM -0800, Sean Christopherson wrote:
> Update the nested dirty log test to validate KVM's handling of READ faults
> when dirty logging is enabled.  Specifically, set the Dirty bit in the
> guest PTEs used to map L2 GPAs, so that KVM will create writable SPTEs
> when handling L2 read faults.  When handling read faults in the shadow MMU,
> KVM opportunistically creates a writable SPTE if the mapping can be
> writable *and* the gPTE is dirty (or doesn't support the Dirty bit), i.e.
> if KVM doesn't need to intercept writes in order to emulate Dirty-bit
> updates.
> 
> To actually test the L2 READ=>WRITE sequence, e.g. without masking a false
> pass by other test activity, route the READ=>WRITE and WRITE=>WRITE
> sequences to separate L1 pages, and differentiate between "marked dirty
> due to a WRITE access/fault" and "marked dirty due to creating a writable
> SPTE for a READ access/fault".  The updated sequence exposes the bug fixed
> by KVM commit 1f4e5fc83a42 ("KVM: x86: fix nested guest live migration
> with PML") when the guest performs a READ=>WRITE sequence with dirty guest
> PTEs.
> 
> Opportunistically tweak and rename the address macros, and add comments,
> to make it more obvious what the test is doing.  E.g. NESTED_TEST_MEM1
> vs. GUEST_TEST_MEM doesn't make it all that obvious that the test is
> creating aliases in both the L2 GPA and GVA address spaces, but only when
> L1 is using TDP to run L2.
> 
> Cc: Yosry Ahmed <yosry.ahmed@xxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>

LGTM with one nit/question below:

Reviewed-by: Yosry Ahmed <yosry.ahmed@xxxxxxxxx>

[..]
> +static void l2_guest_code(vm_vaddr_t base)
>  {
> -	READ_ONCE(*a);
> -	WRITE_ONCE(*a, 1);
> -	GUEST_SYNC(true);
> -	GUEST_SYNC(false);
> +	vm_vaddr_t page0 = TEST_GUEST_ADDR(base, 0);
> +	vm_vaddr_t page1 = TEST_GUEST_ADDR(base, 1);
>  
> -	WRITE_ONCE(*b, 1);
> -	GUEST_SYNC(true);
> -	WRITE_ONCE(*b, 1);
> -	GUEST_SYNC(true);
> -	GUEST_SYNC(false);
> +	READ_ONCE(*(u64 *)page0);
> +	GUEST_SYNC(page0 | TEST_SYNC_READ_FAULT);
> +	WRITE_ONCE(*(u64 *)page0, 1);
> +	GUEST_SYNC(page0 | TEST_SYNC_WRITE_FAULT);
> +	READ_ONCE(*(u64 *)page0);
> +	GUEST_SYNC(page0 | TEST_SYNC_NO_FAULT);
> +
> +	WRITE_ONCE(*(u64 *)page1, 1);
> +	GUEST_SYNC(page1 | TEST_SYNC_WRITE_FAULT);
> +	WRITE_ONCE(*(u64 *)page1, 1);
> +	GUEST_SYNC(page1 | TEST_SYNC_WRITE_FAULT);
> +	READ_ONCE(*(u64 *)page1);
> +	GUEST_SYNC(page1 | TEST_SYNC_NO_FAULT);
> +	GUEST_SYNC(page1 | TEST_SYNC_NO_FAULT);

Extra GUEST_SYNC()?

>  
>  	/* Exit to L1 and never come back.  */
>  	vmcall();