> >>> 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
