KVM: MMU: fast invalidate all mmio sptes

Define some meaningful names instead of raw code

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/mmu.c | 15 +++++----------
 arch/x86/kvm/mmu.h | 14 ++++++++++++++
 arch/x86/kvm/vmx.c |  4 ++--
 3 files changed, 21 insertions(+), 12 deletions(-)

arch/x86/kvm/mmu.c +5 -10

index eca91bd..044d8c0 100644
@@ -3222,17 +3222,12 @@ static u64 walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr)
 	return spte;
 }
 
-/*
- * If it is a real mmio page fault, return 1 and emulat the instruction
- * directly, return 0 to let CPU fault again on the address, -1 is
- * returned if bug is detected.
- */
 int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 {
 	u64 spte;
 
 	if (quickly_check_mmio_pf(vcpu, addr, direct))
-		return 1;
+		return RET_MMIO_PF_EMULATE;
 
 	spte = walk_shadow_page_get_mmio_spte(vcpu, addr);
 

@@ -3245,7 +3240,7 @@ int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 
 		trace_handle_mmio_page_fault(addr, gfn, access);
 		vcpu_cache_mmio_info(vcpu, addr, gfn, access);
-		return 1;
+		return RET_MMIO_PF_EMULATE;
 	}
 
 	/*

@@ -3253,13 +3248,13 @@ int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 	 * it's a BUG if the gfn is not a mmio page.
 	 */
 	if (direct && !check_direct_spte_mmio_pf(spte))
-		return -1;
+		return RET_MMIO_PF_BUG;
 
 	/*
 	 * If the page table is zapped by other cpus, let CPU fault again on
 	 * the address.
 	 */
-	return 0;
+	return RET_MMIO_PF_RETRY;
 }
 EXPORT_SYMBOL_GPL(handle_mmio_page_fault_common);
 

@@ -3269,7 +3264,7 @@ static int handle_mmio_page_fault(struct kvm_vcpu *vcpu, u64 addr,
 	int ret;
 
 	ret = handle_mmio_page_fault_common(vcpu, addr, direct);
-	WARN_ON(ret < 0);
+	WARN_ON(ret == RET_MMIO_PF_BUG);
 	return ret;
 }
 

arch/x86/kvm/mmu.h +3 -0

index 922bfae..ba6a19c 100644
@@ -52,6 +52,20 @@
 
 int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 sptes[4]);
 void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask);
+
+/*
+ * Return values of handle_mmio_page_fault_common:

arch/x86/kvm/mmu.h +1 -0

+
 int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct);
 int kvm_init_shadow_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context);
 

arch/x86/kvm/vmx.c +2 -2

index 78ee123..85c8d51 100644
@@ -5366,10 +5366,10 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
 
 	ret = handle_mmio_page_fault_common(vcpu, gpa, true);
-	if (likely(ret == 1))
+	if (likely(ret == RET_MMIO_PF_EMULATE))
 		return x86_emulate_instruction(vcpu, gpa, 0, NULL, 0) ==
 					      EMULATE_DONE;
-	if (unlikely(!ret))
+	if (unlikely(ret == RET_MMIO_PF_RETRY))
 		return 1;
 
 	/* It is the real ept misconfig */

Signature · 1 line

1.8.1.4

Quoted 81 lines · Xiao Guangrong

Define some meaningful names instead of raw code

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/mmu.c | 15 +++++----------
 arch/x86/kvm/mmu.h | 14 ++++++++++++++
 arch/x86/kvm/vmx.c |  4 ++--
 3 files changed, 21 insertions(+), 12 deletions(-)

arch/x86/kvm/mmu.c +5 -10

index eca91bd..044d8c0 100644
@@ -3222,17 +3222,12 @@ static u64 walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr)
 	return spte;
 }
 
-/*
- * If it is a real mmio page fault, return 1 and emulat the instruction
- * directly, return 0 to let CPU fault again on the address, -1 is
- * returned if bug is detected.
- */
 int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 {
 	u64 spte;
 
 	if (quickly_check_mmio_pf(vcpu, addr, direct))
-		return 1;
+		return RET_MMIO_PF_EMULATE;
 
 	spte = walk_shadow_page_get_mmio_spte(vcpu, addr);
 

@@ -3245,7 +3240,7 @@ int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 
 		trace_handle_mmio_page_fault(addr, gfn, access);
 		vcpu_cache_mmio_info(vcpu, addr, gfn, access);
-		return 1;
+		return RET_MMIO_PF_EMULATE;
 	}
 
 	/*

@@ -3253,13 +3248,13 @@ int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 	 * it's a BUG if the gfn is not a mmio page.
 	 */
 	if (direct && !check_direct_spte_mmio_pf(spte))
-		return -1;
+		return RET_MMIO_PF_BUG;
 
 	/*
 	 * If the page table is zapped by other cpus, let CPU fault again on
 	 * the address.
 	 */
-	return 0;
+	return RET_MMIO_PF_RETRY;
 }
 EXPORT_SYMBOL_GPL(handle_mmio_page_fault_common);
 

@@ -3269,7 +3264,7 @@ static int handle_mmio_page_fault(struct kvm_vcpu *vcpu, u64 addr,
 	int ret;
 
 	ret = handle_mmio_page_fault_common(vcpu, addr, direct);
-	WARN_ON(ret < 0);
+	WARN_ON(ret == RET_MMIO_PF_BUG);
 	return ret;
 }
 

arch/x86/kvm/mmu.h +7 -0

index 922bfae..ba6a19c 100644
@@ -52,6 +52,20 @@
 
 int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 sptes[4]);
 void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask);
+
+/*
+ * Return values of handle_mmio_page_fault_common:
+ * RET_MMIO_PF_EMULATE: it is a real mmio page fault, emulate the instruction
+ *			 directly.
+ * RET_MMIO_PF_RETRY: let CPU fault again on the address.
+ * RET_MMIO_PF_BUG: bug is detected.

arch/x86/kvm/mmu.h line 67

Quoted 6 lines

arch/x86/kvm/mmu.h +6 -0

+ */
+enum {
+	RET_MMIO_PF_EMULATE = 1,
+	RET_MMIO_PF_RETRY = 0,
+	RET_MMIO_PF_BUG = -1
+};

I would order them from -1 to 1 and rename RET_MMIO_PF_BUG to RET_MMIO_PF_ERROR, but no need to resend just for that.

arch/x86/kvm/mmu.h line 67

Quoted 13 lines · Xiao Guangrong

arch/x86/kvm/mmu.h +13 -0

+
+/*
+ * Return values of handle_mmio_page_fault_common:
+ * RET_MMIO_PF_EMULATE: it is a real mmio page fault, emulate the instruction
+ *			 directly.
+ * RET_MMIO_PF_RETRY: let CPU fault again on the address.
+ * RET_MMIO_PF_BUG: bug is detected.
+ */
+enum {
+	RET_MMIO_PF_EMULATE = 1,
+	RET_MMIO_PF_RETRY = 0,
+	RET_MMIO_PF_BUG = -1
+};

I would order them from -1 to 1 and rename RET_MMIO_PF_BUG to RET_MMIO_PF_ERROR, but no need to resend just for that.

Quoted 17 lines · On Mon, 10 Jun 2013 10:57:50 +0300 Gleb Natapov <[email protected]> wrote:

arch/x86/kvm/mmu.h line 67

Quoted 13 lines · Xiao Guangrong

arch/x86/kvm/mmu.h +13 -0

+
+/*
+ * Return values of handle_mmio_page_fault_common:
+ * RET_MMIO_PF_EMULATE: it is a real mmio page fault, emulate the instruction
+ *			 directly.
+ * RET_MMIO_PF_RETRY: let CPU fault again on the address.
+ * RET_MMIO_PF_BUG: bug is detected.
+ */
+enum {
+	RET_MMIO_PF_EMULATE = 1,
+	RET_MMIO_PF_RETRY = 0,
+	RET_MMIO_PF_BUG = -1
+};

I would order them from -1 to 1 and rename RET_MMIO_PF_BUG to RET_MMIO_PF_ERROR, but no need to resend just for that.

Why not just let compilers select the values? -- It's an enum. Any reason to make it start from -1?

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/mmu.c              | 54 +++++++++++++++++++++++++++++++++++------
 arch/x86/kvm/mmu.h              |  5 +++-
 arch/x86/kvm/paging_tmpl.h      |  7 ++++--
 arch/x86/kvm/vmx.c              |  4 +++
 arch/x86/kvm/x86.c              |  3 +--
 6 files changed, 61 insertions(+), 14 deletions(-)

arch/x86/include/asm/kvm_host.h +1 -1

index 1f98c1b..90d05ed 100644
@@ -773,7 +773,7 @@ void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
 				     struct kvm_memory_slot *slot,
 				     gfn_t gfn_offset, unsigned long mask);
 void kvm_mmu_zap_all(struct kvm *kvm);
-void kvm_mmu_zap_mmio_sptes(struct kvm *kvm);
+void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
 unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
 void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages);
 

arch/x86/kvm/mmu.c +46 -8

index 044d8c0..bdc95bc 100644
@@ -205,9 +205,11 @@ EXPORT_SYMBOL_GPL(kvm_mmu_set_mmio_spte_mask);
 #define MMIO_SPTE_GEN_LOW_SHIFT		3
 #define MMIO_SPTE_GEN_HIGH_SHIFT	52
 
+#define MMIO_GEN_SHIFT			19
 #define MMIO_GEN_LOW_SHIFT		9
 #define MMIO_GEN_LOW_MASK		((1 << MMIO_GEN_LOW_SHIFT) - 1)
-#define MMIO_MAX_GEN			((1 << 19) - 1)
+#define MMIO_GEN_MASK			((1 << MMIO_GEN_SHIFT) - 1)
+#define MMIO_MAX_GEN			((1 << MMIO_GEN_SHIFT) - 1)
 
 static u64 generation_mmio_spte_mask(unsigned int gen)
 {

@@ -231,17 +233,23 @@ static unsigned int get_mmio_spte_generation(u64 spte)
 	return gen;
 }
 
+static unsigned int kvm_current_mmio_generation(struct kvm *kvm)
+{
+	return kvm_memslots(kvm)->generation & MMIO_GEN_MASK;
+}
+
 static void mark_mmio_spte(struct kvm *kvm, u64 *sptep, u64 gfn,
 			   unsigned access)
 {
 	struct kvm_mmu_page *sp =  page_header(__pa(sptep));
-	u64 mask = generation_mmio_spte_mask(0);
+	unsigned int gen = kvm_current_mmio_generation(kvm);
+	u64 mask = generation_mmio_spte_mask(gen);
 
 	access &= ACC_WRITE_MASK | ACC_USER_MASK;
 	mask |= shadow_mmio_mask | access | gfn << PAGE_SHIFT;
 	sp->mmio_cached = true;
 
-	trace_mark_mmio_spte(sptep, gfn, access, 0);
+	trace_mark_mmio_spte(sptep, gfn, access, gen);
 	mmu_spte_set(sptep, mask);
 }
 

@@ -271,6 +279,12 @@ static bool set_mmio_spte(struct kvm *kvm, u64 *sptep, gfn_t gfn,
 	return false;
 }
 
+static bool check_mmio_spte(struct kvm *kvm, u64 spte)
+{
+	return likely(get_mmio_spte_generation(spte) ==
+			kvm_current_mmio_generation(kvm));
+}
+
 static inline u64 rsvd_bits(int s, int e)
 {
 	return ((1ULL << (e - s + 1)) - 1) << s;

@@ -3235,6 +3249,9 @@ int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct)
 		gfn_t gfn = get_mmio_spte_gfn(spte);
 		unsigned access = get_mmio_spte_access(spte);
 
+		if (!check_mmio_spte(vcpu->kvm, spte))
+			return RET_MMIO_PF_INVALID;
+
 		if (direct)
 			addr = 0;
 

@@ -3276,8 +3293,12 @@ static int nonpaging_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
 
 	pgprintk("%s: gva %lx error %x\n", __func__, gva, error_code);
 
-	if (unlikely(error_code & PFERR_RSVD_MASK))
-		return handle_mmio_page_fault(vcpu, gva, error_code, true);
+	if (unlikely(error_code & PFERR_RSVD_MASK)) {
+		r = handle_mmio_page_fault(vcpu, gva, error_code, true);
+
+		if (likely(r != RET_MMIO_PF_INVALID))
+			return r;
+	}
 
 	r = mmu_topup_memory_caches(vcpu);
 	if (r)

@@ -3353,8 +3374,12 @@ static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa, u32 error_code,
 	ASSERT(vcpu);
 	ASSERT(VALID_PAGE(vcpu->arch.mmu.root_hpa));
 
-	if (unlikely(error_code & PFERR_RSVD_MASK))
-		return handle_mmio_page_fault(vcpu, gpa, error_code, true);
+	if (unlikely(error_code & PFERR_RSVD_MASK)) {
+		r = handle_mmio_page_fault(vcpu, gpa, error_code, true);
+
+		if (likely(r != RET_MMIO_PF_INVALID))
+			return r;
+	}
 
 	r = mmu_topup_memory_caches(vcpu);
 	if (r)

@@ -4327,7 +4352,7 @@ void kvm_mmu_invalidate_zap_all_pages(struct kvm *kvm)
 	spin_unlock(&kvm->mmu_lock);
 }
 
-void kvm_mmu_zap_mmio_sptes(struct kvm *kvm)
+static void kvm_mmu_zap_mmio_sptes(struct kvm *kvm)
 {
 	struct kvm_mmu_page *sp, *node;
 	LIST_HEAD(invalid_list);

@@ -4350,6 +4375,19 @@ static bool kvm_has_zapped_obsolete_pages(struct kvm *kvm)
 	return unlikely(!list_empty_careful(&kvm->arch.zapped_obsolete_pages));
 }
 
+void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm)
+{
+	/*
+	 * The very rare case: if the generation-number is round,
+	 * zap all shadow pages.
+	 *
+	 * The max value is MMIO_MAX_GEN - 1 since it is not called
+	 * when mark memslot invalid.
+	 */
+	if (unlikely(kvm_current_mmio_generation(kvm) >= (MMIO_MAX_GEN - 1)))
+		kvm_mmu_zap_mmio_sptes(kvm);
+}
+
 static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc)
 {
 	struct kvm *kvm;

arch/x86/kvm/mmu.h +4 -1

index ba6a19c..5b59c57 100644
@@ -56,12 +56,15 @@ void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask);
 /*
  * Return values of handle_mmio_page_fault_common:
  * RET_MMIO_PF_EMULATE: it is a real mmio page fault, emulate the instruction
- *			 directly.
+ *			directly.
+ * RET_MMIO_PF_INVALID: invalid spte is detected then let the real page
+ *			fault path update the mmio spte.
  * RET_MMIO_PF_RETRY: let CPU fault again on the address.
  * RET_MMIO_PF_BUG: bug is detected.
  */
 enum {
 	RET_MMIO_PF_EMULATE = 1,
+	RET_MMIO_PF_INVALID = 2,
 	RET_MMIO_PF_RETRY = 0,
 	RET_MMIO_PF_BUG = -1
 };

arch/x86/kvm/paging_tmpl.h +5 -2

index fb50fa6..7769699 100644
@@ -552,9 +552,12 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 
 	pgprintk("%s: addr %lx err %x\n", __func__, addr, error_code);
 
-	if (unlikely(error_code & PFERR_RSVD_MASK))
-		return handle_mmio_page_fault(vcpu, addr, error_code,
+	if (unlikely(error_code & PFERR_RSVD_MASK)) {
+		r = handle_mmio_page_fault(vcpu, addr, error_code,
 					      mmu_is_nested(vcpu));
+		if (likely(r != RET_MMIO_PF_INVALID))
+			return r;
+	};
 
 	r = mmu_topup_memory_caches(vcpu);
 	if (r)

arch/x86/kvm/vmx.c +4 -0

index 85c8d51..f4a5b3f 100644
@@ -5369,6 +5369,10 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 	if (likely(ret == RET_MMIO_PF_EMULATE))
 		return x86_emulate_instruction(vcpu, gpa, 0, NULL, 0) ==
 					      EMULATE_DONE;
+
+	if (unlikely(ret == RET_MMIO_PF_INVALID))
+		return kvm_mmu_page_fault(vcpu, gpa, 0, NULL, 0);
+
 	if (unlikely(ret == RET_MMIO_PF_RETRY))
 		return 1;
 

arch/x86/kvm/x86.c +1 -2

index 54059ba..9e4afa7 100644
@@ -7067,8 +7067,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 	 * If memory slot is created, or moved, we need to clear all
 	 * mmio sptes.
 	 */
-	if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE))
-		kvm_mmu_zap_mmio_sptes(kvm);
+	kvm_mmu_invalidate_mmio_sptes(kvm);
 }
 
 void kvm_arch_flush_shadow_all(struct kvm *kvm)

Signature · 1 line

1.8.1.4

Quoted 13 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Quoted 3 lines

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Quoted 17 lines · Gleb Natapov

Quoted 16 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Quoted 3 lines

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Something wrong with gfn calculation during mmio:

qemu-system-x86-17003 [000] 3962.625103: handle_mmio_page_fault: addr:c00ba6c0 gfn 100000000ba access a92 qemu-system-x86-17003 [000] 3962.774862: handle_mmio_page_fault: addr:ffffb170 gfn 100000fee00 access a92

Quoted 21 lines · Gleb Natapov

Quoted 20 lines · Gleb Natapov

Quoted 7 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

Quoted 9 lines

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Quoted 5 lines

Something wrong with gfn calculation during mmio:

qemu-system-x86-17003 [000] 3962.625103: handle_mmio_page_fault: addr:c00ba6c0 gfn 100000000ba access a92 qemu-system-x86-17003 [000] 3962.774862: handle_mmio_page_fault: addr:ffffb170 gfn 100000fee00 access a92

Hmm, so I wounder why get_mmio_spte_gfn() does not clear gen bits.

Quoted 22 lines · Gleb Natapov

Quoted 21 lines · Gleb Natapov

Quoted 20 lines · Gleb Natapov

Quoted 7 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

Quoted 9 lines

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Quoted 7 lines

Quoted 5 lines

Something wrong with gfn calculation during mmio:

qemu-system-x86-17003 [000] 3962.625103: handle_mmio_page_fault: addr:c00ba6c0 gfn 100000000ba access a92 qemu-system-x86-17003 [000] 3962.774862: handle_mmio_page_fault: addr:ffffb170 gfn 100000fee00 access a92

Hmm, so I wounder why get_mmio_spte_gfn() does not clear gen bits.

Hmm, something like patch below fixes it. Will test more.

arch/x86/kvm/mmu.c +0 -0

index 1fd2c05..aec9c05 100644

Quoted 39 lines · Gleb Natapov

Quoted 22 lines · Gleb Natapov

Quoted 21 lines · Gleb Natapov

Quoted 20 lines · Gleb Natapov

Quoted 7 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

Quoted 9 lines

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Quoted 7 lines

Quoted 5 lines

Something wrong with gfn calculation during mmio:

qemu-system-x86-17003 [000] 3962.625103: handle_mmio_page_fault: addr:c00ba6c0 gfn 100000000ba access a92 qemu-system-x86-17003 [000] 3962.774862: handle_mmio_page_fault: addr:ffffb170 gfn 100000fee00 access a92

Hmm, so I wounder why get_mmio_spte_gfn() does not clear gen bits.

Hmm, something like patch below fixes it. Will test more.

arch/x86/kvm/mmu.c +0 -0

index 1fd2c05..aec9c05 100644
@@ -260,7 +260,8 @@ static bool is_mmio_spte(u64 spte)
 

arch/x86/kvm/mmu.c static bool is_mmio_spte(u64 spte) · line 265

Quoted 6 lines

arch/x86/kvm/mmu.c +2 -1

 static gfn_t get_mmio_spte_gfn(u64 spte)
 {
-	return (spte & ~shadow_mmio_mask) >> PAGE_SHIFT;
+	u64 mask = generation_mmio_spte_mask(MMIO_MAX_GEN) | shadow_mmio_mask;
+	return (spte & ~mask) >> PAGE_SHIFT;
 }

Looks nice.

Quoted 40 lines · Xiao Guangrong

Quoted 39 lines · Gleb Natapov

Quoted 22 lines · Gleb Natapov

Quoted 21 lines · Gleb Natapov

Quoted 20 lines · Gleb Natapov

Quoted 7 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

Quoted 9 lines

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Quoted 7 lines

Quoted 5 lines

Something wrong with gfn calculation during mmio:

qemu-system-x86-17003 [000] 3962.625103: handle_mmio_page_fault: addr:c00ba6c0 gfn 100000000ba access a92 qemu-system-x86-17003 [000] 3962.774862: handle_mmio_page_fault: addr:ffffb170 gfn 100000fee00 access a92

Hmm, so I wounder why get_mmio_spte_gfn() does not clear gen bits.

Hmm, something like patch below fixes it. Will test more.

arch/x86/kvm/mmu.c +0 -0

index 1fd2c05..aec9c05 100644
@@ -260,7 +260,8 @@ static bool is_mmio_spte(u64 spte)
 

Quoted 9 lines

arch/x86/kvm/mmu.c static bool is_mmio_spte(u64 spte) · line 265

Quoted 6 lines

arch/x86/kvm/mmu.c +2 -1

 static gfn_t get_mmio_spte_gfn(u64 spte)
 {
-	return (spte & ~shadow_mmio_mask) >> PAGE_SHIFT;
+	u64 mask = generation_mmio_spte_mask(MMIO_MAX_GEN) | shadow_mmio_mask;
+	return (spte & ~mask) >> PAGE_SHIFT;
 }

Looks nice.

The question is if get_mmio_spte_access() need the same treatment?

Quoted 50 lines · Gleb Natapov

Quoted 49 lines · Xiao Guangrong

Quoted 38 lines · Gleb Natapov

Quoted 22 lines · Gleb Natapov

Quoted 21 lines · Gleb Natapov

Quoted 20 lines · Gleb Natapov

Quoted 7 lines · Xiao Guangrong

This patch tries to introduce a very simple and scale way to invalidate all mmio sptes - it need not walk any shadow pages and hold mmu-lock

KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

Quoted 9 lines

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

So this commit makes Fedora 9 32 bit reboot during boot, Fedora 9 64 fails too, but I haven't checked what happens exactly.

Quoted 7 lines

Quoted 5 lines

Something wrong with gfn calculation during mmio:

qemu-system-x86-17003 [000] 3962.625103: handle_mmio_page_fault: addr:c00ba6c0 gfn 100000000ba access a92 qemu-system-x86-17003 [000] 3962.774862: handle_mmio_page_fault: addr:ffffb170 gfn 100000fee00 access a92

Hmm, so I wounder why get_mmio_spte_gfn() does not clear gen bits.

Hmm, something like patch below fixes it. Will test more.

arch/x86/kvm/mmu.c +0 -0

index 1fd2c05..aec9c05 100644

arch/x86/kvm/mmu.c static bool is_mmio_spte(u64 spte) · line 265

Quoted 7 lines

arch/x86/kvm/mmu.c +2 -1

@@ -260,7 +260,8 @@ static bool is_mmio_spte(u64 spte)
 
 static gfn_t get_mmio_spte_gfn(u64 spte)
 {
-	return (spte & ~shadow_mmio_mask) >> PAGE_SHIFT;
+	u64 mask = generation_mmio_spte_mask(MMIO_MAX_GEN) | shadow_mmio_mask;
+	return (spte & ~mask) >> PAGE_SHIFT;
 }

Looks nice.

Quoted 1 line

The question is if get_mmio_spte_access() need the same treatment?

It works okay since the Access only uses bit1 and bit2 (and in the direct mmu case, only use gfn). But i am happy to do the same change in get_mmio_spte_access() to make the code more clear.

Changelog:
V3:
All of these changes are from Gleb's review:
1) rename RET_MMIO_PF_EMU to RET_MMIO_PF_EMULATE.
2) smartly adjust kvm generation number in kvm_current_mmio_generatio()

     to avoid kvm_memslots->generation overflow.

V2:
- rename kvm_mmu_invalid_mmio_spte to kvm_mmu_invalid_mmio_sptes
- use kvm->memslots->generation as kvm global generation-number
- fix comment and codestyle
- init kvm generation close to mmio wrap-around value
- keep kvm_mmu_zap_mmio_sptes

The current way is holding hot mmu-lock and walking all shadow pages, this is not scale. This patchset tries to introduce a very simple and scale way to fast invalidate all mmio sptes - it need not walk any shadow pages and hold any locks.

The idea is simple:
KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Xiao Guangrong (6):
KVM: MMU: retain more available bits on mmio spte
KVM: MMU: store generation-number into mmio spte
KVM: MMU: make return value of mmio page fault handler more readable
KVM: MMU: fast invalidate all mmio sptes
KVM: MMU: add tracepoint for check_mmio_spte
KVM: MMU: init kvm generation close to mmio wrap-around value

 arch/x86/include/asm/kvm_host.h |   2 +-
 arch/x86/kvm/mmu.c              | 131 ++++++++++++++++++++++++++++++++--------
 arch/x86/kvm/mmu.h              |  17 ++++++
 arch/x86/kvm/mmutrace.h         |  34 +++++++++--
 arch/x86/kvm/paging_tmpl.h      |  10 ++-
 arch/x86/kvm/vmx.c              |  12 ++--
 arch/x86/kvm/x86.c              |  11 +++-
 7 files changed, 177 insertions(+), 40 deletions(-)

Signature · 1 line

1.8.1.4

Quoted 30 lines · Xiao Guangrong

Changelog:
V3:
All of these changes are from Gleb's review:
1) rename RET_MMIO_PF_EMU to RET_MMIO_PF_EMULATE.
2) smartly adjust kvm generation number in kvm_current_mmio_generatio()

     to avoid kvm_memslots->generation overflow.

V2:
- rename kvm_mmu_invalid_mmio_spte to kvm_mmu_invalid_mmio_sptes
- use kvm->memslots->generation as kvm global generation-number
- fix comment and codestyle
- init kvm generation close to mmio wrap-around value
- keep kvm_mmu_zap_mmio_sptes

The current way is holding hot mmu-lock and walking all shadow pages, this is not scale. This patchset tries to introduce a very simple and scale way to fast invalidate all mmio sptes - it need not walk any shadow pages and hold any locks.

The idea is simple:
KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Quoted 3 lines

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Looks good to me, but doesn't tis obsolete kvm_mmu_zap_mmio_sptes() and sp->mmio_cached, so they should be removed as part of the patch series?

From bc1bc36e2640059f06c4860af802ecc74e1f3d2d Mon Sep 17 00:00:00 2001

Quoted 4 lines · Gleb Natapov

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

Looks good to me, but doesn't tis obsolete kvm_mmu_zap_mmio_sptes() and sp->mmio_cached, so they should be removed as part of the patch series?

Yes, i agree, they should be removed. :)

There is the patch to do these things:

From bc1bc36e2640059f06c4860af802ecc74e1f3d2d Mon Sep 17 00:00:00 2001
From: Xiao Guangrong <[email protected]>
Date: Mon, 10 Jun 2013 16:28:55 +0800
Subject: [PATCH 7/6] KVM: MMU: drop kvm_mmu_zap_mmio_sptes

Drop kvm_mmu_zap_mmio_sptes and use kvm_mmu_invalidate_zap_all_pages instead to handle mmio generation number overflow

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |  1 -
 arch/x86/kvm/mmu.c              | 22 +---------------------
 2 files changed, 1 insertion(+), 22 deletions(-)

arch/x86/include/asm/kvm_host.h +0 -1

index 90d05ed..966f265 100644
@@ -230,7 +230,6 @@ struct kvm_mmu_page {
 #endif
 
 	int write_flooding_count;
-	bool mmio_cached;
 };
 
 struct kvm_pio_request {

arch/x86/kvm/mmu.c +1 -21

index 35cd0b6..c87b19d 100644
@@ -246,13 +246,11 @@ static unsigned int kvm_current_mmio_generation(struct kvm *kvm)
 static void mark_mmio_spte(struct kvm *kvm, u64 *sptep, u64 gfn,
 			   unsigned access)
 {
-	struct kvm_mmu_page *sp =  page_header(__pa(sptep));
 	unsigned int gen = kvm_current_mmio_generation(kvm);
 	u64 mask = generation_mmio_spte_mask(gen);
 
 	access &= ACC_WRITE_MASK | ACC_USER_MASK;
 	mask |= shadow_mmio_mask | access | gfn << PAGE_SHIFT;
-	sp->mmio_cached = true;
 
 	trace_mark_mmio_spte(sptep, gfn, access, gen);
 	mmu_spte_set(sptep, mask);

@@ -4362,24 +4360,6 @@ void kvm_mmu_invalidate_zap_all_pages(struct kvm *kvm)
 	spin_unlock(&kvm->mmu_lock);
 }
 
-static void kvm_mmu_zap_mmio_sptes(struct kvm *kvm)
-{
-	struct kvm_mmu_page *sp, *node;
-	LIST_HEAD(invalid_list);
-
-	spin_lock(&kvm->mmu_lock);
-restart:
-	list_for_each_entry_safe(sp, node, &kvm->arch.active_mmu_pages, link) {
-		if (!sp->mmio_cached)
-			continue;
-		if (kvm_mmu_prepare_zap_page(kvm, sp, &invalid_list))
-			goto restart;
-	}
-
-	kvm_mmu_commit_zap_page(kvm, &invalid_list);
-	spin_unlock(&kvm->mmu_lock);
-}
-
 static bool kvm_has_zapped_obsolete_pages(struct kvm *kvm)
 {
 	return unlikely(!list_empty_careful(&kvm->arch.zapped_obsolete_pages));

@@ -4395,7 +4375,7 @@ void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm)
 	 * when mark memslot invalid.
 	 */
 	if (unlikely(kvm_current_mmio_generation(kvm) >= (MMIO_MAX_GEN - 1)))
-		kvm_mmu_zap_mmio_sptes(kvm);
+		kvm_mmu_invalidate_zap_all_pages(kvm);
 }
 
 static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc)

Signature · 7 lines

1.8.1.4


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 7 lines · On Mon, 10 Jun 2013 16:39:37 +0800 Xiao Guangrong…

Quoted 4 lines · Gleb Natapov

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

Looks good to me, but doesn't tis obsolete kvm_mmu_zap_mmio_sptes() and sp->mmio_cached, so they should be removed as part of the patch series?

Yes, i agree, they should be removed. :)

I'm fine with removing it but please make it clear that you all agree on the same basis.

Last time, Paolo mentioned the possibility to use some bits of spte for other things. The suggestion there was to keep sp->mmio_cached code for the time we would need to reduce the bits for generation numbers.

Do you think that zap_all() is now preemptible and can treat the situation reasonably well as the current kvm_mmu_zap_mmio_sptes()?

One downside is the need to zap unrelated shadow pages, but if this case is really very rare, yes I agree, it should not be a problem: it depends on how many bits we can use.

Just please reconfirm.

Quoted 25 lines

Quoted 7 lines · On Mon, 10 Jun 2013 16:39:37 +0800 Xiao Guangrong…

Quoted 4 lines · Gleb Natapov

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

Looks good to me, but doesn't tis obsolete kvm_mmu_zap_mmio_sptes() and sp->mmio_cached, so they should be removed as part of the patch series?

Yes, i agree, they should be removed. :)

I'm fine with removing it but please make it clear that you all agree on the same basis.

Last time, Paolo mentioned the possibility to use some bits of spte for other things. The suggestion there was to keep sp->mmio_cached code for the time we would need to reduce the bits for generation numbers.

Do you think that zap_all() is now preemptible and can treat the situation reasonably well as the current kvm_mmu_zap_mmio_sptes()?

One downside is the need to zap unrelated shadow pages, but if this case is really very rare, yes I agree, it should not be a problem: it depends on how many bits we can use.

Quoted 2 lines

Just please reconfirm.

That was me who mention the possibility to use some bits of spte for other things. But for now I have a use for one bit only. Now that you have reminded me about that discussion I am not so sure we want to remove kvm_mmu_zap_mmio_sptes(), but on the other hand it is non preemptable, so large number of mmio sptes can cause soft lockups. zap_all() is better in this regards now.

Il 10/06/2013 19:03, Gleb Natapov ha scritto:

Quoted 26 lines · Takuya Yoshikawa

Quoted 25 lines

Quoted 7 lines · On Mon, 10 Jun 2013 16:39:37 +0800 Xiao Guangrong…

Quoted 4 lines · Gleb Natapov

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

Looks good to me, but doesn't tis obsolete kvm_mmu_zap_mmio_sptes() and sp->mmio_cached, so they should be removed as part of the patch series?

Yes, i agree, they should be removed. :)

I'm fine with removing it but please make it clear that you all agree on the same basis.

Last time, Paolo mentioned the possibility to use some bits of spte for other things. The suggestion there was to keep sp->mmio_cached code for the time we would need to reduce the bits for generation numbers.

Do you think that zap_all() is now preemptible and can treat the situation reasonably well as the current kvm_mmu_zap_mmio_sptes()?

One downside is the need to zap unrelated shadow pages, but if this case is really very rare, yes I agree, it should not be a problem: it depends on how many bits we can use.

Quoted 8 lines

Quoted 2 lines

Just please reconfirm.

That was me who mention the possibility to use some bits of spte for other things. But for now I have a use for one bit only. Now that you have reminded me about that discussion I am not so sure we want to remove kvm_mmu_zap_mmio_sptes(), but on the other hand it is non preemptable, so large number of mmio sptes can cause soft lockups. zap_all() is better in this regards now.

I asked Gleb on IRC, and he's fine with applying patch 7 too (otherwise there's hardly any benefit, because kvm_mmu_zap_mmio_sptes is non-preemptable).

I'm also changing the -13 to -150 since it's quite easy to generate 150 calls to KVM_SET_USER_MEMORY_REGION. Using QEMU, and for a pretty basic guest with virtio-net, IDE controller and VGA you get:

- 9-10 calls before starting the guest, depending on the guest memory size

- around 25 during the BIOS

- around 20 during kernel boot

- 34 during a single dump of the 64 KB ROM from a virtio-net device.

Changelog:
V3:
All of these changes are from Gleb's review:
1) rename RET_MMIO_PF_EMU to RET_MMIO_PF_EMULATE.
2) smartly adjust kvm generation number in kvm_current_mmio_generatio()

     to avoid kvm_memslots->generation overflow.

V2:
- rename kvm_mmu_invalid_mmio_spte to kvm_mmu_invalid_mmio_sptes
- use kvm->memslots->generation as kvm global generation-number
- fix comment and codestyle
- init kvm generation close to mmio wrap-around value
- keep kvm_mmu_zap_mmio_sptes

The current way is holding hot mmu-lock and walking all shadow pages, this is not scale. This patchset tries to introduce a very simple and scale way to fast invalidate all mmio sptes - it need not walk any shadow pages and hold any locks.

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

- Where is the generation number increased?

- Should use spinlock breakable code in kvm_mmu_zap_mmio_sptes() (picture guest with 512GB of RAM, even walking all those pages is expensive) (ah, patch to remove kvm_mmu_zap_mmio_sptes does that).
- Is -13 enough to test wraparound? Its highly likely the guest has not began executing by the time 13 kvm_set_memory_calls are made (so no sptes around). Perhaps -2000 is more sensible (should confirm though).

- Why remove "if (change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)" from kvm_arch_commit_memory_region? Its instructive.

Quoted 3 lines · On Thu, 13 Jun 2013 21:08:21 -0300 Marcelo Tosatti <[email protected]> wrote:

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

- Where is the generation number increased?

Looks like when a new slot is installed in update_memslots() because it's based on slots->generation. This is not restricted to "create" and "move".

Quoted 8 lines

- Should use spinlock breakable code in kvm_mmu_zap_mmio_sptes() (picture guest with 512GB of RAM, even walking all those pages is expensive) (ah, patch to remove kvm_mmu_zap_mmio_sptes does that).
- Is -13 enough to test wraparound? Its highly likely the guest has not began executing by the time 13 kvm_set_memory_calls are made (so no sptes around). Perhaps -2000 is more sensible (should confirm though).

In the future, after we've tested enough, we should change the testing code to be executed only for some debugging configs. Especially, if we change zap_mmio_sptes() to zap_all_shadows(), very common guests, even without huge memory like 512GB, can see the effect induced by sudden page faults unnecessarily.

If necessary, developers can test the wraparound code by lowering the max_gen itself anyway.

Quoted 4 lines

- Why remove "if (change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)" from kvm_arch_commit_memory_region? Its instructive.

There may be a chance that we miss generation wraparounds if we don't check other cases: seems unlikely, but theoretically possible.

In short, all memory slot changes make mmio sptes stored in shadow pages obsolete, or zapped for wraparounds, in the new way -- am I right?

Sorry for the delay reply since i was on vacation.

Quoted 10 lines · Takuya Yoshikawa

Quoted 3 lines · On Thu, 13 Jun 2013 21:08:21 -0300 Marcelo Tosatti <[email protected]> wrote:

On Fri, Jun 07, 2013 at 04:51:22PM +0800, Xiao Guangrong wrote:

- Where is the generation number increased?

Looks like when a new slot is installed in update_memslots() because it's based on slots->generation. This is not restricted to "create" and "move".

Yes. It reuses slots->generation to avoid unnecessary synchronizations (RCU, memory barrier).

Increasing mmio generation number in the case of "create" and "move" is ok - it is no addition work unless mmio generation number is overflow which is hardly triggered (since the valid mmio generation number is large enough and zap_all is scale well now.) and the mmio spte is updated only when it is used in the future.

Quoted 9 lines

Quoted 8 lines

- Should use spinlock breakable code in kvm_mmu_zap_mmio_sptes() (picture guest with 512GB of RAM, even walking all those pages is expensive) (ah, patch to remove kvm_mmu_zap_mmio_sptes does that).
- Is -13 enough to test wraparound? Its highly likely the guest has not began executing by the time 13 kvm_set_memory_calls are made (so no sptes around). Perhaps -2000 is more sensible (should confirm though).

Quoted 8 lines

In the future, after we've tested enough, we should change the testing code to be executed only for some debugging configs. Especially, if we change zap_mmio_sptes() to zap_all_shadows(), very common guests, even without huge memory like 512GB, can see the effect induced by sudden page faults unnecessarily.

If necessary, developers can test the wraparound code by lowering the max_gen itself anyway.

I agree.

Quoted 10 lines

Quoted 4 lines

- Why remove "if (change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)" from kvm_arch_commit_memory_region? Its instructive.

There may be a chance that we miss generation wraparounds if we don't check other cases: seems unlikely, but theoretically possible.

In short, all memory slot changes make mmio sptes stored in shadow pages obsolete, or zapped for wraparounds, in the new way -- am I right?

Yes. You are definitely right. :)

Takuya-san, thank you very much for you answering the questions for me and thanks all of you for patiently reviewing my patches.

Marcelo, your points?

Changelog:
V3:
All of these changes are from Gleb's review:
1) rename RET_MMIO_PF_EMU to RET_MMIO_PF_EMULATE.
2) smartly adjust kvm generation number in kvm_current_mmio_generatio()

     to avoid kvm_memslots->generation overflow.

V2:
- rename kvm_mmu_invalid_mmio_spte to kvm_mmu_invalid_mmio_sptes
- use kvm->memslots->generation as kvm global generation-number
- fix comment and codestyle
- init kvm generation close to mmio wrap-around value
- keep kvm_mmu_zap_mmio_sptes

The current way is holding hot mmu-lock and walking all shadow pages, this is not scale. This patchset tries to introduce a very simple and scale way to fast invalidate all mmio sptes - it need not walk any shadow pages and hold any locks.

The idea is simple:
KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Xiao Guangrong (6):
KVM: MMU: retain more available bits on mmio spte
KVM: MMU: store generation-number into mmio spte
KVM: MMU: make return value of mmio page fault handler more readable
KVM: MMU: fast invalidate all mmio sptes
KVM: MMU: add tracepoint for check_mmio_spte
KVM: MMU: init kvm generation close to mmio wrap-around value

 arch/x86/include/asm/kvm_host.h |   2 +-
 arch/x86/kvm/mmu.c              | 131 ++++++++++++++++++++++++++++++++--------
 arch/x86/kvm/mmu.h              |  17 ++++++
 arch/x86/kvm/mmutrace.h         |  34 +++++++++--
 arch/x86/kvm/paging_tmpl.h      |  10 ++-
 arch/x86/kvm/vmx.c              |  12 ++--
 arch/x86/kvm/x86.c              |  11 +++-
 7 files changed, 177 insertions(+), 40 deletions(-)

Il 07/06/2013 10:51, Xiao Guangrong ha scritto:

Quoted 41 lines

Changelog:
V3:
All of these changes are from Gleb's review:
1) rename RET_MMIO_PF_EMU to RET_MMIO_PF_EMULATE.
2) smartly adjust kvm generation number in kvm_current_mmio_generatio()

     to avoid kvm_memslots->generation overflow.

V2:
- rename kvm_mmu_invalid_mmio_spte to kvm_mmu_invalid_mmio_sptes
- use kvm->memslots->generation as kvm global generation-number
- fix comment and codestyle
- init kvm generation close to mmio wrap-around value
- keep kvm_mmu_zap_mmio_sptes

The current way is holding hot mmu-lock and walking all shadow pages, this is not scale. This patchset tries to introduce a very simple and scale way to fast invalidate all mmio sptes - it need not walk any shadow pages and hold any locks.

The idea is simple:
KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Xiao Guangrong (6):
KVM: MMU: retain more available bits on mmio spte
KVM: MMU: store generation-number into mmio spte
KVM: MMU: make return value of mmio page fault handler more readable
KVM: MMU: fast invalidate all mmio sptes
KVM: MMU: add tracepoint for check_mmio_spte
KVM: MMU: init kvm generation close to mmio wrap-around value

Quoted 9 lines

 arch/x86/include/asm/kvm_host.h |   2 +-
 arch/x86/kvm/mmu.c              | 131 ++++++++++++++++++++++++++++++++--------
 arch/x86/kvm/mmu.h              |  17 ++++++
 arch/x86/kvm/mmutrace.h         |  34 +++++++++--
 arch/x86/kvm/paging_tmpl.h      |  10 ++-
 arch/x86/kvm/vmx.c              |  12 ++--
 arch/x86/kvm/x86.c              |  11 +++-
 7 files changed, 177 insertions(+), 40 deletions(-)

Xiao, is it time to add more comments to the code or update Documentation/virtual/kvm/mmu.txt?

Don't worry about the English, it is
more than understandable and I can help with the editing.

Changelog:
V3:
All of these changes are from Gleb's review:
1) rename RET_MMIO_PF_EMU to RET_MMIO_PF_EMULATE.
2) smartly adjust kvm generation number in kvm_current_mmio_generatio()

     to avoid kvm_memslots->generation overflow.

V2:
- rename kvm_mmu_invalid_mmio_spte to kvm_mmu_invalid_mmio_sptes
- use kvm->memslots->generation as kvm global generation-number
- fix comment and codestyle
- init kvm generation close to mmio wrap-around value
- keep kvm_mmu_zap_mmio_sptes

The current way is holding hot mmu-lock and walking all shadow pages, this is not scale. This patchset tries to introduce a very simple and scale way to fast invalidate all mmio sptes - it need not walk any shadow pages and hold any locks.

The idea is simple:
KVM maintains a global mmio valid generation-number which is stored in kvm->memslots.generation and every mmio spte stores the current global generation-number into his available bits when it is created

When KVM need zap all mmio sptes, it just simply increase the global generation-number. When guests do mmio access, KVM intercepts a MMIO #PF then it walks the shadow page table and get the mmio spte. If the generation-number on the spte does not equal the global generation-number, it will go to the normal #PF handler to update the mmio spte

Since 19 bits are used to store generation-number on mmio spte, we zap all mmio sptes when the number is round

Xiao Guangrong (6):
KVM: MMU: retain more available bits on mmio spte
KVM: MMU: store generation-number into mmio spte
KVM: MMU: make return value of mmio page fault handler more readable
KVM: MMU: fast invalidate all mmio sptes
KVM: MMU: add tracepoint for check_mmio_spte
KVM: MMU: init kvm generation close to mmio wrap-around value

 arch/x86/include/asm/kvm_host.h |   2 +-
 arch/x86/kvm/mmu.c              | 131 ++++++++++++++++++++++++++++++++--------
 arch/x86/kvm/mmu.h              |  17 ++++++
 arch/x86/kvm/mmutrace.h         |  34 +++++++++--
 arch/x86/kvm/paging_tmpl.h      |  10 ++-
 arch/x86/kvm/vmx.c              |  12 ++--
 arch/x86/kvm/x86.c              |  11 +++-
 7 files changed, 177 insertions(+), 40 deletions(-)