Re: [PATCH v2 29/29] KVM: selftests: Add a test for nested TLB flushes

[Yosry Ahmed <yosry@kernel.org>]

On Thu, Oct 08, 2026 at 12:35:35AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] kvm: selftests: missing memory clobber in invlpga() allows reordering
> --
> 
> commit c21865242d0952d01a007708f2fbf08ce2550486
> Author: Yosry Ahmed <yosry@kernel.org>
> 
> KVM: selftests: Add a test for nested TLB flushes
> 
> Add a test that exercises most common scenarios for TLB flushes. The
> test is split into guest-triggered and KVM-triggered TLB flushes.
> 
> > diff --git a/tools/testing/selftests/kvm/include/x86/svm.h b/tools/testing/selftests/kvm/include/x86/svm.h
> > index c8539166270ea..7644e37c7042c 100644
> > --- a/tools/testing/selftests/kvm/include/x86/svm.h
> > +++ b/tools/testing/selftests/kvm/include/x86/svm.h
> [ ... ]
> > @@ -316,4 +316,9 @@ struct __attribute__ ((__packed__)) vmcb {
> >  
> >  #define SVM_CR0_SELECTIVE_MASK (X86_CR0_TS | X86_CR0_MP)
> >  
> > +static inline void invlpga(unsigned long addr, u32 asid)
> > +{
> > +	asm volatile("invlpga %0, %1" : : "a"(addr), "c"(asid));
> > +}
> > +
> >  #endif /* SELFTEST_KVM_SVM_H */
> 
> [Severity: Medium]
> Does this inline assembly need a "memory" clobber?
> 
> Unlike the VMX counterparts invvpid and invept introduced in this patch, this
> inline assembly lacks a "memory" clobber. This might allow the compiler to
> reorder prior memory stores past the TLB flush instruction.
> 
> > diff --git a/tools/testing/selftests/kvm/x86/nested_tlb_flush_test.c b/tools/testing/selftests/kvm/x86/nested_tlb_flush_test.c
> > new file mode 100644
> > index 0000000000000..fb32b7bf6b19f
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/x86/nested_tlb_flush_test.c
> [ ... ]
> > +static void l1_guest_code(void *data)
> > +{
> > +	gpa_t gpa;
> > +	int i;
> > +
> > +	prepare_l2(data, l2_guest_code);
> > +
> > +	/*
> > +	 * Alternately switch the PTE (or TDP PTE) mapping TEST_VADDR between
> > +	 * two pages containing VAL1 and VAL2, flush TLBs, and verify that L2
> > +	 * reads the expected value.
> > +	 */
> > +	for (i = 0; i < NR_ITERATIONS; i++) {
> > +		gpa = test_gpa[i % 2];
> > +
> > +		*pte_gva &= ~PHYSICAL_PAGE_MASK;
> > +		*pte_gva |= gpa & PHYSICAL_PAGE_MASK;
> > +
> > +		WRITE_ONCE(l2_expected_val, (i % 2 == 0) ? VAL1 : VAL2);
> > +		run_l2(data, i == 0);
> 
> [Severity: Medium]
> Can the compiler execute the *pte_gva assignments after the TLB flush inside
> run_l2()?
> 
> Because the invlpga() inline assembly lacks a "memory" clobber, and there is no
> other compiler barrier between the assignment and invlpga(), the compiler is
> legally permitted to reorder the memory store to execute after the invlpga()
> instruction.
> 
> If this reordering occurs, the hardware will flush the TLB while the old PTE is
> still in memory, which could allow a speculative hardware page walk to re-cache
> the old PTE before the store completes. When L2 resumes, it may use the stale
> TLB entry, causing the test to spuriously fail. The "memory" clobber in the
> run_guest() wrapper only ensures both operations complete prior to VM-entry; it
> does not enforce their relative execution order.

Will add the "memory" clobber.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261008001425.2458927-1-yosry@kernel.org?part=29