lkml.org 
[lkml]   [2017]   [Oct]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 1/2] KVM: arm64: handle single-stepping trapped instructions
From
Date


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

\
 
 \ /
  Last update: 2017-10-06 14:35    [W:0.091 / U:0.644 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site