Emulator Speedups - Optimize Instruction fetches

Insn fetch fastpath function. Not that arch.walk_mmu->gva_to_gpa can't be used but let's piggyback on top of interface meant for our purpose

Signed-off-by Bandan Das <[email protected]>

---
 arch/x86/kvm/x86.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

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

index 17e3d661..cf69e3b 100644
@@ -4065,29 +4065,38 @@ static int kvm_read_guest_virt_helper(gva_t addr, void *val, unsigned int bytes,
 				      struct x86_exception *exception)
 {
 	void *data = val;
-	struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
 	int r = X86EMUL_CONTINUE;
 
 	while (bytes) {
-		gpa_t gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, addr, access,
-							    exception);
 		unsigned offset = addr & (PAGE_SIZE-1);

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

+		if (ret != X86EMUL_CONTINUE)
+			return ret;
+
+		if (unlikely(kvm_is_error_hva(uaddr))) {
+			r = X86EMUL_PROPAGATE_FAULT;
+			return r;

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

 		bytes -= toread;
 		data += toread;
 		addr += toread;
 	}
-out:
+
 	return r;
 }
 

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

+		ret = __copy_from_user(data, (void __user *)uaddr, toread);

On every instruction fetch, kvm_read_guest_virt_helper does the gva to gpa translation followed by searching for the memslot. Store the gva hva mapping so that if there's a match we can directly call __copy_from_user()

Suggested-by Paolo Bonzini <[email protected]>

Signed-off-by Bandan Das <[email protected]>

---
 arch/x86/include/asm/kvm_emulate.h |  7 ++++++-
 arch/x86/kvm/x86.c                 | 33 +++++++++++++++++++++++----------
 2 files changed, 29 insertions(+), 11 deletions(-)

arch/x86/include/asm/kvm_emulate.h +0 -0

index 085d688..20ccde4 100644

arch/x86/include/asm/kvm_emulate.h +0 -0

 };
 
 /* Repeat String Operation Prefix */

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

index cf69e3b..7afcfc7 100644
@@ -4072,26 +4072,38 @@ static int kvm_read_guest_virt_helper(gva_t addr, void *val, unsigned int bytes,
 		unsigned toread = min(bytes, (unsigned)PAGE_SIZE - offset);
 		int ret;
 		unsigned long uaddr;
+		gfn_t gfn = addr >> PAGE_SHIFT;
 
-		ret = ctxt->ops->memory_prepare(ctxt, addr, toread,
-						exception, false,
-						NULL, &uaddr);

arch/x86/kvm/x86.c +14 -5

+			if (ret != X86EMUL_CONTINUE)
+				return ret;
+
+			if (unlikely(kvm_is_error_hva(uaddr))) {
+				r = X86EMUL_PROPAGATE_FAULT;
+				return r;
+			}
 
-		if (unlikely(kvm_is_error_hva(uaddr))) {
-			r = X86EMUL_PROPAGATE_FAULT;
-			return r;
+			/* Cache gfn and hva */
+			ctxt->addr_cache.gfn = addr >> PAGE_SHIFT;
+			ctxt->addr_cache.uaddr = uaddr >> PAGE_SHIFT;
+			ctxt->addr_cache_valid = true;
 		}
 
 		ret = __copy_from_user(data, (void __user *)uaddr, toread);
 		if (ret < 0) {
 			r = X86EMUL_IO_NEEDED;
+			/* Where else should we invalidate cache ? */
+			ctxt->ops->memory_finish(ctxt, NULL, uaddr);
 			return r;
 		}
 
-		ctxt->ops->memory_finish(ctxt, NULL, uaddr);
-
 		bytes -= toread;
 		data += toread;
 		addr += toread;

@@ -4339,6 +4351,7 @@ static void emulator_memory_finish(struct x86_emulate_ctxt *ctxt,
 	struct kvm_memory_slot *memslot;
 	gfn_t gfn;
 
+	ctxt->addr_cache_valid = false;
 	if (!opaque)
 		return;
 

Il 06/05/2014 02:40, Bandan Das ha scritto:

Quoted 17 lines

On every instruction fetch, kvm_read_guest_virt_helper does the gva to gpa translation followed by searching for the memslot. Store the gva hva mapping so that if there's a match we can directly call __copy_from_user()

Suggested-by Paolo Bonzini <[email protected]>

Signed-off-by Bandan Das <[email protected]>

---
 arch/x86/include/asm/kvm_emulate.h |  7 ++++++-
 arch/x86/kvm/x86.c                 | 33 +++++++++++++++++++++++----------
 2 files changed, 29 insertions(+), 11 deletions(-)

arch/x86/include/asm/kvm_emulate.h +0 -0

index 085d688..20ccde4 100644

arch/x86/include/asm/kvm_emulate.h struct x86_emulate_ctxt { · line 330

Quoted 9 lines

arch/x86/include/asm/kvm_emulate.h +2 -1

@@ -323,10 +323,11 @@ struct x86_emulate_ctxt {
 	int (*execute)(struct x86_emulate_ctxt *ctxt);
 	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
 	/*
-	 * The following five fields are cleared together,
+	 * The following six fields are cleared together,
 	 * the rest are initialized unconditionally in x86_decode_insn
 	 * or elsewhere
 	 */
+	bool addr_cache_valid;

You can just set gfn to -1 to invalidate the fetch.

arch/x86/include/asm/kvm_emulate.h struct x86_emulate_ctxt { · line 355

Quoted 11 lines

arch/x86/include/asm/kvm_emulate.h +4 -0

 	u8 rex_prefix;
 	u8 lock_prefix;
 	u8 rep_prefix;

@@ -348,6 +349,10 @@ struct x86_emulate_ctxt {
 	struct fetch_cache fetch;
 	struct read_cache io_read;
 	struct read_cache mem_read;
+	struct {
+		gfn_t gfn;
+		unsigned long uaddr;
+	} addr_cache;

This is not used by emulate.c, so it should be directly in struct kvm_vcpu. You could invalidate it in init_emulate_ctxt, together with emulate_regs_need_sync_from_vcpu.

In the big picture, however, what we really want is to persist the cache across multiple instructions, and also cache the linearization of the address (where you add RIP and CS.base). This would be a bigger source of improvement. If you do that, the cache has to be indeed in x86_emulate_ctxt, but on the other hand the implementation needs to be in emulate.c.

Quoted 13 lines

arch/x86/include/asm/kvm_emulate.h +0 -0

 };
 
 /* Repeat String Operation Prefix */

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

index cf69e3b..7afcfc7 100644
@@ -4072,26 +4072,38 @@ static int kvm_read_guest_virt_helper(gva_t addr, void *val, unsigned int bytes,
 		unsigned toread = min(bytes, (unsigned)PAGE_SIZE - offset);
 		int ret;
 		unsigned long uaddr;
+		gfn_t gfn = addr >> PAGE_SHIFT;
 

arch/x86/kvm/x86.c static int kvm_read_guest_virt_helper(gva_t addr, void *val, unsigned int bytes, · line 4084

Quoted 13 lines

arch/x86/kvm/x86.c +8 -5

-		ret = ctxt->ops->memory_prepare(ctxt, addr, toread,
-						exception, false,
-						NULL, &uaddr);
-		if (ret != X86EMUL_CONTINUE)
-			return ret;
+		if (ctxt->addr_cache_valid &&
+		    (ctxt->addr_cache.gfn == gfn))
+			uaddr = (ctxt->addr_cache.uaddr << PAGE_SHIFT) +
+				offset_in_page(addr);
+		else {
+			ret = ctxt->ops->memory_prepare(ctxt, addr, toread,
+							exception, false,
+							NULL, &uaddr);

Did you measure the hit rate, and the speedup after every patch? My reading of the code is that the fetch is done only once per page, with

the speedup coming from the microoptimization that patch 2 provides for single-page reads. Single-page reads are indeed a very common case for the emulator.

I suggest to start with making patch 2 ready for inclusion.

Paolo

Quoted 39 lines

arch/x86/kvm/x86.c +14 -5

+			if (ret != X86EMUL_CONTINUE)
+				return ret;
+
+			if (unlikely(kvm_is_error_hva(uaddr))) {
+				r = X86EMUL_PROPAGATE_FAULT;
+				return r;
+			}
 
-		if (unlikely(kvm_is_error_hva(uaddr))) {
-			r = X86EMUL_PROPAGATE_FAULT;
-			return r;
+			/* Cache gfn and hva */
+			ctxt->addr_cache.gfn = addr >> PAGE_SHIFT;
+			ctxt->addr_cache.uaddr = uaddr >> PAGE_SHIFT;
+			ctxt->addr_cache_valid = true;
 		}
 
 		ret = __copy_from_user(data, (void __user *)uaddr, toread);
 		if (ret < 0) {
 			r = X86EMUL_IO_NEEDED;
+			/* Where else should we invalidate cache ? */
+			ctxt->ops->memory_finish(ctxt, NULL, uaddr);
 			return r;
 		}
 
-		ctxt->ops->memory_finish(ctxt, NULL, uaddr);
-
 		bytes -= toread;
 		data += toread;
 		addr += toread;

@@ -4339,6 +4351,7 @@ static void emulator_memory_finish(struct x86_emulate_ctxt *ctxt,
 	struct kvm_memory_slot *memslot;
 	gfn_t gfn;
 
+	ctxt->addr_cache_valid = false;
 	if (!opaque)
 		return;