From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined Date: Thu, 14 Mar 2013 13:13:10 +0100 Message-ID: <5141BED6.4070806@siemens.com> References: <1362024796-4237-1-git-send-email-bharat.bhushan@freescale.com> <1362024796-4237-4-git-send-email-bharat.bhushan@freescale.com> <6A3DF150A5B70D4F9B66A25E3F7C888D065EBAA6@039-SN2MPN1-023.039d.mgd.msft.net> <5141BB20.7040908@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Bhushan Bharat-R65777 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org list:Overall" , Wood Scott-B07421 To: Alexander Graf Return-path: In-Reply-To: Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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@suse.de] >>>>> Sent: Thursday, March 07, 2013 6:51 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: kvm-ppc@vger.kernel.org; kvm@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 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). Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Date: Thu, 14 Mar 2013 12:13:10 +0000 Subject: Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined Message-Id: <5141BED6.4070806@siemens.com> List-Id: References: <1362024796-4237-1-git-send-email-bharat.bhushan@freescale.com> <1362024796-4237-4-git-send-email-bharat.bhushan@freescale.com> <6A3DF150A5B70D4F9B66A25E3F7C888D065EBAA6@039-SN2MPN1-023.039d.mgd.msft.net> <5141BB20.7040908@siemens.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alexander Graf Cc: Bhushan Bharat-R65777 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org list:Overall" , Wood Scott-B07421 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@suse.de] >>>>> Sent: Thursday, March 07, 2013 6:51 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: kvm-ppc@vger.kernel.org; kvm@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 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). Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux