diff mbox

[2/2] KVM: PPC: Emulator: clean up SPR reads and writes

Message ID 1336137115-29390-2-git-send-email-agraf@suse.de
State New, archived
Headers show

Commit Message

Alexander Graf May 4, 2012, 1:11 p.m. UTC 
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 mbox

Patch

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