From 122e51d47418f74a69a93bf02f5535d11ff75bf5 Mon Sep 17 00:00:00 2001 From: James Hogan Date: Mon, 28 Nov 2016 17:23:14 +0000 Subject: [PATCH] KVM: MIPS: Improve kvm_get_inst() error return MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently kvm_get_inst() returns KVM_INVALID_INST in the event of a fault reading the guest instruction. This has the rather arbitrary magic value 0xdeadbeef. This API isn't very robust, and in fact 0xdeadbeef is a valid MIPS64 instruction encoding, namely "ld t1,-16657(s5)". Therefore change the kvm_get_inst() API to return 0 or -EFAULT, and to return the instruction via a u32 *out argument. We can then drop the KVM_INVALID_INST definition entirely. Signed-off-by: James Hogan Cc: Paolo Bonzini Cc: "Radim Krčmář" Cc: Ralf Baechle Cc: linux-mips@linux-mips.org Cc: kvm@vger.kernel.org --- arch/mips/include/asm/kvm_host.h | 3 +- arch/mips/kvm/emulate.c | 90 ++++++++++++++++---------------- arch/mips/kvm/mips.c | 7 ++- arch/mips/kvm/mmu.c | 9 ++-- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h index 6f68f7545b66..f296ebeda9e3 100644 --- a/arch/mips/include/asm/kvm_host.h +++ b/arch/mips/include/asm/kvm_host.h @@ -104,7 +104,6 @@ #define KVM_GUEST_KSEG23ADDR(a) (KVM_GUEST_CPHYSADDR(a) | KVM_GUEST_KSEG23) #define KVM_INVALID_PAGE 0xdeadbeef -#define KVM_INVALID_INST 0xdeadbeef #define KVM_INVALID_ADDR 0xdeadbeef /* @@ -640,7 +639,7 @@ void kvm_trap_emul_invalidate_gva(struct kvm_vcpu *vcpu, unsigned long addr, bool user); /* Emulation */ -u32 kvm_get_inst(u32 *opc, struct kvm_vcpu *vcpu); +int kvm_get_inst(u32 *opc, struct kvm_vcpu *vcpu, u32 *out); enum emulation_result update_pc(struct kvm_vcpu *vcpu, u32 cause); /** diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c index 67ea39973b96..b906fc0589f3 100644 --- a/arch/mips/kvm/emulate.c +++ b/arch/mips/kvm/emulate.c @@ -38,23 +38,25 @@ * Compute the return address and do emulate branch simulation, if required. * This function should be called only in branch delay slot active. */ -unsigned long kvm_compute_return_epc(struct kvm_vcpu *vcpu, - unsigned long instpc) +static int kvm_compute_return_epc(struct kvm_vcpu *vcpu, unsigned long instpc, + unsigned long *out) { unsigned int dspcontrol; union mips_instruction insn; struct kvm_vcpu_arch *arch = &vcpu->arch; long epc = instpc; - long nextpc = KVM_INVALID_INST; + long nextpc; + int err; - if (epc & 3) - goto unaligned; + if (epc & 3) { + kvm_err("%s: unaligned epc\n", __func__); + return -EINVAL; + } /* Read the instruction */ - insn.word = kvm_get_inst((u32 *) epc, vcpu); - - if (insn.word == KVM_INVALID_INST) - return KVM_INVALID_INST; + err = kvm_get_inst((u32 *)epc, vcpu, &insn.word); + if (err) + return err; switch (insn.i_format.opcode) { /* jr and jalr are in r_format format. */ @@ -66,6 +68,8 @@ unsigned long kvm_compute_return_epc(struct kvm_vcpu *vcpu, case jr_op: nextpc = arch->gprs[insn.r_format.rs]; break; + default: + return -EINVAL; } break; @@ -114,8 +118,11 @@ unsigned long kvm_compute_return_epc(struct kvm_vcpu *vcpu, nextpc = epc; break; case bposge32_op: - if (!cpu_has_dsp) - goto sigill; + if (!cpu_has_dsp) { + kvm_err("%s: DSP branch but not DSP ASE\n", + __func__); + return -EINVAL; + } dspcontrol = rddsp(0x01); @@ -125,6 +132,8 @@ unsigned long kvm_compute_return_epc(struct kvm_vcpu *vcpu, epc += 8; nextpc = epc; break; + default: + return -EINVAL; } break; @@ -189,7 +198,7 @@ unsigned long kvm_compute_return_epc(struct kvm_vcpu *vcpu, /* And now the FPA/cp1 branch instructions. */ case cop1_op: kvm_err("%s: unsupported cop1_op\n", __func__); - break; + return -EINVAL; #ifdef CONFIG_CPU_MIPSR6 /* R6 added the following compact branches with forbidden slots */ @@ -198,19 +207,19 @@ unsigned long kvm_compute_return_epc(struct kvm_vcpu *vcpu, /* only rt == 0 isn't compact branch */ if (insn.i_format.rt != 0) goto compact_branch; - break; + return -EINVAL; case pop10_op: case pop30_op: /* only rs == rt == 0 is reserved, rest are compact branches */ if (insn.i_format.rs != 0 || insn.i_format.rt != 0) goto compact_branch; - break; + return -EINVAL; case pop66_op: case pop76_op: /* only rs == 0 isn't compact branch */ if (insn.i_format.rs != 0) goto compact_branch; - break; + return -EINVAL; compact_branch: /* * If we've hit an exception on the forbidden slot, then @@ -221,42 +230,32 @@ compact_branch: break; #else compact_branch: - /* Compact branches not supported before R6 */ - break; + /* Fall through - Compact branches not supported before R6 */ #endif + default: + return -EINVAL; } - return nextpc; - -unaligned: - kvm_err("%s: unaligned epc\n", __func__); - return nextpc; - -sigill: - kvm_err("%s: DSP branch but not DSP ASE\n", __func__); - return nextpc; + *out = nextpc; + return 0; } enum emulation_result update_pc(struct kvm_vcpu *vcpu, u32 cause) { - unsigned long branch_pc; - enum emulation_result er = EMULATE_DONE; + int err; if (cause & CAUSEF_BD) { - branch_pc = kvm_compute_return_epc(vcpu, vcpu->arch.pc); - if (branch_pc == KVM_INVALID_INST) { - er = EMULATE_FAIL; - } else { - vcpu->arch.pc = branch_pc; - kvm_debug("BD update_pc(): New PC: %#lx\n", - vcpu->arch.pc); - } - } else + err = kvm_compute_return_epc(vcpu, vcpu->arch.pc, + &vcpu->arch.pc); + if (err) + return EMULATE_FAIL; + } else { vcpu->arch.pc += 4; + } kvm_debug("update_pc(): New PC: %#lx\n", vcpu->arch.pc); - return er; + return EMULATE_DONE; } /** @@ -1835,12 +1834,14 @@ enum emulation_result kvm_mips_emulate_inst(u32 cause, u32 *opc, { union mips_instruction inst; enum emulation_result er = EMULATE_DONE; + int err; /* Fetch the instruction. */ if (cause & CAUSEF_BD) opc += 1; - - inst.word = kvm_get_inst(opc, vcpu); + err = kvm_get_inst(opc, vcpu, &inst.word); + if (err) + return EMULATE_FAIL; switch (inst.r_format.opcode) { case cop0_op: @@ -2419,6 +2420,7 @@ enum emulation_result kvm_mips_handle_ri(u32 cause, u32 *opc, enum emulation_result er = EMULATE_DONE; unsigned long curr_pc; union mips_instruction inst; + int err; /* * Update PC and hold onto current PC in case there is @@ -2432,11 +2434,9 @@ enum emulation_result kvm_mips_handle_ri(u32 cause, u32 *opc, /* Fetch the instruction. */ if (cause & CAUSEF_BD) opc += 1; - - inst.word = kvm_get_inst(opc, vcpu); - - if (inst.word == KVM_INVALID_INST) { - kvm_err("%s: Cannot get inst @ %p\n", __func__, opc); + err = kvm_get_inst(opc, vcpu, &inst.word); + if (err) { + kvm_err("%s: Cannot get inst @ %p (%d)\n", __func__, opc, err); return EMULATE_FAIL; } diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c index 07ce10e3627a..29afd96069ef 100644 --- a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -1343,6 +1343,7 @@ int kvm_mips_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) u32 __user *opc = (u32 __user *) vcpu->arch.pc; unsigned long badvaddr = vcpu->arch.host_cp0_badvaddr; enum emulation_result er = EMULATE_DONE; + u32 inst; int ret = RESUME_GUEST; /* re-enable HTW before enabling interrupts */ @@ -1467,8 +1468,12 @@ int kvm_mips_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) break; default: + if (cause & CAUSEF_BD) + opc += 1; + inst = 0; + kvm_get_inst(opc, vcpu, &inst); kvm_err("Exception Code: %d, not yet handled, @ PC: %p, inst: 0x%08x BadVaddr: %#lx Status: %#lx\n", - exccode, opc, kvm_get_inst(opc, vcpu), badvaddr, + exccode, opc, inst, badvaddr, kvm_read_c0_guest_status(vcpu->arch.cop0)); kvm_arch_vcpu_dump_regs(vcpu); run->exit_reason = KVM_EXIT_INTERNAL_ERROR; diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c index aab604e75d3b..6379ac1bc7b9 100644 --- a/arch/mips/kvm/mmu.c +++ b/arch/mips/kvm/mmu.c @@ -503,16 +503,15 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) local_irq_restore(flags); } -u32 kvm_get_inst(u32 *opc, struct kvm_vcpu *vcpu) +int kvm_get_inst(u32 *opc, struct kvm_vcpu *vcpu, u32 *out) { - u32 inst; int err; - err = get_user(inst, opc); + err = get_user(*out, opc); if (unlikely(err)) { kvm_err("%s: illegal address: %p\n", __func__, opc); - return KVM_INVALID_INST; + return -EFAULT; } - return inst; + return 0; } -- 2.20.1