Re: [Qemu-devel] [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support

[Alexander Graf]

On 24.06.14 19:59, Madhavan Srinivasan wrote:
> On Tuesday 24 June 2014 10:36 PM, address@hidden wrote:
> > > -----Original Message-----
> > > From: Madhavan Srinivasan [mailto:address@hidden
> > > Sent: Tuesday, June 24, 2014 8:59 PM
> > > To: Bhushan Bharat-R65777; address@hidden
> > > Cc: address@hidden; address@hidden
> > > Subject: Re: [PATCH 4/5 v3][RESEND] ppc: Add software breakpoint support
> > >
> > > On Tuesday 24 June 2014 05:40 PM, Bharat Bhushan wrote:
> > > > This patch allow insert/remove software breakpoint
> > > >
> > > > Signed-off-by: Bharat Bhushan <address@hidden>
> > > > ---
> > > >   target-ppc/kvm.c | 71
> > > > +++++++++++++++++++++++++++++++++++++++++++++-----------
> > > >   1 file changed, 57 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
> > > > 5238de7..8e2dbb3 100644
> > > > --- a/target-ppc/kvm.c
> > > > +++ b/target-ppc/kvm.c
> > > > @@ -1317,6 +1317,53 @@ 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) {
> > > > +    /* Mixed endian case is not handled */
> > > > +    uint32_t sc = debug_inst_opcode;
> > > > +
> > > > +    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)) {
> > > Instead of hard coding, can we use sizeof ()?
> > Yes
> >
> > > > +        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 != debug_inst_opcode ||
> > > > +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) {
> > > > +        return -EINVAL;
> > > > +    }
> > > > +
> > > Same. Can we use sizeof?
> > Yes
> >
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug
> > > > +*dbg) {
> > > > +    /* Software Breakpoint updates */
> > > > +    if (kvm_sw_breakpoints_active(cs)) {
> > > > +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
> > > > +    }
> > > > +}
> > > > +
> > > > +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) {
> > > > +    CPUState *cs = CPU(cpu);
> > > > +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> > > > +    int handle = 0;
> > > > +
> > > > +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> > > > +        handle = 1;
> > > > +    }
> > > > +
> > > > +    return handle;
> > > > +}
> > > > +
> > > >   int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)  {
> > > >       PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -1357,6 +1404,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 */
> > > Kindly can you explain when this will happen?
> > If the debug interrupt condition (breakpoint/watchpoint etc) is not set by 
> > qemu, i.e that is set by guest.
> OK. This is my understanding. Kindly correct if it is wrong.
> If we are here without any breakpoint from qemu, are we not suppose to
> pass it on to guest with an interrupt inject?

Yes. If the guest issued that instruction itself we need to pass in the 
interrupt that the guest would have received. I think in the book3s case 
this would be a PROGRAM interrupt rather than a DEBUG interrupt.


Alex