* [RFC PATCH 0/3] Emulator Speedups - Optimize Instruction fetches
@ 2014-05-06 0:40 Bandan Das
2014-05-06 0:40 ` [RFC PATCH 1/3] KVM: x86: pass ctxt to fetch helper function Bandan Das
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Bandan Das @ 2014-05-06 0:40 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Marcelo Tosatti, Gleb Natapov, linux-kernel
My initial attempt at caching gva->gpa->hva translations. Pretty straight
forward with details in the individual patches.
I haven't yet looked into if there are other possibilities
to speed things up, just thought of sending these out since
the numbers are better
567 cycles/emulated jump instruction
718 cycles/emulated move instruction
730 cycles/emulated arithmetic instruction
946 cycles/emulated memory load instruction
956 cycles/emulated memory store instruction
921 cycles/emulated memory RMW instruction
Old realmode.flat numbers from init ctxt changes -
https://lkml.org/lkml/2014/4/16/848
639 cycles/emulated jump instruction (4.3%)
776 cycles/emulated move instruction (7.5%)
791 cycles/emulated arithmetic instruction (11%)
943 cycles/emulated memory load instruction (5.2%)
948 cycles/emulated memory store instruction (7.6%)
929 cycles/emulated memory RMW instruction (9.0%)
Bandan Das (3):
KVM: x86: pass ctxt to fetch helper function
KVM: x86: use memory_prepare in fetch helper function
KVM: x86: cache userspace address for faster fetches
arch/x86/include/asm/kvm_emulate.h | 7 +++++-
arch/x86/kvm/x86.c | 46 ++++++++++++++++++++++++++++----------
2 files changed, 40 insertions(+), 13 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/3] KVM: x86: pass ctxt to fetch helper function
2014-05-06 0:40 [RFC PATCH 0/3] Emulator Speedups - Optimize Instruction fetches Bandan Das
@ 2014-05-06 0:40 ` Bandan Das
2014-05-06 0:40 ` [RFC PATCH 2/3] KVM: x86: use memory_prepare in " Bandan Das
2014-05-06 0:40 ` [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches Bandan Das
2 siblings, 0 replies; 8+ messages in thread
From: Bandan Das @ 2014-05-06 0:40 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Marcelo Tosatti, Gleb Natapov, linux-kernel
In the following patches, our adress caching struct that's
embedded within struct x86_emulate_ctxt will need to be
accessed
Signed-off-by: Bandan Das <bsd@redhat.com>
---
arch/x86/kvm/x86.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 122410d..17e3d661 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4061,10 +4061,11 @@ gpa_t kvm_mmu_gva_to_gpa_system(struct kvm_vcpu *vcpu, gva_t gva,
}
static int kvm_read_guest_virt_helper(gva_t addr, void *val, unsigned int bytes,
- struct kvm_vcpu *vcpu, u32 access,
+ struct x86_emulate_ctxt *ctxt, u32 access,
struct x86_exception *exception)
{
void *data = val;
+ struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
int r = X86EMUL_CONTINUE;
while (bytes) {
@@ -4098,7 +4099,7 @@ static int kvm_fetch_guest_virt(struct x86_emulate_ctxt *ctxt,
struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
u32 access = (kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0;
- return kvm_read_guest_virt_helper(addr, val, bytes, vcpu,
+ return kvm_read_guest_virt_helper(addr, val, bytes, ctxt,
access | PFERR_FETCH_MASK,
exception);
}
@@ -4110,7 +4111,7 @@ int kvm_read_guest_virt(struct x86_emulate_ctxt *ctxt,
struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
u32 access = (kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0;
- return kvm_read_guest_virt_helper(addr, val, bytes, vcpu, access,
+ return kvm_read_guest_virt_helper(addr, val, bytes, ctxt, access,
exception);
}
EXPORT_SYMBOL_GPL(kvm_read_guest_virt);
@@ -4119,8 +4120,7 @@ static int kvm_read_guest_virt_system(struct x86_emulate_ctxt *ctxt,
gva_t addr, void *val, unsigned int bytes,
struct x86_exception *exception)
{
- struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
- return kvm_read_guest_virt_helper(addr, val, bytes, vcpu, 0, exception);
+ return kvm_read_guest_virt_helper(addr, val, bytes, ctxt, 0, exception);
}
int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/3] KVM: x86: use memory_prepare in fetch helper function
2014-05-06 0:40 [RFC PATCH 0/3] Emulator Speedups - Optimize Instruction fetches Bandan Das
2014-05-06 0:40 ` [RFC PATCH 1/3] KVM: x86: pass ctxt to fetch helper function Bandan Das
@ 2014-05-06 0:40 ` Bandan Das
2014-05-06 8:27 ` Paolo Bonzini
2014-05-06 9:46 ` Paolo Bonzini
2014-05-06 0:40 ` [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches Bandan Das
2 siblings, 2 replies; 8+ messages in thread
From: Bandan Das @ 2014-05-06 0:40 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Marcelo Tosatti, Gleb Natapov, linux-kernel
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 <bsd@redhat.com>
---
arch/x86/kvm/x86.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 17e3d661..cf69e3b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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);
unsigned toread = min(bytes, (unsigned)PAGE_SIZE - offset);
int ret;
+ unsigned long uaddr;
- if (gpa == UNMAPPED_GVA)
- return X86EMUL_PROPAGATE_FAULT;
- ret = kvm_read_guest(vcpu->kvm, gpa, data, toread);
+ ret = ctxt->ops->memory_prepare(ctxt, addr, toread,
+ exception, false,
+ NULL, &uaddr);
+ if (ret != X86EMUL_CONTINUE)
+ return ret;
+
+ if (unlikely(kvm_is_error_hva(uaddr))) {
+ r = X86EMUL_PROPAGATE_FAULT;
+ return r;
+ }
+
+ ret = __copy_from_user(data, (void __user *)uaddr, toread);
if (ret < 0) {
r = X86EMUL_IO_NEEDED;
- goto out;
+ return r;
}
+ ctxt->ops->memory_finish(ctxt, NULL, uaddr);
+
bytes -= toread;
data += toread;
addr += toread;
}
-out:
+
return r;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches
2014-05-06 0:40 [RFC PATCH 0/3] Emulator Speedups - Optimize Instruction fetches Bandan Das
2014-05-06 0:40 ` [RFC PATCH 1/3] KVM: x86: pass ctxt to fetch helper function Bandan Das
2014-05-06 0:40 ` [RFC PATCH 2/3] KVM: x86: use memory_prepare in " Bandan Das
@ 2014-05-06 0:40 ` Bandan Das
2014-05-06 9:40 ` Paolo Bonzini
2 siblings, 1 reply; 8+ messages in thread
From: Bandan Das @ 2014-05-06 0:40 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Marcelo Tosatti, Gleb Natapov, linux-kernel
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 <pbonzini@redhat.com>
Signed-off-by: Bandan Das <bsd@redhat.com>
---
arch/x86/include/asm/kvm_emulate.h | 7 ++++++-
arch/x86/kvm/x86.c | 33 +++++++++++++++++++++++----------
2 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 085d688..20ccde4 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -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;
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;
};
/* Repeat String Operation Prefix */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cf69e3b..7afcfc7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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);
- 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);
+ 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;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/3] KVM: x86: use memory_prepare in fetch helper function
2014-05-06 0:40 ` [RFC PATCH 2/3] KVM: x86: use memory_prepare in " Bandan Das
@ 2014-05-06 8:27 ` Paolo Bonzini
2014-05-06 9:46 ` Paolo Bonzini
1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-05-06 8:27 UTC (permalink / raw)
To: Bandan Das, kvm; +Cc: Marcelo Tosatti, Gleb Natapov, linux-kernel
Il 06/05/2014 02:40, Bandan Das ha scritto:
> 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 <bsd@redhat.com>
> ---
> arch/x86/kvm/x86.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 17e3d661..cf69e3b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -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);
> unsigned toread = min(bytes, (unsigned)PAGE_SIZE - offset);
> int ret;
> + unsigned long uaddr;
>
> - if (gpa == UNMAPPED_GVA)
> - return X86EMUL_PROPAGATE_FAULT;
> - ret = kvm_read_guest(vcpu->kvm, gpa, data, toread);
> + ret = ctxt->ops->memory_prepare(ctxt, addr, toread,
> + exception, false,
> + NULL, &uaddr);
You can call the function from x86.c directly, no need to go through
ctxt->ops.
> + if (ret != X86EMUL_CONTINUE)
> + return ret;
> +
> + if (unlikely(kvm_is_error_hva(uaddr))) {
> + r = X86EMUL_PROPAGATE_FAULT;
> + return r;
> + }
> +
> + ret = __copy_from_user(data, (void __user *)uaddr, toread);
> if (ret < 0) {
> r = X86EMUL_IO_NEEDED;
> - goto out;
> + return r;
> }
>
> + ctxt->ops->memory_finish(ctxt, NULL, uaddr);
No need to call memory_finish, since you know the implementation
(perhaps add a comment).
Paolo
> bytes -= toread;
> data += toread;
> addr += toread;
> }
> -out:
> +
> return r;
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches
2014-05-06 0:40 ` [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches Bandan Das
@ 2014-05-06 9:40 ` Paolo Bonzini
2014-05-07 4:45 ` Bandan Das
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2014-05-06 9:40 UTC (permalink / raw)
To: Bandan Das, kvm; +Cc: Marcelo Tosatti, Gleb Natapov, linux-kernel
Il 06/05/2014 02:40, Bandan Das ha scritto:
> 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 <pbonzini@redhat.com>
> Signed-off-by: Bandan Das <bsd@redhat.com>
> ---
> arch/x86/include/asm/kvm_emulate.h | 7 ++++++-
> arch/x86/kvm/x86.c | 33 +++++++++++++++++++++++----------
> 2 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> index 085d688..20ccde4 100644
> --- a/arch/x86/include/asm/kvm_emulate.h
> +++ b/arch/x86/include/asm/kvm_emulate.h
> @@ -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.
> 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.
> };
>
> /* Repeat String Operation Prefix */
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index cf69e3b..7afcfc7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -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);
> - 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
> + 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;
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/3] KVM: x86: use memory_prepare in fetch helper function
2014-05-06 0:40 ` [RFC PATCH 2/3] KVM: x86: use memory_prepare in " Bandan Das
2014-05-06 8:27 ` Paolo Bonzini
@ 2014-05-06 9:46 ` Paolo Bonzini
1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-05-06 9:46 UTC (permalink / raw)
To: Bandan Das, kvm; +Cc: Marcelo Tosatti, Gleb Natapov, linux-kernel
Il 06/05/2014 02:40, Bandan Das ha scritto:
> + ret = ctxt->ops->memory_prepare(ctxt, addr, toread,
> + exception, false,
> + NULL, &uaddr);
> + if (ret != X86EMUL_CONTINUE)
> + return ret;
> +
> + if (unlikely(kvm_is_error_hva(uaddr))) {
> + r = X86EMUL_PROPAGATE_FAULT;
> + return r;
> + }
What you are doing here is basically optimizing
kvm_read_guest_virt_helper because you know that all reads will be
single-page and you do not need the "next_segment" in kvm_read_guest.
Good catch, but you can use kvm_read_guest_page instead of going through
ctxt->ops->memory_prepare. :)
Paolo
> + ret = __copy_from_user(data, (void __user *)uaddr, toread);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches
2014-05-06 9:40 ` Paolo Bonzini
@ 2014-05-07 4:45 ` Bandan Das
0 siblings, 0 replies; 8+ messages in thread
From: Bandan Das @ 2014-05-07 4:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Marcelo Tosatti, Gleb Natapov, linux-kernel
Paolo Bonzini <pbonzini@redhat.com> writes:
> Il 06/05/2014 02:40, Bandan Das ha scritto:
>> 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 <pbonzini@redhat.com>
>> Signed-off-by: Bandan Das <bsd@redhat.com>
>> ---
>> arch/x86/include/asm/kvm_emulate.h | 7 ++++++-
>> arch/x86/kvm/x86.c | 33 +++++++++++++++++++++++----------
>> 2 files changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
>> index 085d688..20ccde4 100644
>> --- a/arch/x86/include/asm/kvm_emulate.h
>> +++ b/arch/x86/include/asm/kvm_emulate.h
>> @@ -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.
>
>> 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.
Ok.
> 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.
>
>> };
>>
>> /* Repeat String Operation Prefix */
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index cf69e3b..7afcfc7 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -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);
>> - 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
Yes, I did. IIRC, patch 3 actually improved the speedup compared to 2.
A rough estimate for jmp - patch 2 reduced it to the low 600s, I guess
around 610, but I could get a fresh set of numbers.
So, not sure where the speed up is coming from, I will try to find out.
> 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
>
>> + 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;
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-05-07 4:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-06 0:40 [RFC PATCH 0/3] Emulator Speedups - Optimize Instruction fetches Bandan Das
2014-05-06 0:40 ` [RFC PATCH 1/3] KVM: x86: pass ctxt to fetch helper function Bandan Das
2014-05-06 0:40 ` [RFC PATCH 2/3] KVM: x86: use memory_prepare in " Bandan Das
2014-05-06 8:27 ` Paolo Bonzini
2014-05-06 9:46 ` Paolo Bonzini
2014-05-06 0:40 ` [RFC PATCH 3/3] KVM: x86: cache userspace address for faster fetches Bandan Das
2014-05-06 9:40 ` Paolo Bonzini
2014-05-07 4:45 ` Bandan Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).