[PATCH 3/3] arm64: kvm: Fix single step for guest skipped instructions

[Paolo Bonzini]

On 04/10/2017 12:50, Alex Bennée wrote:
> 
> Paolo Bonzini <pbonzini at redhat.com> writes:
> 
>> On 04/10/2017 12:08, Alex Bennée wrote:
>>>
>>> From 2e8fcea695a9eca67fbeb331d3104d1d9e7e337a Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.bennee at linaro.org>
>>> Date: Wed, 4 Oct 2017 09:49:41 +0000
>>> Subject: [PATCH] kvm: exit run loop after emulating IO when single stepping
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> If single-stepping is enabled we should exit the run-loop after
>>> emulating the access. Otherwise single-stepping across emulated IO
>>> accesses may skip an instruction.
>>>
>>> This only addresses user-space emulation. Stuff done in kernel-mode
>>> should be handled there.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee at linaro.org>
>>> ---
>>>  accel/kvm/kvm-all.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>> index 90c88b517d..85bcb2b0d4 100644
>>> --- a/accel/kvm/kvm-all.c
>>> +++ b/accel/kvm/kvm-all.c
>>> @@ -1940,7 +1940,7 @@ int kvm_cpu_exec(CPUState *cpu)
>>>                            run->io.direction,
>>>                            run->io.size,
>>>                            run->io.count);
>>> -            ret = 0;
>>> +            ret = cpu->singlestep_enabled ? EXCP_DEBUG : 0;
>>>              break;
>>>          case KVM_EXIT_MMIO:
>>>              DPRINTF("handle_mmio\n");
>>> @@ -1950,7 +1950,7 @@ int kvm_cpu_exec(CPUState *cpu)
>>>                               run->mmio.data,
>>>                               run->mmio.len,
>>>                               run->mmio.is_write);
>>> -            ret = 0;
>>> +            ret = cpu->singlestep_enabled ? EXCP_DEBUG : 0;
>>>              break;
>>>          case KVM_EXIT_IRQ_WINDOW_OPEN:
>>>              DPRINTF("irq_window_open\n");
>>
>> Singlestep mode doesn't make much sense for KVM.  For TCG the purpose is
>> to build one-instruction translation blocks, but what would it mean for KVM?
> 
> It's used by the kvm_arch_handle_debug() code to verify single-stepping
> is enabled when processing debug exceptions. And also kvm_update_debug:
> 
>     if (cpu->singlestep_enabled) {
>         data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
>     }
> 
> We also have an aliased singlestep_enabled in our disas_context for the
> translator.

Nevermind, I was confusing cpu->singlestep_enabled with the "singlestep"
global.

Paolo