[PATCH 1/2] KVM: x86 emulator: Clean up init_emulate_ctxt()

[PATCH 1/2] KVM: x86 emulator: Clean up init_emulate_ctxt()

[Takuya Yoshikawa]

From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

Use a local pointer to the emulate_ctxt for simplicity.  Then, arrange
the hard-to-read mode selection lines neatly.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 arch/x86/kvm/x86.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index da48622..e7d337e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4508,7 +4508,8 @@ static void inject_emulated_exception(struct kvm_vcpu *vcpu)
 
 static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 {
-	struct decode_cache *c = &vcpu->arch.emulate_ctxt.decode;
+	struct x86_emulate_ctxt *ctxt = &vcpu->arch.emulate_ctxt;
+	struct decode_cache *c = &ctxt->decode;
 	int cs_db, cs_l;
 
 	/*
@@ -4521,15 +4522,15 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 
 	kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l);
 
-	vcpu->arch.emulate_ctxt.eflags = kvm_get_rflags(vcpu);
-	vcpu->arch.emulate_ctxt.eip = kvm_rip_read(vcpu);
-	vcpu->arch.emulate_ctxt.mode =
-		(!is_protmode(vcpu)) ? X86EMUL_MODE_REAL :
-		(vcpu->arch.emulate_ctxt.eflags & X86_EFLAGS_VM)
-		? X86EMUL_MODE_VM86 : cs_l
-		? X86EMUL_MODE_PROT64 :	cs_db
-		? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
-	vcpu->arch.emulate_ctxt.guest_mode = is_guest_mode(vcpu);
+	ctxt->eflags = kvm_get_rflags(vcpu);
+	ctxt->eip = kvm_rip_read(vcpu);
+	ctxt->mode = (!is_protmode(vcpu))		? X86EMUL_MODE_REAL :
+		     (ctxt->eflags & X86_EFLAGS_VM)	? X86EMUL_MODE_VM86 :
+		     cs_l				? X86EMUL_MODE_PROT64 :
+		     cs_db				? X86EMUL_MODE_PROT32 :
+							  X86EMUL_MODE_PROT16;
+	ctxt->guest_mode = is_guest_mode(vcpu);
+
 	memset(c, 0, sizeof(struct decode_cache));
 	memcpy(c->regs, vcpu->arch.regs, sizeof c->regs);
 	vcpu->arch.emulate_regs_need_sync_from_vcpu = false;
-- 
1.7.4.1

[PATCH 2/2] KVM: x86 emulator: Avoid clearing the whole decode_cache

[Takuya Yoshikawa]

From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

During tracing the emulator, we noticed that init_emulate_ctxt()
sometimes took a bit longer time than we expected.

This patch is for mitigating the problem by some degree.

By looking into the function, we soon notice that it clears the whole
decode_cache whose size is about 2.5K bytes now.  Furthermore, most of
the bytes are taken for the two read_cache arrays, which are used only
by a few instructions.

Considering the fact that we are not assuming the cache arrays have
been cleared when we store actual data, we do not need to clear the
arrays: 2K bytes elimination.  In addition, we can avoid clearing the
fetch_cache and regs arrays.

This patch changes the initialization not to clear the arrays.

On our 64-bit host, init_emulate_ctxt() becomes 0.3 to 0.5us faster with
this patch applied.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Cc: Gleb Natapov <gleb@redhat.com>
---
 arch/x86/include/asm/kvm_emulate.h |    5 +++--
 arch/x86/kvm/x86.c                 |   17 +++++++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index ab09ba2..c0f77e0 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -246,8 +246,6 @@ struct decode_cache {
 	unsigned int d;
 	int (*execute)(struct x86_emulate_ctxt *ctxt);
 	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
-	unsigned long regs[NR_VCPU_REGS];
-	unsigned long eip;
 	/* modrm */
 	u8 modrm;
 	u8 modrm_mod;
@@ -255,6 +253,9 @@ struct decode_cache {
 	u8 modrm_rm;
 	u8 modrm_seg;
 	bool rip_relative;
+	unsigned long eip;
+	/* Fields above regs are cleared together. */
+	unsigned long regs[NR_VCPU_REGS];
 	struct fetch_cache fetch;
 	struct read_cache io_read;
 	struct read_cache mem_read;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e7d337e..57f4160 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4506,6 +4506,20 @@ static void inject_emulated_exception(struct kvm_vcpu *vcpu)
 		kvm_queue_exception(vcpu, ctxt->exception.vector);
 }
 
+static void init_decode_cache(struct decode_cache *c,
+			      const unsigned long *regs)
+{
+	memset(c, 0, offsetof(struct decode_cache, regs));
+	memcpy(c->regs, regs, sizeof(c->regs));
+
+	c->fetch.start = 0;
+	c->fetch.end = 0;
+	c->io_read.pos = 0;
+	c->io_read.end = 0;
+	c->mem_read.pos = 0;
+	c->mem_read.end = 0;
+}
+
 static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 {
 	struct x86_emulate_ctxt *ctxt = &vcpu->arch.emulate_ctxt;
@@ -4531,8 +4545,7 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 							  X86EMUL_MODE_PROT16;
 	ctxt->guest_mode = is_guest_mode(vcpu);
 
-	memset(c, 0, sizeof(struct decode_cache));
-	memcpy(c->regs, vcpu->arch.regs, sizeof c->regs);
+	init_decode_cache(c, vcpu->arch.regs);
 	vcpu->arch.emulate_regs_need_sync_from_vcpu = false;
 }
 
-- 
1.7.4.1

Re: [PATCH 2/2] KVM: x86 emulator: Avoid clearing the whole decode_cache

[Avi Kivity]

On 05/25/2011 05:09 AM, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>
> During tracing the emulator, we noticed that init_emulate_ctxt()
> sometimes took a bit longer time than we expected.
>
> This patch is for mitigating the problem by some degree.
>
> By looking into the function, we soon notice that it clears the whole
> decode_cache whose size is about 2.5K bytes now.  Furthermore, most of
> the bytes are taken for the two read_cache arrays, which are used only
> by a few instructions.
>
> Considering the fact that we are not assuming the cache arrays have
> been cleared when we store actual data, we do not need to clear the
> arrays: 2K bytes elimination.  In addition, we can avoid clearing the
> fetch_cache and regs arrays.
>
> This patch changes the initialization not to clear the arrays.
>
> On our 64-bit host, init_emulate_ctxt() becomes 0.3 to 0.5us faster with
> this patch applied.
>

Thanks, applied.

It strikes me that initializing the emulator in x86.c is the wrong 
thing.  We should move the entire thing to x86_decode_insn().

We'll need a few more callbacks for that, though (register, eflags); 
eventually we can read just registers that are used and write only 
registers that were updated.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 2/2] KVM: x86 emulator: Avoid clearing the whole decode_cache

[Gleb Natapov]

On Thu, May 26, 2011 at 11:19:03AM +0300, Avi Kivity wrote:
> On 05/25/2011 05:09 AM, Takuya Yoshikawa wrote:
> >From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
> >
> >During tracing the emulator, we noticed that init_emulate_ctxt()
> >sometimes took a bit longer time than we expected.
> >
> >This patch is for mitigating the problem by some degree.
> >
> >By looking into the function, we soon notice that it clears the whole
> >decode_cache whose size is about 2.5K bytes now.  Furthermore, most of
> >the bytes are taken for the two read_cache arrays, which are used only
> >by a few instructions.
> >
> >Considering the fact that we are not assuming the cache arrays have
> >been cleared when we store actual data, we do not need to clear the
> >arrays: 2K bytes elimination.  In addition, we can avoid clearing the
> >fetch_cache and regs arrays.
> >
> >This patch changes the initialization not to clear the arrays.
> >
> >On our 64-bit host, init_emulate_ctxt() becomes 0.3 to 0.5us faster with
> >this patch applied.
> >
> 
> Thanks, applied.
> 
> It strikes me that initializing the emulator in x86.c is the wrong
> thing.  We should move the entire thing to x86_decode_insn().
> 
We initialize it in task switch and interrupt injection code too.

> We'll need a few more callbacks for that, though (register, eflags);
> eventually we can read just registers that are used and write only
> registers that were updated.
> 

--
			Gleb.