Re: [PATCH v2 3/4] target-arm: kvm - support for single step

2015-04-21 Thread Alex Bennée

Peter Maydell peter.mayd...@linaro.org writes:

 On 31 March 2015 at 16:40, Alex Bennée alex.ben...@linaro.org wrote:
 This adds support for single-step. There isn't much to do on the QEMU
 side as after we set-up the request for single step via the debug ioctl
 it is all handled within the kernel.

 Signed-off-by: Alex Bennée alex.ben...@linaro.org

 ---
 v2
   - convert to using HSR_EC

 diff --git a/target-arm/kvm.c b/target-arm/kvm.c
 index 290c1fe..ae0f8b2 100644
 --- a/target-arm/kvm.c
 +++ b/target-arm/kvm.c
 @@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
  */

  #define HSR_EC_SHIFT26
 +#define HSR_EC_SOFT_STEP0x32
  #define HSR_EC_SW_BKPT  0x3c

 We already include internals.h in this file, so can you just use
 the EC_* constants and ARM_EL_EC_SHIFT rather than defining
 new ones? (Applies for patch 1 as well.)

  static int kvm_handle_debug(CPUState *cs, struct kvm_run *run)
 @@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struct 
 kvm_run *run)
  int hsr_ec = arch_info-hsr  HSR_EC_SHIFT;

  switch (hsr_ec) {
 +case HSR_EC_SOFT_STEP:
 +if (cs-singlestep_enabled) {
 +return true;
 +} else {
 +error_report(Came out of SINGLE STEP when not enabled);

 This can only happen if there's a kernel bug, right?

Sure. Should we report it differently? abort() out?


 +}
 +break;
  case HSR_EC_SW_BKPT:
  if (kvm_find_sw_breakpoint(cs, arch_info-pc)) {
  return true;
 @@ -542,6 +550,9 @@ int kvm_arch_on_sigbus(int code, void *addr)

  void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
  {
 +if (cs-singlestep_enabled) {
 +dbg-control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
 +}
  if (kvm_sw_breakpoints_active(cs)) {
  dbg-control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
  }
 --
 2.3.4



 thanks
 -- PMM

-- 
Alex Bennée
--
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 v2 3/4] target-arm: kvm - support for single step

2015-04-21 Thread Peter Maydell
On 21 April 2015 at 13:56, Alex Bennée alex.ben...@linaro.org wrote:

 Peter Maydell peter.mayd...@linaro.org writes:
  switch (hsr_ec) {
 +case HSR_EC_SOFT_STEP:
 +if (cs-singlestep_enabled) {
 +return true;
 +} else {
 +error_report(Came out of SINGLE STEP when not enabled);

 This can only happen if there's a kernel bug, right?

 Sure. Should we report it differently? abort() out?

Saying 'This would be a kernel bug' in a comment would do...

-- PMM
--
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 v2 3/4] target-arm: kvm - support for single step

2015-04-20 Thread Peter Maydell
On 31 March 2015 at 16:40, Alex Bennée alex.ben...@linaro.org wrote:
 This adds support for single-step. There isn't much to do on the QEMU
 side as after we set-up the request for single step via the debug ioctl
 it is all handled within the kernel.

 Signed-off-by: Alex Bennée alex.ben...@linaro.org

 ---
 v2
   - convert to using HSR_EC

 diff --git a/target-arm/kvm.c b/target-arm/kvm.c
 index 290c1fe..ae0f8b2 100644
 --- a/target-arm/kvm.c
 +++ b/target-arm/kvm.c
 @@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
  */

  #define HSR_EC_SHIFT26
 +#define HSR_EC_SOFT_STEP0x32
  #define HSR_EC_SW_BKPT  0x3c

We already include internals.h in this file, so can you just use
the EC_* constants and ARM_EL_EC_SHIFT rather than defining
new ones? (Applies for patch 1 as well.)

  static int kvm_handle_debug(CPUState *cs, struct kvm_run *run)
 @@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run 
 *run)
  int hsr_ec = arch_info-hsr  HSR_EC_SHIFT;

  switch (hsr_ec) {
 +case HSR_EC_SOFT_STEP:
 +if (cs-singlestep_enabled) {
 +return true;
 +} else {
 +error_report(Came out of SINGLE STEP when not enabled);

This can only happen if there's a kernel bug, right?

 +}
 +break;
  case HSR_EC_SW_BKPT:
  if (kvm_find_sw_breakpoint(cs, arch_info-pc)) {
  return true;
 @@ -542,6 +550,9 @@ int kvm_arch_on_sigbus(int code, void *addr)

  void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
  {
 +if (cs-singlestep_enabled) {
 +dbg-control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
 +}
  if (kvm_sw_breakpoints_active(cs)) {
  dbg-control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
  }
 --
 2.3.4



thanks
-- PMM
--
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


[PATCH v2 3/4] target-arm: kvm - support for single step

2015-03-31 Thread Alex Bennée
This adds support for single-step. There isn't much to do on the QEMU
side as after we set-up the request for single step via the debug ioctl
it is all handled within the kernel.

Signed-off-by: Alex Bennée alex.ben...@linaro.org

---
v2
  - convert to using HSR_EC

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 290c1fe..ae0f8b2 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
 */
 
 #define HSR_EC_SHIFT26
+#define HSR_EC_SOFT_STEP0x32
 #define HSR_EC_SW_BKPT  0x3c
 
 static int kvm_handle_debug(CPUState *cs, struct kvm_run *run)
@@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run 
*run)
 int hsr_ec = arch_info-hsr  HSR_EC_SHIFT;
 
 switch (hsr_ec) {
+case HSR_EC_SOFT_STEP:
+if (cs-singlestep_enabled) {
+return true;
+} else {
+error_report(Came out of SINGLE STEP when not enabled);
+}
+break;
 case HSR_EC_SW_BKPT:
 if (kvm_find_sw_breakpoint(cs, arch_info-pc)) {
 return true;
@@ -542,6 +550,9 @@ int kvm_arch_on_sigbus(int code, void *addr)
 
 void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
 {
+if (cs-singlestep_enabled) {
+dbg-control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
+}
 if (kvm_sw_breakpoints_active(cs)) {
 dbg-control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
 }
-- 
2.3.4

--
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