From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCHv4 3/5] KVM: emulator: move some address manipulation function out of emulator code. Date: Thu, 19 Jul 2012 13:49:51 +0300 Message-ID: <20120719104951.GU26120@redhat.com> References: <1342683653-32114-1-git-send-email-gleb@redhat.com> <1342683653-32114-4-git-send-email-gleb@redhat.com> <5007E497.1040905@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, mtosatti@redhat.com To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:32857 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752265Ab2GSKtw (ORCPT ); Thu, 19 Jul 2012 06:49:52 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q6JAnq4V012041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 19 Jul 2012 06:49:52 -0400 Content-Disposition: inline In-Reply-To: <5007E497.1040905@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 > > --- > > 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.