Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-02-10 Thread Paul Mackerras 
On Thu, Feb 07, 2013 at 03:29:50PM +0100, Alexander Graf wrote:
 
 On 04.02.2013, at 01:41, Paul Mackerras wrote:
 
  On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote:
  This patch adds the one_reg interface to get the special instruction
  to be used for setting software breakpoint from userspace.
  
  Since this presumably is constant for any given platform, wouldn't
  a capability be more appropriate?
 
 How so? A capability only tells you I can do debug. Or I can do debug on 
 e500. I don't want to reteach QEMU how to do debug for every core we 
 implement.

Capabilities aren't just binary - the get-capability (check_extension)
ioctl returns a 32-bit value, and we already have some that return
values other than 0 or 1.  So I was thinking that we could add a
capability which when queried returns the special instruction.  It's
up to you whether you want to do it that way or not.

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

Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-02-10 Thread Paul Mackerras 
On Thu, Feb 07, 2013 at 03:29:50PM +0100, Alexander Graf wrote:
 
 On 04.02.2013, at 01:41, Paul Mackerras wrote:
 
  On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote:
  This patch adds the one_reg interface to get the special instruction
  to be used for setting software breakpoint from userspace.
  
  Since this presumably is constant for any given platform, wouldn't
  a capability be more appropriate?
 
 How so? A capability only tells you I can do debug. Or I can do debug on 
 e500. I don't want to reteach QEMU how to do debug for every core we 
 implement.

Capabilities aren't just binary - the get-capability (check_extension)
ioctl returns a 32-bit value, and we already have some that return
values other than 0 or 1.  So I was thinking that we could add a
capability which when queried returns the special instruction.  It's
up to you whether you want to do it that way or not.

Paul.
--
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 4/8] Added ONE_REG interface for debug instruction

2013-02-07 Thread Alexander Graf 

On 04.02.2013, at 01:41, Paul Mackerras wrote:

 On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote:
 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.
 
 Since this presumably is constant for any given platform, wouldn't
 a capability be more appropriate?

How so? A capability only tells you I can do debug. Or I can do debug on 
e500. I don't want to reteach QEMU how to do debug for every core we implement.


Alex

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

Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-02-03 Thread Paul Mackerras 
On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote:
 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.

Since this presumably is constant for any given platform, wouldn't
a capability be more appropriate?

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

Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-02-03 Thread Paul Mackerras 
On Wed, Jan 16, 2013 at 01:54:41PM +0530, Bharat Bhushan wrote:
 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.

Since this presumably is constant for any given platform, wouldn't
a capability be more appropriate?

Paul.
--
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 4/8] Added ONE_REG interface for debug instruction

2013-01-31 Thread Bhushan Bharat-R65777 


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:18 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  This patch adds the one_reg interface to get the special instruction
  to be used for setting software breakpoint from userspace.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  Documentation/virtual/kvm/api.txt   |1 +
  arch/powerpc/include/asm/kvm_ppc.h  |1 +
  arch/powerpc/include/uapi/asm/kvm.h |3 +++
  arch/powerpc/kvm/44x.c  |5 +
  arch/powerpc/kvm/booke.c|   10 ++
  arch/powerpc/kvm/e500.c |5 +
  arch/powerpc/kvm/e500.h |9 +
  arch/powerpc/kvm/e500mc.c   |5 +
  8 files changed, 39 insertions(+), 0 deletions(-)
 
  diff --git a/Documentation/virtual/kvm/api.txt
  b/Documentation/virtual/kvm/api.txt
  index 09905cb..7e8be9e 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -1775,6 +1775,7 @@ registers, find a list below:
PPC   | KVM_REG_PPC_VPA_DTL   | 128
PPC   | KVM_REG_PPC_EPCR  | 32
PPC   | KVM_REG_PPC_EPR   | 32
  +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
  4.69 KVM_GET_ONE_REG
 
  diff --git a/arch/powerpc/include/asm/kvm_ppc.h
  b/arch/powerpc/include/asm/kvm_ppc.h
  index 44a657a..b3c481e 100644
  --- a/arch/powerpc/include/asm/kvm_ppc.h
  +++ b/arch/powerpc/include/asm/kvm_ppc.h
  @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct
  kvm_sregs *sregs);
  +u32 kvmppc_core_debug_inst_op(void);
 
  void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct
  kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h
  b/arch/powerpc/include/uapi/asm/kvm.h
  index 16064d0..e81ae5b 100644
  --- a/arch/powerpc/include/uapi/asm/kvm.h
  +++ b/arch/powerpc/include/uapi/asm/kvm.h
  @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
  #define KVM_REG_PPC_EPCR(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
  #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
  +/* Debugging: Special instruction for software breakpoint */ #define
  +KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87)
  +
  #endif /* __LINUX_KVM_POWERPC_H */
  diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index
  3d7fd21..41501be 100644
  --- a/arch/powerpc/kvm/44x.c
  +++ b/arch/powerpc/kvm/44x.c
  @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
  return 0;
  }
 
  +u32 kvmppc_core_debug_inst_op(void)
  +{
  +   return -1;
  +}
  +
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs) {
  kvmppc_get_sregs_ivor(vcpu, sregs);
  diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
  d2f502d..453a10f 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
 
 Please provide the DEBUG_INST on a more global level - across all ppc 
 subarchs.

Do you mean defining in powerpc.c ?

We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in respective 
subarchs (booke and books have their separate handler). So how you want this to 
be defined in more common way for all subarchs?

Thanks
-Bharat

 
  @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
 struct kvm_one_reg *reg)
  r = put_user(vcpu-arch.epcr, (u32 __user *)(long)reg-addr);
  break;
  #endif
  +   case KVM_REG_PPC_DEBUG_INST: {
  +   u32 opcode = kvmppc_core_debug_inst_op();
  +   r = copy_to_user((u32 __user *)(long)reg-addr,
  +opcode, sizeof(u32));
  +   break;
  +   }
  default:
  break;
  }
  @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
 struct kvm_one_reg *reg)
  break;
  }
  #endif
  +   case KVM_REG_PPC_DEBUG_INST:
  +   /* This is read only, so write to this is nop*/
  +   r = 0;
  +   break;
 
 Just don't support set_one_reg on this reg.
 
  default:
  break;
  }
  diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index
  6dd4de7..d8a5e8e 100644
  --- a/arch/powerpc/kvm/e500.c
  +++ b/arch/powerpc/kvm/e500.c
  @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
  return 0;
  }
 
  +u32 kvmppc_core_debug_inst_op(void)
  +{
  +   return KVMPPC_INST_GUEST_GDB;
  +}
  +
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs) {
  struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); diff --git
  a/arch/powerpc/kvm/e500.h b/arch

Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-01-31 Thread Alexander Graf 

On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:18 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.
 
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 Documentation/virtual/kvm/api.txt   |1 +
 arch/powerpc/include/asm/kvm_ppc.h  |1 +
 arch/powerpc/include/uapi/asm/kvm.h |3 +++
 arch/powerpc/kvm/44x.c  |5 +
 arch/powerpc/kvm/booke.c|   10 ++
 arch/powerpc/kvm/e500.c |5 +
 arch/powerpc/kvm/e500.h |9 +
 arch/powerpc/kvm/e500mc.c   |5 +
 8 files changed, 39 insertions(+), 0 deletions(-)
 
 diff --git a/Documentation/virtual/kvm/api.txt
 b/Documentation/virtual/kvm/api.txt
 index 09905cb..7e8be9e 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -1775,6 +1775,7 @@ registers, find a list below:
  PPC   | KVM_REG_PPC_VPA_DTL   | 128
  PPC   | KVM_REG_PPC_EPCR   | 32
  PPC   | KVM_REG_PPC_EPR| 32
 +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
 4.69 KVM_GET_ONE_REG
 
 diff --git a/arch/powerpc/include/asm/kvm_ppc.h
 b/arch/powerpc/include/asm/kvm_ppc.h
 index 44a657a..b3c481e 100644
 --- a/arch/powerpc/include/asm/kvm_ppc.h
 +++ b/arch/powerpc/include/asm/kvm_ppc.h
 @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
 *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct
 kvm_sregs *sregs);
 +u32 kvmppc_core_debug_inst_op(void);
 
 void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs
 *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct
 kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h
 b/arch/powerpc/include/uapi/asm/kvm.h
 index 16064d0..e81ae5b 100644
 --- a/arch/powerpc/include/uapi/asm/kvm.h
 +++ b/arch/powerpc/include/uapi/asm/kvm.h
 @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
 #define KVM_REG_PPC_EPCR(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
 #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
 +/* Debugging: Special instruction for software breakpoint */ #define
 +KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87)
 +
 #endif /* __LINUX_KVM_POWERPC_H */
 diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index
 3d7fd21..41501be 100644
 --- a/arch/powerpc/kvm/44x.c
 +++ b/arch/powerpc/kvm/44x.c
 @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
 return 0;
 }
 
 +u32 kvmppc_core_debug_inst_op(void)
 +{
 +   return -1;

The way you handle it here this needs to be an  int 
kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I 
don't think it's worth to worry about a case where we don't know about the inst 
though. Just return the same as what we use on e500v2 here.

 +}
 +
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
 *sregs) {
 kvmppc_get_sregs_ivor(vcpu, sregs);
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
 d2f502d..453a10f 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 
 Please provide the DEBUG_INST on a more global level - across all ppc 
 subarchs.
 
 Do you mean defining in powerpc.c ?
 
 We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in 
 respective subarchs (booke and books have their separate handler). So how you 
 want this to be defined in more common way for all subarchs?

Just add it to all subarch's one_reg handlers.


Alex

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

RE: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-01-31 Thread Bhushan Bharat-R65777 


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 11:23 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
 
 
 On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:18 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug
  instruction
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  This patch adds the one_reg interface to get the special instruction
  to be used for setting software breakpoint from userspace.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  Documentation/virtual/kvm/api.txt   |1 +
  arch/powerpc/include/asm/kvm_ppc.h  |1 +
  arch/powerpc/include/uapi/asm/kvm.h |3 +++
  arch/powerpc/kvm/44x.c  |5 +
  arch/powerpc/kvm/booke.c|   10 ++
  arch/powerpc/kvm/e500.c |5 +
  arch/powerpc/kvm/e500.h |9 +
  arch/powerpc/kvm/e500mc.c   |5 +
  8 files changed, 39 insertions(+), 0 deletions(-)
 
  diff --git a/Documentation/virtual/kvm/api.txt
  b/Documentation/virtual/kvm/api.txt
  index 09905cb..7e8be9e 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -1775,6 +1775,7 @@ registers, find a list below:
   PPC   | KVM_REG_PPC_VPA_DTL   | 128
   PPC   | KVM_REG_PPC_EPCR | 32
   PPC   | KVM_REG_PPC_EPR  | 32
  +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
  4.69 KVM_GET_ONE_REG
 
  diff --git a/arch/powerpc/include/asm/kvm_ppc.h
  b/arch/powerpc/include/asm/kvm_ppc.h
  index 44a657a..b3c481e 100644
  --- a/arch/powerpc/include/asm/kvm_ppc.h
  +++ b/arch/powerpc/include/asm/kvm_ppc.h
  @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct
  kvm_sregs *sregs);
  +u32 kvmppc_core_debug_inst_op(void);
 
  void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct
  kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h
  b/arch/powerpc/include/uapi/asm/kvm.h
  index 16064d0..e81ae5b 100644
  --- a/arch/powerpc/include/uapi/asm/kvm.h
  +++ b/arch/powerpc/include/uapi/asm/kvm.h
  @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
  #define KVM_REG_PPC_EPCR  (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
  #define KVM_REG_PPC_EPR   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
  +/* Debugging: Special instruction for software breakpoint */
  +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 |
  +0x87)
  +
  #endif /* __LINUX_KVM_POWERPC_H */
  diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index
  3d7fd21..41501be 100644
  --- a/arch/powerpc/kvm/44x.c
  +++ b/arch/powerpc/kvm/44x.c
  @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
return 0;
  }
 
  +u32 kvmppc_core_debug_inst_op(void) {
  + return -1;
 
 The way you handle it here this needs to be an  int
 kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I 
 don't
 think it's worth to worry about a case where we don't know about the inst
 though. Just return the same as what we use on e500v2 here.
 
  +}
  +
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs) {
kvmppc_get_sregs_ivor(vcpu, sregs); diff --git
  a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
  d2f502d..453a10f 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
 
  Please provide the DEBUG_INST on a more global level - across all ppc
 subarchs.
 
  Do you mean defining in powerpc.c ?
 
  We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in
 respective subarchs (booke and books have their separate handler). So how you
 want this to be defined in more common way for all subarchs?
 
 Just add it to all subarch's one_reg handlers.

And what book3s etc should return?

-1 ? 

Thanks
-Bharat

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

Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-01-31 Thread Alexander Graf 

On 31.01.2013, at 18:58, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 11:23 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
 
 
 On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:18 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug
 instruction
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.
 
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 Documentation/virtual/kvm/api.txt   |1 +
 arch/powerpc/include/asm/kvm_ppc.h  |1 +
 arch/powerpc/include/uapi/asm/kvm.h |3 +++
 arch/powerpc/kvm/44x.c  |5 +
 arch/powerpc/kvm/booke.c|   10 ++
 arch/powerpc/kvm/e500.c |5 +
 arch/powerpc/kvm/e500.h |9 +
 arch/powerpc/kvm/e500mc.c   |5 +
 8 files changed, 39 insertions(+), 0 deletions(-)
 
 diff --git a/Documentation/virtual/kvm/api.txt
 b/Documentation/virtual/kvm/api.txt
 index 09905cb..7e8be9e 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -1775,6 +1775,7 @@ registers, find a list below:
 PPC   | KVM_REG_PPC_VPA_DTL   | 128
 PPC   | KVM_REG_PPC_EPCR  | 32
 PPC   | KVM_REG_PPC_EPR   | 32
 +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
 4.69 KVM_GET_ONE_REG
 
 diff --git a/arch/powerpc/include/asm/kvm_ppc.h
 b/arch/powerpc/include/asm/kvm_ppc.h
 index 44a657a..b3c481e 100644
 --- a/arch/powerpc/include/asm/kvm_ppc.h
 +++ b/arch/powerpc/include/asm/kvm_ppc.h
 @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
 *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct
 kvm_sregs *sregs);
 +u32 kvmppc_core_debug_inst_op(void);
 
 void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs
 *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct
 kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h
 b/arch/powerpc/include/uapi/asm/kvm.h
 index 16064d0..e81ae5b 100644
 --- a/arch/powerpc/include/uapi/asm/kvm.h
 +++ b/arch/powerpc/include/uapi/asm/kvm.h
 @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
 #define KVM_REG_PPC_EPCR  (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
 #define KVM_REG_PPC_EPR   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
 +/* Debugging: Special instruction for software breakpoint */
 +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 |
 +0x87)
 +
 #endif /* __LINUX_KVM_POWERPC_H */
 diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index
 3d7fd21..41501be 100644
 --- a/arch/powerpc/kvm/44x.c
 +++ b/arch/powerpc/kvm/44x.c
 @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
   return 0;
 }
 
 +u32 kvmppc_core_debug_inst_op(void) {
 + return -1;
 
 The way you handle it here this needs to be an  int
 kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I 
 don't
 think it's worth to worry about a case where we don't know about the inst
 though. Just return the same as what we use on e500v2 here.
 
 +}
 +
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
 *sregs) {
   kvmppc_get_sregs_ivor(vcpu, sregs); diff --git
 a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
 d2f502d..453a10f 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 
 Please provide the DEBUG_INST on a more global level - across all ppc
 subarchs.
 
 Do you mean defining in powerpc.c ?
 
 We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in
 respective subarchs (booke and books have their separate handler). So how you
 want this to be defined in more common way for all subarchs?
 
 Just add it to all subarch's one_reg handlers.
 
 And what book3s etc should return?
 
 -1 ? 

trap maybe?


Alex

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

RE: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-01-31 Thread Bhushan Bharat-R65777 


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 11:23 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
 
 
 On 31.01.2013, at 18:44, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:18 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug
  instruction
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  This patch adds the one_reg interface to get the special instruction
  to be used for setting software breakpoint from userspace.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  Documentation/virtual/kvm/api.txt   |1 +
  arch/powerpc/include/asm/kvm_ppc.h  |1 +
  arch/powerpc/include/uapi/asm/kvm.h |3 +++
  arch/powerpc/kvm/44x.c  |5 +
  arch/powerpc/kvm/booke.c|   10 ++
  arch/powerpc/kvm/e500.c |5 +
  arch/powerpc/kvm/e500.h |9 +
  arch/powerpc/kvm/e500mc.c   |5 +
  8 files changed, 39 insertions(+), 0 deletions(-)
 
  diff --git a/Documentation/virtual/kvm/api.txt
  b/Documentation/virtual/kvm/api.txt
  index 09905cb..7e8be9e 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -1775,6 +1775,7 @@ registers, find a list below:
   PPC   | KVM_REG_PPC_VPA_DTL   | 128
   PPC   | KVM_REG_PPC_EPCR | 32
   PPC   | KVM_REG_PPC_EPR  | 32
  +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
  4.69 KVM_GET_ONE_REG
 
  diff --git a/arch/powerpc/include/asm/kvm_ppc.h
  b/arch/powerpc/include/asm/kvm_ppc.h
  index 44a657a..b3c481e 100644
  --- a/arch/powerpc/include/asm/kvm_ppc.h
  +++ b/arch/powerpc/include/asm/kvm_ppc.h
  @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct
  kvm_sregs *sregs);
  +u32 kvmppc_core_debug_inst_op(void);
 
  void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct
  kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h
  b/arch/powerpc/include/uapi/asm/kvm.h
  index 16064d0..e81ae5b 100644
  --- a/arch/powerpc/include/uapi/asm/kvm.h
  +++ b/arch/powerpc/include/uapi/asm/kvm.h
  @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
  #define KVM_REG_PPC_EPCR  (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
  #define KVM_REG_PPC_EPR   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
  +/* Debugging: Special instruction for software breakpoint */
  +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 |
  +0x87)
  +
  #endif /* __LINUX_KVM_POWERPC_H */
  diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index
  3d7fd21..41501be 100644
  --- a/arch/powerpc/kvm/44x.c
  +++ b/arch/powerpc/kvm/44x.c
  @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
return 0;
  }
 
  +u32 kvmppc_core_debug_inst_op(void) {
  + return -1;
 
 The way you handle it here this needs to be an  int
 kvmppc_core_debug_inst_op(u32 *inst) so you can return an error for 440. I 
 don't
 think it's worth to worry about a case where we don't know about the inst
 though. Just return the same as what we use on e500v2 here.
 
  +}
  +
  void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
  *sregs) {
kvmppc_get_sregs_ivor(vcpu, sregs); diff --git
  a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
  d2f502d..453a10f 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
 
  Please provide the DEBUG_INST on a more global level - across all ppc
 subarchs.
 
  Do you mean defining in powerpc.c ?
 
  We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in
 respective subarchs (booke and books have their separate handler). So how you
 want this to be defined in more common way for all subarchs?
 
 Just add it to all subarch's one_reg handlers.

And what book3s etc should return?

-1 ? 

Thanks
-Bharat

--
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 4/8] Added ONE_REG interface for debug instruction

2013-01-25 Thread Alexander Graf 

On 16.01.2013, at 09:24, Bharat Bhushan wrote:

 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.
 
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 Documentation/virtual/kvm/api.txt   |1 +
 arch/powerpc/include/asm/kvm_ppc.h  |1 +
 arch/powerpc/include/uapi/asm/kvm.h |3 +++
 arch/powerpc/kvm/44x.c  |5 +
 arch/powerpc/kvm/booke.c|   10 ++
 arch/powerpc/kvm/e500.c |5 +
 arch/powerpc/kvm/e500.h |9 +
 arch/powerpc/kvm/e500mc.c   |5 +
 8 files changed, 39 insertions(+), 0 deletions(-)
 
 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 09905cb..7e8be9e 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -1775,6 +1775,7 @@ registers, find a list below:
   PPC   | KVM_REG_PPC_VPA_DTL   | 128
   PPC   | KVM_REG_PPC_EPCR| 32
   PPC   | KVM_REG_PPC_EPR | 32
 +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
 4.69 KVM_GET_ONE_REG
 
 diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
 b/arch/powerpc/include/asm/kvm_ppc.h
 index 44a657a..b3c481e 100644
 --- a/arch/powerpc/include/asm/kvm_ppc.h
 +++ b/arch/powerpc/include/asm/kvm_ppc.h
 @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 +u32 kvmppc_core_debug_inst_op(void);
 
 void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 diff --git a/arch/powerpc/include/uapi/asm/kvm.h 
 b/arch/powerpc/include/uapi/asm/kvm.h
 index 16064d0..e81ae5b 100644
 --- a/arch/powerpc/include/uapi/asm/kvm.h
 +++ b/arch/powerpc/include/uapi/asm/kvm.h
 @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
 #define KVM_REG_PPC_EPCR  (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
 #define KVM_REG_PPC_EPR   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
 +/* Debugging: Special instruction for software breakpoint */
 +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87)
 +
 #endif /* __LINUX_KVM_POWERPC_H */
 diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c
 index 3d7fd21..41501be 100644
 --- a/arch/powerpc/kvm/44x.c
 +++ b/arch/powerpc/kvm/44x.c
 @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
   return 0;
 }
 
 +u32 kvmppc_core_debug_inst_op(void)
 +{
 + return -1;
 +}
 +
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 {
   kvmppc_get_sregs_ivor(vcpu, sregs);
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index d2f502d..453a10f 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c

Please provide the DEBUG_INST on a more global level - across all ppc subarchs.

 @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, 
 struct kvm_one_reg *reg)
   r = put_user(vcpu-arch.epcr, (u32 __user *)(long)reg-addr);
   break;
 #endif
 + case KVM_REG_PPC_DEBUG_INST: {
 + u32 opcode = kvmppc_core_debug_inst_op();
 + r = copy_to_user((u32 __user *)(long)reg-addr,
 +  opcode, sizeof(u32));
 + break;
 + }
   default:
   break;
   }
 @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, 
 struct kvm_one_reg *reg)
   break;
   }
 #endif
 + case KVM_REG_PPC_DEBUG_INST:
 + /* This is read only, so write to this is nop*/
 + r = 0;
 + break;

Just don't support set_one_reg on this reg.

   default:
   break;
   }
 diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
 index 6dd4de7..d8a5e8e 100644
 --- a/arch/powerpc/kvm/e500.c
 +++ b/arch/powerpc/kvm/e500.c
 @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
   return 0;
 }
 
 +u32 kvmppc_core_debug_inst_op(void)
 +{
 + return KVMPPC_INST_GUEST_GDB;
 +}
 +
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 {
   struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
 diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
 index c70d37e..17942d2 100644
 --- a/arch/powerpc/kvm/e500.h
 +++ b/arch/powerpc/kvm/e500.h
 @@ -302,4 +302,13 @@ static inline unsigned int get_tlbmiss_tid(struct 
 kvm_vcpu *vcpu)
 #define get_tlb_sts(gtlbe)  (MAS1_TS)
 #endif /* !BOOKE_HV */
 
 +/* When setting software breakpoint, Change the software breakpoint
 + * instruction to special trap/invalid 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

Re: [PATCH 4/8] Added ONE_REG interface for debug instruction

2013-01-25 Thread Alexander Graf 

On 16.01.2013, at 09:24, Bharat Bhushan wrote:

 This patch adds the one_reg interface to get the special instruction
 to be used for setting software breakpoint from userspace.
 
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 Documentation/virtual/kvm/api.txt   |1 +
 arch/powerpc/include/asm/kvm_ppc.h  |1 +
 arch/powerpc/include/uapi/asm/kvm.h |3 +++
 arch/powerpc/kvm/44x.c  |5 +
 arch/powerpc/kvm/booke.c|   10 ++
 arch/powerpc/kvm/e500.c |5 +
 arch/powerpc/kvm/e500.h |9 +
 arch/powerpc/kvm/e500mc.c   |5 +
 8 files changed, 39 insertions(+), 0 deletions(-)
 
 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 09905cb..7e8be9e 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -1775,6 +1775,7 @@ registers, find a list below:
   PPC   | KVM_REG_PPC_VPA_DTL   | 128
   PPC   | KVM_REG_PPC_EPCR| 32
   PPC   | KVM_REG_PPC_EPR | 32
 +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
 4.69 KVM_GET_ONE_REG
 
 diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
 b/arch/powerpc/include/asm/kvm_ppc.h
 index 44a657a..b3c481e 100644
 --- a/arch/powerpc/include/asm/kvm_ppc.h
 +++ b/arch/powerpc/include/asm/kvm_ppc.h
 @@ -235,6 +235,7 @@ union kvmppc_one_reg {
 
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 +u32 kvmppc_core_debug_inst_op(void);
 
 void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
 diff --git a/arch/powerpc/include/uapi/asm/kvm.h 
 b/arch/powerpc/include/uapi/asm/kvm.h
 index 16064d0..e81ae5b 100644
 --- a/arch/powerpc/include/uapi/asm/kvm.h
 +++ b/arch/powerpc/include/uapi/asm/kvm.h
 @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
 #define KVM_REG_PPC_EPCR  (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
 #define KVM_REG_PPC_EPR   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
 
 +/* Debugging: Special instruction for software breakpoint */
 +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87)
 +
 #endif /* __LINUX_KVM_POWERPC_H */
 diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c
 index 3d7fd21..41501be 100644
 --- a/arch/powerpc/kvm/44x.c
 +++ b/arch/powerpc/kvm/44x.c
 @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
   return 0;
 }
 
 +u32 kvmppc_core_debug_inst_op(void)
 +{
 + return -1;
 +}
 +
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 {
   kvmppc_get_sregs_ivor(vcpu, sregs);
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index d2f502d..453a10f 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c

Please provide the DEBUG_INST on a more global level - across all ppc subarchs.

 @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, 
 struct kvm_one_reg *reg)
   r = put_user(vcpu-arch.epcr, (u32 __user *)(long)reg-addr);
   break;
 #endif
 + case KVM_REG_PPC_DEBUG_INST: {
 + u32 opcode = kvmppc_core_debug_inst_op();
 + r = copy_to_user((u32 __user *)(long)reg-addr,
 +  opcode, sizeof(u32));
 + break;
 + }
   default:
   break;
   }
 @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, 
 struct kvm_one_reg *reg)
   break;
   }
 #endif
 + case KVM_REG_PPC_DEBUG_INST:
 + /* This is read only, so write to this is nop*/
 + r = 0;
 + break;

Just don't support set_one_reg on this reg.

   default:
   break;
   }
 diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
 index 6dd4de7..d8a5e8e 100644
 --- a/arch/powerpc/kvm/e500.c
 +++ b/arch/powerpc/kvm/e500.c
 @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
   return 0;
 }
 
 +u32 kvmppc_core_debug_inst_op(void)
 +{
 + return KVMPPC_INST_GUEST_GDB;
 +}
 +
 void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 {
   struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
 diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
 index c70d37e..17942d2 100644
 --- a/arch/powerpc/kvm/e500.h
 +++ b/arch/powerpc/kvm/e500.h
 @@ -302,4 +302,13 @@ static inline unsigned int get_tlbmiss_tid(struct 
 kvm_vcpu *vcpu)
 #define get_tlb_sts(gtlbe)  (MAS1_TS)
 #endif /* !BOOKE_HV */
 
 +/* When setting software breakpoint, Change the software breakpoint
 + * instruction to special trap/invalid 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