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

[Sean Christopherson]

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

> +static void l2_guest_code(void)
> +{
> +	pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
> +	wrmsr(MSR_IA32_CR_PAT, L2_PAT_MODIFIED);
> +	pat_data->l2_pat_after_write = rdmsr(MSR_IA32_CR_PAT);
> +	pat_data->l2_done = true;
> +	vmmcall();
> +}

...

> +static void run_test(void *l1_code, const char *test_name, bool npt_enabled,
> +		     bool do_save_restore)
> +{
> +	struct pat_test_data *data_hva;
> +	vm_vaddr_t svm_gva, data_gva;
> +	struct kvm_x86_state *state;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	struct ucall uc;
> +
> +	pr_info("Testing: %s\n", test_name);
> +
> +	vm = vm_create_with_one_vcpu(&vcpu, l1_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);
> +
> +	data_gva = vm_vaddr_alloc_page(vm);
> +	data_hva = addr_gva2hva(vm, data_gva);
> +	memset(data_hva, 0, sizeof(*data_hva));

Ugh.

> +
> +	if (npt_enabled)
> +		tdp_identity_map_default_memslots(vm);
> +
> +	vcpu_args_set(vcpu, 2, svm_gva, data_gva);