| Message ID | 1491218914-10992-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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.
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; }
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(-)