| Message ID | 1362024796-4237-8-git-send-email-bharat.bhushan@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28.02.2013, at 05:13, Bharat Bhushan wrote: > This patch adds the debug stub support on booke/bookehv. > Now QEMU debug stub can use hw breakpoint, watchpoint and > software breakpoint to debug guest. > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > arch/powerpc/include/uapi/asm/kvm.h | 22 +++++- > arch/powerpc/kvm/booke.c | 143 +++++++++++++++++++++++++++++++--- > arch/powerpc/kvm/e500_emulate.c | 6 ++ > arch/powerpc/kvm/e500mc.c | 3 +- > 4 files changed, 155 insertions(+), 19 deletions(-) > > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h > index 15f9a00..d7ce449 100644 > --- a/arch/powerpc/include/uapi/asm/kvm.h > +++ b/arch/powerpc/include/uapi/asm/kvm.h > @@ -25,6 +25,7 @@ > /* Select powerpc specific features in <linux/kvm.h> */ > #define __KVM_HAVE_SPAPR_TCE > #define __KVM_HAVE_PPC_SMT > +#define __KVM_HAVE_GUEST_DEBUG > > struct kvm_regs { > __u64 pc; > @@ -267,7 +268,24 @@ struct kvm_fpu { > __u64 fpr[32]; > }; > > +/* > + * Defines for h/w breakpoint, watchpoint (read, write or both) and > + * software breakpoint. > + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status" > + * for KVM_DEBUG_EXIT. > + */ > +#define KVMPPC_DEBUG_NONE 0x0 > +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) > +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) > +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) > struct kvm_debug_exit_arch { > + __u64 address; > + /* > + * exiting to userspace because of h/w breakpoint, watchpoint > + * (read, write or both) and software breakpoint. > + */ > + __u32 status; > + __u32 reserved; > }; > > /* for KVM_SET_GUEST_DEBUG */ > @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { > * Type denotes h/w breakpoint, read watchpoint, write > * watchpoint or watchpoint (both read and write). > */ > -#define KVMPPC_DEBUG_NOTYPE 0x0 > -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) > -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) > -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) > __u32 type; > __u32 reserved; > } bp[16]; > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 1de93a8..21b0313 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) > #endif > } > > +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) > +{ > + /* Synchronize guest's desire to get debug interrupts into shadow MSR */ > +#ifndef CONFIG_KVM_BOOKE_HV > + vcpu->arch.shadow_msr &= ~MSR_DE; > + vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE; > +#endif > + > + /* Force enable debug interrupts when user space wants to debug */ > + if (vcpu->guest_debug) { > +#ifdef CONFIG_KVM_BOOKE_HV > + /* > + * Since there is no shadow MSR, sync MSR_DE into the guest > + * visible MSR. Do not allow guest to change MSR[DE]. > + */ > + vcpu->arch.shared->msr |= MSR_DE; > + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); > +#else > + vcpu->arch.shadow_msr |= MSR_DE; > + vcpu->arch.shared->msr &= ~MSR_DE; > +#endif > + } > +} > + > /* > * Helper function for "full" MSR writes. No need to call this if only > * EE/CE/ME/DE/RI are changing. > @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) > kvmppc_mmu_msr_notify(vcpu, old_msr); > kvmppc_vcpu_sync_spe(vcpu); > kvmppc_vcpu_sync_fpu(vcpu); > + kvmppc_vcpu_sync_debug(vcpu); > } > > static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, > @@ -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) > run->exit_reason = KVM_EXIT_DCR; > return RESUME_HOST; > > + case EMULATE_EXIT_USER: > + run->exit_reason = KVM_EXIT_DEBUG; > + run->debug.arch.address = vcpu->arch.pc; > + run->debug.arch.status = 0; > + kvmppc_account_exit(vcpu, DEBUG_EXITS); As mentioned previously, this is wrong and needs to go into the instruction emulation code for that opcode. > + return RESUME_HOST; > + > case EMULATE_FAIL: > printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n", > __func__, vcpu->arch.pc, vcpu->arch.last_inst); > @@ -751,6 +783,28 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) > } > } > > +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu) > +{ > + u32 dbsr = vcpu->arch.dbsr; > + run->debug.arch.status = 0; > + run->debug.arch.address = vcpu->arch.pc; This should go into the if(breakpoint) branch. > + > + if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) { > + run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT; > + } else { > + if (dbsr & (DBSR_DAC1W | DBSR_DAC2W)) > + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE; > + else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R)) > + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ; > + if (dbsr & (DBSR_DAC1R | DBSR_DAC1W)) > + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[0]; > + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W)) > + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[1]; > + } > + > + return RESUME_HOST; > +} > + > static void kvmppc_fill_pt_regs(struct pt_regs *regs) > { > ulong r1, ip, msr, lr; > @@ -1110,18 +1164,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > } > > case BOOKE_INTERRUPT_DEBUG: { > - u32 dbsr; > - > - vcpu->arch.pc = mfspr(SPRN_CSRR0); > - > - /* clear IAC events in DBSR register */ > - dbsr = mfspr(SPRN_DBSR); > - dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4; > - mtspr(SPRN_DBSR, dbsr); > - > - run->exit_reason = KVM_EXIT_DEBUG; > + r = kvmppc_handle_debug(run, vcpu); > + if (r == RESUME_HOST) { > + run->exit_reason = KVM_EXIT_DEBUG; > + } > kvmppc_account_exit(vcpu, DEBUG_EXITS); > - r = RESUME_HOST; > break; > } > > @@ -1172,7 +1219,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) > kvmppc_set_msr(vcpu, 0); > > #ifndef CONFIG_KVM_BOOKE_HV > - vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS; > + vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS; > vcpu->arch.shadow_pid = 1; > vcpu->arch.shared->msr = 0; > #endif > @@ -1527,10 +1574,80 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) > return r; > } > > +#define BP_NUM KVMPPC_BOOKE_IAC_NUM > +#define WP_NUM KVMPPC_BOOKE_DAC_NUM > + > int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > struct kvm_guest_debug *dbg) > { > - return -EINVAL; > + > + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) { > + /* Clear All debug events */ > + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; > + vcpu->guest_debug = 0; > + return 0; > + } > + > + vcpu->guest_debug = dbg->control; > + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; > + /* Set DBCR0_EDM in guest visible DBCR0 register. */ > + vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; > + > + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) > + vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; > + > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) { /* Code below handles only HW breakpoints */ return 0; } > + struct kvmppc_booke_debug_reg *gdbgr = > + &(vcpu->arch.shadow_dbg_reg); > + int n, b = 0, w = 0; > + const u32 bp_code[] = { > + DBCR0_IAC1 | DBCR0_IDM, > + DBCR0_IAC2 | DBCR0_IDM, > + DBCR0_IAC3 | DBCR0_IDM, > + DBCR0_IAC4 | DBCR0_IDM > + }; > + const u32 wp_code[] = { > + DBCR0_DAC1W | DBCR0_IDM, > + DBCR0_DAC2W | DBCR0_IDM, > + DBCR0_DAC1R | DBCR0_IDM, > + DBCR0_DAC2R | DBCR0_IDM > + }; > + > +#ifndef CONFIG_KVM_BOOKE_HV Please no double negation. Also, what is this about? > + gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | > + DBCR1_IAC3US | DBCR1_IAC4US; > + gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; > +#else > + gdbgr->dbcr1 = 0; > + gdbgr->dbcr2 = 0; > +#endif > + > + for (n = 0; n < (BP_NUM + WP_NUM); n++) { > + u32 type = dbg->arch.bp[n].type; > + > + if (!type) > + break; > + > + if (type & (KVMPPC_DEBUG_WATCH_READ | > + KVMPPC_DEBUG_WATCH_WRITE)) { > + if (w < WP_NUM) { > + if (type & KVMPPC_DEBUG_WATCH_READ) > + gdbgr->dbcr0 |= wp_code[w + 2]; > + if (type & KVMPPC_DEBUG_WATCH_WRITE) > + gdbgr->dbcr0 |= wp_code[w]; > + gdbgr->dac[w] = dbg->arch.bp[n].addr; > + w++; > + } > + } else if (type & KVMPPC_DEBUG_BREAKPOINT) { > + if (b < BP_NUM) { > + gdbgr->dbcr0 |= bp_code[b]; > + gdbgr->iac[b] = dbg->arch.bp[n].addr; > + b++; > + } > + } > + } > + } > + return 0; > } > > int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) > diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c > index e78f353..83ac877 100644 > --- a/arch/powerpc/kvm/e500_emulate.c > +++ b/arch/powerpc/kvm/e500_emulate.c > @@ -26,6 +26,7 @@ > #define XOP_TLBRE 946 > #define XOP_TLBWE 978 > #define XOP_TLBILX 18 > +#define XOP_EHPRIV 270 > > #ifdef CONFIG_KVM_E500MC > static int dbell2prio(ulong param) > @@ -130,6 +131,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, > emulated = kvmppc_e500_emul_tlbivax(vcpu, ea); > break; > > + case XOP_EHPRIV: > + emulated = EMULATE_EXIT_USER; > + *advance = 0; > + break; > + > default: > emulated = EMULATE_FAIL; > } > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c > index 1f89d26..f5fc6f5 100644 > --- a/arch/powerpc/kvm/e500mc.c > +++ b/arch/powerpc/kvm/e500mc.c > @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > { > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > > - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI | \ > - SPRN_EPCR_DUVD; > + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI; Doesn't this route all debug events through the host? Alex > #ifdef CONFIG_64BIT > vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM; > #endif > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, March 07, 2013 7:09 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan > Bharat-R65777 > Subject: Re: [PATCH 7/7] KVM: PPC: Add userspace debug stub support > > > On 28.02.2013, at 05:13, Bharat Bhushan wrote: > > > This patch adds the debug stub support on booke/bookehv. > > Now QEMU debug stub can use hw breakpoint, watchpoint and software > > breakpoint to debug guest. > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > arch/powerpc/include/uapi/asm/kvm.h | 22 +++++- > > arch/powerpc/kvm/booke.c | 143 +++++++++++++++++++++++++++++++--- > > arch/powerpc/kvm/e500_emulate.c | 6 ++ > > arch/powerpc/kvm/e500mc.c | 3 +- > > 4 files changed, 155 insertions(+), 19 deletions(-) > > > > diff --git a/arch/powerpc/include/uapi/asm/kvm.h > > b/arch/powerpc/include/uapi/asm/kvm.h > > index 15f9a00..d7ce449 100644 > > --- a/arch/powerpc/include/uapi/asm/kvm.h > > +++ b/arch/powerpc/include/uapi/asm/kvm.h > > @@ -25,6 +25,7 @@ > > /* Select powerpc specific features in <linux/kvm.h> */ #define > > __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT > > +#define __KVM_HAVE_GUEST_DEBUG > > > > struct kvm_regs { > > __u64 pc; > > @@ -267,7 +268,24 @@ struct kvm_fpu { > > __u64 fpr[32]; > > }; > > > > +/* > > + * Defines for h/w breakpoint, watchpoint (read, write or both) and > > + * software breakpoint. > > + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status" > > + * for KVM_DEBUG_EXIT. > > + */ > > +#define KVMPPC_DEBUG_NONE 0x0 > > +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) > > +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) > > +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) > > struct kvm_debug_exit_arch { > > + __u64 address; > > + /* > > + * exiting to userspace because of h/w breakpoint, watchpoint > > + * (read, write or both) and software breakpoint. > > + */ > > + __u32 status; > > + __u32 reserved; > > }; > > > > /* for KVM_SET_GUEST_DEBUG */ > > @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { > > * Type denotes h/w breakpoint, read watchpoint, write > > * watchpoint or watchpoint (both read and write). > > */ > > -#define KVMPPC_DEBUG_NOTYPE 0x0 > > -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) > > -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) > > -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) > > __u32 type; > > __u32 reserved; > > } bp[16]; > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index > > 1de93a8..21b0313 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu > > *vcpu) #endif } > > > > +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { > > + /* Synchronize guest's desire to get debug interrupts into shadow > > +MSR */ #ifndef CONFIG_KVM_BOOKE_HV > > + vcpu->arch.shadow_msr &= ~MSR_DE; > > + vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE; #endif > > + > > + /* Force enable debug interrupts when user space wants to debug */ > > + if (vcpu->guest_debug) { > > +#ifdef CONFIG_KVM_BOOKE_HV > > + /* > > + * Since there is no shadow MSR, sync MSR_DE into the guest > > + * visible MSR. Do not allow guest to change MSR[DE]. > > + */ > > + vcpu->arch.shared->msr |= MSR_DE; > > + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); #else > > + vcpu->arch.shadow_msr |= MSR_DE; > > + vcpu->arch.shared->msr &= ~MSR_DE; > > +#endif > > + } > > +} > > + > > /* > > * Helper function for "full" MSR writes. No need to call this if > > only > > * EE/CE/ME/DE/RI are changing. > > @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) > > kvmppc_mmu_msr_notify(vcpu, old_msr); > > kvmppc_vcpu_sync_spe(vcpu); > > kvmppc_vcpu_sync_fpu(vcpu); > > + kvmppc_vcpu_sync_debug(vcpu); > > } > > > > static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ > > -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, struct > kvm_vcpu *vcpu) > > run->exit_reason = KVM_EXIT_DCR; > > return RESUME_HOST; > > > > + case EMULATE_EXIT_USER: > > + run->exit_reason = KVM_EXIT_DEBUG; > > + run->debug.arch.address = vcpu->arch.pc; > > + run->debug.arch.status = 0; > > + kvmppc_account_exit(vcpu, DEBUG_EXITS); > > As mentioned previously, this is wrong and needs to go into the instruction > emulation code for that opcode. ok > > > + return RESUME_HOST; > > + > > case EMULATE_FAIL: > > printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n", > > __func__, vcpu->arch.pc, vcpu->arch.last_inst); @@ -751,6 > > +783,28 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu > *vcpu) > > } > > } > > > > +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu > > +*vcpu) { > > + u32 dbsr = vcpu->arch.dbsr; > > + run->debug.arch.status = 0; > > + run->debug.arch.address = vcpu->arch.pc; > > This should go into the if(breakpoint) branch. Can there be the case when do breakpoint and debug interrupt happen? > > > + > > + if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) { > > + run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT; > > + } else { > > + if (dbsr & (DBSR_DAC1W | DBSR_DAC2W)) > > + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE; > > + else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R)) > > + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ; > > + if (dbsr & (DBSR_DAC1R | DBSR_DAC1W)) > > + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[0]; > > + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W)) > > + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[1]; > > + } > > + > > + return RESUME_HOST; > > +} > > + > > static void kvmppc_fill_pt_regs(struct pt_regs *regs) { > > ulong r1, ip, msr, lr; > > @@ -1110,18 +1164,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct > kvm_vcpu *vcpu, > > } > > > > case BOOKE_INTERRUPT_DEBUG: { > > - u32 dbsr; > > - > > - vcpu->arch.pc = mfspr(SPRN_CSRR0); > > - > > - /* clear IAC events in DBSR register */ > > - dbsr = mfspr(SPRN_DBSR); > > - dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4; > > - mtspr(SPRN_DBSR, dbsr); > > - > > - run->exit_reason = KVM_EXIT_DEBUG; > > + r = kvmppc_handle_debug(run, vcpu); > > + if (r == RESUME_HOST) { > > + run->exit_reason = KVM_EXIT_DEBUG; > > + } > > kvmppc_account_exit(vcpu, DEBUG_EXITS); > > - r = RESUME_HOST; > > break; > > } > > > > @@ -1172,7 +1219,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) > > kvmppc_set_msr(vcpu, 0); > > > > #ifndef CONFIG_KVM_BOOKE_HV > > - vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS; > > + vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS; > > vcpu->arch.shadow_pid = 1; > > vcpu->arch.shared->msr = 0; > > #endif > > @@ -1527,10 +1574,80 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > > return r; > > } > > > > +#define BP_NUM KVMPPC_BOOKE_IAC_NUM > > +#define WP_NUM KVMPPC_BOOKE_DAC_NUM > > + > > int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > > struct kvm_guest_debug *dbg) > > { > > - return -EINVAL; > > + > > + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) { > > + /* Clear All debug events */ > > + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; > > + vcpu->guest_debug = 0; > > + return 0; > > + } > > + > > + vcpu->guest_debug = dbg->control; > > + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; > > + /* Set DBCR0_EDM in guest visible DBCR0 register. */ > > + vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; > > + > > + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) > > + vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; > > + > > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { > > if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) { > /* Code below handles only HW breakpoints */ > return 0; > } ok > > > + struct kvmppc_booke_debug_reg *gdbgr = > > + &(vcpu->arch.shadow_dbg_reg); > > + int n, b = 0, w = 0; > > + const u32 bp_code[] = { > > + DBCR0_IAC1 | DBCR0_IDM, > > + DBCR0_IAC2 | DBCR0_IDM, > > + DBCR0_IAC3 | DBCR0_IDM, > > + DBCR0_IAC4 | DBCR0_IDM > > + }; > > + const u32 wp_code[] = { > > + DBCR0_DAC1W | DBCR0_IDM, > > + DBCR0_DAC2W | DBCR0_IDM, > > + DBCR0_DAC1R | DBCR0_IDM, > > + DBCR0_DAC2R | DBCR0_IDM > > + }; > > + > > +#ifndef CONFIG_KVM_BOOKE_HV > > Please no double negation. You mean we should use #ifdef CONFIG_KVM_BOOKE_HV gdbgr->dbcr1 = 0; gdbgr->dbcr2 = 0; #else gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | DBCR1_IAC3US | DBCR1_IAC4US; gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; #endif > Also, what is this about? This These bits says that IAC1-4 and DAC1-2 can happen when MSR.PR is set of not. On BOOKE (e500v2); MSR.PR = 1 when guest is running. So we need to set these bits On BOOKE-HV (e500mc+); MSR.PR = 0 when guest is running. So we do not need these bits to be set. > > > + gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | > > + DBCR1_IAC3US | DBCR1_IAC4US; > > + gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; #else > > + gdbgr->dbcr1 = 0; > > + gdbgr->dbcr2 = 0; > > +#endif > > + > > + for (n = 0; n < (BP_NUM + WP_NUM); n++) { > > + u32 type = dbg->arch.bp[n].type; > > + > > + if (!type) > > + break; > > + > > + if (type & (KVMPPC_DEBUG_WATCH_READ | > > + KVMPPC_DEBUG_WATCH_WRITE)) { > > + if (w < WP_NUM) { > > + if (type & KVMPPC_DEBUG_WATCH_READ) > > + gdbgr->dbcr0 |= wp_code[w + 2]; > > + if (type & KVMPPC_DEBUG_WATCH_WRITE) > > + gdbgr->dbcr0 |= wp_code[w]; > > + gdbgr->dac[w] = dbg->arch.bp[n].addr; > > + w++; > > + } > > + } else if (type & KVMPPC_DEBUG_BREAKPOINT) { > > + if (b < BP_NUM) { > > + gdbgr->dbcr0 |= bp_code[b]; > > + gdbgr->iac[b] = dbg->arch.bp[n].addr; > > + b++; > > + } > > + } > > + } > > + } > > + return 0; > > } > > > > int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu > > *fpu) diff --git a/arch/powerpc/kvm/e500_emulate.c > > b/arch/powerpc/kvm/e500_emulate.c index e78f353..83ac877 100644 > > --- a/arch/powerpc/kvm/e500_emulate.c > > +++ b/arch/powerpc/kvm/e500_emulate.c > > @@ -26,6 +26,7 @@ > > #define XOP_TLBRE 946 > > #define XOP_TLBWE 978 > > #define XOP_TLBILX 18 > > +#define XOP_EHPRIV 270 > > > > #ifdef CONFIG_KVM_E500MC > > static int dbell2prio(ulong param) > > @@ -130,6 +131,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct > kvm_vcpu *vcpu, > > emulated = kvmppc_e500_emul_tlbivax(vcpu, ea); > > break; > > > > + case XOP_EHPRIV: > > + emulated = EMULATE_EXIT_USER; > > + *advance = 0; > > + break; > > + > > default: > > emulated = EMULATE_FAIL; > > } > > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c > > index 1f89d26..f5fc6f5 100644 > > --- a/arch/powerpc/kvm/e500mc.c > > +++ b/arch/powerpc/kvm/e500mc.c > > @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > > { > > struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > > > > - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI | \ > > - SPRN_EPCR_DUVD; > > + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI; > > Doesn't this route all debug events through the host? No; This means that debug events can occur in hypervisor state or not. EPCR.DUVD = 0 ; Debug events can occur in the hypervisor state. EPCR.DUVD = 1 ; Debug events cannot occur in the hypervisor state. So we allow debug events to occur in hypervisor state. On lightweight exit we set ECPU.DUVD (if guest using debug facility) so debug events will not come during guest entry/exit code. On guest exit we clear this bit (after restoring host state) so hypervisor can use debug features. Thanks -Bharat > > > Alex > > > #ifdef CONFIG_64BIT > > vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM; #endif > > -- > > 1.7.0.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14.03.2013, at 06:18, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, March 07, 2013 7:09 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan >> Bharat-R65777 >> Subject: Re: [PATCH 7/7] KVM: PPC: Add userspace debug stub support >> >> >> On 28.02.2013, at 05:13, Bharat Bhushan wrote: >> >>> This patch adds the debug stub support on booke/bookehv. >>> Now QEMU debug stub can use hw breakpoint, watchpoint and software >>> breakpoint to debug guest. >>> >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>> --- >>> arch/powerpc/include/uapi/asm/kvm.h | 22 +++++- >>> arch/powerpc/kvm/booke.c | 143 +++++++++++++++++++++++++++++++--- >>> arch/powerpc/kvm/e500_emulate.c | 6 ++ >>> arch/powerpc/kvm/e500mc.c | 3 +- >>> 4 files changed, 155 insertions(+), 19 deletions(-) >>> >>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h >>> b/arch/powerpc/include/uapi/asm/kvm.h >>> index 15f9a00..d7ce449 100644 >>> --- a/arch/powerpc/include/uapi/asm/kvm.h >>> +++ b/arch/powerpc/include/uapi/asm/kvm.h >>> @@ -25,6 +25,7 @@ >>> /* Select powerpc specific features in <linux/kvm.h> */ #define >>> __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT >>> +#define __KVM_HAVE_GUEST_DEBUG >>> >>> struct kvm_regs { >>> __u64 pc; >>> @@ -267,7 +268,24 @@ struct kvm_fpu { >>> __u64 fpr[32]; >>> }; >>> >>> +/* >>> + * Defines for h/w breakpoint, watchpoint (read, write or both) and >>> + * software breakpoint. >>> + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status" >>> + * for KVM_DEBUG_EXIT. >>> + */ >>> +#define KVMPPC_DEBUG_NONE 0x0 >>> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) >>> +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) >>> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) >>> struct kvm_debug_exit_arch { >>> + __u64 address; >>> + /* >>> + * exiting to userspace because of h/w breakpoint, watchpoint >>> + * (read, write or both) and software breakpoint. >>> + */ >>> + __u32 status; >>> + __u32 reserved; >>> }; >>> >>> /* for KVM_SET_GUEST_DEBUG */ >>> @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { >>> * Type denotes h/w breakpoint, read watchpoint, write >>> * watchpoint or watchpoint (both read and write). >>> */ >>> -#define KVMPPC_DEBUG_NOTYPE 0x0 >>> -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) >>> -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) >>> -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) >>> __u32 type; >>> __u32 reserved; >>> } bp[16]; >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index >>> 1de93a8..21b0313 100644 >>> --- a/arch/powerpc/kvm/booke.c >>> +++ b/arch/powerpc/kvm/booke.c >>> @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu >>> *vcpu) #endif } >>> >>> +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { >>> + /* Synchronize guest's desire to get debug interrupts into shadow >>> +MSR */ #ifndef CONFIG_KVM_BOOKE_HV >>> + vcpu->arch.shadow_msr &= ~MSR_DE; >>> + vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE; #endif >>> + >>> + /* Force enable debug interrupts when user space wants to debug */ >>> + if (vcpu->guest_debug) { >>> +#ifdef CONFIG_KVM_BOOKE_HV >>> + /* >>> + * Since there is no shadow MSR, sync MSR_DE into the guest >>> + * visible MSR. Do not allow guest to change MSR[DE]. >>> + */ >>> + vcpu->arch.shared->msr |= MSR_DE; >>> + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); #else >>> + vcpu->arch.shadow_msr |= MSR_DE; >>> + vcpu->arch.shared->msr &= ~MSR_DE; >>> +#endif >>> + } >>> +} >>> + >>> /* >>> * Helper function for "full" MSR writes. No need to call this if >>> only >>> * EE/CE/ME/DE/RI are changing. >>> @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) >>> kvmppc_mmu_msr_notify(vcpu, old_msr); >>> kvmppc_vcpu_sync_spe(vcpu); >>> kvmppc_vcpu_sync_fpu(vcpu); >>> + kvmppc_vcpu_sync_debug(vcpu); >>> } >>> >>> static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ >>> -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, struct >> kvm_vcpu *vcpu) >>> run->exit_reason = KVM_EXIT_DCR; >>> return RESUME_HOST; >>> >>> + case EMULATE_EXIT_USER: >>> + run->exit_reason = KVM_EXIT_DEBUG; >>> + run->debug.arch.address = vcpu->arch.pc; >>> + run->debug.arch.status = 0; >>> + kvmppc_account_exit(vcpu, DEBUG_EXITS); >> >> As mentioned previously, this is wrong and needs to go into the instruction >> emulation code for that opcode. > > ok > >> >>> + return RESUME_HOST; >>> + >>> case EMULATE_FAIL: >>> printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n", >>> __func__, vcpu->arch.pc, vcpu->arch.last_inst); @@ -751,6 >>> +783,28 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu >> *vcpu) >>> } >>> } >>> >>> +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu >>> +*vcpu) { >>> + u32 dbsr = vcpu->arch.dbsr; >>> + run->debug.arch.status = 0; >>> + run->debug.arch.address = vcpu->arch.pc; >> >> This should go into the if(breakpoint) branch. > > Can there be the case when do breakpoint and debug interrupt happen? At least not according to the code below :). If that's a valid case, then quite a bit of code would need to be remodeled. I'd say ignore the possibility for now. > >> >>> + >>> + if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) { >>> + run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT; >>> + } else { >>> + if (dbsr & (DBSR_DAC1W | DBSR_DAC2W)) >>> + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE; >>> + else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R)) >>> + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ; >>> + if (dbsr & (DBSR_DAC1R | DBSR_DAC1W)) >>> + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[0]; >>> + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W)) >>> + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[1]; >>> + } >>> + >>> + return RESUME_HOST; >>> +} >>> + >>> static void kvmppc_fill_pt_regs(struct pt_regs *regs) { >>> ulong r1, ip, msr, lr; >>> @@ -1110,18 +1164,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct >> kvm_vcpu *vcpu, >>> } >>> >>> case BOOKE_INTERRUPT_DEBUG: { >>> - u32 dbsr; >>> - >>> - vcpu->arch.pc = mfspr(SPRN_CSRR0); >>> - >>> - /* clear IAC events in DBSR register */ >>> - dbsr = mfspr(SPRN_DBSR); >>> - dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4; >>> - mtspr(SPRN_DBSR, dbsr); >>> - >>> - run->exit_reason = KVM_EXIT_DEBUG; >>> + r = kvmppc_handle_debug(run, vcpu); >>> + if (r == RESUME_HOST) { >>> + run->exit_reason = KVM_EXIT_DEBUG; >>> + } >>> kvmppc_account_exit(vcpu, DEBUG_EXITS); >>> - r = RESUME_HOST; >>> break; >>> } >>> >>> @@ -1172,7 +1219,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) >>> kvmppc_set_msr(vcpu, 0); >>> >>> #ifndef CONFIG_KVM_BOOKE_HV >>> - vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS; >>> + vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS; >>> vcpu->arch.shadow_pid = 1; >>> vcpu->arch.shared->msr = 0; >>> #endif >>> @@ -1527,10 +1574,80 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, >> struct kvm_one_reg *reg) >>> return r; >>> } >>> >>> +#define BP_NUM KVMPPC_BOOKE_IAC_NUM >>> +#define WP_NUM KVMPPC_BOOKE_DAC_NUM >>> + >>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, >>> struct kvm_guest_debug *dbg) >>> { >>> - return -EINVAL; >>> + >>> + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) { >>> + /* Clear All debug events */ >>> + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; >>> + vcpu->guest_debug = 0; >>> + return 0; >>> + } >>> + >>> + vcpu->guest_debug = dbg->control; >>> + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; >>> + /* Set DBCR0_EDM in guest visible DBCR0 register. */ >>> + vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; >>> + >>> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) >>> + vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; >>> + >>> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { >> >> if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) { >> /* Code below handles only HW breakpoints */ >> return 0; >> } > > ok > >> >>> + struct kvmppc_booke_debug_reg *gdbgr = >>> + &(vcpu->arch.shadow_dbg_reg); >>> + int n, b = 0, w = 0; >>> + const u32 bp_code[] = { >>> + DBCR0_IAC1 | DBCR0_IDM, >>> + DBCR0_IAC2 | DBCR0_IDM, >>> + DBCR0_IAC3 | DBCR0_IDM, >>> + DBCR0_IAC4 | DBCR0_IDM >>> + }; >>> + const u32 wp_code[] = { >>> + DBCR0_DAC1W | DBCR0_IDM, >>> + DBCR0_DAC2W | DBCR0_IDM, >>> + DBCR0_DAC1R | DBCR0_IDM, >>> + DBCR0_DAC2R | DBCR0_IDM >>> + }; >>> + >>> +#ifndef CONFIG_KVM_BOOKE_HV >> >> Please no double negation. > You mean we should use > #ifdef CONFIG_KVM_BOOKE_HV > gdbgr->dbcr1 = 0; > gdbgr->dbcr2 = 0; > #else > gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | > DBCR1_IAC3US | DBCR1_IAC4US; > gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; > #endif > >> Also, what is this about? > > This These bits says that IAC1-4 and DAC1-2 can happen when MSR.PR is set of not. > On BOOKE (e500v2); MSR.PR = 1 when guest is running. So we need to set these bits > On BOOKE-HV (e500mc+); MSR.PR = 0 when guest is running. So we do not need these bits to be set. Ah, please add a comment explaining this here. > >> >>> + gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | >>> + DBCR1_IAC3US | DBCR1_IAC4US; >>> + gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; #else >>> + gdbgr->dbcr1 = 0; >>> + gdbgr->dbcr2 = 0; >>> +#endif >>> + >>> + for (n = 0; n < (BP_NUM + WP_NUM); n++) { >>> + u32 type = dbg->arch.bp[n].type; >>> + >>> + if (!type) >>> + break; >>> + >>> + if (type & (KVMPPC_DEBUG_WATCH_READ | >>> + KVMPPC_DEBUG_WATCH_WRITE)) { >>> + if (w < WP_NUM) { >>> + if (type & KVMPPC_DEBUG_WATCH_READ) >>> + gdbgr->dbcr0 |= wp_code[w + 2]; >>> + if (type & KVMPPC_DEBUG_WATCH_WRITE) >>> + gdbgr->dbcr0 |= wp_code[w]; >>> + gdbgr->dac[w] = dbg->arch.bp[n].addr; >>> + w++; >>> + } >>> + } else if (type & KVMPPC_DEBUG_BREAKPOINT) { >>> + if (b < BP_NUM) { >>> + gdbgr->dbcr0 |= bp_code[b]; >>> + gdbgr->iac[b] = dbg->arch.bp[n].addr; >>> + b++; >>> + } >>> + } >>> + } >>> + } >>> + return 0; >>> } >>> >>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu >>> *fpu) diff --git a/arch/powerpc/kvm/e500_emulate.c >>> b/arch/powerpc/kvm/e500_emulate.c index e78f353..83ac877 100644 >>> --- a/arch/powerpc/kvm/e500_emulate.c >>> +++ b/arch/powerpc/kvm/e500_emulate.c >>> @@ -26,6 +26,7 @@ >>> #define XOP_TLBRE 946 >>> #define XOP_TLBWE 978 >>> #define XOP_TLBILX 18 >>> +#define XOP_EHPRIV 270 >>> >>> #ifdef CONFIG_KVM_E500MC >>> static int dbell2prio(ulong param) >>> @@ -130,6 +131,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct >> kvm_vcpu *vcpu, >>> emulated = kvmppc_e500_emul_tlbivax(vcpu, ea); >>> break; >>> >>> + case XOP_EHPRIV: >>> + emulated = EMULATE_EXIT_USER; >>> + *advance = 0; >>> + break; >>> + >>> default: >>> emulated = EMULATE_FAIL; >>> } >>> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c >>> index 1f89d26..f5fc6f5 100644 >>> --- a/arch/powerpc/kvm/e500mc.c >>> +++ b/arch/powerpc/kvm/e500mc.c >>> @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) >>> { >>> struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); >>> >>> - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI | \ >>> - SPRN_EPCR_DUVD; >>> + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI; >> >> Doesn't this route all debug events through the host? > > No; This means that debug events can occur in hypervisor state or not. > > EPCR.DUVD = 0 ; Debug events can occur in the hypervisor state. > > EPCR.DUVD = 1 ; Debug events cannot occur in the hypervisor state. > > So we allow debug events to occur in hypervisor state. Why do we care about debug events in our entry/exit code and didn't care about them before? If anything, this is a completely separate patch, orthogonal to this patch series, and requires a good bit of explanation. Alex > On lightweight exit we set ECPU.DUVD (if guest using debug facility) so debug events will not come during guest entry/exit code. On guest exit we clear this bit (after restoring host state) so hypervisor can use debug features. > > Thanks > -Bharat >> >> >> Alex >> >>> #ifdef CONFIG_64BIT >>> vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM; #endif >>> -- >>> 1.7.0.4 >>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On > Behalf Of Alexander Graf > Sent: Thursday, March 14, 2013 5:20 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421 > Subject: Re: [PATCH 7/7] KVM: PPC: Add userspace debug stub support > > > On 14.03.2013, at 06:18, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Thursday, March 07, 2013 7:09 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; > >> Bhushan > >> Bharat-R65777 > >> Subject: Re: [PATCH 7/7] KVM: PPC: Add userspace debug stub support > >> > >> > >> On 28.02.2013, at 05:13, Bharat Bhushan wrote: > >> > >>> This patch adds the debug stub support on booke/bookehv. > >>> Now QEMU debug stub can use hw breakpoint, watchpoint and software > >>> breakpoint to debug guest. > >>> > >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>> --- > >>> arch/powerpc/include/uapi/asm/kvm.h | 22 +++++- > >>> arch/powerpc/kvm/booke.c | 143 +++++++++++++++++++++++++++++++-- > - > >>> arch/powerpc/kvm/e500_emulate.c | 6 ++ > >>> arch/powerpc/kvm/e500mc.c | 3 +- > >>> 4 files changed, 155 insertions(+), 19 deletions(-) > >>> > >>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h > >>> b/arch/powerpc/include/uapi/asm/kvm.h > >>> index 15f9a00..d7ce449 100644 > >>> --- a/arch/powerpc/include/uapi/asm/kvm.h > >>> +++ b/arch/powerpc/include/uapi/asm/kvm.h > >>> @@ -25,6 +25,7 @@ > >>> /* Select powerpc specific features in <linux/kvm.h> */ #define > >>> __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT > >>> +#define __KVM_HAVE_GUEST_DEBUG > >>> > >>> struct kvm_regs { > >>> __u64 pc; > >>> @@ -267,7 +268,24 @@ struct kvm_fpu { > >>> __u64 fpr[32]; > >>> }; > >>> > >>> +/* > >>> + * Defines for h/w breakpoint, watchpoint (read, write or both) and > >>> + * software breakpoint. > >>> + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status" > >>> + * for KVM_DEBUG_EXIT. > >>> + */ > >>> +#define KVMPPC_DEBUG_NONE 0x0 > >>> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) > >>> +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) > >>> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) > >>> struct kvm_debug_exit_arch { > >>> + __u64 address; > >>> + /* > >>> + * exiting to userspace because of h/w breakpoint, watchpoint > >>> + * (read, write or both) and software breakpoint. > >>> + */ > >>> + __u32 status; > >>> + __u32 reserved; > >>> }; > >>> > >>> /* for KVM_SET_GUEST_DEBUG */ > >>> @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { > >>> * Type denotes h/w breakpoint, read watchpoint, write > >>> * watchpoint or watchpoint (both read and write). > >>> */ > >>> -#define KVMPPC_DEBUG_NOTYPE 0x0 > >>> -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) > >>> -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) > >>> -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) > >>> __u32 type; > >>> __u32 reserved; > >>> } bp[16]; > >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > >>> index > >>> 1de93a8..21b0313 100644 > >>> --- a/arch/powerpc/kvm/booke.c > >>> +++ b/arch/powerpc/kvm/booke.c > >>> @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct > >>> kvm_vcpu > >>> *vcpu) #endif } > >>> > >>> +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { > >>> + /* Synchronize guest's desire to get debug interrupts into shadow > >>> +MSR */ #ifndef CONFIG_KVM_BOOKE_HV > >>> + vcpu->arch.shadow_msr &= ~MSR_DE; > >>> + vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE; #endif > >>> + > >>> + /* Force enable debug interrupts when user space wants to debug */ > >>> + if (vcpu->guest_debug) { > >>> +#ifdef CONFIG_KVM_BOOKE_HV > >>> + /* > >>> + * Since there is no shadow MSR, sync MSR_DE into the guest > >>> + * visible MSR. Do not allow guest to change MSR[DE]. > >>> + */ > >>> + vcpu->arch.shared->msr |= MSR_DE; > >>> + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); #else > >>> + vcpu->arch.shadow_msr |= MSR_DE; > >>> + vcpu->arch.shared->msr &= ~MSR_DE; #endif > >>> + } > >>> +} > >>> + > >>> /* > >>> * Helper function for "full" MSR writes. No need to call this if > >>> only > >>> * EE/CE/ME/DE/RI are changing. > >>> @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) > >>> kvmppc_mmu_msr_notify(vcpu, old_msr); > >>> kvmppc_vcpu_sync_spe(vcpu); > >>> kvmppc_vcpu_sync_fpu(vcpu); > >>> + kvmppc_vcpu_sync_debug(vcpu); > >>> } > >>> > >>> static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ > >>> -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, > >>> struct > >> kvm_vcpu *vcpu) > >>> run->exit_reason = KVM_EXIT_DCR; > >>> return RESUME_HOST; > >>> > >>> + case EMULATE_EXIT_USER: > >>> + run->exit_reason = KVM_EXIT_DEBUG; > >>> + run->debug.arch.address = vcpu->arch.pc; > >>> + run->debug.arch.status = 0; > >>> + kvmppc_account_exit(vcpu, DEBUG_EXITS); > >> > >> As mentioned previously, this is wrong and needs to go into the > >> instruction emulation code for that opcode. > > > > ok > > > >> > >>> + return RESUME_HOST; > >>> + > >>> case EMULATE_FAIL: > >>> printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n", > >>> __func__, vcpu->arch.pc, vcpu->arch.last_inst); @@ -751,6 > >>> +783,28 @@ static int emulation_exit(struct kvm_run *run, struct > >>> +kvm_vcpu > >> *vcpu) > >>> } > >>> } > >>> > >>> +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu > >>> +*vcpu) { > >>> + u32 dbsr = vcpu->arch.dbsr; > >>> + run->debug.arch.status = 0; > >>> + run->debug.arch.address = vcpu->arch.pc; > >> > >> This should go into the if(breakpoint) branch. > > > > Can there be the case when do breakpoint and debug interrupt happen? > > At least not according to the code below :). If that's a valid case, then quite > a bit of code would need to be remodeled. I'd say ignore the possibility for > now. ok > > > > >> > >>> + > >>> + if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) { > >>> + run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT; > >>> + } else { > >>> + if (dbsr & (DBSR_DAC1W | DBSR_DAC2W)) > >>> + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE; > >>> + else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R)) > >>> + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ; > >>> + if (dbsr & (DBSR_DAC1R | DBSR_DAC1W)) > >>> + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[0]; > >>> + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W)) > >>> + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[1]; > >>> + } > >>> + > >>> + return RESUME_HOST; > >>> +} > >>> + > >>> static void kvmppc_fill_pt_regs(struct pt_regs *regs) { > >>> ulong r1, ip, msr, lr; > >>> @@ -1110,18 +1164,11 @@ int kvmppc_handle_exit(struct kvm_run *run, > >>> struct > >> kvm_vcpu *vcpu, > >>> } > >>> > >>> case BOOKE_INTERRUPT_DEBUG: { > >>> - u32 dbsr; > >>> - > >>> - vcpu->arch.pc = mfspr(SPRN_CSRR0); > >>> - > >>> - /* clear IAC events in DBSR register */ > >>> - dbsr = mfspr(SPRN_DBSR); > >>> - dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4; > >>> - mtspr(SPRN_DBSR, dbsr); > >>> - > >>> - run->exit_reason = KVM_EXIT_DEBUG; > >>> + r = kvmppc_handle_debug(run, vcpu); > >>> + if (r == RESUME_HOST) { > >>> + run->exit_reason = KVM_EXIT_DEBUG; > >>> + } > >>> kvmppc_account_exit(vcpu, DEBUG_EXITS); > >>> - r = RESUME_HOST; > >>> break; > >>> } > >>> > >>> @@ -1172,7 +1219,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) > >>> kvmppc_set_msr(vcpu, 0); > >>> > >>> #ifndef CONFIG_KVM_BOOKE_HV > >>> - vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS; > >>> + vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS; > >>> vcpu->arch.shadow_pid = 1; > >>> vcpu->arch.shared->msr = 0; > >>> #endif > >>> @@ -1527,10 +1574,80 @@ int kvm_vcpu_ioctl_set_one_reg(struct > >>> kvm_vcpu *vcpu, > >> struct kvm_one_reg *reg) > >>> return r; > >>> } > >>> > >>> +#define BP_NUM KVMPPC_BOOKE_IAC_NUM > >>> +#define WP_NUM KVMPPC_BOOKE_DAC_NUM > >>> + > >>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > >>> struct kvm_guest_debug *dbg) > >>> { > >>> - return -EINVAL; > >>> + > >>> + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) { > >>> + /* Clear All debug events */ > >>> + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; > >>> + vcpu->guest_debug = 0; > >>> + return 0; > >>> + } > >>> + > >>> + vcpu->guest_debug = dbg->control; > >>> + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; > >>> + /* Set DBCR0_EDM in guest visible DBCR0 register. */ > >>> + vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; > >>> + > >>> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) > >>> + vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; > >>> + > >>> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { > >> > >> if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) { > >> /* Code below handles only HW breakpoints */ > >> return 0; > >> } > > > > ok > > > >> > >>> + struct kvmppc_booke_debug_reg *gdbgr = > >>> + &(vcpu->arch.shadow_dbg_reg); > >>> + int n, b = 0, w = 0; > >>> + const u32 bp_code[] = { > >>> + DBCR0_IAC1 | DBCR0_IDM, > >>> + DBCR0_IAC2 | DBCR0_IDM, > >>> + DBCR0_IAC3 | DBCR0_IDM, > >>> + DBCR0_IAC4 | DBCR0_IDM > >>> + }; > >>> + const u32 wp_code[] = { > >>> + DBCR0_DAC1W | DBCR0_IDM, > >>> + DBCR0_DAC2W | DBCR0_IDM, > >>> + DBCR0_DAC1R | DBCR0_IDM, > >>> + DBCR0_DAC2R | DBCR0_IDM > >>> + }; > >>> + > >>> +#ifndef CONFIG_KVM_BOOKE_HV > >> > >> Please no double negation. > > You mean we should use > > #ifdef CONFIG_KVM_BOOKE_HV > > gdbgr->dbcr1 = 0; > > gdbgr->dbcr2 = 0; > > #else > > gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | > > DBCR1_IAC3US | DBCR1_IAC4US; > > gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; #endif > > > >> Also, what is this about? > > > > This These bits says that IAC1-4 and DAC1-2 can happen when MSR.PR is set of > not. > > On BOOKE (e500v2); MSR.PR = 1 when guest is running. So we need to set > > these bits On BOOKE-HV (e500mc+); MSR.PR = 0 when guest is running. So we do > not need these bits to be set. > > Ah, please add a comment explaining this here. ok > > > > >> > >>> + gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | > >>> + DBCR1_IAC3US | DBCR1_IAC4US; > >>> + gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; #else > >>> + gdbgr->dbcr1 = 0; > >>> + gdbgr->dbcr2 = 0; > >>> +#endif > >>> + > >>> + for (n = 0; n < (BP_NUM + WP_NUM); n++) { > >>> + u32 type = dbg->arch.bp[n].type; > >>> + > >>> + if (!type) > >>> + break; > >>> + > >>> + if (type & (KVMPPC_DEBUG_WATCH_READ | > >>> + KVMPPC_DEBUG_WATCH_WRITE)) { > >>> + if (w < WP_NUM) { > >>> + if (type & KVMPPC_DEBUG_WATCH_READ) > >>> + gdbgr->dbcr0 |= wp_code[w + 2]; > >>> + if (type & KVMPPC_DEBUG_WATCH_WRITE) > >>> + gdbgr->dbcr0 |= wp_code[w]; > >>> + gdbgr->dac[w] = dbg->arch.bp[n].addr; > >>> + w++; > >>> + } > >>> + } else if (type & KVMPPC_DEBUG_BREAKPOINT) { > >>> + if (b < BP_NUM) { > >>> + gdbgr->dbcr0 |= bp_code[b]; > >>> + gdbgr->iac[b] = dbg->arch.bp[n].addr; > >>> + b++; > >>> + } > >>> + } > >>> + } > >>> + } > >>> + return 0; > >>> } > >>> > >>> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct > >>> kvm_fpu > >>> *fpu) diff --git a/arch/powerpc/kvm/e500_emulate.c > >>> b/arch/powerpc/kvm/e500_emulate.c index e78f353..83ac877 100644 > >>> --- a/arch/powerpc/kvm/e500_emulate.c > >>> +++ b/arch/powerpc/kvm/e500_emulate.c > >>> @@ -26,6 +26,7 @@ > >>> #define XOP_TLBRE 946 > >>> #define XOP_TLBWE 978 > >>> #define XOP_TLBILX 18 > >>> +#define XOP_EHPRIV 270 > >>> > >>> #ifdef CONFIG_KVM_E500MC > >>> static int dbell2prio(ulong param) > >>> @@ -130,6 +131,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, > >>> struct > >> kvm_vcpu *vcpu, > >>> emulated = kvmppc_e500_emul_tlbivax(vcpu, ea); > >>> break; > >>> > >>> + case XOP_EHPRIV: > >>> + emulated = EMULATE_EXIT_USER; > >>> + *advance = 0; > >>> + break; > >>> + > >>> default: > >>> emulated = EMULATE_FAIL; > >>> } > >>> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c > >>> index 1f89d26..f5fc6f5 100644 > >>> --- a/arch/powerpc/kvm/e500mc.c > >>> +++ b/arch/powerpc/kvm/e500mc.c > >>> @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu > >>> *vcpu) { > >>> struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > >>> > >>> - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI | \ > >>> - SPRN_EPCR_DUVD; > >>> + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI; > >> > >> Doesn't this route all debug events through the host? > > > > No; This means that debug events can occur in hypervisor state or not. > > > > EPCR.DUVD = 0 ; Debug events can occur in the hypervisor state. > > > > EPCR.DUVD = 1 ; Debug events cannot occur in the hypervisor state. > > > > So we allow debug events to occur in hypervisor state. > > Why do we care about debug events in our entry/exit code and didn't care about > them before? We care for single stepping in guest to not step in KVM code. > If anything, this is a completely separate patch, orthogonal to > this patch series, and requires a good bit of explanation. Not sure why you think separate patch; this patch add support for single stepping and also takes care that debug event does not comes in host when doing single stepping. -Bharat > > > Alex > > > On lightweight exit we set ECPU.DUVD (if guest using debug facility) so debug > events will not come during guest entry/exit code. On guest exit we clear this > bit (after restoring host state) so hypervisor can use debug features. > > > > Thanks > > -Bharat > >> > >> > >> Alex > >> > >>> #ifdef CONFIG_64BIT > >>> vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM; #endif > >>> -- > >>> 1.7.0.4 > >>> > >>> > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" > >>> in the body of a message to majordomo@vger.kernel.org More majordomo > >>> info at http://vger.kernel.org/majordomo-info.html > >> > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body > of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/14/2013 08:57:53 AM, Bhushan Bharat-R65777 wrote: > > >>> diff --git a/arch/powerpc/kvm/e500mc.c > b/arch/powerpc/kvm/e500mc.c > > >>> index 1f89d26..f5fc6f5 100644 > > >>> --- a/arch/powerpc/kvm/e500mc.c > > >>> +++ b/arch/powerpc/kvm/e500mc.c > > >>> @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu > > >>> *vcpu) { > > >>> struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > > >>> > > >>> - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | > SPRN_EPCR_DGTMI | \ > > >>> - SPRN_EPCR_DUVD; > > >>> + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | > SPRN_EPCR_DGTMI; > > >> > > >> Doesn't this route all debug events through the host? > > > > > > No; This means that debug events can occur in hypervisor state or > not. > > > > > > EPCR.DUVD = 0 ; Debug events can occur in the hypervisor state. > > > > > > EPCR.DUVD = 1 ; Debug events cannot occur in the hypervisor state. > > > > > > So we allow debug events to occur in hypervisor state. > > > > Why do we care about debug events in our entry/exit code and didn't > care about > > them before? > > We care for single stepping in guest to not step in KVM code. > > > If anything, this is a completely separate patch, orthogonal to > > this patch series, and requires a good bit of explanation. > > Not sure why you think separate patch; this patch add support for > single stepping and also takes care that debug event does not comes > in host when doing single stepping. How does *removing* DUVD ensure that? -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Thursday, March 14, 2013 9:36 PM > To: Bhushan Bharat-R65777 > Cc: Alexander Graf; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott- > B07421 > Subject: Re: [PATCH 7/7] KVM: PPC: Add userspace debug stub support > > On 03/14/2013 08:57:53 AM, Bhushan Bharat-R65777 wrote: > > > >>> diff --git a/arch/powerpc/kvm/e500mc.c > > b/arch/powerpc/kvm/e500mc.c > > > >>> index 1f89d26..f5fc6f5 100644 > > > >>> --- a/arch/powerpc/kvm/e500mc.c > > > >>> +++ b/arch/powerpc/kvm/e500mc.c > > > >>> @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu > > > >>> *vcpu) { > > > >>> struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); > > > >>> > > > >>> - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | > > SPRN_EPCR_DGTMI | \ > > > >>> - SPRN_EPCR_DUVD; > > > >>> + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | > > SPRN_EPCR_DGTMI; > > > >> > > > >> Doesn't this route all debug events through the host? > > > > > > > > No; This means that debug events can occur in hypervisor state or > > not. > > > > > > > > EPCR.DUVD = 0 ; Debug events can occur in the hypervisor state. > > > > > > > > EPCR.DUVD = 1 ; Debug events cannot occur in the hypervisor state. > > > > > > > > So we allow debug events to occur in hypervisor state. > > > > > > Why do we care about debug events in our entry/exit code and didn't > > care about > > > them before? > > > > We care for single stepping in guest to not step in KVM code. > > > > > If anything, this is a completely separate patch, orthogonal to this > > > patch series, and requires a good bit of explanation. > > > > Not sure why you think separate patch; this patch add support for > > single stepping and also takes care that debug event does not comes in > > host when doing single stepping. > > How does *removing* DUVD ensure that? By default we clear DUVD, so debug events can come in hypervisor state. But on lightweight exit, when restoring guest debug context, we set DUVD so the debug interrupt will not come in hypervisor state as debug resource are taken by guest. On guest exit, when restoring the host context we clear DUVD so now debug resource are having host context. With proposed change of save and restore on vcpu_get/vcpu_put this switching witching will be done in vcpu_get/set(). Thanks -Bharat > > -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index 15f9a00..d7ce449 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -25,6 +25,7 @@ /* Select powerpc specific features in <linux/kvm.h> */ #define __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; @@ -267,7 +268,24 @@ struct kvm_fpu { __u64 fpr[32]; }; +/* + * Defines for h/w breakpoint, watchpoint (read, write or both) and + * software breakpoint. + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status" + * for KVM_DEBUG_EXIT. + */ +#define KVMPPC_DEBUG_NONE 0x0 +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) struct kvm_debug_exit_arch { + __u64 address; + /* + * exiting to userspace because of h/w breakpoint, watchpoint + * (read, write or both) and software breakpoint. + */ + __u32 status; + __u32 reserved; }; /* for KVM_SET_GUEST_DEBUG */ @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch { * Type denotes h/w breakpoint, read watchpoint, write * watchpoint or watchpoint (both read and write). */ -#define KVMPPC_DEBUG_NOTYPE 0x0 -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1) -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2) -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3) __u32 type; __u32 reserved; } bp[16]; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1de93a8..21b0313 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,30 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) +{ + /* Synchronize guest's desire to get debug interrupts into shadow MSR */ +#ifndef CONFIG_KVM_BOOKE_HV + vcpu->arch.shadow_msr &= ~MSR_DE; + vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE; +#endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu->guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* + * Since there is no shadow MSR, sync MSR_DE into the guest + * visible MSR. Do not allow guest to change MSR[DE]. + */ + vcpu->arch.shared->msr |= MSR_DE; + mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); +#else + vcpu->arch.shadow_msr |= MSR_DE; + vcpu->arch.shared->msr &= ~MSR_DE; +#endif + } +} + /* * Helper function for "full" MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +174,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) run->exit_reason = KVM_EXIT_DCR; return RESUME_HOST; + case EMULATE_EXIT_USER: + run->exit_reason = KVM_EXIT_DEBUG; + run->debug.arch.address = vcpu->arch.pc; + run->debug.arch.status = 0; + kvmppc_account_exit(vcpu, DEBUG_EXITS); + return RESUME_HOST; + case EMULATE_FAIL: printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n", __func__, vcpu->arch.pc, vcpu->arch.last_inst); @@ -751,6 +783,28 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) } } +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu) +{ + u32 dbsr = vcpu->arch.dbsr; + run->debug.arch.status = 0; + run->debug.arch.address = vcpu->arch.pc; + + if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) { + run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT; + } else { + if (dbsr & (DBSR_DAC1W | DBSR_DAC2W)) + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE; + else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R)) + run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ; + if (dbsr & (DBSR_DAC1R | DBSR_DAC1W)) + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[0]; + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W)) + run->debug.arch.address = vcpu->arch.shadow_dbg_reg.dac[1]; + } + + return RESUME_HOST; +} + static void kvmppc_fill_pt_regs(struct pt_regs *regs) { ulong r1, ip, msr, lr; @@ -1110,18 +1164,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, } case BOOKE_INTERRUPT_DEBUG: { - u32 dbsr; - - vcpu->arch.pc = mfspr(SPRN_CSRR0); - - /* clear IAC events in DBSR register */ - dbsr = mfspr(SPRN_DBSR); - dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4; - mtspr(SPRN_DBSR, dbsr); - - run->exit_reason = KVM_EXIT_DEBUG; + r = kvmppc_handle_debug(run, vcpu); + if (r == RESUME_HOST) { + run->exit_reason = KVM_EXIT_DEBUG; + } kvmppc_account_exit(vcpu, DEBUG_EXITS); - r = RESUME_HOST; break; } @@ -1172,7 +1219,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) kvmppc_set_msr(vcpu, 0); #ifndef CONFIG_KVM_BOOKE_HV - vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS; + vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS; vcpu->arch.shadow_pid = 1; vcpu->arch.shared->msr = 0; #endif @@ -1527,10 +1574,80 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) return r; } +#define BP_NUM KVMPPC_BOOKE_IAC_NUM +#define WP_NUM KVMPPC_BOOKE_DAC_NUM + int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) { - return -EINVAL; + + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) { + /* Clear All debug events */ + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; + vcpu->guest_debug = 0; + return 0; + } + + vcpu->guest_debug = dbg->control; + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; + /* Set DBCR0_EDM in guest visible DBCR0 register. */ + vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; + + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) + vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; + + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { + struct kvmppc_booke_debug_reg *gdbgr = + &(vcpu->arch.shadow_dbg_reg); + int n, b = 0, w = 0; + const u32 bp_code[] = { + DBCR0_IAC1 | DBCR0_IDM, + DBCR0_IAC2 | DBCR0_IDM, + DBCR0_IAC3 | DBCR0_IDM, + DBCR0_IAC4 | DBCR0_IDM + }; + const u32 wp_code[] = { + DBCR0_DAC1W | DBCR0_IDM, + DBCR0_DAC2W | DBCR0_IDM, + DBCR0_DAC1R | DBCR0_IDM, + DBCR0_DAC2R | DBCR0_IDM + }; + +#ifndef CONFIG_KVM_BOOKE_HV + gdbgr->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | + DBCR1_IAC3US | DBCR1_IAC4US; + gdbgr->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US; +#else + gdbgr->dbcr1 = 0; + gdbgr->dbcr2 = 0; +#endif + + for (n = 0; n < (BP_NUM + WP_NUM); n++) { + u32 type = dbg->arch.bp[n].type; + + if (!type) + break; + + if (type & (KVMPPC_DEBUG_WATCH_READ | + KVMPPC_DEBUG_WATCH_WRITE)) { + if (w < WP_NUM) { + if (type & KVMPPC_DEBUG_WATCH_READ) + gdbgr->dbcr0 |= wp_code[w + 2]; + if (type & KVMPPC_DEBUG_WATCH_WRITE) + gdbgr->dbcr0 |= wp_code[w]; + gdbgr->dac[w] = dbg->arch.bp[n].addr; + w++; + } + } else if (type & KVMPPC_DEBUG_BREAKPOINT) { + if (b < BP_NUM) { + gdbgr->dbcr0 |= bp_code[b]; + gdbgr->iac[b] = dbg->arch.bp[n].addr; + b++; + } + } + } + } + return 0; } int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu) diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c index e78f353..83ac877 100644 --- a/arch/powerpc/kvm/e500_emulate.c +++ b/arch/powerpc/kvm/e500_emulate.c @@ -26,6 +26,7 @@ #define XOP_TLBRE 946 #define XOP_TLBWE 978 #define XOP_TLBILX 18 +#define XOP_EHPRIV 270 #ifdef CONFIG_KVM_E500MC static int dbell2prio(ulong param) @@ -130,6 +131,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, emulated = kvmppc_e500_emul_tlbivax(vcpu, ea); break; + case XOP_EHPRIV: + emulated = EMULATE_EXIT_USER; + *advance = 0; + break; + default: emulated = EMULATE_FAIL; } diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 1f89d26..f5fc6f5 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -182,8 +182,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) { struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); - vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI | \ - SPRN_EPCR_DUVD; + vcpu->arch.shadow_epcr = SPRN_EPCR_DSIGS | SPRN_EPCR_DGTMI; #ifdef CONFIG_64BIT vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM; #endif
This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> --- arch/powerpc/include/uapi/asm/kvm.h | 22 +++++- arch/powerpc/kvm/booke.c | 143 +++++++++++++++++++++++++++++++--- arch/powerpc/kvm/e500_emulate.c | 6 ++ arch/powerpc/kvm/e500mc.c | 3 +- 4 files changed, 155 insertions(+), 19 deletions(-)