| Message ID | 1343280734-3359-2-git-send-email-Bharat.Bhushan@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On 07/26/2012 12:32 AM, Bharat Bhushan wrote: > This patch adds: > 1) KVM debug handler added for e500v2. > 2) Guest debug by qemu gdb stub. Does it make sense for these to both be in the same patch? If there's common code used by both, that could be added first. > Signed-off-by: Liu Yu <yu.liu@freescale.com> > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > [bharat.bhushan@freescale.com: Substantial changes] > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > arch/powerpc/include/asm/kvm.h | 21 +++++ > arch/powerpc/include/asm/kvm_host.h | 7 ++ > arch/powerpc/include/asm/kvm_ppc.h | 2 + > arch/powerpc/include/asm/reg_booke.h | 1 + > arch/powerpc/kernel/asm-offsets.c | 31 ++++++- > arch/powerpc/kvm/booke.c | 146 +++++++++++++++++++++++++++--- > arch/powerpc/kvm/booke_interrupts.S | 160 ++++++++++++++++++++++++++++++++- > arch/powerpc/kvm/bookehv_interrupts.S | 141 ++++++++++++++++++++++++++++- > arch/powerpc/kvm/e500mc.c | 3 +- > arch/powerpc/kvm/powerpc.c | 2 +- > 10 files changed, 492 insertions(+), 22 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h > index 3c14202..da71c84 100644 > --- a/arch/powerpc/include/asm/kvm.h > +++ b/arch/powerpc/include/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; > @@ -265,10 +266,19 @@ struct kvm_fpu { > }; > > struct kvm_debug_exit_arch { > + __u32 exception; > + __u32 pc; > + __u32 status; > }; PC must be 64-bit. What goes in "status" and "exception"? > /* for KVM_SET_GUEST_DEBUG */ > struct kvm_guest_debug_arch { > + struct { > + __u64 addr; > + __u32 type; > + __u32 pad1; > + __u64 pad2; > + } bp[16]; > }; What goes in "type"? > /* definition of registers in kvm_run */ > @@ -285,6 +295,17 @@ struct kvm_sync_regs { > #define KVM_CPU_3S_64 4 > #define KVM_CPU_E500MC 5 > > +/* Debug related defines */ > +#define KVM_INST_GUESTGDB 0x7C00021C /* ehpriv OC=0 */ Will this work on all PPC? It certainly won't work on other architectures, so at a minimum it's KVM_PPC_INST_GUEST_GDB, but maybe it needs to be determined at runtime. > +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 > +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 Where do these get used? Any reason for these particular values? If you're trying to create a partition where the upper half is generic and the lower half is arch-specific, say so. > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 7a45194..524af7a 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -458,7 +458,12 @@ struct kvm_vcpu_arch { > u32 ccr0; > u32 ccr1; > u32 dbsr; > + /* guest debug regiters*/ > struct kvmppc_booke_debug_reg dbg_reg; > + /* shadow debug registers */ > + struct kvmppc_booke_debug_reg shadow_dbg_reg; > + /* host debug regiters*/ > + struct kvmppc_booke_debug_reg host_dbg_reg; s/regiter/register/g ...and put a space before */ > @@ -492,6 +497,7 @@ struct kvm_vcpu_arch { > u32 tlbcfg[4]; > u32 mmucfg; > u32 epr; > + u32 crit_save; > #endif What is crit_save? > gpa_t paddr_accessed; > gva_t vaddr_accessed; > @@ -533,6 +539,7 @@ struct kvm_vcpu_arch { > struct kvm_vcpu_arch_shared *shared; > unsigned long magic_page_pa; /* phys addr to map the magic page to */ > unsigned long magic_page_ea; /* effect. addr to map the magic page to */ > + struct kvm_guest_debug_arch dbg; /* debug arg between kvm and qemu */ Is kvm_guest_debug_arch generic or PPC-specific? If the former, why is it in a ppc struct? If the latter, why doesn't it have "ppc" in the name? Please separate out generic things in one patch, then PPC-wide things, then booke things (but keep things bisectable by adding stubs along the way if necessary). > -#ifdef CONFIG_KVM_BOOKE_HV > +#if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC) > DEFINE(THREAD_KVM_VCPU, offsetof(struct thread_struct, kvm_vcpu)); > #endif Why not all booke? > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 6fbdcfc..784a6bf 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -130,6 +130,9 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) > > #ifdef CONFIG_KVM_BOOKE_HV > new_msr |= MSR_GS; > + > + if (vcpu->guest_debug) > + new_msr |= MSR_DE; > #endif > > vcpu->arch.shared->msr = new_msr; > @@ -684,10 +687,21 @@ out: > return ret; > } > > -static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) > +static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, > + int exit_nr) > { > enum emulation_result er; > > + if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) && > + (vcpu->arch.last_inst == KVM_INST_GUESTGDB)) { Unnecessary parens. > + run->exit_reason = KVM_EXIT_DEBUG; > + run->debug.arch.pc = vcpu->arch.pc; > + run->debug.arch.exception = exit_nr; > + run->debug.arch.status = 0; > + kvmppc_account_exit(vcpu, DEBUG_EXITS); > + return RESUME_HOST; The interface isn't (clearly labelled as) booke specific, but you return booke-specific exception numbers. How's userspace supposed to know what to do with them? What do you plan on doing with them in QEMU? > +#define GET_VCPU_POINT(regd) \ > + mfspr regd, SPRN_SPRG_THREAD; \ > + lwz regd, THREAD_KVM_VCPU(regd) "Point" is not an idiomatic abbreviation for pointer. Does this really need to be macroized, which prevents optimization? IIRC, the 64-bit patchset gets rid of that on bookehv (where it was called GET_VCPU). > _GLOBAL(kvmppc_resume_host) > + /* > + * If guest not used debug facility then hw debug registers > + * already have proper host values. If guest used debug > + * facility then restore host debug registers. > + * No Need to save guest debug registers as they are already intact > + * in guest/shadow registers. > + */ > + lwz r9, VCPU_SHADOW_DBG(r4) > + rlwinm. r8, r9, 0, ~DBCR0_IDM > + beq skip_load_host_debug > + lwz r3, VCPU_HOST_DBG(r4) > + andis. r9, r9, DBCR0_AC_BITS@h > + li r9, 0 > + mtspr SPRN_DBCR0, r9 /* disable all debug event */ > + beq ..skip_load_hw_bkpts We don't currently have that weird leading ".." in the bookehv code -- please don't introduce it. > +#ifndef CONFIG_PPC_FSL_BOOK3E > + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > + mtspr SPRN_IAC3, r7 > + mtspr SPRN_IAC4, r8 > +#endif Can you handle this at runtime with a feature section? > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 685829a..38b5d02 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -427,7 +427,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) > int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > struct kvm_guest_debug *dbg) > { > - return -EINVAL; > + return kvmppc_core_set_guest_debug(vcpu, dbg); > } I don't see a stub implementation for non-booke. -Scott -- 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
On 07/30/2012 02:37 AM, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Wood Scott-B07421 >> Sent: Friday, July 27, 2012 7:00 AM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Bhushan Bharat- >> R65777 >> Subject: Re: [PATCH 2/2] KVM: PPC: booke/bookehv: Add guest debug support >> >> On 07/26/2012 12:32 AM, Bharat Bhushan wrote: >>> This patch adds: >>> 1) KVM debug handler added for e500v2. >>> 2) Guest debug by qemu gdb stub. >> >> Does it make sense for these to both be in the same patch? If there's common >> code used by both, that could be added first. > > ok > >> >>> Signed-off-by: Liu Yu <yu.liu@freescale.com> >>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> >>> [bharat.bhushan@freescale.com: Substantial changes] >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>> --- >>> arch/powerpc/include/asm/kvm.h | 21 +++++ >>> arch/powerpc/include/asm/kvm_host.h | 7 ++ >>> arch/powerpc/include/asm/kvm_ppc.h | 2 + >>> arch/powerpc/include/asm/reg_booke.h | 1 + >>> arch/powerpc/kernel/asm-offsets.c | 31 ++++++- >>> arch/powerpc/kvm/booke.c | 146 +++++++++++++++++++++++++++--- >>> arch/powerpc/kvm/booke_interrupts.S | 160 >> ++++++++++++++++++++++++++++++++- >>> arch/powerpc/kvm/bookehv_interrupts.S | 141 ++++++++++++++++++++++++++++- >>> arch/powerpc/kvm/e500mc.c | 3 +- >>> arch/powerpc/kvm/powerpc.c | 2 +- >>> 10 files changed, 492 insertions(+), 22 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/kvm.h >>> b/arch/powerpc/include/asm/kvm.h index 3c14202..da71c84 100644 >>> --- a/arch/powerpc/include/asm/kvm.h >>> +++ b/arch/powerpc/include/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; >>> @@ -265,10 +266,19 @@ struct kvm_fpu { }; >>> >>> struct kvm_debug_exit_arch { >>> + __u32 exception; >>> + __u32 pc; >>> + __u32 status; >>> }; >> >> PC must be 64-bit. What goes in "status" and "exception"? > > ok > >> >>> /* for KVM_SET_GUEST_DEBUG */ >>> struct kvm_guest_debug_arch { >>> + struct { >>> + __u64 addr; >>> + __u32 type; >>> + __u32 pad1; >>> + __u64 pad2; >>> + } bp[16]; >>> }; >> >> What goes in "type"? > > Type denote breakpoint, read watchpoint, write watchpoint or watchpoint (both read and write). Will adding a comment to describe this is ok? Yes, please make sure all of this is well documented. >>> /* definition of registers in kvm_run */ @@ -285,6 +295,17 @@ struct >>> kvm_sync_regs { >>> #define KVM_CPU_3S_64 4 >>> #define KVM_CPU_E500MC 5 >>> >>> +/* Debug related defines */ >>> +#define KVM_INST_GUESTGDB 0x7C00021C /* ehpriv OC=0 */ >> >> Will this work on all PPC? >> >> It certainly won't work on other architectures, so at a minimum it's >> KVM_PPC_INST_GUEST_GDB, but maybe it needs to be determined at runtime. > > How to determine at run time? adding another ioctl ? Or extend an existing one. Is there any other information about debug capabilities that you expose -- number of hardware breakpoints supported, etc? >>> +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 >>> +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 >> >> Where do these get used? Any reason for these particular values? If you're >> trying to create a partition where the upper half is generic and the lower half >> is arch-specific, say so. > > KVM_SET_GUEST_DEBUG ioctl used to set/unset debug interrupts, which > have a "u32 control" element. We have inherited this mechanism from > x86 implementation and it looks like lower 16 bits are generic (like > KVM_GUESTDBG_ENBLE, KVM_GUESTDBG_SINGLESTEP etc and upper 16 bits are > Architecture specific. > > I will add a comment to describe this. I don't think the sw/hw distinction belongs here -- it should be per breakpoint. >>> + run->exit_reason = KVM_EXIT_DEBUG; >>> + run->debug.arch.pc = vcpu->arch.pc; >>> + run->debug.arch.exception = exit_nr; >>> + run->debug.arch.status = 0; >>> + kvmppc_account_exit(vcpu, DEBUG_EXITS); >>> + return RESUME_HOST; >> >> The interface isn't (clearly labelled as) booke specific, but you return booke- >> specific exception numbers. How's userspace supposed to know what to do with >> them? What do you plan on doing with them in QEMU? > > This is booke specific. Then put booke in the name, but what about it really needs to be booke specific? Why does QEMU care about the exception type? >>> +#ifndef CONFIG_PPC_FSL_BOOK3E >>> + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) >>> + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) >>> + mtspr SPRN_IAC3, r7 >>> + mtspr SPRN_IAC4, r8 >>> +#endif >> >> Can you handle this at runtime with a feature section? > > Why you want this to make run time? Removing config_ ? Currently KVM hardcodes the target hardware in a way that is unacceptable in much of the rest of the kernel. We have a long term goal to stop doing that, and we should avoid making it worse by adding random ifdefs for specific CPUs. -Scott -- 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
> >>> diff --git a/arch/powerpc/include/asm/kvm.h > >>> b/arch/powerpc/include/asm/kvm.h index 3c14202..da71c84 100644 > >>> --- a/arch/powerpc/include/asm/kvm.h > >>> +++ b/arch/powerpc/include/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; > >>> @@ -265,10 +266,19 @@ struct kvm_fpu { }; > >>> > >>> struct kvm_debug_exit_arch { > >>> + __u32 exception; > >>> + __u32 pc; > >>> + __u32 status; > >>> }; > >> > >> PC must be 64-bit. What goes in "status" and "exception"? status -> exit because of h/w breakpoint, watchpoint (read, write or both) and software breakpoint. exception -> returns the exception number. If the exit is not handled (say not h/w breakpoint or software breakpoint set for this address) by qemu then it is supposed to inject the exception to guest. This is how it is implemented for x86. > > > > ok > > > >> > >>> /* for KVM_SET_GUEST_DEBUG */ > >>> struct kvm_guest_debug_arch { > >>> + struct { > >>> + __u64 addr; > >>> + __u32 type; > >>> + __u32 pad1; > >>> + __u64 pad2; > >>> + } bp[16]; > >>> }; > >> > >> What goes in "type"? > > > > Type denote breakpoint, read watchpoint, write watchpoint or watchpoint (both > read and write). Will adding a comment to describe this is ok? > > Yes, please make sure all of this is well documented. > > >>> /* definition of registers in kvm_run */ @@ -285,6 +295,17 @@ > >>> struct kvm_sync_regs { > >>> #define KVM_CPU_3S_64 4 > >>> #define KVM_CPU_E500MC 5 > >>> > >>> +/* Debug related defines */ > >>> +#define KVM_INST_GUESTGDB 0x7C00021C /* ehpriv OC=0 */ > >> > >> Will this work on all PPC? > >> > >> It certainly won't work on other architectures, so at a minimum it's > >> KVM_PPC_INST_GUEST_GDB, but maybe it needs to be determined at runtime. > > > > How to determine at run time? adding another ioctl ? > > Or extend an existing one. Is there any other information about debug > capabilities that you expose -- number of hardware breakpoints supported, etc > > >>> +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 > >>> +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 > >> > >> Where do these get used? Any reason for these particular values? If > >> you're trying to create a partition where the upper half is generic > >> and the lower half is arch-specific, say so. > > > > KVM_SET_GUEST_DEBUG ioctl used to set/unset debug interrupts, which > > have a "u32 control" element. We have inherited this mechanism from > > x86 implementation and it looks like lower 16 bits are generic (like > > KVM_GUESTDBG_ENBLE, KVM_GUESTDBG_SINGLESTEP etc and upper 16 bits are > > Architecture specific. > > > > I will add a comment to describe this. > > I don't think the sw/hw distinction belongs here -- it should be per breakpoint. KVM does not track the software breakpoint, so it is not per breakpoint. In KVM, when KVM_GUESTDBG_USE_SW_BP flag is set and special trap instruction is executed by guest then exit to userspace. > > >>> + run->exit_reason = KVM_EXIT_DEBUG; > >>> + run->debug.arch.pc = vcpu->arch.pc; > >>> + run->debug.arch.exception = exit_nr; > >>> + run->debug.arch.status = 0; > >>> + kvmppc_account_exit(vcpu, DEBUG_EXITS); > >>> + return RESUME_HOST; > >> > >> The interface isn't (clearly labelled as) booke specific, but you > >> return booke- specific exception numbers. How's userspace supposed > >> to know what to do with them? What do you plan on doing with them in QEMU? > > > > This is booke specific. > > Then put booke in the name, Which data structure name should have booke? > but what about it really needs to be booke specific? > Why does QEMU care about the exception type? Explained above. Thanks -Bharat > > >>> +#ifndef CONFIG_PPC_FSL_BOOK3E > >>> + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > >>> + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > >>> + mtspr SPRN_IAC3, r7 > >>> + mtspr SPRN_IAC4, r8 > >>> +#endif > >> > >> Can you handle this at runtime with a feature section? > > > > Why you want this to make run time? Removing config_ ? > > Currently KVM hardcodes the target hardware in a way that is unacceptable in > much of the rest of the kernel. We have a long term goal to stop doing that, > and we should avoid making it worse by adding random ifdefs for specific CPUs. > > -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Tuesday, July 31, 2012 3:31 AM > To: Bhushan Bharat-R65777 > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; > agraf@suse.de > Subject: Re: [PATCH 2/2] KVM: PPC: booke/bookehv: Add guest debug support > > On 07/30/2012 02:37 AM, Bhushan Bharat-R65777 wrote: > > > > > >> -----Original Message----- > >> From: Wood Scott-B07421 > >> Sent: Friday, July 27, 2012 7:00 AM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; > >> Bhushan Bharat- > >> R65777 > >> Subject: Re: [PATCH 2/2] KVM: PPC: booke/bookehv: Add guest debug > >> support > >> > >> On 07/26/2012 12:32 AM, Bharat Bhushan wrote: > >>> This patch adds: > >>> 1) KVM debug handler added for e500v2. > >>> 2) Guest debug by qemu gdb stub. > >> > >> Does it make sense for these to both be in the same patch? If > >> there's common code used by both, that could be added first. > > > > ok > > > >> > >>> Signed-off-by: Liu Yu <yu.liu@freescale.com> > >>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > >>> [bharat.bhushan@freescale.com: Substantial changes] > >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > >>> --- > >>> arch/powerpc/include/asm/kvm.h | 21 +++++ > >>> arch/powerpc/include/asm/kvm_host.h | 7 ++ > >>> arch/powerpc/include/asm/kvm_ppc.h | 2 + > >>> arch/powerpc/include/asm/reg_booke.h | 1 + > >>> arch/powerpc/kernel/asm-offsets.c | 31 ++++++- > >>> arch/powerpc/kvm/booke.c | 146 +++++++++++++++++++++++++++--- > >>> arch/powerpc/kvm/booke_interrupts.S | 160 > >> ++++++++++++++++++++++++++++++++- > >>> arch/powerpc/kvm/bookehv_interrupts.S | 141 ++++++++++++++++++++++++++++- > >>> arch/powerpc/kvm/e500mc.c | 3 +- > >>> arch/powerpc/kvm/powerpc.c | 2 +- > >>> 10 files changed, 492 insertions(+), 22 deletions(-) > >>> > >>> diff --git a/arch/powerpc/include/asm/kvm.h > >>> b/arch/powerpc/include/asm/kvm.h index 3c14202..da71c84 100644 > >>> --- a/arch/powerpc/include/asm/kvm.h > >>> +++ b/arch/powerpc/include/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; > >>> @@ -265,10 +266,19 @@ struct kvm_fpu { }; > >>> > >>> struct kvm_debug_exit_arch { > >>> + __u32 exception; > >>> + __u32 pc; > >>> + __u32 status; > >>> }; > >> > >> PC must be 64-bit. What goes in "status" and "exception"? > > > > ok > > > >> > >>> /* for KVM_SET_GUEST_DEBUG */ > >>> struct kvm_guest_debug_arch { > >>> + struct { > >>> + __u64 addr; > >>> + __u32 type; > >>> + __u32 pad1; > >>> + __u64 pad2; > >>> + } bp[16]; > >>> }; > >> > >> What goes in "type"? > > > > Type denote breakpoint, read watchpoint, write watchpoint or watchpoint (both > read and write). Will adding a comment to describe this is ok? > > Yes, please make sure all of this is well documented. > > >>> /* definition of registers in kvm_run */ @@ -285,6 +295,17 @@ > >>> struct kvm_sync_regs { > >>> #define KVM_CPU_3S_64 4 > >>> #define KVM_CPU_E500MC 5 > >>> > >>> +/* Debug related defines */ > >>> +#define KVM_INST_GUESTGDB 0x7C00021C /* ehpriv OC=0 */ > >> > >> Will this work on all PPC? > >> > >> It certainly won't work on other architectures, so at a minimum it's > >> KVM_PPC_INST_GUEST_GDB, but maybe it needs to be determined at runtime. > > > > How to determine at run time? adding another ioctl ? > > Or extend an existing one. Is there any other information about debug > capabilities that you expose -- number of hardware breakpoints supported, etc? > > >>> +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 > >>> +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 > >> > >> Where do these get used? Any reason for these particular values? If > >> you're trying to create a partition where the upper half is generic > >> and the lower half is arch-specific, say so. > > > > KVM_SET_GUEST_DEBUG ioctl used to set/unset debug interrupts, which > > have a "u32 control" element. We have inherited this mechanism from > > x86 implementation and it looks like lower 16 bits are generic (like > > KVM_GUESTDBG_ENBLE, KVM_GUESTDBG_SINGLESTEP etc and upper 16 bits are > > Architecture specific. > > > > I will add a comment to describe this. > > I don't think the sw/hw distinction belongs here -- it should be per breakpoint. > > >>> + run->exit_reason = KVM_EXIT_DEBUG; > >>> + run->debug.arch.pc = vcpu->arch.pc; > >>> + run->debug.arch.exception = exit_nr; > >>> + run->debug.arch.status = 0; > >>> + kvmppc_account_exit(vcpu, DEBUG_EXITS); > >>> + return RESUME_HOST; > >> > >> The interface isn't (clearly labelled as) booke specific, but you > >> return booke- specific exception numbers. How's userspace supposed > >> to know what to do with them? What do you plan on doing with them in QEMU? > > > > This is booke specific. > > Then put booke in the name, but what about it really needs to be booke specific? > Why does QEMU care about the exception type? > > >>> +#ifndef CONFIG_PPC_FSL_BOOK3E > >>> + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > >>> + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > >>> + mtspr SPRN_IAC3, r7 > >>> + mtspr SPRN_IAC4, r8 > >>> +#endif > >> > >> Can you handle this at runtime with a feature section? > > > > Why you want this to make run time? Removing config_ ? > > Currently KVM hardcodes the target hardware in a way that is unacceptable in > much of the rest of the kernel. We have a long term goal to stop doing that, > and we should avoid making it worse by adding random ifdefs for specific CPUs. I do not see any CPU_FTR_* which I can use directly. Should I define a new FTR, something like: #define CPU_FTR_DEBUG_E500 LONG_ASM_CONST(0x4000000000000000) Use this in: CPU_FTRS_E500_2, CPU_FTRS_E500MC, CPU_FTRS_E5500 etc BEGIN_FTR_SECTION PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) mtspr SPRN_IAC3, r7 mtspr SPRN_IAC4, r8 END_FTR_SECTION_IFCLR(CPU_FTR_DEBUG_E500) Thanks -Bharat > > -Scott
On 08/16/2012 03:48 AM, Bhushan Bharat-R65777 wrote: >>>>> diff --git a/arch/powerpc/include/asm/kvm.h >>>>> b/arch/powerpc/include/asm/kvm.h index 3c14202..da71c84 100644 >>>>> --- a/arch/powerpc/include/asm/kvm.h >>>>> +++ b/arch/powerpc/include/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; >>>>> @@ -265,10 +266,19 @@ struct kvm_fpu { }; >>>>> >>>>> struct kvm_debug_exit_arch { >>>>> + __u32 exception; >>>>> + __u32 pc; >>>>> + __u32 status; >>>>> }; >>>> >>>> PC must be 64-bit. What goes in "status" and "exception"? > > status -> exit because of h/w breakpoint, watchpoint (read, write or > both) and software breakpoint. > > exception -> returns the exception number. If the exit is not handled > (say not h/w breakpoint or software breakpoint set for this address) > by qemu then it is supposed to inject the exception to guest. This is > how it is implemented for x86. Where is this documented (including the specific values that are possible)? >>>>> +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 >>>>> +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 >>>> >>>> Where do these get used? Any reason for these particular values? If >>>> you're trying to create a partition where the upper half is generic >>>> and the lower half is arch-specific, say so. >>> >>> KVM_SET_GUEST_DEBUG ioctl used to set/unset debug interrupts, which >>> have a "u32 control" element. We have inherited this mechanism from >>> x86 implementation and it looks like lower 16 bits are generic (like >>> KVM_GUESTDBG_ENBLE, KVM_GUESTDBG_SINGLESTEP etc and upper 16 bits are >>> Architecture specific. >>> >>> I will add a comment to describe this. >> >> I don't think the sw/hw distinction belongs here -- it should be per breakpoint. > > KVM does not track the software breakpoint, so it is not per breakpoint. > In KVM, when KVM_GUESTDBG_USE_SW_BP flag is set and special trap instruction is executed by guest then exit to userspace. Can both types of breakpoint be set at the same time? >>>>> + run->exit_reason = KVM_EXIT_DEBUG; >>>>> + run->debug.arch.pc = vcpu->arch.pc; >>>>> + run->debug.arch.exception = exit_nr; >>>>> + run->debug.arch.status = 0; >>>>> + kvmppc_account_exit(vcpu, DEBUG_EXITS); >>>>> + return RESUME_HOST; >>>> >>>> The interface isn't (clearly labelled as) booke specific, but you >>>> return booke- specific exception numbers. How's userspace supposed >>>> to know what to do with them? What do you plan on doing with them in QEMU? >>> >>> This is booke specific. >> >> Then put booke in the name, > > Which data structure name should have booke? Anything that's booke specific. -Scott -- 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
On 08/16/2012 10:12 AM, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Wood Scott-B07421 >> Sent: Tuesday, July 31, 2012 3:31 AM >> To: Bhushan Bharat-R65777 >> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; >> agraf@suse.de >> Subject: Re: [PATCH 2/2] KVM: PPC: booke/bookehv: Add guest debug support >> >> On 07/30/2012 02:37 AM, Bhushan Bharat-R65777 wrote: >>> >>> >>>> -----Original Message----- >>>> From: Wood Scott-B07421 >>>> Sent: Friday, July 27, 2012 7:00 AM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; >>>> Bhushan Bharat- >>>> R65777 >>>> Subject: Re: [PATCH 2/2] KVM: PPC: booke/bookehv: Add guest debug >>>> support >>>> >>>>> +#ifndef CONFIG_PPC_FSL_BOOK3E >>>>> + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) >>>>> + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) >>>>> + mtspr SPRN_IAC3, r7 >>>>> + mtspr SPRN_IAC4, r8 >>>>> +#endif >>>> >>>> Can you handle this at runtime with a feature section? >>> >>> Why you want this to make run time? Removing config_ ? >> >> Currently KVM hardcodes the target hardware in a way that is unacceptable in >> much of the rest of the kernel. We have a long term goal to stop doing that, >> and we should avoid making it worse by adding random ifdefs for specific CPUs. > > I do not see any CPU_FTR_* which I can use directly. Should I define a new FTR, something like: > > #define CPU_FTR_DEBUG_E500 LONG_ASM_CONST(0x4000000000000000) > > Use this in: CPU_FTRS_E500_2, CPU_FTRS_E500MC, CPU_FTRS_E5500 etc > > BEGIN_FTR_SECTION > PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) > PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) > mtspr SPRN_IAC3, r7 > mtspr SPRN_IAC4, r8 > END_FTR_SECTION_IFCLR(CPU_FTR_DEBUG_E500) It looks like other parts of the kernel use CONFIG_PPC_ADV_DEBUG_IACS for this, though ideally it would be made runtime in the future. -Scott -- 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
diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h index 3c14202..da71c84 100644 --- a/arch/powerpc/include/asm/kvm.h +++ b/arch/powerpc/include/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; @@ -265,10 +266,19 @@ struct kvm_fpu { }; struct kvm_debug_exit_arch { + __u32 exception; + __u32 pc; + __u32 status; }; /* for KVM_SET_GUEST_DEBUG */ struct kvm_guest_debug_arch { + struct { + __u64 addr; + __u32 type; + __u32 pad1; + __u64 pad2; + } bp[16]; }; /* definition of registers in kvm_run */ @@ -285,6 +295,17 @@ struct kvm_sync_regs { #define KVM_CPU_3S_64 4 #define KVM_CPU_E500MC 5 +/* Debug related defines */ +#define KVM_INST_GUESTGDB 0x7C00021C /* ehpriv OC=0 */ + +#define KVM_GUESTDBG_USE_SW_BP 0x00010000 +#define KVM_GUESTDBG_USE_HW_BP 0x00020000 + +#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) + /* for KVM_CAP_SPAPR_TCE */ struct kvm_create_spapr_tce { __u64 liobn; diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 7a45194..524af7a 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -458,7 +458,12 @@ struct kvm_vcpu_arch { u32 ccr0; u32 ccr1; u32 dbsr; + /* guest debug regiters*/ struct kvmppc_booke_debug_reg dbg_reg; + /* shadow debug registers */ + struct kvmppc_booke_debug_reg shadow_dbg_reg; + /* host debug regiters*/ + struct kvmppc_booke_debug_reg host_dbg_reg; u64 mmcr[3]; u32 pmc[8]; @@ -492,6 +497,7 @@ struct kvm_vcpu_arch { u32 tlbcfg[4]; u32 mmucfg; u32 epr; + u32 crit_save; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; @@ -533,6 +539,7 @@ struct kvm_vcpu_arch { struct kvm_vcpu_arch_shared *shared; unsigned long magic_page_pa; /* phys addr to map the magic page to */ unsigned long magic_page_ea; /* effect. addr to map the magic page to */ + struct kvm_guest_debug_arch dbg; /* debug arg between kvm and qemu */ #ifdef CONFIG_KVM_BOOK3S_64_HV struct kvm_vcpu_arch_shared shregs; diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 823d563..c97b234 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -115,6 +115,8 @@ extern int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong val); extern int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *val); +extern int kvmppc_core_set_guest_debug(struct kvm_vcpu *vcpu, + struct kvm_guest_debug *dbg); extern int kvmppc_booke_init(void); extern void kvmppc_booke_exit(void); diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h index e07e6af..b417de3 100644 --- a/arch/powerpc/include/asm/reg_booke.h +++ b/arch/powerpc/include/asm/reg_booke.h @@ -56,6 +56,7 @@ #define SPRN_SPRG7W 0x117 /* Special Purpose Register General 7 Write */ #define SPRN_EPCR 0x133 /* Embedded Processor Control Register */ #define SPRN_DBCR2 0x136 /* Debug Control Register 2 */ +#define SPRN_DBCR4 0x233 /* Debug Control Register 4 */ #define SPRN_MSRP 0x137 /* MSR Protect Register */ #define SPRN_IAC3 0x13A /* Instruction Address Compare 3 */ #define SPRN_IAC4 0x13B /* Instruction Address Compare 4 */ diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c index 52c7ad7..1310775 100644 --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -116,7 +116,7 @@ int main(void) #ifdef CONFIG_KVM_BOOK3S_32_HANDLER DEFINE(THREAD_KVM_SVCPU, offsetof(struct thread_struct, kvm_shadow_vcpu)); #endif -#ifdef CONFIG_KVM_BOOKE_HV +#if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC) DEFINE(THREAD_KVM_VCPU, offsetof(struct thread_struct, kvm_vcpu)); #endif @@ -431,6 +431,9 @@ int main(void) DEFINE(VCPU_KVM, offsetof(struct kvm_vcpu, kvm)); DEFINE(KVM_LPID, offsetof(struct kvm, arch.lpid)); +#ifdef CONFIG_BOOKE + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save)); +#endif /* book3s */ #ifdef CONFIG_KVM_BOOK3S_64_HV @@ -562,6 +565,32 @@ int main(void) DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear)); DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr)); + DEFINE(VCPU_DBSR, offsetof(struct kvm_vcpu, arch.dbsr)); + DEFINE(VCPU_SHADOW_DBG, offsetof(struct kvm_vcpu, arch.shadow_dbg_reg)); + DEFINE(VCPU_HOST_DBG, offsetof(struct kvm_vcpu, arch.host_dbg_reg)); + DEFINE(KVMPPC_DBG_DBCR0, offsetof(struct kvmppc_booke_debug_reg, + dbcr0)); + DEFINE(KVMPPC_DBG_DBCR1, offsetof(struct kvmppc_booke_debug_reg, + dbcr1)); + DEFINE(KVMPPC_DBG_DBCR2, offsetof(struct kvmppc_booke_debug_reg, + dbcr2)); +#ifdef CONFIG_KVM_E500MC + DEFINE(KVMPPC_DBG_DBCR4, offsetof(struct kvmppc_booke_debug_reg, + dbcr4)); +#endif + DEFINE(KVMPPC_DBG_IAC1, offsetof(struct kvmppc_booke_debug_reg, + iac[0])); + DEFINE(KVMPPC_DBG_IAC2, offsetof(struct kvmppc_booke_debug_reg, + iac[1])); + DEFINE(KVMPPC_DBG_IAC3, offsetof(struct kvmppc_booke_debug_reg, + iac[2])); + DEFINE(KVMPPC_DBG_IAC4, offsetof(struct kvmppc_booke_debug_reg, + iac[3])); + DEFINE(KVMPPC_DBG_DAC1, offsetof(struct kvmppc_booke_debug_reg, + dac[0])); + DEFINE(KVMPPC_DBG_DAC2, offsetof(struct kvmppc_booke_debug_reg, + dac[1])); + DEFINE(VCPU_GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug)); #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */ diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 6fbdcfc..784a6bf 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -130,6 +130,9 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) #ifdef CONFIG_KVM_BOOKE_HV new_msr |= MSR_GS; + + if (vcpu->guest_debug) + new_msr |= MSR_DE; #endif vcpu->arch.shared->msr = new_msr; @@ -684,10 +687,21 @@ out: return ret; } -static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) +static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, + int exit_nr) { enum emulation_result er; + if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) && + (vcpu->arch.last_inst == KVM_INST_GUESTGDB)) { + run->exit_reason = KVM_EXIT_DEBUG; + run->debug.arch.pc = vcpu->arch.pc; + run->debug.arch.exception = exit_nr; + run->debug.arch.status = 0; + kvmppc_account_exit(vcpu, DEBUG_EXITS); + return RESUME_HOST; + } + er = kvmppc_emulate_instruction(run, vcpu); switch (er) { case EMULATE_DONE: @@ -714,6 +728,44 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu) default: BUG(); } + + if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_ENABLE) && + (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)) { + run->exit_reason = KVM_EXIT_DEBUG; + return RESUME_HOST; + } +} + +static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu) +{ + u32 dbsr; + +#ifndef CONFIG_KVM_BOOKE_HV + if (cpu_has_feature(CPU_FTR_DEBUG_LVL_EXC)) + vcpu->arch.pc = mfspr(SPRN_DSRR0); + else + vcpu->arch.pc = mfspr(SPRN_CSRR0); +#endif + dbsr = vcpu->arch.dbsr; + + run->debug.arch.pc = vcpu->arch.pc; + run->debug.arch.status = 0; + vcpu->arch.dbsr = 0; + + 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.pc = vcpu->arch.shadow_dbg_reg.dac[0]; + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W)) + run->debug.arch.pc = vcpu->arch.shadow_dbg_reg.dac[1]; + } + + return RESUME_HOST; } static void kvmppc_fill_pt_regs(struct pt_regs *regs) @@ -856,7 +908,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; case BOOKE_INTERRUPT_HV_PRIV: - r = emulation_exit(run, vcpu); + r = emulation_exit(run, vcpu, exit_nr); break; case BOOKE_INTERRUPT_PROGRAM: @@ -875,7 +927,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; } - r = emulation_exit(run, vcpu); + r = emulation_exit(run, vcpu, exit_nr); break; case BOOKE_INTERRUPT_FP_UNAVAIL: @@ -1065,18 +1117,12 @@ 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->debug.arch.exception = exit_nr; + run->exit_reason = KVM_EXIT_DEBUG; + } kvmppc_account_exit(vcpu, DEBUG_EXITS); - r = RESUME_HOST; break; } @@ -1107,6 +1153,78 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, return r; } +#define BP_NUM KVMPPC_BOOKE_IAC_NUM +#define WP_NUM KVMPPC_BOOKE_DAC_NUM + +int kvmppc_core_set_guest_debug(struct kvm_vcpu *vcpu, + struct kvm_guest_debug *dbg) +{ + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) { + vcpu->guest_debug = 0; + return 0; + } + + vcpu->guest_debug = dbg->control; + vcpu->arch.shadow_dbg_reg.dbcr0 = 0; + + 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; +} + /* Initial guest state: 16MB mapping 0 -> 0, PC = 0, MSR = 0, R1 = 16MB */ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu) { diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S index bcb34ea..fb85606 100644 --- a/arch/powerpc/kvm/booke_interrupts.S +++ b/arch/powerpc/kvm/booke_interrupts.S @@ -40,6 +40,8 @@ #define HOST_MIN_STACK_SIZE (HOST_NV_GPR(31) + 4) #define HOST_STACK_SIZE (((HOST_MIN_STACK_SIZE + 15) / 16) * 16) /* Align. */ #define HOST_STACK_LR (HOST_STACK_SIZE + 4) /* In caller stack frame. */ +#define DBCR0_AC_BITS (DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \ + DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W) #define NEED_INST_MASK ((1<<BOOKE_INTERRUPT_PROGRAM) | \ (1<<BOOKE_INTERRUPT_DTLB_MISS) | \ @@ -53,11 +55,17 @@ (1<<BOOKE_INTERRUPT_PROGRAM) | \ (1<<BOOKE_INTERRUPT_DTLB_MISS)) +#define NEED_DEBUG_SAVE (1<<BOOKE_INTERRUPT_DEBUG) + +#define GET_VCPU_POINT(regd) \ + mfspr regd, SPRN_SPRG_THREAD; \ + lwz regd, THREAD_KVM_VCPU(regd) + .macro KVM_HANDLER ivor_nr scratch srr0 _GLOBAL(kvmppc_handler_\ivor_nr) /* Get pointer to vcpu and record exit number. */ mtspr \scratch , r4 - mfspr r4, SPRN_SPRG_RVCPU + GET_VCPU_POINT(r4) stw r3, VCPU_GPR(r3)(r4) stw r5, VCPU_GPR(r5)(r4) stw r6, VCPU_GPR(r6)(r4) @@ -74,6 +82,48 @@ _GLOBAL(kvmppc_handler_\ivor_nr) bctr .endm +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 +_GLOBAL(kvmppc_handler_\ivor_nr) + mtspr \scratch, r4 + GET_VCPU_POINT(r4) + stw r3, VCPU_CRIT_SAVE(r4) + mfcr r3 + mfspr r4, SPRN_CSRR1 + andi. r4, r4, MSR_PR + bne 1f + /* debug interrupt happened in enter/exit path */ + mfspr r4, SPRN_CSRR1 + rlwinm r4, r4, 0, ~MSR_DE + mtspr SPRN_CSRR1, r4 + lis r4, 0xffff + ori r4, r4, 0xffff + mtspr SPRN_DBSR, r4 + GET_VCPU_POINT(r4) + mtcr r3 + lwz r3, VCPU_CRIT_SAVE(r4) + mfspr r4, \scratch + rfci +1: /* debug interrupt happened in guest */ + mfspr r4, \scratch + mtcr r3 + mr r3, r4 + GET_VCPU_POINT(r4) + stw r3, VCPU_GPR(r4)(r4) + stw r5, VCPU_GPR(r5)(r4) + stw r6, VCPU_GPR(r6)(r4) + lwz r3, VCPU_CRIT_SAVE(r4) + mfspr r5, \srr0 + stw r3, VCPU_GPR(r3)(r4) + stw r5, VCPU_PC(r4) + mfctr r5 + lis r6, kvmppc_resume_host@h + stw r5, VCPU_CTR(r4) + li r5, \ivor_nr + ori r6, r6, kvmppc_resume_host@l + mtctr r6 + bctr +.endm + .macro KVM_HANDLER_ADDR ivor_nr .long kvmppc_handler_\ivor_nr .endm @@ -94,7 +144,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0 -KVM_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 +KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0 @@ -176,6 +226,61 @@ _GLOBAL(kvmppc_resume_host) stw r9, VCPU_FAULT_ESR(r4) ..skip_esr: + addi r7, r4, VCPU_HOST_DBG + mfspr r9, SPRN_DBCR0 + lwz r8, KVMPPC_DBG_DBCR0(r7) + andis. r9, r9, DBCR0_AC_BITS@h + beq ..skip_load_host_debug + li r9, 0 + mtspr SPRN_DBCR0, r9 /* disable all debug event */ + lwz r9, KVMPPC_DBG_DBCR1(r7) + mtspr SPRN_DBCR1, r9 + lwz r9, KVMPPC_DBG_DBCR2(r7) + mtspr SPRN_DBCR2, r9 + lwz r9, KVMPPC_DBG_IAC1+4(r7) + mtspr SPRN_IAC1, r9 + lwz r9, KVMPPC_DBG_IAC2+4(r7) + mtspr SPRN_IAC2, r9 +#ifndef CONFIG_PPC_FSL_BOOK3E + lwz r9, KVMPPC_DBG_IAC3+4(r7) + mtspr SPRN_IAC3, r9 + lwz r9, KVMPPC_DBG_IAC4+4(r7) + mtspr SPRN_IAC4, r9 +#endif + lwz r9, KVMPPC_DBG_DAC1+4(r7) + mtspr SPRN_DAC1, r9 + lwz r9, KVMPPC_DBG_DAC2+4(r7) + mtspr SPRN_DAC2, r9 +..skip_load_host_debug: + /* Clear h/w DBSR and save current(guest) DBSR */ + mfspr r9, SPRN_DBSR + mtspr SPRN_DBSR, r9 + isync + andi. r7, r6, NEED_DEBUG_SAVE + beq ..skip_dbsr_save + /* + * If vcpu->guest_debug flag is set then do not check for + * shared->msr.DE as this debugging (say by QEMU) does not + * depends on shared->msr.de. In these scanerios MSR.DE is + * always set using shared_msr and should be handled always. + */ + lwz r7, VCPU_GUEST_DEBUG(r4) + cmpwi r7, 0 + bne ..skip_save_trap_event + PPC_LL r3, VCPU_SHARED(r4) +#ifndef CONFIG_64BIT + lwz r3, (VCPU_SHARED_MSR + 4)(r3) +#else + ld r3, (VCPU_SHARED_MSR)(r3) +#endif + andi. r3, r3, MSR_DE + bne ..skip_save_trap_event + andis. r9, r9, DBSR_TIE@h +..skip_save_trap_event: + stw r9, VCPU_DBSR(r4) +..skip_dbsr_save: + mtspr SPRN_DBCR0, r8 + /* Save remaining volatile guest register state to vcpu. */ stw r0, VCPU_GPR(r0)(r4) stw r1, VCPU_GPR(r1)(r4) @@ -432,6 +537,57 @@ lightweight_exit: PPC_LD(r3, VCPU_SHARED_SPRG7, r5) mtspr SPRN_SPRG7W, r3 + mfmsr r7 + rlwinm r7, r7, 0, ~MSR_DE + mtmsr r7 + addi r5, r4, VCPU_SHADOW_DBG + addi r7, r4, VCPU_HOST_DBG + lwz r6, 0(r5) + mfspr r8, SPRN_DBCR0 + andis. r3, r6, DBCR0_AC_BITS@h + stw r8, KVMPPC_DBG_DBCR0(r7) + beq ..skip_load_guest_debug + mfspr r8, SPRN_DBCR1 + stw r8, KVMPPC_DBG_DBCR1(r7) + mfspr r8, SPRN_DBCR2 + stw r8, KVMPPC_DBG_DBCR2(r7) + mfspr r8, SPRN_IAC1 + stw r8, KVMPPC_DBG_IAC1+4(r7) + mfspr r8, SPRN_IAC2 + stw r8, KVMPPC_DBG_IAC2+4(r7) +#ifndef CONFIG_PPC_FSL_BOOK3E + mfspr r8, SPRN_IAC3 + stw r8, KVMPPC_DBG_IAC3+4(r7) + mfspr r8, SPRN_IAC4 + stw r8, KVMPPC_DBG_IAC4+4(r7) +#endif + mfspr r8, SPRN_DAC1 + stw r8, KVMPPC_DBG_DAC1+4(r7) + mfspr r8, SPRN_DAC2 + stw r8, KVMPPC_DBG_DAC2+4(r7) + li r8, 0 + mtspr SPRN_DBCR0, r8 /* disable all debug event */ + lwz r8, KVMPPC_DBG_DBCR1(r5) + mtspr SPRN_DBCR1, r8 + lwz r8, KVMPPC_DBG_DBCR2(r5) + mtspr SPRN_DBCR2, r8 + lwz r8, KVMPPC_DBG_IAC1+4(r5) + mtspr SPRN_IAC1, r8 + lwz r8, KVMPPC_DBG_IAC2+4(r5) + mtspr SPRN_IAC2, r8 +#ifndef CONFIG_PPC_FSL_BOOK3E + lwz r8, KVMPPC_DBG_IAC3+4(r5) + mtspr SPRN_IAC3, r8 + lwz r8, KVMPPC_DBG_IAC4+4(r5) + mtspr SPRN_IAC4, r8 +#endif + lwz r8, KVMPPC_DBG_DAC1+4(r5) + mtspr SPRN_DAC1, r8 + lwz r8, KVMPPC_DBG_DAC2+4(r5) + mtspr SPRN_DAC2, r8 +..skip_load_guest_debug: + mtspr SPRN_DBCR0, r6 + #ifdef CONFIG_KVM_EXIT_TIMING /* save enter time */ 1: diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index 0fa2ef7..32b9a41 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -59,6 +59,10 @@ #define NEED_EMU 0x00000001 /* emulation -- save nv regs */ #define NEED_DEAR 0x00000002 /* save faulting DEAR */ #define NEED_ESR 0x00000004 /* save faulting ESR */ +#define NEED_DBSR 0x00000008 /* save DBSR */ + +#define DBCR0_AC_BITS (DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \ + DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W) /* * On entry: @@ -202,6 +206,11 @@ PPC_STL r9, VCPU_FAULT_DEAR(r4) .endif + .if \flags & NEED_DBSR + mfspr r9, SPRN_DBSR + stw r9, VCPU_DBSR(r4) + .endif + b kvmppc_resume_host .endm @@ -296,9 +305,9 @@ kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, SPRN_GSRR0, SPRN_GSRR1, 0 kvm_lvl_handler BOOKE_INTERRUPT_GUEST_DBELL_CRIT, \ SPRN_SPRG_RSCRATCH_CRIT, SPRN_CSRR0, SPRN_CSRR1, 0 kvm_lvl_handler BOOKE_INTERRUPT_DEBUG, \ - SPRN_SPRG_RSCRATCH_CRIT, SPRN_CSRR0, SPRN_CSRR1, 0 + SPRN_SPRG_RSCRATCH_CRIT, SPRN_CSRR0, SPRN_CSRR1, NEED_DBSR kvm_lvl_handler BOOKE_INTERRUPT_DEBUG, \ - SPRN_SPRG_RSCRATCH_DBG, SPRN_DSRR0, SPRN_DSRR1, 0 + SPRN_SPRG_RSCRATCH_DBG, SPRN_DSRR0, SPRN_DSRR1, NEED_DBSR /* Registers: @@ -308,6 +317,56 @@ kvm_lvl_handler BOOKE_INTERRUPT_DEBUG, \ * r14: KVM exit number */ _GLOBAL(kvmppc_resume_host) + /* + * If guest not used debug facility then hw debug registers + * already have proper host values. If guest used debug + * facility then restore host debug registers. + * No Need to save guest debug registers as they are already intact + * in guest/shadow registers. + */ + lwz r9, VCPU_SHADOW_DBG(r4) + rlwinm. r8, r9, 0, ~DBCR0_IDM + beq skip_load_host_debug + lwz r3, VCPU_HOST_DBG(r4) + andis. r9, r9, DBCR0_AC_BITS@h + li r9, 0 + mtspr SPRN_DBCR0, r9 /* disable all debug event */ + beq ..skip_load_hw_bkpts + lwz r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) + lwz r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) + lwz r9, VCPU_HOST_DBG+KVMPPC_DBG_DBCR4(r4) + mtspr SPRN_DBCR1, r7 + PPC_LD(r6, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) + mtspr SPRN_DBCR2, r8 + mtspr SPRN_DBCR4, r9 + mtspr SPRN_IAC1, r6 + mtspr SPRN_IAC2, r7 +#ifndef CONFIG_PPC_FSL_BOOK3E + PPC_LD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) + mtspr SPRN_IAC3, r7 + mtspr SPRN_IAC4, r8 +#endif + PPC_LD(r8, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) + PPC_LD(r9, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) + mtspr SPRN_DAC1, r8 + mtspr SPRN_DAC2, r9 +..skip_load_hw_bkpts: + isync + /* Clear h/w DBSR and save current(guest) DBSR */ + mfspr r8, SPRN_DBSR + mtspr SPRN_DBSR, r8 + isync + /* Clear EPCR.DUVD and set host DBCR0 */ + mfspr r8, SPRN_EPCR + rlwinm r8, r8, 0, ~SPRN_EPCR_DUVD + mtspr SPRN_EPCR, r8 + isync + mtspr SPRN_DBCR0, r3 + isync +skip_load_host_debug: + /* Save remaining volatile guest register state to vcpu. */ mfspr r3, SPRN_VRSAVE PPC_STL r0, VCPU_GPR(r0)(r4) @@ -547,6 +606,84 @@ lightweight_exit: mtspr SPRN_SPRG6W, r7 mtspr SPRN_SPRG7W, r8 + mfmsr r7 + rlwinm r7, r7, 0, ~MSR_DE + mtmsr r7 + /* + * Load hw debug registers with guest(shadow) debug registers + * if guest is using the debug facility and also set EPCR.DUVD + * to not allow debug events in HV mode. Do not change the + * debug registers if guest is not using the debug facility. + */ + lwz r6, VCPU_SHADOW_DBG(r4) + rlwinm. r7, r6, 0, ~DBCR0_IDM + beq ..skip_load_guest_debug + /* Save host DBCR0 */ + mfspr r8, SPRN_DBCR0 + stw r8, VCPU_HOST_DBG(r4) + /* + * Save host DBCR1/2, IACx and DACx and load guest DBCR1/2, + * IACx and DACx if guest using hw breakpoint/watchpoints. + */ + andis. r3, r6, DBCR0_AC_BITS@h + beq ..skip_hw_bkpts + mfspr r7, SPRN_DBCR1 + stw r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR1(r4) + mfspr r8, SPRN_DBCR2 + stw r8, VCPU_HOST_DBG+KVMPPC_DBG_DBCR2(r4) + mfspr r7, SPRN_DBCR4 + stw r7, VCPU_HOST_DBG+KVMPPC_DBG_DBCR4(r4) + mfspr r8, SPRN_IAC1 + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC1, r4) + mfspr r7, SPRN_IAC2 + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC2, r4) +#ifndef CONFIG_PPC_FSL_BOOK3E + mfspr r8, SPRN_IAC3 + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_IAC3, r4) + mfspr r7, SPRN_IAC4 + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_IAC4, r4) +#endif + mfspr r8, SPRN_DAC1 + PPC_STD(r8, VCPU_HOST_DBG+KVMPPC_DBG_DAC1, r4) + mfspr r7, SPRN_DAC2 + PPC_STD(r7, VCPU_HOST_DBG+KVMPPC_DBG_DAC2, r4) + li r8, 0 + mtspr SPRN_DBCR0, r8 /* disable all debug event */ + lwz r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR1(r4) + lwz r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR2(r4) + lwz r9, VCPU_SHADOW_DBG+KVMPPC_DBG_DBCR4(r4) + mtspr SPRN_DBCR1, r7 + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC1, r4) + PPC_LD(r3, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC2, r4) + mtspr SPRN_DBCR2, r8 + mtspr SPRN_DBCR4, r9 + mtspr SPRN_IAC1, r7 + mtspr SPRN_IAC2, r3 +#ifndef CONFIG_PPC_FSL_BOOK3E + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC3, r4) + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_IAC4, r4) + mtspr SPRN_IAC3, r7 + mtspr SPRN_IAC4, r8 +#endif + PPC_LD(r7, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC1, r4) + PPC_LD(r8, VCPU_SHADOW_DBG+KVMPPC_DBG_DAC2, r4) + mtspr SPRN_DAC1, r7 + mtspr SPRN_DAC2, r8 +..skip_hw_bkpts: + /* Set EPCR.DUVD and guest DBCR0 */ + mfspr r7, SPRN_EPCR + oris r7, r7, SPRN_EPCR_DUVD@h + mtspr SPRN_EPCR, r7 + isync + /* Clear if any deferred debug event */ + mfspr r8, SPRN_DBSR + mtspr SPRN_DBSR, r8 + isync + /* Restore guest DBCR */ + mtspr SPRN_DBCR0, r6 + isync +..skip_load_guest_debug: + /* Load some guest volatiles. */ PPC_LL r3, VCPU_LR(r4) PPC_LL r5, VCPU_XER(r4) 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 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 685829a..38b5d02 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -427,7 +427,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) { - return -EINVAL; + return kvmppc_core_set_guest_debug(vcpu, dbg); } static void kvmppc_complete_dcr_load(struct kvm_vcpu *vcpu,