[PATCH v2 6/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE

Jim Mattson posted 8 patches 3 weeks, 2 days ago
There is a newer version of this series
[PATCH v2 6/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
Posted by Jim Mattson 3 weeks, 2 days ago
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 vmcb save area of the nested state, and set the flag.

Note that most of the vmcb save area in the nested state is populated with
dead (and potentially already clobbered) vmcb01 state. A few fields hold L1
state to be restored at VMEXIT. Previously, the g_pat field was in the
former category.

Also 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 without
changing any offsets.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/uapi/asm/kvm.h |  3 +++
 arch/x86/kvm/svm/nested.c       | 13 ++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 7ceff6583652..80157b9597db 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	BIT(0)
+
 /* vendor-independent attributes for system fd (group 0) */
 #define KVM_X86_GRP_SYSTEM		0
 #  define KVM_X86_XCOMP_GUEST_SUPP	0
@@ -530,6 +532,7 @@ struct kvm_svm_nested_state_data {
 
 struct kvm_svm_nested_state_hdr {
 	__u64 vmcb_pa;
+	__u32 flags;
 };
 
 /* for KVM_CAP_NESTED_STATE */
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5fb31faf2b46..c50fb7172672 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1789,6 +1789,8 @@ 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;
 		kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
 		kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
 
@@ -1823,6 +1825,11 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
 	if (r)
 		return -EFAULT;
 
+	/*
+	 * vmcb01->save.g_pat is dead now, so it is safe to overwrite it with
+	 * vmcb02->save.g_pat, whether or not nested NPT is enabled.
+	 */
+	svm->vmcb01.ptr->save.g_pat = svm->vmcb->save.g_pat;
 	if (copy_to_user(&user_vmcb->save, &svm->vmcb01.ptr->save,
 			 sizeof(user_vmcb->save)))
 		return -EFAULT;
@@ -1904,7 +1911,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 		goto out_free;
 
 	/*
-	 * Validate host state saved from before VMRUN (see
+	 * Validate host state saved from before VMRUN and gPAT (see
 	 * nested_svm_check_permissions).
 	 */
 	__nested_copy_vmcb_save_to_cache(&save_cached, save);
@@ -1951,6 +1958,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	if (ret)
 		goto out_free;
 
+	if (is_guest_mode(vcpu) && nested_npt_enabled(svm) &&
+	    (kvm_state.hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT))
+		svm->vmcb->save.g_pat = save_cached.g_pat;
+
 	svm->nested.force_msr_bitmap_recalc = true;
 
 	kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
-- 
2.52.0.457.g6b5491de43-goog
Re: [PATCH v2 6/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
Posted by Yosry Ahmed 2 weeks, 3 days ago
On Thu, Jan 15, 2026 at 03:21:45PM -0800, 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 vmcb save area of the nested state, and set the flag.
> 
> Note that most of the vmcb save area in the nested state is populated with
> dead (and potentially already clobbered) vmcb01 state. A few fields hold L1
> state to be restored at VMEXIT. Previously, the g_pat field was in the
> former category.
> 
> Also 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 without
> changing any offsets.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/include/uapi/asm/kvm.h |  3 +++
>  arch/x86/kvm/svm/nested.c       | 13 ++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 7ceff6583652..80157b9597db 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	BIT(0)
> +
>  /* vendor-independent attributes for system fd (group 0) */
>  #define KVM_X86_GRP_SYSTEM		0
>  #  define KVM_X86_XCOMP_GUEST_SUPP	0
> @@ -530,6 +532,7 @@ struct kvm_svm_nested_state_data {
>  
>  struct kvm_svm_nested_state_hdr {
>  	__u64 vmcb_pa;
> +	__u32 flags;
>  };
>  
>  /* for KVM_CAP_NESTED_STATE */
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 5fb31faf2b46..c50fb7172672 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1789,6 +1789,8 @@ 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;
>  		kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
>  		kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
>  
> @@ -1823,6 +1825,11 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
>  	if (r)
>  		return -EFAULT;
>  
> +	/*
> +	 * vmcb01->save.g_pat is dead now, so it is safe to overwrite it with
> +	 * vmcb02->save.g_pat, whether or not nested NPT is enabled.
> +	 */
> +	svm->vmcb01.ptr->save.g_pat = svm->vmcb->save.g_pat;
>  	if (copy_to_user(&user_vmcb->save, &svm->vmcb01.ptr->save,
>  			 sizeof(user_vmcb->save)))
>  		return -EFAULT;
> @@ -1904,7 +1911,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  		goto out_free;
>  
>  	/*
> -	 * Validate host state saved from before VMRUN (see
> +	 * Validate host state saved from before VMRUN and gPAT (see
>  	 * nested_svm_check_permissions).
>  	 */
>  	__nested_copy_vmcb_save_to_cache(&save_cached, save);
> @@ -1951,6 +1958,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  	if (ret)
>  		goto out_free;
>  
> +	if (is_guest_mode(vcpu) && nested_npt_enabled(svm) &&
> +	    (kvm_state.hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT))
> +		svm->vmcb->save.g_pat = save_cached.g_pat;

is_guest_mode() should always be true here, right?

> +
>  	svm->nested.force_msr_bitmap_recalc = true;
>  
>  	kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
> -- 
> 2.52.0.457.g6b5491de43-goog
>
Re: [PATCH v2 6/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
Posted by Jim Mattson 3 weeks, 2 days ago
On Thu, Jan 15, 2026 at 3:22 PM Jim Mattson <jmattson@google.com> 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 vmcb save area of the nested state, and set the flag.
>
> Note that most of the vmcb save area in the nested state is populated with
> dead (and potentially already clobbered) vmcb01 state. A few fields hold L1
> state to be restored at VMEXIT. Previously, the g_pat field was in the
> former category.
>
> Also 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 without
> changing any offsets.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/include/uapi/asm/kvm.h |  3 +++
>  arch/x86/kvm/svm/nested.c       | 13 ++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 7ceff6583652..80157b9597db 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       BIT(0)
> +
>  /* vendor-independent attributes for system fd (group 0) */
>  #define KVM_X86_GRP_SYSTEM             0
>  #  define KVM_X86_XCOMP_GUEST_SUPP     0
> @@ -530,6 +532,7 @@ struct kvm_svm_nested_state_data {
>
>  struct kvm_svm_nested_state_hdr {
>         __u64 vmcb_pa;
> +       __u32 flags;
>  };
>
>  /* for KVM_CAP_NESTED_STATE */
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 5fb31faf2b46..c50fb7172672 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1789,6 +1789,8 @@ 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;
>                 kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
>                 kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
>
> @@ -1823,6 +1825,11 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
>         if (r)
>                 return -EFAULT;
>
> +       /*
> +        * vmcb01->save.g_pat is dead now, so it is safe to overwrite it with
> +        * vmcb02->save.g_pat, whether or not nested NPT is enabled.
> +        */
> +       svm->vmcb01.ptr->save.g_pat = svm->vmcb->save.g_pat;

Is this too disgusting? Should I extend the payload by 8 bytes
instead? It seems like such a waste, since most of the save area is
dead/unused. Maybe I could define a new sparse save state structure,
with the ~200 bytes that are currently used, surrounded by padding for
the other 500+ bytes. Then, I could just grab 8 bytes of the padding,
and it wouldn't seem quite as hacky .

>         if (copy_to_user(&user_vmcb->save, &svm->vmcb01.ptr->save,
>                          sizeof(user_vmcb->save)))
>                 return -EFAULT;
> @@ -1904,7 +1911,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>                 goto out_free;
>
>         /*
> -        * Validate host state saved from before VMRUN (see
> +        * Validate host state saved from before VMRUN and gPAT (see
>          * nested_svm_check_permissions).
>          */
>         __nested_copy_vmcb_save_to_cache(&save_cached, save);
> @@ -1951,6 +1958,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>         if (ret)
>                 goto out_free;
>
> +       if (is_guest_mode(vcpu) && nested_npt_enabled(svm) &&
> +           (kvm_state.hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT))
> +               svm->vmcb->save.g_pat = save_cached.g_pat;
> +
>         svm->nested.force_msr_bitmap_recalc = true;
>
>         kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
> --
> 2.52.0.457.g6b5491de43-goog
>
Re: [PATCH v2 6/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
Posted by Yosry Ahmed 2 weeks, 3 days ago
On Thu, Jan 15, 2026 at 08:23:26PM -0800, Jim Mattson wrote:
> On Thu, Jan 15, 2026 at 3:22 PM Jim Mattson <jmattson@google.com> 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 vmcb save area of the nested state, and set the flag.
> >
> > Note that most of the vmcb save area in the nested state is populated with
> > dead (and potentially already clobbered) vmcb01 state. A few fields hold L1
> > state to be restored at VMEXIT. Previously, the g_pat field was in the
> > former category.
> >
> > Also 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 without
> > changing any offsets.
> >
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/include/uapi/asm/kvm.h |  3 +++
> >  arch/x86/kvm/svm/nested.c       | 13 ++++++++++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > index 7ceff6583652..80157b9597db 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       BIT(0)
> > +
> >  /* vendor-independent attributes for system fd (group 0) */
> >  #define KVM_X86_GRP_SYSTEM             0
> >  #  define KVM_X86_XCOMP_GUEST_SUPP     0
> > @@ -530,6 +532,7 @@ struct kvm_svm_nested_state_data {
> >
> >  struct kvm_svm_nested_state_hdr {
> >         __u64 vmcb_pa;
> > +       __u32 flags;
> >  };
> >
> >  /* for KVM_CAP_NESTED_STATE */
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 5fb31faf2b46..c50fb7172672 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -1789,6 +1789,8 @@ 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;
> >                 kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
> >                 kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
> >
> > @@ -1823,6 +1825,11 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
> >         if (r)
> >                 return -EFAULT;
> >
> > +       /*
> > +        * vmcb01->save.g_pat is dead now, so it is safe to overwrite it with
> > +        * vmcb02->save.g_pat, whether or not nested NPT is enabled.
> > +        */
> > +       svm->vmcb01.ptr->save.g_pat = svm->vmcb->save.g_pat;
> 
> Is this too disgusting? Should I extend the payload by 8 bytes
> instead? It seems like such a waste, since most of the save area is
> dead/unused. Maybe I could define a new sparse save state structure,
> with the ~200 bytes that are currently used, surrounded by padding for
> the other 500+ bytes. Then, I could just grab 8 bytes of the padding,
> and it wouldn't seem quite as hacky .

I think this would be cleaner than reusing the vmcb01 field.

One question though, if we decide to start doing save/restore for one of
the save area fields in vmcb01 in the currently unused 500+ bytes (i.e.
the padding), would this be a problem? IIUC the 8 bytes we'll use for
gPAT will overlap with an existing unused field.

> 
> >         if (copy_to_user(&user_vmcb->save, &svm->vmcb01.ptr->save,
> >                          sizeof(user_vmcb->save)))
> >                 return -EFAULT;
> > @@ -1904,7 +1911,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >                 goto out_free;
> >
> >         /*
> > -        * Validate host state saved from before VMRUN (see
> > +        * Validate host state saved from before VMRUN and gPAT (see
> >          * nested_svm_check_permissions).
> >          */
> >         __nested_copy_vmcb_save_to_cache(&save_cached, save);
> > @@ -1951,6 +1958,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >         if (ret)
> >                 goto out_free;
> >
> > +       if (is_guest_mode(vcpu) && nested_npt_enabled(svm) &&
> > +           (kvm_state.hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT))
> > +               svm->vmcb->save.g_pat = save_cached.g_pat;
> > +
> >         svm->nested.force_msr_bitmap_recalc = true;
> >
> >         kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
> > --
> > 2.52.0.457.g6b5491de43-goog
> >