KVM: Book3S PR: unbreaking software breakpoints

KVM: Book3S PR: unbreaking software breakpoints

[Mark Cave-Ayland]

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.

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.

Re: KVM: Book3S PR: unbreaking software breakpoints

[Alexey Kardashevskiy]

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 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.
> 

-- 
Alexey

Re: KVM: Book3S PR: unbreaking software breakpoints

[Mark Cave-Ayland]

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 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.

Re: KVM: Book3S PR: unbreaking software breakpoints

[Fabiano Rosas]

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

> 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;

This looks reasonable to me.

>
>
> ATB,
>
> Mark.

Re: KVM: Book3S PR: unbreaking software breakpoints

[Fabiano Rosas]

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.

Re: KVM: Book3S PR: unbreaking software breakpoints

[Mark Cave-Ayland]

On 13/05/2019 19:14, Fabiano Rosas wrote:

> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> 
>> 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;
> 
> This looks reasonable to me.

Great, thanks for the review! I'm fairly busy for the next few days, but will send a
proper patch towards the end of the week.


ATB,

Mark.

Re: KVM: Book3S PR: unbreaking software breakpoints

[Mark Cave-Ayland]

On 13/05/2019 19:22, Fabiano Rosas wrote:

> 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.

Yes indeed, I can confirm that's the sequence which is causing the issue with
single-stepping here:

0x0020f0a4:  8203001c  lwz      r16, 0x1c(r3)
0x0020f0a8:  7cba03a6  mtspr    0x1a, r5
0x0020f0ac:  38003000  li       r0, 0x3000
0x0020f0b0:  7c1b03a6  mtspr    0x1b, r0
0x0020f0b4:  4c000064  rfi

What would be the right way to handle this? Is it necessary to emulate rfi if
single-step mode is enabled to ensure MSR_SE is still set?


ATB,

Mark.