Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14.03.2013, at 13:13, Jan Kiszka wrote:

> On 2013-03-14 13:09, Alexander Graf wrote:
>> 
>> On 14.03.2013, at 12:57, Jan Kiszka wrote:
>> 
>>> On 2013-03-14 12:54, Alexander Graf wrote:
>>>> 
>>>> On 14.03.2013, at 05:42, Bhushan Bharat-R65777 wrote:
>>>> 
>>>>> 
>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Alexander Graf [mailto:agraf@xxxxxxx]
>>>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>>>> To: Bhushan Bharat-R65777
>>>>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Wood Scott-B07421; Bhushan
>>>>>> Bharat-R65777
>>>>>> Subject: Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
>>>>>> 
>>>>>> 
>>>>>> On 28.02.2013, at 05:13, Bharat Bhushan wrote:
>>>>>> 
>>>>>>> This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
>>>>>>> ioctl support. Follow up patches will use this for setting up hardware
>>>>>>> breakpoints, watchpoints and software breakpoints.
>>>>>>> 
>>>>>>> Also kvm_arch_vcpu_ioctl_set_guest_debug() is brought one level below.
>>>>>>> This is because I am not sure what is required for book3s. So this
>>>>>>> ioctl behaviour will not change for book3s.
>>>>>>> 
>>>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
>>>>>>> ---
>>>>>>> arch/powerpc/include/uapi/asm/kvm.h |   23 +++++++++++++++++++++++
>>>>>>> arch/powerpc/kvm/book3s.c           |    6 ++++++
>>>>>>> arch/powerpc/kvm/booke.c            |    6 ++++++
>>>>>>> arch/powerpc/kvm/powerpc.c          |    6 ------
>>>>>>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> index c2ff99c..15f9a00 100644
>>>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> @@ -272,8 +272,31 @@ struct kvm_debug_exit_arch {
>>>>>>> 
>>>>>>> /* for KVM_SET_GUEST_DEBUG */
>>>>>>> struct kvm_guest_debug_arch {
>>>>>>> +	struct {
>>>>>>> +		/* H/W breakpoint/watchpoint address */
>>>>>>> +		__u64 addr;
>>>>>>> +		/*
>>>>>>> +		 * Type denotes h/w breakpoint, read watchpoint, write
>>>>>>> +		 * watchpoint or watchpoint (both read and write).
>>>>>>> +		 */
>>>>>>> +#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)
>>>>>>> +		__u32 type;
>>>>>>> +		__u32 reserved;
>>>>>>> +	} bp[16];
>>>>>>> };
>>>>>>> 
>>>>>>> +/* Debug related defines */
>>>>>>> +/*
>>>>>>> + * kvm_guest_debug->control is a 32 bit field. The lower 16 bits are
>>>>>>> +generic
>>>>>>> + * and upper 16 bits are architecture specific. Architecture specific
>>>>>>> +defines
>>>>>>> + * that ioctl is for setting hardware breakpoint or software breakpoint.
>>>>>>> + */
>>>>>>> +#define KVM_GUESTDBG_USE_SW_BP		0x00010000
>>>>>>> +#define KVM_GUESTDBG_USE_HW_BP		0x00020000
>>>>>> 
>>>>>> You only need
>>>>>> 
>>>>>> #define KVM_GUESTDBG_HW_BP 0x00010000
>>>>>> 
>>>>>> In absence of the flag, it's a SW breakpoint.
>>>>> 
>>>>> We kept this for 2 reasons; 1) Same logic is applied for i386, so trying to keep consistent 2) better clarity.
>>>> 
>>>> Jan, was there any special reason to have 2 flags for HW/SW breakpoint on x86 rather than one bit that indicates which one is used?
>>> 
>>> Different mechanics on x86: HW goes via debug registers and shows up as
>>> INT1, SW is INT3 (plus guest patching done by user land).
>> 
>> Well, the same thing goes for us. What I'm asking is whether there is a specific reason (extensibility, oversight, taste, ...) that you did
>> 
>>    #define KVM_GUESTDBG_USE_SW_BP               0x00010000
>>    #define KVM_GUESTDBG_USE_HW_BP               0x00020000
>> 
>> rather than
>> 
>>    #define KVM_GUESTDBG_BP_TYPE			0x00010000
>>    #define KVM_GUESTDBG_BP_TYPE_SW		0x00010000
>>    #define KVM_GUESTDBG_BP_TYPE_HW		0x00000000
>> 
>> :)
> 
> Those bits enable or disable the features separately. You may also leave
> both off if you like (and just use single stepping).

Ah, so these are global configuration bits, not per-breakpoint configuration?


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux