From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f202.google.com (mail-pl1-f202.google.com [209.85.214.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1717C260580 for ; Tue, 17 Feb 2026 23:27:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771370879; cv=none; b=BZZYN736EsYSgETMWyvxVI19VFqKIBqJCNm/fJ0M2kYw/6YM2bVVy1Y+cexTIjfVdh6Y5fEuMuODyGO+3TnjL43btt8nAPUJnJcFtno2FmNyQkeB2OhQsiNzmNDj7iZwsG3wUdUpCB//SUM5puVS29mUz4WYVPdoXo9ZlRrwzM0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771370879; c=relaxed/simple; bh=j+XzuPdbmTm5dTJykXMljs1HvLFtccXg5tafPS5P3bY=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=pZMLVUqRgPYg3Bn1Ck0M9NdD8e+n8KJt8kAoY1a6W1nDlyiqF4leC5pDSphcxYO0JGXTkH549OpFMCKA5ty9xievx8VtjD9fZj/UnDdSFpczfO02zaCda1x2/2Xgtp16Ud8SsH9JjkRAaY0fuT5l0XQvWnYVEd+rVeQTKcbLRB4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=UozcoO6g; arc=none smtp.client-ip=209.85.214.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="UozcoO6g" Received: by mail-pl1-f202.google.com with SMTP id d9443c01a7336-2ab4de9580dso262953835ad.3 for ; Tue, 17 Feb 2026 15:27:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1771370876; x=1771975676; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=5X5KS6rCju+8fHQdzpBX7hCsqBgb5Dp642p6kUU+O1c=; b=UozcoO6gVvD2wzqjO9ETcbTgnBgBxdD2FNBcIkOIqh7G4nCnAJWMQmJkNj+IZZnzcL EEOFygbHFPvR9q0vHrqFmN56EQDkWHdaavC5USPXPNBrHTZXbSO09YhCwv+sFRgsowB0 aKn5aUyH1AvVXIzroeo4yuMCPk7opyZNU6TLJmbrogoNI4TJWD5Z+nGaTXUactoRvtDP Mz4sN7kMqJLbZhyz3x9H5NN/hw5MYwkPSxTspvDRPpixl81wPX7Qby9VOiKI36XWJaCR gQUxy2KAjwYrLahroCj/QkIgwlBbRgDM1orNW/SsGcoa48ly9/UWmB5a7uxAvodXfks+ oqTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1771370876; x=1771975676; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=5X5KS6rCju+8fHQdzpBX7hCsqBgb5Dp642p6kUU+O1c=; b=iPIG+5azA8qi95fWFwsmCDTFRHRvgsfVHLn7dHimzO8167bjicHjimiZVrwGE7nB2V QYLx5yGHsNQmnMFQrDMoAI/9nMNTrHc+9AX/0T0m2SWmFhTOythnMUrxEAQQX4vJSwEK dv5yH0jjBngxvkF/YdZ2Wff9vvdg97OvPSPTongI21btlBfUgIhR+3hRn0LpRhqeyzxT YDeyZWzER85i7pl4490c2VSIEWXxnLiprXTiZWH1rBf3Ywwl2FzkyViUUZFkiPi00Ce/ e6/sqJx7xMf52Ogq0cj0wvmgBcu9d6P6LOmDD1lKXDrDYVbCFvUHXNfGRnJwOtSwKCrp PqMQ== X-Forwarded-Encrypted: i=1; AJvYcCXGoOUOOkihYO5xNFbUlSUiri9tcW+cinh0DcZ82SP9aSMRB8fiRpLs2BwcZFuPocjSlmzWll8df1MsJr4=@vger.kernel.org X-Gm-Message-State: AOJu0YyEZedVrKYowQUjRe+VwxCpX/l+zWX6Qn1ZgzialqCr4KOBXlC+ 6LmBH9zsml9dXyzRe+YHSVighY6ZuvAB2Ek1H1D3xx+1SMqV4H+ZMJeYVcTVSXpAco8vEG5EJLd V9zxKeg== X-Received: from pgc10.prod.google.com ([2002:a05:6a02:2f8a:b0:c6e:1a85:93a6]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6300:6702:b0:38e:90ca:5a2b with SMTP id adf61e73a8af0-39483940fb7mr11214658637.17.1771370876144; Tue, 17 Feb 2026 15:27:56 -0800 (PST) Date: Tue, 17 Feb 2026 15:27:54 -0800 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260212155905.3448571-1-jmattson@google.com> <20260212155905.3448571-5-jmattson@google.com> Message-ID: Subject: Re: [PATCH v4 4/8] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT From: Sean Christopherson To: Jim Mattson Cc: Yosry Ahmed , Paolo Bonzini , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Shuah Khan , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Fri, Feb 13, 2026, Jim Mattson wrote: > On Fri, Feb 13, 2026 at 2:19=E2=80=AFPM Sean Christopherson wrote: > > > > > > + vmcb_set_gpat(svm->vmcb01.ptr, data); > > > > > > + if (is_guest_mode(&svm->vcpu) && !nested_npt_enable= d(svm)) > > > > > > + vmcb_set_gpat(svm->nested.vmcb02.ptr, data)= ; > > > > > > + } > > > > > > +} > > > > > > > > > > Is it me, or is it a bit confusing that svm_set_gpat() sets L2's = gPAT > > > > > not L1's, and svm_set_hpat() calls vmcb_set_gpat()? > > > > > > > > It's not just you. I don't find it confusing per se, more that it'= s really > > > > subtle. > > > > > > > > > "gpat" means different things in the context of the VMCB or other= wise, > > > > > which kinda makes sense but is also not super clear. Maybe > > > > > svm_set_l1_gpat() and svm_set_l2_gpat() is more clear? > > > > > > > > I think just svm_set_l1_pat() and svm_set_l2_pat(), because gpat st= raight up > > > > doesn't exist when NPT is disabled/unsupported. > > > > > > My intention was that "gpat" and "hpat" were from the perspective of = the vCPU. > > > > > > I dislike svm_set_l1_pat() and svm_set_l2_pat(). As you point out > > > above, there is no independent L2 PAT when nested NPT is disabled. I > > > think that's less obvious than the fact that there is no gPAT from th= e > > > vCPU's perspective. My preference is to follow the APM terminology > > > when possible. Making up our own terms just leads to confusion. > > > > How about svm_set_pat() and svm_get_gpat()? Because hPAT doesn't exist= when NPT > > is unsupported/disabled, but KVM still needs to set the vCPU's emulated= PAT value. >=20 > What if we don't break it up this way at all? Instead of distributing > the logic between svm_[gs]set_msr() and a few helper functions, we > could just have svm_[gs]et_msr() call svm_[gs]et_pat(), and all of the > logic can go in these two functions. I like it. And AFAICT it largely Just Works, because the calls from svm_set_nested_state() will always be routed to gpat since the calls are al= ready guarded with is_guest_mode() + nested_npt_enabled(). Side topic, either as a prep patch (to duplicate code) or as a follow-up pa= tch (to move the PAT handling in x86.c to vmx.c), the "common" handling of PAT = should be fully forked between VMX and SVM. As of this patch, it's not just misle= ading, it's actively dangerous since calling kvm_get_msr_common() for SVM would ge= t the wrong value. FWIW, this is what I ended up with when hacking on top of your patches to s= ee how this played out. --- arch/x86/kvm/svm/nested.c | 4 +-- arch/x86/kvm/svm/svm.c | 64 +++++++++++++++++++++++++-------------- arch/x86/kvm/svm/svm.h | 19 +----------- arch/x86/kvm/vmx/vmx.c | 10 ++++-- arch/x86/kvm/x86.c | 9 ------ 5 files changed, 51 insertions(+), 55 deletions(-) diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index d854d29b0bd8..361f189d3967 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -2075,9 +2075,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu= , =20 if (nested_npt_enabled(svm)) { if (kvm_state->hdr.svm.flags & KVM_STATE_SVM_VALID_GPAT) { - svm_set_gpat(svm, kvm_state->hdr.svm.gpat); + svm_set_pat(vcpu, kvm_state->hdr.svm.gpat, true); } else { - svm_set_gpat(svm, vcpu->arch.pat); + svm_set_pat(vcpu, vcpu->arch.pat, true); svm->nested.legacy_gpat_semantics =3D true; } } diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 93ce0c3232c6..94c3b3cadd54 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -251,6 +251,44 @@ int svm_set_efer(struct kvm_vcpu *vcpu, u64 efer) return 0; } =20 +static bool svm_is_access_to_gpat(struct kvm_vcpu *vcpu, bool host_initiat= ed) +{ + /* + * 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 for backward and + * forward KVM_SET_MSRS compatibility with older kernels. + */ + WARN_ON_ONCE(host_initiated && vcpu->wants_to_run); + + return !host_initiated && is_guest_mode(vcpu) && + nested_npt_enabled(to_svm(vcpu)); +} + +void svm_set_pat(struct kvm_vcpu *vcpu, u64 pat, bool host_initiated) +{ + struct vcpu_svm *svm =3D to_svm(vcpu); + + if (svm_is_access_to_gpat(vcpu, host_initiated)) { + vmcb_set_gpat(svm->nested.vmcb02.ptr, pat); + return; + } + + svm->vcpu.arch.pat =3D pat; + + if (!npt_enabled) + return; + + vmcb_set_gpat(svm->vmcb01.ptr, pat); + + if (svm->nested.legacy_gpat_semantics) { + svm->nested.save.g_pat =3D pat; + vmcb_set_gpat(svm->nested.vmcb02.ptr, pat); + } else if (is_guest_mode(&svm->vcpu) && !nested_npt_enabled(svm)) { + vmcb_set_gpat(svm->nested.vmcb02.ptr, pat); + } +} + static u32 svm_get_interrupt_shadow(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm =3D to_svm(vcpu); @@ -2838,16 +2876,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct= msr_data *msr_info) msr_info->data =3D svm->msr_decfg; break; case MSR_IA32_CR_PAT: - /* - * 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 for backward and forward KVM_GET_MSRS compatibility - * with older kernels. - */ - WARN_ON_ONCE(msr_info->host_initiated && vcpu->wants_to_run); - if (!msr_info->host_initiated && is_guest_mode(vcpu) && - nested_npt_enabled(svm)) + if (svm_is_access_to_gpat(vcpu, msr_info->host_initiated)) msr_info->data =3D svm->nested.save.g_pat; else msr_info->data =3D vcpu->arch.pat; @@ -2937,19 +2966,8 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct= msr_data *msr) case MSR_IA32_CR_PAT: if (!kvm_pat_valid(data)) return 1; - /* - * 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 for backward and forward KVM_SET_MSRS compatibility - * with older kernels. - */ - WARN_ON_ONCE(msr->host_initiated && vcpu->wants_to_run); - if (!msr->host_initiated && is_guest_mode(vcpu) && - nested_npt_enabled(svm)) - svm_set_gpat(svm, data); - else - svm_set_hpat(svm, data); + + svm_set_pat(vcpu, data, msr->host_initiated); 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 0bb9fdcb489d..71502db3f679 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -616,24 +616,6 @@ static inline bool nested_npt_enabled(struct vcpu_svm = *svm) return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP; } =20 -static inline void svm_set_gpat(struct vcpu_svm *svm, u64 data) -{ - svm->nested.save.g_pat =3D data; - vmcb_set_gpat(svm->nested.vmcb02.ptr, data); -} - -static inline void svm_set_hpat(struct vcpu_svm *svm, u64 data) -{ - svm->vcpu.arch.pat =3D data; - if (npt_enabled) { - vmcb_set_gpat(svm->vmcb01.ptr, data); - if (is_guest_mode(&svm->vcpu) && !nested_npt_enabled(svm)) - vmcb_set_gpat(svm->nested.vmcb02.ptr, data); - } - if (svm->nested.legacy_gpat_semantics) - svm_set_gpat(svm, data); -} - static inline bool nested_vnmi_enabled(struct vcpu_svm *svm) { return guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_VNMI) && @@ -780,6 +762,7 @@ void svm_enable_lbrv(struct kvm_vcpu *vcpu); void svm_update_lbrv(struct kvm_vcpu *vcpu); =20 int svm_set_efer(struct kvm_vcpu *vcpu, u64 efer); +void svm_set_pat(struct kvm_vcpu *vcpu, u64 pat, bool host_initiated); void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0); void svm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4); void disable_nmi_singlestep(struct vcpu_svm *svm); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 967b58a8ab9d..546056e690eb 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -2141,6 +2141,9 @@ int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_dat= a *msr_info) #endif case MSR_EFER: return kvm_get_msr_common(vcpu, msr_info); + case MSR_IA32_CR_PAT: + msr_info->data =3D vcpu->arch.pat; + break; case MSR_IA32_TSX_CTRL: if (!msr_info->host_initiated && !(vcpu->arch.arch_capabilities & ARCH_CAP_TSX_CTRL_MSR)) @@ -2468,9 +2471,10 @@ int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_da= ta *msr_info) return 1; goto find_uret_msr; case MSR_IA32_CR_PAT: - ret =3D kvm_set_msr_common(vcpu, msr_info); - if (ret) - break; + if (!kvm_pat_valid(data)) + return 1; + + vcpu->arch.pat =3D data; =20 if (is_guest_mode(vcpu) && get_vmcs12(vcpu)->vm_exit_controls & VM_EXIT_SAVE_IA32_PAT) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 416899b5dbe4..41936f83a17f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4025,12 +4025,6 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct= msr_data *msr_info) return 1; } break; - case MSR_IA32_CR_PAT: - if (!kvm_pat_valid(data)) - return 1; - - vcpu->arch.pat =3D data; - break; case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000: case MSR_MTRRdefType: return kvm_mtrr_set_msr(vcpu, msr, data); @@ -4436,9 +4430,6 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct = msr_data *msr_info) msr_info->data =3D kvm_scale_tsc(rdtsc(), ratio) + offset; break; } - case MSR_IA32_CR_PAT: - msr_info->data =3D vcpu->arch.pat; - break; case MSR_MTRRcap: case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000: case MSR_MTRRdefType: base-commit: 7539434a6984ba5accfdd8e296fb834558f95df4 --