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

[sashiko-bot@kernel.org]

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.

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