Re: [PATCH 1/4] kvmppc: guest debug definitions

[Alexander Graf <agraf-l3A5Bk7waGM@public.gmane.org>]

Liu Yu-B13201 wrote:
>  
>
>   
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf-l3A5Bk7waGM@public.gmane.org] 
>> Sent: Wednesday, February 03, 2010 5:51 PM
>> To: Liu Yu-B13201
>> Cc: hollis-yUx37fBWTUITNcAmw9vGhQ@public.gmane.org; kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
>> kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
>>
>> Liu Yu-B13201 wrote:
>>     
>>>  
>>>
>>>   
>>>       
>>>> -----Original Message-----
>>>> From: kvm-ppc-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
>>>> [mailto:kvm-ppc-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Alexander Graf
>>>> Sent: Wednesday, February 03, 2010 4:57 PM
>>>> To: Liu Yu-B13201
>>>> Cc: hollis-yUx37fBWTUITNcAmw9vGhQ@public.gmane.org; kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
>>>> kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Liu Yu-B13201
>>>> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
>>>>
>>>>
>>>> Am 03.02.2010 um 08:53 schrieb Liu Yu <yu.liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org>:
>>>>
>>>>     
>>>>         
>>>>> Signed-off-by: Liu Yu <yu.liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>>>> ---
>>>>> arch/powerpc/include/asm/kvm.h      |   20 ++++++++++++++++++++
>>>>> arch/powerpc/include/asm/kvm_host.h |   16 ++++++++++++++++
>>>>> 2 files changed, 36 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/include/asm/kvm.h 
>>>>>           
>> b/arch/powerpc/include/ 
>>     
>>>>> asm/kvm.h
>>>>> index 81f3b0b..b7f7861 100644
>>>>> --- a/arch/powerpc/include/asm/kvm.h
>>>>> +++ b/arch/powerpc/include/asm/kvm.h
>>>>> @@ -22,6 +22,9 @@
>>>>>
>>>>> #include <linux/types.h>
>>>>>
>>>>> +/* Select powerpc specific features in <linux/kvm.h> */
>>>>> +#define __KVM_HAVE_GUEST_DEBUG
>>>>> +
>>>>> struct kvm_regs {
>>>>>    __u64 pc;
>>>>>    __u64 cr;
>>>>> @@ -71,10 +74,27 @@ struct kvm_fpu {
>>>>> };
>>>>>
>>>>> struct kvm_debug_exit_arch {
>>>>> +    __u32 exception;
>>>>> +    __u32 pc;
>>>>> +    __u32 status;
>>>>> };
>>>>>
>>>>> +#define KVM_INST_GUESTGDB               0x44000022
>>>>>       
>>>>>           
>>>> What instruction is this again? :) Is it something reserved for  
>>>> purposes like this?
>>>>
>>>>     
>>>>         
>>> Just an invalid instruction which can generate program interrupt...
>>> I'm open to it's value btw.
>>>   
>>>       
>> Well this definitely doesn't generate a program interrupt. Or at least
>> it shouldn't :-).
>> I just remembered where I've seen an opcode like this before. 
>> This is a
>> part of a dump of arch/powerpc/boot/ps3-hvcall.o
>>
>> 00000000 <lv1_get_logical_ppe_id>:
>>    0:   7c 08 02 a6     mflr    r0
>>    4:   90 01 00 04     stw     r0,4(r1)
>>    8:   94 21 ff f0     stwu    r1,-16(r1)
>>    c:   90 61 00 08     stw     r3,8(r1)
>>   10:   39 60 00 45     li      r11,69
>>   14:   44 00 00 22     sc      1
>>
>> So as you can see, this is the hypercall instruction for lv1. 
>> IIRC beat
>> uses the same. I don't think we want to reuse that opcode for 
>> ourselves.
>> Maybe one day someone figures it's a good idea to implement a 
>> beat-style
>> ABI in KVM.
>>
>> But IIRC sc can take a lot of values, so we can just take sc 0x1234 or
>> so :-).
>>
>>     
>>>>> +
>>>>> +#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_SET_GUEST_DEBUG */
>>>>> struct kvm_guest_debug_arch {
>>>>> +    struct {
>>>>> +        __u32 addr;
>>>>> +        __u32 type;
>>>>> +    } bp[6];
>>>>>       
>>>>>           
>>>> I can't look up the sources right now. Is this a struct that 
>>>> 1:1 maps  
>>>> to an ioctl struct? If so, we should add padding for a 
>>>> possible future  
>>>> extension of debug registers.
>>>>     
>>>>         
>>> Yes it's used by ioctl.
>>> What's the usually pad size?
>>>   
>>>       
>> I don't think there's a default. I just tend to pad it to something
>> reasonable. I guess in this case we can even just extend bp to 128
>> entries, add a reasonable amount of churn to the debug info 
>> and be good:
>>
>> struct kvm_guest_debug_arch {
>>     struct {
>>         __u64 addr;
>>         __u32 type;
>>         __u32 pad1;
>>         __u64 pad2;
>>     } bp[128];
>> }
>>
>>     
>
> Software breakpoint is maintained by qemu.
> Here it's only used by hardware breakpoint/watchpoint
> Is 128 much too large?
>   

Well, it's only 3kb. And that way we're _really_ future-proof ;-).

Remember, this is only the interface to userspace. The data we keep
around in the kernel can be much smaller.

Alex