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