[PATCH 0/9] KVM: MTRR fixes and some cleanups

[PATCH 0/9] KVM: MTRR fixes and some cleanups

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

This are some MTRR bugs if legacy IOMMU device is used on Intel's CPU:
- In current code, whenever guest MTRR registers are changed
  kvm_mmu_reset_context is called to switch to the new root shadow page
  table, however, it's useless since:
  1) the cache type is not cached into shadow page's attribute so that the
     original root shadow page will be reused

  2) the cache type is set on the last spte, that means we should sync the
     last sptes when MTRR is changed

  We can fix it by dropping all the spte in the gfn range which is
  being updated by MTRR

- some bugs are in get_mtrr_type();
  1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
     MSR which completely control MTRR's enablement that means other bits are
     ignored if it is cleared

  2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit
     10 of IA32_MTRR_DEF_TYPE)
  
  3: if MTRR is disabled, UC is applied to all of physical memory rather than
     mtrr_state->def_type

- we need not to reset mmu once cache policy is changed since shadow page table
  does not virtualize any cache policy

Also, these are some cleanups to make current MMU code more cleaner and help
us fixing the bug more easier. 

Xiao Guangrong (9):
  KVM: MMU: fix decoding cache type from MTRR
  KVM: MMU: introduce slot_handle_level() and its helper
  KVM: MMU: use slot_handle_level and its helper to clean up the code
  KVM: MMU: introduce for_each_rmap_spte()
  KVM: MMU: KVM: introduce for_each_slot_rmap
  KVM: MMU: introduce kvm_zap_rmapp
  KVM: MMU: introduce kvm_zap_gfn_range()
  KVM: MMU: fix MTRR update
  KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed

 arch/x86/include/asm/kvm_host.h |   2 +
 arch/x86/kvm/mmu.c              | 407 ++++++++++++++++++++++------------------
 arch/x86/kvm/mmu.h              |   1 +
 arch/x86/kvm/mmu_audit.c        |   4 +-
 arch/x86/kvm/svm.c              |   5 +
 arch/x86/kvm/vmx.c              |  58 ++++++
 arch/x86/kvm/x86.c              |   5 +-
 7 files changed, 294 insertions(+), 188 deletions(-)

-- 
1.9.3

[PATCH 1/9] KVM: MMU: fix decoding cache type from MTRR

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

There are some bugs in current get_mtrr_type();
1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
   MSR which completely control MTRR's enablement that means other bits are
   ignored if it is cleared

2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit 10
   of IA32_MTRR_DEF_TYPE)

3: if MTRR is disabled, UC is applied to all of physical memory rather than
   mtrr_state->def_type

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index d43867c..ea3e3e4 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2393,19 +2393,20 @@ EXPORT_SYMBOL_GPL(kvm_mmu_unprotect_page);
 static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
 			 u64 start, u64 end)
 {
-	int i;
 	u64 base, mask;
 	u8 prev_match, curr_match;
-	int num_var_ranges = KVM_NR_VAR_MTRR;
+	int i, num_var_ranges = KVM_NR_VAR_MTRR;
 
-	if (!mtrr_state->enabled)
-		return 0xFF;
+	/* 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 && (start < 0x100000)) {
+	if (mtrr_state->have_fixed && (mtrr_state->enabled & 0x1) &&
+	      (start < 0x100000)) {
 		int idx;
 
 		if (start < 0x80000) {
@@ -2428,9 +2429,6 @@ static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
 	 * Look of multiple ranges matching this address and pick type
 	 * as per MTRR precedence
 	 */
-	if (!(mtrr_state->enabled & 2))
-		return mtrr_state->def_type;
-
 	prev_match = 0xFF;
 	for (i = 0; i < num_var_ranges; ++i) {
 		unsigned short start_state, end_state;
-- 
1.9.3

[PATCH 2/9] KVM: MMU: introduce slot_handle_level() and its helper

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

There are several places walking all rmaps for the memslot so that
introduce common functions to cleanup the code

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ea3e3e4..75a3459 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4410,6 +4410,69 @@ void kvm_mmu_setup(struct kvm_vcpu *vcpu)
 	init_kvm_mmu(vcpu);
 }
 
+/* The return value indicates if tlb flush on all vcpus is needed. */
+typedef bool (*slot_level_handler) (struct kvm *kvm, unsigned long *rmap);
+
+/* The caller should hold mmu-lock before calling this function. */
+static bool
+slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		  slot_level_handler fn, int min_level, int max_level,
+		  bool lock_flush_tlb)
+{
+	unsigned long last_gfn;
+	bool flush = false;
+	int level;
+
+	last_gfn = memslot->base_gfn + memslot->npages - 1;
+
+	for (level = min_level; level <= max_level; ++level) {
+		unsigned long *rmapp;
+		unsigned long last_index, index;
+
+		rmapp = memslot->arch.rmap[level - PT_PAGE_TABLE_LEVEL];
+		last_index = gfn_to_index(last_gfn, memslot->base_gfn, level);
+
+		for (index = 0; index <= last_index; ++index, ++rmapp) {
+			if (*rmapp)
+				flush |= fn(kvm, rmapp);
+
+			if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
+				if (flush && lock_flush_tlb) {
+					kvm_flush_remote_tlbs(kvm);
+					flush = false;
+				}
+				cond_resched_lock(&kvm->mmu_lock);
+			}
+		}
+	}
+
+	return flush;
+}
+
+static bool
+slot_handle_all_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		      slot_level_handler fn, bool lock_flush_tlb)
+{
+	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
+		PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1, lock_flush_tlb);
+}
+
+static bool
+slot_handle_large_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+			slot_level_handler fn, bool lock_flush_tlb)
+{
+	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL + 1,
+		PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1, lock_flush_tlb);
+}
+
+static bool
+slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		 slot_level_handler fn, bool lock_flush_tlb)
+{
+	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
+				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
+}
+
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
 				      struct kvm_memory_slot *memslot)
 {
-- 
1.9.3

Re: [PATCH 2/9] KVM: MMU: introduce slot_handle_level() and its helper

[Paolo Bonzini]

On 30/04/2015 12:24, guangrong.xiao@linux.intel.com wrote:
> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> 
> There are several places walking all rmaps for the memslot so that
> introduce common functions to cleanup the code
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/kvm/mmu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index ea3e3e4..75a3459 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -4410,6 +4410,69 @@ void kvm_mmu_setup(struct kvm_vcpu *vcpu)
>  	init_kvm_mmu(vcpu);
>  }
>  
> +/* The return value indicates if tlb flush on all vcpus is needed. */
> +typedef bool (*slot_level_handler) (struct kvm *kvm, unsigned long *rmap);
> +
> +/* The caller should hold mmu-lock before calling this function. */
> +static bool
> +slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
> +		  slot_level_handler fn, int min_level, int max_level,
> +		  bool lock_flush_tlb)

Why not introduce for_each_slot_rmap first, instead of introducing one 
implementation first and then switching to another?  It's a small 
change to reorder the patches like that.  I think we should have three 
iterator macros:

#define for_each_rmap_spte(rmap, iter, spte)

#define for_each_slot_rmap(slot, min_level, max_level, iter, rmapp)

#define for_each_slot_rmap_range(slot, iter, min_level, max_level, \
				 start_gfn, end_gfn, iter, rmapp)

where the last two take care of initializing the walker/iterator in the 
first part of the "for".

This way, this function would be introduced immediately as this very 
readable code:

	struct slot_rmap_iterator iter;
	unsigned long *rmapp;
	bool flush = false;

	for_each_slot_rmap(memslot, min_level, max_level, &iter, rmapp) {
		if (*rmapp)
			flush |= fn(kvm, rmapp);

		if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
			if (flush && lock_flush_tlb) {
				kvm_flush_remote_tlbs(kvm);
				flush = false;
			}
			cond_resched_lock(&kvm->mmu_lock);
		}
	}

	/*
	 * What about adding this here: then callers that pass
	 * lock_flush_tlb == true need not care about the return
	 * value!
	 */
	if (flush && lock_flush_tlb) {
		kvm_flush_remote_tlbs(kvm);
		flush = false;
	}

	return flush;

In addition, some of these functions need to be marked always_inline I 
think; either slot_handle_level/slot_handle_*_level, or the
iterators/walkers.  Can you collect kvm.ko size for both cases?

Thanks,

Paolo

> +{
> +	unsigned long last_gfn;
> +	bool flush = false;
> +	int level;
> +
> +	last_gfn = memslot->base_gfn + memslot->npages - 1;
> +
> +	for (level = min_level; level <= max_level; ++level) {
> +		unsigned long *rmapp;
> +		unsigned long last_index, index;
> +
> +		rmapp = memslot->arch.rmap[level - PT_PAGE_TABLE_LEVEL];
> +		last_index = gfn_to_index(last_gfn, memslot->base_gfn, level);
> +
> +		for (index = 0; index <= last_index; ++index, ++rmapp) {
> +			if (*rmapp)
> +				flush |= fn(kvm, rmapp);
> +
> +			if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
> +				if (flush && lock_flush_tlb) {
> +					kvm_flush_remote_tlbs(kvm);
> +					flush = false;
> +				}
> +				cond_resched_lock(&kvm->mmu_lock);
> +			}
> +		}
> +	}
> +
> +	return flush;
> +}
> +
> +static bool
> +slot_handle_all_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
> +		      slot_level_handler fn, bool lock_flush_tlb)
> +{
> +	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
> +		PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1, lock_flush_tlb);
> +}
> +
> +static bool
> +slot_handle_large_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
> +			slot_level_handler fn, bool lock_flush_tlb)
> +{
> +	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL + 1,
> +		PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1, lock_flush_tlb);
> +}
> +
> +static bool
> +slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
> +		 slot_level_handler fn, bool lock_flush_tlb)
> +{
> +	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
> +				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
> +}
> +
>  void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
>  				      struct kvm_memory_slot *memslot)
>  {
> 

Re: [PATCH 2/9] KVM: MMU: introduce slot_handle_level() and its helper

[Xiao Guangrong]

On 05/07/2015 08:04 PM, Paolo Bonzini wrote:
>
>
> On 30/04/2015 12:24, guangrong.xiao@linux.intel.com wrote:
>> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>>
>> There are several places walking all rmaps for the memslot so that
>> introduce common functions to cleanup the code
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>>   arch/x86/kvm/mmu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index ea3e3e4..75a3459 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -4410,6 +4410,69 @@ void kvm_mmu_setup(struct kvm_vcpu *vcpu)
>>   	init_kvm_mmu(vcpu);
>>   }
>>
>> +/* The return value indicates if tlb flush on all vcpus is needed. */
>> +typedef bool (*slot_level_handler) (struct kvm *kvm, unsigned long *rmap);
>> +
>> +/* The caller should hold mmu-lock before calling this function. */
>> +static bool
>> +slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
>> +		  slot_level_handler fn, int min_level, int max_level,
>> +		  bool lock_flush_tlb)
>
> Why not introduce for_each_slot_rmap first, instead of introducing one
> implementation first and then switching to another?  It's a small
> change to reorder the patches like that.

Yes, it's better, will do it in v2.

> I think we should have three
> iterator macros:
>
> #define for_each_rmap_spte(rmap, iter, spte)
>
> #define for_each_slot_rmap(slot, min_level, max_level, iter, rmapp)
>
> #define for_each_slot_rmap_range(slot, iter, min_level, max_level, \
> 				 start_gfn, end_gfn, iter, rmapp)
>
> where the last two take care of initializing the walker/iterator in the
> first part of the "for".

Okay, i agree.

>
> This way, this function would be introduced immediately as this very
> readable code:
>
> 	struct slot_rmap_iterator iter;
> 	unsigned long *rmapp;
> 	bool flush = false;
>
> 	for_each_slot_rmap(memslot, min_level, max_level, &iter, rmapp) {
> 		if (*rmapp)
> 			flush |= fn(kvm, rmapp);
>
> 		if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
> 			if (flush && lock_flush_tlb) {
> 				kvm_flush_remote_tlbs(kvm);
> 				flush = false;
> 			}
> 			cond_resched_lock(&kvm->mmu_lock);
> 		}
> 	}
>
> 	/*
> 	 * What about adding this here: then callers that pass
> 	 * lock_flush_tlb == true need not care about the return
> 	 * value!
> 	 */
> 	if (flush && lock_flush_tlb) {
> 		kvm_flush_remote_tlbs(kvm);
> 		flush = false;
> 	}
>
> 	return flush;

Good idea.

>
> In addition, some of these functions need to be marked always_inline I
> think; either slot_handle_level/slot_handle_*_level, or the
> iterators/walkers.  Can you collect kvm.ko size for both cases?

After applying patch 1 ~ 5:

no inline:
$ size arch/x86/kvm/kvm.ko
    text    data     bss     dec     hex filename
  366406   51535     473  418414   6626e arch/x86/kvm/kvm.ko

inline:
$ size arch/x86/kvm/kvm.ko
    text    data     bss     dec     hex filename
  366638   51535     473  418646   66356 arch/x86/kvm/kvm.ko

Since there are static functions i prefer allowing GCC automatically
optimizes the code to marking always-inline.

[PATCH 3/9] KVM: MMU: use slot_handle_level and its helper to clean up the code

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

slot_handle_level and its helper functions are ready now, use them to
clean up the code

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 75a3459..fd13991 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4473,35 +4473,19 @@ slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
 				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
 }
 
+static bool slot_rmap_write_protect(struct kvm *kvm, unsigned long *rmapp)
+{
+	return __rmap_write_protect(kvm, rmapp, false);
+}
+
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
 				      struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	int i;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	for (i = PT_PAGE_TABLE_LEVEL;
-	     i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[i - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, i);
-
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= __rmap_write_protect(kvm, rmapp,
-						false);
-
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-				cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
-
+	flush = slot_handle_all_level(kvm, memslot, slot_rmap_write_protect,
+				      false);
 	spin_unlock(&kvm->mmu_lock);
 
 	/*
@@ -4564,59 +4548,23 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
 			struct kvm_memory_slot *memslot)
 {
-	bool flush = false;
-	unsigned long *rmapp;
-	unsigned long last_index, index;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	rmapp = memslot->arch.rmap[0];
-	last_index = gfn_to_index(memslot->base_gfn + memslot->npages - 1,
-				memslot->base_gfn, PT_PAGE_TABLE_LEVEL);
-
-	for (index = 0; index <= last_index; ++index, ++rmapp) {
-		if (*rmapp)
-			flush |= kvm_mmu_zap_collapsible_spte(kvm, rmapp);
-
-		if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
-			if (flush) {
-				kvm_flush_remote_tlbs(kvm);
-				flush = false;
-			}
-			cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
-
+	flush = slot_handle_leaf(kvm, memslot, kvm_mmu_zap_collapsible_spte,
+				 true);
 	if (flush)
 		kvm_flush_remote_tlbs(kvm);
-
 	spin_unlock(&kvm->mmu_lock);
 }
 
 void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm,
 				   struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	unsigned long *rmapp;
-	unsigned long last_index, index;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	rmapp = memslot->arch.rmap[PT_PAGE_TABLE_LEVEL - 1];
-	last_index = gfn_to_index(last_gfn, memslot->base_gfn,
-			PT_PAGE_TABLE_LEVEL);
-
-	for (index = 0; index <= last_index; ++index, ++rmapp) {
-		if (*rmapp)
-			flush |= __rmap_clear_dirty(kvm, rmapp);
-
-		if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-			cond_resched_lock(&kvm->mmu_lock);
-	}
-
+	flush = slot_handle_leaf(kvm, memslot, __rmap_clear_dirty, false);
 	spin_unlock(&kvm->mmu_lock);
 
 	lockdep_assert_held(&kvm->slots_lock);
@@ -4635,31 +4583,11 @@ EXPORT_SYMBOL_GPL(kvm_mmu_slot_leaf_clear_dirty);
 void kvm_mmu_slot_largepage_remove_write_access(struct kvm *kvm,
 					struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	int i;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	for (i = PT_PAGE_TABLE_LEVEL + 1; /* skip rmap for 4K page */
-	     i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[i - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, i);
-
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= __rmap_write_protect(kvm, rmapp,
-						false);
-
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-				cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
+	flush = slot_handle_large_level(kvm, memslot, slot_rmap_write_protect,
+					false);
 	spin_unlock(&kvm->mmu_lock);
 
 	/* see kvm_mmu_slot_remove_write_access */
@@ -4673,31 +4601,10 @@ EXPORT_SYMBOL_GPL(kvm_mmu_slot_largepage_remove_write_access);
 void kvm_mmu_slot_set_dirty(struct kvm *kvm,
 			    struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	int i;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	for (i = PT_PAGE_TABLE_LEVEL;
-	     i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[i - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, i);
-
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= __rmap_set_dirty(kvm, rmapp);
-
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-				cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
-
+	flush = slot_handle_all_level(kvm, memslot, __rmap_set_dirty, false);
 	spin_unlock(&kvm->mmu_lock);
 
 	lockdep_assert_held(&kvm->slots_lock);
-- 
1.9.3

[PATCH 4/9] KVM: MMU: introduce for_each_rmap_spte()

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It's used to walk all the sptes on the rmap to clean up the
code

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index fd13991..4d98c6c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1142,6 +1142,11 @@ static u64 *rmap_get_next(struct rmap_iterator *iter)
 	return NULL;
 }
 
+#define for_each_rmap_spte(_rmap_, _iter_, _spte_)			    \
+	   for (_spte_ = rmap_get_first(*_rmap_, _iter_);		    \
+		_spte_ && ({BUG_ON(!is_shadow_present_pte(*_spte_)); 1;});  \
+			_spte_ = rmap_get_next(_iter_))
+
 static void drop_spte(struct kvm *kvm, u64 *sptep)
 {
 	if (mmu_spte_clear_track_bits(sptep))
@@ -1205,12 +1210,8 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
 	struct rmap_iterator iter;
 	bool flush = false;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		flush |= spte_write_protect(kvm, sptep, pt_protect);
-		sptep = rmap_get_next(&iter);
-	}
 
 	return flush;
 }
@@ -1232,12 +1233,8 @@ static bool __rmap_clear_dirty(struct kvm *kvm, unsigned long *rmapp)
 	struct rmap_iterator iter;
 	bool flush = false;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		flush |= spte_clear_dirty(kvm, sptep);
-		sptep = rmap_get_next(&iter);
-	}
 
 	return flush;
 }
@@ -1259,12 +1256,8 @@ static bool __rmap_set_dirty(struct kvm *kvm, unsigned long *rmapp)
 	struct rmap_iterator iter;
 	bool flush = false;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		flush |= spte_set_dirty(kvm, sptep);
-		sptep = rmap_get_next(&iter);
-	}
 
 	return flush;
 }
@@ -1368,13 +1361,14 @@ static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	struct rmap_iterator iter;
 	int need_tlb_flush = 0;
 
-	while ((sptep = rmap_get_first(*rmapp, &iter))) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
+restart:
+	for_each_rmap_spte(rmapp, &iter, sptep) {
 		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx gfn %llx (%d)\n",
 			     sptep, *sptep, gfn, level);
 
 		drop_spte(kvm, sptep);
 		need_tlb_flush = 1;
+		goto restart;
 	}
 
 	return need_tlb_flush;
@@ -1394,8 +1388,8 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	WARN_ON(pte_huge(*ptep));
 	new_pfn = pte_pfn(*ptep);
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!is_shadow_present_pte(*sptep));
+restart:
+	for_each_rmap_spte(rmapp, &iter, sptep) {
 		rmap_printk("kvm_set_pte_rmapp: spte %p %llx gfn %llx (%d)\n",
 			     sptep, *sptep, gfn, level);
 
@@ -1403,7 +1397,7 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 
 		if (pte_write(*ptep)) {
 			drop_spte(kvm, sptep);
-			sptep = rmap_get_first(*rmapp, &iter);
+			goto restart;
 		} else {
 			new_spte = *sptep & ~PT64_BASE_ADDR_MASK;
 			new_spte |= (u64)new_pfn << PAGE_SHIFT;
@@ -1414,7 +1408,6 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 
 			mmu_spte_clear_track_bits(sptep);
 			mmu_spte_set(sptep, new_spte);
-			sptep = rmap_get_next(&iter);
 		}
 	}
 
@@ -1518,16 +1511,13 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
 
 	BUG_ON(!shadow_accessed_mask);
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;
-	     sptep = rmap_get_next(&iter)) {
-		BUG_ON(!is_shadow_present_pte(*sptep));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		if (*sptep & shadow_accessed_mask) {
 			young = 1;
 			clear_bit((ffs(shadow_accessed_mask) - 1),
 				 (unsigned long *)sptep);
 		}
-	}
+
 	trace_kvm_age_page(gfn, level, slot, young);
 	return young;
 }
@@ -1548,15 +1538,11 @@ static int kvm_test_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	if (!shadow_accessed_mask)
 		goto out;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;
-	     sptep = rmap_get_next(&iter)) {
-		BUG_ON(!is_shadow_present_pte(*sptep));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		if (*sptep & shadow_accessed_mask) {
 			young = 1;
 			break;
 		}
-	}
 out:
 	return young;
 }
@@ -2232,8 +2218,11 @@ static void kvm_mmu_unlink_parents(struct kvm *kvm, struct kvm_mmu_page *sp)
 	u64 *sptep;
 	struct rmap_iterator iter;
 
-	while ((sptep = rmap_get_first(sp->parent_ptes, &iter)))
+restart:
+	for_each_rmap_spte(&sp->parent_ptes, &iter, sptep) {
 		drop_parent_pte(sp, sptep);
+		goto restart;
+	}
 }
 
 static int mmu_zap_unsync_children(struct kvm *kvm,
@@ -4519,9 +4508,8 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 	pfn_t pfn;
 	struct kvm_mmu_page *sp;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+restart:
+	for_each_rmap_spte(rmapp, &iter, sptep) {
 		sp = page_header(__pa(sptep));
 		pfn = spte_to_pfn(*sptep);
 
@@ -4536,10 +4524,9 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 			!kvm_is_reserved_pfn(pfn) &&
 			PageTransCompound(pfn_to_page(pfn))) {
 			drop_spte(kvm, sptep);
-			sptep = rmap_get_first(*rmapp, &iter);
 			need_tlb_flush = 1;
-		} else
-			sptep = rmap_get_next(&iter);
+			goto restart;
+		}
 	}
 
 	return need_tlb_flush;
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 9ade5cf..368d534 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -197,13 +197,11 @@ static void audit_write_protection(struct kvm *kvm, struct kvm_mmu_page *sp)
 
 	rmapp = gfn_to_rmap(kvm, sp->gfn, PT_PAGE_TABLE_LEVEL);
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;
-	     sptep = rmap_get_next(&iter)) {
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		if (is_writable_pte(*sptep))
 			audit_printk(kvm, "shadow page has writable "
 				     "mappings: gfn %llx role %x\n",
 				     sp->gfn, sp->role.word);
-	}
 }
 
 static void audit_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
-- 
1.9.3

[PATCH 5/9] KVM: MMU: KVM: introduce for_each_slot_rmap

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It is used to clean up the code between kvm_handle_hva_range and
slot_handle_level, also it will be used by later patch

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 4d98c6c..fea1e83 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1417,6 +1417,63 @@ restart:
 	return 0;
 }
 
+struct slot_rmap_walker {
+	/* input fields. */
+	struct kvm_memory_slot *slot;
+	gfn_t start_gfn;
+	gfn_t end_gfn;
+	int start_level;
+	int end_level;
+
+	/* output fields. */
+	gfn_t gfn;
+	unsigned long *rmap;
+	int level;
+
+	/* private field. */
+	unsigned long *end_rmap;
+};
+
+static void rmap_walk_init_level(struct slot_rmap_walker *walker, int level)
+{
+	struct kvm_memory_slot *slot = walker->slot;
+
+	walker->level = level;
+	walker->gfn = walker->start_gfn;
+	walker->rmap = __gfn_to_rmap(walker->gfn, walker->level, slot);
+	walker->end_rmap = __gfn_to_rmap(walker->end_gfn, walker->level, slot);
+}
+
+static void slot_rmap_walk_init(struct slot_rmap_walker *walker)
+{
+	rmap_walk_init_level(walker, walker->start_level);
+}
+
+static bool slot_rmap_walk_okay(struct slot_rmap_walker *walker)
+{
+	return !!walker->rmap;
+}
+
+static void slot_rmap_walk_next(struct slot_rmap_walker *walker)
+{
+	if (++walker->rmap <= walker->end_rmap) {
+		walker->gfn += (1UL << KVM_HPAGE_GFN_SHIFT(walker->level));
+		return;
+	}
+
+	if (++walker->level > walker->end_level) {
+		walker->rmap = NULL;
+		return;
+	}
+
+	rmap_walk_init_level(walker, walker->level);
+}
+
+#define for_each_slot_rmap(_walker_)				\
+		for (slot_rmap_walk_init(_walker_);		\
+			slot_rmap_walk_okay(_walker_);		\
+				slot_rmap_walk_next(_walker_))
+
 static int kvm_handle_hva_range(struct kvm *kvm,
 				unsigned long start,
 				unsigned long end,
@@ -1428,10 +1485,10 @@ static int kvm_handle_hva_range(struct kvm *kvm,
 					       int level,
 					       unsigned long data))
 {
-	int j;
-	int ret = 0;
 	struct kvm_memslots *slots;
 	struct kvm_memory_slot *memslot;
+	struct slot_rmap_walker walker;
+	int ret = 0;
 
 	slots = kvm_memslots(kvm);
 
@@ -1451,26 +1508,18 @@ static int kvm_handle_hva_range(struct kvm *kvm,
 		gfn_start = hva_to_gfn_memslot(hva_start, memslot);
 		gfn_end = hva_to_gfn_memslot(hva_end + PAGE_SIZE - 1, memslot);
 
-		for (j = PT_PAGE_TABLE_LEVEL;
-		     j < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++j) {
-			unsigned long idx, idx_end;
-			unsigned long *rmapp;
-			gfn_t gfn = gfn_start;
-
-			/*
-			 * {idx(page_j) | page_j intersects with
-			 *  [hva_start, hva_end)} = {idx, idx+1, ..., idx_end}.
-			 */
-			idx = gfn_to_index(gfn_start, memslot->base_gfn, j);
-			idx_end = gfn_to_index(gfn_end - 1, memslot->base_gfn, j);
-
-			rmapp = __gfn_to_rmap(gfn_start, j, memslot);
-
-			for (; idx <= idx_end;
-			       ++idx, gfn += (1UL << KVM_HPAGE_GFN_SHIFT(j)))
-				ret |= handler(kvm, rmapp++, memslot,
-					       gfn, j, data);
-		}
+		walker = (struct slot_rmap_walker) {
+			.slot = memslot,
+			.start_gfn = gfn_start,
+			.end_gfn = gfn_end - 1,
+			.start_level = PT_PAGE_TABLE_LEVEL,
+			.end_level = PT_PAGE_TABLE_LEVEL +
+					KVM_NR_PAGE_SIZES - 1,
+		};
+
+		for_each_slot_rmap(&walker)
+			ret |= handler(kvm, walker.rmap, memslot, walker.gfn,
+				       walker.level, data);
 	}
 
 	return ret;
@@ -4404,34 +4453,29 @@ typedef bool (*slot_level_handler) (struct kvm *kvm, unsigned long *rmap);
 
 /* The caller should hold mmu-lock before calling this function. */
 static bool
-slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
-		  slot_level_handler fn, int min_level, int max_level,
-		  bool lock_flush_tlb)
+slot_handle_level_range(struct kvm *kvm, struct kvm_memory_slot *memslot,
+			 slot_level_handler fn, int min_level, int max_level,
+			 gfn_t start_gfn, gfn_t last_gfn, bool lock_flush_tlb)
 {
-	unsigned long last_gfn;
 	bool flush = false;
-	int level;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
-
-	for (level = min_level; level <= max_level; ++level) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[level - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, level);
+	struct slot_rmap_walker walker = {
+		.slot = memslot,
+		.start_gfn = start_gfn,
+		.end_gfn = last_gfn,
+		.start_level = min_level,
+		.end_level = max_level,
+	};
 
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= fn(kvm, rmapp);
+	for_each_slot_rmap(&walker) {
+		if (*walker.rmap)
+			flush |= fn(kvm, walker.rmap);
 
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
-				if (flush && lock_flush_tlb) {
-					kvm_flush_remote_tlbs(kvm);
-					flush = false;
-				}
-				cond_resched_lock(&kvm->mmu_lock);
+		if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
+			if (flush && lock_flush_tlb) {
+				kvm_flush_remote_tlbs(kvm);
+				flush = false;
 			}
+			cond_resched_lock(&kvm->mmu_lock);
 		}
 	}
 
@@ -4439,6 +4483,16 @@ slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
 }
 
 static bool
+slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		  slot_level_handler fn, int min_level, int max_level,
+		  bool lock_flush_tlb)
+{
+	return slot_handle_level_range(kvm, memslot, fn, min_level,
+		max_level, memslot->base_gfn,
+		memslot->base_gfn + memslot->npages - 1, lock_flush_tlb);
+}
+
+static bool
 slot_handle_all_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
 		      slot_level_handler fn, bool lock_flush_tlb)
 {
-- 
1.9.3

[PATCH 6/9] KVM: MMU: introduce kvm_zap_rmapp

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Split kvm_unmap_rmapp and introduce kvm_zap_rmapp which will be used in the
later patch

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index fea1e83..5d90a22 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1353,25 +1353,29 @@ static bool rmap_write_protect(struct kvm *kvm, u64 gfn)
 	return write_protected;
 }
 
-static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp,
-			   struct kvm_memory_slot *slot, gfn_t gfn, int level,
-			   unsigned long data)
+static bool kvm_zap_rmapp(struct kvm *kvm, unsigned long *rmapp)
 {
 	u64 *sptep;
 	struct rmap_iterator iter;
-	int need_tlb_flush = 0;
+	bool flush = false;
 
 restart:
 	for_each_rmap_spte(rmapp, &iter, sptep) {
-		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx gfn %llx (%d)\n",
-			     sptep, *sptep, gfn, level);
+		rmap_printk("%s: spte %p %llx.\n", __func__, sptep, *sptep);
 
 		drop_spte(kvm, sptep);
-		need_tlb_flush = 1;
+		flush = true;
 		goto restart;
 	}
 
-	return need_tlb_flush;
+	return flush;
+}
+
+static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp,
+			   struct kvm_memory_slot *slot, gfn_t gfn, int level,
+			   unsigned long data)
+{
+	return kvm_zap_rmapp(kvm, rmapp);
 }
 
 static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
-- 
1.9.3

[PATCH 7/9] KVM: MMU: introduce kvm_zap_gfn_range()

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It is used to zap all the rmaps of the specified gfn range and will
be used by the later patch

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5d90a22..aa9aad0 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4520,6 +4520,36 @@ slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
 				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
 }
 
+void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
+{
+	struct kvm_memslots *slots;
+	struct kvm_memory_slot *memslot;
+	bool flush = false;
+
+	slots = kvm_memslots(kvm);
+
+	spin_lock(&kvm->mmu_lock);
+	kvm_for_each_memslot(memslot, slots) {
+		gfn_t start, end;
+
+		start = max(gfn_start, memslot->base_gfn);
+		end = min(gfn_end, memslot->base_gfn + memslot->npages);
+		if (start >= end)
+			continue;
+
+		flush |= slot_handle_level_range(kvm, memslot, kvm_zap_rmapp,
+			      PT_PAGE_TABLE_LEVEL,
+			      PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1,
+			      start, end - 1, true);
+	}
+
+	if (flush)
+		kvm_flush_remote_tlbs(kvm);
+	spin_unlock(&kvm->mmu_lock);
+
+}
+EXPORT_SYMBOL_GPL(kvm_zap_gfn_range);
+
 static bool slot_rmap_write_protect(struct kvm *kvm, unsigned long *rmapp)
 {
 	return __rmap_write_protect(kvm, rmapp, false);
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index c7d6563..44b75f3 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -170,4 +170,5 @@ static inline bool permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 }
 
 void kvm_mmu_invalidate_zap_all_pages(struct kvm *kvm);
+void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end);
 #endif
-- 
1.9.3

[PATCH 8/9] KVM: MMU: fix MTRR update

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Currently, whenever guest MTRR registers are changed kvm_mmu_reset_context
is called to switch to the new root shadow page table, however, it's useless
since:
1) the cache type is not cached into shadow page's attribute so that the
   original root shadow page will be reused

2) the cache type is set on the last spte, that means we should sync the last
   sptes when MTRR is changed

This patch fixes this issue by dropping all the spte in the gfn range which is
being updated by MTRR

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/svm.c              |  5 ++++
 arch/x86/kvm/vmx.c              | 58 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              |  2 +-
 4 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index dea2e7e..ae9528d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -778,6 +778,8 @@ struct kvm_x86_ops {
 	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
 	int (*get_tdp_level)(void);
 	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
+	void (*set_msr_mtrr)(struct kvm_vcpu *vcpu, u32 msr);
+
 	int (*get_lpage_level)(void);
 	bool (*rdtscp_supported)(void);
 	bool (*invpcid_supported)(void);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ce741b8..c33573c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4078,6 +4078,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	return 0;
 }
 
+static void svm_set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr)
+{
+}
+
 static void svm_cpuid_update(struct kvm_vcpu *vcpu)
 {
 }
@@ -4410,6 +4414,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.set_tss_addr = svm_set_tss_addr,
 	.get_tdp_level = get_npt_level,
 	.get_mt_mask = svm_get_mt_mask,
+	.set_msr_mtrr = svm_set_msr_mtrr,
 
 	.get_exit_info = svm_get_exit_info,
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f7b6168..fcd0001 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8505,6 +8505,63 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	return ret;
 }
 
+static void vmx_set_msr_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 || !enable_ept ||
+	      !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 vmx_get_lpage_level(void)
 {
 	if (enable_ept && !cpu_has_vmx_ept_1g_page())
@@ -10218,6 +10275,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_tss_addr = vmx_set_tss_addr,
 	.get_tdp_level = get_ept_level,
 	.get_mt_mask = vmx_get_mt_mask,
+	.set_msr_mtrr = vmx_set_msr_mtrr,
 
 	.get_exit_info = vmx_get_exit_info,
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c73efcd..579c205 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1887,7 +1887,7 @@ static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		*pt = data;
 	}
 
-	kvm_mmu_reset_context(vcpu);
+	kvm_x86_ops->set_msr_mtrr(vcpu, msr);
 	return 0;
 }
 
-- 
1.9.3

Re: [PATCH 8/9] KVM: MMU: fix MTRR update

[David Matlack]

On Thu, Apr 30, 2015 at 3:24 AM,  <guangrong.xiao@linux.intel.com> wrote:
> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>
> Currently, whenever guest MTRR registers are changed kvm_mmu_reset_context
> is called to switch to the new root shadow page table, however, it's useless
> since:
> 1) the cache type is not cached into shadow page's attribute so that the
>    original root shadow page will be reused
>
> 2) the cache type is set on the last spte, that means we should sync the last
>    sptes when MTRR is changed
>
> This patch fixes this issue by dropping all the spte in the gfn range which is
> being updated by MTRR
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  2 ++
>  arch/x86/kvm/svm.c              |  5 ++++
>  arch/x86/kvm/vmx.c              | 58 +++++++++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/x86.c              |  2 +-
>  4 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index dea2e7e..ae9528d 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -778,6 +778,8 @@ struct kvm_x86_ops {
>         int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
>         int (*get_tdp_level)(void);
>         u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> +       void (*set_msr_mtrr)(struct kvm_vcpu *vcpu, u32 msr);
> +
>         int (*get_lpage_level)(void);
>         bool (*rdtscp_supported)(void);
>         bool (*invpcid_supported)(void);
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index ce741b8..c33573c 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4078,6 +4078,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
>         return 0;
>  }
>
> +static void svm_set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr)
> +{
> +}
> +
>  static void svm_cpuid_update(struct kvm_vcpu *vcpu)
>  {
>  }
> @@ -4410,6 +4414,7 @@ static struct kvm_x86_ops svm_x86_ops = {
>         .set_tss_addr = svm_set_tss_addr,
>         .get_tdp_level = get_npt_level,
>         .get_mt_mask = svm_get_mt_mask,
> +       .set_msr_mtrr = svm_set_msr_mtrr,
>
>         .get_exit_info = svm_get_exit_info,
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f7b6168..fcd0001 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8505,6 +8505,63 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
>         return ret;
>  }
>
> +static void vmx_set_msr_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 || !enable_ept ||
> +             !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;

For variable range MTRRs, I think you want to break out here if the valid flag
(bit 11 of the mask MTRR) is not set.

> +
> +       kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
> +}
> +
>  static int vmx_get_lpage_level(void)
>  {
>         if (enable_ept && !cpu_has_vmx_ept_1g_page())
> @@ -10218,6 +10275,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
>         .set_tss_addr = vmx_set_tss_addr,
>         .get_tdp_level = get_ept_level,
>         .get_mt_mask = vmx_get_mt_mask,
> +       .set_msr_mtrr = vmx_set_msr_mtrr,
>
>         .get_exit_info = vmx_get_exit_info,
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c73efcd..579c205 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1887,7 +1887,7 @@ static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>                 *pt = data;
>         }
>
> -       kvm_mmu_reset_context(vcpu);
> +       kvm_x86_ops->set_msr_mtrr(vcpu, msr);
>         return 0;
>  }
>
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 8/9] KVM: MMU: fix MTRR update

[Xiao Guangrong]

Hi David,

Thanks for your review.

On 05/07/2015 05:36 AM, David Matlack wrote:

>> +static void vmx_set_msr_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 || !enable_ept ||
>> +             !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;
>
> For variable range MTRRs, I think you want to break out here if the valid flag
> (bit 11 of the mask MTRR) is not set.

We should update these MTRRs whenever the valid bit is changed. If here we see
valid bit is zero, guest is disabling MTRR for that range so that we need to
drop cache type for that range we previously set.

Re: [PATCH 8/9] KVM: MMU: fix MTRR update

[Paolo Bonzini]

On 30/04/2015 12:24, guangrong.xiao@linux.intel.com wrote:
> +static void vmx_set_msr_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 || !enable_ept ||
> +	      !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));
> +}

I think this should all be generic logic, even if it causes some extra
zaps on AMD.  (It's AMD's bug that it doesn't honor MTRRs).

Even !enable_ept can be handled in a vendor-independent manner, as
"vcpu->arch.mmu.page_fault == tdp_page_fault".

Paolo

Re: [PATCH 8/9] KVM: MMU: fix MTRR update

[Xiao Guangrong]

On 05/08/2015 12:53 AM, Paolo Bonzini wrote:
>
>
> On 30/04/2015 12:24, guangrong.xiao@linux.intel.com wrote:
>> +static void vmx_set_msr_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 || !enable_ept ||
>> +	      !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));
>> +}
>
> I think this should all be generic logic, even if it causes some extra
> zaps on AMD.  (It's AMD's bug that it doesn't honor MTRRs).

Okay, will move the function to x86.c and kill the callback in x86_ops.

>
> Even !enable_ept can be handled in a vendor-independent manner, as
> "vcpu->arch.mmu.page_fault == tdp_page_fault".

We can directly use 'tdp_enabled', it has already been extern-ed. :)

[PATCH 9/9] KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

CR0.CD and CR0.NW are not used by shadow page table so that need
not adjust mmu if these two bit are changed

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

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 579c205..254c137 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -572,8 +572,7 @@ out:
 int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 {
 	unsigned long old_cr0 = kvm_read_cr0(vcpu);
-	unsigned long update_bits = X86_CR0_PG | X86_CR0_WP |
-				    X86_CR0_CD | X86_CR0_NW;
+	unsigned long update_bits = X86_CR0_PG | X86_CR0_WP;
 
 	cr0 |= X86_CR0_ET;
 
-- 
1.9.3

[PATCH 0/9] KVM: MTRR fixes and some cleanups

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

This are some MTRR bugs if legacy IOMMU device is used on Intel's CPU:
- In current code, whenever guest MTRR registers are changed
  kvm_mmu_reset_context is called to switch to the new root shadow page
  table, however, it's useless since:
  1) the cache type is not cached into shadow page's attribute so that the
     original root shadow page will be reused

  2) the cache type is set on the last spte, that means we should sync the
     last sptes when MTRR is changed

  We can fix it by dropping all the spte in the gfn range which is
  being updated by MTRR

- some bugs are in get_mtrr_type();
  1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
     MSR which completely control MTRR's enablement that means other bits are
     ignored if it is cleared

  2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit
     10 of IA32_MTRR_DEF_TYPE)
  
  3: if MTRR is disabled, UC is applied to all of physical memory rather than
     mtrr_state->def_type

- we need not to reset mmu once cache policy is changed since shadow page table
  does not virtualize any cache policy

Also, these are some cleanups to make current MMU code more cleaner and help
us fixing the bug more easier. 

Xiao Guangrong (9):
  KVM: MMU: fix decoding cache type from MTRR
  KVM: MMU: introduce slot_handle_level() and its helper
  KVM: MMU: use slot_handle_level and its helper to clean up the code
  KVM: MMU: introduce for_each_rmap_spte()
  KVM: MMU: KVM: introduce for_each_slot_rmap
  KVM: MMU: introduce kvm_zap_rmapp
  KVM: MMU: introduce kvm_zap_gfn_range()
  KVM: MMU: fix MTRR update
  KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed

 arch/x86/include/asm/kvm_host.h |   2 +
 arch/x86/kvm/mmu.c              | 407 ++++++++++++++++++++++------------------
 arch/x86/kvm/mmu.h              |   1 +
 arch/x86/kvm/mmu_audit.c        |   4 +-
 arch/x86/kvm/svm.c              |   5 +
 arch/x86/kvm/vmx.c              |  58 ++++++
 arch/x86/kvm/x86.c              |   5 +-
 7 files changed, 294 insertions(+), 188 deletions(-)

-- 
1.9.3

[PATCH 1/9] KVM: MMU: fix decoding cache type from MTRR

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

There are some bugs in current get_mtrr_type();
1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
   MSR which completely control MTRR's enablement that means other bits are
   ignored if it is cleared

2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit 10
   of IA32_MTRR_DEF_TYPE)

3: if MTRR is disabled, UC is applied to all of physical memory rather than
   mtrr_state->def_type

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index d43867c..ea3e3e4 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2393,19 +2393,20 @@ EXPORT_SYMBOL_GPL(kvm_mmu_unprotect_page);
 static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
 			 u64 start, u64 end)
 {
-	int i;
 	u64 base, mask;
 	u8 prev_match, curr_match;
-	int num_var_ranges = KVM_NR_VAR_MTRR;
+	int i, num_var_ranges = KVM_NR_VAR_MTRR;
 
-	if (!mtrr_state->enabled)
-		return 0xFF;
+	/* 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 && (start < 0x100000)) {
+	if (mtrr_state->have_fixed && (mtrr_state->enabled & 0x1) &&
+	      (start < 0x100000)) {
 		int idx;
 
 		if (start < 0x80000) {
@@ -2428,9 +2429,6 @@ static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
 	 * Look of multiple ranges matching this address and pick type
 	 * as per MTRR precedence
 	 */
-	if (!(mtrr_state->enabled & 2))
-		return mtrr_state->def_type;
-
 	prev_match = 0xFF;
 	for (i = 0; i < num_var_ranges; ++i) {
 		unsigned short start_state, end_state;
-- 
1.9.3

Re: [PATCH 1/9] KVM: MMU: fix decoding cache type from MTRR

[David Matlack]

On Thu, Apr 30, 2015 at 3:24 AM,  <guangrong.xiao@linux.intel.com> wrote:
> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>
> There are some bugs in current get_mtrr_type();
> 1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE

bit 1, not bit 2. (code is correct though)

>    MSR which completely control MTRR's enablement that means other bits are
>    ignored if it is cleared
>
> 2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit 10

bit 0, not bit 1. (code is correct though)

>    of IA32_MTRR_DEF_TYPE)
>
> 3: if MTRR is disabled, UC is applied to all of physical memory rather than
>    mtrr_state->def_type

kvm_get_guest_memory_type defaults to MTRR_TYPE_WRBACK, not
mtrr_state->def_type, when get_mtrr_type returns 0xFF.

>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/kvm/mmu.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index d43867c..ea3e3e4 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2393,19 +2393,20 @@ EXPORT_SYMBOL_GPL(kvm_mmu_unprotect_page);
>  static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
>                          u64 start, u64 end)
>  {
> -       int i;
>         u64 base, mask;
>         u8 prev_match, curr_match;
> -       int num_var_ranges = KVM_NR_VAR_MTRR;
> +       int i, num_var_ranges = KVM_NR_VAR_MTRR;
>
> -       if (!mtrr_state->enabled)
> -               return 0xFF;
> +       /* 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 && (start < 0x100000)) {
> +       if (mtrr_state->have_fixed && (mtrr_state->enabled & 0x1) &&
> +             (start < 0x100000)) {
>                 int idx;
>
>                 if (start < 0x80000) {
> @@ -2428,9 +2429,6 @@ static int get_mtrr_type(struct mtrr_state_type *mtrr_state,
>          * Look of multiple ranges matching this address and pick type
>          * as per MTRR precedence
>          */
> -       if (!(mtrr_state->enabled & 2))
> -               return mtrr_state->def_type;
> -
>         prev_match = 0xFF;
>         for (i = 0; i < num_var_ranges; ++i) {
>                 unsigned short start_state, end_state;
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/9] KVM: MMU: fix decoding cache type from MTRR

[Xiao Guangrong]

On 05/07/2015 05:42 AM, David Matlack wrote:
> On Thu, Apr 30, 2015 at 3:24 AM,  <guangrong.xiao@linux.intel.com> wrote:
>> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>>
>> There are some bugs in current get_mtrr_type();
>> 1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
>
> bit 1, not bit 2. (code is correct though)

Oh, i counted the bit from 1, my fault. :(

>
>>     MSR which completely control MTRR's enablement that means other bits are
>>     ignored if it is cleared
>>
>> 2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit 10
>
> bit 0, not bit 1. (code is correct though)

Ditto. Will update the changelog in v2. Thank you for pointing it out.

>
>>     of IA32_MTRR_DEF_TYPE)
>>
>> 3: if MTRR is disabled, UC is applied to all of physical memory rather than
>>     mtrr_state->def_type
>
> kvm_get_guest_memory_type defaults to MTRR_TYPE_WRBACK, not
> mtrr_state->def_type, when get_mtrr_type returns 0xFF.
>

Yeah, that confused me. Based on the comment of vmx_get_mt_mask():
	 *   a. VT-d without snooping control feature: can't guarantee the
	 *	result, try to trust guest.
we need to completely follow guest's MTRR under this case.

[PATCH 2/9] KVM: MMU: introduce slot_handle_level() and its helper

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

There are several places walking all rmaps for the memslot so that
introduce common functions to cleanup the code

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ea3e3e4..75a3459 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4410,6 +4410,69 @@ void kvm_mmu_setup(struct kvm_vcpu *vcpu)
 	init_kvm_mmu(vcpu);
 }
 
+/* The return value indicates if tlb flush on all vcpus is needed. */
+typedef bool (*slot_level_handler) (struct kvm *kvm, unsigned long *rmap);
+
+/* The caller should hold mmu-lock before calling this function. */
+static bool
+slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		  slot_level_handler fn, int min_level, int max_level,
+		  bool lock_flush_tlb)
+{
+	unsigned long last_gfn;
+	bool flush = false;
+	int level;
+
+	last_gfn = memslot->base_gfn + memslot->npages - 1;
+
+	for (level = min_level; level <= max_level; ++level) {
+		unsigned long *rmapp;
+		unsigned long last_index, index;
+
+		rmapp = memslot->arch.rmap[level - PT_PAGE_TABLE_LEVEL];
+		last_index = gfn_to_index(last_gfn, memslot->base_gfn, level);
+
+		for (index = 0; index <= last_index; ++index, ++rmapp) {
+			if (*rmapp)
+				flush |= fn(kvm, rmapp);
+
+			if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
+				if (flush && lock_flush_tlb) {
+					kvm_flush_remote_tlbs(kvm);
+					flush = false;
+				}
+				cond_resched_lock(&kvm->mmu_lock);
+			}
+		}
+	}
+
+	return flush;
+}
+
+static bool
+slot_handle_all_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		      slot_level_handler fn, bool lock_flush_tlb)
+{
+	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
+		PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1, lock_flush_tlb);
+}
+
+static bool
+slot_handle_large_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+			slot_level_handler fn, bool lock_flush_tlb)
+{
+	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL + 1,
+		PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1, lock_flush_tlb);
+}
+
+static bool
+slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		 slot_level_handler fn, bool lock_flush_tlb)
+{
+	return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
+				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
+}
+
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
 				      struct kvm_memory_slot *memslot)
 {
-- 
1.9.3

[PATCH 3/9] KVM: MMU: use slot_handle_level and its helper to clean up the code

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

slot_handle_level and its helper functions are ready now, use them to
clean up the code

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 75a3459..fd13991 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4473,35 +4473,19 @@ slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
 				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
 }
 
+static bool slot_rmap_write_protect(struct kvm *kvm, unsigned long *rmapp)
+{
+	return __rmap_write_protect(kvm, rmapp, false);
+}
+
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm,
 				      struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	int i;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	for (i = PT_PAGE_TABLE_LEVEL;
-	     i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[i - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, i);
-
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= __rmap_write_protect(kvm, rmapp,
-						false);
-
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-				cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
-
+	flush = slot_handle_all_level(kvm, memslot, slot_rmap_write_protect,
+				      false);
 	spin_unlock(&kvm->mmu_lock);
 
 	/*
@@ -4564,59 +4548,23 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
 			struct kvm_memory_slot *memslot)
 {
-	bool flush = false;
-	unsigned long *rmapp;
-	unsigned long last_index, index;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	rmapp = memslot->arch.rmap[0];
-	last_index = gfn_to_index(memslot->base_gfn + memslot->npages - 1,
-				memslot->base_gfn, PT_PAGE_TABLE_LEVEL);
-
-	for (index = 0; index <= last_index; ++index, ++rmapp) {
-		if (*rmapp)
-			flush |= kvm_mmu_zap_collapsible_spte(kvm, rmapp);
-
-		if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
-			if (flush) {
-				kvm_flush_remote_tlbs(kvm);
-				flush = false;
-			}
-			cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
-
+	flush = slot_handle_leaf(kvm, memslot, kvm_mmu_zap_collapsible_spte,
+				 true);
 	if (flush)
 		kvm_flush_remote_tlbs(kvm);
-
 	spin_unlock(&kvm->mmu_lock);
 }
 
 void kvm_mmu_slot_leaf_clear_dirty(struct kvm *kvm,
 				   struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	unsigned long *rmapp;
-	unsigned long last_index, index;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	rmapp = memslot->arch.rmap[PT_PAGE_TABLE_LEVEL - 1];
-	last_index = gfn_to_index(last_gfn, memslot->base_gfn,
-			PT_PAGE_TABLE_LEVEL);
-
-	for (index = 0; index <= last_index; ++index, ++rmapp) {
-		if (*rmapp)
-			flush |= __rmap_clear_dirty(kvm, rmapp);
-
-		if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-			cond_resched_lock(&kvm->mmu_lock);
-	}
-
+	flush = slot_handle_leaf(kvm, memslot, __rmap_clear_dirty, false);
 	spin_unlock(&kvm->mmu_lock);
 
 	lockdep_assert_held(&kvm->slots_lock);
@@ -4635,31 +4583,11 @@ EXPORT_SYMBOL_GPL(kvm_mmu_slot_leaf_clear_dirty);
 void kvm_mmu_slot_largepage_remove_write_access(struct kvm *kvm,
 					struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	int i;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	for (i = PT_PAGE_TABLE_LEVEL + 1; /* skip rmap for 4K page */
-	     i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[i - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, i);
-
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= __rmap_write_protect(kvm, rmapp,
-						false);
-
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-				cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
+	flush = slot_handle_large_level(kvm, memslot, slot_rmap_write_protect,
+					false);
 	spin_unlock(&kvm->mmu_lock);
 
 	/* see kvm_mmu_slot_remove_write_access */
@@ -4673,31 +4601,10 @@ EXPORT_SYMBOL_GPL(kvm_mmu_slot_largepage_remove_write_access);
 void kvm_mmu_slot_set_dirty(struct kvm *kvm,
 			    struct kvm_memory_slot *memslot)
 {
-	gfn_t last_gfn;
-	int i;
-	bool flush = false;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
+	bool flush;
 
 	spin_lock(&kvm->mmu_lock);
-
-	for (i = PT_PAGE_TABLE_LEVEL;
-	     i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[i - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, i);
-
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= __rmap_set_dirty(kvm, rmapp);
-
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock))
-				cond_resched_lock(&kvm->mmu_lock);
-		}
-	}
-
+	flush = slot_handle_all_level(kvm, memslot, __rmap_set_dirty, false);
 	spin_unlock(&kvm->mmu_lock);
 
 	lockdep_assert_held(&kvm->slots_lock);
-- 
1.9.3

[PATCH 4/9] KVM: MMU: introduce for_each_rmap_spte()

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It's used to walk all the sptes on the rmap to clean up the
code

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index fd13991..4d98c6c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1142,6 +1142,11 @@ static u64 *rmap_get_next(struct rmap_iterator *iter)
 	return NULL;
 }
 
+#define for_each_rmap_spte(_rmap_, _iter_, _spte_)			    \
+	   for (_spte_ = rmap_get_first(*_rmap_, _iter_);		    \
+		_spte_ && ({BUG_ON(!is_shadow_present_pte(*_spte_)); 1;});  \
+			_spte_ = rmap_get_next(_iter_))
+
 static void drop_spte(struct kvm *kvm, u64 *sptep)
 {
 	if (mmu_spte_clear_track_bits(sptep))
@@ -1205,12 +1210,8 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
 	struct rmap_iterator iter;
 	bool flush = false;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		flush |= spte_write_protect(kvm, sptep, pt_protect);
-		sptep = rmap_get_next(&iter);
-	}
 
 	return flush;
 }
@@ -1232,12 +1233,8 @@ static bool __rmap_clear_dirty(struct kvm *kvm, unsigned long *rmapp)
 	struct rmap_iterator iter;
 	bool flush = false;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		flush |= spte_clear_dirty(kvm, sptep);
-		sptep = rmap_get_next(&iter);
-	}
 
 	return flush;
 }
@@ -1259,12 +1256,8 @@ static bool __rmap_set_dirty(struct kvm *kvm, unsigned long *rmapp)
 	struct rmap_iterator iter;
 	bool flush = false;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		flush |= spte_set_dirty(kvm, sptep);
-		sptep = rmap_get_next(&iter);
-	}
 
 	return flush;
 }
@@ -1368,13 +1361,14 @@ static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	struct rmap_iterator iter;
 	int need_tlb_flush = 0;
 
-	while ((sptep = rmap_get_first(*rmapp, &iter))) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
+restart:
+	for_each_rmap_spte(rmapp, &iter, sptep) {
 		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx gfn %llx (%d)\n",
 			     sptep, *sptep, gfn, level);
 
 		drop_spte(kvm, sptep);
 		need_tlb_flush = 1;
+		goto restart;
 	}
 
 	return need_tlb_flush;
@@ -1394,8 +1388,8 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	WARN_ON(pte_huge(*ptep));
 	new_pfn = pte_pfn(*ptep);
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!is_shadow_present_pte(*sptep));
+restart:
+	for_each_rmap_spte(rmapp, &iter, sptep) {
 		rmap_printk("kvm_set_pte_rmapp: spte %p %llx gfn %llx (%d)\n",
 			     sptep, *sptep, gfn, level);
 
@@ -1403,7 +1397,7 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 
 		if (pte_write(*ptep)) {
 			drop_spte(kvm, sptep);
-			sptep = rmap_get_first(*rmapp, &iter);
+			goto restart;
 		} else {
 			new_spte = *sptep & ~PT64_BASE_ADDR_MASK;
 			new_spte |= (u64)new_pfn << PAGE_SHIFT;
@@ -1414,7 +1408,6 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
 
 			mmu_spte_clear_track_bits(sptep);
 			mmu_spte_set(sptep, new_spte);
-			sptep = rmap_get_next(&iter);
 		}
 	}
 
@@ -1518,16 +1511,13 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
 
 	BUG_ON(!shadow_accessed_mask);
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;
-	     sptep = rmap_get_next(&iter)) {
-		BUG_ON(!is_shadow_present_pte(*sptep));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		if (*sptep & shadow_accessed_mask) {
 			young = 1;
 			clear_bit((ffs(shadow_accessed_mask) - 1),
 				 (unsigned long *)sptep);
 		}
-	}
+
 	trace_kvm_age_page(gfn, level, slot, young);
 	return young;
 }
@@ -1548,15 +1538,11 @@ static int kvm_test_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
 	if (!shadow_accessed_mask)
 		goto out;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;
-	     sptep = rmap_get_next(&iter)) {
-		BUG_ON(!is_shadow_present_pte(*sptep));
-
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		if (*sptep & shadow_accessed_mask) {
 			young = 1;
 			break;
 		}
-	}
 out:
 	return young;
 }
@@ -2232,8 +2218,11 @@ static void kvm_mmu_unlink_parents(struct kvm *kvm, struct kvm_mmu_page *sp)
 	u64 *sptep;
 	struct rmap_iterator iter;
 
-	while ((sptep = rmap_get_first(sp->parent_ptes, &iter)))
+restart:
+	for_each_rmap_spte(&sp->parent_ptes, &iter, sptep) {
 		drop_parent_pte(sp, sptep);
+		goto restart;
+	}
 }
 
 static int mmu_zap_unsync_children(struct kvm *kvm,
@@ -4519,9 +4508,8 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 	pfn_t pfn;
 	struct kvm_mmu_page *sp;
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;) {
-		BUG_ON(!(*sptep & PT_PRESENT_MASK));
-
+restart:
+	for_each_rmap_spte(rmapp, &iter, sptep) {
 		sp = page_header(__pa(sptep));
 		pfn = spte_to_pfn(*sptep);
 
@@ -4536,10 +4524,9 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
 			!kvm_is_reserved_pfn(pfn) &&
 			PageTransCompound(pfn_to_page(pfn))) {
 			drop_spte(kvm, sptep);
-			sptep = rmap_get_first(*rmapp, &iter);
 			need_tlb_flush = 1;
-		} else
-			sptep = rmap_get_next(&iter);
+			goto restart;
+		}
 	}
 
 	return need_tlb_flush;
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 9ade5cf..368d534 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -197,13 +197,11 @@ static void audit_write_protection(struct kvm *kvm, struct kvm_mmu_page *sp)
 
 	rmapp = gfn_to_rmap(kvm, sp->gfn, PT_PAGE_TABLE_LEVEL);
 
-	for (sptep = rmap_get_first(*rmapp, &iter); sptep;
-	     sptep = rmap_get_next(&iter)) {
+	for_each_rmap_spte(rmapp, &iter, sptep)
 		if (is_writable_pte(*sptep))
 			audit_printk(kvm, "shadow page has writable "
 				     "mappings: gfn %llx role %x\n",
 				     sp->gfn, sp->role.word);
-	}
 }
 
 static void audit_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
-- 
1.9.3

[PATCH 5/9] KVM: MMU: KVM: introduce for_each_slot_rmap

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It is used to clean up the code between kvm_handle_hva_range and
slot_handle_level, also it will be used by later patch

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 4d98c6c..fea1e83 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1417,6 +1417,63 @@ restart:
 	return 0;
 }
 
+struct slot_rmap_walker {
+	/* input fields. */
+	struct kvm_memory_slot *slot;
+	gfn_t start_gfn;
+	gfn_t end_gfn;
+	int start_level;
+	int end_level;
+
+	/* output fields. */
+	gfn_t gfn;
+	unsigned long *rmap;
+	int level;
+
+	/* private field. */
+	unsigned long *end_rmap;
+};
+
+static void rmap_walk_init_level(struct slot_rmap_walker *walker, int level)
+{
+	struct kvm_memory_slot *slot = walker->slot;
+
+	walker->level = level;
+	walker->gfn = walker->start_gfn;
+	walker->rmap = __gfn_to_rmap(walker->gfn, walker->level, slot);
+	walker->end_rmap = __gfn_to_rmap(walker->end_gfn, walker->level, slot);
+}
+
+static void slot_rmap_walk_init(struct slot_rmap_walker *walker)
+{
+	rmap_walk_init_level(walker, walker->start_level);
+}
+
+static bool slot_rmap_walk_okay(struct slot_rmap_walker *walker)
+{
+	return !!walker->rmap;
+}
+
+static void slot_rmap_walk_next(struct slot_rmap_walker *walker)
+{
+	if (++walker->rmap <= walker->end_rmap) {
+		walker->gfn += (1UL << KVM_HPAGE_GFN_SHIFT(walker->level));
+		return;
+	}
+
+	if (++walker->level > walker->end_level) {
+		walker->rmap = NULL;
+		return;
+	}
+
+	rmap_walk_init_level(walker, walker->level);
+}
+
+#define for_each_slot_rmap(_walker_)				\
+		for (slot_rmap_walk_init(_walker_);		\
+			slot_rmap_walk_okay(_walker_);		\
+				slot_rmap_walk_next(_walker_))
+
 static int kvm_handle_hva_range(struct kvm *kvm,
 				unsigned long start,
 				unsigned long end,
@@ -1428,10 +1485,10 @@ static int kvm_handle_hva_range(struct kvm *kvm,
 					       int level,
 					       unsigned long data))
 {
-	int j;
-	int ret = 0;
 	struct kvm_memslots *slots;
 	struct kvm_memory_slot *memslot;
+	struct slot_rmap_walker walker;
+	int ret = 0;
 
 	slots = kvm_memslots(kvm);
 
@@ -1451,26 +1508,18 @@ static int kvm_handle_hva_range(struct kvm *kvm,
 		gfn_start = hva_to_gfn_memslot(hva_start, memslot);
 		gfn_end = hva_to_gfn_memslot(hva_end + PAGE_SIZE - 1, memslot);
 
-		for (j = PT_PAGE_TABLE_LEVEL;
-		     j < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++j) {
-			unsigned long idx, idx_end;
-			unsigned long *rmapp;
-			gfn_t gfn = gfn_start;
-
-			/*
-			 * {idx(page_j) | page_j intersects with
-			 *  [hva_start, hva_end)} = {idx, idx+1, ..., idx_end}.
-			 */
-			idx = gfn_to_index(gfn_start, memslot->base_gfn, j);
-			idx_end = gfn_to_index(gfn_end - 1, memslot->base_gfn, j);
-
-			rmapp = __gfn_to_rmap(gfn_start, j, memslot);
-
-			for (; idx <= idx_end;
-			       ++idx, gfn += (1UL << KVM_HPAGE_GFN_SHIFT(j)))
-				ret |= handler(kvm, rmapp++, memslot,
-					       gfn, j, data);
-		}
+		walker = (struct slot_rmap_walker) {
+			.slot = memslot,
+			.start_gfn = gfn_start,
+			.end_gfn = gfn_end - 1,
+			.start_level = PT_PAGE_TABLE_LEVEL,
+			.end_level = PT_PAGE_TABLE_LEVEL +
+					KVM_NR_PAGE_SIZES - 1,
+		};
+
+		for_each_slot_rmap(&walker)
+			ret |= handler(kvm, walker.rmap, memslot, walker.gfn,
+				       walker.level, data);
 	}
 
 	return ret;
@@ -4404,34 +4453,29 @@ typedef bool (*slot_level_handler) (struct kvm *kvm, unsigned long *rmap);
 
 /* The caller should hold mmu-lock before calling this function. */
 static bool
-slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
-		  slot_level_handler fn, int min_level, int max_level,
-		  bool lock_flush_tlb)
+slot_handle_level_range(struct kvm *kvm, struct kvm_memory_slot *memslot,
+			 slot_level_handler fn, int min_level, int max_level,
+			 gfn_t start_gfn, gfn_t last_gfn, bool lock_flush_tlb)
 {
-	unsigned long last_gfn;
 	bool flush = false;
-	int level;
-
-	last_gfn = memslot->base_gfn + memslot->npages - 1;
-
-	for (level = min_level; level <= max_level; ++level) {
-		unsigned long *rmapp;
-		unsigned long last_index, index;
-
-		rmapp = memslot->arch.rmap[level - PT_PAGE_TABLE_LEVEL];
-		last_index = gfn_to_index(last_gfn, memslot->base_gfn, level);
+	struct slot_rmap_walker walker = {
+		.slot = memslot,
+		.start_gfn = start_gfn,
+		.end_gfn = last_gfn,
+		.start_level = min_level,
+		.end_level = max_level,
+	};
 
-		for (index = 0; index <= last_index; ++index, ++rmapp) {
-			if (*rmapp)
-				flush |= fn(kvm, rmapp);
+	for_each_slot_rmap(&walker) {
+		if (*walker.rmap)
+			flush |= fn(kvm, walker.rmap);
 
-			if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
-				if (flush && lock_flush_tlb) {
-					kvm_flush_remote_tlbs(kvm);
-					flush = false;
-				}
-				cond_resched_lock(&kvm->mmu_lock);
+		if (need_resched() || spin_needbreak(&kvm->mmu_lock)) {
+			if (flush && lock_flush_tlb) {
+				kvm_flush_remote_tlbs(kvm);
+				flush = false;
 			}
+			cond_resched_lock(&kvm->mmu_lock);
 		}
 	}
 
@@ -4439,6 +4483,16 @@ slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
 }
 
 static bool
+slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+		  slot_level_handler fn, int min_level, int max_level,
+		  bool lock_flush_tlb)
+{
+	return slot_handle_level_range(kvm, memslot, fn, min_level,
+		max_level, memslot->base_gfn,
+		memslot->base_gfn + memslot->npages - 1, lock_flush_tlb);
+}
+
+static bool
 slot_handle_all_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
 		      slot_level_handler fn, bool lock_flush_tlb)
 {
-- 
1.9.3

[PATCH 6/9] KVM: MMU: introduce kvm_zap_rmapp

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Split kvm_unmap_rmapp and introduce kvm_zap_rmapp which will be used in the
later patch

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index fea1e83..5d90a22 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1353,25 +1353,29 @@ static bool rmap_write_protect(struct kvm *kvm, u64 gfn)
 	return write_protected;
 }
 
-static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp,
-			   struct kvm_memory_slot *slot, gfn_t gfn, int level,
-			   unsigned long data)
+static bool kvm_zap_rmapp(struct kvm *kvm, unsigned long *rmapp)
 {
 	u64 *sptep;
 	struct rmap_iterator iter;
-	int need_tlb_flush = 0;
+	bool flush = false;
 
 restart:
 	for_each_rmap_spte(rmapp, &iter, sptep) {
-		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx gfn %llx (%d)\n",
-			     sptep, *sptep, gfn, level);
+		rmap_printk("%s: spte %p %llx.\n", __func__, sptep, *sptep);
 
 		drop_spte(kvm, sptep);
-		need_tlb_flush = 1;
+		flush = true;
 		goto restart;
 	}
 
-	return need_tlb_flush;
+	return flush;
+}
+
+static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp,
+			   struct kvm_memory_slot *slot, gfn_t gfn, int level,
+			   unsigned long data)
+{
+	return kvm_zap_rmapp(kvm, rmapp);
 }
 
 static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp,
-- 
1.9.3

[PATCH 7/9] KVM: MMU: introduce kvm_zap_gfn_range()

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It is used to zap all the rmaps of the specified gfn range and will
be used by the later patch

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

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5d90a22..aa9aad0 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4520,6 +4520,36 @@ slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
 				 PT_PAGE_TABLE_LEVEL, lock_flush_tlb);
 }
 
+void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
+{
+	struct kvm_memslots *slots;
+	struct kvm_memory_slot *memslot;
+	bool flush = false;
+
+	slots = kvm_memslots(kvm);
+
+	spin_lock(&kvm->mmu_lock);
+	kvm_for_each_memslot(memslot, slots) {
+		gfn_t start, end;
+
+		start = max(gfn_start, memslot->base_gfn);
+		end = min(gfn_end, memslot->base_gfn + memslot->npages);
+		if (start >= end)
+			continue;
+
+		flush |= slot_handle_level_range(kvm, memslot, kvm_zap_rmapp,
+			      PT_PAGE_TABLE_LEVEL,
+			      PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES - 1,
+			      start, end - 1, true);
+	}
+
+	if (flush)
+		kvm_flush_remote_tlbs(kvm);
+	spin_unlock(&kvm->mmu_lock);
+
+}
+EXPORT_SYMBOL_GPL(kvm_zap_gfn_range);
+
 static bool slot_rmap_write_protect(struct kvm *kvm, unsigned long *rmapp)
 {
 	return __rmap_write_protect(kvm, rmapp, false);
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index c7d6563..44b75f3 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -170,4 +170,5 @@ static inline bool permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 }
 
 void kvm_mmu_invalidate_zap_all_pages(struct kvm *kvm);
+void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end);
 #endif
-- 
1.9.3

[PATCH 8/9] KVM: MMU: fix MTRR update

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Currently, whenever guest MTRR registers are changed kvm_mmu_reset_context
is called to switch to the new root shadow page table, however, it's useless
since:
1) the cache type is not cached into shadow page's attribute so that the
   original root shadow page will be reused

2) the cache type is set on the last spte, that means we should sync the last
   sptes when MTRR is changed

This patch fixes this issue by dropping all the spte in the gfn range which is
being updated by MTRR

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/svm.c              |  5 ++++
 arch/x86/kvm/vmx.c              | 58 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              |  2 +-
 4 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index dea2e7e..ae9528d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -778,6 +778,8 @@ struct kvm_x86_ops {
 	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
 	int (*get_tdp_level)(void);
 	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
+	void (*set_msr_mtrr)(struct kvm_vcpu *vcpu, u32 msr);
+
 	int (*get_lpage_level)(void);
 	bool (*rdtscp_supported)(void);
 	bool (*invpcid_supported)(void);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ce741b8..c33573c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4078,6 +4078,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	return 0;
 }
 
+static void svm_set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr)
+{
+}
+
 static void svm_cpuid_update(struct kvm_vcpu *vcpu)
 {
 }
@@ -4410,6 +4414,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.set_tss_addr = svm_set_tss_addr,
 	.get_tdp_level = get_npt_level,
 	.get_mt_mask = svm_get_mt_mask,
+	.set_msr_mtrr = svm_set_msr_mtrr,
 
 	.get_exit_info = svm_get_exit_info,
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f7b6168..fcd0001 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8505,6 +8505,63 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	return ret;
 }
 
+static void vmx_set_msr_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 || !enable_ept ||
+	      !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 vmx_get_lpage_level(void)
 {
 	if (enable_ept && !cpu_has_vmx_ept_1g_page())
@@ -10218,6 +10275,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_tss_addr = vmx_set_tss_addr,
 	.get_tdp_level = get_ept_level,
 	.get_mt_mask = vmx_get_mt_mask,
+	.set_msr_mtrr = vmx_set_msr_mtrr,
 
 	.get_exit_info = vmx_get_exit_info,
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c73efcd..579c205 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1887,7 +1887,7 @@ static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		*pt = data;
 	}
 
-	kvm_mmu_reset_context(vcpu);
+	kvm_x86_ops->set_msr_mtrr(vcpu, msr);
 	return 0;
 }
 
-- 
1.9.3

[PATCH 9/9] KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed

[guangrong.xiao]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

CR0.CD and CR0.NW are not used by shadow page table so that need
not adjust mmu if these two bit are changed

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

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 579c205..254c137 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -572,8 +572,7 @@ out:
 int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 {
 	unsigned long old_cr0 = kvm_read_cr0(vcpu);
-	unsigned long update_bits = X86_CR0_PG | X86_CR0_WP |
-				    X86_CR0_CD | X86_CR0_NW;
+	unsigned long update_bits = X86_CR0_PG | X86_CR0_WP;
 
 	cr0 |= X86_CR0_ET;
 
-- 
1.9.3

Re: [PATCH 0/9] KVM: MTRR fixes and some cleanups

[Paolo Bonzini]

On 30/04/2015 12:24, guangrong.xiao@linux.intel.com wrote:
> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> 
> This are some MTRR bugs if legacy IOMMU device is used on Intel's CPU:
> - In current code, whenever guest MTRR registers are changed
>   kvm_mmu_reset_context is called to switch to the new root shadow page
>   table, however, it's useless since:
>   1) the cache type is not cached into shadow page's attribute so that the
>      original root shadow page will be reused
> 
>   2) the cache type is set on the last spte, that means we should sync the
>      last sptes when MTRR is changed
> 
>   We can fix it by dropping all the spte in the gfn range which is
>   being updated by MTRR
> 
> - some bugs are in get_mtrr_type();
>   1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
>      MSR which completely control MTRR's enablement that means other bits are
>      ignored if it is cleared
> 
>   2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit
>      10 of IA32_MTRR_DEF_TYPE)
>   
>   3: if MTRR is disabled, UC is applied to all of physical memory rather than
>      mtrr_state->def_type
> 
> - we need not to reset mmu once cache policy is changed since shadow page table
>   does not virtualize any cache policy
> 
> Also, these are some cleanups to make current MMU code more cleaner and help
> us fixing the bug more easier. 
> 
> Xiao Guangrong (9):
>   KVM: MMU: fix decoding cache type from MTRR
>   KVM: MMU: introduce slot_handle_level() and its helper
>   KVM: MMU: use slot_handle_level and its helper to clean up the code
>   KVM: MMU: introduce for_each_rmap_spte()
>   KVM: MMU: KVM: introduce for_each_slot_rmap
>   KVM: MMU: introduce kvm_zap_rmapp
>   KVM: MMU: introduce kvm_zap_gfn_range()
>   KVM: MMU: fix MTRR update
>   KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed
> 
>  arch/x86/include/asm/kvm_host.h |   2 +
>  arch/x86/kvm/mmu.c              | 407 ++++++++++++++++++++++------------------
>  arch/x86/kvm/mmu.h              |   1 +
>  arch/x86/kvm/mmu_audit.c        |   4 +-
>  arch/x86/kvm/svm.c              |   5 +
>  arch/x86/kvm/vmx.c              |  58 ++++++
>  arch/x86/kvm/x86.c              |   5 +-
>  7 files changed, 294 insertions(+), 188 deletions(-)
> 

Very nice cleanups.  I made a couple comments on the patch ordering and
the API of iterator macros, but the basic ideas are great.  I'm going to
apply patch 1, and do some testing with kvm_arch_has_noncoherent_dma
forced to return true.

Paolo