Re: [PATCH v7 9/9] KVM: selftests: nSVM: Add svm_nested_pat test

Jim Mattson
2026-04-07T03:59:02Z

On Mon, Apr 6, 2026 at 5:07 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Mar 27, 2026, Jim Mattson 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
> >
> > 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
> > * Save/restore with the vCPU in guest mode preserves both hPAT and gPAT
> >
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> > tools/arch/x86/include/uapi/asm/kvm.h | 2 +
>
> Don't update uAPI headers in tools/, they're not used by KVM selftests (perf
> folks will sync them as needed).
>
> > +#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 */
> > +
> > +/*
> > + * Shared state between L1 and L2 for verification.
> > + */
> > +struct pat_test_data {
> > + uint64_t l2_pat_read;
> > + uint64_t l2_pat_after_write;
> > + uint64_t l1_pat_after_vmexit;
> > + uint64_t vmcb12_gpat_after_exit;
> > + bool l2_done;
> > +};
> > +
> > +static struct pat_test_data *pat_data;
>
> This is ridiculous. Whatever AI you're using is reinventing sync_global_to_guest()
> in a very obfuscated way. Drop the indirection along with the params and the
> full page allocation, and just sync the damn struct.
>
> Actually, this is even dumber than that. The "data" is only ever accessed from
> within the guest; it's used to pass info between L1 and L2. Drop the struct
> entirely and just write global variables.
>
> In general, please clean this test up before submitting v8. All of the L2 code
> is basically copy+paste of itself. This is the second vibe coded selftest (AFAIK)
> you've posted, and it has many of the same flaws as the first one[*]. While I'm
> not opposed to using fancy tools, and the bar is generally lower for selftests,
> the code still needs to be readable and maintainable. This ain't.
>
> [*] https://lore.kernel.org/all/aXJal3srw2-3J5Dm@google.com

I contend that the kvm selftests infrastructure already fails to meet
that bar, but I will toss this in the bit bucket and write a selftest
by hand for the next version.