[PATCH v2 0/8] KVM: x86: nSVM: Improve PAT virtualization

[PATCH v2 0/8] KVM: x86: nSVM: Improve PAT virtualization

[Jim Mattson]

Currently, KVM's implementation of nested SVM treats the PAT MSR the same
way whether or not nested NPT is enabled: L1 and L2 share a single
PAT. However, the APM specifies that when nested NPT is enabled, the host
(L1) and the guest (L2) should have independent PATs: hPAT for L1 and gPAT
for L2. This patch series implements the architectural specification in
KVM.

The existing PAT MSR (vcpu->arch.pat) is used for hPAT, and the
vmcb02.save.g_pat field is used for gPAT. With nested NPT enabled, guest
accesses to the IA32_PAT MSR are redirected to gPAT, which is stored in
vmcb02->save.g_pat. All other accesses, including userspace accesses via
KVM_{GET,SET}_MSRS, continue to reference hPAT.

The special handling of userspace accesses ensures save/restore forward
compatibility (i.e. resuming a new checkpoint on an older kernel). When an
old kernel restores a checkpoint from a new kernel, the gPAT will be lost,
and L2 will simply use L1's PAT, which is the behavior of the old kernel
anyway.

v1 -> v2:
  Adhere to the architectural specification
  Drop the preservation of vmcb01->g_pat across virtual SMM
  Store the gPAT rather than the hPAT in the nested state (save.g_pat)
  Fix forward compatibility
  Handle backward compatibility when MSRs are restored after nested state
  (setq-default fill-column 75) [Sean]
  Or the KVM_STATE_SVM_VALID_GPAT bit into flags [Sean]
  
Jim Mattson (8):
  KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT
  KVM: x86: nSVM: Cache g_pat in vmcb_save_area_cached
  KVM: x86: nSVM: Add validity check for vmcb12 g_pat
  KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
  KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT
  KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
  KVM: x86: nSVM: Handle restore of legacy nested state
  KVM: selftests: nSVM: Add svm_nested_pat test

 arch/x86/include/uapi/asm/kvm.h               |   3 +
 arch/x86/kvm/svm/nested.c                     |  49 ++-
 arch/x86/kvm/svm/svm.c                        |  39 +-
 arch/x86/kvm/svm/svm.h                        |   7 +
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/x86/svm_nested_pat_test.c   | 357 ++++++++++++++++++
 6 files changed, 442 insertions(+), 14 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_pat_test.c


base-commit: f62b64b970570c92fe22503b0cdc65be7ce7fc7c
-- 
2.52.0.457.g6b5491de43-goog

[PATCH v2 1/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT

[Jim Mattson]

When the vCPU is in guest mode with nested NPT enabled, guest accesses to
IA32_PAT are redirected to the gPAT register, which is stored in
vmcb02->save.g_pat.

Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
to hPAT, which is stored in vcpu->arch.pat.

This is architected behavior. It also makes it possible to restore a new
checkpoint on an old kernel with reasonable semantics. After the restore,
gPAT will be lost, and L2 will run on L1's PAT. Note that the old kernel
would have always run L2 on L1's PAT.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/svm.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7041498a8091..3f8581adf0c1 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2846,6 +2846,13 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_AMD64_DE_CFG:
 		msr_info->data = svm->msr_decfg;
 		break;
+	case MSR_IA32_CR_PAT:
+		if (!msr_info->host_initiated && is_guest_mode(vcpu) &&
+		    nested_npt_enabled(svm))
+			msr_info->data = svm->vmcb->save.g_pat; /* gPAT */
+		else
+			msr_info->data = vcpu->arch.pat; /* hPAT */
+		break;
 	default:
 		return kvm_get_msr_common(vcpu, msr_info);
 	}
@@ -2929,14 +2936,24 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 
 		break;
 	case MSR_IA32_CR_PAT:
-		ret = kvm_set_msr_common(vcpu, msr);
-		if (ret)
-			break;
+		if (!kvm_pat_valid(data))
+			return 1;
 
-		svm->vmcb01.ptr->save.g_pat = data;
-		if (is_guest_mode(vcpu))
-			nested_vmcb02_compute_g_pat(svm);
-		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
+		if (!msr->host_initiated && is_guest_mode(vcpu) &&
+		    nested_npt_enabled(svm)) {
+			svm->vmcb->save.g_pat = data; /* gPAT */
+			vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
+		} else {
+			vcpu->arch.pat = data; /* hPAT */
+			if (npt_enabled) {
+				svm->vmcb01.ptr->save.g_pat = data;
+				vmcb_mark_dirty(svm->vmcb01.ptr, VMCB_NPT);
+				if (is_guest_mode(vcpu)) {
+					svm->vmcb->save.g_pat = data;
+					vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
+				}
+			}
+		}
 		break;
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr->host_initiated &&
-- 
2.52.0.457.g6b5491de43-goog

[PATCH v2 2/8] KVM: x86: nSVM: Cache g_pat in vmcb_save_area_cached

[Jim Mattson]

To avoid TOCTTOU issues, all fields in the vmcb12 save area that are
subject to validation must be copied to svm->nested.save prior to
validation, since vmcb12 is writable by the guest. Add g_pat to this set in
preparation for validting it.

Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 2 ++
 arch/x86/kvm/svm/svm.h    | 1 +
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f295a41ec659..07a57a43fc3b 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -506,6 +506,8 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
 
 	to->dr6 = from->dr6;
 	to->dr7 = from->dr7;
+
+	to->g_pat = from->g_pat;
 }
 
 void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 7d28a739865f..39138378531e 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -145,6 +145,7 @@ struct vmcb_save_area_cached {
 	u64 cr0;
 	u64 dr7;
 	u64 dr6;
+	u64 g_pat;
 };
 
 struct vmcb_ctrl_area_cached {
-- 
2.52.0.457.g6b5491de43-goog

[PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat

[Jim Mattson]

Add a validity check for g_pat, so that when nested paging is enabled for
vmcb12, an invalid g_pat causes an immediate VMEXIT with exit code
VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
VMRUN/VMEXIT."

Update the signature of __nested_vmcb_check_save() to include a pointer to
a struct vmcb_ctrl_area_cached, since the g_pat validity check depend on
the nested paging control bit.

Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 07a57a43fc3b..e65291434be9 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -369,7 +369,8 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 
 /* Common checks that apply to both L1 and L2 state.  */
 static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
-				     struct vmcb_save_area_cached *save)
+				     struct vmcb_save_area_cached *save,
+				     struct vmcb_ctrl_area_cached *control)
 {
 	if (CC(!(save->efer & EFER_SVME)))
 		return false;
@@ -400,6 +401,10 @@ static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
 	if (CC(!kvm_valid_efer(vcpu, save->efer)))
 		return false;
 
+	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
+	       npt_enabled && !kvm_pat_valid(save->g_pat)))
+		return false;
+
 	return true;
 }
 
@@ -407,8 +412,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	struct vmcb_save_area_cached *save = &svm->nested.save;
+	struct vmcb_ctrl_area_cached *ctl = &svm->nested.ctl;
 
-	return __nested_vmcb_check_save(vcpu, save);
+	return __nested_vmcb_check_save(vcpu, save, ctl);
 }
 
 static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
@@ -1892,7 +1898,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	if (!(save->cr0 & X86_CR0_PG) ||
 	    !(save->cr0 & X86_CR0_PE) ||
 	    (save->rflags & X86_EFLAGS_VM) ||
-	    !__nested_vmcb_check_save(vcpu, &save_cached))
+	    !__nested_vmcb_check_save(vcpu, &save_cached, &ctl_cached))
 		goto out_free;
 
 
-- 
2.52.0.457.g6b5491de43-goog

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

[Jim Mattson]

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 e65291434be9..b0c0184e6e24 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -656,9 +656,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;
@@ -666,6 +663,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.save.g_pat;
+				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.52.0.457.g6b5491de43-goog

[PATCH v2 5/8] KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT

[Jim Mattson]

According to the APM volume 3 pseudo-code for "VMRUN," when nested paging
is enabled in the vmcb, the guest PAT register (gPAT) is saved to the vmcb
on emulated VMEXIT.

Under KVM, the guest PAT register lives in the vmcb02 g_pat field. Save
this value to the vmcb12 g_pat field on emulated VMEXIT.

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

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index b0c0184e6e24..5fb31faf2b46 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1189,6 +1189,9 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 	vmcb12->save.dr6    = svm->vcpu.arch.dr6;
 	vmcb12->save.cpl    = vmcb02->save.cpl;
 
+	if (nested_npt_enabled(svm))
+		vmcb12->save.g_pat = svm->vmcb->save.g_pat;
+
 	if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK)) {
 		vmcb12->save.s_cet	= vmcb02->save.s_cet;
 		vmcb12->save.isst_addr	= vmcb02->save.isst_addr;
-- 
2.52.0.457.g6b5491de43-goog

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

[Jim Mattson]

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

[PATCH v2 7/8] KVM: x86: nSVM: Handle restore of legacy nested state

[Jim Mattson]

When nested NPT is enabled and KVM_SET_NESTED_STATE is used to restore an
old checkpoint (without a valid gPAT), the current IA32_PAT value must be
copied to vmcb02->save.g_pat.

Unfortunately, the current IA32_PAT value may be restored by KVM_SET_MSRS
after KVM_SET_NESTED_STATE.

Introduce a new boolean, svm->nested.restore_gpat_from_pat. If set,
svm_vcpu_pre_run() will copy vcpu->arch.pat to vmcb02->save.g_pat and clear
the boolean.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/svm/nested.c | 9 ++++++---
 arch/x86/kvm/svm/svm.c    | 8 ++++++++
 arch/x86/kvm/svm/svm.h    | 6 ++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index c50fb7172672..61a3e7226cde 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1958,9 +1958,12 @@ 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;
+	if (is_guest_mode(vcpu) && nested_npt_enabled(svm)) {
+		svm->nested.restore_gpat_from_pat =
+			!(kvm_state->hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT);
+		if (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;
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3f8581adf0c1..5dceab9f4c3f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4217,9 +4217,17 @@ static void svm_cancel_injection(struct kvm_vcpu *vcpu)
 
 static int svm_vcpu_pre_run(struct kvm_vcpu *vcpu)
 {
+	struct vcpu_svm *svm = to_svm(vcpu);
+
 	if (to_kvm_sev_info(vcpu->kvm)->need_init)
 		return -EINVAL;
 
+	if (svm->nested.restore_gpat_from_pat) {
+		svm->vmcb->save.g_pat = vcpu->arch.pat;
+		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
+		svm->nested.restore_gpat_from_pat = false;
+	}
+
 	return 1;
 }
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 39138378531e..1964ab6e45f4 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -219,6 +219,12 @@ struct svm_nested_state {
 	 * on its side.
 	 */
 	bool force_msr_bitmap_recalc;
+
+	/*
+	 * Indicates that vcpu->arch.pat should be copied to
+	 * vmcb02->save.g_pat at the next vcpu_run.
+	 */
+	bool restore_gpat_from_pat;
 };
 
 struct vcpu_sev_es_state {
-- 
2.52.0.457.g6b5491de43-goog

[PATCH v2 8/8] KVM: selftests: nSVM: Add svm_nested_pat test

[Jim Mattson]

Verify KVM's virtualization of the PAT MSR and--when nested NPT is
enabled--the vmcb12 g_pat field and the guest PAT register (gPAT).

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/x86/svm_nested_pat_test.c   | 357 ++++++++++++++++++
 2 files changed, 358 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_pat_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 33ff81606638..27f8087eafec 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -109,6 +109,7 @@ TEST_GEN_PROGS_x86 += x86/state_test
 TEST_GEN_PROGS_x86 += x86/vmx_preemption_timer_test
 TEST_GEN_PROGS_x86 += x86/svm_vmcall_test
 TEST_GEN_PROGS_x86 += x86/svm_int_ctl_test
+TEST_GEN_PROGS_x86 += x86/svm_nested_pat_test
 TEST_GEN_PROGS_x86 += x86/svm_nested_shutdown_test
 TEST_GEN_PROGS_x86 += x86/svm_nested_soft_inject_test
 TEST_GEN_PROGS_x86 += x86/tsc_scaling_sync
diff --git a/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c b/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
new file mode 100644
index 000000000000..fa016e65dbf6
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KVM nested SVM PAT test
+ *
+ * Copyright (C) 2026, Google LLC.
+ *
+ * Test that KVM correctly virtualizes the PAT MSR and VMCB g_pat field
+ * for nested SVM guests:
+ *
+ * o With nested NPT disabled:
+ *     - L1 and L2 share the same PAT
+ *     - The vmcb12.g_pat is ignored
+ * o With nested NPT enabled:
+ *     - Invalid g_pat in vmcb12 should cause VMEXIT_INVALID
+ *     - L2 should see vmcb12.g_pat via RDMSR, not L1's PAT
+ *     - L2's writes to PAT should be saved to vmcb12 on exit
+ *     - L1's PAT should be restored after #VMEXIT from L2
+ *     - State save/restore should preserve both L1's and L2's PAT values
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "svm_util.h"
+
+#define L2_GUEST_STACK_SIZE 256
+
+#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;
+
+static void l2_guest_code_npt_disabled(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 l2_guest_code_npt_enabled(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 l2_guest_code_saverestoretest(void)
+{
+	pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
+
+	GUEST_SYNC(1);
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), pat_data->l2_pat_read);
+
+	wrmsr(MSR_IA32_CR_PAT, L2_PAT_MODIFIED);
+	pat_data->l2_pat_after_write = rdmsr(MSR_IA32_CR_PAT);
+
+	GUEST_SYNC(2);
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L2_PAT_MODIFIED);
+
+	pat_data->l2_done = true;
+	vmmcall();
+}
+
+static void l1_svm_code_npt_disabled(struct svm_test_data *svm,
+				     struct pat_test_data *data)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+
+	pat_data = data;
+
+	wrmsr(MSR_IA32_CR_PAT, L1_PAT_VALUE);
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+	generic_svm_setup(svm, l2_guest_code_npt_disabled,
+			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = L2_VMCB12_PAT;
+
+	vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
+
+	run_guest(vmcb, svm->vmcb_gpa);
+
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+	GUEST_ASSERT(data->l2_done);
+
+	GUEST_ASSERT_EQ(data->l2_pat_read, L1_PAT_VALUE);
+
+	GUEST_ASSERT_EQ(data->l2_pat_after_write, L2_PAT_MODIFIED);
+
+	data->l1_pat_after_vmexit = rdmsr(MSR_IA32_CR_PAT);
+	GUEST_ASSERT_EQ(data->l1_pat_after_vmexit, L2_PAT_MODIFIED);
+
+	GUEST_DONE();
+}
+
+static void l1_svm_code_invalid_gpat(struct svm_test_data *svm,
+				     struct pat_test_data *data)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+
+	pat_data = data;
+
+	generic_svm_setup(svm, l2_guest_code_npt_enabled,
+			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = INVALID_PAT_VALUE;
+
+	run_guest(vmcb, svm->vmcb_gpa);
+
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_ERR);
+
+	GUEST_ASSERT(!data->l2_done);
+
+	GUEST_DONE();
+}
+
+static void l1_svm_code_npt_enabled(struct svm_test_data *svm,
+				    struct pat_test_data *data)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+	uint64_t l1_pat_before;
+
+	pat_data = data;
+
+	wrmsr(MSR_IA32_CR_PAT, L1_PAT_VALUE);
+	l1_pat_before = rdmsr(MSR_IA32_CR_PAT);
+	GUEST_ASSERT_EQ(l1_pat_before, L1_PAT_VALUE);
+
+	generic_svm_setup(svm, l2_guest_code_npt_enabled,
+			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = L2_VMCB12_PAT;
+
+	vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
+
+	run_guest(vmcb, svm->vmcb_gpa);
+
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+	GUEST_ASSERT(data->l2_done);
+
+	GUEST_ASSERT_EQ(data->l2_pat_read, L2_VMCB12_PAT);
+
+	GUEST_ASSERT_EQ(data->l2_pat_after_write, L2_PAT_MODIFIED);
+
+	data->vmcb12_gpat_after_exit = vmcb->save.g_pat;
+	GUEST_ASSERT_EQ(data->vmcb12_gpat_after_exit, L2_PAT_MODIFIED);
+
+	data->l1_pat_after_vmexit = rdmsr(MSR_IA32_CR_PAT);
+	GUEST_ASSERT_EQ(data->l1_pat_after_vmexit, L1_PAT_VALUE);
+
+	GUEST_DONE();
+}
+
+static void l1_svm_code_saverestore(struct svm_test_data *svm,
+				    struct pat_test_data *data)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+
+	pat_data = data;
+
+	wrmsr(MSR_IA32_CR_PAT, L1_PAT_VALUE);
+
+	generic_svm_setup(svm, l2_guest_code_saverestoretest,
+			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = L2_VMCB12_PAT;
+	vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
+
+	run_guest(vmcb, svm->vmcb_gpa);
+
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+	GUEST_ASSERT(data->l2_done);
+
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+	GUEST_ASSERT_EQ(vmcb->save.g_pat, L2_PAT_MODIFIED);
+
+	GUEST_DONE();
+}
+
+/*
+ * L2 guest code for multiple VM-entry test.
+ * On first VM-entry, read and modify PAT, then VM-exit.
+ * On second VM-entry, verify we see our modified PAT from first VM-entry.
+ */
+static void l2_guest_code_multi_vmentry(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);
+	vmmcall();
+
+	pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
+	pat_data->l2_done = true;
+	vmmcall();
+}
+
+static void l1_svm_code_multi_vmentry(struct svm_test_data *svm,
+				      struct pat_test_data *data)
+{
+	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	struct vmcb *vmcb = svm->vmcb;
+
+	pat_data = data;
+
+	wrmsr(MSR_IA32_CR_PAT, L1_PAT_VALUE);
+
+	generic_svm_setup(svm, l2_guest_code_multi_vmentry,
+			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	vmcb->save.g_pat = L2_VMCB12_PAT;
+	vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
+
+	run_guest(vmcb, svm->vmcb_gpa);
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+
+	GUEST_ASSERT_EQ(data->l2_pat_after_write, L2_PAT_MODIFIED);
+
+	GUEST_ASSERT_EQ(vmcb->save.g_pat, L2_PAT_MODIFIED);
+
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+	vmcb->save.rip += 3;  /* vmmcall */
+	run_guest(vmcb, svm->vmcb_gpa);
+
+	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+	GUEST_ASSERT(data->l2_done);
+
+	GUEST_ASSERT_EQ(data->l2_pat_read, L2_PAT_MODIFIED);
+
+	GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+	GUEST_DONE();
+}
+
+static void l1_guest_code(struct svm_test_data *svm, struct pat_test_data *data,
+			  int test_num)
+{
+	switch (test_num) {
+	case 0:
+		l1_svm_code_npt_disabled(svm, data);
+		break;
+	case 1:
+		l1_svm_code_invalid_gpat(svm, data);
+		break;
+	case 2:
+		l1_svm_code_npt_enabled(svm, data);
+		break;
+	case 3:
+		l1_svm_code_saverestore(svm, data);
+		break;
+	case 4:
+		l1_svm_code_multi_vmentry(svm, data);
+		break;
+	}
+}
+
+static void run_test(int test_number, 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: %d: %s\n", test_number, test_name);
+
+	vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code);
+	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));
+
+	if (npt_enabled)
+		tdp_identity_map_default_memslots(vm);
+
+	vcpu_args_set(vcpu, 3, svm_gva, data_gva, test_number);
+
+	for (;;) {
+		vcpu_run(vcpu);
+		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			/* NOT REACHED */
+		case UCALL_SYNC:
+			if (do_save_restore) {
+				pr_info("  Save/restore at sync point %ld\n",
+					uc.args[1]);
+				state = vcpu_save_state(vcpu);
+				kvm_vm_release(vm);
+				vcpu = vm_recreate_with_one_vcpu(vm);
+				vcpu_load_state(vcpu, state);
+				kvm_x86_state_cleanup(state);
+			}
+			break;
+		case UCALL_DONE:
+			pr_info("  PASSED\n");
+			kvm_vm_free(vm);
+			return;
+		default:
+			TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_NPT));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_NESTED_STATE));
+
+	run_test(0, "nested NPT disabled", false, false);
+
+	run_test(1, "invalid g_pat", true, false);
+
+	run_test(2, "nested NPT enabled", true, false);
+
+	run_test(3, "save/restore", true, true);
+
+	run_test(4, "multiple entries", true, false);
+
+	return 0;
+}
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH v2 1/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT

[Jim Mattson]

On Thu, Jan 15, 2026 at 3:22 PM Jim Mattson <jmattson@google.com> wrote:
>
> When the vCPU is in guest mode with nested NPT enabled, guest accesses to
> IA32_PAT are redirected to the gPAT register, which is stored in
> vmcb02->save.g_pat.
>
> Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
> to hPAT, which is stored in vcpu->arch.pat.
>
> This is architected behavior. It also makes it possible to restore a new
> checkpoint on an old kernel with reasonable semantics. After the restore,
> gPAT will be lost, and L2 will run on L1's PAT. Note that the old kernel
> would have always run L2 on L1's PAT.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7041498a8091..3f8581adf0c1 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2846,6 +2846,13 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>         case MSR_AMD64_DE_CFG:
>                 msr_info->data = svm->msr_decfg;
>                 break;
> +       case MSR_IA32_CR_PAT:
> +               if (!msr_info->host_initiated && is_guest_mode(vcpu) &&
> +                   nested_npt_enabled(svm))
> +                       msr_info->data = svm->vmcb->save.g_pat; /* gPAT */
> +               else
> +                       msr_info->data = vcpu->arch.pat; /* hPAT */
> +               break;
>         default:
>                 return kvm_get_msr_common(vcpu, msr_info);
>         }
> @@ -2929,14 +2936,24 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>
>                 break;
>         case MSR_IA32_CR_PAT:
> -               ret = kvm_set_msr_common(vcpu, msr);
> -               if (ret)
> -                       break;
> +               if (!kvm_pat_valid(data))
> +                       return 1;
>
> -               svm->vmcb01.ptr->save.g_pat = data;
> -               if (is_guest_mode(vcpu))
> -                       nested_vmcb02_compute_g_pat(svm);
> -               vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> +               if (!msr->host_initiated && is_guest_mode(vcpu) &&
> +                   nested_npt_enabled(svm)) {
> +                       svm->vmcb->save.g_pat = data; /* gPAT */
> +                       vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> +               } else {
> +                       vcpu->arch.pat = data; /* hPAT */
> +                       if (npt_enabled) {
> +                               svm->vmcb01.ptr->save.g_pat = data;
> +                               vmcb_mark_dirty(svm->vmcb01.ptr, VMCB_NPT);
> +                               if (is_guest_mode(vcpu)) {

Oops. That should be "is_guest_mode(vcpu) && !nested_npt_enabled(svm)".

> +                                       svm->vmcb->save.g_pat = data;
> +                                       vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> +                               }
> +                       }
> +               }
>                 break;
>         case MSR_IA32_SPEC_CTRL:
>                 if (!msr->host_initiated &&
> --
> 2.52.0.457.g6b5491de43-goog
>

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

[Jim Mattson]

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 7/8] KVM: x86: nSVM: Handle restore of legacy nested state

[Jim Mattson]

On Thu, Jan 15, 2026 at 3:22 PM Jim Mattson <jmattson@google.com> wrote:
>
> When nested NPT is enabled and KVM_SET_NESTED_STATE is used to restore an
> old checkpoint (without a valid gPAT), the current IA32_PAT value must be
> copied to vmcb02->save.g_pat.
>
> Unfortunately, the current IA32_PAT value may be restored by KVM_SET_MSRS
> after KVM_SET_NESTED_STATE.
>
> Introduce a new boolean, svm->nested.restore_gpat_from_pat. If set,
> svm_vcpu_pre_run() will copy vcpu->arch.pat to vmcb02->save.g_pat and clear
> the boolean.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 9 ++++++---
>  arch/x86/kvm/svm/svm.c    | 8 ++++++++
>  arch/x86/kvm/svm/svm.h    | 6 ++++++
>  3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index c50fb7172672..61a3e7226cde 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1958,9 +1958,12 @@ 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;
> +       if (is_guest_mode(vcpu) && nested_npt_enabled(svm)) {
> +               svm->nested.restore_gpat_from_pat =
> +                       !(kvm_state->hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT);
> +               if (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;
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3f8581adf0c1..5dceab9f4c3f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4217,9 +4217,17 @@ static void svm_cancel_injection(struct kvm_vcpu *vcpu)
>
>  static int svm_vcpu_pre_run(struct kvm_vcpu *vcpu)
>  {
> +       struct vcpu_svm *svm = to_svm(vcpu);
> +
>         if (to_kvm_sev_info(vcpu->kvm)->need_init)
>                 return -EINVAL;
>
> +       if (svm->nested.restore_gpat_from_pat) {
> +               svm->vmcb->save.g_pat = vcpu->arch.pat;
> +               vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> +               svm->nested.restore_gpat_from_pat = false;
> +       }
> +

Something like this is probably needed in svm_get_nested_state() as
well, but without clearing the boolean.

>         return 1;
>  }
>
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 39138378531e..1964ab6e45f4 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -219,6 +219,12 @@ struct svm_nested_state {
>          * on its side.
>          */
>         bool force_msr_bitmap_recalc;
> +
> +       /*
> +        * Indicates that vcpu->arch.pat should be copied to
> +        * vmcb02->save.g_pat at the next vcpu_run.
> +        */
> +       bool restore_gpat_from_pat;
>  };
>
>  struct vcpu_sev_es_state {
> --
> 2.52.0.457.g6b5491de43-goog
>

Re: [PATCH v2 1/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT

[Yosry Ahmed]

On Thu, Jan 15, 2026 at 03:21:40PM -0800, Jim Mattson wrote:
> When the vCPU is in guest mode with nested NPT enabled, guest accesses to
> IA32_PAT are redirected to the gPAT register, which is stored in
> vmcb02->save.g_pat.
> 
> Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
> to hPAT, which is stored in vcpu->arch.pat.
> 
> This is architected behavior. It also makes it possible to restore a new
> checkpoint on an old kernel with reasonable semantics. After the restore,
> gPAT will be lost, and L2 will run on L1's PAT. Note that the old kernel
> would have always run L2 on L1's PAT.

This creates a difference in MSR_IA32_CR_PAT handling with nested SVM
and nested VMX, right?

AFAICT, reading MSR_IA32_CR_PAT while an L2 VMX guest is running will
read L2's PAT. With this change, the same scenario on SVM will read L1's
PAT. We can claim that it was always L1's PAT though, because we've
always been running L2 with L1's PAT.

I am just raising this in case it's a problem to have different behavior
for SVM and VMX. I understand that we need to do this to be able to
save/restore L1's PAT with SVM in guest mode and maintain backward
compatibility.

IIUC VMX does not have the same issue because host and guest PAT are
both in vmcs12 and are both saved/restored appropriately.

> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7041498a8091..3f8581adf0c1 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2846,6 +2846,13 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	case MSR_AMD64_DE_CFG:
>  		msr_info->data = svm->msr_decfg;
>  		break;
> +	case MSR_IA32_CR_PAT:
> +		if (!msr_info->host_initiated && is_guest_mode(vcpu) &&
> +		    nested_npt_enabled(svm))
> +			msr_info->data = svm->vmcb->save.g_pat; /* gPAT */
> +		else
> +			msr_info->data = vcpu->arch.pat; /* hPAT */
> +		break;
>  	default:
>  		return kvm_get_msr_common(vcpu, msr_info);
>  	}
> @@ -2929,14 +2936,24 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  
>  		break;
>  	case MSR_IA32_CR_PAT:
> -		ret = kvm_set_msr_common(vcpu, msr);
> -		if (ret)
> -			break;
> +		if (!kvm_pat_valid(data))
> +			return 1;
>  
> -		svm->vmcb01.ptr->save.g_pat = data;

This is a bug fix, L2 is now able to alter L1's PAT, right?

> -		if (is_guest_mode(vcpu))
> -			nested_vmcb02_compute_g_pat(svm);
> -		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);

This looks like another bug fix, it seems like we'll update vmcb01 but
clear the clean bit in vmcb02 if we're in guest mode.

Probably worth calling these out (and CC:stable, Fixes:.., etc)?

We probably need a comment here explaining the gPAT vs hPAT case, I
don't think it's super obvious why we only redirect L2's own accesses to
its PAT but not userspace's.

> +		if (!msr->host_initiated && is_guest_mode(vcpu) &&
> +		    nested_npt_enabled(svm)) {
> +			svm->vmcb->save.g_pat = data; /* gPAT */
> +			vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> +		} else {
> +			vcpu->arch.pat = data; /* hPAT */

Should we call kvm_set_msr_common() here instead of setting
vcpu->arch.pat? The kvm_pat_valid() call would be redundant but that
should be fine. My main concern is if kvm_set_msr_common() gains more
logic for MSR_IA32_CR_PAT that isn't reflected here. Probably unlikely
tho..

> +			if (npt_enabled) {
> +				svm->vmcb01.ptr->save.g_pat = data;
> +				vmcb_mark_dirty(svm->vmcb01.ptr, VMCB_NPT);
> +				if (is_guest_mode(vcpu)) {

IIUC (with the fix you mentioned) this is because L1 and L2 share the
PAT if nested NPT is disabled, and if we're already in guest mode then
we also need to update vmcb02 (as it was already created based on vmcb01
with the old PAT). Probably worth a comment.

> +					svm->vmcb->save.g_pat = data;
> +					vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> +				}
> +			}
> +		}
>  		break;
>  	case MSR_IA32_SPEC_CTRL:
>  		if (!msr->host_initiated &&
> -- 
> 2.52.0.457.g6b5491de43-goog
> 

Re: [PATCH v2 2/8] KVM: x86: nSVM: Cache g_pat in vmcb_save_area_cached

[Yosry Ahmed]

On Thu, Jan 15, 2026 at 03:21:41PM -0800, Jim Mattson wrote:
> To avoid TOCTTOU issues, all fields in the vmcb12 save area that are
> subject to validation must be copied to svm->nested.save prior to
> validation, since vmcb12 is writable by the guest. Add g_pat to this set in
> preparation for validting it.
> 
> Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")

g_pat is not currently used from VMCB12, so this patch isn't technically
fixing an issue, right? I suspect this only applies to patch 3.

Anyway, I think it's probably best to squash this into patch 3. Also
maybe CC stable?

> Signed-off-by: Jim Mattson <jmattson@google.com>

If you decide to keep this as an individual patch, feel free to add:

Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>

> ---
>  arch/x86/kvm/svm/nested.c | 2 ++
>  arch/x86/kvm/svm/svm.h    | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index f295a41ec659..07a57a43fc3b 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -506,6 +506,8 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
>  
>  	to->dr6 = from->dr6;
>  	to->dr7 = from->dr7;
> +
> +	to->g_pat = from->g_pat;
>  }
>  
>  void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 7d28a739865f..39138378531e 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -145,6 +145,7 @@ struct vmcb_save_area_cached {
>  	u64 cr0;
>  	u64 dr7;
>  	u64 dr6;
> +	u64 g_pat;
>  };
>  
>  struct vmcb_ctrl_area_cached {
> -- 
> 2.52.0.457.g6b5491de43-goog
> 

Re: [PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat

[Yosry Ahmed]

On Thu, Jan 15, 2026 at 03:21:42PM -0800, Jim Mattson wrote:
> Add a validity check for g_pat, so that when nested paging is enabled for
> vmcb12, an invalid g_pat causes an immediate VMEXIT with exit code
> VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
> VMRUN/VMEXIT."
> 
> Update the signature of __nested_vmcb_check_save() to include a pointer to
> a struct vmcb_ctrl_area_cached, since the g_pat validity check depend on
> the nested paging control bit.
> 
> Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 07a57a43fc3b..e65291434be9 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -369,7 +369,8 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
>  
>  /* Common checks that apply to both L1 and L2 state.  */
>  static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> -				     struct vmcb_save_area_cached *save)
> +				     struct vmcb_save_area_cached *save,
> +				     struct vmcb_ctrl_area_cached *control)
>  {
>  	if (CC(!(save->efer & EFER_SVME)))
>  		return false;
> @@ -400,6 +401,10 @@ static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
>  	if (CC(!kvm_valid_efer(vcpu, save->efer)))
>  		return false;
>  
> +	if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
> +	       npt_enabled && !kvm_pat_valid(save->g_pat)))

If this lands after "KVM: nSVM: Drop the non-architectural consistency
check for NP_ENABLE" [1], the npt_enabled check can be dropped, as
SVM_NESTED_CTL_NP_ENABLE will be cleared if the guest cannot use NPTs.

[1]https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/

> +		return false;
> +
>  	return true;
>  }
>  
> @@ -407,8 +412,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	struct vmcb_save_area_cached *save = &svm->nested.save;
> +	struct vmcb_ctrl_area_cached *ctl = &svm->nested.ctl;
>  
> -	return __nested_vmcb_check_save(vcpu, save);
> +	return __nested_vmcb_check_save(vcpu, save, ctl);
>  }
>  
>  static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
> @@ -1892,7 +1898,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  	if (!(save->cr0 & X86_CR0_PG) ||
>  	    !(save->cr0 & X86_CR0_PE) ||
>  	    (save->rflags & X86_EFLAGS_VM) ||
> -	    !__nested_vmcb_check_save(vcpu, &save_cached))
> +	    !__nested_vmcb_check_save(vcpu, &save_cached, &ctl_cached))

save_cached here is from vmcb01, but ctl_cached is from vmcb12, right?
So we're conditioning the PAT validity check on L1's gPAT on NPT being
enabled for L2 IIUC.

Perhaps we should pass 'bool guest_npt' instead of the cached control
area?  Then we can just pass npt_enabled here IIUC.

>  		goto out_free;
>  
>  
> -- 
> 2.52.0.457.g6b5491de43-goog
> 

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

[Yosry Ahmed]

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
> >

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

[Yosry Ahmed]

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 4/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT

[Yosry Ahmed]

On Thu, Jan 15, 2026 at 03:21:43PM -0800, 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.
> 
> 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 e65291434be9..b0c0184e6e24 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -656,9 +656,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);

This is last use of the function, right? Should we drop it now?

> -	vmcb_mark_dirty(vmcb02, VMCB_NPT);
> -
>  	/* Load the nested guest state */
>  	if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
>  		new_vmcb12 = true;
> @@ -666,6 +663,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.save.g_pat;
> +				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.52.0.457.g6b5491de43-goog
> 

Re: [PATCH v2 1/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT

[Jim Mattson]

On Wed, Jan 21, 2026 at 5:21 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Thu, Jan 15, 2026 at 03:21:40PM -0800, Jim Mattson wrote:
> > When the vCPU is in guest mode with nested NPT enabled, guest accesses to
> > IA32_PAT are redirected to the gPAT register, which is stored in
> > vmcb02->save.g_pat.
> >
> > Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
> > to hPAT, which is stored in vcpu->arch.pat.
> >
> > This is architected behavior. It also makes it possible to restore a new
> > checkpoint on an old kernel with reasonable semantics. After the restore,
> > gPAT will be lost, and L2 will run on L1's PAT. Note that the old kernel
> > would have always run L2 on L1's PAT.
>
> This creates a difference in MSR_IA32_CR_PAT handling with nested SVM
> and nested VMX, right?

Correct.

> AFAICT, reading MSR_IA32_CR_PAT while an L2 VMX guest is running will
> read L2's PAT. With this change, the same scenario on SVM will read L1's
> PAT. We can claim that it was always L1's PAT though, because we've
> always been running L2 with L1's PAT.
>
> I am just raising this in case it's a problem to have different behavior
> for SVM and VMX. I understand that we need to do this to be able to
> save/restore L1's PAT with SVM in guest mode and maintain backward
> compatibility.
>
> IIUC VMX does not have the same issue because host and guest PAT are
> both in vmcs12 and are both saved/restored appropriately.

Correct.

> >
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/kvm/svm/svm.c | 31 ++++++++++++++++++++++++-------
> >  1 file changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 7041498a8091..3f8581adf0c1 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -2846,6 +2846,13 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >       case MSR_AMD64_DE_CFG:
> >               msr_info->data = svm->msr_decfg;
> >               break;
> > +     case MSR_IA32_CR_PAT:
> > +             if (!msr_info->host_initiated && is_guest_mode(vcpu) &&
> > +                 nested_npt_enabled(svm))
> > +                     msr_info->data = svm->vmcb->save.g_pat; /* gPAT */
> > +             else
> > +                     msr_info->data = vcpu->arch.pat; /* hPAT */
> > +             break;
> >       default:
> >               return kvm_get_msr_common(vcpu, msr_info);
> >       }
> > @@ -2929,14 +2936,24 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> >
> >               break;
> >       case MSR_IA32_CR_PAT:
> > -             ret = kvm_set_msr_common(vcpu, msr);
> > -             if (ret)
> > -                     break;
> > +             if (!kvm_pat_valid(data))
> > +                     return 1;
> >
> > -             svm->vmcb01.ptr->save.g_pat = data;
>
> This is a bug fix, L2 is now able to alter L1's PAT, right?

Yes, L1 and L2 share a PAT today. The whole series is fixing that mistake.

> > -             if (is_guest_mode(vcpu))
> > -                     nested_vmcb02_compute_g_pat(svm);
> > -             vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
>
> This looks like another bug fix, it seems like we'll update vmcb01 but
> clear the clean bit in vmcb02 if we're in guest mode.

I will split this out into a separate patch, since this is tangential
to the rest of the series.

> Probably worth calling these out (and CC:stable, Fixes:.., etc)?
>
> We probably need a comment here explaining the gPAT vs hPAT case, I
> don't think it's super obvious why we only redirect L2's own accesses to
> its PAT but not userspace's.

Point taken.

> > +             if (!msr->host_initiated && is_guest_mode(vcpu) &&
> > +                 nested_npt_enabled(svm)) {
> > +                     svm->vmcb->save.g_pat = data; /* gPAT */
> > +                     vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> > +             } else {
> > +                     vcpu->arch.pat = data; /* hPAT */
>
> Should we call kvm_set_msr_common() here instead of setting
> vcpu->arch.pat? The kvm_pat_valid() call would be redundant but that
> should be fine. My main concern is if kvm_set_msr_common() gains more
> logic for MSR_IA32_CR_PAT that isn't reflected here. Probably unlikely
> tho..

There are already three open-coded assignments to vcpu->arch.pat on
the VMX side. Perhaps this should be avoided, but it's tangential to
this series.

> > +                     if (npt_enabled) {
> > +                             svm->vmcb01.ptr->save.g_pat = data;
> > +                             vmcb_mark_dirty(svm->vmcb01.ptr, VMCB_NPT);
> > +                             if (is_guest_mode(vcpu)) {
>
> IIUC (with the fix you mentioned) this is because L1 and L2 share the
> PAT if nested NPT is disabled, and if we're already in guest mode then
> we also need to update vmcb02 (as it was already created based on vmcb01
> with the old PAT). Probably worth a comment.

Noted.

> > +                                     svm->vmcb->save.g_pat = data;
> > +                                     vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> > +                             }
> > +                     }
> > +             }
> >               break;
> >       case MSR_IA32_SPEC_CTRL:
> >               if (!msr->host_initiated &&
> > --
> > 2.52.0.457.g6b5491de43-goog
> >

Re: [PATCH v2 2/8] KVM: x86: nSVM: Cache g_pat in vmcb_save_area_cached

[Jim Mattson]

On Wed, Jan 21, 2026 at 5:28 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Thu, Jan 15, 2026 at 03:21:41PM -0800, Jim Mattson wrote:
> > To avoid TOCTTOU issues, all fields in the vmcb12 save area that are
> > subject to validation must be copied to svm->nested.save prior to
> > validation, since vmcb12 is writable by the guest. Add g_pat to this set in
> > preparation for validting it.
> >
> > Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
>
> g_pat is not currently used from VMCB12, so this patch isn't technically
> fixing an issue, right? I suspect this only applies to patch 3.

The fact that VMCB12.g_pat isn't used *is* the issue. This patch is
part of the fix. To make the fix easier to review, it's broken into
several pieces.

> Anyway, I think it's probably best to squash this into patch 3. Also
> maybe CC stable?

I will squash it in the next version.

> > Signed-off-by: Jim Mattson <jmattson@google.com>
>
> If you decide to keep this as an individual patch, feel free to add:
>
> Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>
>
> > ---
> >  arch/x86/kvm/svm/nested.c | 2 ++
> >  arch/x86/kvm/svm/svm.h    | 1 +
> >  2 files changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index f295a41ec659..07a57a43fc3b 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -506,6 +506,8 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
> >
> >       to->dr6 = from->dr6;
> >       to->dr7 = from->dr7;
> > +
> > +     to->g_pat = from->g_pat;
> >  }
> >
> >  void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 7d28a739865f..39138378531e 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -145,6 +145,7 @@ struct vmcb_save_area_cached {
> >       u64 cr0;
> >       u64 dr7;
> >       u64 dr6;
> > +     u64 g_pat;
> >  };
> >
> >  struct vmcb_ctrl_area_cached {
> > --
> > 2.52.0.457.g6b5491de43-goog
> >

Re: [PATCH v2 3/8] KVM: x86: nSVM: Add validity check for vmcb12 g_pat

[Jim Mattson]

On Wed, Jan 21, 2026 at 5:41 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Thu, Jan 15, 2026 at 03:21:42PM -0800, Jim Mattson wrote:
> > Add a validity check for g_pat, so that when nested paging is enabled for
> > vmcb12, an invalid g_pat causes an immediate VMEXIT with exit code
> > VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
> > VMRUN/VMEXIT."
> >
> > Update the signature of __nested_vmcb_check_save() to include a pointer to
> > a struct vmcb_ctrl_area_cached, since the g_pat validity check depend on
> > the nested paging control bit.
> >
> > Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/kvm/svm/nested.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 07a57a43fc3b..e65291434be9 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -369,7 +369,8 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
> >
> >  /* Common checks that apply to both L1 and L2 state.  */
> >  static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> > -                                  struct vmcb_save_area_cached *save)
> > +                                  struct vmcb_save_area_cached *save,
> > +                                  struct vmcb_ctrl_area_cached *control)
> >  {
> >       if (CC(!(save->efer & EFER_SVME)))
> >               return false;
> > @@ -400,6 +401,10 @@ static bool __nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> >       if (CC(!kvm_valid_efer(vcpu, save->efer)))
> >               return false;
> >
> > +     if (CC((control->nested_ctl & SVM_NESTED_CTL_NP_ENABLE) &&
> > +            npt_enabled && !kvm_pat_valid(save->g_pat)))
>
> If this lands after "KVM: nSVM: Drop the non-architectural consistency
> check for NP_ENABLE" [1], the npt_enabled check can be dropped, as
> SVM_NESTED_CTL_NP_ENABLE will be cleared if the guest cannot use NPTs.
>
> [1]https://lore.kernel.org/kvm/20260115011312.3675857-16-yosry.ahmed@linux.dev/

I don't see it today, but I will probably forget to keep checking. :)

> > +             return false;
> > +
> >       return true;
> >  }
> >
> > @@ -407,8 +412,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu)
> >  {
> >       struct vcpu_svm *svm = to_svm(vcpu);
> >       struct vmcb_save_area_cached *save = &svm->nested.save;
> > +     struct vmcb_ctrl_area_cached *ctl = &svm->nested.ctl;
> >
> > -     return __nested_vmcb_check_save(vcpu, save);
> > +     return __nested_vmcb_check_save(vcpu, save, ctl);
> >  }
> >
> >  static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
> > @@ -1892,7 +1898,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >       if (!(save->cr0 & X86_CR0_PG) ||
> >           !(save->cr0 & X86_CR0_PE) ||
> >           (save->rflags & X86_EFLAGS_VM) ||
> > -         !__nested_vmcb_check_save(vcpu, &save_cached))
> > +         !__nested_vmcb_check_save(vcpu, &save_cached, &ctl_cached))
>
> save_cached here is from vmcb01, but ctl_cached is from vmcb12, right?
> So we're conditioning the PAT validity check on L1's gPAT on NPT being
> enabled for L2 IIUC.
>
> Perhaps we should pass 'bool guest_npt' instead of the cached control
> area?  Then we can just pass npt_enabled here IIUC.

I'm going to cache it on its own to avoid confusion.

> >               goto out_free;
> >
> >
> > --
> > 2.52.0.457.g6b5491de43-goog
> >