[PATCH] KVM: PPC: Book3S PR: Do not always inject facility unavailable exceptions

[PATCH] KVM: PPC: Book3S PR: Do not always inject facility unavailable exceptions

[Thomas Huth]

KVM should not inject a facility unavailable exception into the guest
when it tries to execute a mtspr/mfspr instruction for an SPR that
is unavailable, and the vCPU is *not* running in PRoblem state.

It's right that we inject an exception when the vCPU is in PR mode, since
chapter "6.2.10 Facility Status and Control Register" of the PowerISA
v2.07 says that "When the FSCR makes a facility unavailable, attempted
usage of the facility in *problem state* is treated as follows: [...]
Access of an SPR using mfspr/mtspr causes a Facility Unavailable
interrupt". But if the guest vCPU is not in PR mode, we should follow
the behavior that is described in chapter "4.4.4 Move To/From System
Register Instructions" instead and treat the instruction as a NOP.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 arch/powerpc/kvm/book3s_pr.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index d4dfc0c..3e6c0b3 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -27,6 +27,7 @@
 #include <asm/reg.h>
 #include <asm/cputable.h>
 #include <asm/cacheflush.h>
+#include <asm/disassemble.h>
 #include <asm/tlbflush.h>
 #include <linux/uaccess.h>
 #include <asm/io.h>
@@ -830,6 +831,31 @@ static void kvmppc_emulate_fac(struct kvm_vcpu *vcpu, ulong fac)
 	}
 }
 
+static void kvmppc_handle_fac_not_enabled(struct kvm_vcpu *vcpu, ulong fac)
+{
+	enum emulation_result er;
+	u32 inst;
+
+	if (kvmppc_get_msr(vcpu) & MSR_PR) {
+		kvmppc_trigger_fac_interrupt(vcpu, fac);
+		return;
+	}
+
+	er = kvmppc_get_last_inst(vcpu, INST_GENERIC, &inst);
+	if (er != EMULATE_DONE)
+		return;
+
+	if (get_op(inst) == 31 && (get_xop(inst) == OP_31_XOP_MTSPR ||
+				   get_xop(inst) == OP_31_XOP_MFSPR)) {
+		/* mtspr and mfspr are treated as NOP for unsupported SPRs */
+		kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
+		pr_debug_ratelimited("%s: write/read of disabled SPR: %d\n",
+				     __func__, get_sprn(inst));
+	} else {
+		kvmppc_trigger_fac_interrupt(vcpu, fac);
+	}
+}
+
 /* Enable facilities (TAR, EBB, DSCR) for the guest */
 static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
 {
@@ -855,7 +881,7 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
 
 	if (!guest_fac_enabled) {
 		/* Facility not enabled by the guest */
-		kvmppc_trigger_fac_interrupt(vcpu, fac);
+		kvmppc_handle_fac_not_enabled(vcpu, fac);
 		return RESUME_GUEST;
 	}
 
-- 
1.8.3.1

[PATCH] KVM: PPC: Book3S PR: Do not always inject facility unavailable exceptions

[Thomas Huth]

KVM should not inject a facility unavailable exception into the guest
when it tries to execute a mtspr/mfspr instruction for an SPR that
is unavailable, and the vCPU is *not* running in PRoblem state.

It's right that we inject an exception when the vCPU is in PR mode, since
chapter "6.2.10 Facility Status and Control Register" of the PowerISA
v2.07 says that "When the FSCR makes a facility unavailable, attempted
usage of the facility in *problem state* is treated as follows: [...]
Access of an SPR using mfspr/mtspr causes a Facility Unavailable
interrupt". But if the guest vCPU is not in PR mode, we should follow
the behavior that is described in chapter "4.4.4 Move To/From System
Register Instructions" instead and treat the instruction as a NOP.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 arch/powerpc/kvm/book3s_pr.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index d4dfc0c..3e6c0b3 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -27,6 +27,7 @@
 #include <asm/reg.h>
 #include <asm/cputable.h>
 #include <asm/cacheflush.h>
+#include <asm/disassemble.h>
 #include <asm/tlbflush.h>
 #include <linux/uaccess.h>
 #include <asm/io.h>
@@ -830,6 +831,31 @@ static void kvmppc_emulate_fac(struct kvm_vcpu *vcpu, ulong fac)
 	}
 }
 
+static void kvmppc_handle_fac_not_enabled(struct kvm_vcpu *vcpu, ulong fac)
+{
+	enum emulation_result er;
+	u32 inst;
+
+	if (kvmppc_get_msr(vcpu) & MSR_PR) {
+		kvmppc_trigger_fac_interrupt(vcpu, fac);
+		return;
+	}
+
+	er = kvmppc_get_last_inst(vcpu, INST_GENERIC, &inst);
+	if (er != EMULATE_DONE)
+		return;
+
+	if (get_op(inst) = 31 && (get_xop(inst) = OP_31_XOP_MTSPR ||
+				   get_xop(inst) = OP_31_XOP_MFSPR)) {
+		/* mtspr and mfspr are treated as NOP for unsupported SPRs */
+		kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
+		pr_debug_ratelimited("%s: write/read of disabled SPR: %d\n",
+				     __func__, get_sprn(inst));
+	} else {
+		kvmppc_trigger_fac_interrupt(vcpu, fac);
+	}
+}
+
 /* Enable facilities (TAR, EBB, DSCR) for the guest */
 static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
 {
@@ -855,7 +881,7 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
 
 	if (!guest_fac_enabled) {
 		/* Facility not enabled by the guest */
-		kvmppc_trigger_fac_interrupt(vcpu, fac);
+		kvmppc_handle_fac_not_enabled(vcpu, fac);
 		return RESUME_GUEST;
 	}
 
-- 
1.8.3.1

Re: [PATCH] KVM: PPC: Book3S PR: Do not always inject facility unavailable exceptions

[Paul Mackerras]

On Mon, Apr 03, 2017 at 01:28:34PM +0200, Thomas Huth wrote:
> KVM should not inject a facility unavailable exception into the guest
> when it tries to execute a mtspr/mfspr instruction for an SPR that
> is unavailable, and the vCPU is *not* running in PRoblem state.
> 
> It's right that we inject an exception when the vCPU is in PR mode, since
> chapter "6.2.10 Facility Status and Control Register" of the PowerISA
> v2.07 says that "When the FSCR makes a facility unavailable, attempted
> usage of the facility in *problem state* is treated as follows: [...]
> Access of an SPR using mfspr/mtspr causes a Facility Unavailable
> interrupt". But if the guest vCPU is not in PR mode, we should follow
> the behavior that is described in chapter "4.4.4 Move To/From System
> Register Instructions" instead and treat the instruction as a NOP.

This doesn't seem quite right.  My reading of the ISA is that the FSCR
bit for a facility being 0 doesn't prevent privileged code from
accessing the facility, so we shouldn't be treating mfspr/mtspr as
NOP.  Instead we should be set the facility's bit in the shadow
FSCR and re-execute the instruction (remembering of course to clear
the FSCR bit when we go back to emulated problem state).

For TM it's a bit different as the MSR[TM] bit does prevent privileged
code from accessing TM registers and instructions, so for TM we should
be delivering a facility unavailable interrupt even when the guest is
in emulated privileged state.

So I don't see any case where mfspr/mtspr should be treated as a NOP
in response to a facility unavailable interrupt.

Paul.

Re: [PATCH] KVM: PPC: Book3S PR: Do not always inject facility unavailable exceptions

[Paul Mackerras]

On Mon, Apr 03, 2017 at 01:28:34PM +0200, Thomas Huth wrote:
> KVM should not inject a facility unavailable exception into the guest
> when it tries to execute a mtspr/mfspr instruction for an SPR that
> is unavailable, and the vCPU is *not* running in PRoblem state.
> 
> It's right that we inject an exception when the vCPU is in PR mode, since
> chapter "6.2.10 Facility Status and Control Register" of the PowerISA
> v2.07 says that "When the FSCR makes a facility unavailable, attempted
> usage of the facility in *problem state* is treated as follows: [...]
> Access of an SPR using mfspr/mtspr causes a Facility Unavailable
> interrupt". But if the guest vCPU is not in PR mode, we should follow
> the behavior that is described in chapter "4.4.4 Move To/From System
> Register Instructions" instead and treat the instruction as a NOP.

This doesn't seem quite right.  My reading of the ISA is that the FSCR
bit for a facility being 0 doesn't prevent privileged code from
accessing the facility, so we shouldn't be treating mfspr/mtspr as
NOP.  Instead we should be set the facility's bit in the shadow
FSCR and re-execute the instruction (remembering of course to clear
the FSCR bit when we go back to emulated problem state).

For TM it's a bit different as the MSR[TM] bit does prevent privileged
code from accessing TM registers and instructions, so for TM we should
be delivering a facility unavailable interrupt even when the guest is
in emulated privileged state.

So I don't see any case where mfspr/mtspr should be treated as a NOP
in response to a facility unavailable interrupt.

Paul.