All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/15] vMTRR bugfix and optimization
@ 2015-06-15  8:55 Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 01/15] KVM: x86: fix CR0.CD virtualization Xiao Guangrong
                   ` (14 more replies)
  0 siblings, 15 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Changlog:
- let's fix the bugs for noncoherent_dma guests first

- these changes are from Paolo's review
  1) use inline functions instead of union definition for 'struct kvm_mtrr'
  2) improve code style
  3) fix fixed_msr_to_range_index which does not return the actual value
  4) introduce kvm_set_var_mtrr() to make the code more clearer
  5) improve mtrr_for_each_mem_type() APIs and clean it up
  6) drop @unit_size in 'struct fixed_mtrr_segment'

- these changes are from David Matlack's review
  1) improve comment
  2) fix fixed_mtrr_range_end_addr that did not count entries correctly
  

There are some bugs in current code if noncoherent_dma is detected:
- KVM still uses hugepage even if the 4K pages in that ranges span between
  on different memory types and uses the cache type of first page to do memory
  mapping
  
- CR0.CD is not checked so that guest memory is not UC as it expected  

This patchset fixes these bugs and also do optimization and cleanups.

Xiao Guangrong (15):
  KVM: x86: fix CR0.CD virtualization
  KVM: x86: move MTRR related code to a separate file
  KVM: MTRR: handle MSR_MTRRcap in kvm_mtrr_get_msr
  KVM: MTRR: remove mtrr_state.have_fixed
  KVM: MTRR: exactly define the size of variable MTRRs
  KVM: MTRR: clean up mtrr default type
  KVM: MTRR: do not split 64 bits MSR content
  KVM: MTRR: improve kvm_mtrr_get_guest_memory_type
  KVM: MTRR: introduce fixed_mtrr_segment table
  KVM: MTRR: introduce var_mtrr_range
  KVM: MTRR: sort variable MTRRs
  KVM: MTRR: introduce fixed_mtrr_addr_* functions
  KVM: MTRR: introduce mtrr_for_each_mem_type
  KVM: MTRR: simplify kvm_mtrr_get_guest_memory_type
  KVM: MTRR: do not map huage page for non-consistent range

 arch/x86/include/asm/kvm_host.h |  17 +-
 arch/x86/kvm/Makefile           |   2 +-
 arch/x86/kvm/mmu.c              | 123 ++-----
 arch/x86/kvm/mtrr.c             | 708 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx.c              |  32 +-
 arch/x86/kvm/x86.c              | 223 +------------
 arch/x86/kvm/x86.h              |   6 +
 7 files changed, 778 insertions(+), 333 deletions(-)
 create mode 100644 arch/x86/kvm/mtrr.c

-- 
2.1.0


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 01/15] KVM: x86: fix CR0.CD virtualization
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 02/15] KVM: x86: move MTRR related code to a separate file Xiao Guangrong
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Currently, CR0.CD is not checked when we virtualize memory cache type for
noncoherent_dma guests, this patch fixes it by :

- setting UC for all memory if CR0.CD = 1
- zapping all the last sptes in MMU if CR0.CD is changed

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 32 ++++++++++++++++++++++----------
 arch/x86/kvm/x86.c |  4 ++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 06a186b..2764381 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8628,7 +8628,8 @@ static int get_ept_level(void)
 
 static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 {
-	u64 ret;
+	u8 cache;
+	u64 ipat = 0;
 
 	/* For VT-d and EPT combination
 	 * 1. MMIO: always map as UC
@@ -8641,16 +8642,27 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	 * 3. EPT without VT-d: always map as WB and set IPAT=1 to keep
 	 *    consistent with host MTRR
 	 */
-	if (is_mmio)
-		ret = MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT;
-	else if (kvm_arch_has_noncoherent_dma(vcpu->kvm))
-		ret = kvm_get_guest_memory_type(vcpu, gfn) <<
-		      VMX_EPT_MT_EPTE_SHIFT;
-	else
-		ret = (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT)
-			| VMX_EPT_IPAT_BIT;
+	if (is_mmio) {
+		cache = MTRR_TYPE_UNCACHABLE;
+		goto exit;
+	}
 
-	return ret;
+	if (!kvm_arch_has_noncoherent_dma(vcpu->kvm)) {
+		ipat = VMX_EPT_IPAT_BIT;
+		cache = MTRR_TYPE_WRBACK;
+		goto exit;
+	}
+
+	if (kvm_read_cr0(vcpu) & X86_CR0_CD) {
+		ipat = VMX_EPT_IPAT_BIT;
+		cache = MTRR_TYPE_UNCACHABLE;
+		goto exit;
+	}
+
+	cache = kvm_get_guest_memory_type(vcpu, gfn);
+
+exit:
+	return (cache << VMX_EPT_MT_EPTE_SHIFT) | ipat;
 }
 
 static int vmx_get_lpage_level(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 43f0df7..43fdb10 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -621,6 +621,10 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 
 	if ((cr0 ^ old_cr0) & update_bits)
 		kvm_mmu_reset_context(vcpu);
+
+	if ((cr0 ^ old_cr0) & X86_CR0_CD)
+		kvm_zap_gfn_range(vcpu->kvm, 0, ~0ULL);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(kvm_set_cr0);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 02/15] KVM: x86: move MTRR related code to a separate file
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 01/15] KVM: x86: fix CR0.CD virtualization Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 03/15] KVM: MTRR: handle MSR_MTRRcap in kvm_mtrr_get_msr Xiao Guangrong
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

MTRR code locates in x86.c and mmu.c so that move them to a separate file to
make the organization more clearer and it will be the place where we fully
implement vMTRR

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |   1 -
 arch/x86/kvm/Makefile           |   2 +-
 arch/x86/kvm/mmu.c              | 103 ------------
 arch/x86/kvm/mtrr.c             | 335 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx.c              |   2 +-
 arch/x86/kvm/x86.c              | 214 +------------------------
 arch/x86/kvm/x86.h              |   3 +
 7 files changed, 342 insertions(+), 318 deletions(-)
 create mode 100644 arch/x86/kvm/mtrr.c

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 8ca32cf..cf8d320 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -894,7 +894,6 @@ int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3);
 
 int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 			  const void *val, int bytes);
-u8 kvm_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn);
 
 struct kvm_irq_mask_notifier {
 	void (*func)(struct kvm_irq_mask_notifier *kimn, bool masked);
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 16e8f96..470dc6c9 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -12,7 +12,7 @@ kvm-y			+= $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
 kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
 
 kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
-			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o
+			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o
 kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
 kvm-intel-y		+= vmx.o
 kvm-amd-y		+= svm.o
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index c88f0e4..532aad2 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2437,109 +2437,6 @@ int kvm_mmu_unprotect_page(struct kvm *kvm, gfn_t gfn)
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_unprotect_page);
 
-/*
- * The function is based on mtrr_type_lookup() in
- * arch/x86/kernel/cpu/mtrr/generic.c
- */
-static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
-			 u64 start, u64 end)
-{
-	u64 base, mask;
-	u8 prev_match, curr_match;
-	int i, num_var_ranges = KVM_NR_VAR_MTRR;
-
-	/* MTRR is completely disabled, use UC for all of physical memory. */
-	if (!(mtrr_state->enabled & 0x2))
-		return MTRR_TYPE_UNCACHABLE;
-
-	/* Make end inclusive end, instead of exclusive */
-	end--;
-
-	/* Look in fixed ranges. Just return the type as per start */
-	if (mtrr_state->have_fixed && (mtrr_state->enabled & 0x1) &&
-	      (start < 0x100000)) {
-		int idx;
-
-		if (start < 0x80000) {
-			idx = 0;
-			idx += (start >> 16);
-			return mtrr_state->fixed_ranges[idx];
-		} else if (start < 0xC0000) {
-			idx = 1 * 8;
-			idx += ((start - 0x80000) >> 14);
-			return mtrr_state->fixed_ranges[idx];
-		} else if (start < 0x1000000) {
-			idx = 3 * 8;
-			idx += ((start - 0xC0000) >> 12);
-			return mtrr_state->fixed_ranges[idx];
-		}
-	}
-
-	/*
-	 * Look in variable ranges
-	 * Look of multiple ranges matching this address and pick type
-	 * as per MTRR precedence
-	 */
-	prev_match = 0xFF;
-	for (i = 0; i < num_var_ranges; ++i) {
-		unsigned short start_state, end_state;
-
-		if (!(mtrr_state->var_ranges[i].mask_lo & (1 << 11)))
-			continue;
-
-		base = (((u64)mtrr_state->var_ranges[i].base_hi) << 32) +
-		       (mtrr_state->var_ranges[i].base_lo & PAGE_MASK);
-		mask = (((u64)mtrr_state->var_ranges[i].mask_hi) << 32) +
-		       (mtrr_state->var_ranges[i].mask_lo & PAGE_MASK);
-
-		start_state = ((start & mask) == (base & mask));
-		end_state = ((end & mask) == (base & mask));
-		if (start_state != end_state)
-			return 0xFE;
-
-		if ((start & mask) != (base & mask))
-			continue;
-
-		curr_match = mtrr_state->var_ranges[i].base_lo & 0xff;
-		if (prev_match == 0xFF) {
-			prev_match = curr_match;
-			continue;
-		}
-
-		if (prev_match == MTRR_TYPE_UNCACHABLE ||
-		    curr_match == MTRR_TYPE_UNCACHABLE)
-			return MTRR_TYPE_UNCACHABLE;
-
-		if ((prev_match == MTRR_TYPE_WRBACK &&
-		     curr_match == MTRR_TYPE_WRTHROUGH) ||
-		    (prev_match == MTRR_TYPE_WRTHROUGH &&
-		     curr_match == MTRR_TYPE_WRBACK)) {
-			prev_match = MTRR_TYPE_WRTHROUGH;
-			curr_match = MTRR_TYPE_WRTHROUGH;
-		}
-
-		if (prev_match != curr_match)
-			return MTRR_TYPE_UNCACHABLE;
-	}
-
-	if (prev_match != 0xFF)
-		return prev_match;
-
-	return mtrr_state->def_type;
-}
-
-u8 kvm_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
-{
-	u8 mtrr;
-
-	mtrr = get_mtrr_type(&vcpu->arch.mtrr_state, gfn << PAGE_SHIFT,
-			     (gfn << PAGE_SHIFT) + PAGE_SIZE);
-	if (mtrr == 0xfe || mtrr == 0xff)
-		mtrr = MTRR_TYPE_WRBACK;
-	return mtrr;
-}
-EXPORT_SYMBOL_GPL(kvm_get_guest_memory_type);
-
 static void __kvm_unsync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 {
 	trace_kvm_mmu_unsync_page(sp);
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
new file mode 100644
index 0000000..fb2f7e1
--- /dev/null
+++ b/arch/x86/kvm/mtrr.c
@@ -0,0 +1,335 @@
+/*
+ * vMTRR implementation
+ *
+ * Copyright (C) 2006 Qumranet, Inc.
+ * Copyright 2010 Red Hat, Inc. and/or its affiliates.
+ * Copyright(C) 2015 Intel Corporation.
+ *
+ * Authors:
+ *   Yaniv Kamay  <yaniv@qumranet.com>
+ *   Avi Kivity   <avi@qumranet.com>
+ *   Marcelo Tosatti <mtosatti@redhat.com>
+ *   Paolo Bonzini <pbonzini@redhat.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <linux/kvm_host.h>
+#include <asm/mtrr.h>
+
+#include "cpuid.h"
+#include "mmu.h"
+
+static bool msr_mtrr_valid(unsigned msr)
+{
+	switch (msr) {
+	case 0x200 ... 0x200 + 2 * KVM_NR_VAR_MTRR - 1:
+	case MSR_MTRRfix64K_00000:
+	case MSR_MTRRfix16K_80000:
+	case MSR_MTRRfix16K_A0000:
+	case MSR_MTRRfix4K_C0000:
+	case MSR_MTRRfix4K_C8000:
+	case MSR_MTRRfix4K_D0000:
+	case MSR_MTRRfix4K_D8000:
+	case MSR_MTRRfix4K_E0000:
+	case MSR_MTRRfix4K_E8000:
+	case MSR_MTRRfix4K_F0000:
+	case MSR_MTRRfix4K_F8000:
+	case MSR_MTRRdefType:
+	case MSR_IA32_CR_PAT:
+		return true;
+	case 0x2f8:
+		return true;
+	}
+	return false;
+}
+
+static bool valid_pat_type(unsigned t)
+{
+	return t < 8 && (1 << t) & 0xf3; /* 0, 1, 4, 5, 6, 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)
+{
+	int i;
+	u64 mask;
+
+	if (!msr_mtrr_valid(msr))
+		return false;
+
+	if (msr == MSR_IA32_CR_PAT) {
+		for (i = 0; i < 8; i++)
+			if (!valid_pat_type((data >> (i * 8)) & 0xff))
+				return false;
+		return true;
+	} else if (msr == MSR_MTRRdefType) {
+		if (data & ~0xcff)
+			return false;
+		return valid_mtrr_type(data & 0xff);
+	} else if (msr >= MSR_MTRRfix64K_00000 && msr <= MSR_MTRRfix4K_F8000) {
+		for (i = 0; i < 8 ; i++)
+			if (!valid_mtrr_type((data >> (i * 8)) & 0xff))
+				return false;
+		return true;
+	}
+
+	/* variable MTRRs */
+	WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
+
+	mask = (~0ULL) << cpuid_maxphyaddr(vcpu);
+	if ((msr & 1) == 0) {
+		/* MTRR base */
+		if (!valid_mtrr_type(data & 0xff))
+			return false;
+		mask |= 0xf00;
+	} else
+		/* MTRR mask */
+		mask |= 0x7ff;
+	if (data & mask) {
+		kvm_inject_gp(vcpu, 0);
+		return false;
+	}
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
+
+static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
+{
+	struct mtrr_state_type *mtrr_state = &vcpu->arch.mtrr_state;
+	unsigned char mtrr_enabled = mtrr_state->enabled;
+	gfn_t start, end, mask;
+	int index;
+	bool is_fixed = true;
+
+	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
+	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
+		return;
+
+	if (!(mtrr_enabled & 0x2) && msr != MSR_MTRRdefType)
+		return;
+
+	switch (msr) {
+	case MSR_MTRRfix64K_00000:
+		start = 0x0;
+		end = 0x80000;
+		break;
+	case MSR_MTRRfix16K_80000:
+		start = 0x80000;
+		end = 0xa0000;
+		break;
+	case MSR_MTRRfix16K_A0000:
+		start = 0xa0000;
+		end = 0xc0000;
+		break;
+	case MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000:
+		index = msr - MSR_MTRRfix4K_C0000;
+		start = 0xc0000 + index * (32 << 10);
+		end = start + (32 << 10);
+		break;
+	case MSR_MTRRdefType:
+		is_fixed = false;
+		start = 0x0;
+		end = ~0ULL;
+		break;
+	default:
+		/* variable range MTRRs. */
+		is_fixed = false;
+		index = (msr - 0x200) / 2;
+		start = (((u64)mtrr_state->var_ranges[index].base_hi) << 32) +
+		       (mtrr_state->var_ranges[index].base_lo & PAGE_MASK);
+		mask = (((u64)mtrr_state->var_ranges[index].mask_hi) << 32) +
+		       (mtrr_state->var_ranges[index].mask_lo & PAGE_MASK);
+		mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
+
+		end = ((start & mask) | ~mask) + 1;
+	}
+
+	if (is_fixed && !(mtrr_enabled & 0x1))
+		return;
+
+	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
+}
+
+int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
+
+	if (!kvm_mtrr_valid(vcpu, msr, data))
+		return 1;
+
+	if (msr == MSR_MTRRdefType) {
+		vcpu->arch.mtrr_state.def_type = data;
+		vcpu->arch.mtrr_state.enabled = (data & 0xc00) >> 10;
+	} else if (msr == MSR_MTRRfix64K_00000)
+		p[0] = data;
+	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
+		p[1 + msr - MSR_MTRRfix16K_80000] = data;
+	else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
+		p[3 + msr - MSR_MTRRfix4K_C0000] = data;
+	else if (msr == MSR_IA32_CR_PAT)
+		vcpu->arch.pat = data;
+	else {	/* Variable MTRRs */
+		int idx, is_mtrr_mask;
+		u64 *pt;
+
+		idx = (msr - 0x200) / 2;
+		is_mtrr_mask = msr - 0x200 - 2 * idx;
+		if (!is_mtrr_mask)
+			pt =
+			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].base_lo;
+		else
+			pt =
+			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].mask_lo;
+		*pt = data;
+	}
+
+	update_mtrr(vcpu, msr);
+	return 0;
+}
+
+int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+{
+	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
+
+	if (!msr_mtrr_valid(msr))
+		return 1;
+
+	if (msr == MSR_MTRRdefType)
+		*pdata = vcpu->arch.mtrr_state.def_type +
+			 (vcpu->arch.mtrr_state.enabled << 10);
+	else if (msr == MSR_MTRRfix64K_00000)
+		*pdata = p[0];
+	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
+		*pdata = p[1 + msr - MSR_MTRRfix16K_80000];
+	else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
+		*pdata = p[3 + msr - MSR_MTRRfix4K_C0000];
+	else if (msr == MSR_IA32_CR_PAT)
+		*pdata = vcpu->arch.pat;
+	else {	/* Variable MTRRs */
+		int idx, is_mtrr_mask;
+		u64 *pt;
+
+		idx = (msr - 0x200) / 2;
+		is_mtrr_mask = msr - 0x200 - 2 * idx;
+		if (!is_mtrr_mask)
+			pt =
+			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].base_lo;
+		else
+			pt =
+			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].mask_lo;
+		*pdata = *pt;
+	}
+
+	return 0;
+}
+
+/*
+ * The function is based on mtrr_type_lookup() in
+ * arch/x86/kernel/cpu/mtrr/generic.c
+ */
+static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
+			 u64 start, u64 end)
+{
+	u64 base, mask;
+	u8 prev_match, curr_match;
+	int i, num_var_ranges = KVM_NR_VAR_MTRR;
+
+	/* MTRR is completely disabled, use UC for all of physical memory. */
+	if (!(mtrr_state->enabled & 0x2))
+		return MTRR_TYPE_UNCACHABLE;
+
+	/* Make end inclusive end, instead of exclusive */
+	end--;
+
+	/* Look in fixed ranges. Just return the type as per start */
+	if (mtrr_state->have_fixed && (mtrr_state->enabled & 0x1) &&
+	      (start < 0x100000)) {
+		int idx;
+
+		if (start < 0x80000) {
+			idx = 0;
+			idx += (start >> 16);
+			return mtrr_state->fixed_ranges[idx];
+		} else if (start < 0xC0000) {
+			idx = 1 * 8;
+			idx += ((start - 0x80000) >> 14);
+			return mtrr_state->fixed_ranges[idx];
+		} else if (start < 0x1000000) {
+			idx = 3 * 8;
+			idx += ((start - 0xC0000) >> 12);
+			return mtrr_state->fixed_ranges[idx];
+		}
+	}
+
+	/*
+	 * Look in variable ranges
+	 * Look of multiple ranges matching this address and pick type
+	 * as per MTRR precedence
+	 */
+	prev_match = 0xFF;
+	for (i = 0; i < num_var_ranges; ++i) {
+		unsigned short start_state, end_state;
+
+		if (!(mtrr_state->var_ranges[i].mask_lo & (1 << 11)))
+			continue;
+
+		base = (((u64)mtrr_state->var_ranges[i].base_hi) << 32) +
+		       (mtrr_state->var_ranges[i].base_lo & PAGE_MASK);
+		mask = (((u64)mtrr_state->var_ranges[i].mask_hi) << 32) +
+		       (mtrr_state->var_ranges[i].mask_lo & PAGE_MASK);
+
+		start_state = ((start & mask) == (base & mask));
+		end_state = ((end & mask) == (base & mask));
+		if (start_state != end_state)
+			return 0xFE;
+
+		if ((start & mask) != (base & mask))
+			continue;
+
+		curr_match = mtrr_state->var_ranges[i].base_lo & 0xff;
+		if (prev_match == 0xFF) {
+			prev_match = curr_match;
+			continue;
+		}
+
+		if (prev_match == MTRR_TYPE_UNCACHABLE ||
+		    curr_match == MTRR_TYPE_UNCACHABLE)
+			return MTRR_TYPE_UNCACHABLE;
+
+		if ((prev_match == MTRR_TYPE_WRBACK &&
+		     curr_match == MTRR_TYPE_WRTHROUGH) ||
+		    (prev_match == MTRR_TYPE_WRTHROUGH &&
+		     curr_match == MTRR_TYPE_WRBACK)) {
+			prev_match = MTRR_TYPE_WRTHROUGH;
+			curr_match = MTRR_TYPE_WRTHROUGH;
+		}
+
+		if (prev_match != curr_match)
+			return MTRR_TYPE_UNCACHABLE;
+	}
+
+	if (prev_match != 0xFF)
+		return prev_match;
+
+	return mtrr_state->def_type;
+}
+
+u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
+{
+	u8 mtrr;
+
+	mtrr = get_mtrr_type(&vcpu->arch.mtrr_state, gfn << PAGE_SHIFT,
+			     (gfn << PAGE_SHIFT) + PAGE_SIZE);
+	if (mtrr == 0xfe || mtrr == 0xff)
+		mtrr = MTRR_TYPE_WRBACK;
+	return mtrr;
+}
+EXPORT_SYMBOL_GPL(kvm_mtrr_get_guest_memory_type);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2764381..44eafdb 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8659,7 +8659,7 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 		goto exit;
 	}
 
-	cache = kvm_get_guest_memory_type(vcpu, gfn);
+	cache = kvm_mtrr_get_guest_memory_type(vcpu, gfn);
 
 exit:
 	return (cache << VMX_EPT_MT_EPTE_SHIFT) | ipat;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 43fdb10..e2bc798 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -57,7 +57,6 @@
 #include <asm/debugreg.h>
 #include <asm/msr.h>
 #include <asm/desc.h>
-#include <asm/mtrr.h>
 #include <asm/mce.h>
 #include <asm/i387.h>
 #include <asm/fpu-internal.h> /* Ugh! */
@@ -1803,179 +1802,6 @@ static void kvmclock_sync_fn(struct work_struct *work)
 					KVMCLOCK_SYNC_PERIOD);
 }
 
-static bool msr_mtrr_valid(unsigned msr)
-{
-	switch (msr) {
-	case 0x200 ... 0x200 + 2 * KVM_NR_VAR_MTRR - 1:
-	case MSR_MTRRfix64K_00000:
-	case MSR_MTRRfix16K_80000:
-	case MSR_MTRRfix16K_A0000:
-	case MSR_MTRRfix4K_C0000:
-	case MSR_MTRRfix4K_C8000:
-	case MSR_MTRRfix4K_D0000:
-	case MSR_MTRRfix4K_D8000:
-	case MSR_MTRRfix4K_E0000:
-	case MSR_MTRRfix4K_E8000:
-	case MSR_MTRRfix4K_F0000:
-	case MSR_MTRRfix4K_F8000:
-	case MSR_MTRRdefType:
-	case MSR_IA32_CR_PAT:
-		return true;
-	case 0x2f8:
-		return true;
-	}
-	return false;
-}
-
-static bool valid_pat_type(unsigned t)
-{
-	return t < 8 && (1 << t) & 0xf3; /* 0, 1, 4, 5, 6, 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)
-{
-	int i;
-	u64 mask;
-
-	if (!msr_mtrr_valid(msr))
-		return false;
-
-	if (msr == MSR_IA32_CR_PAT) {
-		for (i = 0; i < 8; i++)
-			if (!valid_pat_type((data >> (i * 8)) & 0xff))
-				return false;
-		return true;
-	} else if (msr == MSR_MTRRdefType) {
-		if (data & ~0xcff)
-			return false;
-		return valid_mtrr_type(data & 0xff);
-	} else if (msr >= MSR_MTRRfix64K_00000 && msr <= MSR_MTRRfix4K_F8000) {
-		for (i = 0; i < 8 ; i++)
-			if (!valid_mtrr_type((data >> (i * 8)) & 0xff))
-				return false;
-		return true;
-	}
-
-	/* variable MTRRs */
-	WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
-
-	mask = (~0ULL) << cpuid_maxphyaddr(vcpu);
-	if ((msr & 1) == 0) {
-		/* MTRR base */
-		if (!valid_mtrr_type(data & 0xff))
-			return false;
-		mask |= 0xf00;
-	} else
-		/* MTRR mask */
-		mask |= 0x7ff;
-	if (data & mask) {
-		kvm_inject_gp(vcpu, 0);
-		return false;
-	}
-
-	return true;
-}
-EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
-
-static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
-{
-	struct mtrr_state_type *mtrr_state = &vcpu->arch.mtrr_state;
-	unsigned char mtrr_enabled = mtrr_state->enabled;
-	gfn_t start, end, mask;
-	int index;
-	bool is_fixed = true;
-
-	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
-	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
-		return;
-
-	if (!(mtrr_enabled & 0x2) && msr != MSR_MTRRdefType)
-		return;
-
-	switch (msr) {
-	case MSR_MTRRfix64K_00000:
-		start = 0x0;
-		end = 0x80000;
-		break;
-	case MSR_MTRRfix16K_80000:
-		start = 0x80000;
-		end = 0xa0000;
-		break;
-	case MSR_MTRRfix16K_A0000:
-		start = 0xa0000;
-		end = 0xc0000;
-		break;
-	case MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000:
-		index = msr - MSR_MTRRfix4K_C0000;
-		start = 0xc0000 + index * (32 << 10);
-		end = start + (32 << 10);
-		break;
-	case MSR_MTRRdefType:
-		is_fixed = false;
-		start = 0x0;
-		end = ~0ULL;
-		break;
-	default:
-		/* variable range MTRRs. */
-		is_fixed = false;
-		index = (msr - 0x200) / 2;
-		start = (((u64)mtrr_state->var_ranges[index].base_hi) << 32) +
-		       (mtrr_state->var_ranges[index].base_lo & PAGE_MASK);
-		mask = (((u64)mtrr_state->var_ranges[index].mask_hi) << 32) +
-		       (mtrr_state->var_ranges[index].mask_lo & PAGE_MASK);
-		mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
-
-		end = ((start & mask) | ~mask) + 1;
-	}
-
-	if (is_fixed && !(mtrr_enabled & 0x1))
-		return;
-
-	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
-}
-
-static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
-{
-	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
-
-	if (!kvm_mtrr_valid(vcpu, msr, data))
-		return 1;
-
-	if (msr == MSR_MTRRdefType) {
-		vcpu->arch.mtrr_state.def_type = data;
-		vcpu->arch.mtrr_state.enabled = (data & 0xc00) >> 10;
-	} else if (msr == MSR_MTRRfix64K_00000)
-		p[0] = data;
-	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
-		p[1 + msr - MSR_MTRRfix16K_80000] = data;
-	else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
-		p[3 + msr - MSR_MTRRfix4K_C0000] = data;
-	else if (msr == MSR_IA32_CR_PAT)
-		vcpu->arch.pat = data;
-	else {	/* Variable MTRRs */
-		int idx, is_mtrr_mask;
-		u64 *pt;
-
-		idx = (msr - 0x200) / 2;
-		is_mtrr_mask = msr - 0x200 - 2 * idx;
-		if (!is_mtrr_mask)
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].base_lo;
-		else
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].mask_lo;
-		*pt = data;
-	}
-
-	update_mtrr(vcpu, msr);
-	return 0;
-}
-
 static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	u64 mcg_cap = vcpu->arch.mcg_cap;
@@ -2267,7 +2093,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			    __func__, data);
 		break;
 	case 0x200 ... 0x2ff:
-		return set_msr_mtrr(vcpu, msr, data);
+		return kvm_mtrr_set_msr(vcpu, msr, data);
 	case MSR_IA32_APICBASE:
 		return kvm_set_apic_base(vcpu, msr_info);
 	case APIC_BASE_MSR ... APIC_BASE_MSR + 0x3ff:
@@ -2479,42 +2305,6 @@ int kvm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 }
 EXPORT_SYMBOL_GPL(kvm_get_msr);
 
-static int get_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
-{
-	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
-
-	if (!msr_mtrr_valid(msr))
-		return 1;
-
-	if (msr == MSR_MTRRdefType)
-		*pdata = vcpu->arch.mtrr_state.def_type +
-			 (vcpu->arch.mtrr_state.enabled << 10);
-	else if (msr == MSR_MTRRfix64K_00000)
-		*pdata = p[0];
-	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
-		*pdata = p[1 + msr - MSR_MTRRfix16K_80000];
-	else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
-		*pdata = p[3 + msr - MSR_MTRRfix4K_C0000];
-	else if (msr == MSR_IA32_CR_PAT)
-		*pdata = vcpu->arch.pat;
-	else {	/* Variable MTRRs */
-		int idx, is_mtrr_mask;
-		u64 *pt;
-
-		idx = (msr - 0x200) / 2;
-		is_mtrr_mask = msr - 0x200 - 2 * idx;
-		if (!is_mtrr_mask)
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].base_lo;
-		else
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].mask_lo;
-		*pdata = *pt;
-	}
-
-	return 0;
-}
-
 static int get_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 {
 	u64 data;
@@ -2656,7 +2446,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = 0x500 | KVM_NR_VAR_MTRR;
 		break;
 	case 0x200 ... 0x2ff:
-		return get_msr_mtrr(vcpu, msr_info->index, &msr_info->data);
+		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
 	case 0xcd: /* fsb frequency */
 		msr_info->data = 3;
 		break;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 01a1d01..aeb0bb2 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -162,7 +162,10 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
 	gva_t addr, void *val, unsigned int bytes,
 	struct x86_exception *exception);
 
+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);
 
 #define KVM_SUPPORTED_XCR0     (XSTATE_FP | XSTATE_SSE | XSTATE_YMM \
 				| XSTATE_BNDREGS | XSTATE_BNDCSR \
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 03/15] KVM: MTRR: handle MSR_MTRRcap in kvm_mtrr_get_msr
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 01/15] KVM: x86: fix CR0.CD virtualization Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 02/15] KVM: x86: move MTRR related code to a separate file Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 04/15] KVM: MTRR: remove mtrr_state.have_fixed Xiao Guangrong
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

MSR_MTRRcap is a MTRR msr so move the handler to the common place, also
add some comments to make the hard code more readable

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 12 ++++++++++++
 arch/x86/kvm/x86.c  |  2 --
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index fb2f7e1..a05846a 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -199,6 +199,18 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 {
 	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
 
+	/* MSR_MTRRcap is a readonly MSR. */
+	if (msr == MSR_MTRRcap) {
+		/*
+		 * SMRR = 0
+		 * WC = 1
+		 * FIX = 1
+		 * VCNT = KVM_NR_VAR_MTRR
+		 */
+		*pdata = 0x500 | KVM_NR_VAR_MTRR;
+		return 0;
+	}
+
 	if (!msr_mtrr_valid(msr))
 		return 1;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e2bc798..fb6c9a1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2443,8 +2443,6 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = 0x100000000ULL;
 		break;
 	case MSR_MTRRcap:
-		msr_info->data = 0x500 | KVM_NR_VAR_MTRR;
-		break;
 	case 0x200 ... 0x2ff:
 		return kvm_mtrr_get_msr(vcpu, msr_info->index, &msr_info->data);
 	case 0xcd: /* fsb frequency */
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 04/15] KVM: MTRR: remove mtrr_state.have_fixed
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (2 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 03/15] KVM: MTRR: handle MSR_MTRRcap in kvm_mtrr_get_msr Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 05/15] KVM: MTRR: exactly define the size of variable MTRRs Xiao Guangrong
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

vMTRR does not depend on any host MTRR feature and fixed MTRRs have always
been implemented, so drop this field

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h | 9 ++++++++-
 arch/x86/kvm/mtrr.c             | 7 +++----
 arch/x86/kvm/x86.c              | 1 -
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cf8d320..cbf9f07 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -342,6 +342,13 @@ enum {
 	KVM_DEBUGREG_RELOAD = 4,
 };
 
+struct kvm_mtrr {
+	struct mtrr_var_range var_ranges[MTRR_MAX_VAR_RANGES];
+	mtrr_type fixed_ranges[KVM_NR_FIXED_MTRR_REGION];
+	unsigned char enabled;
+	mtrr_type def_type;
+};
+
 struct kvm_vcpu_arch {
 	/*
 	 * rip and regs accesses must go through
@@ -472,7 +479,7 @@ struct kvm_vcpu_arch {
 	bool nmi_injected;    /* Trying to inject an NMI this entry */
 	bool smi_pending;    /* SMI queued after currently running handler */
 
-	struct mtrr_state_type mtrr_state;
+	struct kvm_mtrr mtrr_state;
 	u64 pat;
 
 	unsigned switch_db_regs;
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index a05846a..ce061ff 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -103,7 +103,7 @@ EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
 
 static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 {
-	struct mtrr_state_type *mtrr_state = &vcpu->arch.mtrr_state;
+	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
 	unsigned char mtrr_enabled = mtrr_state->enabled;
 	gfn_t start, end, mask;
 	int index;
@@ -247,7 +247,7 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
  * The function is based on mtrr_type_lookup() in
  * arch/x86/kernel/cpu/mtrr/generic.c
  */
-static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
+static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 			 u64 start, u64 end)
 {
 	u64 base, mask;
@@ -262,8 +262,7 @@ static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
 	end--;
 
 	/* Look in fixed ranges. Just return the type as per start */
-	if (mtrr_state->have_fixed && (mtrr_state->enabled & 0x1) &&
-	      (start < 0x100000)) {
+	if ((mtrr_state->enabled & 0x1) && (start < 0x100000)) {
 		int idx;
 
 		if (start < 0x80000) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fb6c9a1..2ffad7f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7379,7 +7379,6 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
 	int r;
 
-	vcpu->arch.mtrr_state.have_fixed = 1;
 	r = vcpu_load(vcpu);
 	if (r)
 		return r;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 05/15] KVM: MTRR: exactly define the size of variable MTRRs
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (3 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 04/15] KVM: MTRR: remove mtrr_state.have_fixed Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 06/15] KVM: MTRR: clean up mtrr default type Xiao Guangrong
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Only KVM_NR_VAR_MTRR variable MTRRs are available in KVM guest

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cbf9f07..fe9cbe4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -343,7 +343,7 @@ enum {
 };
 
 struct kvm_mtrr {
-	struct mtrr_var_range var_ranges[MTRR_MAX_VAR_RANGES];
+	struct mtrr_var_range var_ranges[KVM_NR_VAR_MTRR];
 	mtrr_type fixed_ranges[KVM_NR_FIXED_MTRR_REGION];
 	unsigned char enabled;
 	mtrr_type def_type;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 06/15] KVM: MTRR: clean up mtrr default type
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (4 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 05/15] KVM: MTRR: exactly define the size of variable MTRRs Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 07/15] KVM: MTRR: do not split 64 bits MSR content Xiao Guangrong
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Drop kvm_mtrr->enable, omit the decode/code workload and get rid of
all the hard code

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |  3 +--
 arch/x86/kvm/mtrr.c             | 40 ++++++++++++++++++++++++++++------------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index fe9cbe4..8d43006 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -345,8 +345,7 @@ enum {
 struct kvm_mtrr {
 	struct mtrr_var_range var_ranges[KVM_NR_VAR_MTRR];
 	mtrr_type fixed_ranges[KVM_NR_FIXED_MTRR_REGION];
-	unsigned char enabled;
-	mtrr_type def_type;
+	u64 deftype;
 };
 
 struct kvm_vcpu_arch {
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index ce061ff..191de64 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -22,6 +22,10 @@
 #include "cpuid.h"
 #include "mmu.h"
 
+#define IA32_MTRR_DEF_TYPE_E		(1ULL << 11)
+#define IA32_MTRR_DEF_TYPE_FE		(1ULL << 10)
+#define IA32_MTRR_DEF_TYPE_TYPE_MASK	(0xff)
+
 static bool msr_mtrr_valid(unsigned msr)
 {
 	switch (msr) {
@@ -101,10 +105,24 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 }
 EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
 
+static bool mtrr_is_enabled(struct kvm_mtrr *mtrr_state)
+{
+	return !!(mtrr_state->deftype & IA32_MTRR_DEF_TYPE_E);
+}
+
+static bool fixed_mtrr_is_enabled(struct kvm_mtrr *mtrr_state)
+{
+	return !!(mtrr_state->deftype & IA32_MTRR_DEF_TYPE_FE);
+}
+
+static u8 mtrr_default_type(struct kvm_mtrr *mtrr_state)
+{
+	return mtrr_state->deftype & IA32_MTRR_DEF_TYPE_TYPE_MASK;
+}
+
 static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
-	unsigned char mtrr_enabled = mtrr_state->enabled;
 	gfn_t start, end, mask;
 	int index;
 	bool is_fixed = true;
@@ -113,7 +131,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
 		return;
 
-	if (!(mtrr_enabled & 0x2) && msr != MSR_MTRRdefType)
+	if (!mtrr_is_enabled(mtrr_state) && msr != MSR_MTRRdefType)
 		return;
 
 	switch (msr) {
@@ -152,7 +170,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 		end = ((start & mask) | ~mask) + 1;
 	}
 
-	if (is_fixed && !(mtrr_enabled & 0x1))
+	if (is_fixed && !fixed_mtrr_is_enabled(mtrr_state))
 		return;
 
 	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
@@ -165,10 +183,9 @@ int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 	if (!kvm_mtrr_valid(vcpu, msr, data))
 		return 1;
 
-	if (msr == MSR_MTRRdefType) {
-		vcpu->arch.mtrr_state.def_type = data;
-		vcpu->arch.mtrr_state.enabled = (data & 0xc00) >> 10;
-	} else if (msr == MSR_MTRRfix64K_00000)
+	if (msr == MSR_MTRRdefType)
+		vcpu->arch.mtrr_state.deftype = data;
+	else if (msr == MSR_MTRRfix64K_00000)
 		p[0] = data;
 	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
 		p[1 + msr - MSR_MTRRfix16K_80000] = data;
@@ -215,8 +232,7 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 		return 1;
 
 	if (msr == MSR_MTRRdefType)
-		*pdata = vcpu->arch.mtrr_state.def_type +
-			 (vcpu->arch.mtrr_state.enabled << 10);
+		*pdata = vcpu->arch.mtrr_state.deftype;
 	else if (msr == MSR_MTRRfix64K_00000)
 		*pdata = p[0];
 	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
@@ -255,14 +271,14 @@ static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 	int i, num_var_ranges = KVM_NR_VAR_MTRR;
 
 	/* MTRR is completely disabled, use UC for all of physical memory. */
-	if (!(mtrr_state->enabled & 0x2))
+	if (!mtrr_is_enabled(mtrr_state))
 		return MTRR_TYPE_UNCACHABLE;
 
 	/* Make end inclusive end, instead of exclusive */
 	end--;
 
 	/* Look in fixed ranges. Just return the type as per start */
-	if ((mtrr_state->enabled & 0x1) && (start < 0x100000)) {
+	if (fixed_mtrr_is_enabled(mtrr_state) && (start < 0x100000)) {
 		int idx;
 
 		if (start < 0x80000) {
@@ -330,7 +346,7 @@ static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 	if (prev_match != 0xFF)
 		return prev_match;
 
-	return mtrr_state->def_type;
+	return mtrr_default_type(mtrr_state);
 }
 
 u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 07/15] KVM: MTRR: do not split 64 bits MSR content
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (5 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 06/15] KVM: MTRR: clean up mtrr default type Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 08/15] KVM: MTRR: improve kvm_mtrr_get_guest_memory_type Xiao Guangrong
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Variable MTRR MSRs are 64 bits which are directly accessed with full length,
no reason to split them to two 32 bits

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |  7 ++++++-
 arch/x86/kvm/mtrr.c             | 32 ++++++++++----------------------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 8d43006..f735548 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -342,8 +342,13 @@ enum {
 	KVM_DEBUGREG_RELOAD = 4,
 };
 
+struct kvm_mtrr_range {
+	u64 base;
+	u64 mask;
+};
+
 struct kvm_mtrr {
-	struct mtrr_var_range var_ranges[KVM_NR_VAR_MTRR];
+	struct kvm_mtrr_range var_ranges[KVM_NR_VAR_MTRR];
 	mtrr_type fixed_ranges[KVM_NR_FIXED_MTRR_REGION];
 	u64 deftype;
 };
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index 191de64..c4b80a4 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -161,10 +161,8 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 		/* variable range MTRRs. */
 		is_fixed = false;
 		index = (msr - 0x200) / 2;
-		start = (((u64)mtrr_state->var_ranges[index].base_hi) << 32) +
-		       (mtrr_state->var_ranges[index].base_lo & PAGE_MASK);
-		mask = (((u64)mtrr_state->var_ranges[index].mask_hi) << 32) +
-		       (mtrr_state->var_ranges[index].mask_lo & PAGE_MASK);
+		start = mtrr_state->var_ranges[index].base & PAGE_MASK;
+		mask = mtrr_state->var_ranges[index].mask & PAGE_MASK;
 		mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
 
 		end = ((start & mask) | ~mask) + 1;
@@ -195,17 +193,13 @@ int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		vcpu->arch.pat = data;
 	else {	/* Variable MTRRs */
 		int idx, is_mtrr_mask;
-		u64 *pt;
 
 		idx = (msr - 0x200) / 2;
 		is_mtrr_mask = msr - 0x200 - 2 * idx;
 		if (!is_mtrr_mask)
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].base_lo;
+			vcpu->arch.mtrr_state.var_ranges[idx].base = data;
 		else
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].mask_lo;
-		*pt = data;
+			vcpu->arch.mtrr_state.var_ranges[idx].mask = data;
 	}
 
 	update_mtrr(vcpu, msr);
@@ -243,17 +237,13 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 		*pdata = vcpu->arch.pat;
 	else {	/* Variable MTRRs */
 		int idx, is_mtrr_mask;
-		u64 *pt;
 
 		idx = (msr - 0x200) / 2;
 		is_mtrr_mask = msr - 0x200 - 2 * idx;
 		if (!is_mtrr_mask)
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].base_lo;
+			*pdata = vcpu->arch.mtrr_state.var_ranges[idx].base;
 		else
-			pt =
-			  (u64 *)&vcpu->arch.mtrr_state.var_ranges[idx].mask_lo;
-		*pdata = *pt;
+			*pdata = vcpu->arch.mtrr_state.var_ranges[idx].mask;
 	}
 
 	return 0;
@@ -305,13 +295,11 @@ static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 	for (i = 0; i < num_var_ranges; ++i) {
 		unsigned short start_state, end_state;
 
-		if (!(mtrr_state->var_ranges[i].mask_lo & (1 << 11)))
+		if (!(mtrr_state->var_ranges[i].mask & (1 << 11)))
 			continue;
 
-		base = (((u64)mtrr_state->var_ranges[i].base_hi) << 32) +
-		       (mtrr_state->var_ranges[i].base_lo & PAGE_MASK);
-		mask = (((u64)mtrr_state->var_ranges[i].mask_hi) << 32) +
-		       (mtrr_state->var_ranges[i].mask_lo & PAGE_MASK);
+		base = mtrr_state->var_ranges[i].base & PAGE_MASK;
+		mask = mtrr_state->var_ranges[i].mask & PAGE_MASK;
 
 		start_state = ((start & mask) == (base & mask));
 		end_state = ((end & mask) == (base & mask));
@@ -321,7 +309,7 @@ static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 		if ((start & mask) != (base & mask))
 			continue;
 
-		curr_match = mtrr_state->var_ranges[i].base_lo & 0xff;
+		curr_match = mtrr_state->var_ranges[i].base & 0xff;
 		if (prev_match == 0xFF) {
 			prev_match = curr_match;
 			continue;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 08/15] KVM: MTRR: improve kvm_mtrr_get_guest_memory_type
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (6 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 07/15] KVM: MTRR: do not split 64 bits MSR content Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 09/15] KVM: MTRR: introduce fixed_mtrr_segment table Xiao Guangrong
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

 - kvm_mtrr_get_guest_memory_type() only checks one page in MTRRs so
   that it's unnecessary to check to see if the range is partially
   covered in MTRR

 - optimize the check of overlap memory type and add some comments
   to explain the precedence

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 94 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index c4b80a4..2ea1213 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -249,24 +249,22 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	return 0;
 }
 
-/*
- * The function is based on mtrr_type_lookup() in
- * arch/x86/kernel/cpu/mtrr/generic.c
- */
-static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
-			 u64 start, u64 end)
+u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
-	u64 base, mask;
-	u8 prev_match, curr_match;
-	int i, num_var_ranges = KVM_NR_VAR_MTRR;
+	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
+	u64 base, mask, start;
+	int i, num_var_ranges, type;
+	const int wt_wb_mask = (1 << MTRR_TYPE_WRBACK)
+			       | (1 << MTRR_TYPE_WRTHROUGH);
+
+	start = gfn_to_gpa(gfn);
+	num_var_ranges = KVM_NR_VAR_MTRR;
+	type = -1;
 
 	/* MTRR is completely disabled, use UC for all of physical memory. */
 	if (!mtrr_is_enabled(mtrr_state))
 		return MTRR_TYPE_UNCACHABLE;
 
-	/* Make end inclusive end, instead of exclusive */
-	end--;
-
 	/* Look in fixed ranges. Just return the type as per start */
 	if (fixed_mtrr_is_enabled(mtrr_state) && (start < 0x100000)) {
 		int idx;
@@ -291,9 +289,8 @@ static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 	 * Look of multiple ranges matching this address and pick type
 	 * as per MTRR precedence
 	 */
-	prev_match = 0xFF;
 	for (i = 0; i < num_var_ranges; ++i) {
-		unsigned short start_state, end_state;
+		int curr_type;
 
 		if (!(mtrr_state->var_ranges[i].mask & (1 << 11)))
 			continue;
@@ -301,50 +298,57 @@ static int get_mtrr_type(struct kvm_mtrr *mtrr_state,
 		base = mtrr_state->var_ranges[i].base & PAGE_MASK;
 		mask = mtrr_state->var_ranges[i].mask & PAGE_MASK;
 
-		start_state = ((start & mask) == (base & mask));
-		end_state = ((end & mask) == (base & mask));
-		if (start_state != end_state)
-			return 0xFE;
-
 		if ((start & mask) != (base & mask))
 			continue;
 
-		curr_match = mtrr_state->var_ranges[i].base & 0xff;
-		if (prev_match == 0xFF) {
-			prev_match = curr_match;
+		/*
+		 * Please refer to Intel SDM Volume 3: 11.11.4.1 MTRR
+		 * Precedences.
+		 */
+
+		curr_type = mtrr_state->var_ranges[i].base & 0xff;
+		if (type == -1) {
+			type = curr_type;
 			continue;
 		}
 
-		if (prev_match == MTRR_TYPE_UNCACHABLE ||
-		    curr_match == MTRR_TYPE_UNCACHABLE)
+		/*
+		 * If two or more variable memory ranges match and the
+		 * memory types are identical, then that memory type is
+		 * used.
+		 */
+		if (type == curr_type)
+			continue;
+
+		/*
+		 * If two or more variable memory ranges match and one of
+		 * the memory types is UC, the UC memory type used.
+		 */
+		if (curr_type == MTRR_TYPE_UNCACHABLE)
 			return MTRR_TYPE_UNCACHABLE;
 
-		if ((prev_match == MTRR_TYPE_WRBACK &&
-		     curr_match == MTRR_TYPE_WRTHROUGH) ||
-		    (prev_match == MTRR_TYPE_WRTHROUGH &&
-		     curr_match == MTRR_TYPE_WRBACK)) {
-			prev_match = MTRR_TYPE_WRTHROUGH;
-			curr_match = MTRR_TYPE_WRTHROUGH;
+		/*
+		 * If two or more variable memory ranges match and the
+		 * memory types are WT and WB, the WT memory type is used.
+		 */
+		if (((1 << type) & wt_wb_mask) &&
+		      ((1 << curr_type) & wt_wb_mask)) {
+			type = MTRR_TYPE_WRTHROUGH;
+			continue;
 		}
 
-		if (prev_match != curr_match)
-			return MTRR_TYPE_UNCACHABLE;
+		/*
+		 * For overlaps not defined by the above rules, processor
+		 * behavior is undefined.
+		 */
+
+		/* We use WB for this undefined behavior. :( */
+		return MTRR_TYPE_WRBACK;
 	}
 
-	if (prev_match != 0xFF)
-		return prev_match;
+	if (type != -1)
+		return type;
 
 	return mtrr_default_type(mtrr_state);
 }
-
-u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
-{
-	u8 mtrr;
-
-	mtrr = get_mtrr_type(&vcpu->arch.mtrr_state, gfn << PAGE_SHIFT,
-			     (gfn << PAGE_SHIFT) + PAGE_SIZE);
-	if (mtrr == 0xfe || mtrr == 0xff)
-		mtrr = MTRR_TYPE_WRBACK;
-	return mtrr;
-}
 EXPORT_SYMBOL_GPL(kvm_mtrr_get_guest_memory_type);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 09/15] KVM: MTRR: introduce fixed_mtrr_segment table
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (7 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 08/15] KVM: MTRR: improve kvm_mtrr_get_guest_memory_type Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 10/15] KVM: MTRR: introduce var_mtrr_range Xiao Guangrong
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

This table summarizes the information of fixed MTRRs and introduce some APIs
to abstract its operation which helps us to clean up the code and will be
used in later patches

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 200 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 147 insertions(+), 53 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index 2ea1213..df73149 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -120,12 +120,132 @@ static u8 mtrr_default_type(struct kvm_mtrr *mtrr_state)
 	return mtrr_state->deftype & IA32_MTRR_DEF_TYPE_TYPE_MASK;
 }
 
+/*
+* Three terms are used in the following code:
+* - segment, it indicates the address segments covered by fixed MTRRs.
+* - unit, it corresponds to the MSR entry in the segment.
+* - range, a range is covered in one memory cache type.
+*/
+struct fixed_mtrr_segment {
+	u64 start;
+	u64 end;
+
+	u64 range_size;
+
+	/* the start position in kvm_mtrr.fixed_ranges[]. */
+	int range_start;
+};
+
+static struct fixed_mtrr_segment fixed_seg_table[] = {
+	/* MSR_MTRRfix64K_00000, 1 unit. 64K fixed mtrr. */
+	{
+		.start = 0x0,
+		.end = 0x80000,
+		.range_size = 64 * 1024,
+		.range_start = 0,
+	},
+
+	/*
+	 * MSR_MTRRfix16K_80000 ... MSR_MTRRfix16K_A0000, 2 units,
+	 * 16K fixed mtrr.
+	 */
+	{
+		.start = 0x80000,
+		.end = 0xc0000,
+		.range_size = 16 * 1024,
+		.range_start = 8,
+	},
+
+	/*
+	 * MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000, 8 units,
+	 * 4K fixed mtrr.
+	 */
+	{
+		.start = 0xc0000,
+		.end = 0x100000,
+		.range_size = 4 * 1024,
+		.range_start = 24,
+	}
+};
+
+/*
+ * The size of unit is covered in one MSR, one MSR entry contains
+ * 8 ranges so that unit size is always range_size * 8.
+ */
+static u64 fixed_mtrr_seg_unit_size(int seg)
+{
+	return fixed_seg_table[seg].range_size * 8;
+}
+
+static bool fixed_msr_to_seg_unit(u32 msr, int *seg, int *unit)
+{
+	switch (msr) {
+	case MSR_MTRRfix64K_00000:
+		*seg = 0;
+		*unit = 0;
+		break;
+	case MSR_MTRRfix16K_80000 ... MSR_MTRRfix16K_A0000:
+		*seg = 1;
+		*unit = msr - MSR_MTRRfix16K_80000;
+		break;
+	case MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000:
+		*seg = 2;
+		*unit = msr - MSR_MTRRfix4K_C0000;
+		break;
+	default:
+		return false;
+	}
+
+	return true;
+}
+
+static void fixed_mtrr_seg_unit_range(int seg, int unit, u64 *start, u64 *end)
+{
+	struct fixed_mtrr_segment *mtrr_seg = &fixed_seg_table[seg];
+	u64 unit_size = fixed_mtrr_seg_unit_size(seg);
+
+	*start = mtrr_seg->start + unit * unit_size;
+	*end = *start + unit_size;
+	WARN_ON(*end > mtrr_seg->end);
+}
+
+static int fixed_mtrr_seg_unit_range_index(int seg, int unit)
+{
+	struct fixed_mtrr_segment *mtrr_seg = &fixed_seg_table[seg];
+
+	WARN_ON(mtrr_seg->start + unit * fixed_mtrr_seg_unit_size(seg)
+		> mtrr_seg->end);
+
+	/* each unit has 8 ranges. */
+	return mtrr_seg->range_start + 8 * unit;
+}
+
+static bool fixed_msr_to_range(u32 msr, u64 *start, u64 *end)
+{
+	int seg, unit;
+
+	if (!fixed_msr_to_seg_unit(msr, &seg, &unit))
+		return false;
+
+	fixed_mtrr_seg_unit_range(seg, unit, start, end);
+	return true;
+}
+
+static int fixed_msr_to_range_index(u32 msr)
+{
+	int seg, unit;
+
+	if (!fixed_msr_to_seg_unit(msr, &seg, &unit))
+		return -1;
+
+	return fixed_mtrr_seg_unit_range_index(seg, unit);
+}
+
 static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
 	gfn_t start, end, mask;
 	int index;
-	bool is_fixed = true;
 
 	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
 	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
@@ -134,32 +254,15 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 	if (!mtrr_is_enabled(mtrr_state) && msr != MSR_MTRRdefType)
 		return;
 
-	switch (msr) {
-	case MSR_MTRRfix64K_00000:
-		start = 0x0;
-		end = 0x80000;
-		break;
-	case MSR_MTRRfix16K_80000:
-		start = 0x80000;
-		end = 0xa0000;
-		break;
-	case MSR_MTRRfix16K_A0000:
-		start = 0xa0000;
-		end = 0xc0000;
-		break;
-	case MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000:
-		index = msr - MSR_MTRRfix4K_C0000;
-		start = 0xc0000 + index * (32 << 10);
-		end = start + (32 << 10);
-		break;
-	case MSR_MTRRdefType:
-		is_fixed = false;
+	/* fixed MTRRs. */
+	if (fixed_msr_to_range(msr, &start, &end)) {
+		if (!fixed_mtrr_is_enabled(mtrr_state))
+			return;
+	} else if (msr == MSR_MTRRdefType) {
 		start = 0x0;
 		end = ~0ULL;
-		break;
-	default:
+	} else {
 		/* variable range MTRRs. */
-		is_fixed = false;
 		index = (msr - 0x200) / 2;
 		start = mtrr_state->var_ranges[index].base & PAGE_MASK;
 		mask = mtrr_state->var_ranges[index].mask & PAGE_MASK;
@@ -168,38 +271,32 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 		end = ((start & mask) | ~mask) + 1;
 	}
 
-	if (is_fixed && !fixed_mtrr_is_enabled(mtrr_state))
-		return;
-
 	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
 }
 
 int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
-	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
+	int index;
 
 	if (!kvm_mtrr_valid(vcpu, msr, data))
 		return 1;
 
-	if (msr == MSR_MTRRdefType)
+	index = fixed_msr_to_range_index(msr);
+	if (index >= 0)
+		*(u64 *)&vcpu->arch.mtrr_state.fixed_ranges[index] = data;
+	else if (msr == MSR_MTRRdefType)
 		vcpu->arch.mtrr_state.deftype = data;
-	else if (msr == MSR_MTRRfix64K_00000)
-		p[0] = data;
-	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
-		p[1 + msr - MSR_MTRRfix16K_80000] = data;
-	else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
-		p[3 + msr - MSR_MTRRfix4K_C0000] = data;
 	else if (msr == MSR_IA32_CR_PAT)
 		vcpu->arch.pat = data;
 	else {	/* Variable MTRRs */
-		int idx, is_mtrr_mask;
+		int is_mtrr_mask;
 
-		idx = (msr - 0x200) / 2;
-		is_mtrr_mask = msr - 0x200 - 2 * idx;
+		index = (msr - 0x200) / 2;
+		is_mtrr_mask = msr - 0x200 - 2 * index;
 		if (!is_mtrr_mask)
-			vcpu->arch.mtrr_state.var_ranges[idx].base = data;
+			vcpu->arch.mtrr_state.var_ranges[index].base = data;
 		else
-			vcpu->arch.mtrr_state.var_ranges[idx].mask = data;
+			vcpu->arch.mtrr_state.var_ranges[index].mask = data;
 	}
 
 	update_mtrr(vcpu, msr);
@@ -208,7 +305,7 @@ 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)
 {
-	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
+	int index;
 
 	/* MSR_MTRRcap is a readonly MSR. */
 	if (msr == MSR_MTRRcap) {
@@ -225,25 +322,22 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	if (!msr_mtrr_valid(msr))
 		return 1;
 
-	if (msr == MSR_MTRRdefType)
+	index = fixed_msr_to_range_index(msr);
+	if (index >= 0)
+		*pdata = *(u64 *)&vcpu->arch.mtrr_state.fixed_ranges[index];
+	else if (msr == MSR_MTRRdefType)
 		*pdata = vcpu->arch.mtrr_state.deftype;
-	else if (msr == MSR_MTRRfix64K_00000)
-		*pdata = p[0];
-	else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
-		*pdata = p[1 + msr - MSR_MTRRfix16K_80000];
-	else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
-		*pdata = p[3 + msr - MSR_MTRRfix4K_C0000];
 	else if (msr == MSR_IA32_CR_PAT)
 		*pdata = vcpu->arch.pat;
 	else {	/* Variable MTRRs */
-		int idx, is_mtrr_mask;
+		int is_mtrr_mask;
 
-		idx = (msr - 0x200) / 2;
-		is_mtrr_mask = msr - 0x200 - 2 * idx;
+		index = (msr - 0x200) / 2;
+		is_mtrr_mask = msr - 0x200 - 2 * index;
 		if (!is_mtrr_mask)
-			*pdata = vcpu->arch.mtrr_state.var_ranges[idx].base;
+			*pdata = vcpu->arch.mtrr_state.var_ranges[index].base;
 		else
-			*pdata = vcpu->arch.mtrr_state.var_ranges[idx].mask;
+			*pdata = vcpu->arch.mtrr_state.var_ranges[index].mask;
 	}
 
 	return 0;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 10/15] KVM: MTRR: introduce var_mtrr_range
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (8 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 09/15] KVM: MTRR: introduce fixed_mtrr_segment table Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-17 15:38   ` Paolo Bonzini
  2015-06-15  8:55 ` [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs Xiao Guangrong
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

It gets the range for the specified variable MTRR

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index df73149..cb9702d 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -241,10 +241,21 @@ static int fixed_msr_to_range_index(u32 msr)
 	return fixed_mtrr_seg_unit_range_index(seg, unit);
 }
 
+static void var_mtrr_range(struct kvm_mtrr_range *range, u64 *start, u64 *end)
+{
+	u64 mask;
+
+	*start = range->base & PAGE_MASK;
+
+	mask = range->mask & PAGE_MASK;
+	mask |= ~0ULL << boot_cpu_data.x86_phys_bits;
+	*end = ((*start & mask) | ~mask) + 1;
+}
+
 static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
-	gfn_t start, end, mask;
+	gfn_t start, end;
 	int index;
 
 	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
@@ -264,11 +275,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 	} else {
 		/* variable range MTRRs. */
 		index = (msr - 0x200) / 2;
-		start = mtrr_state->var_ranges[index].base & PAGE_MASK;
-		mask = mtrr_state->var_ranges[index].mask & PAGE_MASK;
-		mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
-
-		end = ((start & mask) | ~mask) + 1;
+		var_mtrr_range(&mtrr_state->var_ranges[index], &start, &end);
 	}
 
 	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (9 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 10/15] KVM: MTRR: introduce var_mtrr_range Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-17 15:40   ` Paolo Bonzini
  2015-06-17 16:11   ` Paolo Bonzini
  2015-06-15  8:55 ` [PATCH v2 12/15] KVM: MTRR: introduce fixed_mtrr_addr_* functions Xiao Guangrong
                   ` (3 subsequent siblings)
  14 siblings, 2 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Sort all valid variable MTRRs based on its base address, it will help us to
check a range to see if it's fully contained in variable MTRRs

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |  3 ++
 arch/x86/kvm/mtrr.c             | 63 ++++++++++++++++++++++++++++++++++-------
 arch/x86/kvm/x86.c              |  2 +-
 arch/x86/kvm/x86.h              |  1 +
 4 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f735548..f2d60cc 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -345,12 +345,15 @@ enum {
 struct kvm_mtrr_range {
 	u64 base;
 	u64 mask;
+	struct list_head node;
 };
 
 struct kvm_mtrr {
 	struct kvm_mtrr_range var_ranges[KVM_NR_VAR_MTRR];
 	mtrr_type fixed_ranges[KVM_NR_FIXED_MTRR_REGION];
 	u64 deftype;
+
+	struct list_head head;
 };
 
 struct kvm_vcpu_arch {
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index cb9702d..c06ec13 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -281,6 +281,52 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
 	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
 }
 
+static bool var_mtrr_range_is_valid(struct kvm_mtrr_range *range)
+{
+	u64 start, end;
+
+	if (!(range->mask & (1 << 11)))
+		return false;
+
+	var_mtrr_range(range, &start, &end);
+	return end > start;
+}
+
+static void set_var_mtrr_start(struct kvm_mtrr *mtrr_state, int index)
+{
+	/* remove the entry if it's in the list. */
+	if (var_mtrr_range_is_valid(&mtrr_state->var_ranges[index]))
+		list_del(&mtrr_state->var_ranges[index].node);
+}
+
+static void set_var_mtrr_end(struct kvm_mtrr *mtrr_state, int index)
+{
+	struct kvm_mtrr_range *tmp, *cur = &mtrr_state->var_ranges[index];
+
+	/* add it to the list if it's valid. */
+	if (var_mtrr_range_is_valid(&mtrr_state->var_ranges[index])) {
+		list_for_each_entry(tmp, &mtrr_state->head, node)
+			if (cur->base < tmp->base)
+				list_add_tail(&cur->node, &tmp->node);
+
+		list_add_tail(&cur->node, &mtrr_state->head);
+	}
+}
+
+static void set_var_mtrr_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+	int index, is_mtrr_mask;
+
+	index = (msr - 0x200) / 2;
+	is_mtrr_mask = msr - 0x200 - 2 * index;
+	set_var_mtrr_start(&vcpu->arch.mtrr_state, index);
+	if (!is_mtrr_mask)
+		vcpu->arch.mtrr_state.var_ranges[index].base = data;
+	else
+		vcpu->arch.mtrr_state.var_ranges[index].mask = data;
+	set_var_mtrr_end(&vcpu->arch.mtrr_state, index);
+}
+
 int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	int index;
@@ -295,16 +341,8 @@ int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		vcpu->arch.mtrr_state.deftype = data;
 	else if (msr == MSR_IA32_CR_PAT)
 		vcpu->arch.pat = data;
-	else {	/* Variable MTRRs */
-		int is_mtrr_mask;
-
-		index = (msr - 0x200) / 2;
-		is_mtrr_mask = msr - 0x200 - 2 * index;
-		if (!is_mtrr_mask)
-			vcpu->arch.mtrr_state.var_ranges[index].base = data;
-		else
-			vcpu->arch.mtrr_state.var_ranges[index].mask = data;
-	}
+	else
+		set_var_mtrr_msr(vcpu, msr, data);
 
 	update_mtrr(vcpu, msr);
 	return 0;
@@ -350,6 +388,11 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	return 0;
 }
 
+void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu)
+{
+	INIT_LIST_HEAD(&vcpu->arch.mtrr_state.head);
+}
+
 u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2ffad7f..6574fa3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7379,13 +7379,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
 	int r;
 
+	kvm_vcpu_mtrr_init(vcpu);
 	r = vcpu_load(vcpu);
 	if (r)
 		return r;
 	kvm_vcpu_reset(vcpu, false);
 	kvm_mmu_setup(vcpu);
 	vcpu_put(vcpu);
-
 	return r;
 }
 
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index aeb0bb2..0e4727c 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -162,6 +162,7 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
 	gva_t addr, void *val, unsigned int bytes,
 	struct x86_exception *exception);
 
+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);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 12/15] KVM: MTRR: introduce fixed_mtrr_addr_* functions
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (10 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 13/15] KVM: MTRR: introduce mtrr_for_each_mem_type Xiao Guangrong
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Two functions are introduced:
- fixed_mtrr_addr_to_seg() translates the address to the fixed
  MTRR segment

- fixed_mtrr_addr_seg_to_range_index() translates the address to
  the index of kvm_mtrr.fixed_ranges[]

They will be used in the later patch

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index c06ec13..e5e16a1 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -241,6 +241,31 @@ static int fixed_msr_to_range_index(u32 msr)
 	return fixed_mtrr_seg_unit_range_index(seg, unit);
 }
 
+static int fixed_mtrr_addr_to_seg(u64 addr)
+{
+	struct fixed_mtrr_segment *mtrr_seg;
+	int seg, seg_num = ARRAY_SIZE(fixed_seg_table);
+
+	for (seg = 0; seg < seg_num; seg++) {
+		mtrr_seg = &fixed_seg_table[seg];
+		if (mtrr_seg->start >= addr && addr < mtrr_seg->end)
+			return seg;
+	}
+
+	return -1;
+}
+
+static int fixed_mtrr_addr_seg_to_range_index(u64 addr, int seg)
+{
+	struct fixed_mtrr_segment *mtrr_seg;
+	int index;
+
+	mtrr_seg = &fixed_seg_table[seg];
+	index = mtrr_seg->range_start;
+	index += (addr - mtrr_seg->start) / mtrr_seg->range_size;
+	return index;
+}
+
 static void var_mtrr_range(struct kvm_mtrr_range *range, u64 *start, u64 *end)
 {
 	u64 mask;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 13/15] KVM: MTRR: introduce mtrr_for_each_mem_type
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (11 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 12/15] KVM: MTRR: introduce fixed_mtrr_addr_* functions Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 14/15] KVM: MTRR: simplify kvm_mtrr_get_guest_memory_type Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 15/15] KVM: MTRR: do not map huage page for non-consistent range Xiao Guangrong
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

It walks all MTRRs and gets all the memory cache type setting for the
specified range also it checks if the range is fully covered by MTRRs

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index e5e16a1..10f0148 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -220,6 +220,15 @@ static int fixed_mtrr_seg_unit_range_index(int seg, int unit)
 	return mtrr_seg->range_start + 8 * unit;
 }
 
+static int fixed_mtrr_seg_end_range_index(int seg)
+{
+	struct fixed_mtrr_segment *mtrr_seg = &fixed_seg_table[seg];
+	int n;
+
+	n = (mtrr_seg->end - mtrr_seg->start) / mtrr_seg->range_size;
+	return mtrr_seg->range_start + n - 1;
+}
+
 static bool fixed_msr_to_range(u32 msr, u64 *start, u64 *end)
 {
 	int seg, unit;
@@ -266,6 +275,14 @@ static int fixed_mtrr_addr_seg_to_range_index(u64 addr, int seg)
 	return index;
 }
 
+static u64 fixed_mtrr_range_end_addr(int seg, int index)
+{
+	struct fixed_mtrr_segment *mtrr_seg = &fixed_seg_table[seg];
+	int pos = index - mtrr_seg->range_start;
+
+	return mtrr_seg->start + mtrr_seg->range_size * (pos + 1);
+}
+
 static void var_mtrr_range(struct kvm_mtrr_range *range, u64 *start, u64 *end)
 {
 	u64 mask;
@@ -418,6 +435,177 @@ void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu)
 	INIT_LIST_HEAD(&vcpu->arch.mtrr_state.head);
 }
 
+struct mtrr_iter {
+	/* input fields. */
+	struct kvm_mtrr *mtrr_state;
+	u64 start;
+	u64 end;
+
+	/* output fields. */
+	int mem_type;
+	/* [start, end) is not fully covered in MTRRs? */
+	bool partial_map;
+
+	/* private fields. */
+	union {
+		/* used for fixed MTRRs. */
+		struct {
+			int index;
+			int seg;
+		};
+
+		/* used for var MTRRs. */
+		struct {
+			struct kvm_mtrr_range *range;
+			/* max address has been covered in var MTRRs. */
+			u64 start_max;
+		};
+	};
+
+	bool fixed;
+};
+
+static bool mtrr_lookup_fixed_start(struct mtrr_iter *iter)
+{
+	int seg, index;
+
+	if (!fixed_mtrr_is_enabled(iter->mtrr_state))
+		return false;
+
+	seg = fixed_mtrr_addr_to_seg(iter->start);
+	if (seg < 0)
+		return false;
+
+	iter->fixed = true;
+	index = fixed_mtrr_addr_seg_to_range_index(iter->start, seg);
+	iter->index = index;
+	iter->seg = seg;
+	return true;
+}
+
+static bool match_var_range(struct mtrr_iter *iter,
+			    struct kvm_mtrr_range *range)
+{
+	u64 start, end;
+
+	var_mtrr_range(range, &start, &end);
+	if (!(start >= iter->end || end <= iter->start)) {
+		iter->range = range;
+
+		/*
+		 * the function is called when we do kvm_mtrr.head walking.
+		 * Range has the minimum base address which interleaves
+		 * [looker->start_max, looker->end).
+		 */
+		iter->partial_map |= iter->start_max < start;
+
+		/* update the max address has been covered. */
+		iter->start_max = max(iter->start_max, end);
+		return true;
+	}
+
+	return false;
+}
+
+static void __mtrr_lookup_var_next(struct mtrr_iter *iter)
+{
+	struct kvm_mtrr *mtrr_state = iter->mtrr_state;
+
+	list_for_each_entry_continue(iter->range, &mtrr_state->head, node)
+		if (match_var_range(iter, iter->range))
+			return;
+
+	iter->range = NULL;
+	iter->partial_map |= iter->start_max < iter->end;
+}
+
+static void mtrr_lookup_var_start(struct mtrr_iter *iter)
+{
+	struct kvm_mtrr *mtrr_state = iter->mtrr_state;
+
+	iter->fixed = false;
+	iter->start_max = iter->start;
+	iter->range = list_prepare_entry(iter->range, &mtrr_state->head, node);
+
+	__mtrr_lookup_var_next(iter);
+}
+
+static void mtrr_lookup_fixed_next(struct mtrr_iter *iter)
+{
+	/* terminate the lookup. */
+	if (fixed_mtrr_range_end_addr(iter->seg, iter->index) >= iter->end) {
+		iter->fixed = false;
+		iter->range = NULL;
+		return;
+	}
+
+	iter->index++;
+
+	/* have looked up for all fixed MTRRs. */
+	if (iter->index >= ARRAY_SIZE(iter->mtrr_state->fixed_ranges))
+		return mtrr_lookup_var_start(iter);
+
+	/* switch to next segment. */
+	if (iter->index > fixed_mtrr_seg_end_range_index(iter->seg))
+		iter->seg++;
+}
+
+static void mtrr_lookup_var_next(struct mtrr_iter *iter)
+{
+	__mtrr_lookup_var_next(iter);
+}
+
+static void mtrr_lookup_start(struct mtrr_iter *iter)
+{
+	if (!mtrr_is_enabled(iter->mtrr_state)) {
+		iter->partial_map = true;
+		return;
+	}
+
+	if (!mtrr_lookup_fixed_start(iter))
+		mtrr_lookup_var_start(iter);
+}
+
+static void mtrr_lookup_init(struct mtrr_iter *iter,
+			     struct kvm_mtrr *mtrr_state, u64 start, u64 end)
+{
+	iter->mtrr_state = mtrr_state;
+	iter->start = start;
+	iter->end = end;
+	iter->partial_map = false;
+	iter->fixed = false;
+	iter->range = NULL;
+
+	mtrr_lookup_start(iter);
+}
+
+static bool mtrr_lookup_okay(struct mtrr_iter *iter)
+{
+	if (iter->fixed) {
+		iter->mem_type = iter->mtrr_state->fixed_ranges[iter->index];
+		return true;
+	}
+
+	if (iter->range) {
+		iter->mem_type = iter->range->base & 0xff;
+		return true;
+	}
+
+	return false;
+}
+
+static void mtrr_lookup_next(struct mtrr_iter *iter)
+{
+	if (iter->fixed)
+		mtrr_lookup_fixed_next(iter);
+	else
+		mtrr_lookup_var_next(iter);
+}
+
+#define mtrr_for_each_mem_type(_iter_, _mtrr_, _gpa_start_, _gpa_end_) \
+	for (mtrr_lookup_init(_iter_, _mtrr_, _gpa_start_, _gpa_end_); \
+	     mtrr_lookup_okay(_iter_); mtrr_lookup_next(_iter_))
+
 u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 14/15] KVM: MTRR: simplify kvm_mtrr_get_guest_memory_type
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (12 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 13/15] KVM: MTRR: introduce mtrr_for_each_mem_type Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  2015-06-15  8:55 ` [PATCH v2 15/15] KVM: MTRR: do not map huage page for non-consistent range Xiao Guangrong
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

mtrr_for_each_mem_type() is ready now, use it to simplify
kvm_mtrr_get_guest_memory_type()

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mtrr.c | 64 ++++++++++++++---------------------------------------
 1 file changed, 16 insertions(+), 48 deletions(-)

diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index 10f0148..097d616 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -609,61 +609,23 @@ static void mtrr_lookup_next(struct mtrr_iter *iter)
 u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
 	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
-	u64 base, mask, start;
-	int i, num_var_ranges, type;
+	struct mtrr_iter iter;
+	u64 start, end;
+	int type = -1;
 	const int wt_wb_mask = (1 << MTRR_TYPE_WRBACK)
 			       | (1 << MTRR_TYPE_WRTHROUGH);
 
 	start = gfn_to_gpa(gfn);
-	num_var_ranges = KVM_NR_VAR_MTRR;
-	type = -1;
-
-	/* MTRR is completely disabled, use UC for all of physical memory. */
-	if (!mtrr_is_enabled(mtrr_state))
-		return MTRR_TYPE_UNCACHABLE;
-
-	/* Look in fixed ranges. Just return the type as per start */
-	if (fixed_mtrr_is_enabled(mtrr_state) && (start < 0x100000)) {
-		int idx;
-
-		if (start < 0x80000) {
-			idx = 0;
-			idx += (start >> 16);
-			return mtrr_state->fixed_ranges[idx];
-		} else if (start < 0xC0000) {
-			idx = 1 * 8;
-			idx += ((start - 0x80000) >> 14);
-			return mtrr_state->fixed_ranges[idx];
-		} else if (start < 0x1000000) {
-			idx = 3 * 8;
-			idx += ((start - 0xC0000) >> 12);
-			return mtrr_state->fixed_ranges[idx];
-		}
-	}
-
-	/*
-	 * Look in variable ranges
-	 * Look of multiple ranges matching this address and pick type
-	 * as per MTRR precedence
-	 */
-	for (i = 0; i < num_var_ranges; ++i) {
-		int curr_type;
-
-		if (!(mtrr_state->var_ranges[i].mask & (1 << 11)))
-			continue;
-
-		base = mtrr_state->var_ranges[i].base & PAGE_MASK;
-		mask = mtrr_state->var_ranges[i].mask & PAGE_MASK;
+	end = start + PAGE_SIZE;
 
-		if ((start & mask) != (base & mask))
-			continue;
+	mtrr_for_each_mem_type(&iter, mtrr_state, start, end) {
+		int curr_type = iter.mem_type;
 
 		/*
 		 * Please refer to Intel SDM Volume 3: 11.11.4.1 MTRR
 		 * Precedences.
 		 */
 
-		curr_type = mtrr_state->var_ranges[i].base & 0xff;
 		if (type == -1) {
 			type = curr_type;
 			continue;
@@ -703,9 +665,15 @@ u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
 		return MTRR_TYPE_WRBACK;
 	}
 
-	if (type != -1)
-		return type;
-
-	return mtrr_default_type(mtrr_state);
+	/* It is not covered by MTRRs. */
+	if (iter.partial_map) {
+		/*
+		 * We just check one page, partially covered by MTRRs is
+		 * impossible.
+		 */
+		WARN_ON(type != -1);
+		type = mtrr_default_type(mtrr_state);
+	}
+	return type;
 }
 EXPORT_SYMBOL_GPL(kvm_mtrr_get_guest_memory_type);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 15/15] KVM: MTRR: do not map huage page for non-consistent range
  2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
                   ` (13 preceding siblings ...)
  2015-06-15  8:55 ` [PATCH v2 14/15] KVM: MTRR: simplify kvm_mtrr_get_guest_memory_type Xiao Guangrong
@ 2015-06-15  8:55 ` Xiao Guangrong
  14 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-15  8:55 UTC (permalink / raw)
  To: pbonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack, Xiao Guangrong

Based on Intel's SDM, mapping huge page which do not have consistent
memory cache for each 4k page will cause undefined behavior

In order to avoiding this kind of undefined behavior, we force to use
4k pages under this case

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mmu.c  | 20 +++++++++++++++++++-
 arch/x86/kvm/mtrr.c | 29 +++++++++++++++++++++++++++++
 arch/x86/kvm/x86.h  |  2 ++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 532aad2..f807496 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3446,6 +3446,16 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,
 	return false;
 }
 
+static bool
+check_hugepage_cache_consistency(struct kvm_vcpu *vcpu, gfn_t gfn, int level)
+{
+	int page_num = KVM_PAGES_PER_HPAGE(level);
+
+	gfn &= ~(page_num - 1);
+
+	return kvm_mtrr_check_gfn_range_consistency(vcpu, gfn, page_num);
+}
+
 static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa, u32 error_code,
 			  bool prefault)
 {
@@ -3471,9 +3481,17 @@ static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa, u32 error_code,
 	if (r)
 		return r;
 
-	force_pt_level = mapping_level_dirty_bitmap(vcpu, gfn);
+	if (mapping_level_dirty_bitmap(vcpu, gfn) ||
+	    !check_hugepage_cache_consistency(vcpu, gfn, PT_DIRECTORY_LEVEL))
+		force_pt_level = 1;
+	else
+		force_pt_level = 0;
+
 	if (likely(!force_pt_level)) {
 		level = mapping_level(vcpu, gfn);
+		if (level > PT_DIRECTORY_LEVEL &&
+		    !check_hugepage_cache_consistency(vcpu, gfn, level))
+			level = PT_DIRECTORY_LEVEL;
 		gfn &= ~(KVM_PAGES_PER_HPAGE(level) - 1);
 	} else
 		level = PT_PAGE_TABLE_LEVEL;
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
index 097d616..5b26048 100644
--- a/arch/x86/kvm/mtrr.c
+++ b/arch/x86/kvm/mtrr.c
@@ -677,3 +677,32 @@ u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
 	return type;
 }
 EXPORT_SYMBOL_GPL(kvm_mtrr_get_guest_memory_type);
+
+bool kvm_mtrr_check_gfn_range_consistency(struct kvm_vcpu *vcpu, gfn_t gfn,
+					  int page_num)
+{
+	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
+	struct mtrr_iter iter;
+	u64 start, end;
+	int type = -1;
+
+	start = gfn_to_gpa(gfn);
+	end = gfn_to_gpa(gfn + page_num);
+	mtrr_for_each_mem_type(&iter, mtrr_state, start, end) {
+		if (type == -1) {
+			type = iter.mem_type;
+			continue;
+		}
+
+		if (type != iter.mem_type)
+			return false;
+	}
+
+	if (!iter.partial_map)
+		return true;
+
+	if (type == -1)
+		return true;
+
+	return type == mtrr_default_type(mtrr_state);
+}
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 0e4727c..edc8cdc 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -167,6 +167,8 @@ 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,
+					  int page_num);
 
 #define KVM_SUPPORTED_XCR0     (XSTATE_FP | XSTATE_SSE | XSTATE_YMM \
 				| XSTATE_BNDREGS | XSTATE_BNDCSR \
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 10/15] KVM: MTRR: introduce var_mtrr_range
  2015-06-15  8:55 ` [PATCH v2 10/15] KVM: MTRR: introduce var_mtrr_range Xiao Guangrong
@ 2015-06-17 15:38   ` Paolo Bonzini
  0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2015-06-17 15:38 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 15/06/2015 10:55, Xiao Guangrong wrote:
> It gets the range for the specified variable MTRR
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/kvm/mtrr.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
> index df73149..cb9702d 100644
> --- a/arch/x86/kvm/mtrr.c
> +++ b/arch/x86/kvm/mtrr.c
> @@ -241,10 +241,21 @@ static int fixed_msr_to_range_index(u32 msr)
>  	return fixed_mtrr_seg_unit_range_index(seg, unit);
>  }
>  
> +static void var_mtrr_range(struct kvm_mtrr_range *range, u64 *start, u64 *end)
> +{
> +	u64 mask;
> +
> +	*start = range->base & PAGE_MASK;
> +
> +	mask = range->mask & PAGE_MASK;
> +	mask |= ~0ULL << boot_cpu_data.x86_phys_bits;
> +	*end = ((*start & mask) | ~mask) + 1;

This is just (*start | ~mask) + 1.  I will adjust this.

Paolo

> +}
> +
>  static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>  {
>  	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
> -	gfn_t start, end, mask;
> +	gfn_t start, end;
>  	int index;
>  
>  	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
> @@ -264,11 +275,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>  	} else {
>  		/* variable range MTRRs. */
>  		index = (msr - 0x200) / 2;
> -		start = mtrr_state->var_ranges[index].base & PAGE_MASK;
> -		mask = mtrr_state->var_ranges[index].mask & PAGE_MASK;
> -		mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
> -
> -		end = ((start & mask) | ~mask) + 1;
> +		var_mtrr_range(&mtrr_state->var_ranges[index], &start, &end);
>  	}
>  
>  	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-15  8:55 ` [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs Xiao Guangrong
@ 2015-06-17 15:40   ` Paolo Bonzini
  2015-06-17 16:11   ` Paolo Bonzini
  1 sibling, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2015-06-17 15:40 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 15/06/2015 10:55, Xiao Guangrong wrote:
> Sort all valid variable MTRRs based on its base address, it will help us to
> check a range to see if it's fully contained in variable MTRRs
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  3 ++
>  arch/x86/kvm/mtrr.c             | 63 ++++++++++++++++++++++++++++++++++-------
>  arch/x86/kvm/x86.c              |  2 +-
>  arch/x86/kvm/x86.h              |  1 +
>  4 files changed, 58 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index f735548..f2d60cc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -345,12 +345,15 @@ enum {
>  struct kvm_mtrr_range {
>  	u64 base;
>  	u64 mask;
> +	struct list_head node;
>  };
>  
>  struct kvm_mtrr {
>  	struct kvm_mtrr_range var_ranges[KVM_NR_VAR_MTRR];
>  	mtrr_type fixed_ranges[KVM_NR_FIXED_MTRR_REGION];
>  	u64 deftype;
> +
> +	struct list_head head;
>  };
>  
>  struct kvm_vcpu_arch {
> diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
> index cb9702d..c06ec13 100644
> --- a/arch/x86/kvm/mtrr.c
> +++ b/arch/x86/kvm/mtrr.c
> @@ -281,6 +281,52 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>  	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
>  }
>  
> +static bool var_mtrr_range_is_valid(struct kvm_mtrr_range *range)
> +{
> +	u64 start, end;
> +
> +	if (!(range->mask & (1 << 11)))
> +		return false;
> +
> +	var_mtrr_range(range, &start, &end);
> +	return end > start;
> +}

I think this test is incorrect; it is always true unless end overflows
to zero, which cannot happen because writing an invalid value to the
MSR causes a #GP.

Paolo

> +static void set_var_mtrr_start(struct kvm_mtrr *mtrr_state, int index)
> +{
> +	/* remove the entry if it's in the list. */
> +	if (var_mtrr_range_is_valid(&mtrr_state->var_ranges[index]))
> +		list_del(&mtrr_state->var_ranges[index].node);
> +}
> +
> +static void set_var_mtrr_end(struct kvm_mtrr *mtrr_state, int index)
> +{
> +	struct kvm_mtrr_range *tmp, *cur = &mtrr_state->var_ranges[index];
> +
> +	/* add it to the list if it's valid. */
> +	if (var_mtrr_range_is_valid(&mtrr_state->var_ranges[index])) {
> +		list_for_each_entry(tmp, &mtrr_state->head, node)
> +			if (cur->base < tmp->base)
> +				list_add_tail(&cur->node, &tmp->node);
> +
> +		list_add_tail(&cur->node, &mtrr_state->head);
> +	}
> +}
> +
> +static void set_var_mtrr_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> +	int index, is_mtrr_mask;
> +
> +	index = (msr - 0x200) / 2;
> +	is_mtrr_mask = msr - 0x200 - 2 * index;
> +	set_var_mtrr_start(&vcpu->arch.mtrr_state, index);
> +	if (!is_mtrr_mask)
> +		vcpu->arch.mtrr_state.var_ranges[index].base = data;
> +	else
> +		vcpu->arch.mtrr_state.var_ranges[index].mask = data;
> +	set_var_mtrr_end(&vcpu->arch.mtrr_state, index);
> +}
> +
>  int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  {
>  	int index;
> @@ -295,16 +341,8 @@ int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  		vcpu->arch.mtrr_state.deftype = data;
>  	else if (msr == MSR_IA32_CR_PAT)
>  		vcpu->arch.pat = data;
> -	else {	/* Variable MTRRs */
> -		int is_mtrr_mask;
> -
> -		index = (msr - 0x200) / 2;
> -		is_mtrr_mask = msr - 0x200 - 2 * index;
> -		if (!is_mtrr_mask)
> -			vcpu->arch.mtrr_state.var_ranges[index].base = data;
> -		else
> -			vcpu->arch.mtrr_state.var_ranges[index].mask = data;
> -	}
> +	else
> +		set_var_mtrr_msr(vcpu, msr, data);
>  
>  	update_mtrr(vcpu, msr);
>  	return 0;
> @@ -350,6 +388,11 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>  	return 0;
>  }
>  
> +void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu)
> +{
> +	INIT_LIST_HEAD(&vcpu->arch.mtrr_state.head);
> +}
> +
>  u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn)
>  {
>  	struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2ffad7f..6574fa3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7379,13 +7379,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  {
>  	int r;
>  
> +	kvm_vcpu_mtrr_init(vcpu);
>  	r = vcpu_load(vcpu);
>  	if (r)
>  		return r;
>  	kvm_vcpu_reset(vcpu, false);
>  	kvm_mmu_setup(vcpu);
>  	vcpu_put(vcpu);
> -
>  	return r;
>  }
>  
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index aeb0bb2..0e4727c 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -162,6 +162,7 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
>  	gva_t addr, void *val, unsigned int bytes,
>  	struct x86_exception *exception);
>  
> +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);
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-15  8:55 ` [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs Xiao Guangrong
  2015-06-17 15:40   ` Paolo Bonzini
@ 2015-06-17 16:11   ` Paolo Bonzini
  2015-06-22 11:24       ` Paolo Bonzini
  1 sibling, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2015-06-17 16:11 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 15/06/2015 10:55, Xiao Guangrong wrote:
> +	/* add it to the list if it's valid. */
> +	if (var_mtrr_range_is_valid(&mtrr_state->var_ranges[index])) {
> +		list_for_each_entry(tmp, &mtrr_state->head, node)
> +			if (cur->base < tmp->base)
> +				list_add_tail(&cur->node, &tmp->node);
> +		list_add_tail(&cur->node, &mtrr_state->head);

Also, this loop looks weird.  Is this what you wanted?

        list_for_each_entry(tmp, &mtrr_state->head, node)
                if (cur->base >= tmp->base)
                        break;
        list_add_tail(&cur->node, &tmp->node);

If so, can you look at kvm/queue and see if it is okay for you (so that
we can get the series in 4.2)?

Paolo

> +	}
> +}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-17 16:11   ` Paolo Bonzini
@ 2015-06-22 11:24       ` Paolo Bonzini
  0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2015-06-22 11:24 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 17/06/2015 18:11, Paolo Bonzini wrote:
> Also, this loop looks weird.  Is this what you wanted?
> 
>         list_for_each_entry(tmp, &mtrr_state->head, node)
>                 if (cur->base >= tmp->base)
>                         break;
>         list_add_tail(&cur->node, &tmp->node);
> 
> If so, can you look at kvm/queue and see if it is okay for you (so that
> we can get the series in 4.2)?

Ping?

If I don't get testing results before Wednesday, I'll drop this series
from the 4.2 pull request.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
@ 2015-06-22 11:24       ` Paolo Bonzini
  0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2015-06-22 11:24 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 17/06/2015 18:11, Paolo Bonzini wrote:
> Also, this loop looks weird.  Is this what you wanted?
> 
>         list_for_each_entry(tmp, &mtrr_state->head, node)
>                 if (cur->base >= tmp->base)
>                         break;
>         list_add_tail(&cur->node, &tmp->node);
> 
> If so, can you look at kvm/queue and see if it is okay for you (so that
> we can get the series in 4.2)?

Ping?

If I don't get testing results before Wednesday, I'll drop this series
from the 4.2 pull request.

Paolo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-22 11:24       ` Paolo Bonzini
@ 2015-06-23  2:29         ` Xiao Guangrong
  -1 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-23  2:29 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 06/22/2015 07:24 PM, Paolo Bonzini wrote:
>
>
> On 17/06/2015 18:11, Paolo Bonzini wrote:
>> Also, this loop looks weird.  Is this what you wanted?
>>
>>          list_for_each_entry(tmp, &mtrr_state->head, node)
>>                  if (cur->base >= tmp->base)
>>                          break;
>>          list_add_tail(&cur->node, &tmp->node);
>>
>> If so, can you look at kvm/queue and see if it is okay for you (so that
>> we can get the series in 4.2)?
>
> Ping?
>
> If I don't get testing results before Wednesday, I'll drop this series
> from the 4.2 pull request.

Paolo, sorry for the delay. Your changes are good to me. Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
@ 2015-06-23  2:29         ` Xiao Guangrong
  0 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-23  2:29 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 06/22/2015 07:24 PM, Paolo Bonzini wrote:
>
>
> On 17/06/2015 18:11, Paolo Bonzini wrote:
>> Also, this loop looks weird.  Is this what you wanted?
>>
>>          list_for_each_entry(tmp, &mtrr_state->head, node)
>>                  if (cur->base >= tmp->base)
>>                          break;
>>          list_add_tail(&cur->node, &tmp->node);
>>
>> If so, can you look at kvm/queue and see if it is okay for you (so that
>> we can get the series in 4.2)?
>
> Ping?
>
> If I don't get testing results before Wednesday, I'll drop this series
> from the 4.2 pull request.

Paolo, sorry for the delay. Your changes are good to me. Thanks!
--
To unsubscribe from this list: send the line "unsubscribe kvm" in

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-23  2:29         ` Xiao Guangrong
  (?)
@ 2015-06-23  8:00         ` Paolo Bonzini
  2015-06-23  8:27           ` Xiao Guangrong
  -1 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2015-06-23  8:00 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 23/06/2015 04:29, Xiao Guangrong wrote:
>>>
>>>
>>> If so, can you look at kvm/queue and see if it is okay for you (so that
>>> we can get the series in 4.2)?
>>
>> Ping?
>>
>> If I don't get testing results before Wednesday, I'll drop this series
>> from the 4.2 pull request.
> 
> Paolo, sorry for the delay. Your changes are good to me. Thanks!

Is this a Tested-by? :)

Paolo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs
  2015-06-23  8:00         ` Paolo Bonzini
@ 2015-06-23  8:27           ` Xiao Guangrong
  0 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2015-06-23  8:27 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: gleb, mtosatti, kvm, linux-kernel, dmatlack



On 06/23/2015 04:00 PM, Paolo Bonzini wrote:
>
>
> On 23/06/2015 04:29, Xiao Guangrong wrote:
>>>>
>>>>
>>>> If so, can you look at kvm/queue and see if it is okay for you (so that
>>>> we can get the series in 4.2)?
>>>
>>> Ping?
>>>
>>> If I don't get testing results before Wednesday, I'll drop this series
>>> from the 4.2 pull request.
>>
>> Paolo, sorry for the delay. Your changes are good to me. Thanks!
>
> Is this a Tested-by? :)

Yes! :)

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-06-23  8:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-15  8:55 [PATCH v2 00/15] vMTRR bugfix and optimization Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 01/15] KVM: x86: fix CR0.CD virtualization Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 02/15] KVM: x86: move MTRR related code to a separate file Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 03/15] KVM: MTRR: handle MSR_MTRRcap in kvm_mtrr_get_msr Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 04/15] KVM: MTRR: remove mtrr_state.have_fixed Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 05/15] KVM: MTRR: exactly define the size of variable MTRRs Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 06/15] KVM: MTRR: clean up mtrr default type Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 07/15] KVM: MTRR: do not split 64 bits MSR content Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 08/15] KVM: MTRR: improve kvm_mtrr_get_guest_memory_type Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 09/15] KVM: MTRR: introduce fixed_mtrr_segment table Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 10/15] KVM: MTRR: introduce var_mtrr_range Xiao Guangrong
2015-06-17 15:38   ` Paolo Bonzini
2015-06-15  8:55 ` [PATCH v2 11/15] KVM: MTRR: sort variable MTRRs Xiao Guangrong
2015-06-17 15:40   ` Paolo Bonzini
2015-06-17 16:11   ` Paolo Bonzini
2015-06-22 11:24     ` Paolo Bonzini
2015-06-22 11:24       ` Paolo Bonzini
2015-06-23  2:29       ` Xiao Guangrong
2015-06-23  2:29         ` Xiao Guangrong
2015-06-23  8:00         ` Paolo Bonzini
2015-06-23  8:27           ` Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 12/15] KVM: MTRR: introduce fixed_mtrr_addr_* functions Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 13/15] KVM: MTRR: introduce mtrr_for_each_mem_type Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 14/15] KVM: MTRR: simplify kvm_mtrr_get_guest_memory_type Xiao Guangrong
2015-06-15  8:55 ` [PATCH v2 15/15] KVM: MTRR: do not map huage page for non-consistent range Xiao Guangrong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.