From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WuiD5-0008MS-PU for qemu-devel@nongnu.org; Wed, 11 Jun 2014 09:05:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WuiCz-0005lj-FY for qemu-devel@nongnu.org; Wed, 11 Jun 2014 09:04:59 -0400 Message-ID: <539853F4.2030001@suse.de> Date: Wed, 11 Jun 2014 15:04:52 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1402412796-17299-1-git-send-email-Bharat.Bhushan@freescale.com> <1402412796-17299-4-git-send-email-Bharat.Bhushan@freescale.com> In-Reply-To: <1402412796-17299-4-git-send-email-Bharat.Bhushan@freescale.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharat Bhushan Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 06/10/2014 05:06 PM, Bharat Bhushan wrote: > This patch adds software breakpoint, hardware breakpoint and > hardware watchpoint support for ppc. If the debug interrupt is > not handled then this is injected to guest. > > Signed-off-by: Bharat Bhushan > --- > hw/ppc/e500.c | 1 + > target-ppc/kvm.c | 304 ++++++++++++++++++++++++++++++++++++++++++++++----- > target-ppc/kvm_ppc.h | 1 + > 3 files changed, 278 insertions(+), 28 deletions(-) > > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index a973c18..514c595 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -853,6 +853,7 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) > if (kvm_enabled()) { > kvmppc_init(); > } > + kvmppc_e500_hw_breakpoint_init(); > } > > static int e500_ccsr_initfn(SysBusDevice *dev) > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 1d2384d..f5fbec6 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -38,6 +38,7 @@ > #include "hw/ppc/ppc.h" > #include "sysemu/watchdog.h" > #include "trace.h" > +#include "exec/gdbstub.h" > > //#define DEBUG_KVM > > @@ -768,6 +769,38 @@ static int kvm_put_vpa(CPUState *cs) > > static int kvmppc_inject_debug_exception(CPUState *cs) > { > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + struct kvm_sregs sregs; > + int ret; > + > + if (!cap_booke_sregs) { > + return -1; > + } > + > + ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs); > + if (ret < 0) { > + return -1; > + } > + I don't think any of this code should ever run for non-e500, no? > + if (sregs.u.e.features & KVM_SREGS_E_ED) { Hrm - we never seem to set E_ED in kvm? > + sregs.u.e.dsrr0 = env->nip; > + sregs.u.e.dsrr1 = env->msr; > + } else { > + sregs.u.e.csrr0 = env->nip; > + sregs.u.e.csrr1 = env->msr; > + } > + > + sregs.u.e.update_special = KVM_SREGS_E_UPDATE_DBSR; > + sregs.u.e.dbsr = env->spr[SPR_BOOKE_DBSR]; > + > + ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs); > + if (ret < 0) { > + return -1; > + } > + > + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DEBUG); > + > return 0; > } > > @@ -1275,6 +1308,239 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat > return 0; > } > > +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > +{ > + uint32_t sc = tswap32(debug_inst_opcode); Heh - this will become a lot of fun for real LE host as well as guest systems. For now just remove the tswap and add a comment that this needs fixing for LE. > + > + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 1)) { > + return -EINVAL; > + } > + > + return 0; > +} > + > +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > +{ > + uint32_t sc; > + > + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, 4, 0) || > + sc != tswap32(debug_inst_opcode) || Same here. In fact, neither of the 2 operations are in a fast path. Can't we just fetch the debug inst opcode on demand in a function here? That will allow for easier byte swapping depending on the guest's MSR.LE setting later as well. > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) { > + return -EINVAL; > + } > + > + return 0; > +} > + > +static struct HWBreakpoint { > + target_ulong addr; > + int type; > +} hw_breakpoint[6]; > + > +static int nb_hw_breakpoint; > +static int nb_hw_watchpoint; > +static int max_hw_breakpoint = 4; > +static int max_hw_watchpoint = 2; > + > +void kvmppc_e500_hw_breakpoint_init(void) > +{ > + max_hw_breakpoint = 2; > + max_hw_watchpoint = 2; Can we somehow get this information from kvm and set it in kvm_arch_init? Worst case we'll have to look at the cpu class and derive it from there, but it really should live solely inside the kvm file. > +} > + > +static int find_hw_breakpoint(target_ulong addr, int type) > +{ > + int n; > + > + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) { > + if (hw_breakpoint[n].addr == addr && hw_breakpoint[n].type == type) { > + return n; > + } > + } > + > + return -1; > +} > + > +static int find_hw_watchpoint(target_ulong addr, int *flag) > +{ > + int n; > + > + n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS); > + if (n >= 0) { > + *flag = BP_MEM_ACCESS; > + return n; > + } > + > + n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE); > + if (n >= 0) { > + *flag = BP_MEM_WRITE; > + return n; > + } > + > + n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ); > + if (n >= 0) { > + *flag = BP_MEM_READ; > + return n; > + } > + > + return -1; > +} > + > +int kvm_arch_insert_hw_breakpoint(target_ulong addr, > + target_ulong len, int type) > +{ > + hw_breakpoint[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr; > + hw_breakpoint[nb_hw_breakpoint + nb_hw_watchpoint].type = type; > + > + switch (type) { > + case GDB_BREAKPOINT_HW: > + if (nb_hw_breakpoint >= max_hw_breakpoint) { > + return -ENOBUFS; > + } > + > + if (find_hw_breakpoint(addr, type) >= 0) { > + return -EEXIST; > + } > + > + nb_hw_breakpoint++; > + break; > + > + case GDB_WATCHPOINT_WRITE: > + case GDB_WATCHPOINT_READ: > + case GDB_WATCHPOINT_ACCESS: > + if (nb_hw_watchpoint >= max_hw_watchpoint) { > + return -ENOBUFS; > + } > + > + if (find_hw_breakpoint(addr, type) >= 0) { > + return -EEXIST; > + } > + > + nb_hw_watchpoint++; > + break; > + > + default: > + return -ENOSYS; > + } > + > + return 0; > +} > + > +int kvm_arch_remove_hw_breakpoint(target_ulong addr, > + target_ulong len, int type) > +{ > + int n; > + > + n = find_hw_breakpoint(addr, type); > + if (n < 0) { > + return -ENOENT; > + } > + > + switch (type) { > + case GDB_BREAKPOINT_HW: > + nb_hw_breakpoint--; > + break; > + > + case GDB_WATCHPOINT_WRITE: > + case GDB_WATCHPOINT_READ: > + case GDB_WATCHPOINT_ACCESS: > + nb_hw_watchpoint--; > + break; > + > + default: > + return -ENOSYS; > + } > + hw_breakpoint[n] = hw_breakpoint[nb_hw_breakpoint + nb_hw_watchpoint]; > + > + return 0; > +} > + > +void kvm_arch_remove_all_hw_breakpoints(void) > +{ > + nb_hw_breakpoint = nb_hw_watchpoint = 0; > +} > + > +static CPUWatchpoint hw_watchpoint; > + > + > +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) > +{ > + CPUState *cs = CPU(cpu); > + CPUPPCState *env = &cpu->env; > + int handle = 0; > + int n; > + int flag = 0; > + struct kvm_debug_exit_arch *arch_info = &run->debug.arch; > + > + if (cs->singlestep_enabled) { > + handle = 1; > + } else if (arch_info->status) { > + if (arch_info->status & KVMPPC_DEBUG_BREAKPOINT) { > + n = find_hw_breakpoint(arch_info->address, GDB_BREAKPOINT_HW); > + if (n >= 0) { > + handle = 1; > + } > + } else if (arch_info->status & (KVMPPC_DEBUG_WATCH_READ | > + KVMPPC_DEBUG_WATCH_WRITE)) { > + n = find_hw_watchpoint(arch_info->address, &flag); > + if (n >= 0) { > + handle = 1; > + cs->watchpoint_hit = &hw_watchpoint; > + hw_watchpoint.vaddr = hw_breakpoint[n].addr; > + hw_watchpoint.flags = flag; > + } > + } > + } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > + handle = 1; > + } > + > + cpu_synchronize_state(cs); > + if (handle) { > + env->spr[SPR_BOOKE_DBSR] = 0; This is pretty e500 specific. Alex > + } else { > + printf("unhandled\n"); > + /* inject guest debug exception */ > + env->pending_interrupts |= 1 << PPC_INTERRUPT_DEBUG; > + } > + > + return handle; > +} > + > +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) > +{ > + if (kvm_sw_breakpoints_active(cs)) { > + dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; > + } > + > + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { > + int n; > + > + dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP; > + memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp)); > + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) { > + switch (hw_breakpoint[n].type) { > + case GDB_BREAKPOINT_HW: > + dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT; > + break; > + case GDB_WATCHPOINT_WRITE: > + dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE; > + break; > + case GDB_WATCHPOINT_READ: > + dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ; > + break; > + case GDB_WATCHPOINT_ACCESS: > + dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE | > + KVMPPC_DEBUG_WATCH_READ; > + break; > + default: > + cpu_abort(cs, "Unsupported breakpoint type\n"); > + } > + dbg->arch.bp[n].addr = hw_breakpoint[n].addr; > + } > + } > +} > + > int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -1315,6 +1581,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) > ret = 0; > break; > > + case KVM_EXIT_DEBUG: > + DPRINTF("handle debug exception\n"); > + if (kvm_handle_debug(cpu, run)) { > + ret = EXCP_DEBUG; > + break; > + } > + /* re-enter, this exception was guest-internal */ > + ret = 0; > + break; > + > default: > fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); > ret = -1; > @@ -2002,34 +2278,6 @@ void kvm_arch_init_irq_routing(KVMState *s) > { > } > > -int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp) > -{ > - return -EINVAL; > -} > - > -int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp) > -{ > - return -EINVAL; > -} > - > -int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type) > -{ > - return -EINVAL; > -} > - > -int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong len, int type) > -{ > - return -EINVAL; > -} > - > -void kvm_arch_remove_all_hw_breakpoints(void) > -{ > -} > - > -void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg) > -{ > -} > - > struct kvm_get_htab_buf { > struct kvm_get_htab_header header; > /* > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > index 412cc7f..89244e5 100644 > --- a/target-ppc/kvm_ppc.h > +++ b/target-ppc/kvm_ppc.h > @@ -30,6 +30,7 @@ int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits); > int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits); > int kvmppc_set_tcr(PowerPCCPU *cpu); > int kvmppc_booke_watchdog_enable(PowerPCCPU *cpu); > +void kvmppc_e500_hw_breakpoint_init(void); > #ifndef CONFIG_USER_ONLY > off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem); > bool kvmppc_spapr_use_multitce(void);