* Re: [Qemu-devel] [PATCH 1/3] ppc debug stub: Get trap instruction opcode from KVM
[not found] ` <1402412796-17299-2-git-send-email-Bharat.Bhushan@freescale.com>
@ 2014-06-11 12:41 ` Alexander Graf
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Graf @ 2014-06-11 12:41 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: qemu-ppc, qemu-devel
On 06/10/2014 05:06 PM, Bharat Bhushan wrote:
> Get trap instruction opcode from KVM and this opcode will
> be used for setting software breakpoint in following patch
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> target-ppc/kvm.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index dfa5a26..4fc005f 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -71,6 +71,8 @@ static int cap_papr;
> static int cap_htab_fd;
> static int cap_fixup_hcalls;
>
> +static uint32_t debug_inst_opcode;
> +
> /* XXX We have a race condition where we actually have a level triggered
> * interrupt, but the infrastructure can't expose that yet, so the guest
> * takes but ignores it, goes to sleep and never gets notified that there's
> @@ -412,6 +414,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *cenv = &cpu->env;
> + struct kvm_one_reg reg;
> int ret;
>
> /* Gather server mmu info from KVM and update the CPU state */
> @@ -434,6 +437,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
> break;
> }
>
> + reg.id = KVM_REG_PPC_DEBUG_INST,
> + reg.addr = (uintptr_t) &debug_inst_opcode,
> +
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if (ret) {
> + return ret;
> + }
How about kvm_get_one_reg()? :)
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support
[not found] ` <1402412796-17299-4-git-send-email-Bharat.Bhushan@freescale.com>
@ 2014-06-11 13:04 ` Alexander Graf
2014-06-12 7:05 ` Bharat.Bhushan
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2014-06-11 13:04 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: qemu-ppc, qemu-devel
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 <Bharat.Bhushan@freescale.com>
> ---
> 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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support
2014-06-11 13:04 ` [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support Alexander Graf
@ 2014-06-12 7:05 ` Bharat.Bhushan
2014-06-13 11:24 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Bharat.Bhushan @ 2014-06-12 7:05 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Wednesday, June 11, 2014 6:35 PM
> To: Bhushan Bharat-R65777
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH 3/3] ppc debug: Add debug stub support
>
> 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 <Bharat.Bhushan@freescale.com>
> > ---
> > 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?
You mean the code below in this function?
>
> > + if (sregs.u.e.features & KVM_SREGS_E_ED) {
>
> Hrm - we never seem to set E_ED in kvm?
Uhh, you are right. Going through the whole discussion about interrupt injection to guest I found that one patch missed for upstream.
Will send that patch
>
> > + 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.
I am trying to understand the problem here, We want to byteswap opcode only if it is mixed endian (host and guest are of different endianess) case?
> 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?
Ok will do that.
> 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?
Will add one_reg to get this information.
> 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.
You mean accessing DBSR, right ?
To handle core specific stuff, should we add some generic function like kvm_ppc_set/clear_debug_event() in hw/ppc/e500_debug.c, hw/ppc/ppc4xx_debug.c etc ?
Thanks
-Bharat
>
>
> 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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support
2014-06-12 7:05 ` Bharat.Bhushan
@ 2014-06-13 11:24 ` Alexander Graf
2014-06-16 4:27 ` Bharat.Bhushan
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2014-06-13 11:24 UTC (permalink / raw)
To: Bharat.Bhushan@freescale.com; +Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
On 12.06.14 09:05, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Wednesday, June 11, 2014 6:35 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
>> Subject: Re: [PATCH 3/3] ppc debug: Add debug stub support
>>
>> 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 <Bharat.Bhushan@freescale.com>
>>> ---
>>> 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?
> You mean the code below in this function?
Yeah :).
>
>>> + if (sregs.u.e.features & KVM_SREGS_E_ED) {
>> Hrm - we never seem to set E_ED in kvm?
> Uhh, you are right. Going through the whole discussion about interrupt injection to guest I found that one patch missed for upstream.
> Will send that patch
>
>>> + 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.
> I am trying to understand the problem here, We want to byteswap opcode only if it is mixed endian (host and guest are of different endianess) case?
Yes :).
>
>> 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?
> Ok will do that.
>
>> 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?
> Will add one_reg to get this information.
We can also fall back to always make this 2 when there's no other way to
enumerate. But setting the default to 4/2 and then revert to 2/2 hard
codedly seems odd.
Btw, where does the 4/2 come from?
>
>> 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.
> You mean accessing DBSR, right ?
> To handle core specific stuff, should we add some generic function like kvm_ppc_set/clear_debug_event() in hw/ppc/e500_debug.c, hw/ppc/ppc4xx_debug.c etc ?
I don't think this is hardware, it's part of the core. So it'd belong to
target-ppc/. Probably part of the excp helper file.
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support
2014-06-13 11:24 ` Alexander Graf
@ 2014-06-16 4:27 ` Bharat.Bhushan
2014-06-16 9:04 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Bharat.Bhushan @ 2014-06-16 4:27 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, June 13, 2014 4:55 PM
> To: Bhushan Bharat-R65777
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH 3/3] ppc debug: Add debug stub support
>
>
> On 12.06.14 09:05, Bharat.Bhushan@freescale.com wrote:
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:agraf@suse.de]
> >> Sent: Wednesday, June 11, 2014 6:35 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
> >> Subject: Re: [PATCH 3/3] ppc debug: Add debug stub support
> >>
> >> 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 <Bharat.Bhushan@freescale.com>
> >>> ---
> >>> 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?
> > You mean the code below in this function?
>
> Yeah :).
Why you think accessing sregs (cssr0/1, dsrr0/1 and ioctl) is e500 specific. Are not these valid for 4xx as well?
>
> >
> >>> + if (sregs.u.e.features & KVM_SREGS_E_ED) {
> >> Hrm - we never seem to set E_ED in kvm?
> > Uhh, you are right. Going through the whole discussion about interrupt
> injection to guest I found that one patch missed for upstream.
> > Will send that patch
> >
> >>> + 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.
> > I am trying to understand the problem here, We want to byteswap opcode only if
> it is mixed endian (host and guest are of different endianess) case?
>
> Yes :).
>
> >
> >> 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?
> > Ok will do that.
> >
> >> 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?
> > Will add one_reg to get this information.
>
> We can also fall back to always make this 2 when there's no other way to
> enumerate. But setting the default to 4/2 and then revert to 2/2 hard codedly
> seems odd.
>
> Btw, where does the 4/2 come from?
Because 44x have 4 IACs.
>
> >
> >> 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.
> > You mean accessing DBSR, right ?
> > To handle core specific stuff, should we add some generic function like
> kvm_ppc_set/clear_debug_event() in hw/ppc/e500_debug.c, hw/ppc/ppc4xx_debug.c
> etc ?
>
> I don't think this is hardware, it's part of the core. So it'd belong to target-
> ppc/. Probably part of the excp helper file.
Is not this (dbsr) is booke specific and not just e500 specific? Should this be accessed under POWERPC_MMU_BOOKE and POWERPC_MMU_BOOKE206 cpu models?
Thanks
-Bharat
>
>
> Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support
2014-06-16 4:27 ` Bharat.Bhushan
@ 2014-06-16 9:04 ` Alexander Graf
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Graf @ 2014-06-16 9:04 UTC (permalink / raw)
To: Bharat.Bhushan@freescale.com; +Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
On 16.06.14 06:27, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Friday, June 13, 2014 4:55 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
>> Subject: Re: [PATCH 3/3] ppc debug: Add debug stub support
>>
>>
>> On 12.06.14 09:05, Bharat.Bhushan@freescale.com wrote:
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Wednesday, June 11, 2014 6:35 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
>>>> Subject: Re: [PATCH 3/3] ppc debug: Add debug stub support
>>>>
>>>> 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 <Bharat.Bhushan@freescale.com>
>>>>> ---
>>>>> 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?
>>> You mean the code below in this function?
>> Yeah :).
> Why you think accessing sregs (cssr0/1, dsrr0/1 and ioctl) is e500 specific. Are not these valid for 4xx as well?
Ah, misunderstanding. I don't really care about 440 - we don't seem to
have any users there. However, I do care about book3s - and the code
isn't compatible with book3s at all ;).
>
>>>>> + if (sregs.u.e.features & KVM_SREGS_E_ED) {
>>>> Hrm - we never seem to set E_ED in kvm?
>>> Uhh, you are right. Going through the whole discussion about interrupt
>> injection to guest I found that one patch missed for upstream.
>>> Will send that patch
>>>
>>>>> + 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.
>>> I am trying to understand the problem here, We want to byteswap opcode only if
>> it is mixed endian (host and guest are of different endianess) case?
>>
>> Yes :).
>>
>>>> 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?
>>> Ok will do that.
>>>
>>>> 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?
>>> Will add one_reg to get this information.
>> We can also fall back to always make this 2 when there's no other way to
>> enumerate. But setting the default to 4/2 and then revert to 2/2 hard codedly
>> seems odd.
>>
>> Btw, where does the 4/2 come from?
> Because 44x have 4 IACs.
Ah, ok. Just ignore 440.
>
>>>> 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.
>>> You mean accessing DBSR, right ?
>>> To handle core specific stuff, should we add some generic function like
>> kvm_ppc_set/clear_debug_event() in hw/ppc/e500_debug.c, hw/ppc/ppc4xx_debug.c
>> etc ?
>>
>> I don't think this is hardware, it's part of the core. So it'd belong to target-
>> ppc/. Probably part of the excp helper file.
> Is not this (dbsr) is booke specific and not just e500 specific? Should this be accessed under POWERPC_MMU_BOOKE and POWERPC_MMU_BOOKE206 cpu models?
Again, I think we can easily ignore 440, so only care about booke206 and
book3s for now. And I'm pretty sure we can't reuse any of the code in
kvm_handle_debug() between the two targets, so you want to branch out
early and have a specific booke206 / e500 handler.
if (env->excp_model == EXCP_BOOKE206)
return kvm_handle_debug_booke206(...);
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-16 9:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1402412796-17299-1-git-send-email-Bharat.Bhushan@freescale.com>
[not found] ` <1402412796-17299-2-git-send-email-Bharat.Bhushan@freescale.com>
2014-06-11 12:41 ` [Qemu-devel] [PATCH 1/3] ppc debug stub: Get trap instruction opcode from KVM Alexander Graf
[not found] ` <1402412796-17299-4-git-send-email-Bharat.Bhushan@freescale.com>
2014-06-11 13:04 ` [Qemu-devel] [PATCH 3/3] ppc debug: Add debug stub support Alexander Graf
2014-06-12 7:05 ` Bharat.Bhushan
2014-06-13 11:24 ` Alexander Graf
2014-06-16 4:27 ` Bharat.Bhushan
2014-06-16 9:04 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).