From: alex.bennee@linaro•org (Alex Bennée)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 3/3] arm64: kvm: Fix single step for guest skipped instructions
Date: Wed, 04 Oct 2017 19:23:01 +0100 [thread overview]
Message-ID: <87d162ydq2.fsf@linaro.org> (raw)
In-Reply-To: <edd5e138-bda0-7cdf-83b2-8698cfb4304f@arm.com>
Julien Thierry <julien.thierry@arm•com> writes:
> On 04/10/17 16:42, Alex Benn?e wrote:
>>
<snip>
>>
>> Looking more closely though it has a point. The IO/MMIO exits work
>> purely from the address and data entries in the run structure. When we
>> return to KVM_RUN we do:
>>
>> if (run->exit_reason == KVM_EXIT_MMIO) {
>> ret = kvm_handle_mmio_return(vcpu, vcpu->run);
>> if (ret)
>> return ret;
>> }
>>
>> So you are correct the instruction emulation is not complete. Once that
>> fixup is done however I think we are good to return. So perhaps we can
>> avoid involving QEMU entirely in this by generating a debug exit
>> here.
>>
>> QEMU ->
>> kvm_run ->
>> switch ->
>> guest
>> <-
>> trap
>> <-
>> exit mmio
>> <-
>> QEMU
>> -> kvm_run
>> handle_mmio_return
>> exit debug
>> <-
>> QEMU
>>
>
> Thanks for the explanation. This approach sounds good to me. Also, it
> means QEMU doesn't need to get patched, is that correct?
Correct.
>
>> I don't think this affects the handle_exit() case as we only force the
>> exit for successfully emulated instructions inside kvm.
>>
>
> Yes, in handle_exit MMIO handled by the kernel will exit with
> KVM_EXIT_DEBUG and MMIO handled by the userland will exit with
> KVM_EXIT_MMIO. So from your patch we just need to add the exit debug
> after handle_mmio_return.
One minor wrinkle in kvm_handle_mmio_return() I can't use
vcpu->debug_flags as the v7 code doesn't have it in kvm_vcpu_arch.
>
>> Looking at x86 for reference it does seem happy with triggering exits on
>> single stepped emulation, see kvm_vcpu_do_singlestep(). However it also
>> has a number of comments on IO emulation:
>>
>> /* FIXME: return into emulator if single-stepping. */
>>
>> So ARM is at least not behind the curve on this support ;-)
>>
>>>> I'd like to test these patches on some real life examples. I tried
>>>> tracing over the pl011_write code in a kernel boot but I ran into the
>>>> problem of el1_irq's occurring as I tried to step through the guest
>>>> kernel. Is this something you've come across? What MMIO accesses have
>>>> you been using in your testing?
>>>>
>>>
>>> I didn't know which MMIOs were handled by userland so I have only
>>> tested traps and MMIOs handled by the kernel.
>>
>> Any particular MMIOs I could also use to replicate in my tests?
>>
>
> For MMIOs handled by the kernel, I was testing with the GIC. You could
> try to break on gic_cpu_config in the guest, where it will write to
> distributor registers. The function should be called during
> initialization, before IRQs are enabled, so you shouldn't be bothered
> by the earlier trouble.
Thanks - this will be useful.
>>> This sounds like an issue when you are debugging an interruptible
>>> context, an issue Pratyush has been looking at [1]. Are you taking a
>>> guest interrupt when you try to execute the instruction to be stepped?
>>> I don't know what's the status of this patch series. Can you test the
>>> userland MMIO in a non-interruptible context?
>>>
>>> [1]
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/517234.html
>>
>> Again looking at x86 it looks like the approach is to suspend IRQs if
>> you are single-stepping. I'll have a look at Pratyush's patches.
>>
>
> I think that was the idea with Pratyush's patches as well.
>
> So, I'm happy to replace my patch with yours in this series (unless
> you want to post it separated since it doesn't depend on my patches).
Nope I'm happy for you to carry it to merge. I'll see if I can send you
a v2 once I've addressed the mmio completion.
>
> Thank you for looking at this and providing your input.
No problem, sorry it took so long.
--
Alex Benn?e
prev parent reply other threads:[~2017-10-04 18:23 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-30 9:01 [PATCH 0/3] Fix single step for traps Julien Thierry
2017-08-30 9:01 ` [PATCH 1/3] arm64: Use existing defines for mdscr Julien Thierry
2017-08-30 9:01 ` [PATCH 2/3] arm64: Fix single stepping in kernel traps Julien Thierry
2017-08-30 9:01 ` [PATCH 3/3] arm64: kvm: Fix single step for guest skipped instructions Julien Thierry
2017-08-30 9:19 ` Marc Zyngier
2017-08-30 9:40 ` Julien Thierry
2017-08-30 18:53 ` Christoffer Dall
2017-08-31 8:45 ` Julien Thierry
2017-08-31 8:54 ` Christoffer Dall
2017-08-31 9:37 ` Julien Thierry
2017-08-31 10:53 ` Christoffer Dall
2017-08-31 12:56 ` Julien Thierry
2017-08-31 13:28 ` Christoffer Dall
2017-08-31 13:57 ` Julien Thierry
2017-08-31 14:01 ` Christoffer Dall
2017-09-29 12:38 ` Julien Thierry
2017-10-03 14:57 ` Alex Bennée
2017-10-03 15:07 ` Julien Thierry
2017-10-03 15:48 ` Alex Bennée
2017-10-03 16:17 ` Julien Thierry
2017-10-03 16:30 ` Alex Bennée
2017-10-03 17:08 ` Julien Thierry
2017-10-03 17:26 ` Alex Bennée
2017-10-04 8:07 ` Julien Thierry
2017-10-04 10:08 ` Alex Bennée
2017-10-04 10:28 ` Paolo Bonzini
2017-10-04 10:50 ` Alex Bennée
2017-10-04 14:19 ` Paolo Bonzini
2017-10-04 10:42 ` Julien Thierry
2017-10-04 15:42 ` Alex Bennée
2017-10-04 16:10 ` Julien Thierry
2017-10-04 18:23 ` Alex Bennée [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d162ydq2.fsf@linaro.org \
--to=alex.bennee@linaro$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox