RE: [RFC PATCH 06/17] KVM: PPC: e500: Add emulation helper for getting instruction ea

-----Original Message-----
From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
owner@vger.kernel.org] On Behalf Of Alexander Graf
Sent: Wednesday, July 04, 2012 4:56 PM
To: Caraman Mihai Claudiu-B02008
Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
dev@lists.ozlabs.org; qemu-ppc@nongnu.org
Subject: Re: [RFC PATCH 06/17] KVM: PPC: e500: Add emulation helper for
getting instruction ea
=20
=20
On 25.06.2012, at 14:26, Mihai Caraman wrote:
=20

quoted

Add emulation helper for getting instruction ea and refactor tlb

instruction

quoted

emulation to use it.

Signed-off-by: Mihai Caraman <redacted>
---
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,

quoted

				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);

quoted

-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

quoted

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)

quoted

}
#endif

+static inline ulong kvmppc_get_ea_indexed(struct kvm_vcpu *vcpu, int

ra, int rb)

quoted

+{
+	ulong ea;
+
+	ea =3D kvmppc_get_gpr(vcpu, rb);
+	if (ra)
+		ea +=3D kvmppc_get_gpr(vcpu, ra);
+
+	return ea;
+}
+

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

Yep. This is similar with what I had in my internal version before emulatio=
n
refactoring took place upstream. The only difference is that I split the em=
bedded
and server implementation touching this files:
	arch/powerpc/include/asm/kvm_booke.h
	arch/powerpc/include/asm/kvm_book3s.h

Which approach do you prefer?

=20

quoted

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,

quoted

	int ra =3D get_ra(inst);
	int rb =3D get_rb(inst);
	int rt =3D 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,

quoted

			break;

		case XOP_TLBSX:
-			emulated =3D kvmppc_e500_emul_tlbsx(vcpu,rb);
+			ea =3D kvmppc_get_ea_indexed(vcpu, ra, rb);
+			emulated =3D kvmppc_e500_emul_tlbsx(vcpu, ea);
			break;

		case XOP_TLBILX:
-			emulated =3D kvmppc_e500_emul_tlbilx(vcpu, rt, ra, rb);
+			ea =3D kvmppc_get_ea_indexed(vcpu, ra, rb);
+			emulated =3D kvmppc_e500_emul_tlbilx(vcpu, rt, ea);

=20
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.

Why to send it as a pointer? rt which should be rather named t in this case
is an [in] value for tlbilx, according to section 6.11.4.9 in the PowerISA =
2.06b.

-Mike