| Message ID | e84fd80c-d6a6-8f19-a4e1-ed309fa68aa9@ilande.co.uk |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: Book3S PR: unbreaking software breakpoints | expand |
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. >
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.
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.
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.
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.
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.
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;
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? advance = 0; ATB, Mark.