Messages in this thread | | | Subject | Re: [PATCH v1 1/2] KVM: arm64: handle single-stepping trapped instructions | From | Julien Thierry <> | Date | Fri, 6 Oct 2017 13:34:36 +0100 |
| |
On 06/10/17 13:27, Marc Zyngier wrote: > On 06/10/17 12:39, Alex Bennée wrote: >> If we are using guest debug to single-step the guest we need to ensure >> we exit after emulating the instruction. This only affects >> instructions completely emulated by the kernel. For userspace emulated >> instructions we need to exit and return to complete the emulation. >> >> We fake debug.arch.hsr to contain ESR_ELx_EC_SOFTSTP_LOW so QEMU knows >> it was a single-step event (and without altering the userspace ABI). >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> arch/arm64/kvm/handle_exit.c | 48 +++++++++++++++++++++++++++++++------------- >> 1 file changed, 34 insertions(+), 14 deletions(-) >> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> index 7debb74843a0..c918d291cb58 100644 >> --- a/arch/arm64/kvm/handle_exit.c >> +++ b/arch/arm64/kvm/handle_exit.c >> @@ -178,6 +178,39 @@ static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu) >> return arm_exit_handlers[hsr_ec]; >> } >> >> +/* >> + * When handling traps we need to ensure exit the guest if we >> + * completely emulated the instruction while single-stepping. Stuff to >> + * be emulated in userspace needs to complete that first. >> + */ >> + >> +static int handle_trap_exceptions(struct kvm_vcpu *vcpu, struct kvm_run *run) >> +{ >> + int handled; >> + >> + /* >> + * See ARM ARM B1.14.1: "Hyp traps on instructions >> + * that fail their condition code check" >> + */ >> + if (!kvm_condition_valid(vcpu)) { >> + kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); >> + handled = 1; >> + } else { >> + exit_handle_fn exit_handler; >> + >> + exit_handler = kvm_get_exit_handler(vcpu); >> + handled = exit_handler(vcpu, run); >> + } >> + >> + if (handled && (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)) { >> + handled = 0; >> + run->exit_reason = KVM_EXIT_DEBUG; >> + run->debug.arch.hsr = ESR_ELx_EC_SOFTSTP_LOW << ESR_ELx_EC_SHIFT; >> + } > > Doesn't this break an MMIO read? The registers haven't been updated yet, > and the debugger may not see the right thing... > > How about something like: > > if (handled && (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)) { > if (run->exit_reason == KVM_EXIT_MMIO) > kvm_handle_mmio_return(vcpu, run); > [...] > } > > Or am I missing something?
If the MMIO was not handled by the kernel, exit_handler will return 0, so handled will be false and we won't pretend we have a debug exception (but will still return to userland with KVM_EXIT_MMIO).
I think the second patch takes care of properly handling single step for userland MMIO.
-- Julien Thierry
| |