Re: [PATCH v5 08/10] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE

[Sean Christopherson <seanjc@google.com>]

On Mon, Feb 23, 2026, Jim Mattson wrote:
> Add a 'flags' field to the SVM nested state header, and use bit 0 of the
> flags to indicate that gPAT is stored in the nested state.
> 
> If in guest mode with NPT enabled, store the current vmcb->save.g_pat value
> into the header of the nested state, and set the flag.
> 
> Note that struct kvm_svm_nested_state_hdr is included in a union padded to
> 120 bytes, so there is room to add the flags field and the gpat field
> without changing any offsets.
> 
> Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  arch/x86/include/uapi/asm/kvm.h |  5 +++++
>  arch/x86/kvm/svm/nested.c       | 17 +++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 846a63215ce1..664d04d1db3f 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -495,6 +495,8 @@ struct kvm_sync_regs {
>  
>  #define KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE	0x00000001
>  
> +#define KVM_STATE_SVM_VALID_GPAT	0x00000001
> +
>  /* vendor-independent attributes for system fd (group 0) */
>  #define KVM_X86_GRP_SYSTEM		0
>  #  define KVM_X86_XCOMP_GUEST_SUPP	0
> @@ -531,6 +533,9 @@ struct kvm_svm_nested_state_data {
>  
>  struct kvm_svm_nested_state_hdr {
>  	__u64 vmcb_pa;
> +	__u32 flags;
> +	__u32 reserved;
> +	__u64 gpat;
>  };
>  
>  /* for KVM_CAP_NESTED_STATE */
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 26f758e294ab..5a35277f2364 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1893,6 +1893,10 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
>  	/* First fill in the header and copy it out.  */
>  	if (is_guest_mode(vcpu)) {
>  		kvm_state.hdr.svm.vmcb_pa = svm->nested.vmcb12_gpa;
> +		if (nested_npt_enabled(svm)) {
> +			kvm_state.hdr.svm.flags |= KVM_STATE_SVM_VALID_GPAT;

Bugger.  This isn't going to work.  KVM doesn't reserve any bytes in the header
for SVM, so there's no guarantee/requirement that old userspace won't provide
garbage.  The flag needs to go in kvm_nested_state.flags.  We only reserved space
for 16 flags between VMX and SVM, but that's a future problem as we can always
add e.g. KVM_STATE_NESTED_EXT_FLAGS when we've exausted the current space.

Ugh.  And vmx_set_nested_state() doesn't check for unsupported flags, at all.
But that too is largely a future problem though, i.e. is a non-issue until nVMX
wants to add a new flag.  *sigh*

Anyways, for gPAT, unless I'm missing something, I'm going to squash this:

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 664d04d1db3f..0c1f97c9a2d8 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -485,6 +485,7 @@ struct kvm_sync_regs {
 #define KVM_STATE_NESTED_EVMCS         0x00000004
 #define KVM_STATE_NESTED_MTF_PENDING   0x00000008
 #define KVM_STATE_NESTED_GIF_SET       0x00000100
+#define KVM_STATE_NESTED_GPAT_VALID    0x00000200
 
 #define KVM_STATE_NESTED_SMM_GUEST_MODE        0x00000001
 #define KVM_STATE_NESTED_SMM_VMXON     0x00000002
@@ -495,8 +496,6 @@ struct kvm_sync_regs {
 
 #define KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE        0x00000001
 
-#define KVM_STATE_SVM_VALID_GPAT       0x00000001
-
 /* vendor-independent attributes for system fd (group 0) */
 #define KVM_X86_GRP_SYSTEM             0
 #  define KVM_X86_XCOMP_GUEST_SUPP     0
@@ -533,8 +532,6 @@ struct kvm_svm_nested_state_data {
 
 struct kvm_svm_nested_state_hdr {
        __u64 vmcb_pa;
-       __u32 flags;
-       __u32 reserved;
        __u64 gpat;
 };
 
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 991ee4c03363..099bf8ac10ee 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1848,7 +1848,7 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
        if (is_guest_mode(vcpu)) {
                kvm_state.hdr.svm.vmcb_pa = svm->nested.vmcb12_gpa;
                if (nested_npt_enabled(svm)) {
-                       kvm_state.hdr.svm.flags |= KVM_STATE_SVM_VALID_GPAT;
+                       kvm_state->flags |= KVM_STATE_NESTED_GPAT_VALID;
                        kvm_state.hdr.svm.gpat = svm->vmcb->save.g_pat;
                }
                kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
@@ -1914,7 +1914,8 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 
        if (kvm_state->flags & ~(KVM_STATE_NESTED_GUEST_MODE |
                                 KVM_STATE_NESTED_RUN_PENDING |
-                                KVM_STATE_NESTED_GIF_SET))
+                                KVM_STATE_NESTED_GIF_SET |
+                                KVM_STATE_NESTED_GPAT_VALID))
                return -EINVAL;
 
        /*
@@ -1984,7 +1985,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
         * vmcb_save_area_cached validation above, because gPAT is L2
         * state, but the vmcb_save_area_cached is populated with L1 state.
         */
-       if ((kvm_state->hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT) &&
+       if ((kvm_state->flags & KVM_STATE_NESTED_GPAT_VALID) &&
            !kvm_pat_valid(kvm_state->hdr.svm.gpat))
                goto out_free;
 
@@ -2013,7 +2014,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
        svm_switch_vmcb(svm, &svm->nested.vmcb02);
 
        if (nested_npt_enabled(svm)) {
-               if (kvm_state->hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT)
+               if (kvm_state->flags & KVM_STATE_NESTED_GPAT_VALID)
                        vmcb_set_gpat(svm->vmcb, kvm_state->hdr.svm.gpat);
        }
--

One could argue we should even be more paranoid and do this as well:

	if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE)) {
		if (kvm_state->flags & KVM_STATE_NESTED_GPAT_VALID)
			return -EINVAL:

		svm_leave_nested(vcpu);
		svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
		return 0;
	}

but KVM doesn't enforce GUEST_MODE for KVM_STATE_NESTED_RUN_PENDING, and it's
easy to just ignore gPAT, so I'm inclined to not bother.