All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
@ 2026-05-28 23:10 Sean Christopherson
  2026-05-29  0:47 ` Yosry Ahmed
  2026-06-05 18:31 ` Sean Christopherson
  0 siblings, 2 replies; 9+ messages in thread
From: Sean Christopherson @ 2026-05-28 23:10 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson; +Cc: kvm, linux-kernel, Yosry Ahmed

From: Yosry Ahmed <yosry@kernel.org>

When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled, verify that KVM
correctly virtualizes the host PAT MSR and the guest PAT register for
nested SVM guests.

With nested NPT disabled:
 * L1 and L2 share the same PAT
 * The vmcb12.g_pat is ignored

With nested NPT enabled:
 * An invalid g_pat in vmcb12 causes VMEXIT_INVALID
 * RDMSR(IA32_PAT) from L2 returns the value of the guest PAT register
 * WRMSR(IA32_PAT) from L2 is reflected in vmcb12's g_pat on VMEXIT
 * RDMSR(IA32_PAT) from L1 returns the value of the host PAT MSR

Verify that save/restore with the vCPU in guest mode behaves as expected in
both cases, e.g. preserves both hPAT and gPAT when NPT is enabled.

Originally-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
[sean: use even fancier macro shenanigans]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

v4:
 - Stop trying to use macros, and use macros.
 - Identity map TDP after allocating SVM to play nice with upcoming automatic
   stack allocation/configuration.
 - Run the npt=false testcase if NPT is unsupported, instead of skipping the
   entire test.
 - Run multiple iterations and save/restore for npt=false as well.
 - Run the variants via the main run macro.
 - Name the run macro so it looks like a function.
 - Drop printfs leftover from initial development.

v3:
 - Fixed multiple VM-Entries test case.
 - Fixed typos.

v2:
 - Rewrote most of the test to dedup L1 and L2 code, move assertions to
   L2 where possible to simplify the test, and drop the shared test
   struct.

v1: https://lore.kernel.org/kvm/20260327234023.2659476-10-jmattson@google.com

 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/x86/svm_nested_pat_test.c   | 195 ++++++++++++++++++
 2 files changed, 196 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_pat_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 2908eca1647a..e0ddd3ff9472 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -118,6 +118,7 @@ TEST_GEN_PROGS_x86 += x86/svm_nested_clear_efer_svme
 TEST_GEN_PROGS_x86 += x86/svm_nested_shutdown_test
 TEST_GEN_PROGS_x86 += x86/svm_nested_soft_inject_test
 TEST_GEN_PROGS_x86 += x86/svm_nested_vmcb12_gpa
+TEST_GEN_PROGS_x86 += x86/svm_nested_pat_test
 TEST_GEN_PROGS_x86 += x86/svm_lbr_nested_state
 TEST_GEN_PROGS_x86 += x86/tsc_scaling_sync
 TEST_GEN_PROGS_x86 += x86/sync_regs_test
diff --git a/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c b/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
new file mode 100644
index 000000000000..a319966c9ef0
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026, Google LLC.
+ *
+ * Test that KVM correctly virtualizes the PAT MSR and VMCB g_pat field
+ * for nested SVM guests:
+ *
+ * o With nested NPT disabled:
+ *     - L1 and L2 share the same PAT
+ *     - The vmcb12.g_pat is ignored
+ * o With nested NPT enabled:
+ *     - Invalid g_pat in vmcb12 should cause VMEXIT_INVALID
+ *     - L2 should see vmcb12.g_pat via RDMSR, not L1's PAT
+ *     - L2's writes to PAT should be saved to vmcb12 on exit
+ *     - L1's PAT should be restored after #VMEXIT from L2
+ *     - State save/restore should preserve both L1's and L2's PAT values
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "svm_util.h"
+
+#define L2_GUEST_STACK_SIZE 256
+
+#define PAT_DEFAULT		0x0007040600070406ULL
+#define L1_PAT_VALUE		0x0007040600070404ULL  /* Change PA0 to WT */
+#define L2_VMCB12_PAT		0x0606060606060606ULL  /* All WB */
+#define L2_PAT_MODIFIED		0x0606060606060604ULL  /* Change PA0 to WT */
+#define INVALID_PAT_VALUE	0x0808080808080808ULL  /* 8 is reserved */
+
+bool npt_enabled;
+int nr_iterations;
+
+static void l2_guest_code(void)
+{
+	u64 expected_pat = npt_enabled ? L2_VMCB12_PAT : L1_PAT_VALUE;
+	int i;
+
+	for (i = 0; i < nr_iterations; i++) {
+		GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), expected_pat);
+		GUEST_SYNC(1);
+		GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), expected_pat);
+
+		wrmsr(MSR_IA32_CR_PAT, L2_PAT_MODIFIED);
+		expected_pat = L2_PAT_MODIFIED;
+
+		GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L2_PAT_MODIFIED);
+		GUEST_SYNC(2);
+		GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L2_PAT_MODIFIED);
+
+		vmmcall();
+	}
+}
+
+static void l1_guest_code(struct svm_test_data *svm)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+	int i;
+
+	wrmsr(MSR_IA32_CR_PAT, L1_PAT_VALUE);
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+	generic_svm_setup(svm, l2_guest_code, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = L2_VMCB12_PAT;
+	vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
+
+	for (i = 0; i < nr_iterations; i++) {
+		run_guest(vmcb, svm->vmcb_gpa);
+
+		GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+
+		/*
+		 * If NPT is enabled by L1, L2 has a unique PAT and L1's PAT is
+		 * unchanged. Otherwise, PAT is shared between L1 and L2.
+		 */
+		if (npt_enabled) {
+			GUEST_ASSERT_EQ(vmcb->save.g_pat, L2_PAT_MODIFIED);
+			GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+		} else {
+			GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L2_PAT_MODIFIED);
+		}
+		vmcb->save.rip += 3; /* skip over VMMCALL */
+	}
+
+	GUEST_DONE();
+}
+
+static void l1_guest_code_invalid_gpat(struct svm_test_data *svm)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+
+	/* VMRUN should fail without running L2 */
+	generic_svm_setup(svm, NULL, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = INVALID_PAT_VALUE;
+	run_guest(vmcb, svm->vmcb_gpa);
+
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_ERR);
+	GUEST_DONE();
+}
+
+static void run_test(void *guest_code, bool do_save_restore, int nr_iters)
+{
+	struct kvm_x86_state *state;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct ucall uc;
+	gva_t svm_gva;
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+	vm_enable_cap(vm, KVM_CAP_DISABLE_QUIRKS2,
+		      KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+
+	if (npt_enabled)
+		vm_enable_npt(vm);
+
+	vcpu_alloc_svm(vm, &svm_gva);
+
+	if (npt_enabled)
+		tdp_identity_map_default_memslots(vm);
+
+	vcpu_args_set(vcpu, 1, svm_gva);
+
+	nr_iterations = nr_iters;
+	sync_global_to_guest(vm, npt_enabled);
+	sync_global_to_guest(vm, nr_iterations);
+
+	for (;;) {
+		vcpu_run(vcpu);
+		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			/* NOT REACHED */
+		case UCALL_SYNC:
+			if (do_save_restore) {
+				state = vcpu_save_state(vcpu);
+				kvm_vm_release(vm);
+				vcpu = vm_recreate_with_one_vcpu(vm);
+				vm_enable_cap(vm, KVM_CAP_DISABLE_QUIRKS2,
+					      KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+				vcpu_load_state(vcpu, state);
+				kvm_x86_state_cleanup(state);
+			}
+			break;
+		case UCALL_DONE:
+			kvm_vm_free(vm);
+			return;
+		default:
+			TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+}
+
+#define gpat_test(test_name, guest_code, npt_setting)			\
+do {									\
+	npt_setting;							\
+									\
+	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);
+
+	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;
+}

base-commit: d1568b1332b6b3b36b222c2868fc102727c12a34
-- 
2.54.0.823.g6e5bcc1fc9-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-28 23:10 [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2 Sean Christopherson
@ 2026-05-29  0:47 ` Yosry Ahmed
  2026-05-29 13:27   ` Sean Christopherson
  2026-06-05 18:31 ` Sean Christopherson
  1 sibling, 1 reply; 9+ messages in thread
From: Yosry Ahmed @ 2026-05-29  0:47 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel

> +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);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-29  0:47 ` Yosry Ahmed
@ 2026-05-29 13:27   ` Sean Christopherson
  2026-05-29 14:10     ` Jim Mattson
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2026-05-29 13:27 UTC (permalink / raw)
  To: Yosry Ahmed; +Cc: Paolo Bonzini, kvm, linux-kernel

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-29 13:27   ` Sean Christopherson
@ 2026-05-29 14:10     ` Jim Mattson
  2026-05-29 22:55       ` Sean Christopherson
  0 siblings, 1 reply; 9+ messages in thread
From: Jim Mattson @ 2026-05-29 14:10 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Yosry Ahmed, Paolo Bonzini, kvm, linux-kernel

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();

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-29 14:10     ` Jim Mattson
@ 2026-05-29 22:55       ` Sean Christopherson
  2026-05-29 23:01         ` Yosry Ahmed
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2026-05-29 22:55 UTC (permalink / raw)
  To: Jim Mattson; +Cc: Yosry Ahmed, Paolo Bonzini, kvm, linux-kernel

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).

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-29 22:55       ` Sean Christopherson
@ 2026-05-29 23:01         ` Yosry Ahmed
  2026-06-03 18:33           ` Sean Christopherson
  0 siblings, 1 reply; 9+ messages in thread
From: Yosry Ahmed @ 2026-05-29 23:01 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Jim Mattson, Paolo Bonzini, kvm, linux-kernel

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 :)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-29 23:01         ` Yosry Ahmed
@ 2026-06-03 18:33           ` Sean Christopherson
  2026-06-03 18:39             ` Yosry Ahmed
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2026-06-03 18:33 UTC (permalink / raw)
  To: Yosry Ahmed; +Cc: Jim Mattson, Paolo Bonzini, kvm, linux-kernel

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-06-03 18:33           ` Sean Christopherson
@ 2026-06-03 18:39             ` Yosry Ahmed
  0 siblings, 0 replies; 9+ messages in thread
From: Yosry Ahmed @ 2026-06-03 18:39 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Jim Mattson, Paolo Bonzini, kvm, linux-kernel

> > > 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:

Looks good, thank you!

>
> #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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2
  2026-05-28 23:10 [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2 Sean Christopherson
  2026-05-29  0:47 ` Yosry Ahmed
@ 2026-06-05 18:31 ` Sean Christopherson
  1 sibling, 0 replies; 9+ messages in thread
From: Sean Christopherson @ 2026-06-05 18:31 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Yosry Ahmed

On Thu, 28 May 2026 16:10:52 -0700, Sean Christopherson wrote:
> When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled, verify that KVM
> correctly virtualizes the host PAT MSR and the guest PAT register for
> nested SVM guests.
> 
> With nested NPT disabled:
>  * L1 and L2 share the same PAT
>  * The vmcb12.g_pat is ignored
> 
> [...]

Applied to kvm-x86 svm, thanks!

[1/1] KVM: selftests: Add a test for gPAT handling in L2
      https://github.com/kvm-x86/linux/commit/adeb462cecae

--
https://github.com/kvm-x86/linux/tree/next

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-06-05 18:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 23:10 [PATCH v4] KVM: selftests: Add a test for gPAT handling in L2 Sean Christopherson
2026-05-29  0:47 ` Yosry Ahmed
2026-05-29 13:27   ` Sean Christopherson
2026-05-29 14:10     ` Jim Mattson
2026-05-29 22:55       ` Sean Christopherson
2026-05-29 23:01         ` Yosry Ahmed
2026-06-03 18:33           ` Sean Christopherson
2026-06-03 18:39             ` Yosry Ahmed
2026-06-05 18:31 ` Sean Christopherson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.