| Message ID | 1403767388-16003-2-git-send-email-Bharat.Bhushan@freescale.com |
|---|---|
| State | New |
| Headers | show |
On 26.06.14 09:23, Bharat Bhushan wrote: > This patch adds 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 <Bharat.Bhushan@freescale.com> > --- > v4->v5 > - cleanup in kvmppc_hw_debug_points_init() > - replaced assert in kvm_arch_insert_hw_breakpoint() with return. > This allows gdb to through warning "You may have requested too many > hardware breakpoints/watchpoints.". Now user can remove extra breakpint > and continue. > > target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 223 insertions(+), 15 deletions(-) > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 3195491..3347c1b 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 > > @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu) > return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu)); > } > > +/* e500 supports 2 h/w breakpoint and 2 watchpoint. > + * book3s supports only 1 watchpoint, so array size > + * of 4 is sufficient for now. > + */ > +#define MAX_HW_BKPTS 4 > + > +static struct HWBreakpoint { > + target_ulong addr; > + int type; > +} hw_debug_points[MAX_HW_BKPTS]; > + > +static CPUWatchpoint hw_watchpoint; > + > +/* Default there is no breakpoint and watchpoint supported */ > +static int max_hw_breakpoint; > +static int max_hw_watchpoint; > +static int nb_hw_breakpoint; > +static int nb_hw_watchpoint; > + > +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv) > +{ > + if (cenv->excp_model == POWERPC_EXCP_BOOKE) { > + max_hw_breakpoint = 2; > + max_hw_watchpoint = 2; > + } > + > + if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) { > + fprintf(stderr, "Error initializing h/w breakpoints\n"); > + return; > + } > +} > + > int kvm_arch_init_vcpu(CPUState *cs) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -437,6 +470,7 @@ int kvm_arch_init_vcpu(CPUState *cs) > } > > kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode); > + kvmppc_hw_debug_points_init(cenv); > > return ret; > } > @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > return 0; > } > > +static int find_hw_breakpoint(target_ulong addr, int type) > +{ > + int n; > + > + assert((nb_hw_breakpoint + nb_hw_watchpoint) > + <= ARRAY_SIZE(hw_debug_points)); > + > + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) { > + if (hw_debug_points[n].addr == addr && hw_debug_points[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) > +{ > + if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points)) > + return -ENOBUFS; > + > + hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr; > + hw_debug_points[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_debug_points[n] = hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint]; > + > + return 0; > +} > + > +void kvm_arch_remove_all_hw_breakpoints(void) > +{ > + nb_hw_breakpoint = nb_hw_watchpoint = 0; > +} > + > void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) > { > + int n; > + > /* Software Breakpoint updates */ > if (kvm_sw_breakpoints_active(cs)) { > dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; > } > + > + assert((nb_hw_breakpoint + nb_hw_watchpoint) > + <= ARRAY_SIZE(hw_debug_points)); > + assert((nb_hw_breakpoint + nb_hw_watchpoint) <= ARRAY_SIZE(dbg->arch.bp)); > + > + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { > + 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_debug_points[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_debug_points[n].addr; > + } > + } > +} > + > +static void kvm_e500_handle_debug(CPUState *cs, int handle) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + env->spr[SPR_BOOKE_DBSR] = 0; How would KVM ever know that DBSR is now 0? And why do we need this in the first place? The guest's DBSR value doesn't get set on debug interrupts unless we call inject_debug_exception(). So there's no need to clear it either, no? Alex > } > > 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; > int handle = 0; > + int n; > + int flag = 0; > > - if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > + if (cs->singlestep_enabled) { > + handle = 1; > + } else if (arch_info->status) { > + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { > + 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_debug_points[n].addr; > + hw_watchpoint.flags = flag; > + } > + } > + } > + } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > handle = 1; > } > > + if (handle) { > + if (env->excp_model == POWERPC_EXCP_BOOKE) { > + kvm_e500_handle_debug(cs, handle); > + } > + } else { > + /* inject debug exception into guest */ > + env->pending_interrupts |= 1 << PPC_INTERRUPT_DEBUG; > + } > return handle; > } > > @@ -2103,20 +2325,6 @@ void kvm_arch_init_irq_routing(KVMState *s) > { > } > > -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) > -{ > -} > - > struct kvm_get_htab_buf { > struct kvm_get_htab_header header; > /*
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, June 26, 2014 3:52 PM > To: Bhushan Bharat-R65777 > Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; maddy@linux.vnet.ibm.com > Subject: Re: [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support > > > On 26.06.14 09:23, Bharat Bhushan wrote: > > This patch adds 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 <Bharat.Bhushan@freescale.com> > > --- > > v4->v5 > > - cleanup in kvmppc_hw_debug_points_init() > > - replaced assert in kvm_arch_insert_hw_breakpoint() with return. > > This allows gdb to through warning "You may have requested too many > > hardware breakpoints/watchpoints.". Now user can remove extra breakpint > > and continue. > > > > target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++-- > -- > > 1 file changed, 223 insertions(+), 15 deletions(-) > > > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index > > 3195491..3347c1b 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 > > > > @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu) > > return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu)); > > } > > > > +/* e500 supports 2 h/w breakpoint and 2 watchpoint. > > + * book3s supports only 1 watchpoint, so array size > > + * of 4 is sufficient for now. > > + */ > > +#define MAX_HW_BKPTS 4 > > + > > +static struct HWBreakpoint { > > + target_ulong addr; > > + int type; > > +} hw_debug_points[MAX_HW_BKPTS]; > > + > > +static CPUWatchpoint hw_watchpoint; > > + > > +/* Default there is no breakpoint and watchpoint supported */ static > > +int max_hw_breakpoint; static int max_hw_watchpoint; static int > > +nb_hw_breakpoint; static int nb_hw_watchpoint; > > + > > +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv) { > > + if (cenv->excp_model == POWERPC_EXCP_BOOKE) { > > + max_hw_breakpoint = 2; > > + max_hw_watchpoint = 2; > > + } > > + > > + if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) { > > + fprintf(stderr, "Error initializing h/w breakpoints\n"); > > + return; > > + } > > +} > > + > > int kvm_arch_init_vcpu(CPUState *cs) > > { > > PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -437,6 +470,7 @@ int > > kvm_arch_init_vcpu(CPUState *cs) > > } > > > > kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode); > > + kvmppc_hw_debug_points_init(cenv); > > > > return ret; > > } > > @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, > struct kvm_sw_breakpoint *bp) > > return 0; > > } > > > > +static int find_hw_breakpoint(target_ulong addr, int type) { > > + int n; > > + > > + assert((nb_hw_breakpoint + nb_hw_watchpoint) > > + <= ARRAY_SIZE(hw_debug_points)); > > + > > + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) { > > + if (hw_debug_points[n].addr == addr && hw_debug_points[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) { > > + if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points)) > > + return -ENOBUFS; > > + > > + hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr; > > + hw_debug_points[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_debug_points[n] = hw_debug_points[nb_hw_breakpoint + > > + nb_hw_watchpoint]; > > + > > + return 0; > > +} > > + > > +void kvm_arch_remove_all_hw_breakpoints(void) > > +{ > > + nb_hw_breakpoint = nb_hw_watchpoint = 0; } > > + > > void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) > > { > > + int n; > > + > > /* Software Breakpoint updates */ > > if (kvm_sw_breakpoints_active(cs)) { > > dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; > > } > > + > > + assert((nb_hw_breakpoint + nb_hw_watchpoint) > > + <= ARRAY_SIZE(hw_debug_points)); > > + assert((nb_hw_breakpoint + nb_hw_watchpoint) <= > > + ARRAY_SIZE(dbg->arch.bp)); > > + > > + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { > > + 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_debug_points[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_debug_points[n].addr; > > + } > > + } > > +} > > + > > +static void kvm_e500_handle_debug(CPUState *cs, int handle) { > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + CPUPPCState *env = &cpu->env; > > + > > + env->spr[SPR_BOOKE_DBSR] = 0; > > How would KVM ever know that DBSR is now 0? Yes, guest will not come to know of this "0" value. I was wrong, thinking about that this will be used in inject_debug_exception(), but inject_debug_exception() is not called in this flow. > And why do we need this in the first > place? The guest's DBSR value doesn't get set on debug interrupts unless we call > inject_debug_exception(). vcpu->arch.dbsr is set on debug exception in KVM and that is what guest sees, so we should clear DBSR. > So there's no need to clear it either, no? So I think I need a one_reg interface to set/clear DBSR. Also I do not need to explicitly set "sregs.u.e.dbsr" in inject_debug_exception(). Thanks -Bharat > > > Alex > > > } > > > > 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; > > int handle = 0; > > + int n; > > + int flag = 0; > > > > - if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > > + if (cs->singlestep_enabled) { > > + handle = 1; > > + } else if (arch_info->status) { > > + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { > > + 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_debug_points[n].addr; > > + hw_watchpoint.flags = flag; > > + } > > + } > > + } > > + } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > > handle = 1; > > } > > > > + if (handle) { > > + if (env->excp_model == POWERPC_EXCP_BOOKE) { > > + kvm_e500_handle_debug(cs, handle); > > + } > > + } else { > > + /* inject debug exception into guest */ > > + env->pending_interrupts |= 1 << PPC_INTERRUPT_DEBUG; > > + } > > return handle; > > } > > > > @@ -2103,20 +2325,6 @@ void kvm_arch_init_irq_routing(KVMState *s) > > { > > } > > > > -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) > > -{ > > -} > > - > > struct kvm_get_htab_buf { > > struct kvm_get_htab_header header; > > /*
On 27.06.14 07:51, Bharat.Bhushan@freescale.com wrote: > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, June 26, 2014 3:52 PM >> To: Bhushan Bharat-R65777 >> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; maddy@linux.vnet.ibm.com >> Subject: Re: [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support >> >> >> On 26.06.14 09:23, Bharat Bhushan wrote: >>> This patch adds 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 <Bharat.Bhushan@freescale.com> >>> --- >>> v4->v5 >>> - cleanup in kvmppc_hw_debug_points_init() >>> - replaced assert in kvm_arch_insert_hw_breakpoint() with return. >>> This allows gdb to through warning "You may have requested too many >>> hardware breakpoints/watchpoints.". Now user can remove extra breakpint >>> and continue. >>> >>> target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++-- >> -- >>> 1 file changed, 223 insertions(+), 15 deletions(-) >>> >>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index >>> 3195491..3347c1b 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 >>> >>> @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu) >>> return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu)); >>> } >>> >>> +/* e500 supports 2 h/w breakpoint and 2 watchpoint. >>> + * book3s supports only 1 watchpoint, so array size >>> + * of 4 is sufficient for now. >>> + */ >>> +#define MAX_HW_BKPTS 4 >>> + >>> +static struct HWBreakpoint { >>> + target_ulong addr; >>> + int type; >>> +} hw_debug_points[MAX_HW_BKPTS]; >>> + >>> +static CPUWatchpoint hw_watchpoint; >>> + >>> +/* Default there is no breakpoint and watchpoint supported */ static >>> +int max_hw_breakpoint; static int max_hw_watchpoint; static int >>> +nb_hw_breakpoint; static int nb_hw_watchpoint; >>> + >>> +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv) { >>> + if (cenv->excp_model == POWERPC_EXCP_BOOKE) { >>> + max_hw_breakpoint = 2; >>> + max_hw_watchpoint = 2; >>> + } >>> + >>> + if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) { >>> + fprintf(stderr, "Error initializing h/w breakpoints\n"); >>> + return; >>> + } >>> +} >>> + >>> int kvm_arch_init_vcpu(CPUState *cs) >>> { >>> PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -437,6 +470,7 @@ int >>> kvm_arch_init_vcpu(CPUState *cs) >>> } >>> >>> kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode); >>> + kvmppc_hw_debug_points_init(cenv); >>> >>> return ret; >>> } >>> @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, >> struct kvm_sw_breakpoint *bp) >>> return 0; >>> } >>> >>> +static int find_hw_breakpoint(target_ulong addr, int type) { >>> + int n; >>> + >>> + assert((nb_hw_breakpoint + nb_hw_watchpoint) >>> + <= ARRAY_SIZE(hw_debug_points)); >>> + >>> + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) { >>> + if (hw_debug_points[n].addr == addr && hw_debug_points[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) { >>> + if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points)) >>> + return -ENOBUFS; >>> + >>> + hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr; >>> + hw_debug_points[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_debug_points[n] = hw_debug_points[nb_hw_breakpoint + >>> + nb_hw_watchpoint]; >>> + >>> + return 0; >>> +} >>> + >>> +void kvm_arch_remove_all_hw_breakpoints(void) >>> +{ >>> + nb_hw_breakpoint = nb_hw_watchpoint = 0; } >>> + >>> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) >>> { >>> + int n; >>> + >>> /* Software Breakpoint updates */ >>> if (kvm_sw_breakpoints_active(cs)) { >>> dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >>> } >>> + >>> + assert((nb_hw_breakpoint + nb_hw_watchpoint) >>> + <= ARRAY_SIZE(hw_debug_points)); >>> + assert((nb_hw_breakpoint + nb_hw_watchpoint) <= >>> + ARRAY_SIZE(dbg->arch.bp)); >>> + >>> + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { >>> + 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_debug_points[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_debug_points[n].addr; >>> + } >>> + } >>> +} >>> + >>> +static void kvm_e500_handle_debug(CPUState *cs, int handle) { >>> + PowerPCCPU *cpu = POWERPC_CPU(cs); >>> + CPUPPCState *env = &cpu->env; >>> + >>> + env->spr[SPR_BOOKE_DBSR] = 0; >> How would KVM ever know that DBSR is now 0? > Yes, guest will not come to know of this "0" value. I was wrong, thinking about that this will be used in inject_debug_exception(), but inject_debug_exception() is not called in this flow. > >> And why do we need this in the first >> place? The guest's DBSR value doesn't get set on debug interrupts unless we call >> inject_debug_exception(). > vcpu->arch.dbsr is set on debug exception in KVM and that is what guest sees, so we should clear DBSR. Ah, ok. So do we actually need inject_debug_exception()? > >> So there's no need to clear it either, no? > So I think I need a one_reg interface to set/clear DBSR. The sregs interface should be good enough for now, no? Alex
> >>> + > >>> +static void kvm_e500_handle_debug(CPUState *cs, int handle) { > >>> + PowerPCCPU *cpu = POWERPC_CPU(cs); > >>> + CPUPPCState *env = &cpu->env; > >>> + > >>> + env->spr[SPR_BOOKE_DBSR] = 0; > >> How would KVM ever know that DBSR is now 0? > > Yes, guest will not come to know of this "0" value. I was wrong, thinking > about that this will be used in inject_debug_exception(), but > inject_debug_exception() is not called in this flow. > > > >> And why do we need this in the first > >> place? The guest's DBSR value doesn't get set on debug interrupts > >> unless we call inject_debug_exception(). > > vcpu->arch.dbsr is set on debug exception in KVM and that is what guest sees, > so we should clear DBSR. > > Ah, ok. So do we actually need inject_debug_exception()? If not then how we to inject interrupt to guest ? > > > > >> So there's no need to clear it either, no? > > So I think I need a one_reg interface to set/clear DBSR. > > The sregs interface should be good enough for now, no? So basically we want two things 1) Just clear DBSR in KVM when debug interrupts are handled by QEMU 2) Set DSRR0/1 or CSRR0/1 and set DBSR when interrupt are not handled by QEMU For (1); We can add a function which will use SREGS interface to clear DBSR. For (2); we will use inject_debug_exception(). Thanks -Bharat > > > Alex
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 3195491..3347c1b 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 @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu) return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu)); } +/* e500 supports 2 h/w breakpoint and 2 watchpoint. + * book3s supports only 1 watchpoint, so array size + * of 4 is sufficient for now. + */ +#define MAX_HW_BKPTS 4 + +static struct HWBreakpoint { + target_ulong addr; + int type; +} hw_debug_points[MAX_HW_BKPTS]; + +static CPUWatchpoint hw_watchpoint; + +/* Default there is no breakpoint and watchpoint supported */ +static int max_hw_breakpoint; +static int max_hw_watchpoint; +static int nb_hw_breakpoint; +static int nb_hw_watchpoint; + +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv) +{ + if (cenv->excp_model == POWERPC_EXCP_BOOKE) { + max_hw_breakpoint = 2; + max_hw_watchpoint = 2; + } + + if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) { + fprintf(stderr, "Error initializing h/w breakpoints\n"); + return; + } +} + int kvm_arch_init_vcpu(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -437,6 +470,7 @@ int kvm_arch_init_vcpu(CPUState *cs) } kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode); + kvmppc_hw_debug_points_init(cenv); return ret; } @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) return 0; } +static int find_hw_breakpoint(target_ulong addr, int type) +{ + int n; + + assert((nb_hw_breakpoint + nb_hw_watchpoint) + <= ARRAY_SIZE(hw_debug_points)); + + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) { + if (hw_debug_points[n].addr == addr && hw_debug_points[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) +{ + if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points)) + return -ENOBUFS; + + hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr; + hw_debug_points[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_debug_points[n] = hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint]; + + return 0; +} + +void kvm_arch_remove_all_hw_breakpoints(void) +{ + nb_hw_breakpoint = nb_hw_watchpoint = 0; +} + void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) { + int n; + /* Software Breakpoint updates */ if (kvm_sw_breakpoints_active(cs)) { dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; } + + assert((nb_hw_breakpoint + nb_hw_watchpoint) + <= ARRAY_SIZE(hw_debug_points)); + assert((nb_hw_breakpoint + nb_hw_watchpoint) <= ARRAY_SIZE(dbg->arch.bp)); + + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { + 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_debug_points[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_debug_points[n].addr; + } + } +} + +static void kvm_e500_handle_debug(CPUState *cs, int handle) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + + env->spr[SPR_BOOKE_DBSR] = 0; } 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; int handle = 0; + int n; + int flag = 0; - if (kvm_find_sw_breakpoint(cs, arch_info->address)) { + if (cs->singlestep_enabled) { + handle = 1; + } else if (arch_info->status) { + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { + 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_debug_points[n].addr; + hw_watchpoint.flags = flag; + } + } + } + } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) { handle = 1; } + if (handle) { + if (env->excp_model == POWERPC_EXCP_BOOKE) { + kvm_e500_handle_debug(cs, handle); + } + } else { + /* inject debug exception into guest */ + env->pending_interrupts |= 1 << PPC_INTERRUPT_DEBUG; + } return handle; } @@ -2103,20 +2325,6 @@ void kvm_arch_init_irq_routing(KVMState *s) { } -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) -{ -} - struct kvm_get_htab_buf { struct kvm_get_htab_header header; /*
This patch adds 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 <Bharat.Bhushan@freescale.com> --- v4->v5 - cleanup in kvmppc_hw_debug_points_init() - replaced assert in kvm_arch_insert_hw_breakpoint() with return. This allows gdb to through warning "You may have requested too many hardware breakpoints/watchpoints.". Now user can remove extra breakpint and continue. target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 223 insertions(+), 15 deletions(-)