[PATCH v2 0/8] KVM: x86: Clean up MSR PAT handling

[PATCH v2 0/8] KVM: x86: Clean up MSR PAT handling

[Sean Christopherson]

Clean up KVM's handling of MSR PAT.  The PAT is currently lumped in with
MTRRs, and while the PAT does affect memtypes, it's not an MTRR and is
exempted from KVM's kludgy attempts to play nice with UC memory for guests
with passthrough devices that have non-coherent DMA (KVM does NOT set
"ignore guest PAT" in this case, and so the guest PAT is virtualized by
hardware, not emulated by KVM).

v2:
 - Move common bits of write path to kvm_set_msr_common(). [Kai]
 - Reword changelogs to call out that KVM enables hardware virtualization
   of guest PAT when the guest has non-coherent DMA. [Kai]
 - Clean up more open coded references to MTRR MSRs. [Yan]
 - Squash away the standalone WARN patch. [Kai]

v1: https://lore.kernel.org/all/20230503182852.3431281-1-seanjc@google.com

Ke Guo (1):
  KVM: SVM: Use kvm_pat_valid() directly instead of kvm_mtrr_valid()

Sean Christopherson (6):
  KVM: x86: Add helper to query if variable MTRR MSR is base (versus
    mask)
  KVM: x86: Add helper to get variable MTRR range from MSR index
  KVM: x86: Use MTRR macros to define possible MTRR MSR ranges
  KVM: x86: Move PAT MSR handling out of mtrr.c
  KVM: x86: Make kvm_mtrr_valid() static now that there are no external
    users
  KVM: x86: Move common handling of PAT MSR writes to
    kvm_set_msr_common()

Wenyao Hai (1):
  KVM: VMX: Open code writing vCPU's PAT in VMX's MSR handler

 arch/x86/kvm/mtrr.c    | 64 ++++++++++++++++++++----------------------
 arch/x86/kvm/svm/svm.c |  7 +++--
 arch/x86/kvm/vmx/vmx.c | 11 +++-----
 arch/x86/kvm/x86.c     | 17 ++++++++---
 arch/x86/kvm/x86.h     |  1 -
 5 files changed, 52 insertions(+), 48 deletions(-)


base-commit: 5c291b93e5d665380dbecc6944973583f9565ee5
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 1/8] KVM: VMX: Open code writing vCPU's PAT in VMX's MSR handler

[Sean Christopherson]

From: Wenyao Hai <haiwenyao@uniontech.com>

Open code setting "vcpu->arch.pat" in vmx_set_msr() instead of bouncing
through kvm_set_msr_common() to get to the same code in kvm_mtrr_set_msr().
This aligns VMX with SVM, avoids hiding a very simple operation behind a
relatively complicated function call (finding the PAT MSR case in
kvm_set_msr_common() is non-trivial), and most importantly, makes it clear
that not unwinding the VMCS updates if kvm_set_msr_common() isn't a bug
(because kvm_set_msr_common() can never fail for PAT).

Opportunistically set vcpu->arch.pat before updating the VMCS info so that
a future patch can move the common bits (back) into kvm_set_msr_common()
without a functional change.

Note, MSR_IA32_CR_PAT is 0x277, and is very subtly handled by

	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:

in kvm_set_msr_common().

Cc: Kai Huang <kai.huang@intel.com>
Signed-off-by: Wenyao Hai <haiwenyao@uniontech.com>
[sean: massage changelog, hoist setting vcpu->arch.pat up]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 44fb619803b8..33b8625d3541 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2290,16 +2290,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		if (!kvm_pat_valid(data))
 			return 1;
 
+		vcpu->arch.pat = data;
+
 		if (is_guest_mode(vcpu) &&
 		    get_vmcs12(vcpu)->vm_exit_controls & VM_EXIT_SAVE_IA32_PAT)
 			get_vmcs12(vcpu)->guest_ia32_pat = data;
 
-		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
+		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT)
 			vmcs_write64(GUEST_IA32_PAT, data);
-			vcpu->arch.pat = data;
-			break;
-		}
-		ret = kvm_set_msr_common(vcpu, msr_info);
 		break;
 	case MSR_IA32_MCG_EXT_CTL:
 		if ((!msr_info->host_initiated &&
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 2/8] KVM: SVM: Use kvm_pat_valid() directly instead of kvm_mtrr_valid()

[Sean Christopherson]

From: Ke Guo <guoke@uniontech.com>

Use kvm_pat_valid() directly instead of bouncing through kvm_mtrr_valid().
The PAT is not an MTRR, and kvm_mtrr_valid() just redirects to
kvm_pat_valid(), i.e. is exempt from KVM's "zap SPTEs" logic that's
needed to honor guest MTRRs when the VM has a passthrough device with
non-coherent DMA (KVM does NOT set "ignore guest PAT" in this case, and so
enables hardware virtualization of the guest's PAT, i.e. doesn't need to
manually emulate the PAT memtype).

Signed-off-by: Ke Guo <guoke@uniontech.com>
[sean: massage changelog]
Reviewed-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index eb308c9994f9..db237ccdc957 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2935,7 +2935,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 
 		break;
 	case MSR_IA32_CR_PAT:
-		if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
+		if (!kvm_pat_valid(data))
 			return 1;
 		vcpu->arch.pat = data;
 		svm->vmcb01.ptr->save.g_pat = data;
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 3/8] KVM: x86: Add helper to query if variable MTRR MSR is base (versus mask)

[Sean Christopherson]

Add a helper to query whether a variable MTRR MSR is a base versus as mask
MSR.  Replace the unnecessarily complex math with a simple check on bit 0;
base MSRs are even, mask MSRs are odd.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mtrr.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index 9fac1ec03463..f65ce4b3980f 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -25,6 +25,12 @@
 #define IA32_MTRR_DEF_TYPE_FE		(1ULL << 10)
 #define IA32_MTRR_DEF_TYPE_TYPE_MASK	(0xff)
 
+static bool is_mtrr_base_msr(unsigned int msr)
+{
+	/* MTRR base MSRs use even numbers, masks use odd numbers. */
+	return !(msr & 0x1);
+}
+
 static bool msr_mtrr_valid(unsigned msr)
 {
 	switch (msr) {
@@ -342,10 +348,9 @@ static void set_var_mtrr_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
 	struct kvm_mtrr_range *tmp, *cur;
-	int index, is_mtrr_mask;
+	int index;
 
 	index = (msr - 0x200) / 2;
-	is_mtrr_mask = msr - 0x200 - 2 * index;
 	cur = &mtrr_state->var_ranges[index];
 
 	/* remove the entry if it's in the list. */
@@ -356,7 +361,7 @@ static void set_var_mtrr_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 	 * Set all illegal GPA bits in the mask, since those bits must
 	 * implicitly be 0.  The bits are then cleared when reading them.
 	 */
-	if (!is_mtrr_mask)
+	if (is_mtrr_base_msr(msr))
 		cur->base = data;
 	else
 		cur->mask = data | kvm_vcpu_reserved_gpa_bits_raw(vcpu);
@@ -418,11 +423,8 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	else if (msr == MSR_IA32_CR_PAT)
 		*pdata = vcpu->arch.pat;
 	else {	/* Variable MTRRs */
-		int is_mtrr_mask;
-
 		index = (msr - 0x200) / 2;
-		is_mtrr_mask = msr - 0x200 - 2 * index;
-		if (!is_mtrr_mask)
+		if (is_mtrr_base_msr(msr))
 			*pdata = vcpu->arch.mtrr_state.var_ranges[index].base;
 		else
 			*pdata = vcpu->arch.mtrr_state.var_ranges[index].mask;
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 4/8] KVM: x86: Add helper to get variable MTRR range from MSR index

[Sean Christopherson]

Add a helper to dedup the logic for retrieving a variable MTRR range
structure given a variable MTRR MSR index.

No functional change intended.

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

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index f65ce4b3980f..59851dbebfea 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -31,6 +31,14 @@ static bool is_mtrr_base_msr(unsigned int msr)
 	return !(msr & 0x1);
 }
 
+static struct kvm_mtrr_range *var_mtrr_msr_to_range(struct kvm_vcpu *vcpu,
+						    unsigned int msr)
+{
+	int index = (msr - 0x200) / 2;
+
+	return &vcpu->arch.mtrr_state.var_ranges[index];
+}
+
 static bool msr_mtrr_valid(unsigned msr)
 {
 	switch (msr) {
@@ -314,7 +322,6 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
 	gfn_t start, end;
-	int index;
 
 	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
 	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
@@ -332,8 +339,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 		end = ~0ULL;
 	} else {
 		/* variable range MTRRs. */
-		index = (msr - 0x200) / 2;
-		var_mtrr_range(&mtrr_state->var_ranges[index], &start, &end);
+		var_mtrr_range(var_mtrr_msr_to_range(vcpu, msr), &start, &end);
 	}
 
 	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
@@ -348,14 +354,12 @@ static void set_var_mtrr_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
 	struct kvm_mtrr_range *tmp, *cur;
-	int index;
 
-	index = (msr - 0x200) / 2;
-	cur = &mtrr_state->var_ranges[index];
+	cur = var_mtrr_msr_to_range(vcpu, msr);
 
 	/* remove the entry if it's in the list. */
 	if (var_mtrr_range_is_valid(cur))
-		list_del(&mtrr_state->var_ranges[index].node);
+		list_del(&cur->node);
 
 	/*
 	 * Set all illegal GPA bits in the mask, since those bits must
@@ -423,11 +427,10 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	else if (msr == MSR_IA32_CR_PAT)
 		*pdata = vcpu->arch.pat;
 	else {	/* Variable MTRRs */
-		index = (msr - 0x200) / 2;
 		if (is_mtrr_base_msr(msr))
-			*pdata = vcpu->arch.mtrr_state.var_ranges[index].base;
+			*pdata = var_mtrr_msr_to_range(vcpu, msr)->base;
 		else
-			*pdata = vcpu->arch.mtrr_state.var_ranges[index].mask;
+			*pdata = var_mtrr_msr_to_range(vcpu, msr)->mask;
 
 		*pdata &= ~kvm_vcpu_reserved_gpa_bits_raw(vcpu);
 	}
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Sean Christopherson]

Use the MTRR macros to identify the ranges of possible MTRR MSRs instead
of bounding the ranges with a mismash of open coded values and unrelated
MSR indices.  Carving out the gap for the machine check MSRs in particular
is confusing, as it's easy to incorrectly think the case statement handles
MCE MSRs instead of skipping them.

Drop the range-based funneling of MSRs between the end of the MCE MSRs
and MTRR_DEF_TYPE, i.e. 0x2A0-0x2FF, and instead handle MTTR_DEF_TYPE as
the one-off case that it is.

Extract PAT (0x277) as well in anticipation of dropping PAT "handling"
from the MTRR code.

Keep the range-based handling for the variable+fixed MTRRs even though
capturing unknown MSRs 0x214-0x24F is arguably "wrong".  There is a gap in
the fixed MTRRs, 0x260-0x267, i.e. the MTRR code needs to filter out
unknown MSRs anyways, and using a single range generates marginally better
code for the big switch statement.

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

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index 59851dbebfea..dc213b940141 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -34,7 +34,7 @@ static bool is_mtrr_base_msr(unsigned int msr)
 static struct kvm_mtrr_range *var_mtrr_msr_to_range(struct kvm_vcpu *vcpu,
 						    unsigned int msr)
 {
-	int index = (msr - 0x200) / 2;
+	int index = (msr - MTRRphysBase_MSR(0)) / 2;
 
 	return &vcpu->arch.mtrr_state.var_ranges[index];
 }
@@ -42,7 +42,7 @@ static struct kvm_mtrr_range *var_mtrr_msr_to_range(struct kvm_vcpu *vcpu,
 static bool msr_mtrr_valid(unsigned msr)
 {
 	switch (msr) {
-	case 0x200 ... 0x200 + 2 * KVM_NR_VAR_MTRR - 1:
+	case MTRRphysBase_MSR(0) ... MTRRphysMask_MSR(KVM_NR_VAR_MTRR - 1):
 	case MSR_MTRRfix64K_00000:
 	case MSR_MTRRfix16K_80000:
 	case MSR_MTRRfix16K_A0000:
@@ -88,7 +88,8 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 	}
 
 	/* variable MTRRs */
-	WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
+	WARN_ON(!(msr >= MTRRphysBase_MSR(0) &&
+		  msr <= MTRRphysMask_MSR(KVM_NR_VAR_MTRR - 1)));
 
 	mask = kvm_vcpu_reserved_gpa_bits_raw(vcpu);
 	if ((msr & 1) == 0) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e7f78fe79b32..8b356c9d8a81 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3700,8 +3700,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			return 1;
 		}
 		break;
-	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
-	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
+	case MSR_IA32_CR_PAT:
+	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
+	case MSR_MTRRdefType:
 		return kvm_mtrr_set_msr(vcpu, msr, data);
 	case MSR_IA32_APICBASE:
 		return kvm_set_apic_base(vcpu, msr_info);
@@ -4108,9 +4109,10 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = kvm_scale_tsc(rdtsc(), ratio) + offset;
 		break;
 	}
+	case MSR_IA32_CR_PAT:
 	case MSR_MTRRcap:
-	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
-	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
+	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
+	case MSR_MTRRdefType:
 		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
 	case 0xcd: /* fsb frequency */
 		msr_info->data = 3;
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 6/8] KVM: x86: Move PAT MSR handling out of mtrr.c

[Sean Christopherson]

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

[PATCH v2 7/8] KVM: x86: Make kvm_mtrr_valid() static now that there are no external users

[Sean Christopherson]

Make kvm_mtrr_valid() local to mtrr.c now that it's not used to check the
validity of a PAT MSR value.

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

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index cdbbb511f940..3eb6e7f47e96 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -65,7 +65,7 @@ static bool valid_mtrr_type(unsigned t)
 	return t < 8 && (1 << t) & 0x73; /* 0, 1, 4, 5, 6 */
 }
 
-bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+static bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	int i;
 	u64 mask;
@@ -100,7 +100,6 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 
 	return (data & mask) == 0;
 }
-EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
 
 static bool mtrr_is_enabled(struct kvm_mtrr *mtrr_state)
 {
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index c544602d07a3..82e3dafc5453 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -309,7 +309,6 @@ void kvm_deliver_exception_payload(struct kvm_vcpu *vcpu,
 
 void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu);
 u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn);
-bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data);
 int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data);
 int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
 bool kvm_mtrr_check_gfn_range_consistency(struct kvm_vcpu *vcpu, gfn_t gfn,
-- 
2.40.1.606.ga4b1b128d6-goog

[PATCH v2 8/8] KVM: x86: Move common handling of PAT MSR writes to kvm_set_msr_common()

[Sean Christopherson]

Move the common check-and-set handling of PAT MSR writes out of vendor
code and into kvm_set_msr_common().  This aligns writes with reads, which
are already handled in common code, i.e. makes the handling of reads and
writes symmetrical in common code.

Alternatively, the common handling in kvm_get_msr_common() could be moved
to vendor code, but duplicating code is generally undesirable (even though
the duplicatated code is trivial in this case), and guest writes to PAT
should be rare, i.e. the overhead of the extra function call is a
non-issue in practice.

Suggested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 7 ++++---
 arch/x86/kvm/vmx/vmx.c | 7 +++----
 arch/x86/kvm/x86.c     | 6 ------
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index db237ccdc957..61d329760f6c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2935,9 +2935,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 
 		break;
 	case MSR_IA32_CR_PAT:
-		if (!kvm_pat_valid(data))
-			return 1;
-		vcpu->arch.pat = data;
+		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);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 33b8625d3541..2d9d155691a7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2287,10 +2287,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			return 1;
 		goto find_uret_msr;
 	case MSR_IA32_CR_PAT:
-		if (!kvm_pat_valid(data))
-			return 1;
-
-		vcpu->arch.pat = data;
+		ret = kvm_set_msr_common(vcpu, msr_info);
+		if (ret)
+			break;
 
 		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 d71cf924cd8f..3759737c0873 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3701,12 +3701,6 @@ 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;
 
-- 
2.40.1.606.ga4b1b128d6-goog

Re: [PATCH v2 1/8] KVM: VMX: Open code writing vCPU's PAT in VMX's MSR handler

[Huang, Kai]

On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> From: Wenyao Hai <haiwenyao@uniontech.com>
> 
> Open code setting "vcpu->arch.pat" in vmx_set_msr() instead of bouncing
> through kvm_set_msr_common() to get to the same code in kvm_mtrr_set_msr().
> This aligns VMX with SVM, avoids hiding a very simple operation behind a
> relatively complicated function call (finding the PAT MSR case in
> kvm_set_msr_common() is non-trivial), and most importantly, makes it clear
> that not unwinding the VMCS updates if kvm_set_msr_common() isn't a bug
> (because kvm_set_msr_common() can never fail for PAT).
> 
> Opportunistically set vcpu->arch.pat before updating the VMCS info so that
> a future patch can move the common bits (back) into kvm_set_msr_common()
> without a functional change.
> 
> Note, MSR_IA32_CR_PAT is 0x277, and is very subtly handled by
> 
> 	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> 
> in kvm_set_msr_common().
> 
> Cc: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Wenyao Hai <haiwenyao@uniontech.com>
> [sean: massage changelog, hoist setting vcpu->arch.pat up]
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kvm/vmx/vmx.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 44fb619803b8..33b8625d3541 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2290,16 +2290,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		if (!kvm_pat_valid(data))
>  			return 1;
>  
> +		vcpu->arch.pat = data;
> +
>  		if (is_guest_mode(vcpu) &&
>  		    get_vmcs12(vcpu)->vm_exit_controls & VM_EXIT_SAVE_IA32_PAT)
>  			get_vmcs12(vcpu)->guest_ia32_pat = data;
>  
> -		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
> +		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT)
>  			vmcs_write64(GUEST_IA32_PAT, data);
> -			vcpu->arch.pat = data;
> -			break;
> -		}
> -		ret = kvm_set_msr_common(vcpu, msr_info);
>  		break;
>  	case MSR_IA32_MCG_EXT_CTL:
>  		if ((!msr_info->host_initiated &&
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v2 4/8] KVM: x86: Add helper to get variable MTRR range from MSR index

[Huang, Kai]

On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> Add a helper to dedup the logic for retrieving a variable MTRR range
> structure given a variable MTRR MSR index.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kvm/mtrr.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
> index f65ce4b3980f..59851dbebfea 100644
> --- a/arch/x86/kvm/mtrr.c
> +++ b/arch/x86/kvm/mtrr.c
> @@ -31,6 +31,14 @@ static bool is_mtrr_base_msr(unsigned int msr)
>  	return !(msr & 0x1);
>  }
>  
> +static struct kvm_mtrr_range *var_mtrr_msr_to_range(struct kvm_vcpu *vcpu,
> +						    unsigned int msr)
> +{
> +	int index = (msr - 0x200) / 2;
> +
> +	return &vcpu->arch.mtrr_state.var_ranges[index];
> +}
> +
>  static bool msr_mtrr_valid(unsigned msr)
>  {
>  	switch (msr) {
> @@ -314,7 +322,6 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>  {
>  	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
>  	gfn_t start, end;
> -	int index;
>  
>  	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
>  	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
> @@ -332,8 +339,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>  		end = ~0ULL;
>  	} else {
>  		/* variable range MTRRs. */
> -		index = (msr - 0x200) / 2;
> -		var_mtrr_range(&mtrr_state->var_ranges[index], &start, &end);
> +		var_mtrr_range(var_mtrr_msr_to_range(vcpu, msr), &start, &end);
>  	}
>  
>  	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
> @@ -348,14 +354,12 @@ static void set_var_mtrr_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  {
>  	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
>  	struct kvm_mtrr_range *tmp, *cur;
> -	int index;
>  
> -	index = (msr - 0x200) / 2;
> -	cur = &mtrr_state->var_ranges[index];
> +	cur = var_mtrr_msr_to_range(vcpu, msr);
>  
>  	/* remove the entry if it's in the list. */
>  	if (var_mtrr_range_is_valid(cur))
> -		list_del(&mtrr_state->var_ranges[index].node);
> +		list_del(&cur->node);
>  
>  	/*
>  	 * Set all illegal GPA bits in the mask, since those bits must
> @@ -423,11 +427,10 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>  	else if (msr == MSR_IA32_CR_PAT)
>  		*pdata = vcpu->arch.pat;
>  	else {	/* Variable MTRRs */
> -		index = (msr - 0x200) / 2;
>  		if (is_mtrr_base_msr(msr))
> -			*pdata = vcpu->arch.mtrr_state.var_ranges[index].base;
> +			*pdata = var_mtrr_msr_to_range(vcpu, msr)->base;
>  		else
> -			*pdata = vcpu->arch.mtrr_state.var_ranges[index].mask;
> +			*pdata = var_mtrr_msr_to_range(vcpu, msr)->mask;
>  
>  		*pdata &= ~kvm_vcpu_reserved_gpa_bits_raw(vcpu);
>  	}
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Huang, Kai]

On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> Use the MTRR macros to identify the ranges of possible MTRR MSRs instead
> of bounding the ranges with a mismash of open coded values and unrelated
				^
				mishmash?

> MSR indices.  Carving out the gap for the machine check MSRs in particular
> is confusing, as it's easy to incorrectly think the case statement handles
> MCE MSRs instead of skipping them.
> 
> Drop the range-based funneling of MSRs between the end of the MCE MSRs
> and MTRR_DEF_TYPE, i.e. 0x2A0-0x2FF, and instead handle MTTR_DEF_TYPE as
> the one-off case that it is.
> 
> Extract PAT (0x277) as well in anticipation of dropping PAT "handling"
> from the MTRR code.
> 
> Keep the range-based handling for the variable+fixed MTRRs even though
> capturing unknown MSRs 0x214-0x24F is arguably "wrong".  There is a gap in
> the fixed MTRRs, 0x260-0x267, i.e. the MTRR code needs to filter out
> unknown MSRs anyways, and using a single range generates marginally better
> code for the big switch statement.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

One Nit below ...

> ---
>  arch/x86/kvm/mtrr.c |  7 ++++---
>  arch/x86/kvm/x86.c  | 10 ++++++----
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
> index 59851dbebfea..dc213b940141 100644
> --- a/arch/x86/kvm/mtrr.c
> +++ b/arch/x86/kvm/mtrr.c
> @@ -34,7 +34,7 @@ static bool is_mtrr_base_msr(unsigned int msr)
>  static struct kvm_mtrr_range *var_mtrr_msr_to_range(struct kvm_vcpu *vcpu,
>  						    unsigned int msr)
>  {
> -	int index = (msr - 0x200) / 2;
> +	int index = (msr - MTRRphysBase_MSR(0)) / 2;
>  
>  	return &vcpu->arch.mtrr_state.var_ranges[index];
>  }
> @@ -42,7 +42,7 @@ static struct kvm_mtrr_range *var_mtrr_msr_to_range(struct kvm_vcpu *vcpu,
>  static bool msr_mtrr_valid(unsigned msr)
>  {
>  	switch (msr) {
> -	case 0x200 ... 0x200 + 2 * KVM_NR_VAR_MTRR - 1:
> +	case MTRRphysBase_MSR(0) ... MTRRphysMask_MSR(KVM_NR_VAR_MTRR - 1):
>  	case MSR_MTRRfix64K_00000:
>  	case MSR_MTRRfix16K_80000:
>  	case MSR_MTRRfix16K_A0000:
> @@ -88,7 +88,8 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  	}
>  
>  	/* variable MTRRs */
> -	WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
> +	WARN_ON(!(msr >= MTRRphysBase_MSR(0) &&
> +		  msr <= MTRRphysMask_MSR(KVM_NR_VAR_MTRR - 1)));
>  
>  	mask = kvm_vcpu_reserved_gpa_bits_raw(vcpu);
>  	if ((msr & 1) == 0) {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e7f78fe79b32..8b356c9d8a81 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3700,8 +3700,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  			return 1;
>  		}
>  		break;
> -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> +	case MSR_IA32_CR_PAT:
> +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> +	case MSR_MTRRdefType:
>  		return kvm_mtrr_set_msr(vcpu, msr, data);
>  	case MSR_IA32_APICBASE:
>  		return kvm_set_apic_base(vcpu, msr_info);
> @@ -4108,9 +4109,10 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		msr_info->data = kvm_scale_tsc(rdtsc(), ratio) + offset;
>  		break;
>  	}
> +	case MSR_IA32_CR_PAT:
>  	case MSR_MTRRcap:

... Should we put MSR_IA32_CR_PAT after MSR_MTRRcap so it can be symmetric to
kvm_set_msr_common()?

Looks there's no reason to put it before MSR_MTRRcap.

> -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> +	case MSR_MTRRdefType:
>  		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
>  	case 0xcd: /* fsb frequency */
>  		msr_info->data = 3;
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v2 6/8] KVM: x86: Move PAT MSR handling out of mtrr.c

[Huang, Kai]

On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> 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>

Reviewed-by: Kai Huang <kai.huang@intel.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
> 

Re: [PATCH v2 7/8] KVM: x86: Make kvm_mtrr_valid() static now that there are no external users

[Huang, Kai]

On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> Make kvm_mtrr_valid() local to mtrr.c now that it's not used to check the
> validity of a PAT MSR value.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

Nit: looks this patch can be moved to right after patch 2 (KVM: SVM: Use
kvm_pat_valid() directly instead of kvm_mtrr_valid()), as after that one it
seems there is no other caller outside of mtrr.c.

> ---
>  arch/x86/kvm/mtrr.c | 3 +--
>  arch/x86/kvm/x86.h  | 1 -
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
> index cdbbb511f940..3eb6e7f47e96 100644
> --- a/arch/x86/kvm/mtrr.c
> +++ b/arch/x86/kvm/mtrr.c
> @@ -65,7 +65,7 @@ static bool valid_mtrr_type(unsigned t)
>  	return t < 8 && (1 << t) & 0x73; /* 0, 1, 4, 5, 6 */
>  }
>  
> -bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +static bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  {
>  	int i;
>  	u64 mask;
> @@ -100,7 +100,6 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  
>  	return (data & mask) == 0;
>  }
> -EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
>  
>  static bool mtrr_is_enabled(struct kvm_mtrr *mtrr_state)
>  {
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index c544602d07a3..82e3dafc5453 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -309,7 +309,6 @@ void kvm_deliver_exception_payload(struct kvm_vcpu *vcpu,
>  
>  void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu);
>  u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn);
> -bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data);
>  int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data);
>  int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
>  bool kvm_mtrr_check_gfn_range_consistency(struct kvm_vcpu *vcpu, gfn_t gfn,
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v2 8/8] KVM: x86: Move common handling of PAT MSR writes to kvm_set_msr_common()

[Huang, Kai]

On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> Move the common check-and-set handling of PAT MSR writes out of vendor
> code and into kvm_set_msr_common().  This aligns writes with reads, which
> are already handled in common code, i.e. makes the handling of reads and
> writes symmetrical in common code.
> 
> Alternatively, the common handling in kvm_get_msr_common() could be moved
> to vendor code, but duplicating code is generally undesirable (even though
> the duplicatated code is trivial in this case), and guest writes to PAT
> should be rare, i.e. the overhead of the extra function call is a
> non-issue in practice.
> 
> Suggested-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/kvm/svm/svm.c | 7 ++++---
>  arch/x86/kvm/vmx/vmx.c | 7 +++----
>  arch/x86/kvm/x86.c     | 6 ------
>  3 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index db237ccdc957..61d329760f6c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2935,9 +2935,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  
>  		break;
>  	case MSR_IA32_CR_PAT:
> -		if (!kvm_pat_valid(data))
> -			return 1;
> -		vcpu->arch.pat = data;
> +		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);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 33b8625d3541..2d9d155691a7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2287,10 +2287,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  			return 1;
>  		goto find_uret_msr;
>  	case MSR_IA32_CR_PAT:
> -		if (!kvm_pat_valid(data))
> -			return 1;
> -
> -		vcpu->arch.pat = data;
> +		ret = kvm_set_msr_common(vcpu, msr_info);
> +		if (ret)
> +			break;
>  
>  		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 d71cf924cd8f..3759737c0873 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3701,12 +3701,6 @@ 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;
>  
> -- 
> 2.40.1.606.ga4b1b128d6-goog
> 

Re: [PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Sean Christopherson]

On Fri, May 12, 2023, Kai Huang wrote:
> On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index e7f78fe79b32..8b356c9d8a81 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -3700,8 +3700,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  			return 1;
> >  		}
> >  		break;
> > -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> > -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> > +	case MSR_IA32_CR_PAT:
> > +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> > +	case MSR_MTRRdefType:
> >  		return kvm_mtrr_set_msr(vcpu, msr, data);
> >  	case MSR_IA32_APICBASE:
> >  		return kvm_set_apic_base(vcpu, msr_info);
> > @@ -4108,9 +4109,10 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		msr_info->data = kvm_scale_tsc(rdtsc(), ratio) + offset;
> >  		break;
> >  	}
> > +	case MSR_IA32_CR_PAT:
> >  	case MSR_MTRRcap:
> 
> ... Should we put MSR_IA32_CR_PAT after MSR_MTRRcap so it can be symmetric to
> kvm_set_msr_common()?
> 
> Looks there's no reason to put it before MSR_MTRRcap.

No, it's above MTRRcap for two reasons:

 1. When PAT is moved out of mtrr.c, PAT doesn't get bunded with the other MTRRs
    and so would just need to be hoisted back up.  The end code looks like:

	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:
		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
 
 2. There is no MSR_MTRRcap case statement in kvm_set_msr_common() because it's
    a read-only MSR, i.e. the two can't be symmetric :-)

> > -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> > -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> > +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> > +	case MSR_MTRRdefType:
> >  		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
> >  	case 0xcd: /* fsb frequency */
> >  		msr_info->data = 3;
> > -- 
> > 2.40.1.606.ga4b1b128d6-goog
> > 
> 

Re: [PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Huang, Kai]

On Fri, 2023-05-12 at 09:35 -0700, Sean Christopherson wrote:
> On Fri, May 12, 2023, Kai Huang wrote:
> > On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index e7f78fe79b32..8b356c9d8a81 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -3700,8 +3700,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > >  			return 1;
> > >  		}
> > >  		break;
> > > -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> > > -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> > > +	case MSR_IA32_CR_PAT:
> > > +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> > > +	case MSR_MTRRdefType:
> > >  		return kvm_mtrr_set_msr(vcpu, msr, data);
> > >  	case MSR_IA32_APICBASE:
> > >  		return kvm_set_apic_base(vcpu, msr_info);
> > > @@ -4108,9 +4109,10 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > >  		msr_info->data = kvm_scale_tsc(rdtsc(), ratio) + offset;
> > >  		break;
> > >  	}
> > > +	case MSR_IA32_CR_PAT:
> > >  	case MSR_MTRRcap:
> > 
> > ... Should we put MSR_IA32_CR_PAT after MSR_MTRRcap so it can be symmetric to
> > kvm_set_msr_common()?
> > 
> > Looks there's no reason to put it before MSR_MTRRcap.
> 
> No, it's above MTRRcap for two reasons:
> 
>  1. When PAT is moved out of mtrr.c, PAT doesn't get bunded with the other MTRRs
>     and so would just need to be hoisted back up.  The end code looks like:
> 
> 	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:
> 		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);

Sorry I mistakenly thought MSR_MTRRcap wasn't handled in kvm_mtrr_get_msr(). 
Yes looks good to me.

>  
>  2. There is no MSR_MTRRcap case statement in kvm_set_msr_common() because it's
>     a read-only MSR, i.e. the two can't be symmetric :-)

Do you know why it is a read-only MSR, which enables both FIXED ranges and
(fixed number of) dynamic ranges?

I am asking because there's a x86 series to fake a simple synthetic MTRR which
neither has fixed nor dynamic ranges but only has a default MSR_MTRRdefType:

https://lore.kernel.org/lkml/20230509233641.GGZFrZCTDH7VwUMp5R@fat_crate.local/T/

The main use cases are Xen PV guests and SEV-SNP guests running under
Hyper-V, but it appears TDX guest is also desired to have similar handling,
because: 

1) TDX module exposes MTRR in CPUID to guest, but handles nothing about MTRR
MSRs but only injects #VE.

2) TDX module always maps guest private memory as WB (and ignores guest's PAT
IIUC);

3) For shared memory, w/o non-coherent DMA guest's MTRRs are ignored by KVM
anyway.  TDX doesn't officially support non-trusted device assignment AFAICT.
Even we want to consider non-coherent DMA, it would only add confusion to honor
guest's MTRRs since they can point to private memory, which is always mapped as
WB.

So basically looks there's no value to exposing FIXED and dynamic MTRR ranges to
TDX guest.

Re: [PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Sean Christopherson]

On Mon, May 15, 2023, Kai Huang wrote:
> On Fri, 2023-05-12 at 09:35 -0700, Sean Christopherson wrote:
> > On Fri, May 12, 2023, Kai Huang wrote:
> > > On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index e7f78fe79b32..8b356c9d8a81 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -3700,8 +3700,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > > >  			return 1;
> > > >  		}
> > > >  		break;
> > > > -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> > > > -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> > > > +	case MSR_IA32_CR_PAT:
> > > > +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> > > > +	case MSR_MTRRdefType:
> > > >  		return kvm_mtrr_set_msr(vcpu, msr, data);
> > > >  	case MSR_IA32_APICBASE:
> > > >  		return kvm_set_apic_base(vcpu, msr_info);
> > > > @@ -4108,9 +4109,10 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > > >  		msr_info->data = kvm_scale_tsc(rdtsc(), ratio) + offset;
> > > >  		break;
> > > >  	}
> > > > +	case MSR_IA32_CR_PAT:
> > > >  	case MSR_MTRRcap:
> > > 
> > > ... Should we put MSR_IA32_CR_PAT after MSR_MTRRcap so it can be symmetric to
> > > kvm_set_msr_common()?
> > > 
> > > Looks there's no reason to put it before MSR_MTRRcap.
> > 
> > No, it's above MTRRcap for two reasons:
> > 
> >  1. When PAT is moved out of mtrr.c, PAT doesn't get bunded with the other MTRRs
> >     and so would just need to be hoisted back up.  The end code looks like:
> > 
> > 	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:
> > 		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
> 
> Sorry I mistakenly thought MSR_MTRRcap wasn't handled in kvm_mtrr_get_msr(). 
> Yes looks good to me.
> 
> >  
> >  2. There is no MSR_MTRRcap case statement in kvm_set_msr_common() because it's
> >     a read-only MSR, i.e. the two can't be symmetric :-)
> 
> Do you know why it is a read-only MSR, which enables both FIXED ranges and
> (fixed number of) dynamic ranges?

MTTRcap doesn't "enable" anything, it's a capabilities MSR (MTRR Capability is
its given name in the SDM), similar to ARCH_CAPABILITIES, PERF_CAPABILITIES, etc.
They're all essentially CPUID leafs, but presumably are MSRs due to being relevant
only to CPL0.  Or maybe some higher ups at Intel just spin a wheel to determine
whether to use a CPUID leaf or an MSR.  :-)

> I am asking because there's a x86 series to fake a simple synthetic MTRR which
> neither has fixed nor dynamic ranges but only has a default MSR_MTRRdefType:
> 
> https://lore.kernel.org/lkml/20230509233641.GGZFrZCTDH7VwUMp5R@fat_crate.local/T/
> 
> The main use cases are Xen PV guests and SEV-SNP guests running under
> Hyper-V, but it appears TDX guest is also desired to have similar handling,
> because:�
> 
> 1) TDX module exposes MTRR in CPUID to guest, but handles nothing about MTRR
> MSRs but only injects #VE.
> 
> 2) TDX module always maps guest private memory as WB (and ignores guest's PAT
> IIUC);
> 
> 3) For shared memory, w/o non-coherent DMA guest's MTRRs are ignored by KVM
> anyway.  TDX doesn't officially support non-trusted device assignment AFAICT.
> Even we want to consider non-coherent DMA, it would only add confusion to honor
> guest's MTRRs since they can point to private memory, which is always mapped as
> WB.

Yeah, I think the best option is for KVM to disallow attaching non-coherent DMA
to TDX VMs.  AFAIK, there's no use case for such a setup.

> So basically looks there's no value to exposing FIXED and dynamic MTRR ranges to
> TDX guest.

Agreed.

Re: [PATCH v2 5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Huang, Kai]

On Mon, 2023-05-15 at 10:49 -0700, Sean Christopherson wrote:
> On Mon, May 15, 2023, Kai Huang wrote:
> > On Fri, 2023-05-12 at 09:35 -0700, Sean Christopherson wrote:
> > > On Fri, May 12, 2023, Kai Huang wrote:
> > > > On Thu, 2023-05-11 at 16:33 -0700, Sean Christopherson wrote:
> > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > > index e7f78fe79b32..8b356c9d8a81 100644
> > > > > --- a/arch/x86/kvm/x86.c
> > > > > +++ b/arch/x86/kvm/x86.c
> > > > > @@ -3700,8 +3700,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > > > >  			return 1;
> > > > >  		}
> > > > >  		break;
> > > > > -	case 0x200 ... MSR_IA32_MC0_CTL2 - 1:
> > > > > -	case MSR_IA32_MCx_CTL2(KVM_MAX_MCE_BANKS) ... 0x2ff:
> > > > > +	case MSR_IA32_CR_PAT:
> > > > > +	case MTRRphysBase_MSR(0) ... MSR_MTRRfix4K_F8000:
> > > > > +	case MSR_MTRRdefType:
> > > > >  		return kvm_mtrr_set_msr(vcpu, msr, data);
> > > > >  	case MSR_IA32_APICBASE:
> > > > >  		return kvm_set_apic_base(vcpu, msr_info);
> > > > > @@ -4108,9 +4109,10 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > > > >  		msr_info->data = kvm_scale_tsc(rdtsc(), ratio) + offset;
> > > > >  		break;
> > > > >  	}
> > > > > +	case MSR_IA32_CR_PAT:
> > > > >  	case MSR_MTRRcap:
> > > > 
> > > > ... Should we put MSR_IA32_CR_PAT after MSR_MTRRcap so it can be symmetric to
> > > > kvm_set_msr_common()?
> > > > 
> > > > Looks there's no reason to put it before MSR_MTRRcap.
> > > 
> > > No, it's above MTRRcap for two reasons:
> > > 
> > >  1. When PAT is moved out of mtrr.c, PAT doesn't get bunded with the other MTRRs
> > >     and so would just need to be hoisted back up.  The end code looks like:
> > > 
> > > 	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:
> > > 		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
> > 
> > Sorry I mistakenly thought MSR_MTRRcap wasn't handled in kvm_mtrr_get_msr(). 
> > Yes looks good to me.
> > 
> > >  
> > >  2. There is no MSR_MTRRcap case statement in kvm_set_msr_common() because it's
> > >     a read-only MSR, i.e. the two can't be symmetric :-)
> > 
> > Do you know why it is a read-only MSR, which enables both FIXED ranges and
> > (fixed number of) dynamic ranges?
> 
> MTTRcap doesn't "enable" anything, it's a capabilities MSR (MTRR Capability is
> its given name in the SDM), similar to ARCH_CAPABILITIES, PERF_CAPABILITIES, etc.
> They're all essentially CPUID leafs, but presumably are MSRs due to being relevant
> only to CPL0.  Or maybe some higher ups at Intel just spin a wheel to determine
> whether to use a CPUID leaf or an MSR.  :-)

I meant it may make sense to allow Qemu to configure it.  Anyway thanks!

> 
> > I am asking because there's a x86 series to fake a simple synthetic MTRR which
> > neither has fixed nor dynamic ranges but only has a default MSR_MTRRdefType:
> > 
> > https://lore.kernel.org/lkml/20230509233641.GGZFrZCTDH7VwUMp5R@fat_crate.local/T/
> > 
> > The main use cases are Xen PV guests and SEV-SNP guests running under
> > Hyper-V, but it appears TDX guest is also desired to have similar handling,
> > because:�
> > 
> > 1) TDX module exposes MTRR in CPUID to guest, but handles nothing about MTRR
> > MSRs but only injects #VE.
> > 
> > 2) TDX module always maps guest private memory as WB (and ignores guest's PAT
> > IIUC);
> > 
> > 3) For shared memory, w/o non-coherent DMA guest's MTRRs are ignored by KVM
> > anyway.  TDX doesn't officially support non-trusted device assignment AFAICT.
> > Even we want to consider non-coherent DMA, it would only add confusion to honor
> > guest's MTRRs since they can point to private memory, which is always mapped as
> > WB.
> 
> Yeah, I think the best option is for KVM to disallow attaching non-coherent DMA
> to TDX VMs.  AFAIK, there's no use case for such a setup.

+Isaku,

Presumably the only case is assigning GPU to TDX VMs, but again not sure we need
to consider this as AFAICT TDX *officially* doesn't support untrusted device
passthrough.  Isaku may have more information.

And Iskau,

Do you have any comments here, especially considering TDX 2.0 support for TEE-
IO?  We probably need a solution that is future extensible.

> 
> > So basically looks there's no value to exposing FIXED and dynamic MTRR ranges to
> > TDX guest.
> 
> Agreed.

Re: [PATCH v2 0/8] KVM: x86: Clean up MSR PAT handling

[Sean Christopherson]

On Thu, 11 May 2023 16:33:43 -0700, Sean Christopherson wrote:
> Clean up KVM's handling of MSR PAT.  The PAT is currently lumped in with
> MTRRs, and while the PAT does affect memtypes, it's not an MTRR and is
> exempted from KVM's kludgy attempts to play nice with UC memory for guests
> with passthrough devices that have non-coherent DMA (KVM does NOT set
> "ignore guest PAT" in this case, and so the guest PAT is virtualized by
> hardware, not emulated by KVM).
> 
> [...]

Applied to kvm-x86 misc, thanks!

[1/8] KVM: VMX: Open code writing vCPU's PAT in VMX's MSR handler
      https://github.com/kvm-x86/linux/commit/a33ba1bf0dc6
[2/8] KVM: SVM: Use kvm_pat_valid() directly instead of kvm_mtrr_valid()
      https://github.com/kvm-x86/linux/commit/7aeae027611f
[3/8] KVM: x86: Add helper to query if variable MTRR MSR is base (versus mask)
      https://github.com/kvm-x86/linux/commit/ebda79e50577
[4/8] KVM: x86: Add helper to get variable MTRR range from MSR index
      https://github.com/kvm-x86/linux/commit/9ae38b4fb135
[5/8] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges
      https://github.com/kvm-x86/linux/commit/34a83deac31c
[6/8] KVM: x86: Move PAT MSR handling out of mtrr.c
      https://github.com/kvm-x86/linux/commit/bc7fe2f0b751
[7/8] KVM: x86: Make kvm_mtrr_valid() static now that there are no external users
      https://github.com/kvm-x86/linux/commit/3a5f49078eb5
[8/8] KVM: x86: Move common handling of PAT MSR writes to kvm_set_msr_common()
      https://github.com/kvm-x86/linux/commit/dee321977a23

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes