> >>> 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 ��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�