[PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT

Jim Mattson posted 8 patches 3 days, 10 hours ago
[PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
Posted by Jim Mattson 3 days, 10 hours ago
When nested NPT is enabled in vmcb12, copy the (cached and validated)
vmcb12 g_pat field to the guest PAT register. Under KVM, the guest PAT
register lives in the vmcb02 g_pat field.

When NPT is enabled, but nested NPT is disabled, copy L1's IA32_PAT MSR to
the vmcb02 g_pat field, since L2 shares the IA32_PAT MSR with L1.

When NPT is disabled, the vmcb02 g_pat field is ignored by hardware.

Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 1d4ff6408b34..1ff2ede96094 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -646,9 +646,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
 	struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
 	struct kvm_vcpu *vcpu = &svm->vcpu;
 
-	nested_vmcb02_compute_g_pat(svm);
-	vmcb_mark_dirty(vmcb02, VMCB_NPT);
-
 	/* Load the nested guest state */
 	if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
 		new_vmcb12 = true;
@@ -656,6 +653,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
 		svm->nested.force_msr_bitmap_recalc = true;
 	}
 
+	if (npt_enabled) {
+		if (nested_npt_enabled(svm)) {
+			if (unlikely(new_vmcb12 ||
+				     vmcb_is_dirty(vmcb12, VMCB_NPT))) {
+				vmcb02->save.g_pat = svm->nested.gpat;
+				vmcb_mark_dirty(vmcb02, VMCB_NPT);
+			}
+		} else {
+			vmcb02->save.g_pat = vcpu->arch.pat;
+			vmcb_mark_dirty(vmcb02, VMCB_NPT);
+		}
+	}
+
 	if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_SEG))) {
 		vmcb02->save.es = vmcb12->save.es;
 		vmcb02->save.cs = vmcb12->save.cs;
-- 
2.53.0.rc2.204.g2597b5adb4-goog
Re: [PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
Posted by Sean Christopherson 2 days, 13 hours ago
On Thu, Feb 05, 2026, Jim Mattson wrote:
> When nested NPT is enabled in vmcb12, copy the (cached and validated)
> vmcb12 g_pat field to the guest PAT register. Under KVM, the guest PAT
> register lives in the vmcb02 g_pat field.
> 
> When NPT is enabled, but nested NPT is disabled, copy L1's IA32_PAT MSR to
> the vmcb02 g_pat field, since L2 shares the IA32_PAT MSR with L1.
> 
> When NPT is disabled, the vmcb02 g_pat field is ignored by hardware.

Uber nit, the "vmcb02" qualifier can be dropped, i.e.

  When NPT is disabled, the g_pat field is ignored by hardware.

Scoping it to vmcb02 makes it sound like there's a special rule about vmcb02.

> Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 1d4ff6408b34..1ff2ede96094 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -646,9 +646,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
>  	struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
>  	struct kvm_vcpu *vcpu = &svm->vcpu;
>  
> -	nested_vmcb02_compute_g_pat(svm);
> -	vmcb_mark_dirty(vmcb02, VMCB_NPT);
> -
>  	/* Load the nested guest state */
>  	if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
>  		new_vmcb12 = true;
> @@ -656,6 +653,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
>  		svm->nested.force_msr_bitmap_recalc = true;
>  	}
>  
> +	if (npt_enabled) {
> +		if (nested_npt_enabled(svm)) {
> +			if (unlikely(new_vmcb12 ||
> +				     vmcb_is_dirty(vmcb12, VMCB_NPT))) {
> +				vmcb02->save.g_pat = svm->nested.gpat;
> +				vmcb_mark_dirty(vmcb02, VMCB_NPT);
> +			}
> +		} else {
> +			vmcb02->save.g_pat = vcpu->arch.pat;
> +			vmcb_mark_dirty(vmcb02, VMCB_NPT);
> +		}
> +	}

To reduce indentation, how about this?  There's a consistency check for
nested_npt_enabled() vs. npt_enabled, so it's guaranteed to do the right thing.

	if (nested_npt_enabled(svm)) {
		if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_NPT))) {
			vmcb02->save.g_pat = svm->nested.gpat;
			vmcb_mark_dirty(vmcb02, VMCB_NPT);
		}
	} else if (npt_enabled) {
		vmcb02->save.g_pat = vcpu->arch.pat;
		vmcb_mark_dirty(vmcb02, VMCB_NPT);
	}

> +
>  	if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_SEG))) {
>  		vmcb02->save.es = vmcb12->save.es;
>  		vmcb02->save.cs = vmcb12->save.cs;
> -- 
> 2.53.0.rc2.204.g2597b5adb4-goog
>
Re: [PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
Posted by Yosry Ahmed 2 days, 13 hours ago
February 6, 2026 at 10:23 AM, "Sean Christopherson" <seanjc@google.com> wrote:
> 
> On Thu, Feb 05, 2026, Jim Mattson wrote:
> 
> > 
> > When nested NPT is enabled in vmcb12, copy the (cached and validated)
> >  vmcb12 g_pat field to the guest PAT register. Under KVM, the guest PAT
> >  register lives in the vmcb02 g_pat field.
> >  
> >  When NPT is enabled, but nested NPT is disabled, copy L1's IA32_PAT MSR to
> >  the vmcb02 g_pat field, since L2 shares the IA32_PAT MSR with L1.
> >  
> >  When NPT is disabled, the vmcb02 g_pat field is ignored by hardware.
> > 
> Uber nit, the "vmcb02" qualifier can be dropped, i.e.
> 
>  When NPT is disabled, the g_pat field is ignored by hardware.
> 
> Scoping it to vmcb02 makes it sound like there's a special rule about vmcb02.
> 
> > 
> > Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
> >  Signed-off-by: Jim Mattson <jmattson@google.com>
> >  ---
> >  arch/x86/kvm/svm/nested.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> >  
> >  diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> >  index 1d4ff6408b34..1ff2ede96094 100644
> >  --- a/arch/x86/kvm/svm/nested.c
> >  +++ b/arch/x86/kvm/svm/nested.c
> >  @@ -646,9 +646,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> >  struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
> >  struct kvm_vcpu *vcpu = &svm->vcpu;
> >  
> >  - nested_vmcb02_compute_g_pat(svm);
> >  - vmcb_mark_dirty(vmcb02, VMCB_NPT);
> >  -
> >  /* Load the nested guest state */
> >  if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
> >  new_vmcb12 = true;
> >  @@ -656,6 +653,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> >  svm->nested.force_msr_bitmap_recalc = true;
> >  }
> >  
> >  + if (npt_enabled) {
> >  + if (nested_npt_enabled(svm)) {
> >  + if (unlikely(new_vmcb12 ||
> >  + vmcb_is_dirty(vmcb12, VMCB_NPT))) {
> >  + vmcb02->save.g_pat = svm->nested.gpat;
> >  + vmcb_mark_dirty(vmcb02, VMCB_NPT);
> >  + }
> >  + } else {
> >  + vmcb02->save.g_pat = vcpu->arch.pat;
> >  + vmcb_mark_dirty(vmcb02, VMCB_NPT);
> >  + }
> >  + }
> > 
> To reduce indentation, how about this? There's a consistency check for
> nested_npt_enabled() vs. npt_enabled, so it's guaranteed to do the right thing.

You mean the one that goes away after this patch: https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/?

> 
>  if (nested_npt_enabled(svm)) {
>  if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_NPT))) {
>  vmcb02->save.g_pat = svm->nested.gpat;
>  vmcb_mark_dirty(vmcb02, VMCB_NPT);
>  }
>  } else if (npt_enabled) {
>  vmcb02->save.g_pat = vcpu->arch.pat;
>  vmcb_mark_dirty(vmcb02, VMCB_NPT);
>  }
> 
> > 
> > +
> >  if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_SEG))) {
> >  vmcb02->save.es = vmcb12->save.es;
> >  vmcb02->save.cs = vmcb12->save.cs;
> >  -- 
> >  2.53.0.rc2.204.g2597b5adb4-goog
> >
>
Re: [PATCH v3 3/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
Posted by Sean Christopherson 2 days, 12 hours ago
On Fri, Feb 06, 2026, Yosry Ahmed wrote:
> February 6, 2026 at 10:23 AM, "Sean Christopherson" <seanjc@google.com> wrote:
> > >  if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
> > >  new_vmcb12 = true;
> > >  @@ -656,6 +653,19 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> > >  svm->nested.force_msr_bitmap_recalc = true;
> > >  }
> > >  
> > >  + if (npt_enabled) {
> > >  + if (nested_npt_enabled(svm)) {
> > >  + if (unlikely(new_vmcb12 ||
> > >  + vmcb_is_dirty(vmcb12, VMCB_NPT))) {
> > >  + vmcb02->save.g_pat = svm->nested.gpat;
> > >  + vmcb_mark_dirty(vmcb02, VMCB_NPT);
> > >  + }
> > >  + } else {
> > >  + vmcb02->save.g_pat = vcpu->arch.pat;
> > >  + vmcb_mark_dirty(vmcb02, VMCB_NPT);
> > >  + }
> > >  + }
> > > 
> > To reduce indentation, how about this? There's a consistency check for
> > nested_npt_enabled() vs. npt_enabled, so it's guaranteed to do the right thing.
> 
> You mean the one that goes away after this patch: https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/?

Heh, still fine.  All that matters is that nested_npt_enabled() can't be %true
if npt_enabled is %false.