On 2012-09-25 12:47, Alexander Graf wrote: > > On 25.09.2012, at 12:38, Jan Kiszka wrote: > >> On 2012-09-24 16:46, Alexander Graf wrote: >>> >>> On 07.09.2012, at 00:56, Scott Wood wrote: >>> >>>> On 09/06/2012 09:56 AM, Bhushan Bharat-R65777 wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Wood Scott-B07421 >>>>>> Sent: Thursday, September 06, 2012 4:57 AM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: [email protected]; [email protected]; [email protected]; Bhushan >>>>>> Bharat- >>>>>> R65777 >>>>>> Subject: Re: [PATCH 6/6] KVM: booke/bookehv: Add debug stub support >>>>>> >>>>>> On 09/05/2012 06:23 PM, Scott Wood wrote: >>>>>>> On 08/21/2012 08:52 AM, 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 <[email protected]> >>>>>>>> --- >>>>>>>> arch/powerpc/include/asm/kvm.h | 29 ++++++- >>>>>>>> arch/powerpc/include/asm/kvm_host.h | 5 + >>>>>>>> arch/powerpc/kernel/asm-offsets.c | 26 ++++++ >>>>>>>> arch/powerpc/kvm/booke.c | 144 >>>>>>>> +++++++++++++++++++++++++++++-- >>>>>> -- >>>>>>>> arch/powerpc/kvm/booke_interrupts.S | 110 +++++++++++++++++++++++++ >>>>>>>> arch/powerpc/kvm/bookehv_interrupts.S | 141 >>>>>> +++++++++++++++++++++++++++++++- >>>>>>>> arch/powerpc/kvm/e500mc.c | 3 +- >>>>>>>> 7 files changed, 435 insertions(+), 23 deletions(-) >>>>>>>> >>>>>>>> diff --git a/arch/powerpc/include/asm/kvm.h >>>>>>>> b/arch/powerpc/include/asm/kvm.h index 61b197e..53479ea 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; >>>>>>>> @@ -264,7 +265,31 @@ 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 { >>>>>>> >>>>>>> That says "arch", but it's not in an arch-specific file. >>>>>> >>>>>> Sigh, I can't read today apparently. >>>>>> >>>>>>>> + __u64 pc; >>>>>>>> + /* >>>>>>>> + * exception -> returns the exception number. If the >>>>>>>> KVM_DEBUG_EXIT >>>>>>>> + * exit is not handled (say not h/w breakpoint or software >>>>>>>> breakpoint >>>>>>>> + * set for this address) by qemu then it is supposed to inject >>>>>>>> this >>>>>>>> + * exception to guest. >>>>>>>> + */ >>>>>>>> + __u32 exception; >>>>>>>> + /* >>>>>>>> + * exiting to userspace because of h/w breakpoint, watchpoint >>>>>>>> + * (read, write or both) and software breakpoint. >>>>>>>> + */ >>>>>>>> + __u32 status; >>>>>>>> }; >>>>>>> >>>>>>> What does "exception number" mean in a generic API? >>>>>> >>>>>> Still, "exception number" is not a well-defined concept powerpc-wide. >>>>> >>>>> Just for background why we added is that, on x86 this exception number is >>>>> used to inject the exception to guest if QEMU is not able to handle the >>>>> debug exception. >>>>> >>>>> Should we just through a print with clearing the exception condition? Or >>>>> something else you would like to suggest? >>>> >>>> We can pass up the exception type; it just needs more documentation >>>> about what exactly you're referring to, and probably some enumeration >>>> that says which exception numberspace it is. >>>> >>>> For booke the exception number should probably be related to the fixed >>>> offsets rather than the IVOR number, as IVORs are phased out. >>> >>> Jan, I would like to get your comment on this one. >>> >>> Since we don't have standardized exception vectors like x86 does, we need >>> to convert things between different semantics in user space if we want to >>> make use of the exception type. Do we actually need to know about it in >>> user space or do we only need to store it in case we get a migration at >>> that point? >>> >>> If it's the latter, can we maybe keep the reinjection logic internal to KVM >>> and make DEBUG exits non-migratable similar to how we already handle >>> MMIO/PIO exits today? >> >> Reinjection depends on userspace. It has to check if the debug event was >> related to the host debugging the guest or if it was due to a >> guest-internal debugging event. That's because the kernel does not track >> individual breakpoints, just controls the trapping. So we need a way to >> manage the reinjection, and we do this (on x86) by passing the exception >> up and then using it for SET_VCPU_EVENTS to reinject it if necessary. >> >> The general logic (reinjection filter in userspace) should be generic >> enough, but all the details can, of course, be defined in an >> arch-specific manner. > > Well, we could always just pass a payload to user space that it passes into > an ioctl to the kernel to reinject the interrupt. But that would break > migratability as this payload wouldn't get stored in env.
Right. > > But the same thing applies to x86, right? So if just pass a struct > kvm_vcpu_events with the debug interrupt as return value, user space can > simply use that to reinject it with SET_VCPU_EVENTS. Sorry, I'm slow today: That's a proposal for booke now? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
