[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4 v7] ppc: Add software breakpoint support
From: |
address@hidden |
Subject: |
Re: [Qemu-devel] [PATCH 3/4 v7] ppc: Add software breakpoint support |
Date: |
Mon, 14 Jul 2014 08:55:49 +0000 |
> -----Original Message-----
> From: Madhavan Srinivasan [mailto:address@hidden
> Sent: Friday, July 11, 2014 10:51 AM
> To: Bhushan Bharat-R65777; address@hidden
> Cc: address@hidden; address@hidden
> Subject: Re: [PATCH 3/4 v7] ppc: Add software breakpoint support
>
> On Thursday 10 July 2014 07:49 PM, Bharat Bhushan wrote:
> > This patch allow insert/remove software breakpoint.
> >
> > When QEMU is not able to handle debug exception then we inject program
> > exception to guest because for software breakpoint QEMU uses a
> > ehpriv-1 instruction; So there cannot be any reason that we are in
> > qemu with exit reason KVM_EXIT_DEBUG for guest set debug exception,
> > only possibility is guest executed ehpriv-1 privilege instruction and
> > that's why we are injecting program exception.
> >
> > Signed-off-by: Bharat Bhushan <address@hidden>
> > ---
> > v6->v7
> > - Moved exception injection to this patch
> > - Inject the fault directly
> >
> > target-ppc/kvm.c | 87
> > +++++++++++++++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 73 insertions(+), 14 deletions(-)
> >
> > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
> > e00a20f..afa2291 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -1275,6 +1275,69 @@ 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,
> > + sizeof(sc), 0) ||
> > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 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, sizeof(sc), 0) ||
> > + sc != debug_inst_opcode ||
> > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> > + sizeof(sc), 1)) {
> > + return -EINVAL;
> > + }
> > +
> > + 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);
> > + CPUPPCState *env = &cpu->env;
> > + struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> > + int handle = 0;
> > +
> > + if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> > + handle = 1;
> > + } else {
> > + /* QEMU is not able to handle debug exception, so inject
> > + * program exception to guest;
> > + * Yes program exception NOT debug exception !!
> > + * For software breakpoint QEMU uses a ehpriv-1 instruction;
> > + * So there cannot be any reason that we are here for guest
> > + * set debug exception, only possibility is guest executed a
> > + * privilege instruction and that's why we are injecting
> > + * program exception.
> > + */
> > + cs->exception_index = POWERPC_EXCP_PROGRAM;
> > + env->error_code = POWERPC_EXCP_INVAL;
> > + ppc_cpu_do_interrupt(cs);
> > + }
> > +
>
> Excellent. This is the change I had as part of server side patch for no sw
> breakpoint case. Also have one more addition to this, which I found in the
> debug.
>
> Only issue in here (using TCG for injecting) is that, KVM gives us PC, but
> incase of TCG, it uses nip. So nip gets decremented in ppc_cpu_do_interrupt
> function ending up sending the wrong pc to guest.
This is a good catch, I did not hit this because of some other issue (srr0/1
not getting sync properly on Booke-hv.
Thanks
-Bharat
> So Alex suggested to increment the nip by 4 before calling the
> ppc_cpu_do_interrupt function. Also kindly add cpu_synchronize_state before
> calling since we are changing the register values.
>
> Regards
> Maddy
>
> > + return handle;
> > +}
> > +
> > int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) {
> > PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -1315,6 +1378,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;
> > @@ -2003,16 +2076,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;
> > @@ -2027,10 +2090,6 @@ 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;
> > /*
> >