public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kai Huang <kai.huang@intel.com>,
	Wenyao Hai <haiwenyao@uniontech.com>,
	Ke Guo <guoke@uniontech.com>
Subject: [PATCH v2 6/8] KVM: x86: Move PAT MSR handling out of mtrr.c
Date: Thu, 11 May 2023 16:33:49 -0700	[thread overview]
Message-ID: <20230511233351.635053-7-seanjc@google.com> (raw)
In-Reply-To: <20230511233351.635053-1-seanjc@google.com>

Drop handling of MSR_IA32_CR_PAT from mtrr.c now that SVM and VMX handle
writes without bouncing through kvm_set_msr_common().  PAT isn't truly an
MTRR even though it affects memory types, and more importantly KVM enables
hardware virtualization of guest PAT (by NOT setting "ignore guest PAT")
when a guest has non-coherent DMA, i.e. KVM doesn't need to zap SPTEs when
the guest PAT changes.

The read path is and always has been trivial, i.e. burying it in the MTRR
code does more harm than good.

WARN and continue for the PAT case in kvm_set_msr_common(), as that code
is _currently_ reached if and only if KVM is buggy.  Defer cleaning up the
lack of symmetry between the read and write paths to a future patch.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mtrr.c | 19 ++++++-------------
 arch/x86/kvm/x86.c  | 13 +++++++++++++
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index dc213b940141..cdbbb511f940 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -55,7 +55,6 @@ static bool msr_mtrr_valid(unsigned msr)
 	case MSR_MTRRfix4K_F0000:
 	case MSR_MTRRfix4K_F8000:
 	case MSR_MTRRdefType:
-	case MSR_IA32_CR_PAT:
 		return true;
 	}
 	return false;
@@ -74,9 +73,7 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 	if (!msr_mtrr_valid(msr))
 		return false;
 
-	if (msr == MSR_IA32_CR_PAT) {
-		return kvm_pat_valid(data);
-	} else if (msr == MSR_MTRRdefType) {
+	if (msr == MSR_MTRRdefType) {
 		if (data & ~0xcff)
 			return false;
 		return valid_mtrr_type(data & 0xff);
@@ -324,8 +321,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
 	gfn_t start, end;
 
-	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
-	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
+	if (!tdp_enabled || !kvm_arch_has_noncoherent_dma(vcpu->kvm))
 		return;
 
 	if (!mtrr_is_enabled(mtrr_state) && msr != MSR_MTRRdefType)
@@ -392,8 +388,6 @@ int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		*(u64 *)&vcpu->arch.mtrr_state.fixed_ranges[index] = data;
 	else if (msr == MSR_MTRRdefType)
 		vcpu->arch.mtrr_state.deftype = data;
-	else if (msr == MSR_IA32_CR_PAT)
-		vcpu->arch.pat = data;
 	else
 		set_var_mtrr_msr(vcpu, msr, data);
 
@@ -421,13 +415,12 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 		return 1;
 
 	index = fixed_msr_to_range_index(msr);
-	if (index >= 0)
+	if (index >= 0) {
 		*pdata = *(u64 *)&vcpu->arch.mtrr_state.fixed_ranges[index];
-	else if (msr == MSR_MTRRdefType)
+	} else if (msr == MSR_MTRRdefType) {
 		*pdata = vcpu->arch.mtrr_state.deftype;
-	else if (msr == MSR_IA32_CR_PAT)
-		*pdata = vcpu->arch.pat;
-	else {	/* Variable MTRRs */
+	} else {
+		/* Variable MTRRs */
 		if (is_mtrr_base_msr(msr))
 			*pdata = var_mtrr_msr_to_range(vcpu, msr)->base;
 		else
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8b356c9d8a81..d71cf924cd8f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3701,6 +3701,17 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		}
 		break;
 	case MSR_IA32_CR_PAT:
+		/*
+		 * Writes to PAT should be handled by vendor code as both SVM
+		 * and VMX track the guest's PAT in the VMCB/VMCS.
+		 */
+		WARN_ON_ONCE(1);
+
+		if (!kvm_pat_valid(data))
+			return 1;
+
+		vcpu->arch.pat = data;
+		break;
 	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
 	case MSR_MTRRdefType:
 		return kvm_mtrr_set_msr(vcpu, msr, data);
@@ -4110,6 +4121,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	}
 	case MSR_IA32_CR_PAT:
+		msr_info->data = vcpu->arch.pat;
+		break;
 	case MSR_MTRRcap:
 	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
 	case MSR_MTRRdefType:
-- 
2.40.1.606.ga4b1b128d6-goog


  parent reply	other threads:[~2023-05-11 23:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-11 23:33 [PATCH v2 0/8] KVM: x86: Clean up MSR PAT handling Sean Christopherson
2023-05-11 23:33 ` [PATCH v2 1/8] KVM: VMX: Open code writing vCPU's PAT in VMX's MSR handler Sean Christopherson
2023-05-12 10:18   ` Huang, Kai
2023-05-11 23:33 ` [PATCH v2 2/8] KVM: SVM: Use kvm_pat_valid() directly instead of kvm_mtrr_valid() Sean Christopherson
2023-05-11 23:33 ` [PATCH v2 3/8] KVM: x86: Add helper to query if variable MTRR MSR is base (versus mask) Sean Christopherson
2023-05-11 23:33 ` [PATCH v2 4/8] KVM: x86: Add helper to get variable MTRR range from MSR index Sean Christopherson
2023-05-12 10:21   ` Huang, Kai
2023-05-11 23:33 ` [PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges Sean Christopherson
2023-05-12 10:35   ` Huang, Kai
2023-05-12 16:35     ` Sean Christopherson
2023-05-15  0:37       ` Huang, Kai
2023-05-15 17:49         ` Sean Christopherson
2023-05-15 22:21           ` Huang, Kai
2023-05-11 23:33 ` Sean Christopherson [this message]
2023-05-12 10:40   ` [PATCH v2 6/8] KVM: x86: Move PAT MSR handling out of mtrr.c Huang, Kai
2023-05-11 23:33 ` [PATCH v2 7/8] KVM: x86: Make kvm_mtrr_valid() static now that there are no external users Sean Christopherson
2023-05-12 10:46   ` Huang, Kai
2023-05-11 23:33 ` [PATCH v2 8/8] KVM: x86: Move common handling of PAT MSR writes to kvm_set_msr_common() Sean Christopherson
2023-05-12 10:48   ` Huang, Kai
2023-06-02  1:21 ` [PATCH v2 0/8] KVM: x86: Clean up MSR PAT handling Sean Christopherson

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=20230511233351.635053-7-seanjc@google.com \
    --to=seanjc@google.com \
    --cc=guoke@uniontech.com \
    --cc=haiwenyao@uniontech.com \
    --cc=kai.huang@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /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