| Message ID | 1342683653-32114-4-git-send-email-gleb@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/19/2012 10:40 AM, Gleb Natapov wrote: > The functions will be used outside of the emulator. > > Signed-off-by: Gleb Natapov <gleb@redhat.com> > --- > arch/x86/include/asm/kvm_host.h | 25 +++++++++++++++++++++++++ > arch/x86/kvm/emulate.c | 15 ++------------- > 2 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index de2aff8..6212575 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -980,4 +980,29 @@ int kvm_pmu_read_pmc(struct kvm_vcpu *vcpu, unsigned pmc, u64 *data); > void kvm_handle_pmu_event(struct kvm_vcpu *vcpu); > void kvm_deliver_pmi(struct kvm_vcpu *vcpu); > > +static inline unsigned long kvm_ad_mask(u8 ad_bytes) > +{ > + return (1UL << (ad_bytes << 3)) - 1; > +} > + > +/* Access/update address held in a register, based on addressing mode. */ > +static inline unsigned long > +kvm_address_mask(u8 ad_bytes, unsigned long reg) > +{ > + if (ad_bytes == sizeof(unsigned long)) > + return reg; > + else > + return reg & kvm_ad_mask(ad_bytes); > +} > + > +static inline void > +kvm_register_address_increment(u8 ad_bytes, unsigned long *reg, int inc) > +{ > + if (ad_bytes == sizeof(unsigned long)) > + *reg += inc; > + else > + *reg = (*reg & ~kvm_ad_mask(ad_bytes)) | > + ((*reg + inc) & kvm_ad_mask(ad_bytes)); > +} > + > #endif /* _ASM_X86_KVM_HOST_H */ > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 79899df..e317588 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -433,19 +433,11 @@ static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt, > return ctxt->ops->intercept(ctxt, &info, stage); > } > > -static inline unsigned long ad_mask(struct x86_emulate_ctxt *ctxt) > -{ > - return (1UL << (ctxt->ad_bytes << 3)) - 1; > -} > - > /* Access/update address held in a register, based on addressing mode. */ > static inline unsigned long > address_mask(struct x86_emulate_ctxt *ctxt, unsigned long reg) > { > - if (ctxt->ad_bytes == sizeof(unsigned long)) > - return reg; > - else > - return reg & ad_mask(ctxt); > + return kvm_address_mask(ctxt->ad_bytes, reg); > } > > static inline unsigned long > @@ -457,10 +449,7 @@ register_address(struct x86_emulate_ctxt *ctxt, unsigned long reg) > static inline void > register_address_increment(struct x86_emulate_ctxt *ctxt, unsigned long *reg, int inc) > { > - if (ctxt->ad_bytes == sizeof(unsigned long)) > - *reg += inc; > - else > - *reg = (*reg & ~ad_mask(ctxt)) | ((*reg + inc) & ad_mask(ctxt)); > + return kvm_register_address_increment(ctxt->ad_bytes, reg, inc); > } All those exports suggest it's better to move the fast path into the emulator. Suppose we change register access to use callbacks instead of bulk load/save, could we reuse the exising code?
On Thu, Jul 19, 2012 at 01:42:31PM +0300, Avi Kivity wrote: > On 07/19/2012 10:40 AM, Gleb Natapov wrote: > > The functions will be used outside of the emulator. > > > > Signed-off-by: Gleb Natapov <gleb@redhat.com> > > --- > > arch/x86/include/asm/kvm_host.h | 25 +++++++++++++++++++++++++ > > arch/x86/kvm/emulate.c | 15 ++------------- > > 2 files changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index de2aff8..6212575 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -980,4 +980,29 @@ int kvm_pmu_read_pmc(struct kvm_vcpu *vcpu, unsigned pmc, u64 *data); > > void kvm_handle_pmu_event(struct kvm_vcpu *vcpu); > > void kvm_deliver_pmi(struct kvm_vcpu *vcpu); > > > > +static inline unsigned long kvm_ad_mask(u8 ad_bytes) > > +{ > > + return (1UL << (ad_bytes << 3)) - 1; > > +} > > + > > +/* Access/update address held in a register, based on addressing mode. */ > > +static inline unsigned long > > +kvm_address_mask(u8 ad_bytes, unsigned long reg) > > +{ > > + if (ad_bytes == sizeof(unsigned long)) > > + return reg; > > + else > > + return reg & kvm_ad_mask(ad_bytes); > > +} > > + > > +static inline void > > +kvm_register_address_increment(u8 ad_bytes, unsigned long *reg, int inc) > > +{ > > + if (ad_bytes == sizeof(unsigned long)) > > + *reg += inc; > > + else > > + *reg = (*reg & ~kvm_ad_mask(ad_bytes)) | > > + ((*reg + inc) & kvm_ad_mask(ad_bytes)); > > +} > > + > > #endif /* _ASM_X86_KVM_HOST_H */ > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > > index 79899df..e317588 100644 > > --- a/arch/x86/kvm/emulate.c > > +++ b/arch/x86/kvm/emulate.c > > @@ -433,19 +433,11 @@ static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt, > > return ctxt->ops->intercept(ctxt, &info, stage); > > } > > > > -static inline unsigned long ad_mask(struct x86_emulate_ctxt *ctxt) > > -{ > > - return (1UL << (ctxt->ad_bytes << 3)) - 1; > > -} > > - > > /* Access/update address held in a register, based on addressing mode. */ > > static inline unsigned long > > address_mask(struct x86_emulate_ctxt *ctxt, unsigned long reg) > > { > > - if (ctxt->ad_bytes == sizeof(unsigned long)) > > - return reg; > > - else > > - return reg & ad_mask(ctxt); > > + return kvm_address_mask(ctxt->ad_bytes, reg); > > } > > > > static inline unsigned long > > @@ -457,10 +449,7 @@ register_address(struct x86_emulate_ctxt *ctxt, unsigned long reg) > > static inline void > > register_address_increment(struct x86_emulate_ctxt *ctxt, unsigned long *reg, int inc) > > { > > - if (ctxt->ad_bytes == sizeof(unsigned long)) > > - *reg += inc; > > - else > > - *reg = (*reg & ~ad_mask(ctxt)) | ((*reg + inc) & ad_mask(ctxt)); > > + return kvm_register_address_increment(ctxt->ad_bytes, reg, inc); > > } > > All those exports suggest it's better to move the fast path into the > emulator. We've already being through that. Putting the code into emulator gives us nothing unless it also works on emulator context and working on partially initialized emulator context is first dangerous and second slower. > Suppose we change register access to use callbacks instead of > bulk load/save, could we reuse the exising code? > I do not see that problem. This helper function do basic arithmetics on three values. The value itself will be fetched on demand by the emulator. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/19/2012 01:49 PM, Gleb Natapov wrote: >> All those exports suggest it's better to move the fast path into the >> emulator. > > We've already being through that. Putting the code into emulator gives > us nothing unless it also works on emulator context and working on > partially initialized emulator context is first dangerous and second > slower. We can make it work on an x86_pio_ctxt. What it gives us is common code. (x86_emulate_ctxt can include a x86_pio_ctxt when it does pio). > >> Suppose we change register access to use callbacks instead of >> bulk load/save, could we reuse the exising code? >> > I do not see that problem. This helper function do basic arithmetics > on three values. The value itself will be fetched on demand by the emulator. I meant to reduce the emulator initialization cost, so it isn't slow. btw, I'm guessing that the main speedup comes not from avoiding the decode, but by doing page-at-a-time instead of word-at-a-time.
On Thu, Jul 19, 2012 at 04:34:50PM +0300, Avi Kivity wrote: > On 07/19/2012 01:49 PM, Gleb Natapov wrote: > >> All those exports suggest it's better to move the fast path into the > >> emulator. > > > > We've already being through that. Putting the code into emulator gives > > us nothing unless it also works on emulator context and working on > > partially initialized emulator context is first dangerous and second > > slower. > > We can make it work on an x86_pio_ctxt. What it gives us is common > code. (x86_emulate_ctxt can include a x86_pio_ctxt when it does pio). > My patches do similar thing, but instead of x86_pio_ctxt they use x86_linearize_ctx. The code is common. > > > >> Suppose we change register access to use callbacks instead of > >> bulk load/save, could we reuse the exising code? > >> > > I do not see that problem. This helper function do basic arithmetics > > on three values. The value itself will be fetched on demand by the emulator. > > I meant to reduce the emulator initialization cost, so it isn't slow. > > btw, I'm guessing that the main speedup comes not from avoiding the > decode, but by doing page-at-a-time instead of word-at-a-time. > And hacking emulator doing page at a time is not trivial an complicates it for no good reason. BTW you were the one who suggested implementing fast pio as a separate code path. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index de2aff8..6212575 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -980,4 +980,29 @@ int kvm_pmu_read_pmc(struct kvm_vcpu *vcpu, unsigned pmc, u64 *data); void kvm_handle_pmu_event(struct kvm_vcpu *vcpu); void kvm_deliver_pmi(struct kvm_vcpu *vcpu); +static inline unsigned long kvm_ad_mask(u8 ad_bytes) +{ + return (1UL << (ad_bytes << 3)) - 1; +} + +/* Access/update address held in a register, based on addressing mode. */ +static inline unsigned long +kvm_address_mask(u8 ad_bytes, unsigned long reg) +{ + if (ad_bytes == sizeof(unsigned long)) + return reg; + else + return reg & kvm_ad_mask(ad_bytes); +} + +static inline void +kvm_register_address_increment(u8 ad_bytes, unsigned long *reg, int inc) +{ + if (ad_bytes == sizeof(unsigned long)) + *reg += inc; + else + *reg = (*reg & ~kvm_ad_mask(ad_bytes)) | + ((*reg + inc) & kvm_ad_mask(ad_bytes)); +} + #endif /* _ASM_X86_KVM_HOST_H */ diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 79899df..e317588 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -433,19 +433,11 @@ static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt, return ctxt->ops->intercept(ctxt, &info, stage); } -static inline unsigned long ad_mask(struct x86_emulate_ctxt *ctxt) -{ - return (1UL << (ctxt->ad_bytes << 3)) - 1; -} - /* Access/update address held in a register, based on addressing mode. */ static inline unsigned long address_mask(struct x86_emulate_ctxt *ctxt, unsigned long reg) { - if (ctxt->ad_bytes == sizeof(unsigned long)) - return reg; - else - return reg & ad_mask(ctxt); + return kvm_address_mask(ctxt->ad_bytes, reg); } static inline unsigned long @@ -457,10 +449,7 @@ register_address(struct x86_emulate_ctxt *ctxt, unsigned long reg) static inline void register_address_increment(struct x86_emulate_ctxt *ctxt, unsigned long *reg, int inc) { - if (ctxt->ad_bytes == sizeof(unsigned long)) - *reg += inc; - else - *reg = (*reg & ~ad_mask(ctxt)) | ((*reg + inc) & ad_mask(ctxt)); + return kvm_register_address_increment(ctxt->ad_bytes, reg, inc); } static inline void jmp_rel(struct x86_emulate_ctxt *ctxt, int rel)
The functions will be used outside of the emulator. Signed-off-by: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_host.h | 25 +++++++++++++++++++++++++ arch/x86/kvm/emulate.c | 15 ++------------- 2 files changed, 27 insertions(+), 13 deletions(-)