Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
On 14.03.2013, at 13:22, Jan Kiszka wrote:
> On 2013-03-14 13:19, Alexander Graf wrote:
>>
>> 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:ag...@suse.de]
>>>>>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>>>>>> To: Bhushan Bharat-R65777
>>>>>>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>>>>>>>> ---
>>>>>>>>> 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.
>>
Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
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:ag...@suse.de]
>>> Sent: Thursday, March 07, 2013 6:51 PM
>>> To: Bhushan Bharat-R65777
>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>>> ---
>>>> 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_BP0x0001
>>>> +#define KVM_GUESTDBG_USE_HW_BP0x0002
>>>
>>> You only need
>>>
>>> #define KVM_GUESTDBG_HW_BP 0x0001
>>>
>>> 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).
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-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
On 2013-03-14 13:19, Alexander Graf wrote:
>
> 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:ag...@suse.de]
>>>>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>>>>> To: Bhushan Bharat-R65777
>>>>>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>>>>>>> ---
>>>>>>>> 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_BP0x0001
>>>>>>>> +#define KVM_GUESTDBG_USE_HW_BP0x0002
>>>>>>>
>>>>>>> You only need
>>>>>>>
>>>>>>> #defi
Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
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:ag...@suse.de]
>>>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>>>> To: Bhushan Bharat-R65777
>>>>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>>>>>> ---
>>>>>>> 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_NOTYPE0x0
>>>>>>> +#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 0x0001
>>>>>>> +#define KVM_GUESTDBG_USE_HW_BP 0x0002
>>>>>>
>>>>>> You only need
>>>>>>
>>>>>> #define KVM_GUESTDBG_HW_BP 0x0001
>>>>>>
>>>>>> 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 0x0001
>>#define KVM_GUESTDBG_USE_HW_BP 0x0002
>>
>> rather than
>>
>>#define KVM_GUESTDBG_BP_TYPE 0x0001
>>#define KVM_GUESTDBG_BP_TYPE_SW 0x0001
>>#define KVM_GUESTDBG_BP_TYPE_HW 0x
>>
>> :)
>
> 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-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
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:ag...@suse.de]
>>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>>> To: Bhushan Bharat-R65777
>>>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>>>>> ---
>>>>>> 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 0x0001
>>>>>> +#define KVM_GUESTDBG_USE_HW_BP 0x0002
>>>>>
>>>>> You only need
>>>>>
>>>>> #define KVM_GUESTDBG_HW_BP 0x0001
>>>>>
>>>>> 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 0x0001
> #define KVM_GUESTDBG_USE_HW_BP 0x0002
>
> rather than
>
> #define KVM_GUESTDBG_BP_TYPE 0x0001
> #define KVM_GUESTDBG_BP_TYPE_SW 0x0001
> #define KVM_GUESTDBG_BP_TYPE_HW 0x
>
> :)
Those bits enable or disable the features separately. You may also leave
both off if you like (and just use single stepping).
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-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
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:ag...@suse.de]
>>>> Sent: Thursday, March 07, 2013 6:51 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>>>> ---
>>>>> 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 0x0001
>>>>> +#define KVM_GUESTDBG_USE_HW_BP 0x0002
>>>>
>>>> You only need
>>>>
>>>> #define KVM_GUESTDBG_HW_BP 0x0001
>>>>
>>>> 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 0x0001
#define KVM_GUESTDBG_USE_HW_BP 0x0002
rather than
#define KVM_GUESTDBG_BP_TYPE0x0001
#define KVM_GUESTDBG_BP_TYPE_SW 0x0001
#define KVM_GUESTDBG_BP_TYPE_HW 0x
:)
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
On 14.03.2013, at 05:42, Bhushan Bharat-R65777 wrote:
>
>
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de]
>> Sent: Thursday, March 07, 2013 6:51 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
>>> ---
>>> 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_NOTYPE0x0
>>> +#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 0x0001
>>> +#define KVM_GUESTDBG_USE_HW_BP 0x0002
>>
>> You only need
>>
>> #define KVM_GUESTDBG_HW_BP 0x0001
>>
>> 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?
Alex
>
> If you want than I can code this as you described.
>
> -Bharat
>
>>
>>
>> Alex
>>
>>> +
>>> /* definition of registers in kvm_run */ struct kvm_sync_regs { };
>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>> index 975a401..cb85d73 100644
>>> --- a/arch/powerpc/kvm/book3s.c
>>> +++ b/arch/powerpc/kvm/book3s.c
>>> @@ -613,6 +613,12 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
>>> *vcpu,
>>> return 0;
>>> }
>>>
>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> + struct kvm_guest_debug *dbg)
>>> +{
>>> + return -EINVAL;
>>> +}
>>> +
>>> void kvmppc_decrementer_func(unsigned long data) {
>>> struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data; diff --git
>>> a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
>>> a41cd6d..1de93a8 100644
>>> --- a/arch/powerpc/kvm/booke.c
>>> +++ b/arch/powerpc/kvm/booke.c
>>> @@ -1527,6 +1527,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
>> struct kvm_one_reg *reg)
>>> return r;
>>> }
>>>
>>> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> +
RE: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, March 07, 2013 6:51 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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
> > ---
> > 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_NOTYPE0x0
> > +#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 0x0001
> > +#define KVM_GUESTDBG_USE_HW_BP 0x0002
>
> You only need
>
> #define KVM_GUESTDBG_HW_BP 0x0001
>
> 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.
If you want than I can code this as you described.
-Bharat
>
>
> Alex
>
> > +
> > /* definition of registers in kvm_run */ struct kvm_sync_regs { };
> > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> > index 975a401..cb85d73 100644
> > --- a/arch/powerpc/kvm/book3s.c
> > +++ b/arch/powerpc/kvm/book3s.c
> > @@ -613,6 +613,12 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
> > *vcpu,
> > return 0;
> > }
> >
> > +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > + struct kvm_guest_debug *dbg)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > void kvmppc_decrementer_func(unsigned long data) {
> > struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data; diff --git
> > a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > a41cd6d..1de93a8 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -1527,6 +1527,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
> struct kvm_one_reg *reg)
> > return r;
> > }
> >
> > +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > +struct kvm_guest_debug *dbg)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu
> > *fpu) {
> > return -ENOTSUPP;
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 934413c..4c94ca9 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> > #endif }
> >
> > -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > -struct kvm_guest_debug *dbg)
> > -{
> > - return -EINVAL;
> > -}
> > -
> > static void kvmppc_complete_dcr_load(struct kvm_vcpu *vcpu,
> > struct kvm_run *run) {
> > --
> > 1.7.0.4
> >
> >
>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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
> ---
> 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 0x0001
> +#define KVM_GUESTDBG_USE_HW_BP 0x0002
You only need
#define KVM_GUESTDBG_HW_BP 0x0001
In absence of the flag, it's a SW breakpoint.
Alex
> +
> /* definition of registers in kvm_run */
> struct kvm_sync_regs {
> };
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 975a401..cb85d73 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -613,6 +613,12 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> + struct kvm_guest_debug *dbg)
> +{
> + return -EINVAL;
> +}
> +
> void kvmppc_decrementer_func(unsigned long data)
> {
> struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data;
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index a41cd6d..1de93a8 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -1527,6 +1527,12 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
> struct kvm_one_reg *reg)
> return r;
> }
>
> +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> + struct kvm_guest_debug *dbg)
> +{
> + return -EINVAL;
> +}
> +
> int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
> {
> return -ENOTSUPP;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 934413c..4c94ca9 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> #endif
> }
>
> -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> -struct kvm_guest_debug *dbg)
> -{
> - return -EINVAL;
> -}
> -
> static void kvmppc_complete_dcr_load(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> {
> --
> 1.7.0.4
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
