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

From: Yosry Ahmed

Date: Fri May 29 2026 - 19:04:23 EST


On Fri, May 29, 2026 at 3:56 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Fri, May 29, 2026, Jim Mattson wrote:
> > On Fri, May 29, 2026 at 6:29 AM Sean Christopherson <seanjc@xxxxxxxxxx> 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 :)