> -----Original Message----- > From: Madhavan Srinivasan [mailto:ma...@linux.vnet.ibm.com] > Sent: Friday, July 11, 2014 10:51 AM > To: Bhushan Bharat-R65777; ag...@suse.de > Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org > 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 <bharat.bhus...@freescale.com> > > --- > > 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; > > /* > >