From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Jonathan Corbet <corbet@lwn.net>,
Shuah Khan <skhan@linuxfoundation.org>,
Thomas Gleixner <tglx@kernel.org>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
Yosry Ahmed <yosry@kernel.org>
Subject: Re: [PATCH v7 5/9] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT
Date: Mon, 6 Apr 2026 16:45:45 -0700 [thread overview]
Message-ID: <adRFqdTKAtfzPMnG@google.com> (raw)
In-Reply-To: <20260327234023.2659476-6-jmattson@google.com>
On Fri, Mar 27, 2026, Jim Mattson wrote:
> When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and 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's g_pat field.
>
> Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
> to hPAT, which is stored in vcpu->arch.pat.
>
> Directing host-initiated accesses to hPAT ensures that KVM_GET/SET_MSRS and
> KVM_GET/SET_NESTED_STATE are independent of each other and can be ordered
> arbitrarily during save and restore. gPAT is saved and restored separately
> via KVM_GET/SET_NESTED_STATE.
>
> Use WARN_ON_ONCE to flag any host-initiated accesses originating from KVM
> itself rather than userspace.
>
> Use pr_warn_once to flag any use of the common MSR-handling code (now
> shared by VMX and TDX) for IA32_PAT by a vCPU that is SVM-capable.
>
> Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/svm/nested.c | 9 -------
> arch/x86/kvm/svm/svm.c | 53 ++++++++++++++++++++++++++++++++++-----
> arch/x86/kvm/svm/svm.h | 1 -
> arch/x86/kvm/x86.c | 6 +++++
> 4 files changed, 53 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 8170042d5fb3..ccc556eb4d2f 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -703,15 +703,6 @@ static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
> return 0;
> }
>
> -void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
> -{
> - if (!svm->nested.vmcb02.ptr)
> - return;
> -
> - /* FIXME: merge g_pat from vmcb01 and vmcb12. */
> - vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
> -}
> -
> static bool nested_vmcb12_has_lbrv(struct kvm_vcpu *vcpu)
> {
> return guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index af808e83173e..34fd07d6ad4d 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2776,6 +2776,47 @@ static bool sev_es_prevent_msr_access(struct kvm_vcpu *vcpu,
> !msr_write_intercepted(vcpu, msr_info->index);
> }
>
> +static bool svm_pat_accesses_gpat(struct kvm_vcpu *vcpu, bool from_host)
> +{
> + struct vcpu_svm *svm = to_svm(vcpu);
> +
> + /*
> + * When nested NPT is enabled, L2 has a separate PAT from L1. Guest
Hmm, do we also want to add "and KVM's quirk is disabled"?
/*
* When nested NPT is enabled and KVM's quirk is disabled, L2 has a
* separate PAT from L1. Guest accesses to IA32_PAT while running L2
* target L2's gPAT; host-initiated accesses always target L1's hPAT so
* that KVM_GET/SET_MSRS and KVM_GET/SET_NESTED_STATE are independent
* of each other and can be ordered arbitrarily during save and restore.
*/
> + * accesses to IA32_PAT while running L2 target L2's gPAT;
> + * host-initiated accesses always target L1's hPAT so that
> + * KVM_GET/SET_MSRS and KVM_GET/SET_NESTED_STATE are independent of
> + * each other and can be ordered arbitrarily during save and restore.
> + */
> + WARN_ON_ONCE(from_host && vcpu->wants_to_run);
> + return !from_host && is_guest_mode(vcpu) && l2_has_separate_pat(svm);
> +}
> +
> +static u64 svm_get_pat(struct kvm_vcpu *vcpu, bool from_host)
> +{
> + if (svm_pat_accesses_gpat(vcpu, from_host))
> + return to_svm(vcpu)->vmcb->save.g_pat;
> + else
> + return vcpu->arch.pat;
> +}
> +
> +static void svm_set_pat(struct kvm_vcpu *vcpu, bool from_host, u64 data)
> +{
> + struct vcpu_svm *svm = to_svm(vcpu);
> +
> + if (svm_pat_accesses_gpat(vcpu, from_host)) {
> + vmcb_set_gpat(svm->vmcb, data);
> + return;
> + }
> +
> + svm->vcpu.arch.pat = data;
> +
> + if (npt_enabled) {
> + vmcb_set_gpat(svm->vmcb01.ptr, data);
> + if (is_guest_mode(&svm->vcpu) && !l2_has_separate_pat(svm))
> + vmcb_set_gpat(svm->vmcb, data);
> + }
> +}
> +
> static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> @@ -2892,6 +2933,9 @@ 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:
> + msr_info->data = svm_get_pat(vcpu, msr_info->host_initiated);
> + break;
> default:
> return kvm_get_msr_common(vcpu, msr_info);
> }
> @@ -2975,13 +3019,10 @@ 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;
>
> - vmcb_set_gpat(svm->vmcb01.ptr, data);
> - if (is_guest_mode(vcpu))
> - nested_vmcb02_compute_g_pat(svm);
> + svm_set_pat(vcpu, msr->host_initiated, data);
> break;
> case MSR_IA32_SPEC_CTRL:
> if (!msr->host_initiated &&
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index b43e37b0448c..0b0279734486 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -868,7 +868,6 @@ void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
> void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
> struct vmcb_save_area *save);
> void nested_sync_control_from_vmcb02(struct vcpu_svm *svm);
> -void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm);
> void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb);
>
> extern struct kvm_x86_nested_ops svm_nested_ops;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0b5d48e75b65..cfb2517f692a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4025,6 +4025,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> }
> break;
> case MSR_IA32_CR_PAT:
> + if (!(efer_reserved_bits & EFER_SVME))
> + pr_warn_once("%s: MSR_IA32_CR_PAT should be handled by AMD vendor-specific code\n", __func__);
If we want to add a sanity check, simply do
WARN_ON_ONCE(efer_reserved_bits & EFER_SVME)?
so that bugs fail loudly.
Hrm. Though I'm leaning towards saying we should keep the "common" behavior in
x86.c. If VMX and TDX didn't need to add get() APIs, I would say move the common
code to vmx/common.h, but that ends up being pretty ugly.
If we do that, the total diff for svm.c is quite nice. Duplicating the
kvm_pat_valid() sucks, but it seems like the lesser of all evils :-/
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e7fdd7a9c280..6281599d5311 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2767,6 +2767,21 @@ static bool sev_es_prevent_msr_access(struct kvm_vcpu *vcpu,
!msr_write_intercepted(vcpu, msr_info->index);
}
+static bool svm_pat_accesses_gpat(struct kvm_vcpu *vcpu, bool from_host)
+{
+ struct vcpu_svm *svm = to_svm(vcpu);
+
+ /*
+ * When nested NPT is enabled, L2 has a separate PAT from L1. Guest
+ * accesses to IA32_PAT while running L2 target L2's gPAT;
+ * host-initiated accesses always target L1's hPAT so that
+ * KVM_GET/SET_MSRS and KVM_GET/SET_NESTED_STATE are independent of
+ * each other and can be ordered arbitrarily during save and restore.
+ */
+ WARN_ON_ONCE(from_host && vcpu->wants_to_run);
+ return !from_host && is_guest_mode(vcpu) && l2_has_separate_pat(svm);
+}
+
static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
{
struct vcpu_svm *svm = to_svm(vcpu);
@@ -2883,6 +2898,12 @@ 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 (svm_pat_accesses_gpat(vcpu, msr_info->host_initiated)) {
+ msr_info->data = to_svm(vcpu)->vmcb->save.g_pat;
+ break;
+ }
+ return kvm_get_msr_common(vcpu, msr_info);
default:
return kvm_get_msr_common(vcpu, msr_info);
}
@@ -2966,14 +2987,23 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
break;
case MSR_IA32_CR_PAT:
+ if (svm_pat_accesses_gpat(vcpu, msr->host_initiated)) {
+ if (kvm_pat_valid(data))
+ return 1;
+
+ vmcb_set_gpat(svm->vmcb, data);
+ break;
+ }
+
ret = kvm_set_msr_common(vcpu, msr);
if (ret)
break;
- 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 (npt_enabled) {
+ vmcb_set_gpat(svm->vmcb01.ptr, data);
+ if (is_guest_mode(&svm->vcpu) && !l2_has_separate_pat(svm))
+ vmcb_set_gpat(svm->vmcb, data);
+ }
break;
case MSR_IA32_SPEC_CTRL:
if (!msr->host_initiated &&
> +
> if (!kvm_pat_valid(data))
> return 1;
>
> @@ -4436,6 +4439,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> break;
> }
> case MSR_IA32_CR_PAT:
> + if (!(efer_reserved_bits & EFER_SVME))
> + pr_warn_once("%s: MSR_IA32_CR_PAT should be handled by AMD vendor-specific code\n", __func__);
And obviously the same here..
> +
> msr_info->data = vcpu->arch.pat;
> break;
> case MSR_MTRRcap:
> --
> 2.53.0.1018.g2bb0e51243-goog
>
next prev parent reply other threads:[~2026-04-06 23:45 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-27 23:40 [PATCH v7 0/9] KVM: x86: nSVM: Improve PAT virtualization Jim Mattson
2026-03-27 23:40 ` [PATCH v7 1/9] KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT Jim Mattson
2026-03-30 7:49 ` kernel test robot
2026-04-02 19:39 ` Jim Mattson
2026-04-02 20:26 ` Yosry Ahmed
2026-04-06 23:27 ` Sean Christopherson
2026-03-27 23:40 ` [PATCH v7 2/9] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode Jim Mattson
2026-03-27 23:40 ` [PATCH v7 3/9] KVM: x86: nSVM: Cache and validate vmcb12 g_pat Jim Mattson
2026-03-27 23:40 ` [PATCH v7 4/9] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT Jim Mattson
2026-03-27 23:40 ` [PATCH v7 5/9] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT Jim Mattson
2026-04-06 23:45 ` Sean Christopherson [this message]
2026-03-27 23:40 ` [PATCH v7 6/9] KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT Jim Mattson
2026-03-27 23:40 ` [PATCH v7 7/9] KVM: Documentation: document KVM_{GET,SET}_NESTED_STATE for SVM Jim Mattson
2026-03-27 23:40 ` [PATCH v7 8/9] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE Jim Mattson
2026-04-06 23:47 ` Sean Christopherson
2026-04-07 3:08 ` Jim Mattson
2026-03-27 23:40 ` [PATCH v7 9/9] KVM: selftests: nSVM: Add svm_nested_pat test Jim Mattson
2026-04-07 0:07 ` Sean Christopherson
2026-04-07 3:58 ` Jim Mattson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=adRFqdTKAtfzPMnG@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=corbet@lwn.net \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=skhan@linuxfoundation.org \
--cc=tglx@kernel.org \
--cc=x86@kernel.org \
--cc=yosry@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox