Commit 122e51d4 authored by James Hogan's avatar James Hogan
Browse files

KVM: MIPS: Improve kvm_get_inst() error return 



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: default avatarJames Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: kvm@vger.kernel.org
parent a1ecc54d

arch/mips/include/asm/kvm_host.h

+1 −2
Original line number Diff line number Diff line
@@ -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);



/**

arch/mips/kvm/emulate.c

+45 −45
Original line number Diff line number Diff line
@@ -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;

		err = kvm_compute_return_epc(vcpu, vcpu->arch.pc,

					     &vcpu->arch.pc);

		if (err)

			return EMULATE_FAIL;

	} else {

			vcpu->arch.pc = branch_pc;

			kvm_debug("BD update_pc(): New PC: %#lx\n",

				  vcpu->arch.pc);

		}

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

	}

arch/mips/kvm/mips.c

+6 −1
Original line number Diff line number Diff line
@@ -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;

arch/mips/kvm/mmu.c

+4 −5
Original line number Diff line number Diff line
@@ -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;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE