All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8
@ 2017-09-05  8:57 Sergej Proskurin
  2017-09-05  8:57 ` [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events Sergej Proskurin
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Sergej Proskurin @ 2017-09-05  8:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Sergej Proskurin, tamas, james.morse

Hi all,

This patch series introduces support for single-stepping of guest VMs on
ARMv8. For detailed information about the single-stepping mechanism on
ARMv8, we refer the reader to ARM DDI 0487B.a Section D2.12 (Software
Step exceptions).

Our current implementation supports a rudimentary single-stepping
functionality of the guest's kernel executing in EL1 and is by no means
complete. While the hardware architecture also allows to single-step
EL2, we do not yet implement this feature. Another limitation is that
the current implementation does not yet support single-stepping over
load-exclusive/store-exclusive instructions (LDAXR/STXR), as noticed by
James Morse [0].

This patch series has been submitted as an RFC patch in order to discuss
potential implementation flaws. In the following, we describe the test
environment and appeared effects, the solution to which we would like to
find out.

Our general idea is to make use of the single-stepping functionality as
a means for tracing the guest kernel, executing in EL1. Therefore, we
would like to inject SMC instructions to desired locations within the
guest kernel's text segment. That is, upon execution of injected SMC
instructions, the guest would trap into the hypervisor, where we can
trace the trapping event. While trapped in the hypervisor, we would like
to replace the previously injected SMC with the original instruction (as
to ensure correct guest execution), single-step this original
instruction, and finally place back the SMC instruction before we
continue guest execution.

Our test case is a simple kernel module, which we inject inside of the
guest. Upon trapping the SMC instruction in Xen, we activate
single-stepping and increase the guest's PC by four to continue
execution.  Now, the issue that we are experiencing is that upon
execution of the SMC instruction, the guest seems to trap into a
synchronous interrupt handler. That is, the next guest instruction that
generates a software step exception is the first instruction of the
interrupt handler; not the next instruction (if we increase the pc by
four). This is deterministic and independent of whether we increment the
PC by four or not (to the instruction following the trapping SMC
instruction). As a result, because of the fact that the guest handles
the interrupt, we cannot single-step the replaced original instruction
until the interrupt handler finishes.

Our tests have shown that before the guest (that is currently configured
to use only one VCPU) generates a software step exception that traps
into the hypervisor at do_trap_guest_sync, the hypervisor interrupts the
guest and executes the handler do_trap_irq. We believe that the
interrupt gets injected by Xen into the guest (e.g., timer interrupt).
Which is the reason, why the next instruction that generates a software
step exception resides in the interrupt handler routine. This happens
deterministically every time the SMC gets executed.

We would like to understand if and how we can suspend guest interrupt
injections (if this is truly the cause of our problems), as long as we
are single-stepping the guest, without causing issues. This approach
would prevent SMC instructions to be followed by an in-guest interrupt
handling procedure and thus facilitate our use case.

It would be of great help if we would discuss the upper issue and
hopefully even find a solution to the presented issue.

Thank you very much in advance.

Cheers,
~Sergej

[0] https://lists.xen.org/archives/html/xen-devel/2017-08/msg00661.html

Sergej Proskurin (4):
  arm/monitor: Introduce monitoring of single-step events
  arm/domctl: Add XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_{ON|OFF}
  arm/traps: Allow trapping on single-step events
  vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h>

 xen/arch/arm/arm64/entry.S       |  2 ++
 xen/arch/arm/domctl.c            | 35 ++++++++++++++++++++++++++++
 xen/arch/arm/monitor.c           | 23 ++++++++++++++++++
 xen/arch/arm/traps.c             | 50 +++++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/vm_event.c          | 11 +++++++++
 xen/include/asm-arm/domain.h     |  3 +++
 xen/include/asm-arm/monitor.h    |  5 +++-
 xen/include/asm-arm/perfc_defn.h |  1 +
 xen/include/asm-arm/processor.h  |  2 ++
 xen/include/asm-arm/vm_event.h   |  6 -----
 xen/include/asm-x86/vm_event.h   |  3 ---
 xen/include/xen/vm_event.h       |  3 +++
 12 files changed, 133 insertions(+), 11 deletions(-)

--
2.13.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events
  2017-09-05  8:57 [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Sergej Proskurin
@ 2017-09-05  8:57 ` Sergej Proskurin
  2017-09-05  9:34   ` Razvan Cojocaru
  2017-09-05 16:13   ` Tamas K Lengyel
  2017-09-05  8:57 ` [RFC PATCH 2/4] arm/domctl: Add XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_{ON|OFF} Sergej Proskurin
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 9+ messages in thread
From: Sergej Proskurin @ 2017-09-05  8:57 UTC (permalink / raw)
  To: xen-devel
  Cc: tamas, Razvan Cojocaru, Sergej Proskurin, Julien Grall,
	Stefano Stabellini, james.morse

In this commit, we extend the capabilities of the monitor to allow
tracing of single-step events on ARM.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/monitor.c        | 23 +++++++++++++++++++++++
 xen/include/asm-arm/domain.h  |  1 +
 xen/include/asm-arm/monitor.h |  5 ++++-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
index 59ce8f635f..a4466c9574 100644
--- a/xen/arch/arm/monitor.c
+++ b/xen/arch/arm/monitor.c
@@ -32,6 +32,20 @@ int arch_monitor_domctl_event(struct domain *d,
 
     switch ( mop->event )
     {
+    case XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP:
+    {
+        bool old_status = ad->monitor.single_step_enabled;
+
+        if ( unlikely(old_status == requested_status) )
+            return -EEXIST;
+
+        domain_pause(d);
+        ad->monitor.single_step_enabled = requested_status;
+        domain_unpause(d);
+
+        break;
+    }
+
     case XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL:
     {
         bool_t old_status = ad->monitor.privileged_call_enabled;
@@ -66,6 +80,15 @@ int monitor_smc(void)
     return monitor_traps(current, 1, &req);
 }
 
+int monitor_ss(void)
+{
+    vm_event_request_t req = {
+        .reason = VM_EVENT_REASON_SINGLESTEP,
+    };
+
+    return monitor_traps(current, 1, &req);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 8dfc1d1ec2..0e4ee2956e 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -143,6 +143,7 @@ struct arch_domain
 
     /* Monitor options */
     struct {
+        uint8_t single_step_enabled     : 1;
         uint8_t privileged_call_enabled : 1;
     } monitor;
 }  __cacheline_aligned;
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 7567be66bd..66c7fe14fe 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -57,12 +57,15 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 {
     uint32_t capabilities = 0;
 
-    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST |
+    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP |
+                    1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST |
                     1U << XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL);
 
     return capabilities;
 }
 
+int monitor_ss(void);
+
 int monitor_smc(void);
 
 #endif /* __ASM_ARM_MONITOR_H__ */
-- 
2.13.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 2/4] arm/domctl: Add XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_{ON|OFF}
  2017-09-05  8:57 [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Sergej Proskurin
  2017-09-05  8:57 ` [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events Sergej Proskurin
@ 2017-09-05  8:57 ` Sergej Proskurin
  2017-09-05  8:57 ` [RFC PATCH 3/4] arm/traps: Allow trapping on single-step events Sergej Proskurin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Sergej Proskurin @ 2017-09-05  8:57 UTC (permalink / raw)
  To: xen-devel
  Cc: Sergej Proskurin, Julien Grall, tamas, james.morse, Stefano Stabellini

This commit adds the domctl that is required to enable single-stepping
on ARM.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domctl.c        | 35 +++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/domain.h |  2 ++
 2 files changed, 37 insertions(+)

diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 971caecd58..f640519b5c 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -20,6 +20,28 @@ void arch_get_domain_info(const struct domain *d,
     info->flags |= XEN_DOMINF_hap;
 }
 
+int debug_do_domctl(struct vcpu *v, int32_t op)
+{
+    int rc;
+
+    switch ( op )
+    {
+    case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
+    case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
+        /* XXX: check whether the cpu supports singlestepping. */
+
+        rc = 0;
+        vcpu_pause(v);
+        v->arch.single_step = (op == XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON);
+        vcpu_unpause(v); /* guest will latch new state */
+        break;
+    default:
+        rc = -ENOSYS;
+    }
+
+    return rc;
+}
+
 long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
                     XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
@@ -114,6 +136,19 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
 
         return 0;
     }
+    case XEN_DOMCTL_debug_op:
+    {
+        struct vcpu *v;
+
+        if ( (domctl->u.debug_op.vcpu >= d->max_vcpus) ||
+             ((v = d->vcpu[domctl->u.debug_op.vcpu]) == NULL) )
+            return -EINVAL;
+
+        if ( (v == current) )
+            return -EINVAL;
+
+        return debug_do_domctl(v, domctl->u.debug_op.op);
+    }
 
     case XEN_DOMCTL_disable_migrate:
         d->disable_migrate = domctl->u.disable_migrate.disable;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 0e4ee2956e..105bad0b5b 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -282,6 +282,8 @@ struct arch_vcpu
     struct vtimer phys_timer;
     struct vtimer virt_timer;
     bool_t vtimer_initialized;
+
+    bool single_step;
 }  __cacheline_aligned;
 
 void vcpu_show_execution_state(struct vcpu *);
-- 
2.13.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 3/4] arm/traps: Allow trapping on single-step events
  2017-09-05  8:57 [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Sergej Proskurin
  2017-09-05  8:57 ` [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events Sergej Proskurin
  2017-09-05  8:57 ` [RFC PATCH 2/4] arm/domctl: Add XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_{ON|OFF} Sergej Proskurin
@ 2017-09-05  8:57 ` Sergej Proskurin
  2017-09-05  8:57 ` [RFC PATCH 4/4] vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h> Sergej Proskurin
  2017-09-11 11:10 ` [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Julien Grall
  4 siblings, 0 replies; 9+ messages in thread
From: Sergej Proskurin @ 2017-09-05  8:57 UTC (permalink / raw)
  To: xen-devel
  Cc: Sergej Proskurin, Julien Grall, tamas, james.morse, Stefano Stabellini

This commit concludes the single-stepping functionality on ARM by adding
trapping on and setting up single-stepping events of the architecture.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/entry.S       |  2 ++
 xen/arch/arm/traps.c             | 50 +++++++++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/perfc_defn.h |  1 +
 xen/include/asm-arm/processor.h  |  2 ++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 6d99e46f0f..5e89f24494 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -138,6 +138,8 @@ lr      .req    x30             /* link register */
 
         bl      leave_hypervisor_tail /* Disables interrupts on return */
 
+        bl      setup_single_step
+
         exit_guest \compat
 
         .endif
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa838e8e77..9c45b0706e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -163,7 +163,7 @@ void init_traps(void)
     WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
 
     /* Trap Debug and Performance Monitor accesses */
-    WRITE_SYSREG(HDCR_TDRA|HDCR_TDOSA|HDCR_TDA|HDCR_TPM|HDCR_TPMCR,
+    WRITE_SYSREG(HDCR_TDRA|HDCR_TDOSA|HDCR_TDA|HDCR_TPM|HDCR_TPMCR|HDCR_TDE,
                  MDCR_EL2);
 
     /* Trap CP15 c15 used for implementation defined registers */
@@ -1332,6 +1332,20 @@ int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc)
 }
 
 #ifdef CONFIG_ARM_64
+static void do_trap_ss(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    int rc = 0;
+
+    /* XXX: We do not support single-stepping of EL2, yet. */
+    BUG_ON(hyp_mode(regs));
+
+    if ( current->domain->arch.monitor.single_step_enabled )
+        rc = monitor_ss();
+
+    if ( rc != 1 )
+        inject_undef_exception(regs, hsr);
+}
+
 static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
 {
     /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive
@@ -2943,6 +2957,12 @@ asmlinkage void do_trap_guest_sync(struct cpu_user_regs *regs)
         perfc_incr(trap_dabt);
         do_trap_data_abort_guest(regs, hsr);
         break;
+#ifdef CONFIG_ARM_64
+    case HSR_EC_SS_LOWER_EL:
+        perfc_incr(trap_ss);
+        do_trap_ss(regs, hsr);
+        break;
+#endif
 
     default:
         gprintk(XENLOG_WARNING,
@@ -2999,6 +3019,34 @@ asmlinkage void do_trap_fiq(struct cpu_user_regs *regs)
     gic_interrupt(regs, 1);
 }
 
+asmlinkage void setup_single_step(void)
+{
+    uint32_t mdscr, mdcr;
+    struct vcpu *v = current;
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+#define MDSCR_EL1_SS    (_AC(1,U) << 0)
+#define SPSR_EL2_SS     (_AC(1,U) << 21)
+
+    mdscr = READ_SYSREG(MDSCR_EL1);
+    mdcr = READ_SYSREG(MDCR_EL2);
+
+    if ( unlikely(v->arch.single_step) )
+    {
+        mdcr |= HDCR_TDE;
+        mdscr |= MDSCR_EL1_SS;
+        regs->cpsr |= SPSR_EL2_SS;
+    }
+    else
+    {
+        mdcr &= ~HDCR_TDE;
+        mdscr &= ~MDSCR_EL1_SS;
+    }
+
+    WRITE_SYSREG(mdscr, MDSCR_EL1);
+    WRITE_SYSREG(mdcr, MDCR_EL2);
+}
+
 asmlinkage void leave_hypervisor_tail(void)
 {
     while (1)
diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
index 5f957ee6ec..46b82e4fee 100644
--- a/xen/include/asm-arm/perfc_defn.h
+++ b/xen/include/asm-arm/perfc_defn.h
@@ -18,6 +18,7 @@ PERFCOUNTER(trap_hvc32,    "trap: 32-bit hvc")
 PERFCOUNTER(trap_smc64,    "trap: 64-bit smc")
 PERFCOUNTER(trap_hvc64,    "trap: 64-bit hvc")
 PERFCOUNTER(trap_sysreg,   "trap: sysreg access")
+PERFCOUNTER(trap_ss,       "trap: software step")
 #endif
 PERFCOUNTER(trap_iabt,     "trap: guest instr abort")
 PERFCOUNTER(trap_dabt,     "trap: guest data abort")
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 9f7a42f86b..3e0ec4f537 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -323,6 +323,8 @@
 #define HSR_EC_DATA_ABORT_LOWER_EL  0x24
 #define HSR_EC_DATA_ABORT_CURR_EL   0x25
 #ifdef CONFIG_ARM_64
+#define HSR_EC_SS_LOWER_EL          0x32
+#define HSR_EC_SS_CURR_EL           0x33
 #define HSR_EC_BRK                  0x3c
 #endif
 
-- 
2.13.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 4/4] vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h>
  2017-09-05  8:57 [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Sergej Proskurin
                   ` (2 preceding siblings ...)
  2017-09-05  8:57 ` [RFC PATCH 3/4] arm/traps: Allow trapping on single-step events Sergej Proskurin
@ 2017-09-05  8:57 ` Sergej Proskurin
  2017-09-05  9:39   ` Razvan Cojocaru
  2017-09-11 11:10 ` [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Julien Grall
  4 siblings, 1 reply; 9+ messages in thread
From: Sergej Proskurin @ 2017-09-05  8:57 UTC (permalink / raw)
  To: xen-devel
  Cc: tamas, Razvan Cojocaru, Sergej Proskurin, Julien Grall,
	Stefano Stabellini, james.morse, Jan Beulich, Andrew Cooper

In this commit we move the declaration of the function
vm_event_toggle_singlestep from <asm/vm_event.h> to <xen/vm_event.h> and
implement the associated functionality on ARM.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/arm/vm_event.c        | 11 +++++++++++
 xen/include/asm-arm/vm_event.h |  6 ------
 xen/include/asm-x86/vm_event.h |  3 ---
 xen/include/xen/vm_event.h     |  3 +++
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/vm_event.c b/xen/arch/arm/vm_event.c
index eaac92078d..a3bb525e9e 100644
--- a/xen/arch/arm/vm_event.c
+++ b/xen/arch/arm/vm_event.c
@@ -47,6 +47,17 @@ void vm_event_monitor_next_interrupt(struct vcpu *v)
     /* Not supported on ARM. */
 }
 
+void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
+                                vm_event_response_t *rsp)
+{
+    if ( !(rsp->flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP) )
+        return;
+
+    ASSERT(atomic_read(&v->vm_event_pause_count));
+
+    v->arch.single_step = !v->arch.single_step;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 66f2474fe1..0d7a5446f2 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -34,12 +34,6 @@ static inline void vm_event_cleanup_domain(struct domain *d)
     memset(&d->monitor, 0, sizeof(d->monitor));
 }
 
-static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
-                                              vm_event_response_t *rsp)
-{
-    /* Not supported on ARM. */
-}
-
 static inline
 void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
 {
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 39e73c83ca..139867178a 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -40,9 +40,6 @@ int vm_event_init_domain(struct domain *d);
 
 void vm_event_cleanup_domain(struct domain *d);
 
-void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
-                                vm_event_response_t *rsp);
-
 void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp);
 
 void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp);
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index 2fb39519b1..210aab1b37 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -80,6 +80,9 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
 
 void vm_event_monitor_next_interrupt(struct vcpu *v);
 
+void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
+                                vm_event_response_t *rsp);
+
 #endif /* __VM_EVENT_H__ */
 
 /*
-- 
2.13.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events
  2017-09-05  8:57 ` [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events Sergej Proskurin
@ 2017-09-05  9:34   ` Razvan Cojocaru
  2017-09-05 16:13   ` Tamas K Lengyel
  1 sibling, 0 replies; 9+ messages in thread
From: Razvan Cojocaru @ 2017-09-05  9:34 UTC (permalink / raw)
  To: Sergej Proskurin, xen-devel
  Cc: Julien Grall, tamas, james.morse, Stefano Stabellini

On 09/05/2017 11:57 AM, Sergej Proskurin wrote:
> In this commit, we extend the capabilities of the monitor to allow
> tracing of single-step events on ARM.
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 4/4] vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h>
  2017-09-05  8:57 ` [RFC PATCH 4/4] vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h> Sergej Proskurin
@ 2017-09-05  9:39   ` Razvan Cojocaru
  0 siblings, 0 replies; 9+ messages in thread
From: Razvan Cojocaru @ 2017-09-05  9:39 UTC (permalink / raw)
  To: Sergej Proskurin, xen-devel
  Cc: tamas, Andrew Cooper, Julien Grall, Stefano Stabellini,
	james.morse, Jan Beulich

On 09/05/2017 11:57 AM, Sergej Proskurin wrote:
> In this commit we move the declaration of the function
> vm_event_toggle_singlestep from <asm/vm_event.h> to <xen/vm_event.h> and
> implement the associated functionality on ARM.
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events
  2017-09-05  8:57 ` [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events Sergej Proskurin
  2017-09-05  9:34   ` Razvan Cojocaru
@ 2017-09-05 16:13   ` Tamas K Lengyel
  1 sibling, 0 replies; 9+ messages in thread
From: Tamas K Lengyel @ 2017-09-05 16:13 UTC (permalink / raw)
  To: Sergej Proskurin
  Cc: Xen-devel, Julien Grall, Stefano Stabellini, james.morse,
	Razvan Cojocaru

> diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
> index 7567be66bd..66c7fe14fe 100644
> --- a/xen/include/asm-arm/monitor.h
> +++ b/xen/include/asm-arm/monitor.h
> @@ -57,12 +57,15 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>  {
>      uint32_t capabilities = 0;
>
> -    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST |
> +    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP |
> +                    1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST |
>                      1U << XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL);

Please append the new capability bit instead of prepending.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8
  2017-09-05  8:57 [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Sergej Proskurin
                   ` (3 preceding siblings ...)
  2017-09-05  8:57 ` [RFC PATCH 4/4] vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h> Sergej Proskurin
@ 2017-09-11 11:10 ` Julien Grall
  4 siblings, 0 replies; 9+ messages in thread
From: Julien Grall @ 2017-09-11 11:10 UTC (permalink / raw)
  To: Sergej Proskurin, xen-devel; +Cc: tamas, james.morse, Stefano Stabellini



On 05/09/17 09:57, Sergej Proskurin wrote:
> Hi all,

Hello,

I am pretty sure I already said it before. It is quite annoying to have 
to search for the cover letter because you don't CC person involved in 
this. More that it contains useful details to understand the rest of 
this series...

> This patch series introduces support for single-stepping of guest VMs on
> ARMv8. For detailed information about the single-stepping mechanism on
> ARMv8, we refer the reader to ARM DDI 0487B.a Section D2.12 (Software
> Step exceptions).
> 
> Our current implementation supports a rudimentary single-stepping
> functionality of the guest's kernel executing in EL1 and is by no means
> complete. While the hardware architecture also allows to single-step
> EL2, we do not yet implement this feature. Another limitation is that
> the current implementation does not yet support single-stepping over
> load-exclusive/store-exclusive instructions (LDAXR/STXR), as noticed by
> James Morse [0].

EL2 and EL1 single-step are 2 distinct use case. The former is for 
debugging the hypervisor, whilst the latter is for the guest. We should 
really differentiate both and focus on EL1 single-stepping.

> 
> This patch series has been submitted as an RFC patch in order to discuss
> potential implementation flaws. In the following, we describe the test
> environment and appeared effects, the solution to which we would like to
> find out.
> 
> Our general idea is to make use of the single-stepping functionality as
> a means for tracing the guest kernel, executing in EL1. Therefore, we
> would like to inject SMC instructions to desired locations within the
> guest kernel's text segment. That is, upon execution of injected SMC
> instructions, the guest would trap into the hypervisor, where we can
> trace the trapping event. While trapped in the hypervisor, we would like
> to replace the previously injected SMC with the original instruction (as
> to ensure correct guest execution), single-step this original
> instruction, and finally place back the SMC instruction before we
> continue guest execution.
> 
> Our test case is a simple kernel module, which we inject inside of the
> guest. Upon trapping the SMC instruction in Xen, we activate
> single-stepping and increase the guest's PC by four to continue
> execution.  Now, the issue that we are experiencing is that upon
> execution of the SMC instruction, the guest seems to trap into a
> synchronous interrupt handler. That is, the next guest instruction that
> generates a software step exception is the first instruction of the
> interrupt handler; not the next instruction (if we increase the pc by
> four). This is deterministic and independent of whether we increment the
> PC by four or not (to the instruction following the trapping SMC
> instruction). As a result, because of the fact that the guest handles
> the interrupt, we cannot single-step the replaced original instruction
> until the interrupt handler finishes.
> 
> Our tests have shown that before the guest (that is currently configured
> to use only one VCPU) generates a software step exception that traps
> into the hypervisor at do_trap_guest_sync, the hypervisor interrupts the
> guest and executes the handler do_trap_irq. We believe that the
> interrupt gets injected by Xen into the guest (e.g., timer interrupt).
> Which is the reason, why the next instruction that generates a software
> step exception resides in the interrupt handler routine. This happens
> deterministically every time the SMC gets executed.

It makes sense, the monitor likely adds enough delay to get the timer 
raising an interrupt for the next deadline.

> 
> We would like to understand if and how we can suspend guest interrupt
> injections (if this is truly the cause of our problems), as long as we
> are single-stepping the guest, without causing issues. This approach
> would prevent SMC instructions to be followed by an in-guest interrupt
> handling procedure and thus facilitate our use case.

One way to confirm if it is the cause is to disable the virtual CPU 
interface temporarily (see GICH_HCR.En on GICv2).

But what do you mean "as long as we are single-stepping the guest"? Is 
it only one instruction and could be multiple instructions?

If the latter, it means you delay the interrupt that may result in an 
infinite loop in the guest (imagine a code relying on interrupt...).

You may also want to single-step the interaction between a code and the 
interrupt.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-09-11 11:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05  8:57 [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Sergej Proskurin
2017-09-05  8:57 ` [RFC PATCH 1/4] arm/monitor: Introduce monitoring of single-step events Sergej Proskurin
2017-09-05  9:34   ` Razvan Cojocaru
2017-09-05 16:13   ` Tamas K Lengyel
2017-09-05  8:57 ` [RFC PATCH 2/4] arm/domctl: Add XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_{ON|OFF} Sergej Proskurin
2017-09-05  8:57 ` [RFC PATCH 3/4] arm/traps: Allow trapping on single-step events Sergej Proskurin
2017-09-05  8:57 ` [RFC PATCH 4/4] vm_event: Move vm_event_toggle_singlestep to <xen/vm_event.h> Sergej Proskurin
2017-09-05  9:39   ` Razvan Cojocaru
2017-09-11 11:10 ` [RFC PATCH 0/4] Introduce Single-Stepping to ARMv8 Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.