From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752149AbdJFMem (ORCPT ); Fri, 6 Oct 2017 08:34:42 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:58786 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751807AbdJFMek (ORCPT ); Fri, 6 Oct 2017 08:34:40 -0400 Subject: Re: [PATCH v1 1/2] KVM: arm64: handle single-stepping trapped instructions To: Marc Zyngier , =?UTF-8?Q?Alex_Benn=c3=a9e?= , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org Cc: Catalin Marinas , Will Deacon , open list References: <20171006113921.24880-1-alex.bennee@linaro.org> <20171006113921.24880-2-alex.bennee@linaro.org> From: Julien Thierry Message-ID: <49ddad48-d3ce-43e2-7aae-f6cc66ef1fad@arm.com> Date: Fri, 6 Oct 2017 13:34:36 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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