public inbox for kvm-ppc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] KVM: PPC: Emulator: clean up instruction parsing
@ 2012-05-04 13:11 Alexander Graf
  2012-05-04 13:11 ` [PATCH 2/2] KVM: PPC: Emulator: clean up SPR reads and writes Alexander Graf
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Graf @ 2012-05-04 13:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: kvm list

Instructions on PPC are pretty similarly encoded. So instead of
every instruction emulation code decoding the instruction fields
itself, we can move that code to more generic places and rely on
the compiler to optimize the unused bits away.

This has 2 advantages. It makes the code smaller and it makes the
code less error prone, as the instruction fields are always
available, so accidental misusage is reduced.

Functionally, this patch doesn't change anything.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/kvm/44x_emulate.c    |   27 ++++----------
 arch/powerpc/kvm/book3s_emulate.c |   72 +++++++++++++++++++-----------------
 arch/powerpc/kvm/booke_emulate.c  |    7 +---
 arch/powerpc/kvm/e500_emulate.c   |   16 +++------
 arch/powerpc/kvm/emulate.c        |   71 ++----------------------------------
 5 files changed, 56 insertions(+), 137 deletions(-)

diff --git a/arch/powerpc/kvm/44x_emulate.c b/arch/powerpc/kvm/44x_emulate.c
index 549bb2c..da81a2d 100644
--- a/arch/powerpc/kvm/44x_emulate.c
+++ b/arch/powerpc/kvm/44x_emulate.c
@@ -37,22 +37,19 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                            unsigned int inst, int *advance)
 {
 	int emulated = EMULATE_DONE;
-	int dcrn;
-	int ra;
-	int rb;
-	int rc;
-	int rs;
-	int rt;
-	int ws;
+	int dcrn = get_dcrn(inst);
+	int ra = get_ra(inst);
+	int rb = get_rb(inst);
+	int rc = get_rc(inst);
+	int rs = get_rs(inst);
+	int rt = get_rt(inst);
+	int ws = get_ws(inst);
 
 	switch (get_op(inst)) {
 	case 31:
 		switch (get_xop(inst)) {
 
 		case XOP_MFDCR:
-			dcrn = get_dcrn(inst);
-			rt = get_rt(inst);
-
 			/* The guest may access CPR0 registers to determine the timebase
 			 * frequency, and it must know the real host frequency because it
 			 * can directly access the timebase registers.
@@ -88,9 +85,6 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			break;
 
 		case XOP_MTDCR:
-			dcrn = get_dcrn(inst);
-			rs = get_rs(inst);
-
 			/* emulate some access in kernel */
 			switch (dcrn) {
 			case DCRN_CPR0_CONFIG_ADDR:
@@ -108,17 +102,10 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			break;
 
 		case XOP_TLBWE:
-			ra = get_ra(inst);
-			rs = get_rs(inst);
-			ws = get_ws(inst);
 			emulated = kvmppc_44x_emul_tlbwe(vcpu, ra, rs, ws);
 			break;
 
 		case XOP_TLBSX:
-			rt = get_rt(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-			rc = get_rc(inst);
 			emulated = kvmppc_44x_emul_tlbsx(vcpu, rt, ra, rb, rc);
 			break;
 
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 135663a..c023bcd 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -87,6 +87,10 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                            unsigned int inst, int *advance)
 {
 	int emulated = EMULATE_DONE;
+	int rt = get_rt(inst);
+	int rs = get_rs(inst);
+	int ra = get_ra(inst);
+	int rb = get_rb(inst);
 
 	switch (get_op(inst)) {
 	case 19:
@@ -106,21 +110,22 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	case 31:
 		switch (get_xop(inst)) {
 		case OP_31_XOP_MFMSR:
-			kvmppc_set_gpr(vcpu, get_rt(inst),
-				       vcpu->arch.shared->msr);
+			kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->msr);
 			break;
 		case OP_31_XOP_MTMSRD:
 		{
-			ulong rs = kvmppc_get_gpr(vcpu, get_rs(inst));
+			ulong rs_val = kvmppc_get_gpr(vcpu, rs);
 			if (inst & 0x10000) {
-				vcpu->arch.shared->msr &= ~(MSR_RI | MSR_EE);
-				vcpu->arch.shared->msr |= rs & (MSR_RI | MSR_EE);
+				ulong new_msr = vcpu->arch.shared->msr;
+				new_msr &= ~(MSR_RI | MSR_EE);
+				new_msr |= rs_val & (MSR_RI | MSR_EE);
+				vcpu->arch.shared->msr = new_msr;
 			} else
-				kvmppc_set_msr(vcpu, rs);
+				kvmppc_set_msr(vcpu, rs_val);
 			break;
 		}
 		case OP_31_XOP_MTMSR:
-			kvmppc_set_msr(vcpu, kvmppc_get_gpr(vcpu, get_rs(inst)));
+			kvmppc_set_msr(vcpu, kvmppc_get_gpr(vcpu, rs));
 			break;
 		case OP_31_XOP_MFSR:
 		{
@@ -130,7 +135,7 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			if (vcpu->arch.mmu.mfsrin) {
 				u32 sr;
 				sr = vcpu->arch.mmu.mfsrin(vcpu, srnum);
-				kvmppc_set_gpr(vcpu, get_rt(inst), sr);
+				kvmppc_set_gpr(vcpu, rt, sr);
 			}
 			break;
 		}
@@ -138,29 +143,29 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		{
 			int srnum;
 
-			srnum = (kvmppc_get_gpr(vcpu, get_rb(inst)) >> 28) & 0xf;
+			srnum = (kvmppc_get_gpr(vcpu, rb) >> 28) & 0xf;
 			if (vcpu->arch.mmu.mfsrin) {
 				u32 sr;
 				sr = vcpu->arch.mmu.mfsrin(vcpu, srnum);
-				kvmppc_set_gpr(vcpu, get_rt(inst), sr);
+				kvmppc_set_gpr(vcpu, rt, sr);
 			}
 			break;
 		}
 		case OP_31_XOP_MTSR:
 			vcpu->arch.mmu.mtsrin(vcpu,
 				(inst >> 16) & 0xf,
-				kvmppc_get_gpr(vcpu, get_rs(inst)));
+				kvmppc_get_gpr(vcpu, rs));
 			break;
 		case OP_31_XOP_MTSRIN:
 			vcpu->arch.mmu.mtsrin(vcpu,
-				(kvmppc_get_gpr(vcpu, get_rb(inst)) >> 28) & 0xf,
-				kvmppc_get_gpr(vcpu, get_rs(inst)));
+				(kvmppc_get_gpr(vcpu, rb) >> 28) & 0xf,
+				kvmppc_get_gpr(vcpu, rs));
 			break;
 		case OP_31_XOP_TLBIE:
 		case OP_31_XOP_TLBIEL:
 		{
 			bool large = (inst & 0x00200000) ? true : false;
-			ulong addr = kvmppc_get_gpr(vcpu, get_rb(inst));
+			ulong addr = kvmppc_get_gpr(vcpu, rb);
 			vcpu->arch.mmu.tlbie(vcpu, addr, large);
 			break;
 		}
@@ -171,15 +176,15 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 				return EMULATE_FAIL;
 
 			vcpu->arch.mmu.slbmte(vcpu,
-					kvmppc_get_gpr(vcpu, get_rs(inst)),
-					kvmppc_get_gpr(vcpu, get_rb(inst)));
+					kvmppc_get_gpr(vcpu, rs),
+					kvmppc_get_gpr(vcpu, rb));
 			break;
 		case OP_31_XOP_SLBIE:
 			if (!vcpu->arch.mmu.slbie)
 				return EMULATE_FAIL;
 
 			vcpu->arch.mmu.slbie(vcpu,
-					kvmppc_get_gpr(vcpu, get_rb(inst)));
+					kvmppc_get_gpr(vcpu, rb));
 			break;
 		case OP_31_XOP_SLBIA:
 			if (!vcpu->arch.mmu.slbia)
@@ -191,22 +196,22 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			if (!vcpu->arch.mmu.slbmfee) {
 				emulated = EMULATE_FAIL;
 			} else {
-				ulong t, rb;
+				ulong t, rb_val;
 
-				rb = kvmppc_get_gpr(vcpu, get_rb(inst));
-				t = vcpu->arch.mmu.slbmfee(vcpu, rb);
-				kvmppc_set_gpr(vcpu, get_rt(inst), t);
+				rb_val = kvmppc_get_gpr(vcpu, rb);
+				t = vcpu->arch.mmu.slbmfee(vcpu, rb_val);
+				kvmppc_set_gpr(vcpu, rt, t);
 			}
 			break;
 		case OP_31_XOP_SLBMFEV:
 			if (!vcpu->arch.mmu.slbmfev) {
 				emulated = EMULATE_FAIL;
 			} else {
-				ulong t, rb;
+				ulong t, rb_val;
 
-				rb = kvmppc_get_gpr(vcpu, get_rb(inst));
-				t = vcpu->arch.mmu.slbmfev(vcpu, rb);
-				kvmppc_set_gpr(vcpu, get_rt(inst), t);
+				rb_val = kvmppc_get_gpr(vcpu, rb);
+				t = vcpu->arch.mmu.slbmfev(vcpu, rb_val);
+				kvmppc_set_gpr(vcpu, rt, t);
 			}
 			break;
 		case OP_31_XOP_DCBA:
@@ -214,17 +219,17 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			break;
 		case OP_31_XOP_DCBZ:
 		{
-			ulong rb = kvmppc_get_gpr(vcpu, get_rb(inst));
-			ulong ra = 0;
+			ulong rb_val = kvmppc_get_gpr(vcpu, rb);
+			ulong ra_val = 0;
 			ulong addr, vaddr;
 			u32 zeros[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
 			u32 dsisr;
 			int r;
 
-			if (get_ra(inst))
-				ra = kvmppc_get_gpr(vcpu, get_ra(inst));
+			if (ra)
+				ra_val = kvmppc_get_gpr(vcpu, ra);
 
-			addr = (ra + rb) & ~31ULL;
+			addr = (ra_val + rb_val) & ~31ULL;
 			if (!(vcpu->arch.shared->msr & MSR_SF))
 				addr &= 0xffffffff;
 			vaddr = addr;
@@ -565,23 +570,22 @@ u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
 ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 {
 	ulong dar = 0;
-	ulong ra;
+	ulong ra = get_ra(inst);
+	ulong rb = get_rb(inst);
 
 	switch (get_op(inst)) {
 	case OP_LFS:
 	case OP_LFD:
 	case OP_STFD:
 	case OP_STFS:
-		ra = get_ra(inst);
 		if (ra)
 			dar = kvmppc_get_gpr(vcpu, ra);
 		dar += (s32)((s16)inst);
 		break;
 	case 31:
-		ra = get_ra(inst);
 		if (ra)
 			dar = kvmppc_get_gpr(vcpu, ra);
-		dar += kvmppc_get_gpr(vcpu, get_rb(inst));
+		dar += kvmppc_get_gpr(vcpu, rb);
 		break;
 	default:
 		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 904412b..e14f7b2 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -40,8 +40,8 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                             unsigned int inst, int *advance)
 {
 	int emulated = EMULATE_DONE;
-	int rs;
-	int rt;
+	int rs = get_rs(inst);
+	int rt = get_rt(inst);
 
 	switch (get_op(inst)) {
 	case 19:
@@ -62,19 +62,16 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		switch (get_xop(inst)) {
 
 		case OP_31_XOP_MFMSR:
-			rt = get_rt(inst);
 			kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->msr);
 			kvmppc_set_exit_type(vcpu, EMULATED_MFMSR_EXITS);
 			break;
 
 		case OP_31_XOP_MTMSR:
-			rs = get_rs(inst);
 			kvmppc_set_exit_type(vcpu, EMULATED_MTMSR_EXITS);
 			kvmppc_set_msr(vcpu, kvmppc_get_gpr(vcpu, rs));
 			break;
 
 		case OP_31_XOP_WRTEE:
-			rs = get_rs(inst);
 			vcpu->arch.shared->msr = (vcpu->arch.shared->msr & ~MSR_EE)
 					| (kvmppc_get_gpr(vcpu, rs) & MSR_EE);
 			kvmppc_set_exit_type(vcpu, EMULATED_WRTEE_EXITS);
diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
index 99155f8..9b2dcda 100644
--- a/arch/powerpc/kvm/e500_emulate.c
+++ b/arch/powerpc/kvm/e500_emulate.c
@@ -86,9 +86,9 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                            unsigned int inst, int *advance)
 {
 	int emulated = EMULATE_DONE;
-	int ra;
-	int rb;
-	int rt;
+	int ra = get_ra(inst);
+	int rb = get_rb(inst);
+	int rt = get_rt(inst);
 
 	switch (get_op(inst)) {
 	case 31:
@@ -96,11 +96,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 
 #ifdef CONFIG_KVM_E500MC
 		case XOP_MSGSND:
-			emulated = kvmppc_e500_emul_msgsnd(vcpu, get_rb(inst));
+			emulated = kvmppc_e500_emul_msgsnd(vcpu, rb);
 			break;
 
 		case XOP_MSGCLR:
-			emulated = kvmppc_e500_emul_msgclr(vcpu, get_rb(inst));
+			emulated = kvmppc_e500_emul_msgclr(vcpu, rb);
 			break;
 #endif
 
@@ -113,20 +113,14 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			break;
 
 		case XOP_TLBSX:
-			rb = get_rb(inst);
 			emulated = kvmppc_e500_emul_tlbsx(vcpu,rb);
 			break;
 
 		case XOP_TLBILX:
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-			rt = get_rt(inst);
 			emulated = kvmppc_e500_emul_tlbilx(vcpu, rt, ra, rb);
 			break;
 
 		case XOP_TLBIVAX:
-			ra = get_ra(inst);
-			rb = get_rb(inst);
 			emulated = kvmppc_e500_emul_tlbivax(vcpu, ra, rb);
 			break;
 
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index a27d4dc..f63b5cb 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -148,11 +148,10 @@ u32 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb)
 int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 {
 	u32 inst = kvmppc_get_last_inst(vcpu);
-	int ra;
-	int rb;
-	int rs;
-	int rt;
-	int sprn;
+	int ra = get_ra(inst);
+	int rs = get_rs(inst);
+	int rt = get_rt(inst);
+	int sprn = get_sprn(inst);
 	enum emulation_result emulated = EMULATE_DONE;
 	int advance = 1;
 
@@ -189,43 +188,31 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			advance = 0;
 			break;
 		case OP_31_XOP_LWZX:
-			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 4, 1);
 			break;
 
 		case OP_31_XOP_LBZX:
-			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
 			break;
 
 		case OP_31_XOP_LBZUX:
-			rt = get_rt(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
 			kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed);
 			break;
 
 		case OP_31_XOP_STWX:
-			rs = get_rs(inst);
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               4, 1);
 			break;
 
 		case OP_31_XOP_STBX:
-			rs = get_rs(inst);
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               1, 1);
 			break;
 
 		case OP_31_XOP_STBUX:
-			rs = get_rs(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               1, 1);
@@ -233,28 +220,19 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			break;
 
 		case OP_31_XOP_LHAX:
-			rt = get_rt(inst);
 			emulated = kvmppc_handle_loads(run, vcpu, rt, 2, 1);
 			break;
 
 		case OP_31_XOP_LHZX:
-			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
 			break;
 
 		case OP_31_XOP_LHZUX:
-			rt = get_rt(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
 			kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed);
 			break;
 
 		case OP_31_XOP_MFSPR:
-			sprn = get_sprn(inst);
-			rt = get_rt(inst);
-
 			switch (sprn) {
 			case SPRN_SRR0:
 				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->srr0);
@@ -310,20 +288,12 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			break;
 
 		case OP_31_XOP_STHX:
-			rs = get_rs(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               2, 1);
 			break;
 
 		case OP_31_XOP_STHUX:
-			rs = get_rs(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               2, 1);
@@ -331,8 +301,6 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			break;
 
 		case OP_31_XOP_MTSPR:
-			sprn = get_sprn(inst);
-			rs = get_rs(inst);
 			switch (sprn) {
 			case SPRN_SRR0:
 				vcpu->arch.shared->srr0 = kvmppc_get_gpr(vcpu, rs);
@@ -384,7 +352,6 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			break;
 
 		case OP_31_XOP_LWBRX:
-			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 4, 0);
 			break;
 
@@ -392,25 +359,16 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			break;
 
 		case OP_31_XOP_STWBRX:
-			rs = get_rs(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               4, 0);
 			break;
 
 		case OP_31_XOP_LHBRX:
-			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 2, 0);
 			break;
 
 		case OP_31_XOP_STHBRX:
-			rs = get_rs(inst);
-			ra = get_ra(inst);
-			rb = get_rb(inst);
-
 			emulated = kvmppc_handle_store(run, vcpu,
 						       kvmppc_get_gpr(vcpu, rs),
 			                               2, 0);
@@ -423,39 +381,30 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		break;
 
 	case OP_LWZ:
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 4, 1);
 		break;
 
 	case OP_LWZU:
-		ra = get_ra(inst);
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 4, 1);
 		kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed);
 		break;
 
 	case OP_LBZ:
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
 		break;
 
 	case OP_LBZU:
-		ra = get_ra(inst);
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
 		kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed);
 		break;
 
 	case OP_STW:
-		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu,
 					       kvmppc_get_gpr(vcpu, rs),
 		                               4, 1);
 		break;
 
 	case OP_STWU:
-		ra = get_ra(inst);
-		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu,
 					       kvmppc_get_gpr(vcpu, rs),
 		                               4, 1);
@@ -463,15 +412,12 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		break;
 
 	case OP_STB:
-		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu,
 					       kvmppc_get_gpr(vcpu, rs),
 		                               1, 1);
 		break;
 
 	case OP_STBU:
-		ra = get_ra(inst);
-		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu,
 					       kvmppc_get_gpr(vcpu, rs),
 		                               1, 1);
@@ -479,39 +425,30 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		break;
 
 	case OP_LHZ:
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
 		break;
 
 	case OP_LHZU:
-		ra = get_ra(inst);
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
 		kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed);
 		break;
 
 	case OP_LHA:
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_loads(run, vcpu, rt, 2, 1);
 		break;
 
 	case OP_LHAU:
-		ra = get_ra(inst);
-		rt = get_rt(inst);
 		emulated = kvmppc_handle_loads(run, vcpu, rt, 2, 1);
 		kvmppc_set_gpr(vcpu, ra, vcpu->arch.vaddr_accessed);
 		break;
 
 	case OP_STH:
-		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu,
 					       kvmppc_get_gpr(vcpu, rs),
 		                               2, 1);
 		break;
 
 	case OP_STHU:
-		ra = get_ra(inst);
-		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu,
 					       kvmppc_get_gpr(vcpu, rs),
 		                               2, 1);
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH 2/2] KVM: PPC: Emulator: clean up SPR reads and writes
  2012-05-04 13:11 [PATCH 1/2] KVM: PPC: Emulator: clean up instruction parsing Alexander Graf
@ 2012-05-04 13:11 ` Alexander Graf
  0 siblings, 0 replies; 2+ messages in thread
From: Alexander Graf @ 2012-05-04 13:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: kvm list

When reading and writing SPRs, every SPR emulation piece had to read
or write the respective GPR the value was read from or stored in itself.

This approach is pretty prone to failure. What if we accidentally
implement mfspr emulation where we just do "break" and nothing else?
Suddenly we would get a random value in the return register - which is
always a bad idea.

So let's consolidate the generic code paths and only give the core
specific SPR handling code readily made variables to read/write from/to.

Functionally, this patch doesn't change anything, but it increases the
readability of the code and makes is less prone to bugs.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/include/asm/kvm_ppc.h |    6 +-
 arch/powerpc/kvm/44x_emulate.c     |   24 ++++----
 arch/powerpc/kvm/book3s_emulate.c  |   34 +++++------
 arch/powerpc/kvm/book3s_hv.c       |    4 +-
 arch/powerpc/kvm/booke.h           |    4 +-
 arch/powerpc/kvm/booke_emulate.c   |   88 +++++++++++++++++------------
 arch/powerpc/kvm/e500_emulate.c    |  108 ++++++++++++++++++++++--------------
 arch/powerpc/kvm/emulate.c         |   64 ++++++++++++---------
 8 files changed, 190 insertions(+), 142 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index c87e3b5..f68c22f 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -107,8 +107,10 @@ extern void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
 
 extern int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                                   unsigned int op, int *advance);
-extern int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs);
-extern int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt);
+extern int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn,
+				     ulong val);
+extern int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn,
+				     ulong *val);
 
 extern int kvmppc_booke_init(void);
 extern void kvmppc_booke_exit(void);
diff --git a/arch/powerpc/kvm/44x_emulate.c b/arch/powerpc/kvm/44x_emulate.c
index da81a2d..c8c6157 100644
--- a/arch/powerpc/kvm/44x_emulate.c
+++ b/arch/powerpc/kvm/44x_emulate.c
@@ -128,41 +128,41 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	return emulated;
 }
 
-int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
+int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 {
 	int emulated = EMULATE_DONE;
 
 	switch (sprn) {
 	case SPRN_PID:
-		kvmppc_set_pid(vcpu, kvmppc_get_gpr(vcpu, rs)); break;
+		kvmppc_set_pid(vcpu, spr_val); break;
 	case SPRN_MMUCR:
-		vcpu->arch.mmucr = kvmppc_get_gpr(vcpu, rs); break;
+		vcpu->arch.mmucr = spr_val; break;
 	case SPRN_CCR0:
-		vcpu->arch.ccr0 = kvmppc_get_gpr(vcpu, rs); break;
+		vcpu->arch.ccr0 = spr_val; break;
 	case SPRN_CCR1:
-		vcpu->arch.ccr1 = kvmppc_get_gpr(vcpu, rs); break;
+		vcpu->arch.ccr1 = spr_val; break;
 	default:
-		emulated = kvmppc_booke_emulate_mtspr(vcpu, sprn, rs);
+		emulated = kvmppc_booke_emulate_mtspr(vcpu, sprn, spr_val);
 	}
 
 	return emulated;
 }
 
-int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
+int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 {
 	int emulated = EMULATE_DONE;
 
 	switch (sprn) {
 	case SPRN_PID:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.pid); break;
+		*spr_val = vcpu->arch.pid; break;
 	case SPRN_MMUCR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.mmucr); break;
+		*spr_val = vcpu->arch.mmucr; break;
 	case SPRN_CCR0:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ccr0); break;
+		*spr_val = vcpu->arch.ccr0; break;
 	case SPRN_CCR1:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ccr1); break;
+		*spr_val = vcpu->arch.ccr1; break;
 	default:
-		emulated = kvmppc_booke_emulate_mfspr(vcpu, sprn, rt);
+		emulated = kvmppc_booke_emulate_mfspr(vcpu, sprn, spr_val);
 	}
 
 	return emulated;
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index c023bcd..b9a989d 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -318,10 +318,9 @@ static struct kvmppc_bat *kvmppc_find_bat(struct kvm_vcpu *vcpu, int sprn)
 	return bat;
 }
 
-int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
+int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 {
 	int emulated = EMULATE_DONE;
-	ulong spr_val = kvmppc_get_gpr(vcpu, rs);
 
 	switch (sprn) {
 	case SPRN_SDR1:
@@ -433,7 +432,7 @@ unprivileged:
 	return emulated;
 }
 
-int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
+int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 {
 	int emulated = EMULATE_DONE;
 
@@ -446,46 +445,46 @@ int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
 		struct kvmppc_bat *bat = kvmppc_find_bat(vcpu, sprn);
 
 		if (sprn % 2)
-			kvmppc_set_gpr(vcpu, rt, bat->raw >> 32);
+			*spr_val = bat->raw >> 32;
 		else
-			kvmppc_set_gpr(vcpu, rt, bat->raw);
+			*spr_val = bat->raw;
 
 		break;
 	}
 	case SPRN_SDR1:
 		if (!spr_allowed(vcpu, PRIV_HYPER))
 			goto unprivileged;
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->sdr1);
+		*spr_val = to_book3s(vcpu)->sdr1;
 		break;
 	case SPRN_DSISR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->dsisr);
+		*spr_val = vcpu->arch.shared->dsisr;
 		break;
 	case SPRN_DAR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->dar);
+		*spr_val = vcpu->arch.shared->dar;
 		break;
 	case SPRN_HIOR:
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->hior);
+		*spr_val = to_book3s(vcpu)->hior;
 		break;
 	case SPRN_HID0:
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->hid[0]);
+		*spr_val = to_book3s(vcpu)->hid[0];
 		break;
 	case SPRN_HID1:
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->hid[1]);
+		*spr_val = to_book3s(vcpu)->hid[1];
 		break;
 	case SPRN_HID2:
 	case SPRN_HID2_GEKKO:
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->hid[2]);
+		*spr_val = to_book3s(vcpu)->hid[2];
 		break;
 	case SPRN_HID4:
 	case SPRN_HID4_GEKKO:
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->hid[4]);
+		*spr_val = to_book3s(vcpu)->hid[4];
 		break;
 	case SPRN_HID5:
-		kvmppc_set_gpr(vcpu, rt, to_book3s(vcpu)->hid[5]);
+		*spr_val = to_book3s(vcpu)->hid[5];
 		break;
 	case SPRN_CFAR:
 	case SPRN_PURR:
-		kvmppc_set_gpr(vcpu, rt, 0);
+		*spr_val = 0;
 		break;
 	case SPRN_GQR0:
 	case SPRN_GQR1:
@@ -495,8 +494,7 @@ int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
 	case SPRN_GQR5:
 	case SPRN_GQR6:
 	case SPRN_GQR7:
-		kvmppc_set_gpr(vcpu, rt,
-			       to_book3s(vcpu)->gqr[sprn - SPRN_GQR0]);
+		*spr_val = to_book3s(vcpu)->gqr[sprn - SPRN_GQR0];
 		break;
 	case SPRN_THRM1:
 	case SPRN_THRM2:
@@ -511,7 +509,7 @@ int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
 	case SPRN_PMC3_GEKKO:
 	case SPRN_PMC4_GEKKO:
 	case SPRN_WPAR_GEKKO:
-		kvmppc_set_gpr(vcpu, rt, 0);
+		*spr_val = 0;
 		break;
 	default:
 unprivileged:
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index bb5a0f4..db36598 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1505,12 +1505,12 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	return EMULATE_FAIL;
 }
 
-int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
+int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 {
 	return EMULATE_FAIL;
 }
 
-int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
+int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 {
 	return EMULATE_FAIL;
 }
diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
index 62c4fe5..ba61974 100644
--- a/arch/powerpc/kvm/booke.h
+++ b/arch/powerpc/kvm/booke.h
@@ -75,8 +75,8 @@ void kvmppc_clr_tsr_bits(struct kvm_vcpu *vcpu, u32 tsr_bits);
 
 int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                             unsigned int inst, int *advance);
-int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt);
-int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs);
+int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val);
+int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val);
 
 /* low-level asm code to transfer guest state */
 void kvmppc_load_guest_spe(struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index e14f7b2..6c76397 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -102,22 +102,26 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
  * will return the wrong result if called for them in another context
  * (such as debugging).
  */
-int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
+int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 {
 	int emulated = EMULATE_DONE;
-	ulong spr_val = kvmppc_get_gpr(vcpu, rs);
 
 	switch (sprn) {
 	case SPRN_DEAR:
-		vcpu->arch.shared->dar = spr_val; break;
+		vcpu->arch.shared->dar = spr_val;
+		break;
 	case SPRN_ESR:
-		vcpu->arch.shared->esr = spr_val; break;
+		vcpu->arch.shared->esr = spr_val;
+		break;
 	case SPRN_DBCR0:
-		vcpu->arch.dbcr0 = spr_val; break;
+		vcpu->arch.dbcr0 = spr_val;
+		break;
 	case SPRN_DBCR1:
-		vcpu->arch.dbcr1 = spr_val; break;
+		vcpu->arch.dbcr1 = spr_val;
+		break;
 	case SPRN_DBSR:
-		vcpu->arch.dbsr &= ~spr_val; break;
+		vcpu->arch.dbsr &= ~spr_val;
+		break;
 	case SPRN_TSR:
 		kvmppc_clr_tsr_bits(vcpu, spr_val);
 		break;
@@ -131,13 +135,17 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
 	 * guest (PR-mode only).
 	 */
 	case SPRN_SPRG4:
-		vcpu->arch.shared->sprg4 = spr_val; break;
+		vcpu->arch.shared->sprg4 = spr_val;
+		break;
 	case SPRN_SPRG5:
-		vcpu->arch.shared->sprg5 = spr_val; break;
+		vcpu->arch.shared->sprg5 = spr_val;
+		break;
 	case SPRN_SPRG6:
-		vcpu->arch.shared->sprg6 = spr_val; break;
+		vcpu->arch.shared->sprg6 = spr_val;
+		break;
 	case SPRN_SPRG7:
-		vcpu->arch.shared->sprg7 = spr_val; break;
+		vcpu->arch.shared->sprg7 = spr_val;
+		break;
 
 	case SPRN_IVPR:
 		vcpu->arch.ivpr = spr_val;
@@ -207,75 +215,83 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
 	return emulated;
 }
 
-int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
+int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 {
 	int emulated = EMULATE_DONE;
 
 	switch (sprn) {
 	case SPRN_IVPR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivpr); break;
+		*spr_val = vcpu->arch.ivpr;
+		break;
 	case SPRN_DEAR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->dar); break;
+		*spr_val = vcpu->arch.shared->dar;
+		break;
 	case SPRN_ESR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->esr); break;
+		*spr_val = vcpu->arch.shared->esr;
+		break;
 	case SPRN_DBCR0:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.dbcr0); break;
+		*spr_val = vcpu->arch.dbcr0;
+		break;
 	case SPRN_DBCR1:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.dbcr1); break;
+		*spr_val = vcpu->arch.dbcr1;
+		break;
 	case SPRN_DBSR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.dbsr); break;
+		*spr_val = vcpu->arch.dbsr;
+		break;
 	case SPRN_TSR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.tsr); break;
+		*spr_val = vcpu->arch.tsr;
+		break;
 	case SPRN_TCR:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.tcr); break;
+		*spr_val = vcpu->arch.tcr;
+		break;
 
 	case SPRN_IVOR0:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_CRITICAL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_CRITICAL];
 		break;
 	case SPRN_IVOR1:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_MACHINE_CHECK]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_MACHINE_CHECK];
 		break;
 	case SPRN_IVOR2:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_DATA_STORAGE]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_DATA_STORAGE];
 		break;
 	case SPRN_IVOR3:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_INST_STORAGE]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_INST_STORAGE];
 		break;
 	case SPRN_IVOR4:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_EXTERNAL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_EXTERNAL];
 		break;
 	case SPRN_IVOR5:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_ALIGNMENT]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_ALIGNMENT];
 		break;
 	case SPRN_IVOR6:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_PROGRAM]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_PROGRAM];
 		break;
 	case SPRN_IVOR7:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_FP_UNAVAIL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_FP_UNAVAIL];
 		break;
 	case SPRN_IVOR8:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_SYSCALL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SYSCALL];
 		break;
 	case SPRN_IVOR9:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_AP_UNAVAIL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_AP_UNAVAIL];
 		break;
 	case SPRN_IVOR10:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_DECREMENTER]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_DECREMENTER];
 		break;
 	case SPRN_IVOR11:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_FIT]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_FIT];
 		break;
 	case SPRN_IVOR12:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_WATCHDOG]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_WATCHDOG];
 		break;
 	case SPRN_IVOR13:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_DTLB_MISS]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_DTLB_MISS];
 		break;
 	case SPRN_IVOR14:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_ITLB_MISS]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_ITLB_MISS];
 		break;
 	case SPRN_IVOR15:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_DEBUG]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_DEBUG];
 		break;
 
 	default:
diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
index 9b2dcda..9d4663d 100644
--- a/arch/powerpc/kvm/e500_emulate.c
+++ b/arch/powerpc/kvm/e500_emulate.c
@@ -140,11 +140,10 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	return emulated;
 }
 
-int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
+int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 {
 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
 	int emulated = EMULATE_DONE;
-	ulong spr_val = kvmppc_get_gpr(vcpu, rs);
 
 	switch (sprn) {
 #ifndef CONFIG_KVM_BOOKE_HV
@@ -154,25 +153,32 @@ int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
 	case SPRN_PID1:
 		if (spr_val != 0)
 			return EMULATE_FAIL;
-		vcpu_e500->pid[1] = spr_val; break;
+		vcpu_e500->pid[1] = spr_val;
+		break;
 	case SPRN_PID2:
 		if (spr_val != 0)
 			return EMULATE_FAIL;
-		vcpu_e500->pid[2] = spr_val; break;
+		vcpu_e500->pid[2] = spr_val;
+		break;
 	case SPRN_MAS0:
-		vcpu->arch.shared->mas0 = spr_val; break;
+		vcpu->arch.shared->mas0 = spr_val;
+		break;
 	case SPRN_MAS1:
-		vcpu->arch.shared->mas1 = spr_val; break;
+		vcpu->arch.shared->mas1 = spr_val;
+		break;
 	case SPRN_MAS2:
-		vcpu->arch.shared->mas2 = spr_val; break;
+		vcpu->arch.shared->mas2 = spr_val;
+		break;
 	case SPRN_MAS3:
 		vcpu->arch.shared->mas7_3 &= ~(u64)0xffffffff;
 		vcpu->arch.shared->mas7_3 |= spr_val;
 		break;
 	case SPRN_MAS4:
-		vcpu->arch.shared->mas4 = spr_val; break;
+		vcpu->arch.shared->mas4 = spr_val;
+		break;
 	case SPRN_MAS6:
-		vcpu->arch.shared->mas6 = spr_val; break;
+		vcpu->arch.shared->mas6 = spr_val;
+		break;
 	case SPRN_MAS7:
 		vcpu->arch.shared->mas7_3 &= (u64)0xffffffff;
 		vcpu->arch.shared->mas7_3 |= (u64)spr_val << 32;
@@ -183,11 +189,14 @@ int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
 		vcpu_e500->l1csr0 &= ~(L1CSR0_DCFI | L1CSR0_CLFC);
 		break;
 	case SPRN_L1CSR1:
-		vcpu_e500->l1csr1 = spr_val; break;
+		vcpu_e500->l1csr1 = spr_val;
+		break;
 	case SPRN_HID0:
-		vcpu_e500->hid0 = spr_val; break;
+		vcpu_e500->hid0 = spr_val;
+		break;
 	case SPRN_HID1:
-		vcpu_e500->hid1 = spr_val; break;
+		vcpu_e500->hid1 = spr_val;
+		break;
 
 	case SPRN_MMUCSR0:
 		emulated = kvmppc_e500_emul_mt_mmucsr0(vcpu_e500,
@@ -216,13 +225,13 @@ int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
 		break;
 #endif
 	default:
-		emulated = kvmppc_booke_emulate_mtspr(vcpu, sprn, rs);
+		emulated = kvmppc_booke_emulate_mtspr(vcpu, sprn, spr_val);
 	}
 
 	return emulated;
 }
 
-int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
+int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 {
 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
 	int emulated = EMULATE_DONE;
@@ -232,74 +241,89 @@ int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
 		unsigned long val;
 
 	case SPRN_PID:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->pid[0]); break;
+		*spr_val = vcpu_e500->pid[0];
+		break;
 	case SPRN_PID1:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->pid[1]); break;
+		*spr_val = vcpu_e500->pid[1];
+		break;
 	case SPRN_PID2:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->pid[2]); break;
+		*spr_val = vcpu_e500->pid[2];
+		break;
 	case SPRN_MAS0:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->mas0); break;
+		*spr_val = vcpu->arch.shared->mas0;
+		break;
 	case SPRN_MAS1:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->mas1); break;
+		*spr_val = vcpu->arch.shared->mas1;
+		break;
 	case SPRN_MAS2:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->mas2); break;
+		*spr_val = vcpu->arch.shared->mas2;
+		break;
 	case SPRN_MAS3:
-		val = (u32)vcpu->arch.shared->mas7_3;
-		kvmppc_set_gpr(vcpu, rt, val);
+		*spr_val = (u32)vcpu->arch.shared->mas7_3;
 		break;
 	case SPRN_MAS4:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->mas4); break;
+		*spr_val = vcpu->arch.shared->mas4;
+		break;
 	case SPRN_MAS6:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->mas6); break;
+		*spr_val = vcpu->arch.shared->mas6;
+		break;
 	case SPRN_MAS7:
-		val = vcpu->arch.shared->mas7_3 >> 32;
-		kvmppc_set_gpr(vcpu, rt, val);
+		*spr_val = vcpu->arch.shared->mas7_3 >> 32;
 		break;
 #endif
 	case SPRN_TLB0CFG:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.tlbcfg[0]); break;
+		*spr_val = vcpu->arch.tlbcfg[0];
+		break;
 	case SPRN_TLB1CFG:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.tlbcfg[1]); break;
+		*spr_val = vcpu->arch.tlbcfg[1];
+		break;
 	case SPRN_L1CSR0:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->l1csr0); break;
+		*spr_val = vcpu_e500->l1csr0;
+		break;
 	case SPRN_L1CSR1:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->l1csr1); break;
+		*spr_val = vcpu_e500->l1csr1;
+		break;
 	case SPRN_HID0:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->hid0); break;
+		*spr_val = vcpu_e500->hid0;
+		break;
 	case SPRN_HID1:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->hid1); break;
+		*spr_val = vcpu_e500->hid1;
+		break;
 	case SPRN_SVR:
-		kvmppc_set_gpr(vcpu, rt, vcpu_e500->svr); break;
+		*spr_val = vcpu_e500->svr;
+		break;
 
 	case SPRN_MMUCSR0:
-		kvmppc_set_gpr(vcpu, rt, 0); break;
+		*spr_val = 0;
+		break;
 
 	case SPRN_MMUCFG:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.mmucfg); break;
+		*spr_val = vcpu->arch.mmucfg;
+		break;
 
 	/* extra exceptions */
 	case SPRN_IVOR32:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL];
 		break;
 	case SPRN_IVOR33:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA];
 		break;
 	case SPRN_IVOR34:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND];
 		break;
 	case SPRN_IVOR35:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_PERFORMANCE_MONITOR]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_PERFORMANCE_MONITOR];
 		break;
 #ifdef CONFIG_KVM_BOOKE_HV
 	case SPRN_IVOR36:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_DBELL]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_DBELL];
 		break;
 	case SPRN_IVOR37:
-		kvmppc_set_gpr(vcpu, rt, vcpu->arch.ivor[BOOKE_IRQPRIO_DBELL_CRIT]);
+		*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_DBELL_CRIT];
 		break;
 #endif
 	default:
-		emulated = kvmppc_booke_emulate_mfspr(vcpu, sprn, rt);
+		emulated = kvmppc_booke_emulate_mfspr(vcpu, sprn, spr_val);
 	}
 
 	return emulated;
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index f63b5cb..f90e86d 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -154,6 +154,7 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 	int sprn = get_sprn(inst);
 	enum emulation_result emulated = EMULATE_DONE;
 	int advance = 1;
+	ulong spr_val = 0;
 
 	/* this default type might be overwritten by subcategories */
 	kvmppc_set_exit_type(vcpu, EMULATED_INST_EXITS);
@@ -235,55 +236,59 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		case OP_31_XOP_MFSPR:
 			switch (sprn) {
 			case SPRN_SRR0:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->srr0);
+				spr_val = vcpu->arch.shared->srr0;
 				break;
 			case SPRN_SRR1:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->srr1);
+				spr_val = vcpu->arch.shared->srr1;
 				break;
 			case SPRN_PVR:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.pvr); break;
+				spr_val = vcpu->arch.pvr;
+				break;
 			case SPRN_PIR:
-				kvmppc_set_gpr(vcpu, rt, vcpu->vcpu_id); break;
+				spr_val = vcpu->vcpu_id;
+				break;
 			case SPRN_MSSSR0:
-				kvmppc_set_gpr(vcpu, rt, 0); break;
+				spr_val = 0;
+				break;
 
 			/* Note: mftb and TBRL/TBWL are user-accessible, so
 			 * the guest can always access the real TB anyways.
 			 * In fact, we probably will never see these traps. */
 			case SPRN_TBWL:
-				kvmppc_set_gpr(vcpu, rt, get_tb() >> 32); break;
+				spr_val = get_tb() >> 32;
+				break;
 			case SPRN_TBWU:
-				kvmppc_set_gpr(vcpu, rt, get_tb()); break;
+				spr_val = get_tb();
+				break;
 
 			case SPRN_SPRG0:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->sprg0);
+				spr_val = vcpu->arch.shared->sprg0;
 				break;
 			case SPRN_SPRG1:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->sprg1);
+				spr_val = vcpu->arch.shared->sprg1;
 				break;
 			case SPRN_SPRG2:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->sprg2);
+				spr_val = vcpu->arch.shared->sprg2;
 				break;
 			case SPRN_SPRG3:
-				kvmppc_set_gpr(vcpu, rt, vcpu->arch.shared->sprg3);
+				spr_val = vcpu->arch.shared->sprg3;
 				break;
 			/* Note: SPRG4-7 are user-readable, so we don't get
 			 * a trap. */
 
 			case SPRN_DEC:
-			{
-				kvmppc_set_gpr(vcpu, rt,
-					       kvmppc_get_dec(vcpu, get_tb()));
+				spr_val = kvmppc_get_dec(vcpu, get_tb());
 				break;
-			}
 			default:
-				emulated = kvmppc_core_emulate_mfspr(vcpu, sprn, rt);
-				if (emulated = EMULATE_FAIL) {
-					printk("mfspr: unknown spr %x\n", sprn);
-					kvmppc_set_gpr(vcpu, rt, 0);
+				emulated = kvmppc_core_emulate_mfspr(vcpu, sprn,
+								     &spr_val);
+				if (unlikely(emulated = EMULATE_FAIL)) {
+					printk(KERN_INFO "mfspr: unknown spr "
+						"0x%x\n", sprn);
 				}
 				break;
 			}
+			kvmppc_set_gpr(vcpu, rt, spr_val);
 			kvmppc_set_exit_type(vcpu, EMULATED_MFSPR_EXITS);
 			break;
 
@@ -301,12 +306,13 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			break;
 
 		case OP_31_XOP_MTSPR:
+			spr_val = kvmppc_get_gpr(vcpu, rs);
 			switch (sprn) {
 			case SPRN_SRR0:
-				vcpu->arch.shared->srr0 = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.shared->srr0 = spr_val;
 				break;
 			case SPRN_SRR1:
-				vcpu->arch.shared->srr1 = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.shared->srr1 = spr_val;
 				break;
 
 			/* XXX We need to context-switch the timebase for
@@ -317,27 +323,29 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			case SPRN_MSSSR0: break;
 
 			case SPRN_DEC:
-				vcpu->arch.dec = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.dec = spr_val;
 				kvmppc_emulate_dec(vcpu);
 				break;
 
 			case SPRN_SPRG0:
-				vcpu->arch.shared->sprg0 = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.shared->sprg0 = spr_val;
 				break;
 			case SPRN_SPRG1:
-				vcpu->arch.shared->sprg1 = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.shared->sprg1 = spr_val;
 				break;
 			case SPRN_SPRG2:
-				vcpu->arch.shared->sprg2 = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.shared->sprg2 = spr_val;
 				break;
 			case SPRN_SPRG3:
-				vcpu->arch.shared->sprg3 = kvmppc_get_gpr(vcpu, rs);
+				vcpu->arch.shared->sprg3 = spr_val;
 				break;
 
 			default:
-				emulated = kvmppc_core_emulate_mtspr(vcpu, sprn, rs);
+				emulated = kvmppc_core_emulate_mtspr(vcpu, sprn,
+								     spr_val);
 				if (emulated = EMULATE_FAIL)
-					printk("mtspr: unknown spr %x\n", sprn);
+					printk(KERN_INFO "mtspr: unknown spr "
+						"0x%x\n", sprn);
 				break;
 			}
 			kvmppc_set_exit_type(vcpu, EMULATED_MTSPR_EXITS);
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2012-05-04 13:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-04 13:11 [PATCH 1/2] KVM: PPC: Emulator: clean up instruction parsing Alexander Graf
2012-05-04 13:11 ` [PATCH 2/2] KVM: PPC: Emulator: clean up SPR reads and writes Alexander Graf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox