| Message ID | 20190228225759.21328-6-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: single step for KVM HV | expand |
On 01/03/2019 09:57, Fabiano Rosas wrote: > The hardware singlestep mechanism in POWER works via a Trace Interrupt > (0xd00) that happens after any instruction executes, whenever MSR_SE = > 1 (PowerISA Section 6.5.15 - Trace Interrupt). > > However, with kvm_hv, the Trace Interrupt happens inside the guest and > KVM has no visibility of it. Therefore, when the gdbstub uses the > KVM_SET_GUEST_DEBUG ioctl to enable singlestep, KVM simply ignores it. > > This patch takes advantage of the Trace Interrupt to perform the step > inside the guest, but uses a breakpoint at the Trace Interrupt handler > to return control to KVM. The exit is treated by KVM as a regular > breakpoint and it returns to the host (and QEMU eventually). > > Before signalling GDB, QEMU sets the Next Instruction Pointer to the > instruction following the one being stepped and restores the MSR, > SRR0, SRR1 values from before the step, effectively skipping the > interrupt handler execution and hiding the trace interrupt breakpoint > from GDB. > > This approach works with both of GDB's 'scheduler-locking' options > (off, step). > > Note: > > - kvm_arch_set_singlestep happens after GDB asks for a single step, > while the vcpus are stopped. > > - kvm_handle_singlestep executes after the step, during the handling > of the Emulation Assist Interrupt (breakpoint). > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> Looks good to me, does not break what already works. However I cannot debug SLOF real mode and I am not sure why. (gdb) set endian big The target is assumed to be big endian (gdb) b *0x3f00 Breakpoint 2 at 0x3f00 (gdb) info breakpoints Num Type Disp Enb Address What 1 breakpoint keep y 0x0000000000000100 2 breakpoint keep y 0x0000000000003f00 (gdb) p $pc $1 = (void (*)()) 0x100 (gdb) x/1w $pc 0x100: 0x48003f00 (gdb) c Continuing. Thread 1 hit Breakpoint 1, 0x0000000000000100 in ?? () (gdb) c Continuing. So breakpoint#2 did not stop and single stepping did not work either although there is an exception handler in SLOF. What do I miss? Thanks, SLOF is loaded at 0 and the guest starts at 0x100 which is https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/llfw/startup.S;h=bbd3ce3ac51bf7f6fabbbe0de76dab8915bd536f;hb=HEAD#l33 > --- > target/ppc/cpu.h | 16 ++++ > target/ppc/excp_helper.c | 13 +++ > target/ppc/gdbstub.c | 35 +++++++ > target/ppc/kvm.c | 195 +++++++++++++++++++++++++++++++++++++-- > 4 files changed, 252 insertions(+), 7 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 26604ddf98..fb1bf1e9d7 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -1171,6 +1171,11 @@ struct CPUPPCState { > uint32_t tm_vscr; > uint64_t tm_dscr; > uint64_t tm_tar; > + > + /* Used for software single step */ > + target_ulong sstep_msr; > + target_ulong sstep_srr0; > + target_ulong sstep_srr1; > }; > > #define SET_FIT_PERIOD(a_, b_, c_, d_) \ > @@ -1266,6 +1271,7 @@ struct PPCVirtualHypervisorClass { > OBJECT_GET_CLASS(PPCVirtualHypervisorClass, (obj), \ > TYPE_PPC_VIRTUAL_HYPERVISOR) > > +target_ulong ppc_get_trace_int_handler_addr(CPUState *cs); > void ppc_cpu_do_interrupt(CPUState *cpu); > bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req); > void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf, > @@ -1281,6 +1287,12 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cpu, uint8_t *buf, int reg); > void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu); > const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name); > #endif > +uint32_t ppc_gdb_read_insn(CPUState *cs, target_ulong addr); > +uint32_t ppc_gdb_get_op(uint32_t insn); > +uint32_t ppc_gdb_get_xop(uint32_t insn); > +uint32_t ppc_gdb_get_spr(uint32_t insn); > +uint32_t ppc_gdb_get_rt(uint32_t insn); > + > int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs, > int cpuid, void *opaque); > int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs, > @@ -2227,6 +2239,10 @@ enum { > PPC2_ISA300) > }; > > +#define XOP_RFID 18 > +#define XOP_MFMSR 83 > +#define XOP_MTSPR 467 > + > /*****************************************************************************/ > /* Memory access type : > * may be needed for precise access rights control and precise exceptions. > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index beafcf1ebd..cf8c86de91 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -112,6 +112,8 @@ static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail) > uint64_t offset = 0; > > switch (ail) { > + case AIL_NONE: > + break; > case AIL_0001_8000: > offset = 0x18000; > break; > @@ -768,6 +770,17 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > check_tlb_flush(env, false); > } > > +target_ulong ppc_get_trace_int_handler_addr(CPUState *cs) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + int ail; > + > + ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; > + return env->excp_vectors[POWERPC_EXCP_TRACE] | > + ppc_excp_vector_offset(cs, ail); > +} > + > void ppc_cpu_do_interrupt(CPUState *cs) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c > index fbf3821f4b..a5b2705ade 100644 > --- a/target/ppc/gdbstub.c > +++ b/target/ppc/gdbstub.c > @@ -380,3 +380,38 @@ const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name) > return NULL; > } > #endif > + > +uint32_t ppc_gdb_read_insn(CPUState *cs, target_ulong addr) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + uint32_t insn; > + > + cpu_memory_rw_debug(cs, addr, (uint8_t *)&insn, sizeof(insn), 0); > + > + if (msr_le) { > + return ldl_le_p(&insn); > + } else { > + return ldl_be_p(&insn); > + } > +} > + > +uint32_t ppc_gdb_get_op(uint32_t insn) > +{ > + return extract32(insn, 26, 6); > +} > + > +uint32_t ppc_gdb_get_xop(uint32_t insn) > +{ > + return extract32(insn, 1, 10); > +} > + > +uint32_t ppc_gdb_get_spr(uint32_t insn) > +{ > + return extract32(insn, 11, 5) << 5 | extract32(insn, 16, 5); > +} > + > +uint32_t ppc_gdb_get_rt(uint32_t insn) > +{ > + return extract32(insn, 21, 5); > +} > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 9392fba192..2bcb8814f4 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -1554,6 +1554,86 @@ void kvm_arch_remove_all_hw_breakpoints(void) > nb_hw_breakpoint = nb_hw_watchpoint = 0; > } > > +void kvm_arch_set_singlestep(CPUState *cs, int enabled) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + target_ulong trace_handler_addr; > + uint32_t insn; > + bool rfid; > + > + if (!enabled) { > + return; > + } > + > + cpu_synchronize_state(cs); > + insn = ppc_gdb_read_insn(cs, env->nip); > + > + /* > + * rfid needs special handling because it: > + * - overwrites NIP with SRR0; > + * - overwrites MSR with SRR1; > + * - cannot be single stepped. > + */ > + rfid = ppc_gdb_get_op(insn) == 19 && ppc_gdb_get_xop(insn) == XOP_RFID; > + > + if (rfid && kvm_find_sw_breakpoint(cs, env->spr[SPR_SRR0])) { > + /* > + * There is a breakpoint at the next instruction address. It > + * will already cause the vm exit we need for the single step, > + * so there's nothing to be done. > + */ > + return; > + } > + > + /* > + * Save the registers that will be affected by the single step > + * mechanism. These will be restored after the step at > + * kvm_handle_singlestep. > + */ > + env->sstep_msr = env->msr; > + env->sstep_srr0 = env->spr[SPR_SRR0]; > + env->sstep_srr1 = env->spr[SPR_SRR1]; > + > + /* > + * MSR_SE = 1 will cause a Trace Interrupt in the guest after the > + * next instruction executes. If this is a rfid, use SRR1 instead > + * of MSR. > + */ > + if (rfid) { > + if ((env->spr[SPR_SRR1] >> MSR_SE) & 1) { > + /* > + * The guest is doing a single step itself. Make sure we > + * restore it later. > + */ > + env->sstep_msr |= (1ULL << MSR_SE); > + } > + > + env->spr[SPR_SRR1] |= (1ULL << MSR_SE); > + } else { > + env->msr |= (1ULL << MSR_SE); > + } > + > + /* > + * We set a breakpoint at the interrupt handler address so > + * that the singlestep will be seen by KVM (this is treated by > + * KVM like an ordinary breakpoint) and control is returned to > + * QEMU. > + */ > + trace_handler_addr = ppc_get_trace_int_handler_addr(cs); > + > + if (env->nip == trace_handler_addr) { > + /* > + * We are trying to step over the interrupt handler > + * address itself; move the breakpoint to the next > + * instruction. > + */ > + trace_handler_addr += 4; > + } > + > + kvm_insert_breakpoint(cs, trace_handler_addr, 4, GDB_BREAKPOINT_SW); > +} > + > void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) > { > int n; > @@ -1593,6 +1673,91 @@ void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) > } > } > > +/* Revert any side-effects caused during single step */ > +static void restore_singlestep_env(CPUState *cs) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + uint32_t insn; > + int reg; > + int spr; > + > + insn = ppc_gdb_read_insn(cs, env->spr[SPR_SRR0] - 4); > + > + env->spr[SPR_SRR0] = env->sstep_srr0; > + env->spr[SPR_SRR1] = env->sstep_srr1; > + > + if (ppc_gdb_get_op(insn) != 31) { > + return; > + } > + > + reg = ppc_gdb_get_rt(insn); > + > + switch (ppc_gdb_get_xop(insn)) { > + case XOP_MTSPR: > + /* > + * mtspr: the guest altered the SRR, so do not use the > + * pre-step value. > + */ > + spr = ppc_gdb_get_spr(insn); > + if (spr == SPR_SRR0 || spr == SPR_SRR1) { > + env->spr[spr] = env->gpr[reg]; > + } > + break; > + case XOP_MFMSR: > + /* > + * mfmsr: clear MSR_SE bit to avoid the guest knowing > + * that it is being single-stepped. > + */ > + env->gpr[reg] &= ~(1ULL << MSR_SE); > + break; > + } > +} > + > +static int kvm_handle_singlestep(CPUState *cs, target_ulong address) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + target_ulong trace_handler_addr; > + > + if (kvm_has_guestdbg_singlestep()) { > + return 1; > + } > + > + cpu_synchronize_state(cs); > + trace_handler_addr = ppc_get_trace_int_handler_addr(cs); > + > + if (address == trace_handler_addr) { > + kvm_remove_breakpoint(cs, trace_handler_addr, 4, GDB_BREAKPOINT_SW); > + > + if (env->sstep_msr & (1ULL << MSR_SE)) { > + /* > + * The guest expects the last instruction to have caused a > + * single step, go back into the interrupt handler. > + */ > + return 1; > + } > + > + env->nip = env->spr[SPR_SRR0]; > + /* Bits 33-36, 43-47 are set by the interrupt */ > + env->msr = env->spr[SPR_SRR1] & ~(1ULL << MSR_SE | > + PPC_BITMASK(33, 36) | > + PPC_BITMASK(43, 47)); > + restore_singlestep_env(cs); > + > + } else if (address == trace_handler_addr + 4) { > + /* > + * A step at trace_handler_addr would interfere with the > + * singlestep mechanism itself, so we have previously > + * displaced the breakpoint to the next instruction. > + */ > + kvm_remove_breakpoint(cs, trace_handler_addr + 4, 4, GDB_BREAKPOINT_SW); > + restore_singlestep_env(cs); > + } > + > + return 1; > +} > + > static int kvm_handle_hw_breakpoint(CPUState *cs, > struct kvm_debug_exit_arch *arch_info) > { > @@ -1620,13 +1785,29 @@ static int kvm_handle_hw_breakpoint(CPUState *cs, > return handle; > } > > -static int kvm_handle_singlestep(void) > +static int kvm_handle_sw_breakpoint(CPUState *cs, target_ulong address) > { > - return 1; > -} > + target_ulong trace_handler_addr; > > -static int kvm_handle_sw_breakpoint(void) > -{ > + if (kvm_has_guestdbg_singlestep()) { > + return 1; > + } > + > + cpu_synchronize_state(cs); > + trace_handler_addr = ppc_get_trace_int_handler_addr(cs); > + > + if (address == trace_handler_addr) { > + CPU_FOREACH(cs) { > + if (cs->singlestep_enabled) { > + /* > + * We hit this breakpoint while another cpu is doing a > + * software single step. Go back into the guest to > + * give chance for the single step to finish. > + */ > + return 0; > + } > + } > + } > return 1; > } > > @@ -1637,7 +1818,7 @@ static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) > struct kvm_debug_exit_arch *arch_info = &run->debug.arch; > > if (cs->singlestep_enabled) { > - return kvm_handle_singlestep(); > + return kvm_handle_singlestep(cs, arch_info->address); > } > > if (arch_info->status) { > @@ -1645,7 +1826,7 @@ static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) > } > > if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > - return kvm_handle_sw_breakpoint(); > + return kvm_handle_sw_breakpoint(cs, arch_info->address); > } > > /* >
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > Looks good to me, does not break what already works. However I cannot > debug SLOF real mode and I am not sure why. > > (gdb) set endian big > > The target is assumed to be big endian > (gdb) b *0x3f00 > > Breakpoint 2 at 0x3f00 I think I'm missing the point here. Why 0x3f00? (qemu) info roms addr=0000000000000000 size=0x0e22b8 mem=ram name="...qemu/slof.bin" addr=0000000000400000 size=0x17976d0 mem=ram name="...vmlinux" $ objdump -d board-qemu/llfw/stage1.elf | grep "_start>" 0000000000000100 <__start>: 100: 48 00 3f 00 b 4000 <_start> 0000000000004000 <_start>: Thread 1 hit Breakpoint 3, _start () at startup.S:82 (gdb) p/x $pc $1 = 0x4000 (gdb) si (gdb) p/x $pc $3 = 0x4004 (gdb) c Thread 1 hit Breakpoint 4, early_c_entry (start_addr=49056, fdt_addr=49024) at stage2.c:202 (gdb) p/x $pc $4 = 0x4d18
On 20/03/2019 01:32, Fabiano Rosas wrote: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: > >> Looks good to me, does not break what already works. However I cannot >> debug SLOF real mode and I am not sure why. >> >> (gdb) set endian big >> >> The target is assumed to be big endian >> (gdb) b *0x3f00 >> >> Breakpoint 2 at 0x3f00 > > I think I'm missing the point here. Why 0x3f00? Because I am stupid and did not realize that 0x3f00 is a relative offset and 0x4000 is the correct address which works. Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > (qemu) info roms > addr=0000000000000000 size=0x0e22b8 mem=ram name="...qemu/slof.bin" > addr=0000000000400000 size=0x17976d0 mem=ram name="...vmlinux" > > > $ objdump -d board-qemu/llfw/stage1.elf | grep "_start>" > 0000000000000100 <__start>: > 100: 48 00 3f 00 b 4000 <_start> > 0000000000004000 <_start>: > > > Thread 1 hit Breakpoint 3, _start () at startup.S:82 > (gdb) p/x $pc > $1 = 0x4000 > (gdb) si > (gdb) p/x $pc > $3 = 0x4004 > (gdb) c > Thread 1 hit Breakpoint 4, early_c_entry (start_addr=49056, fdt_addr=49024) at stage2.c:202 > (gdb) p/x $pc > $4 = 0x4d18 >
Are you reposting this any time soon? In meanwhile I hit a problem when I cannot step over the "stdu" instruction. I basically put this: stdu r1,-368(r1) and "ni" in gdb does not stop on the next instruction which is quite confusing. Ideas? On 20/03/2019 12:42, Alexey Kardashevskiy wrote: > > > On 20/03/2019 01:32, Fabiano Rosas wrote: >> Alexey Kardashevskiy <aik@ozlabs.ru> writes: >> >>> Looks good to me, does not break what already works. However I cannot >>> debug SLOF real mode and I am not sure why. >>> >>> (gdb) set endian big >>> >>> The target is assumed to be big endian >>> (gdb) b *0x3f00 >>> >>> Breakpoint 2 at 0x3f00 >> >> I think I'm missing the point here. Why 0x3f00? > > Because I am stupid and did not realize that 0x3f00 is a relative offset > and 0x4000 is the correct address which works. > > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > >> >> (qemu) info roms >> addr=0000000000000000 size=0x0e22b8 mem=ram name="...qemu/slof.bin" >> addr=0000000000400000 size=0x17976d0 mem=ram name="...vmlinux" >> >> >> $ objdump -d board-qemu/llfw/stage1.elf | grep "_start>" >> 0000000000000100 <__start>: >> 100: 48 00 3f 00 b 4000 <_start> >> 0000000000004000 <_start>: >> >> >> Thread 1 hit Breakpoint 3, _start () at startup.S:82 >> (gdb) p/x $pc >> $1 = 0x4000 >> (gdb) si >> (gdb) p/x $pc >> $3 = 0x4004 >> (gdb) c >> Thread 1 hit Breakpoint 4, early_c_entry (start_addr=49056, fdt_addr=49024) at stage2.c:202 >> (gdb) p/x $pc >> $4 = 0x4d18 >> >
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > Are you reposting this any time soon? I have sent a v2 to the kernel side of it: https://lore.kernel.org/kvm/20190529222219.27994-1-farosas@linux.ibm.com/ I'm depending on what we decide to do there. The core of this patchset will not change, just the mechanism by which the feature is selected (patch 2, kvm-all: Introduce kvm_set_singlestep). > In meanwhile I hit a problem when I cannot step over the "stdu" instruction. > > I basically put this: > stdu r1,-368(r1) > > and "ni" in gdb does not stop on the next instruction which is quite > confusing. Ideas? Perhaps the next instruction is one that is not traced? See the programming note at the end of section 6.5.15 in Book III. Or maybe the step got preempted? You should see GDB messages indicating changing threads right before or after the stdu. However that would only happen with more than one VCPU and if 'show scheduler-locking' in GDB is 'off'. And even then, that should not cause any issues, but it is a more complex scenario so there could be a bug in the code. > On 20/03/2019 12:42, Alexey Kardashevskiy wrote: >> >> >> On 20/03/2019 01:32, Fabiano Rosas wrote: >>> Alexey Kardashevskiy <aik@ozlabs.ru> writes: >>> >>>> Looks good to me, does not break what already works. However I cannot >>>> debug SLOF real mode and I am not sure why. >>>> >>>> (gdb) set endian big >>>> >>>> The target is assumed to be big endian >>>> (gdb) b *0x3f00 >>>> >>>> Breakpoint 2 at 0x3f00 >>> >>> I think I'm missing the point here. Why 0x3f00? >> >> Because I am stupid and did not realize that 0x3f00 is a relative offset >> and 0x4000 is the correct address which works. >> >> >> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> >> >>> >>> (qemu) info roms >>> addr=0000000000000000 size=0x0e22b8 mem=ram name="...qemu/slof.bin" >>> addr=0000000000400000 size=0x17976d0 mem=ram name="...vmlinux" >>> >>> >>> $ objdump -d board-qemu/llfw/stage1.elf | grep "_start>" >>> 0000000000000100 <__start>: >>> 100: 48 00 3f 00 b 4000 <_start> >>> 0000000000004000 <_start>: >>> >>> >>> Thread 1 hit Breakpoint 3, _start () at startup.S:82 >>> (gdb) p/x $pc >>> $1 = 0x4000 >>> (gdb) si >>> (gdb) p/x $pc >>> $3 = 0x4004 >>> (gdb) c >>> Thread 1 hit Breakpoint 4, early_c_entry (start_addr=49056, fdt_addr=49024) at stage2.c:202 >>> (gdb) p/x $pc >>> $4 = 0x4d18 >>> >>
On 12/06/2019 23:34, Fabiano Rosas wrote: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: > >> Are you reposting this any time soon? > > I have sent a v2 to the kernel side of it: > > https://lore.kernel.org/kvm/20190529222219.27994-1-farosas@linux.ibm.com/ > > I'm depending on what we decide to do there. The core of this patchset > will not change, just the mechanism by which the feature is selected > (patch 2, kvm-all: Introduce kvm_set_singlestep). > >> In meanwhile I hit a problem when I cannot step over the "stdu" instruction. >> >> I basically put this: >> stdu r1,-368(r1) >> >> and "ni" in gdb does not stop on the next instruction which is quite >> confusing. Ideas? > > Perhaps the next instruction is one that is not traced? See the programming > note at the end of section 6.5.15 in Book III. No, it is not it, it is more subtle. The problem piece was originally this: c0000000010f16b4: f0 ff c1 fb std r30,-16(r1) c0000000010f16b8: f8 ff e1 fb std r31,-8(r1) c0000000010f16bc: 91 fe 21 f8 stdu r1,-368(r1) c0000000010f16c0: 39 00 22 3d addis r9,r2,57 c0000000010f16c4: 18 97 49 39 addi r10,r9,-26856 c0000000010f16c8: 08 00 22 3d addis r9,r2,8 c0000000010f16cc: 00 78 29 39 addi r9,r9,30720 It is Linux'es prom_init(). > Or maybe the step got preempted? You should see GDB messages indicating > changing threads right before or after the stdu. However that would only > happen with more than one VCPU and if 'show scheduler-locking' in GDB is > 'off'. And even then, that should not cause any issues, but it is a more > complex scenario so there could be a bug in the code. It is TCG, a single CPU with a single thread and no matter where I put this "stdu r1,-368(r1)" - GDB does not stop on the next one and just runs. In the example above: 1. "b *0x10f16bc" makes GDB stop there, "ni" continues without stopping on at 0x10f16c0. 2. "b *0x10f16bc" and "b *0x10f16c0" make GDB stop at 0x10f16bc and "ni" steps to 0x10f16c0 but it is rather because it is a breakpoint and not the next instruction. 3. "b *0x10f16bc" and "b *0x10f16c4" make GDB stop at 0x10f16bc and "ni" stops GDB at 0x10f16bc but again it is a breakpoint. In 2 and 3 it is possible to continue step debugging till the next "stdu". > >> On 20/03/2019 12:42, Alexey Kardashevskiy wrote: >>> >>> >>> On 20/03/2019 01:32, Fabiano Rosas wrote: >>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes: >>>> >>>>> Looks good to me, does not break what already works. However I cannot >>>>> debug SLOF real mode and I am not sure why. >>>>> >>>>> (gdb) set endian big >>>>> >>>>> The target is assumed to be big endian >>>>> (gdb) b *0x3f00 >>>>> >>>>> Breakpoint 2 at 0x3f00 >>>> >>>> I think I'm missing the point here. Why 0x3f00? >>> >>> Because I am stupid and did not realize that 0x3f00 is a relative offset >>> and 0x4000 is the correct address which works. >>> >>> >>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>> >>> >>>> >>>> (qemu) info roms >>>> addr=0000000000000000 size=0x0e22b8 mem=ram name="...qemu/slof.bin" >>>> addr=0000000000400000 size=0x17976d0 mem=ram name="...vmlinux" >>>> >>>> >>>> $ objdump -d board-qemu/llfw/stage1.elf | grep "_start>" >>>> 0000000000000100 <__start>: >>>> 100: 48 00 3f 00 b 4000 <_start> >>>> 0000000000004000 <_start>: >>>> >>>> >>>> Thread 1 hit Breakpoint 3, _start () at startup.S:82 >>>> (gdb) p/x $pc >>>> $1 = 0x4000 >>>> (gdb) si >>>> (gdb) p/x $pc >>>> $3 = 0x4004 >>>> (gdb) c >>>> Thread 1 hit Breakpoint 4, early_c_entry (start_addr=49056, fdt_addr=49024) at stage2.c:202 >>>> (gdb) p/x $pc >>>> $4 = 0x4d18 >>>> >>>
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > It is TCG, a single CPU with a single thread and no matter where I put Ok, but then none of this code gets executed because it is kvm-only: qemu/exec.c void cpu_single_step(CPUState *cpu, int enabled) { if (cpu->singlestep_enabled != enabled) { cpu->singlestep_enabled = enabled; if (kvm_enabled()) { -> kvm_set_singlestep(cpu, enabled); } else { /* must flush all the translated code to avoid inconsistencies */ /* XXX: only flush what is necessary */ tb_flush(cpu); } } } > this "stdu r1,-368(r1)" - GDB does not stop on the next one and just > runs. > > In the example above: > 1. "b *0x10f16bc" makes GDB stop there, "ni" continues without stopping > on at 0x10f16c0. But this seems wrong anyway. Let me try to reproduce it and see what I can find. > 2. "b *0x10f16bc" and "b *0x10f16c0" make GDB stop at 0x10f16bc and "ni" > steps to 0x10f16c0 but it is rather because it is a breakpoint and not > the next instruction. > 3. "b *0x10f16bc" and "b *0x10f16c4" make GDB stop at 0x10f16bc and "ni" > stops GDB at 0x10f16bc but again it is a breakpoint. > > In 2 and 3 it is possible to continue step debugging till the next "stdu". >
On 13/06/2019 12:01, Fabiano Rosas wrote: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: > >> It is TCG, a single CPU with a single thread and no matter where I put > > Ok, but then none of this code gets executed because it is kvm-only: I was not clear, this is a generic issue, not related to your patchset, I just thought since you are looking at this already, you might find the answer faster, that's it :) > > qemu/exec.c > void cpu_single_step(CPUState *cpu, int enabled) > { > if (cpu->singlestep_enabled != enabled) { > cpu->singlestep_enabled = enabled; > if (kvm_enabled()) { > -> kvm_set_singlestep(cpu, enabled); > } else { > /* must flush all the translated code to avoid inconsistencies */ > /* XXX: only flush what is necessary */ > tb_flush(cpu); > } > } > } > >> this "stdu r1,-368(r1)" - GDB does not stop on the next one and just >> runs. >> >> In the example above: >> 1. "b *0x10f16bc" makes GDB stop there, "ni" continues without stopping >> on at 0x10f16c0. > > But this seems wrong anyway. Let me try to reproduce it and see what I > can find. Thanks! > >> 2. "b *0x10f16bc" and "b *0x10f16c0" make GDB stop at 0x10f16bc and "ni" >> steps to 0x10f16c0 but it is rather because it is a breakpoint and not >> the next instruction. >> 3. "b *0x10f16bc" and "b *0x10f16c4" make GDB stop at 0x10f16bc and "ni" >> stops GDB at 0x10f16bc but again it is a breakpoint. >> >> In 2 and 3 it is possible to continue step debugging till the next "stdu". >> >
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 26604ddf98..fb1bf1e9d7 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1171,6 +1171,11 @@ struct CPUPPCState { uint32_t tm_vscr; uint64_t tm_dscr; uint64_t tm_tar; + + /* Used for software single step */ + target_ulong sstep_msr; + target_ulong sstep_srr0; + target_ulong sstep_srr1; }; #define SET_FIT_PERIOD(a_, b_, c_, d_) \ @@ -1266,6 +1271,7 @@ struct PPCVirtualHypervisorClass { OBJECT_GET_CLASS(PPCVirtualHypervisorClass, (obj), \ TYPE_PPC_VIRTUAL_HYPERVISOR) +target_ulong ppc_get_trace_int_handler_addr(CPUState *cs); void ppc_cpu_do_interrupt(CPUState *cpu); bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req); void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf, @@ -1281,6 +1287,12 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cpu, uint8_t *buf, int reg); void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu); const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name); #endif +uint32_t ppc_gdb_read_insn(CPUState *cs, target_ulong addr); +uint32_t ppc_gdb_get_op(uint32_t insn); +uint32_t ppc_gdb_get_xop(uint32_t insn); +uint32_t ppc_gdb_get_spr(uint32_t insn); +uint32_t ppc_gdb_get_rt(uint32_t insn); + int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs, int cpuid, void *opaque); int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs, @@ -2227,6 +2239,10 @@ enum { PPC2_ISA300) }; +#define XOP_RFID 18 +#define XOP_MFMSR 83 +#define XOP_MTSPR 467 + /*****************************************************************************/ /* Memory access type : * may be needed for precise access rights control and precise exceptions. diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index beafcf1ebd..cf8c86de91 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -112,6 +112,8 @@ static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail) uint64_t offset = 0; switch (ail) { + case AIL_NONE: + break; case AIL_0001_8000: offset = 0x18000; break; @@ -768,6 +770,17 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) check_tlb_flush(env, false); } +target_ulong ppc_get_trace_int_handler_addr(CPUState *cs) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + int ail; + + ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; + return env->excp_vectors[POWERPC_EXCP_TRACE] | + ppc_excp_vector_offset(cs, ail); +} + void ppc_cpu_do_interrupt(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c index fbf3821f4b..a5b2705ade 100644 --- a/target/ppc/gdbstub.c +++ b/target/ppc/gdbstub.c @@ -380,3 +380,38 @@ const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name) return NULL; } #endif + +uint32_t ppc_gdb_read_insn(CPUState *cs, target_ulong addr) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + uint32_t insn; + + cpu_memory_rw_debug(cs, addr, (uint8_t *)&insn, sizeof(insn), 0); + + if (msr_le) { + return ldl_le_p(&insn); + } else { + return ldl_be_p(&insn); + } +} + +uint32_t ppc_gdb_get_op(uint32_t insn) +{ + return extract32(insn, 26, 6); +} + +uint32_t ppc_gdb_get_xop(uint32_t insn) +{ + return extract32(insn, 1, 10); +} + +uint32_t ppc_gdb_get_spr(uint32_t insn) +{ + return extract32(insn, 11, 5) << 5 | extract32(insn, 16, 5); +} + +uint32_t ppc_gdb_get_rt(uint32_t insn) +{ + return extract32(insn, 21, 5); +} diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 9392fba192..2bcb8814f4 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -1554,6 +1554,86 @@ void kvm_arch_remove_all_hw_breakpoints(void) nb_hw_breakpoint = nb_hw_watchpoint = 0; } +void kvm_arch_set_singlestep(CPUState *cs, int enabled) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + target_ulong trace_handler_addr; + uint32_t insn; + bool rfid; + + if (!enabled) { + return; + } + + cpu_synchronize_state(cs); + insn = ppc_gdb_read_insn(cs, env->nip); + + /* + * rfid needs special handling because it: + * - overwrites NIP with SRR0; + * - overwrites MSR with SRR1; + * - cannot be single stepped. + */ + rfid = ppc_gdb_get_op(insn) == 19 && ppc_gdb_get_xop(insn) == XOP_RFID; + + if (rfid && kvm_find_sw_breakpoint(cs, env->spr[SPR_SRR0])) { + /* + * There is a breakpoint at the next instruction address. It + * will already cause the vm exit we need for the single step, + * so there's nothing to be done. + */ + return; + } + + /* + * Save the registers that will be affected by the single step + * mechanism. These will be restored after the step at + * kvm_handle_singlestep. + */ + env->sstep_msr = env->msr; + env->sstep_srr0 = env->spr[SPR_SRR0]; + env->sstep_srr1 = env->spr[SPR_SRR1]; + + /* + * MSR_SE = 1 will cause a Trace Interrupt in the guest after the + * next instruction executes. If this is a rfid, use SRR1 instead + * of MSR. + */ + if (rfid) { + if ((env->spr[SPR_SRR1] >> MSR_SE) & 1) { + /* + * The guest is doing a single step itself. Make sure we + * restore it later. + */ + env->sstep_msr |= (1ULL << MSR_SE); + } + + env->spr[SPR_SRR1] |= (1ULL << MSR_SE); + } else { + env->msr |= (1ULL << MSR_SE); + } + + /* + * We set a breakpoint at the interrupt handler address so + * that the singlestep will be seen by KVM (this is treated by + * KVM like an ordinary breakpoint) and control is returned to + * QEMU. + */ + trace_handler_addr = ppc_get_trace_int_handler_addr(cs); + + if (env->nip == trace_handler_addr) { + /* + * We are trying to step over the interrupt handler + * address itself; move the breakpoint to the next + * instruction. + */ + trace_handler_addr += 4; + } + + kvm_insert_breakpoint(cs, trace_handler_addr, 4, GDB_BREAKPOINT_SW); +} + void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) { int n; @@ -1593,6 +1673,91 @@ void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) } } +/* Revert any side-effects caused during single step */ +static void restore_singlestep_env(CPUState *cs) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + uint32_t insn; + int reg; + int spr; + + insn = ppc_gdb_read_insn(cs, env->spr[SPR_SRR0] - 4); + + env->spr[SPR_SRR0] = env->sstep_srr0; + env->spr[SPR_SRR1] = env->sstep_srr1; + + if (ppc_gdb_get_op(insn) != 31) { + return; + } + + reg = ppc_gdb_get_rt(insn); + + switch (ppc_gdb_get_xop(insn)) { + case XOP_MTSPR: + /* + * mtspr: the guest altered the SRR, so do not use the + * pre-step value. + */ + spr = ppc_gdb_get_spr(insn); + if (spr == SPR_SRR0 || spr == SPR_SRR1) { + env->spr[spr] = env->gpr[reg]; + } + break; + case XOP_MFMSR: + /* + * mfmsr: clear MSR_SE bit to avoid the guest knowing + * that it is being single-stepped. + */ + env->gpr[reg] &= ~(1ULL << MSR_SE); + break; + } +} + +static int kvm_handle_singlestep(CPUState *cs, target_ulong address) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + target_ulong trace_handler_addr; + + if (kvm_has_guestdbg_singlestep()) { + return 1; + } + + cpu_synchronize_state(cs); + trace_handler_addr = ppc_get_trace_int_handler_addr(cs); + + if (address == trace_handler_addr) { + kvm_remove_breakpoint(cs, trace_handler_addr, 4, GDB_BREAKPOINT_SW); + + if (env->sstep_msr & (1ULL << MSR_SE)) { + /* + * The guest expects the last instruction to have caused a + * single step, go back into the interrupt handler. + */ + return 1; + } + + env->nip = env->spr[SPR_SRR0]; + /* Bits 33-36, 43-47 are set by the interrupt */ + env->msr = env->spr[SPR_SRR1] & ~(1ULL << MSR_SE | + PPC_BITMASK(33, 36) | + PPC_BITMASK(43, 47)); + restore_singlestep_env(cs); + + } else if (address == trace_handler_addr + 4) { + /* + * A step at trace_handler_addr would interfere with the + * singlestep mechanism itself, so we have previously + * displaced the breakpoint to the next instruction. + */ + kvm_remove_breakpoint(cs, trace_handler_addr + 4, 4, GDB_BREAKPOINT_SW); + restore_singlestep_env(cs); + } + + return 1; +} + static int kvm_handle_hw_breakpoint(CPUState *cs, struct kvm_debug_exit_arch *arch_info) { @@ -1620,13 +1785,29 @@ static int kvm_handle_hw_breakpoint(CPUState *cs, return handle; } -static int kvm_handle_singlestep(void) +static int kvm_handle_sw_breakpoint(CPUState *cs, target_ulong address) { - return 1; -} + target_ulong trace_handler_addr; -static int kvm_handle_sw_breakpoint(void) -{ + if (kvm_has_guestdbg_singlestep()) { + return 1; + } + + cpu_synchronize_state(cs); + trace_handler_addr = ppc_get_trace_int_handler_addr(cs); + + if (address == trace_handler_addr) { + CPU_FOREACH(cs) { + if (cs->singlestep_enabled) { + /* + * We hit this breakpoint while another cpu is doing a + * software single step. Go back into the guest to + * give chance for the single step to finish. + */ + return 0; + } + } + } return 1; } @@ -1637,7 +1818,7 @@ static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) struct kvm_debug_exit_arch *arch_info = &run->debug.arch; if (cs->singlestep_enabled) { - return kvm_handle_singlestep(); + return kvm_handle_singlestep(cs, arch_info->address); } if (arch_info->status) { @@ -1645,7 +1826,7 @@ static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) } if (kvm_find_sw_breakpoint(cs, arch_info->address)) { - return kvm_handle_sw_breakpoint(); + return kvm_handle_sw_breakpoint(cs, arch_info->address); } /*
The hardware singlestep mechanism in POWER works via a Trace Interrupt (0xd00) that happens after any instruction executes, whenever MSR_SE = 1 (PowerISA Section 6.5.15 - Trace Interrupt). However, with kvm_hv, the Trace Interrupt happens inside the guest and KVM has no visibility of it. Therefore, when the gdbstub uses the KVM_SET_GUEST_DEBUG ioctl to enable singlestep, KVM simply ignores it. This patch takes advantage of the Trace Interrupt to perform the step inside the guest, but uses a breakpoint at the Trace Interrupt handler to return control to KVM. The exit is treated by KVM as a regular breakpoint and it returns to the host (and QEMU eventually). Before signalling GDB, QEMU sets the Next Instruction Pointer to the instruction following the one being stepped and restores the MSR, SRR0, SRR1 values from before the step, effectively skipping the interrupt handler execution and hiding the trace interrupt breakpoint from GDB. This approach works with both of GDB's 'scheduler-locking' options (off, step). Note: - kvm_arch_set_singlestep happens after GDB asks for a single step, while the vcpus are stopped. - kvm_handle_singlestep executes after the step, during the handling of the Emulation Assist Interrupt (breakpoint). Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- target/ppc/cpu.h | 16 ++++ target/ppc/excp_helper.c | 13 +++ target/ppc/gdbstub.c | 35 +++++++ target/ppc/kvm.c | 195 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 252 insertions(+), 7 deletions(-)