Commit bd7d95ca authored by Mark Rutland's avatar Mark Rutland Committed by Marc Zyngier
Browse files

arm64: KVM: Consistently advance singlestep when emulating instructions 



When we emulate a guest instruction, we don't advance the hardware
singlestep state machine, and thus the guest will receive a software
step exception after a next instruction which is not emulated by the
host.

We bodge around this in an ad-hoc fashion. Sometimes we explicitly check
whether userspace requested a single step, and fake a debug exception
from within the kernel. Other times, we advance the HW singlestep state
rely on the HW to generate the exception for us. Thus, the observed step
behaviour differs for host and guest.

Let's make this simpler and consistent by always advancing the HW
singlestep state machine when we skip an instruction. Thus we can rely
on the hardware to generate the singlestep exception for us, and never
need to explicitly check for an active-pending step, nor do we need to
fake a debug exception from the guest.

Cc: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: default avatarAlex Bennée <alex.bennee@linaro.org>
Reviewed-by: default avatarChristoffer Dall <christoffer.dall@arm.com>
Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Signed-off-by: default avatarMarc Zyngier <marc.zyngier@arm.com>
parent 0d640732

arch/arm/include/asm/kvm_host.h

+0 −5
Original line number Diff line number Diff line
@@ -296,11 +296,6 @@ static inline void kvm_arm_init_debug(void) {} 
static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {}

static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {}

static inline void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu) {}

static inline bool kvm_arm_handle_step_debug(struct kvm_vcpu *vcpu,

					     struct kvm_run *run)

{

	return false;

}



int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,

			       struct kvm_device_attr *attr);

arch/arm64/include/asm/kvm_emulate.h

+27 −8
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 


#include <linux/kvm_host.h>



#include <asm/debug-monitors.h>

#include <asm/esr.h>

#include <asm/kvm_arm.h>

#include <asm/kvm_hyp.h>
@@ -147,14 +148,6 @@ static inline bool kvm_condition_valid(const struct kvm_vcpu *vcpu) 
	return true;

}



static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool is_wide_instr)

{

	if (vcpu_mode_is_32bit(vcpu))

		kvm_skip_instr32(vcpu, is_wide_instr);

	else

		*vcpu_pc(vcpu) += 4;

}



static inline void vcpu_set_thumb(struct kvm_vcpu *vcpu)

{

	*vcpu_cpsr(vcpu) |= PSR_AA32_T_BIT;
@@ -424,4 +417,30 @@ static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu, 
	return data;		/* Leave LE untouched */

}



static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool is_wide_instr)

{

	if (vcpu_mode_is_32bit(vcpu))

		kvm_skip_instr32(vcpu, is_wide_instr);

	else

		*vcpu_pc(vcpu) += 4;



	/* advance the singlestep state machine */

	*vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;

}



/*

 * Skip an instruction which has been emulated at hyp while most guest sysregs

 * are live.

 */

static inline void __hyp_text __kvm_skip_instr(struct kvm_vcpu *vcpu)

{

	*vcpu_pc(vcpu) = read_sysreg_el2(elr);

	vcpu->arch.ctxt.gp_regs.regs.pstate = read_sysreg_el2(spsr);



	kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));



	write_sysreg_el2(vcpu->arch.ctxt.gp_regs.regs.pstate, spsr);

	write_sysreg_el2(*vcpu_pc(vcpu), elr);

}



#endif /* __ARM64_KVM_EMULATE_H__ */

arch/arm64/include/asm/kvm_host.h

+0 −1
Original line number Diff line number Diff line
@@ -445,7 +445,6 @@ void kvm_arm_init_debug(void); 
void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);

void kvm_arm_clear_debug(struct kvm_vcpu *vcpu);

void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu);

bool kvm_arm_handle_step_debug(struct kvm_vcpu *vcpu, struct kvm_run *run);

int kvm_arm_vcpu_arch_set_attr(struct kvm_vcpu *vcpu,

			       struct kvm_device_attr *attr);

int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,

arch/arm64/kvm/debug.c

+0 −21
Original line number Diff line number Diff line
@@ -236,24 +236,3 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) 
		}

	}

}
 




/*

 * After successfully emulating an instruction, we might want to

 * return to user space with a KVM_EXIT_DEBUG. We can only do this

 * once the emulation is complete, though, so for userspace emulations

 * we have to wait until we have re-entered KVM before calling this

 * helper.

 *

 * Return true (and set exit_reason) to return to userspace or false

 * if no further action is required.

 */

bool kvm_arm_handle_step_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)

{

	if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {

		run->exit_reason = KVM_EXIT_DEBUG;

		run->debug.arch.hsr = ESR_ELx_EC_SOFTSTP_LOW << ESR_ELx_EC_SHIFT;

		return true;

	}

	return false;

}

arch/arm64/kvm/handle_exit.c

+1 −13
Original line number Diff line number Diff line
@@ -229,13 +229,6 @@ static int handle_trap_exceptions(struct kvm_vcpu *vcpu, struct kvm_run *run) 
		handled = exit_handler(vcpu, run);

	}



	/*

	 * kvm_arm_handle_step_debug() sets the exit_reason on the kvm_run

	 * structure if we need to return to userspace.

	 */

	if (handled > 0 && kvm_arm_handle_step_debug(vcpu, run))

		handled = 0;



	return handled;

}
@@ -269,11 +262,6 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run, 
	case ARM_EXCEPTION_IRQ:

		return 1;

	case ARM_EXCEPTION_EL1_SERROR:

		/* We may still need to return for single-step */

		if (!(*vcpu_cpsr(vcpu) & DBG_SPSR_SS)

			&& kvm_arm_handle_step_debug(vcpu, run))

			return 0;

		else

		return 1;

	case ARM_EXCEPTION_TRAP:

		return handle_trap_exceptions(vcpu, run);

CRA Git | Maintained and supported by SUSTech CRA and CCSE