From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45554) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X5COj-0006Is-Hs for qemu-devel@nongnu.org; Thu, 10 Jul 2014 07:20:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X5COc-0002pU-Ns for qemu-devel@nongnu.org; Thu, 10 Jul 2014 07:20:21 -0400 Message-ID: <53BE76ED.2050109@suse.de> Date: Thu, 10 Jul 2014 13:20:13 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1404989882-17362-1-git-send-email-Bharat.Bhushan@freescale.com> <1404989882-17362-7-git-send-email-Bharat.Bhushan@freescale.com> In-Reply-To: <1404989882-17362-7-git-send-email-Bharat.Bhushan@freescale.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/6 v6] ppc: Add hw breakpoint watchpoint support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharat Bhushan Cc: maddy@linux.vnet.ibm.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 10.07.14 12:58, Bharat Bhushan wrote: > This patch adds hardware breakpoint and hardware watchpoint support > for ppc. > > On BOOKE architecture we cannot share debug resources between QEMU > and guest because: > When QEMU is using debug resources then debug exception must > be always enabled. To achieve this we set MSR_DE and also set > MSRP_DEP so guest cannot change MSR_DE. > > When emulating debug resource for guest we want guest > to control MSR_DE (enable/disable debug interrupt on need). > > So above mentioned two configuration cannot be supported > at the same time. So the result is that we cannot share > debug resources between QEMU and Guest on BOOKE architecture. > > In the current design QEMU gets priority over guest, > this means that if QEMU is using debug resources then guest > cannot use them and if guest is using debug resource then > qemu can overwrite them. > > When QEMU is not able to handle debug exception then we inject program > exception to guest. Yes program exception NOT debug exception and the > reason is: > 1) QEMU and guest not sharing debug resources > 2) 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 > --- > v5->v6 > - Inject program exception rather than debug exception if > guest is not able to handle debug exception. why? detail > in respective patch. > > target-ppc/kvm.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 229 insertions(+), 16 deletions(-) > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index c4a1fa5..6156f09 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 > > @@ -412,6 +413,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); > @@ -439,6 +472,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; > } > @@ -1348,24 +1382,217 @@ 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 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; > + } else { > + /* QEMU is not able to handle debug exception, so inject > + * program exception to guest; > + * Yes program exception NOT debug exception !! > + * When QEMU is using debug resources then debug exception must > + * be always set. To achieve this we set MSR_DE and also set > + * MSRP_DEP so guest cannot change MSR_DE. > + * When emulating debug resource for guest we want guest > + * to control MSR_DE (enable/disable debug interrupt on need). > + * Supporting both configurations are NOT possible. > + * So the result is that we cannot share debug resources > + * between QEMU and Guest on BOOKE architecture. > + * In the current design QEMU gets the priority over guest, > + * this means that if QEMU is using debug resources then guest > + * cannot use them; > + * 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. > + */ > + env->pending_interrupts |= 1 << PPC_INTERRUPT_DEBUG; This should be part of patch 5/6. Also you don't have to mess with env->pending_interrupts at all here - just inject the fault straight away. Alex