Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2

[Sean Christopherson <seanjc@google.com>]

On Fri, May 29, 2026, Yosry Ahmed wrote:
> On Fri, May 29, 2026 at 3:56 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, May 29, 2026, Jim Mattson wrote:
> > > On Fri, May 29, 2026 at 6:29 AM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > On Thu, May 28, 2026, Yosry Ahmed wrote:
> > > > > > +int main(int argc, char *argv[])
> > > > > > +{
> > > > > > +       TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
> > > > > > +       TEST_REQUIRE(kvm_has_cap(KVM_CAP_NESTED_STATE));
> > > > > > +       TEST_REQUIRE(kvm_check_cap(KVM_CAP_DISABLE_QUIRKS2) &
> > > > > > +                    KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
> > > > > > +
> > > > > > +       if (!kvm_cpu_has(X86_FEATURE_NPT))
> > > > > > +               goto skip_npt;
> > > > > > +
> > > > > > +       gpat_test("Invalid gPAT", l1_guest_code_invalid_gpat, npt_enabled = true);
> > > > > > +       gpat_test("Nested NPT enabled", l1_guest_code, npt_enabled = true);
> > > > > > +skip_npt:
> > > > > > +       gpat_test("Nested NPT disabled", l1_guest_code, npt_enabled = false);
> > > > > > +       return 0;
> > > > > > +}
> > > > >
> > > > > The goto is really unnecessary here:
> > > > >
> > > > > if (kvm_cpu_has(X86_FEATURE_NPT)) {
> > > > >        gpat_test("Invalid gPAT", l1_guest_code_invalid_gpat,
> > > > > npt_enabled = true);
> > > > >        gpat_test("Nested NPT enabled", l1_guest_code, npt_enabled = true);
> > > > > }
> > > > > gpat_test("Nested NPT disabled", l1_guest_code, npt_enabled = false);
> > > >
> > > > Well, it's necessary to avoid the indentation.
> > >
> > > Or refactor as:
> > >
> > > if (kvm_cpu_has(X86_FEATURE_NPT))
> > >         run_npt_tests();
> >
> > Anyone have a strong preference?  I like the goto approach (obviously), so that
> > that all the gpat_test() invocations are prettily aligned.  But I can live with
> > an if-statement as Yosry suggested.  I think the only option I don't like is
> > moving the NPT tests to a separate helper (it's two lines of code, and testing
> > the NPT case really is the focal point of the test).
> 
> No strong preference, I obviously also like my suggestion :)

Ok, third option, which I've pushed to kvm-x86/svm (but not kvm-x86/next, yet)
in anticipation of overwhelming support:

#define gpat_test(test_name, guest_code, npt_setting)			\
do {									\
	npt_setting;							\
									\
	if (npt_enabled && !kvm_cpu_has(X86_FEATURE_NPT)) {		\
		pr_info("Skipping: " test_name " (no NPT support)\n");	\
		break;							\
	}								\
									\
	pr_info("Testing: " test_name "\n");				\
	run_test(guest_code, false, 1);					\
									\
	if (guest_code == l1_guest_code) {				\
		pr_info("Testing: " test_name " Save/Restore\n");	\
		run_test(guest_code, true, 1);				\
									\
		pr_info("Testing: " test_name " Multiple VMRUNs\n");	\
		run_test(guest_code, false, 10);			\
	}								\
} while (0)

int main(int argc, char *argv[])
{
	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
	TEST_REQUIRE(kvm_has_cap(KVM_CAP_NESTED_STATE));
	TEST_REQUIRE(kvm_check_cap(KVM_CAP_DISABLE_QUIRKS2) &
		     KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);

	gpat_test("Invalid gPAT", l1_guest_code_invalid_gpat, npt_enabled = true);
	gpat_test("Nested NPT enabled", l1_guest_code, npt_enabled = true);
	gpat_test("Nested NPT disabled", l1_guest_code, npt_enabled = false);
	return 0;
}

Which (obviously) allows printing out which subtests are being skipped:

# ./x86/svm_nested_pat_test 
Random seed: 0x6b8b4567
Skipping: Invalid gPAT (no NPT support)
Skipping: Nested NPT enabled (no NPT support)
Testing: Nested NPT disabled
Testing: Nested NPT disabled Save/Restore
Testing: Nested NPT disabled Multiple VMRUNs