Patchwork [4/6] KVM: PPC: debug stub interface parameter defined

login
register
mail settings
Submitter Bharat Bhushan
Date Aug. 21, 2012, 1:51 p.m.
Message ID <1345557120-16197-5-git-send-email-Bharat.Bhushan@freescale.com>
Download mbox | patch
Permalink /patch/179069/
State New
Headers show

Comments

Bharat Bhushan - Aug. 21, 2012, 1:51 p.m.
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.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
 arch/powerpc/include/asm/kvm.h |   33 +++++++++++++++++++++++++++++++++
 arch/powerpc/kvm/book3s.c      |    6 ++++++
 arch/powerpc/kvm/booke.c       |    6 ++++++
 arch/powerpc/kvm/powerpc.c     |    6 ------
 4 files changed, 45 insertions(+), 6 deletions(-)
Alexander Graf - Sept. 24, 2012, 3:38 p.m.
On 21.08.2012, at 15:51, 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.
> 
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> arch/powerpc/include/asm/kvm.h |   33 +++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s.c      |    6 ++++++
> arch/powerpc/kvm/booke.c       |    6 ++++++
> arch/powerpc/kvm/powerpc.c     |    6 ------
> 4 files changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
> index 3c14202..61b197e 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -269,8 +269,41 @@ 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 pad1;

Why the padding?

> +		__u64 pad2;
> +	} bp[16];

Why 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
> +
> +/* When setting software breakpoint, Change the software breakpoint
> + * instruction to special trap instruction and set KVM_GUESTDBG_USE_SW_BP
> + * flag in kvm_guest_debug->control. KVM does keep track of software
> + * breakpoints. So when KVM_GUESTDBG_USE_SW_BP flag is set and special trap
> + * instruction is executed by guest then exit to userspace.
> + * NOTE: A Nice interface can be added to get the special trap instruction.
> + */
> +#define KVMPPC_INST_GUEST_GDB		0x7C00021C	/* ehpriv OC=0 */

This definitely has to be passed to user space (which writes that instruction into guest phys memory). Other PPC subarchs will use different instructions. Just model it as a read-only ONE_REG.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan - Oct. 4, 2012, 7:14 a.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Monday, September 24, 2012 9:09 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/6] KVM: PPC: debug stub interface parameter defined
> 
> 
> On 21.08.2012, at 15:51, 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.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > ---
> > arch/powerpc/include/asm/kvm.h |   33 +++++++++++++++++++++++++++++++++
> > arch/powerpc/kvm/book3s.c      |    6 ++++++
> > arch/powerpc/kvm/booke.c       |    6 ++++++
> > arch/powerpc/kvm/powerpc.c     |    6 ------
> > 4 files changed, 45 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm.h
> > b/arch/powerpc/include/asm/kvm.h index 3c14202..61b197e 100644
> > --- a/arch/powerpc/include/asm/kvm.h
> > +++ b/arch/powerpc/include/asm/kvm.h
> > @@ -269,8 +269,41 @@ 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 pad1;
> 
> Why the padding?

Not sure why, I will remove this.

> 
> > +		__u64 pad2;
> > +	} bp[16];
> 
> Why 16?

I think for now 6 (4 iac + 2 dac) is sufficient for BOOKE. We kept 16 to have some room for future and other platforms.

Thanks
-Bharat
> 
> > };
> >
> > +/* 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
> > +
> > +/* When setting software breakpoint, Change the software breakpoint
> > + * instruction to special trap instruction and set
> > +KVM_GUESTDBG_USE_SW_BP
> > + * flag in kvm_guest_debug->control. KVM does keep track of software
> > + * breakpoints. So when KVM_GUESTDBG_USE_SW_BP flag is set and
> > +special trap
> > + * instruction is executed by guest then exit to userspace.
> > + * NOTE: A Nice interface can be added to get the special trap instruction.
> > + */
> > +#define KVMPPC_INST_GUEST_GDB		0x7C00021C	/* ehpriv OC=0 */
> 
> This definitely has to be passed to user space (which writes that instruction
> into guest phys memory). Other PPC subarchs will use different instructions.
> Just model it as a read-only ONE_REG.
> 
> 
> Alex
> 


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bharat Bhushan - Oct. 4, 2012, 7:37 a.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Monday, September 24, 2012 9:09 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/6] KVM: PPC: debug stub interface parameter defined
> 
> 
> On 21.08.2012, at 15:51, 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.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > ---
> > arch/powerpc/include/asm/kvm.h |   33 +++++++++++++++++++++++++++++++++
> > arch/powerpc/kvm/book3s.c      |    6 ++++++
> > arch/powerpc/kvm/booke.c       |    6 ++++++
> > arch/powerpc/kvm/powerpc.c     |    6 ------
> > 4 files changed, 45 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm.h
> > b/arch/powerpc/include/asm/kvm.h index 3c14202..61b197e 100644
> > --- a/arch/powerpc/include/asm/kvm.h
> > +++ b/arch/powerpc/include/asm/kvm.h
> > @@ -269,8 +269,41 @@ 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 pad1;
> 
> Why the padding?
> 
> > +		__u64 pad2;
> > +	} bp[16];
> 
> Why 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
> > +
> > +/* When setting software breakpoint, Change the software breakpoint
> > + * instruction to special trap instruction and set
> > +KVM_GUESTDBG_USE_SW_BP
> > + * flag in kvm_guest_debug->control. KVM does keep track of software
> > + * breakpoints. So when KVM_GUESTDBG_USE_SW_BP flag is set and
> > +special trap
> > + * instruction is executed by guest then exit to userspace.
> > + * NOTE: A Nice interface can be added to get the special trap instruction.
> > + */
> > +#define KVMPPC_INST_GUEST_GDB		0x7C00021C	/* ehpriv OC=0 */
> 
> This definitely has to be passed to user space (which writes that instruction
> into guest phys memory). Other PPC subarchs will use different instructions.
> Just model it as a read-only ONE_REG.

Ok.

Thanks
-Bharat


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - Dec. 17, 2012, 4:56 p.m.
On 04.10.2012, at 09:14, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Monday, September 24, 2012 9:09 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
>> Subject: Re: [PATCH 4/6] KVM: PPC: debug stub interface parameter defined
>> 
>> 
>> On 21.08.2012, at 15:51, 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.
>>> 
>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
>>> ---
>>> arch/powerpc/include/asm/kvm.h |   33 +++++++++++++++++++++++++++++++++
>>> arch/powerpc/kvm/book3s.c      |    6 ++++++
>>> arch/powerpc/kvm/booke.c       |    6 ++++++
>>> arch/powerpc/kvm/powerpc.c     |    6 ------
>>> 4 files changed, 45 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/arch/powerpc/include/asm/kvm.h
>>> b/arch/powerpc/include/asm/kvm.h index 3c14202..61b197e 100644
>>> --- a/arch/powerpc/include/asm/kvm.h
>>> +++ b/arch/powerpc/include/asm/kvm.h
>>> @@ -269,8 +269,41 @@ 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 pad1;
>> 
>> Why the padding?
> 
> Not sure why, I will remove this.

Oh, I think the padding makes sense - depending on the type of interrupt this could for example mean "break when write to address x becomes value y".

I'm not sure the amount of padding and the structure of it is the right way to go though: We probably want to have a union here defining type specific payloads.

> 
>> 
>>> +		__u64 pad2;
>>> +	} bp[16];
>> 
>> Why 16?
> 
> I think for now 6 (4 iac + 2 dac) is sufficient for BOOKE. We kept 16 to have some room for future and other platforms.

Ok :)


Alex

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

Patch

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 3c14202..61b197e 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -269,8 +269,41 @@  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 pad1;
+		__u64 pad2;
+	} 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
+
+/* When setting software breakpoint, Change the software breakpoint
+ * instruction to special trap instruction and set KVM_GUESTDBG_USE_SW_BP
+ * flag in kvm_guest_debug->control. KVM does keep track of software
+ * breakpoints. So when KVM_GUESTDBG_USE_SW_BP flag is set and special trap
+ * instruction is executed by guest then exit to userspace.
+ * NOTE: A Nice interface can be added to get the special trap instruction.
+ */
+#define KVMPPC_INST_GUEST_GDB		0x7C00021C	/* ehpriv OC=0 */
+
 /* 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 e946665..385b027 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -475,6 +475,12 @@  int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 	return 0;
 }
 
+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/booke.c b/arch/powerpc/kvm/booke.c
index 90d4798..dd0e5b8 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1403,6 +1403,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 32d217c..c565f5d 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -518,12 +518,6 @@  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 	vcpu->cpu = -1;
 }
 
-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)
 {