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

[Jim Mattson]

On Mon, Apr 6, 2026 at 5:07 PM Sean Christopherson <seanjc@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx

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.