Re: KVM: Book3S PR: unbreaking software breakpoints

[Fabiano Rosas <farosas@linux.ibm.com>]

Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:

> On 13/05/2019 07:01, Alexey Kardashevskiy wrote:
>
>> On 12/05/2019 00:08, Mark Cave-Ayland wrote:
>>> Hi all,
>>>
>>> Whilst trying to investigate some issues with MacOS under KVM PR I noticed that when
>>> setting software breakpoints the KVM VCPU would stop as requested, but QEMU's gdbstub
>>> would hang indefinitely.
>> 
>> What are you trying to do exactly? Just breakpoints or single stepping?
>> Anyway, I am cc-ing Fabiano who is fixing single stepping and knows this
>> code well.
>
> I'm currently investigating why MacOS 9 fails to start up on KVM using a G4 Mac Mini,
> and my starting point is to do a side-by-side comparison with TCG which can boot MacOS 9.
>
> I discovered this issue setting a software breakpoint using QEMU's gdbstub and
> finding that whilst execution of the vCPU paused as expected, QEMU would hang because
> with run->debug.arch.status != 0 the gdbstub tries to handle it as a hardware
> breakpoint instead of a software breakpoint causing confusion.
>
> I've also tried using single-stepping which mostly works, however during OS startup
> as soon as I step over a mtsrr1 instruction, I lose the single-stepping and vCPU runs
> as normal. My suspicion here is that something in the emulation code is losing the
> MSR_SE bit, but I need to dig a bit deeper here.

I would expect that a mtsrr1 followed by rfid would cause this sort of
behavior since MSR_SE is set/cleared at each guest entry/exit
(kvmppc_setup_debug and kvmppc_clear_debug functions) and whatever was
copied into SRR1 might not have MSR_SE set.

>>> I eventually traced it down to this code in QEMU's target/ppc/kvm.c:
>>>
>>>
>>> static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
>>> {
>>>     CPUState *cs = CPU(cpu);
>>>     CPUPPCState *env = &cpu->env;
>>>     struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
>>>
>>>     if (cs->singlestep_enabled) {
>>>         return kvm_handle_singlestep();
>>>     }
>>>
>>>     if (arch_info->status) {
>>>         return kvm_handle_hw_breakpoint(cs, arch_info);
>>>     }
>>>
>>>     if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
>>>         return kvm_handle_sw_breakpoint();
>>>     }
>>>
>>>
>>> The problem here is that with Book3S PR on my Mac hardware, run->debug.arch.status !>>> 0 which causes QEMU to think that this is a hardware breakpoint and so the software
>>> breakpoint doesn't get handled correctly.
>>>
>>> For comparison both booke.c and e500_emulate.c set debug.arch.status = 0 for software
>>> breakpoints, whereas both book3s_hv.c and book3s_pr.c do not. Given that emulate.c
>>> contains shared code for handling software breakpoints, would the following simple
>>> patch suffice?
>>>
>>>
>>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
>>> index 9f5b8c01c4e1..e77becaad5dd 100644
>>> --- a/arch/powerpc/kvm/emulate.c
>>> +++ b/arch/powerpc/kvm/emulate.c
>>> @@ -282,6 +282,7 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct
>>> kvm_vcpu *vcpu)
>>>                  */
>>>                 if (inst = KVMPPC_INST_SW_BREAKPOINT) {
>>>                         run->exit_reason = KVM_EXIT_DEBUG;
>>> +                       run->debug.arch.status = 0;
>>>                         run->debug.arch.address = kvmppc_get_pc(vcpu);
>>>                         emulated = EMULATE_EXIT_USER;
>>>                         advance = 0;
>
>
> ATB,
>
> Mark.