Re: [PATCHv4 3/5] KVM: emulator: move some address manipulation function out of emulator code.

[Avi Kivity <avi@redhat.com>]

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?


-- 
error compiling committee.c: too many arguments to function