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:[email protected]]
>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: [email protected]; [email protected]; 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 <[email protected]>
>>>>> ---
>>>>> 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
:)
Alex
--
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
