Re: [Qemu-ppc] [RFC PATCH 06/17] KVM: PPC: e500: Add emulation helper for getting instruction ea

[Alexander Graf]

On 25.06.2012, at 14:26, Mihai Caraman wrote:

> Add emulation helper for getting instruction ea and refactor tlb instruction
> emulation to use it.
> 
> Signed-off-by: Mihai Caraman <address@hidden>
> ---
> arch/powerpc/kvm/e500.h         |    6 +++---
> arch/powerpc/kvm/e500_emulate.c |   21 ++++++++++++++++++---
> arch/powerpc/kvm/e500_tlb.c     |   23 ++++++-----------------
> 3 files changed, 27 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
> index 3e31098..70bfed4 100644
> --- a/arch/powerpc/kvm/e500.h
> +++ b/arch/powerpc/kvm/e500.h
> @@ -130,9 +130,9 @@ int kvmppc_e500_emul_mt_mmucsr0(struct kvmppc_vcpu_e500 
> *vcpu_e500,
>                               ulong value);
> int kvmppc_e500_emul_tlbwe(struct kvm_vcpu *vcpu);
> int kvmppc_e500_emul_tlbre(struct kvm_vcpu *vcpu);
> -int kvmppc_e500_emul_tlbivax(struct kvm_vcpu *vcpu, int ra, int rb);
> -int kvmppc_e500_emul_tlbilx(struct kvm_vcpu *vcpu, int rt, int ra, int rb);
> -int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, int rb);
> +int kvmppc_e500_emul_tlbivax(struct kvm_vcpu *vcpu, gva_t ea);
> +int kvmppc_e500_emul_tlbilx(struct kvm_vcpu *vcpu, int rt, gva_t ea);
> +int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, gva_t ea);
> int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500);
> void kvmppc_e500_tlb_uninit(struct kvmppc_vcpu_e500 *vcpu_e500);
> 
> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
> index 8b99e07..81288f7 100644
> --- a/arch/powerpc/kvm/e500_emulate.c
> +++ b/arch/powerpc/kvm/e500_emulate.c
> @@ -82,6 +82,17 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, 
> int rb)
> }
> #endif
> 
> +static inline ulong kvmppc_get_ea_indexed(struct kvm_vcpu *vcpu, int ra, int 
> rb)
> +{
> +     ulong ea;
> +
> +     ea = kvmppc_get_gpr(vcpu, rb);
> +     if (ra)
> +             ea += kvmppc_get_gpr(vcpu, ra);
> +
> +     return ea;
> +}
> +

Please move this one to arch/powerpc/include/asm/kvm_ppc.h.

> int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
>                            unsigned int inst, int *advance)
> {
> @@ -89,6 +100,7 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>       int ra = get_ra(inst);
>       int rb = get_rb(inst);
>       int rt = get_rt(inst);
> +     gva_t ea;
> 
>       switch (get_op(inst)) {
>       case 31:
> @@ -113,15 +125,18 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>                       break;
> 
>               case XOP_TLBSX:
> -                     emulated = kvmppc_e500_emul_tlbsx(vcpu,rb);
> +                     ea = kvmppc_get_ea_indexed(vcpu, ra, rb);
> +                     emulated = kvmppc_e500_emul_tlbsx(vcpu, ea);
>                       break;
> 
>               case XOP_TLBILX:
> -                     emulated = kvmppc_e500_emul_tlbilx(vcpu, rt, ra, rb);
> +                     ea = kvmppc_get_ea_indexed(vcpu, ra, rb);
> +                     emulated = kvmppc_e500_emul_tlbilx(vcpu, rt, ea);

What's the point in hiding ra+rb, but not rt? I like the idea of hiding the 
register semantics, but please move rt into a local variable that gets passed 
as pointer to kvmppc_e500_emul_tlbilx.


Alex