xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities
@ 2016-05-04 14:51 Tamas K Lengyel
  2016-05-04 14:51 ` [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common Tamas K Lengyel
                   ` (7 more replies)
  0 siblings, 8 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Tamas K Lengyel

The monitor_get_capabilities check actually belongs to the monitor subsystem so
relocating and renaming it to sanitize the code's name and location. Mechanical
patch, no code changes introduced.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Jan Beulich <jbeulich@suse.com>
Stefano Stabellini <sstabellini@kernel.org>
Julien Grall <julien.grall@arm.com>
---
 xen/arch/x86/monitor.c         |  2 +-
 xen/common/monitor.c           |  5 ++---
 xen/include/asm-arm/monitor.h  | 11 ++++++++++-
 xen/include/asm-arm/vm_event.h |  9 ---------
 xen/include/asm-x86/monitor.h  | 23 +++++++++++++++++++++++
 xen/include/asm-x86/vm_event.h | 23 -----------------------
 6 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 1fec412..621f91a 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -126,7 +126,7 @@ int arch_monitor_domctl_event(struct domain *d,
 
     default:
         /*
-         * Should not be reached unless vm_event_monitor_get_capabilities() is
+         * Should not be reached unless arch_monitor_get_capabilities() is
          * not properly implemented.
          */
         ASSERT_UNREACHABLE();
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index d950a7c..7c3d1c8 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -24,7 +24,6 @@
 #include <xsm/xsm.h>
 #include <public/domctl.h>
 #include <asm/monitor.h>
-#include <asm/vm_event.h>
 
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
@@ -48,12 +47,12 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
         if ( unlikely(mop->event > 31) )
             return -EINVAL;
         /* Check if event type is available. */
-        if ( unlikely(!(vm_event_monitor_get_capabilities(d) & (1U << mop->event))) )
+        if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << mop->event))) )
             return -EOPNOTSUPP;
         break;
 
     case XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES:
-        mop->event = vm_event_monitor_get_capabilities(d);
+        mop->event = arch_monitor_get_capabilities(d);
         return 0;
 
     default:
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 6e36e99..3fd3c9d 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -39,11 +39,20 @@ int arch_monitor_domctl_event(struct domain *d,
     /*
      * No arch-specific monitor vm-events on ARM.
      *
-     * Should not be reached unless vm_event_monitor_get_capabilities() is not
+     * Should not be reached unless arch_monitor_get_capabilities() is not
      * properly implemented.
      */
     ASSERT_UNREACHABLE();
     return -EOPNOTSUPP;
 }
 
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+    uint32_t capabilities = 0;
+
+    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+    return capabilities;
+}
+
 #endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 014d9ba..a3fc4ce 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -59,13 +59,4 @@ static inline void vm_event_fill_regs(vm_event_request_t *req)
     /* Not supported on ARM. */
 }
 
-static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
-{
-    uint32_t capabilities = 0;
-
-    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
-    return capabilities;
-}
-
 #endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 0954b59..446b2af 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -50,4 +50,27 @@ int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
 int arch_monitor_domctl_event(struct domain *d,
                               struct xen_domctl_monitor_op *mop);
 
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+    uint32_t capabilities = 0;
+
+    /*
+     * At the moment only Intel HVM domains are supported. However, event
+     * delivery could be extended to AMD and PV domains.
+     */
+    if ( !is_hvm_domain(d) || !cpu_has_vmx )
+        return capabilities;
+
+    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+    /* Since we know this is on VMX, we can just call the hvm func */
+    if ( hvm_is_singlestep_supported() )
+        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+
+    return capabilities;
+}
+
 #endif /* __ASM_X86_MONITOR_H__ */
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 0470240..026f42e 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -44,27 +44,4 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
 
 void vm_event_fill_regs(vm_event_request_t *req);
 
-static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
-{
-    uint32_t capabilities = 0;
-
-    /*
-     * At the moment only Intel HVM domains are supported. However, event
-     * delivery could be extended to AMD and PV domains.
-     */
-    if ( !is_hvm_domain(d) || !cpu_has_vmx )
-        return capabilities;
-
-    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
-    /* Since we know this is on VMX, we can just call the hvm func */
-    if ( hvm_is_singlestep_supported() )
-        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
-
-    return capabilities;
-}
-
 #endif /* __ASM_X86_VM_EVENT_H__ */
-- 
2.8.1


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

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

* [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-05  9:34   ` Razvan Cojocaru
  2016-05-16  9:48   ` Julien Grall
  2016-05-04 14:51 ` [PATCH v3 3/9] monitor: ARM SMC events Tamas K Lengyel
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Tamas K Lengyel, Razvan Cojocaru, Andrew Cooper, Julien Grall,
	Stefano Stabellini, Jan Beulich

The prototype of vm_event_fill_regs will differ on x86 and ARM so in this patch
we move components from common to arch-specific that use this function. As
part of this patch we rename and relocate vm_event_monitor_guest_request as
monitor_guest_request from vm_event to monitor.

Signed-off-by: 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>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/arm/Makefile      |  1 +
 xen/arch/arm/hvm.c         |  4 ++--
 xen/arch/arm/monitor.c     | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/event.c   |  3 +++
 xen/arch/x86/hvm/hvm.c     |  3 ++-
 xen/arch/x86/monitor.c     | 18 +++++++++++++++++
 xen/common/vm_event.c      | 17 ----------------
 xen/include/xen/monitor.h  |  1 +
 xen/include/xen/vm_event.h |  2 --
 9 files changed, 75 insertions(+), 22 deletions(-)
 create mode 100644 xen/arch/arm/monitor.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 0328b50..6e3dcff 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,6 +40,7 @@ obj-y += device.o
 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-y += monitor.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index c01123a..d999bde 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -22,7 +22,7 @@
 #include <xen/errno.h>
 #include <xen/guest_access.h>
 #include <xen/sched.h>
-#include <xen/vm_event.h>
+#include <xen/monitor.h>
 
 #include <xsm/xsm.h>
 
@@ -75,7 +75,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case HVMOP_guest_request_vm_event:
         if ( guest_handle_is_null(arg) )
-            vm_event_monitor_guest_request();
+            monitor_guest_request();
         else
             rc = -EINVAL;
         break;
diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
new file mode 100644
index 0000000..f957257
--- /dev/null
+++ b/xen/arch/arm/monitor.c
@@ -0,0 +1,48 @@
+/*
+ * arch/arm/monitor.c
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2015-2016 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <asm/vm_event.h>
+#include <public/vm_event.h>
+
+void monitor_guest_request(void)
+{
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
+
+    if ( d->monitor.guest_request_enabled )
+    {
+        vm_event_request_t req = {
+            .reason = VM_EVENT_REASON_GUEST_REQUEST,
+            .vcpu_id = curr->vcpu_id,
+        };
+
+        vm_event_monitor_traps(curr, d->monitor.guest_request_sync, &req);
+    }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 56c5514..73d0a26 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -47,6 +47,7 @@ bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
             .u.write_ctrlreg.old_value = old
         };
 
+        vm_event_fill_regs(&req);
         vm_event_monitor_traps(curr, sync, &req);
         return 1;
     }
@@ -68,6 +69,7 @@ void hvm_event_msr(unsigned int msr, uint64_t value)
             .u.mov_to_msr.value = value,
         };
 
+        vm_event_fill_regs(&req);
         vm_event_monitor_traps(curr, 1, &req);
     }
 }
@@ -115,6 +117,7 @@ int hvm_event_breakpoint(unsigned long rip,
     }
 
     req.vcpu_id = curr->vcpu_id;
+    vm_event_fill_regs(&req);
 
     return vm_event_monitor_traps(curr, 1, &req);
 }
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8cb6e9e..5d740f7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -37,6 +37,7 @@
 #include <xen/mem_access.h>
 #include <xen/rangeset.h>
 #include <xen/vm_event.h>
+#include <xen/monitor.h>
 #include <asm/shadow.h>
 #include <asm/hap.h>
 #include <asm/current.h>
@@ -5695,7 +5696,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case HVMOP_guest_request_vm_event:
         if ( guest_handle_is_null(arg) )
-            vm_event_monitor_guest_request();
+            monitor_guest_request();
         else
             rc = -EINVAL;
         break;
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 621f91a..8598049 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -20,6 +20,7 @@
  */
 
 #include <asm/monitor.h>
+#include <asm/vm_event.h>
 #include <public/vm_event.h>
 
 int arch_monitor_domctl_event(struct domain *d,
@@ -136,6 +137,23 @@ int arch_monitor_domctl_event(struct domain *d,
     return 0;
 }
 
+void monitor_guest_request(void)
+{
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
+
+    if ( d->monitor.guest_request_enabled )
+    {
+        vm_event_request_t req = {
+            .reason = VM_EVENT_REASON_GUEST_REQUEST,
+            .vcpu_id = curr->vcpu_id,
+        };
+
+        vm_event_fill_regs(&req);
+        vm_event_monitor_traps(curr, d->monitor.guest_request_sync, &req);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 2906407..31a8830 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -818,28 +818,11 @@ int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
         req->altp2m_idx = altp2m_vcpu_idx(v);
     }
 
-    vm_event_fill_regs(req);
     vm_event_put_request(d, &d->vm_event->monitor, req);
 
     return 1;
 }
 
-void vm_event_monitor_guest_request(void)
-{
-    struct vcpu *curr = current;
-    struct domain *d = curr->domain;
-
-    if ( d->monitor.guest_request_enabled )
-    {
-        vm_event_request_t req = {
-            .reason = VM_EVENT_REASON_GUEST_REQUEST,
-            .vcpu_id = curr->vcpu_id,
-        };
-
-        vm_event_monitor_traps(curr, d->monitor.guest_request_sync, &req);
-    }
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
index 7015e6d..204d5cc 100644
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -26,5 +26,6 @@ struct domain;
 struct xen_domctl_monitor_op;
 
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);
+void monitor_guest_request(void);
 
 #endif /* __XEN_MONITOR_H__ */
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index beda9fe..89e6243 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -81,8 +81,6 @@ void vm_event_vcpu_unpause(struct vcpu *v);
 int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
                            vm_event_request_t *req);
 
-void vm_event_monitor_guest_request(void);
-
 #endif /* __VM_EVENT_H__ */
 
 
-- 
2.8.1


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

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

* [PATCH v3 3/9] monitor: ARM SMC events
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
  2016-05-04 14:51 ` [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-05  9:36   ` Razvan Cojocaru
  2016-05-16  9:56   ` Julien Grall
  2016-05-04 14:51 ` [PATCH v3 4/9] arm/vm_event: get/set registers Tamas K Lengyel
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Julien Grall, Tamas K Lengyel, Stefano Stabellini, Razvan Cojocaru

Add support for monitoring ARM SMC events. This patch only adds the required
bits to enable/disable monitoring and forwarding the event through vm_event.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>

v3: Split parts off as separate patches
    Union for arm32/64 register structs in vm_event
    Cosmetic fixes
---
 xen/arch/arm/monitor.c        | 49 +++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c          | 16 ++++++++++++--
 xen/include/asm-arm/domain.h  |  5 +++++
 xen/include/asm-arm/monitor.h | 24 ++++++---------------
 xen/include/public/domctl.h   |  1 +
 xen/include/public/vm_event.h |  2 ++
 6 files changed, 77 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
index f957257..9c481ac 100644
--- a/xen/arch/arm/monitor.c
+++ b/xen/arch/arm/monitor.c
@@ -22,6 +22,55 @@
 #include <asm/vm_event.h>
 #include <public/vm_event.h>
 
+int arch_monitor_domctl_event(struct domain *d,
+                              struct xen_domctl_monitor_op *mop)
+{
+    struct arch_domain *ad = &d->arch;
+    bool_t requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op);
+
+    switch ( mop->event )
+    {
+    case XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL:
+    {
+        bool_t old_status = ad->monitor.privileged_call_enabled;
+
+        if ( unlikely(old_status == requested_status) )
+            return -EEXIST;
+
+        domain_pause(d);
+        ad->monitor.privileged_call_enabled = requested_status;
+        domain_unpause(d);
+        break;
+    }
+
+    default:
+        /*
+         * Should not be reached unless arch_monitor_get_capabilities() is
+         * not properly implemented.
+         */
+        ASSERT_UNREACHABLE();
+        return -EOPNOTSUPP;
+    }
+
+    return 0;
+}
+
+bool_t monitor_smc(const struct cpu_user_regs *regs) {
+    struct vcpu *curr = current;
+    vm_event_request_t req = { 0 };
+
+    if ( !curr->domain->arch.monitor.privileged_call_enabled )
+        return 0;
+
+    req.reason = VM_EVENT_REASON_PRIVILEGED_CALL;
+    req.vcpu_id = curr->vcpu_id;
+
+    if ( vm_event_monitor_traps(curr, 1, &req) <= 0 )
+        return 0;
+    else
+        return 1;
+}
+
 void monitor_guest_request(void)
 {
     struct vcpu *curr = current;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9abfc3c..f26e12e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -41,6 +41,7 @@
 #include <asm/mmio.h>
 #include <asm/cpufeature.h>
 #include <asm/flushtlb.h>
+#include <asm/monitor.h>
 
 #include "decode.h"
 #include "vtimer.h"
@@ -2491,6 +2492,17 @@ bad_data_abort:
     inject_dabt_exception(regs, info.gva, hsr.len);
 }
 
+static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    bool_t handled = 0;
+
+    if ( current->domain->arch.monitor.privileged_call_enabled )
+        handled = monitor_smc(regs);
+
+    if ( handled != 1 )
+        inject_undef_exception(regs, hsr);
+}
+
 static void enter_hypervisor_head(struct cpu_user_regs *regs)
 {
     if ( guest_mode(regs) )
@@ -2566,7 +2578,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
          */
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_smc32);
-        inject_undef32_exception(regs);
+        do_trap_smc(regs, hsr);
         break;
     case HSR_EC_HVC32:
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
@@ -2599,7 +2611,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
          */
         GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_smc64);
-        inject_undef64_exception(regs, hsr.len);
+        do_trap_smc(regs, hsr);
         break;
     case HSR_EC_SYSREG:
         GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c35ed40..87c7d7d 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -129,6 +129,11 @@ struct arch_domain
     paddr_t efi_acpi_gpa;
     paddr_t efi_acpi_len;
 #endif
+
+    /* Monitor options */
+    struct {
+        uint8_t privileged_call_enabled : 1;
+    } monitor;
 }  __cacheline_aligned;
 
 struct arch_vcpu
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 3fd3c9d..9a10d1f 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -3,7 +3,7 @@
  *
  * Arch-specific monitor_op domctl handler.
  *
- * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2015-2016 Tamas K Lengyel (tamas@tklengyel.com)
  * Copyright (c) 2016, Bitdefender S.R.L.
  *
  * This program is free software; you can redistribute it and/or
@@ -32,27 +32,15 @@ int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
     return -EOPNOTSUPP;
 }
 
-static inline
 int arch_monitor_domctl_event(struct domain *d,
-                              struct xen_domctl_monitor_op *mop)
-{
-    /*
-     * No arch-specific monitor vm-events on ARM.
-     *
-     * Should not be reached unless arch_monitor_get_capabilities() is not
-     * properly implemented.
-     */
-    ASSERT_UNREACHABLE();
-    return -EOPNOTSUPP;
-}
+                              struct xen_domctl_monitor_op *mop);
 
 static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 {
-    uint32_t capabilities = 0;
-
-    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
-    return capabilities;
+    return (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
+           (1U << XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL);
 }
 
+bool_t monitor_smc(const struct cpu_user_regs *regs);
+
 #endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 2457698..35adce2 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1080,6 +1080,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
 #define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP            2
 #define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT   3
 #define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST         4
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL       5
 
 struct xen_domctl_monitor_op {
     uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 9270d52..3acf217 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -119,6 +119,8 @@
 #define VM_EVENT_REASON_SINGLESTEP              7
 /* An event has been requested via HVMOP_guest_request_vm_event. */
 #define VM_EVENT_REASON_GUEST_REQUEST           8
+/* Privileged call executed (e.g. SMC) */
+#define VM_EVENT_REASON_PRIVILEGED_CALL         9
 
 /* Supported values for the vm_event_write_ctrlreg index. */
 #define VM_EVENT_X86_CR0    0
-- 
2.8.1


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

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

* [PATCH v3 4/9] arm/vm_event: get/set registers
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
  2016-05-04 14:51 ` [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common Tamas K Lengyel
  2016-05-04 14:51 ` [PATCH v3 3/9] monitor: ARM SMC events Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-16 10:14   ` Julien Grall
  2016-05-04 14:51 ` [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Julien Grall, Tamas K Lengyel, Stefano Stabellini, Razvan Cojocaru

Add support for getting/setting registers through vm_event on ARM.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/arm/Makefile          |   1 +
 xen/arch/arm/vm_event.c        | 141 +++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/vm_event.h |  13 ++--
 xen/include/public/vm_event.h  |  54 +++++++++++++++-
 4 files changed, 198 insertions(+), 11 deletions(-)
 create mode 100644 xen/arch/arm/vm_event.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 6e3dcff..118be99 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -41,6 +41,7 @@ obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
 obj-y += monitor.o
+obj-y += vm_event.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/vm_event.c b/xen/arch/arm/vm_event.c
new file mode 100644
index 0000000..59962c7
--- /dev/null
+++ b/xen/arch/arm/vm_event.c
@@ -0,0 +1,141 @@
+/*
+ * arch/arm/vm_event.c
+ *
+ * Architecture-specific vm_event handling routines
+ *
+ * Copyright (c) 2016 Tamas K Lengyel (tamas@tklengyel.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/sched.h>
+#include <asm/vm_event.h>
+
+void vm_event_fill_regs(vm_event_request_t *req,
+                        const struct cpu_user_regs *regs,
+                        struct domain *d)
+{
+    if ( is_32bit_domain(d) )
+    {
+        req->data.regs.arm32.r0_usr = regs->r0;
+        req->data.regs.arm32.r1_usr = regs->r1;
+        req->data.regs.arm32.r2_usr = regs->r2;
+        req->data.regs.arm32.r3_usr = regs->r3;
+        req->data.regs.arm32.r4_usr = regs->r4;
+        req->data.regs.arm32.r5_usr = regs->r5;
+        req->data.regs.arm32.r6_usr = regs->r6;
+        req->data.regs.arm32.r7_usr = regs->r7;
+        req->data.regs.arm32.r8_usr = regs->r8;
+        req->data.regs.arm32.r9_usr = regs->r9;
+        req->data.regs.arm32.r10_usr = regs->r10;
+        req->data.regs.arm32.r12_usr = regs->r12;
+        req->data.regs.arm32.pc = regs->pc32;
+        req->data.regs.arm32.sp_usr = regs->sp_usr;
+        req->data.regs.arm32.sp_svc = regs->sp_svc;
+        req->data.regs.arm32.fp = regs->fp;
+        req->data.regs.arm32.lr_usr = regs->lr_usr;
+        req->data.regs.arm32.cpsr = regs->cpsr;
+        req->data.regs.arm32.spsr_svc = regs->spsr_svc;
+        req->data.regs.arm32.ttbr0 = READ_SYSREG64(TTBR0_EL1);
+        req->data.regs.arm32.ttbr1 = READ_SYSREG64(TTBR1_EL1);
+    }
+#ifdef CONFIG_ARM_64
+    else
+    {
+        req->data.regs.arm64.x0 = regs->x0;
+        req->data.regs.arm64.x1 = regs->x1;
+        req->data.regs.arm64.x2 = regs->x2;
+        req->data.regs.arm64.x3 = regs->x3;
+        req->data.regs.arm64.x4 = regs->x4;
+        req->data.regs.arm64.x5 = regs->x5;
+        req->data.regs.arm64.x6 = regs->x6;
+        req->data.regs.arm64.x7 = regs->x7;
+        req->data.regs.arm64.x8 = regs->x8;
+        req->data.regs.arm64.x9 = regs->x9;
+        req->data.regs.arm64.x10 = regs->x10;
+        req->data.regs.arm64.x16 = regs->x16;
+        req->data.regs.arm64.pc = regs->pc;
+        req->data.regs.arm64.sp_el0 = regs->sp_el0;
+        req->data.regs.arm64.sp_el1 = regs->sp_el1;
+        req->data.regs.arm64.fp = regs->fp;
+        req->data.regs.arm64.lr = regs->lr;
+        req->data.regs.arm64.cpsr = regs->cpsr;
+        req->data.regs.arm64.spsr_el1 = regs->spsr_svc;
+        req->data.regs.arm64.ttbr0 = READ_SYSREG64(TTBR0_EL1);
+        req->data.regs.arm64.ttbr1 = READ_SYSREG64(TTBR1_EL1);
+    }
+#endif
+}
+
+void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
+{
+    struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;
+
+    if ( is_32bit_domain(v->domain) )
+    {
+        regs->r0 = rsp->data.regs.arm32.r0_usr;
+        regs->r1 = rsp->data.regs.arm32.r1_usr;
+        regs->r2 = rsp->data.regs.arm32.r2_usr;
+        regs->r3 = rsp->data.regs.arm32.r3_usr;
+        regs->r4 = rsp->data.regs.arm32.r4_usr;
+        regs->r5 = rsp->data.regs.arm32.r5_usr;
+        regs->r6 = rsp->data.regs.arm32.r6_usr;
+        regs->r7 = rsp->data.regs.arm32.r7_usr;
+        regs->r8 = rsp->data.regs.arm32.r8_usr;
+        regs->r9 = rsp->data.regs.arm32.r9_usr;
+        regs->r10 = rsp->data.regs.arm32.r10_usr;
+        regs->r12 = rsp->data.regs.arm32.r12_usr;
+        regs->pc32 = rsp->data.regs.arm32.pc;
+        regs->sp_usr = rsp->data.regs.arm32.sp_usr;
+        regs->sp_svc = rsp->data.regs.arm32.sp_svc;
+        regs->fp = rsp->data.regs.arm32.fp;
+        regs->lr_usr = rsp->data.regs.arm32.lr_usr;
+        regs->cpsr = rsp->data.regs.arm32.cpsr;
+        v->arch.ttbr0 = rsp->data.regs.arm32.ttbr0;
+        v->arch.ttbr1 = rsp->data.regs.arm32.ttbr1;
+    }
+#ifdef CONFIG_ARM_64
+    else
+    {
+        regs->x0 = rsp->data.regs.arm64.x0;
+        regs->x1 = rsp->data.regs.arm64.x1;
+        regs->x2 = rsp->data.regs.arm64.x2;
+        regs->x3 = rsp->data.regs.arm64.x3;
+        regs->x4 = rsp->data.regs.arm64.x4;
+        regs->x5 = rsp->data.regs.arm64.x5;
+        regs->x6 = rsp->data.regs.arm64.x6;
+        regs->x7 = rsp->data.regs.arm64.x7;
+        regs->x8 = rsp->data.regs.arm64.x8;
+        regs->x9 = rsp->data.regs.arm64.x9;
+        regs->x10 = rsp->data.regs.arm64.x10;
+        regs->x16 = rsp->data.regs.arm64.x16;
+        regs->pc = rsp->data.regs.arm64.pc;
+        regs->sp_el0 = rsp->data.regs.arm64.sp_el0;
+        regs->sp_el1 = rsp->data.regs.arm64.sp_el1;
+        regs->fp = rsp->data.regs.arm64.fp;
+        regs->lr = rsp->data.regs.arm64.lr;
+        regs->cpsr = rsp->data.regs.arm64.cpsr;
+        v->arch.ttbr0 = rsp->data.regs.arm64.ttbr0;
+        v->arch.ttbr1 = rsp->data.regs.arm64.ttbr1;
+    }
+#endif
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index a3fc4ce..814d0da 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -48,15 +48,10 @@ void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
     /* Not supported on ARM. */
 }
 
-static inline
-void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
-{
-    /* Not supported on ARM. */
-}
+void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
 
-static inline void vm_event_fill_regs(vm_event_request_t *req)
-{
-    /* Not supported on ARM. */
-}
+void vm_event_fill_regs(vm_event_request_t *req,
+                        const struct cpu_user_regs *regs,
+                        struct domain *d);
 
 #endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 3acf217..fabeee8 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -129,8 +129,8 @@
 #define VM_EVENT_X86_XCR0   3
 
 /*
- * Using a custom struct (not hvm_hw_cpu) so as to not fill
- * the vm_event ring buffer too quickly.
+ * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM
+ * so as to not fill the vm_event ring buffer too quickly.
  */
 struct vm_event_regs_x86 {
     uint64_t rax;
@@ -168,6 +168,54 @@ struct vm_event_regs_x86 {
     uint32_t _pad;
 };
 
+struct vm_event_regs_arm32 {
+    uint32_t r0_usr;
+    uint32_t r1_usr;
+    uint32_t r2_usr;
+    uint32_t r3_usr;
+    uint32_t r4_usr;
+    uint32_t r5_usr;
+    uint32_t r6_usr;
+    uint32_t r7_usr;
+    uint32_t r8_usr;
+    uint32_t r9_usr;
+    uint32_t r10_usr;
+    uint32_t r12_usr;
+    uint32_t lr_usr;
+    uint32_t sp_usr;
+    uint32_t sp_svc;
+    uint32_t spsr_svc;
+    uint32_t fp;
+    uint32_t pc;
+    uint32_t cpsr;
+    uint32_t ttbr0;
+    uint32_t ttbr1;
+};
+
+struct vm_event_regs_arm64 {
+    uint64_t x0;
+    uint64_t x1;
+    uint64_t x2;
+    uint64_t x3;
+    uint64_t x4;
+    uint64_t x5;
+    uint64_t x6;
+    uint64_t x7;
+    uint64_t x8;
+    uint64_t x9;
+    uint64_t x10;
+    uint64_t x16;
+    uint64_t lr;
+    uint64_t sp_el0;
+    uint64_t sp_el1;
+    uint32_t spsr_el1;
+    uint64_t fp;
+    uint64_t pc;
+    uint32_t cpsr;
+    uint64_t ttbr0;
+    uint64_t ttbr1;
+};
+
 /*
  * mem_access flag definitions
  *
@@ -256,6 +304,8 @@ typedef struct vm_event_st {
     union {
         union {
             struct vm_event_regs_x86 x86;
+            struct vm_event_regs_arm32 arm32;
+            struct vm_event_regs_arm64 arm64;
         } regs;
 
         struct vm_event_emul_read_data emul_read_data;
-- 
2.8.1


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

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

* [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
                   ` (2 preceding siblings ...)
  2016-05-04 14:51 ` [PATCH v3 4/9] arm/vm_event: get/set registers Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-04 20:08   ` Konrad Rzeszutek Wilk
  2016-05-04 14:51 ` [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Tamas K Lengyel, Ian Jackson

These are the user-space components for the new ARM SMC events.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxc/include/xenctrl.h |  2 ++
 tools/libxc/xc_monitor.c      | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 42f201b..4b75ae4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2160,6 +2160,8 @@ int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
                                    bool enable);
 int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
                              bool enable, bool sync);
+int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
+                               bool enable);
 
 /**
  * This function enables / disables emulation for each REP for a
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b1705dd..2cb4247 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -156,3 +156,27 @@ int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
 
     return do_domctl(xch, &domctl);
 }
+
+int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
+                               bool enable)
+{
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_monitor_op;
+    domctl.domain = domain_id;
+    domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+                                    : XEN_DOMCTL_MONITOR_OP_DISABLE;
+    domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL;
+
+    return do_domctl(xch, &domctl);
+}
+
+/*
++ * Local variables:
++ * mode: C
++ * c-file-style: "BSD"
++ * c-basic-offset: 4
++ * tab-width: 4
++ * indent-tabs-mode: nil
++ * End:
++ */
-- 
2.8.1


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

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

* [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
                   ` (3 preceding siblings ...)
  2016-05-04 14:51 ` [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-04 15:35   ` Jan Beulich
  2016-05-05  9:37   ` Razvan Cojocaru
  2016-05-04 14:51 ` [PATCH v3 7/9] x86/hvm: Rename hvm/event to hvm/monitor Tamas K Lengyel
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Tamas K Lengyel, Ian Jackson, Razvan Cojocaru

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/tests/xen-access/xen-access.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index f26e723..db65846 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -1,4 +1,3 @@
-/*
  * xen-access.c
  *
  * Exercises the basic per-page access mechanisms
@@ -334,6 +333,8 @@ void usage(char* progname)
     fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
             fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec");
+#elif defined(__arm__) || defined(__aarch64__)
+            fprintf(stderr, "|privcall");
 #endif
             fprintf(stderr,
             "\n"
@@ -357,6 +358,7 @@ int main(int argc, char *argv[])
     int required = 0;
     int breakpoint = 0;
     int shutting_down = 0;
+    int privcall = 0;
     int altp2m = 0;
     uint16_t altp2m_view_id = 0;
 
@@ -412,6 +414,11 @@ int main(int argc, char *argv[])
         default_access = XENMEM_access_rw;
         altp2m = 1;
     }
+#elif defined(__arm__) || defined(__aarch64__)
+    else if ( !strcmp(argv[0], "privcall") )
+    {
+        privcall = 1;
+    }
 #endif
     else
     {
@@ -524,6 +531,16 @@ int main(int argc, char *argv[])
         }
     }
 
+    if ( privcall )
+    {
+        rc = xc_monitor_privileged_call(xch, domain_id, 1);
+        if ( rc < 0 )
+        {
+            ERROR("Error %d setting privileged call trapping with vm_event\n", rc);
+            goto exit;
+        }
+    }
+
     /* Wait for access */
     for (;;)
     {
@@ -535,6 +552,9 @@ int main(int argc, char *argv[])
             if ( breakpoint )
                 rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
 
+            if ( privcall )
+                rc = xc_monitor_privileged_call(xch, domain_id, 0);
+
             if ( altp2m )
             {
                 rc = xc_altp2m_switch_to_view( xch, domain_id, 0 );
@@ -635,7 +655,7 @@ int main(int argc, char *argv[])
                 rsp.u.mem_access = req.u.mem_access;
                 break;
             case VM_EVENT_REASON_SOFTWARE_BREAKPOINT:
-                printf("Breakpoint: rip=%016"PRIx64", gfn=%"PRIx64" (vcpu %d)\n",
+                printf("Breakpoint: rip=%"PRIx64" gfn=%"PRIx64" (vcpu %d)\n",
                        req.data.regs.x86.rip,
                        req.u.software_breakpoint.gfn,
                        req.vcpu_id);
@@ -650,7 +670,15 @@ int main(int argc, char *argv[])
                     interrupted = -1;
                     continue;
                 }
+                break;
+            case VM_EVENT_REASON_PRIVILEGED_CALL:
+                printf("Privileged call: pc=%"PRIx32" (vcpu %d)\n",
+                       req.data.regs.arm32.pc,
+                       req.vcpu_id);
 
+                rsp.data.regs.arm32 = req.data.regs.arm32;
+                rsp.data.regs.arm32.pc += 4;
+                rsp.flags |= VM_EVENT_FLAG_SET_REGISTERS;
                 break;
             case VM_EVENT_REASON_SINGLESTEP:
                 printf("Singlestep: rip=%016"PRIx64", vcpu %d, altp2m %u\n",
-- 
2.8.1


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

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

* [PATCH v3 7/9] x86/hvm: Rename hvm/event to hvm/monitor
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
                   ` (4 preceding siblings ...)
  2016-05-04 14:51 ` [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-05  9:39   ` Razvan Cojocaru
  2016-05-04 14:51 ` [PATCH v3 8/9] x86/hvm: Add debug exception vm_events Tamas K Lengyel
  2016-05-04 14:51 ` [PATCH v3 9/9] MAINTAINERS: Update monitor/vm_event covered code Tamas K Lengyel
  7 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Jun Nakajima, Andrew Cooper, Tamas K Lengyel, Razvan Cojocaru,
	Jan Beulich

Mechanical renaming to better describe that the code in hvm/event is part of
the monitor subsystem.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/hvm/Makefile                      |  2 +-
 xen/arch/x86/hvm/hvm.c                         | 12 +++++------
 xen/arch/x86/hvm/{event.c => monitor.c}        | 17 ++++++++-------
 xen/arch/x86/hvm/vmx/vmx.c                     | 13 +++++------
 xen/include/asm-x86/hvm/{event.h => monitor.h} | 30 +++++++++++++-------------
 5 files changed, 38 insertions(+), 36 deletions(-)
 rename xen/arch/x86/hvm/{event.c => monitor.c} (88%)
 rename xen/include/asm-x86/hvm/{event.h => monitor.h} (60%)

diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 8bc55a9..f750d13 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -3,7 +3,6 @@ subdir-y += vmx
 
 obj-y += asid.o
 obj-y += emulate.o
-obj-y += event.o
 obj-y += hpet.o
 obj-y += hvm.o
 obj-y += i8254.o
@@ -11,6 +10,7 @@ obj-y += intercept.o
 obj-y += io.o
 obj-y += ioreq.o
 obj-y += irq.o
+obj-y += monitor.o
 obj-y += mtrr.o
 obj-y += nestedhvm.o
 obj-y += pmtimer.o
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5d740f7..2689378 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -60,7 +60,7 @@
 #include <asm/hvm/cacheattr.h>
 #include <asm/hvm/trace.h>
 #include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/event.h>
+#include <asm/hvm/monitor.h>
 #include <asm/hvm/ioreq.h>
 #include <asm/hvm/vmx/vmx.h>
 #include <asm/altp2m.h>
@@ -1961,7 +1961,7 @@ int hvm_handle_xsetbv(u32 index, u64 new_bv)
 {
     int rc;
 
-    hvm_event_crX(XCR0, new_bv, current->arch.xcr0);
+    hvm_monitor_crX(XCR0, new_bv, current->arch.xcr0);
 
     rc = handle_xsetbv(index, new_bv);
     if ( rc )
@@ -2197,7 +2197,7 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
     {
         ASSERT(v->arch.vm_event);
 
-        if ( hvm_event_crX(CR0, value, old_value) )
+        if ( hvm_monitor_crX(CR0, value, old_value) )
         {
             /* The actual write will occur in hvm_do_resume(), if permitted. */
             v->arch.vm_event->write_data.do_write.cr0 = 1;
@@ -2299,7 +2299,7 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
     {
         ASSERT(v->arch.vm_event);
 
-        if ( hvm_event_crX(CR3, value, old) )
+        if ( hvm_monitor_crX(CR3, value, old) )
         {
             /* The actual write will occur in hvm_do_resume(), if permitted. */
             v->arch.vm_event->write_data.do_write.cr3 = 1;
@@ -2379,7 +2379,7 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
     {
         ASSERT(v->arch.vm_event);
 
-        if ( hvm_event_crX(CR4, value, old_cr) )
+        if ( hvm_monitor_crX(CR4, value, old_cr) )
         {
             /* The actual write will occur in hvm_do_resume(), if permitted. */
             v->arch.vm_event->write_data.do_write.cr4 = 1;
@@ -3712,7 +3712,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
         v->arch.vm_event->write_data.msr = msr;
         v->arch.vm_event->write_data.value = msr_content;
 
-        hvm_event_msr(msr, msr_content);
+        hvm_monitor_msr(msr, msr_content);
         return X86EMUL_OKAY;
     }
 
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/monitor.c
similarity index 88%
rename from xen/arch/x86/hvm/event.c
rename to xen/arch/x86/hvm/monitor.c
index 73d0a26..19048f4 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -1,5 +1,5 @@
 /*
- * arch/x86/hvm/event.c
+ * arch/x86/hvm/monitor.c
  *
  * Arch-specific hardware virtual machine event abstractions.
  *
@@ -7,6 +7,7 @@
  * Copyright (c) 2005, International Business Machines Corporation.
  * Copyright (c) 2008, Citrix Systems, Inc.
  * Copyright (c) 2016, Bitdefender S.R.L.
+ * Copyright (c) 2016, Tamas K Lengyel (tamas@tklengyel.com)
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -22,12 +23,12 @@
  */
 
 #include <xen/vm_event.h>
-#include <asm/hvm/event.h>
+#include <asm/hvm/monitor.h>
 #include <asm/monitor.h>
 #include <asm/vm_event.h>
 #include <public/vm_event.h>
 
-bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
+bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old)
 {
     struct vcpu *curr = current;
     struct arch_domain *ad = &curr->domain->arch;
@@ -55,7 +56,7 @@ bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
     return 0;
 }
 
-void hvm_event_msr(unsigned int msr, uint64_t value)
+void hvm_monitor_msr(unsigned int msr, uint64_t value)
 {
     struct vcpu *curr = current;
     struct arch_domain *ad = &curr->domain->arch;
@@ -89,8 +90,8 @@ static inline unsigned long gfn_of_rip(unsigned long rip)
     return paging_gva_to_gfn(curr, sreg.base + rip, &pfec);
 }
 
-int hvm_event_breakpoint(unsigned long rip,
-                         enum hvm_event_breakpoint_type type)
+int hvm_monitor_breakpoint(unsigned long rip,
+                         enum hvm_monitor_breakpoint_type type)
 {
     struct vcpu *curr = current;
     struct arch_domain *ad = &curr->domain->arch;
@@ -98,14 +99,14 @@ int hvm_event_breakpoint(unsigned long rip,
 
     switch ( type )
     {
-    case HVM_EVENT_SOFTWARE_BREAKPOINT:
+    case HVM_MONITOR_SOFTWARE_BREAKPOINT:
         if ( !ad->monitor.software_breakpoint_enabled )
             return 0;
         req.reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT;
         req.u.software_breakpoint.gfn = gfn_of_rip(rip);
         break;
 
-    case HVM_EVENT_SINGLESTEP_BREAKPOINT:
+    case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
         if ( !ad->monitor.singlestep_enabled )
             return 0;
         req.reason = VM_EVENT_REASON_SINGLESTEP;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index bc4410f..476a606 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -50,7 +50,7 @@
 #include <asm/hvm/vpt.h>
 #include <public/hvm/save.h>
 #include <asm/hvm/trace.h>
-#include <asm/hvm/event.h>
+#include <asm/hvm/monitor.h>
 #include <asm/xenoprof.h>
 #include <asm/debugger.h>
 #include <asm/apic.h>
@@ -2473,10 +2473,10 @@ static int vmx_cr_access(unsigned long exit_qualification)
 
         /*
          * Special case unlikely to be interesting to a
-         * VM_EVENT_FLAG_DENY-capable application, so the hvm_event_crX()
+         * VM_EVENT_FLAG_DENY-capable application, so the hvm_monitor_crX()
          * return value is ignored for now.
          */
-        hvm_event_crX(CR0, value, old);
+        hvm_monitor_crX(CR0, value, old);
         curr->arch.hvm_vcpu.guest_cr[0] = value;
         vmx_update_guest_cr(curr, 0);
         HVMTRACE_0D(CLTS);
@@ -3389,8 +3389,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
             }
             else {
                 int handled =
-                        hvm_event_breakpoint(regs->eip,
-                                             HVM_EVENT_SOFTWARE_BREAKPOINT);
+                    hvm_monitor_breakpoint(regs->eip,
+                                           HVM_MONITOR_SOFTWARE_BREAKPOINT);
 
                 if ( handled < 0 ) 
                 {
@@ -3717,7 +3717,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
         vmx_update_cpu_exec_control(v);
         if ( v->arch.hvm_vcpu.single_step )
         {
-            hvm_event_breakpoint(regs->eip, HVM_EVENT_SINGLESTEP_BREAKPOINT);
+            hvm_monitor_breakpoint(regs->eip,
+                                   HVM_MONITOR_SINGLESTEP_BREAKPOINT);
             if ( v->domain->debugger_attached )
                 domain_pause_for_debugger();
         }
diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/monitor.h
similarity index 60%
rename from xen/include/asm-x86/hvm/event.h
rename to xen/include/asm-x86/hvm/monitor.h
index 03f7fee..1d3c668 100644
--- a/xen/include/asm-x86/hvm/event.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -1,7 +1,7 @@
 /*
- * include/asm-x86/hvm/event.h
+ * include/asm-x86/hvm/monitor.h
  *
- * Arch-specific hardware virtual machine event abstractions.
+ * Arch-specific hardware virtual machine monitor abstractions.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -16,17 +16,17 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef __ASM_X86_HVM_EVENT_H__
-#define __ASM_X86_HVM_EVENT_H__
+#ifndef __ASM_X86_HVM_MONITOR_H__
+#define __ASM_X86_HVM_MONITOR_H__
 
 #include <xen/sched.h>
 #include <xen/paging.h>
 #include <public/vm_event.h>
 
-enum hvm_event_breakpoint_type
+enum hvm_monitor_breakpoint_type
 {
-    HVM_EVENT_SOFTWARE_BREAKPOINT,
-    HVM_EVENT_SINGLESTEP_BREAKPOINT,
+    HVM_MONITOR_SOFTWARE_BREAKPOINT,
+    HVM_MONITOR_SINGLESTEP_BREAKPOINT,
 };
 
 /*
@@ -34,15 +34,15 @@ enum hvm_event_breakpoint_type
  * The event might not fire if the client has subscribed to it in onchangeonly
  * mode, hence the bool_t return type for control register write events.
  */
-bool_t hvm_event_cr(unsigned int index, unsigned long value,
-                    unsigned long old);
-#define hvm_event_crX(cr, new, old) \
-    hvm_event_cr(VM_EVENT_X86_##cr, new, old)
-void hvm_event_msr(unsigned int msr, uint64_t value);
-int hvm_event_breakpoint(unsigned long rip,
-                         enum hvm_event_breakpoint_type type);
+bool_t hvm_monitor_cr(unsigned int index, unsigned long value,
+                      unsigned long old);
+#define hvm_monitor_crX(cr, new, old) \
+    hvm_monitor_cr(VM_EVENT_X86_##cr, new, old)
+void hvm_monitor_msr(unsigned int msr, uint64_t value);
+int hvm_monitor_breakpoint(unsigned long rip,
+                           enum hvm_monitor_breakpoint_type type);
 
-#endif /* __ASM_X86_HVM_EVENT_H__ */
+#endif /* __ASM_X86_HVM_MONITOR_H__ */
 
 /*
  * Local variables:
-- 
2.8.1


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

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

* [PATCH v3 8/9] x86/hvm: Add debug exception vm_events
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
                   ` (5 preceding siblings ...)
  2016-05-04 14:51 ` [PATCH v3 7/9] x86/hvm: Rename hvm/event to hvm/monitor Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-05  9:56   ` Razvan Cojocaru
  2016-05-04 14:51 ` [PATCH v3 9/9] MAINTAINERS: Update monitor/vm_event covered code Tamas K Lengyel
  7 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Tamas K Lengyel, Wei Liu, Jun Nakajima,
	Razvan Cojocaru, Andrew Cooper, Ian Jackson, Jan Beulich

Since in-guest debug exceptions are now unconditionally trapped to Xen, adding
a hook for vm_event subscribers to tap into this new always-on guest event. We
rename along the way hvm_event_breakpoint_type to hvm_event_type to better
match the various events that can be passed with it. We also introduce the
necessary monitor_op domctl's to enable subscribing to the events.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>

v3: Increment vm_event interface version
v2: Rename hvm_monitor_event to hvm_monitor_debug
---
 tools/libxc/include/xenctrl.h       |  3 +-
 tools/libxc/xc_monitor.c            | 15 +++++++++
 tools/tests/xen-access/xen-access.c | 61 ++++++++++++++++++++++++++++++++-----
 xen/arch/x86/hvm/monitor.c          | 26 ++++++++++++++--
 xen/arch/x86/hvm/vmx/vmx.c          | 26 +++++++++++++---
 xen/arch/x86/monitor.c              | 16 ++++++++++
 xen/include/asm-x86/domain.h        |  2 ++
 xen/include/asm-x86/hvm/monitor.h   |  7 +++--
 xen/include/asm-x86/monitor.h       |  3 +-
 xen/include/public/domctl.h         |  6 ++++
 xen/include/public/vm_event.h       | 14 +++++++--
 11 files changed, 157 insertions(+), 22 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4b75ae4..ff3ba9e 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2162,7 +2162,8 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
                              bool enable, bool sync);
 int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
                                bool enable);
-
+int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
+                                bool enable, bool sync);
 /**
  * This function enables / disables emulation for each REP for a
  * REP-compatible instruction.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index 2cb4247..76b9d2c 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -171,6 +171,21 @@ int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
     return do_domctl(xch, &domctl);
 }
 
+int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
+                                bool enable, bool sync)
+{
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_monitor_op;
+    domctl.domain = domain_id;
+    domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+                                    : XEN_DOMCTL_MONITOR_OP_DISABLE;
+    domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION;
+    domctl.u.monitor_op.u.debug_exception.sync = sync;
+
+    return do_domctl(xch, &domctl);
+}
+
 /*
 + * Local variables:
 + * mode: C
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index db65846..64ce82f 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -52,6 +52,10 @@
 #define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
 #define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
 
+/* From xen/include/asm-x86/processor.h */
+#define X86_TRAP_DEBUG  1
+#define X86_TRAP_INT3   3
+
 typedef struct vm_event {
     domid_t domain_id;
     xenevtchn_handle *xce_handle;
@@ -332,7 +336,7 @@ void usage(char* progname)
 {
     fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
-            fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec");
+            fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
 #elif defined(__arm__) || defined(__aarch64__)
             fprintf(stderr, "|privcall");
 #endif
@@ -355,11 +359,13 @@ int main(int argc, char *argv[])
     xc_interface *xch;
     xenmem_access_t default_access = XENMEM_access_rwx;
     xenmem_access_t after_first_access = XENMEM_access_rwx;
+    int memaccess = 0;
     int required = 0;
     int breakpoint = 0;
     int shutting_down = 0;
     int privcall = 0;
     int altp2m = 0;
+    int debug = 0;
     uint16_t altp2m_view_id = 0;
 
     char* progname = argv[0];
@@ -393,11 +399,13 @@ int main(int argc, char *argv[])
     {
         default_access = XENMEM_access_rx;
         after_first_access = XENMEM_access_rwx;
+        memaccess = 1;
     }
     else if ( !strcmp(argv[0], "exec") )
     {
         default_access = XENMEM_access_rw;
         after_first_access = XENMEM_access_rwx;
+        memaccess = 1;
     }
 #if defined(__i386__) || defined(__x86_64__)
     else if ( !strcmp(argv[0], "breakpoint") )
@@ -408,11 +416,17 @@ int main(int argc, char *argv[])
     {
         default_access = XENMEM_access_rx;
         altp2m = 1;
+        memaccess = 1;
     }
     else if ( !strcmp(argv[0], "altp2m_exec") )
     {
         default_access = XENMEM_access_rw;
         altp2m = 1;
+        memaccess = 1;
+    }
+    else if ( !strcmp(argv[0], "debug") )
+    {
+        debug = 1;
     }
 #elif defined(__arm__) || defined(__aarch64__)
     else if ( !strcmp(argv[0], "privcall") )
@@ -453,7 +467,7 @@ int main(int argc, char *argv[])
     }
 
     /* With altp2m we just create a new, restricted view of the memory */
-    if ( altp2m )
+    if ( memaccess && altp2m )
     {
         xen_pfn_t gfn = 0;
         unsigned long perm_set = 0;
@@ -500,7 +514,7 @@ int main(int argc, char *argv[])
         }
     }
 
-    if ( !altp2m )
+    if ( memaccess && !altp2m )
     {
         /* Set the default access type and convert all pages to it */
         rc = xc_set_mem_access(xch, domain_id, default_access, ~0ull, 0);
@@ -541,6 +555,16 @@ int main(int argc, char *argv[])
         }
     }
 
+    if ( debug )
+    {
+        rc = xc_monitor_debug_exceptions(xch, domain_id, 1, 1);
+        if ( rc < 0 )
+        {
+            ERROR("Error %d setting debug exception listener with vm_event\n", rc);
+            goto exit;
+        }
+    }
+
     /* Wait for access */
     for (;;)
     {
@@ -551,9 +575,10 @@ int main(int argc, char *argv[])
 
             if ( breakpoint )
                 rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
-
             if ( privcall )
                 rc = xc_monitor_privileged_call(xch, domain_id, 0);
+            if ( debug )
+                rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
 
             if ( altp2m )
             {
@@ -661,9 +686,10 @@ int main(int argc, char *argv[])
                        req.vcpu_id);
 
                 /* Reinject */
-                rc = xc_hvm_inject_trap(
-                    xch, domain_id, req.vcpu_id, 3,
-                    HVMOP_TRAP_sw_exc, -1, 0, 0);
+                rc = xc_hvm_inject_trap(xch, domain_id, req.vcpu_id,
+                                        X86_TRAP_INT3,
+                                        req.u.software_breakpoint.type, -1,
+                                        req.u.software_breakpoint.insn_length, 0);
                 if (rc < 0)
                 {
                     ERROR("Error %d injecting breakpoint\n", rc);
@@ -697,6 +723,27 @@ int main(int argc, char *argv[])
                 rsp.flags |= VM_EVENT_FLAG_TOGGLE_SINGLESTEP;
 
                 break;
+            case VM_EVENT_REASON_DEBUG_EXCEPTION:
+                printf("Debug exception: rip=%016"PRIx64", vcpu %d. Type: %u. Length: %u\n",
+                       req.data.regs.x86.rip,
+                       req.vcpu_id,
+                       req.u.debug_exception.type,
+                       req.u.debug_exception.insn_length);
+
+                /* Reinject */
+                rc = xc_hvm_inject_trap(xch, domain_id, req.vcpu_id,
+                                        X86_TRAP_DEBUG,
+                                        req.u.debug_exception.type, -1,
+                                        req.u.debug_exception.insn_length,
+                                        req.data.regs.x86.cr2);
+                if (rc < 0)
+                {
+                    ERROR("Error %d injecting breakpoint\n", rc);
+                    interrupted = -1;
+                    continue;
+                }
+
+                break;
             default:
                 fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
             }
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 19048f4..4069dff 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -90,12 +90,14 @@ static inline unsigned long gfn_of_rip(unsigned long rip)
     return paging_gva_to_gfn(curr, sreg.base + rip, &pfec);
 }
 
-int hvm_monitor_breakpoint(unsigned long rip,
-                         enum hvm_monitor_breakpoint_type type)
+int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
+                      unsigned long trap_type, unsigned long insn_length)
 {
     struct vcpu *curr = current;
     struct arch_domain *ad = &curr->domain->arch;
     vm_event_request_t req = {};
+    bool_t sync;
+    int rc;
 
     switch ( type )
     {
@@ -104,6 +106,9 @@ int hvm_monitor_breakpoint(unsigned long rip,
             return 0;
         req.reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT;
         req.u.software_breakpoint.gfn = gfn_of_rip(rip);
+        req.u.software_breakpoint.type = trap_type;
+        req.u.software_breakpoint.insn_length = insn_length;
+        sync = 1;
         break;
 
     case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
@@ -111,6 +116,17 @@ int hvm_monitor_breakpoint(unsigned long rip,
             return 0;
         req.reason = VM_EVENT_REASON_SINGLESTEP;
         req.u.singlestep.gfn = gfn_of_rip(rip);
+        sync = 1;
+        break;
+
+    case HVM_MONITOR_DEBUG_EXCEPTION:
+        if ( !ad->monitor.debug_exception_enabled )
+            return 0;
+        req.reason = VM_EVENT_REASON_DEBUG_EXCEPTION;
+        req.u.debug_exception.gfn = gfn_of_rip(rip);
+        req.u.debug_exception.type = trap_type;
+        req.u.debug_exception.insn_length = insn_length;
+        sync = !!ad->monitor.debug_exception_sync;
         break;
 
     default:
@@ -120,7 +136,11 @@ int hvm_monitor_breakpoint(unsigned long rip,
     req.vcpu_id = curr->vcpu_id;
     vm_event_fill_regs(&req);
 
-    return vm_event_monitor_traps(curr, 1, &req);
+    rc = vm_event_monitor_traps(curr, sync, &req);
+    if ( type == HVM_MONITOR_DEBUG_EXCEPTION && rc == 1 && !sync )
+        rc = 0;
+
+    return rc;
 }
 
 /*
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 476a606..ac546ab 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3373,9 +3373,25 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
             HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
             write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
             if ( !v->domain->debugger_attached )
-                vmx_propagate_intr(intr_info);
+            {
+                unsigned long insn_length = 0;
+                int handled;
+                unsigned long trap_type = MASK_EXTR(intr_info,
+                                                    INTR_INFO_INTR_TYPE_MASK);
+
+                if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
+                    __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
+
+                handled = hvm_monitor_debug(regs->eip,
+                                            HVM_MONITOR_DEBUG_EXCEPTION,
+                                            trap_type, insn_length);
+                if ( handled <= 0 )
+                    vmx_propagate_intr(intr_info);
+
+            }
             else
                 domain_pause_for_debugger();
+
             break;
         case TRAP_int3: 
         {
@@ -3389,8 +3405,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
             }
             else {
                 int handled =
-                    hvm_monitor_breakpoint(regs->eip,
-                                           HVM_MONITOR_SOFTWARE_BREAKPOINT);
+                        hvm_monitor_debug(regs->eip,
+                                          HVM_MONITOR_SOFTWARE_BREAKPOINT,
+                                          X86_EVENTTYPE_SW_EXCEPTION, 1);
 
                 if ( handled < 0 ) 
                 {
@@ -3717,8 +3734,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
         vmx_update_cpu_exec_control(v);
         if ( v->arch.hvm_vcpu.single_step )
         {
-            hvm_monitor_breakpoint(regs->eip,
-                                   HVM_MONITOR_SINGLESTEP_BREAKPOINT);
+            hvm_monitor_debug(regs->eip, HVM_MONITOR_SINGLESTEP_BREAKPOINT, 0, 0);
             if ( v->domain->debugger_attached )
                 domain_pause_for_debugger();
         }
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 8598049..acde759 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -125,6 +125,22 @@ int arch_monitor_domctl_event(struct domain *d,
         break;
     }
 
+    case XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION:
+    {
+        bool_t old_status = ad->monitor.debug_exception_enabled;
+
+        if ( unlikely(old_status == requested_status) )
+            return -EEXIST;
+
+        domain_pause(d);
+        ad->monitor.debug_exception_enabled = requested_status;
+        ad->monitor.debug_exception_sync = requested_status ?
+                                            mop->u.debug_exception.sync :
+                                            0;
+        domain_unpause(d);
+        break;
+    }
+
     default:
         /*
          * Should not be reached unless arch_monitor_get_capabilities() is
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 165e533..1cf97c3 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -405,6 +405,8 @@ struct arch_domain
         unsigned int mov_to_msr_extended         : 1;
         unsigned int singlestep_enabled          : 1;
         unsigned int software_breakpoint_enabled : 1;
+        unsigned int debug_exception_enabled     : 1;
+        unsigned int debug_exception_sync        : 1;
     } monitor;
 
     /* Mem_access emulation control */
diff --git a/xen/include/asm-x86/hvm/monitor.h b/xen/include/asm-x86/hvm/monitor.h
index 1d3c668..422bc11 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -23,10 +23,11 @@
 #include <xen/paging.h>
 #include <public/vm_event.h>
 
-enum hvm_monitor_breakpoint_type
+enum hvm_monitor_debug_type
 {
     HVM_MONITOR_SOFTWARE_BREAKPOINT,
     HVM_MONITOR_SINGLESTEP_BREAKPOINT,
+    HVM_MONITOR_DEBUG_EXCEPTION,
 };
 
 /*
@@ -39,8 +40,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value,
 #define hvm_monitor_crX(cr, new, old) \
     hvm_monitor_cr(VM_EVENT_X86_##cr, new, old)
 void hvm_monitor_msr(unsigned int msr, uint64_t value);
-int hvm_monitor_breakpoint(unsigned long rip,
-                           enum hvm_monitor_breakpoint_type type);
+int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
+                      unsigned long trap_type, unsigned long insn_length);
 
 #endif /* __ASM_X86_HVM_MONITOR_H__ */
 
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 446b2af..240a94e 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -64,7 +64,8 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
     capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
                    (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
                    (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION);
 
     /* Since we know this is on VMX, we can just call the hvm func */
     if ( hvm_is_singlestep_supported() )
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 35adce2..b69b1bb 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1081,6 +1081,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
 #define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT   3
 #define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST         4
 #define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL       5
+#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION       6
 
 struct xen_domctl_monitor_op {
     uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
@@ -1116,6 +1117,11 @@ struct xen_domctl_monitor_op {
             /* Pause vCPU until response */
             uint8_t sync;
         } guest_request;
+
+        struct {
+            /* Pause vCPU until response */
+            uint8_t sync;
+        } debug_exception;
     } u;
 };
 typedef struct xen_domctl_monitor_op xen_domctl_monitor_op_t;
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index fabeee8..17dfa61 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -29,7 +29,7 @@
 
 #include "xen.h"
 
-#define VM_EVENT_INTERFACE_VERSION 0x00000001
+#define VM_EVENT_INTERFACE_VERSION 0x00000002
 
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
 
@@ -121,6 +121,8 @@
 #define VM_EVENT_REASON_GUEST_REQUEST           8
 /* Privileged call executed (e.g. SMC) */
 #define VM_EVENT_REASON_PRIVILEGED_CALL         9
+/* A debug exception was caught */
+#define VM_EVENT_REASON_DEBUG_EXCEPTION         10
 
 /* Supported values for the vm_event_write_ctrlreg index. */
 #define VM_EVENT_X86_CR0    0
@@ -253,8 +255,15 @@ struct vm_event_write_ctrlreg {
     uint64_t old_value;
 };
 
+struct vm_event_singlestep {
+    uint64_t gfn;
+};
+
 struct vm_event_debug {
     uint64_t gfn;
+    uint8_t type;        /* HVMOP_TRAP_* */
+    uint8_t insn_length;
+    uint8_t _pad[6];
 };
 
 struct vm_event_mov_to_msr {
@@ -298,7 +307,8 @@ typedef struct vm_event_st {
         struct vm_event_write_ctrlreg         write_ctrlreg;
         struct vm_event_mov_to_msr            mov_to_msr;
         struct vm_event_debug                 software_breakpoint;
-        struct vm_event_debug                 singlestep;
+        struct vm_event_singlestep            singlestep;
+        struct vm_event_debug                 debug_exception;
     } u;
 
     union {
-- 
2.8.1


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

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

* [PATCH v3 9/9] MAINTAINERS: Update monitor/vm_event covered code
  2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
                   ` (6 preceding siblings ...)
  2016-05-04 14:51 ` [PATCH v3 8/9] x86/hvm: Add debug exception vm_events Tamas K Lengyel
@ 2016-05-04 14:51 ` Tamas K Lengyel
  2016-05-05  9:53   ` Razvan Cojocaru
  7 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 14:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Tamas K Lengyel, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Stefano Stabellini

Add headers to the covered list.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 MAINTAINERS | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5af7a0c..1a634a8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -351,16 +351,16 @@ S:	Supported
 T:	hg http://xenbits.xen.org/linux-2.6.18-xen.hg
 F:	drivers/xen/usb*/
 
-VM EVENT AND MEM ACCESS
+VM EVENT, MEM ACCESS and MONITOR
 M:	Razvan Cojocaru <rcojocaru@bitdefender.com>
 M:	Tamas K Lengyel <tamas@tklengyel.com>
 S:	Supported
-F:	xen/common/vm_event.c
 F:	xen/common/mem_access.c
-F:	xen/common/monitor.c
-F:	xen/arch/x86/hvm/event.c
-F:	xen/arch/x86/monitor.c
-F:	xen/arch/*/vm_event.c
+F:	xen/*/vm_event.c
+F:	xen/*/monitor.c
+F:	xen/include/*/mem_access.h
+F:	xen/include/*/monitor.h
+F:	xen/include/*/vm_event.h
 F:	tools/tests/xen-access
 
 VTPM
-- 
2.8.1


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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 14:51 ` [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
@ 2016-05-04 15:35   ` Jan Beulich
  2016-05-04 17:16     ` Tamas K Lengyel
  2016-05-05  9:37   ` Razvan Cojocaru
  1 sibling, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2016-05-04 15:35 UTC (permalink / raw)
  To: Tamas K Lengyel; +Cc: Ian Jackson, Wei Liu, Razvan Cojocaru, xen-devel

>>> On 04.05.16 at 16:51, <tamas@tklengyel.com> wrote:
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -1,4 +1,3 @@
> -/*
>   * xen-access.c
>   *
>   * Exercises the basic per-page access mechanisms

Are you saying this still builds with such a change?

Jan


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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 15:35   ` Jan Beulich
@ 2016-05-04 17:16     ` Tamas K Lengyel
  2016-05-04 17:33       ` Wei Liu
  2016-05-05 16:25       ` Jan Beulich
  0 siblings, 2 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 17:16 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Xen-devel, Ian Jackson, Wei Liu, Razvan Cojocaru


[-- Attachment #1.1: Type: text/plain, Size: 436 bytes --]

On May 4, 2016 09:35, "Jan Beulich" <JBeulich@suse.com> wrote:
>
> >>> On 04.05.16 at 16:51, <tamas@tklengyel.com> wrote:
> > --- a/tools/tests/xen-access/xen-access.c
> > +++ b/tools/tests/xen-access/xen-access.c
> > @@ -1,4 +1,3 @@
> > -/*
> >   * xen-access.c
> >   *
> >   * Exercises the basic per-page access mechanisms
>
> Are you saying this still builds with such a change?
>

It builds fine, so not sure what you mean.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 734 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 17:16     ` Tamas K Lengyel
@ 2016-05-04 17:33       ` Wei Liu
  2016-05-04 17:42         ` Tamas K Lengyel
  2016-05-05 16:25       ` Jan Beulich
  1 sibling, 1 reply; 31+ messages in thread
From: Wei Liu @ 2016-05-04 17:33 UTC (permalink / raw)
  To: Tamas K Lengyel
  Cc: Xen-devel, Ian Jackson, Razvan Cojocaru, Jan Beulich, Wei Liu

On Wed, May 04, 2016 at 11:16:24AM -0600, Tamas K Lengyel wrote:
> On May 4, 2016 09:35, "Jan Beulich" <JBeulich@suse.com> wrote:
> >
> > >>> On 04.05.16 at 16:51, <tamas@tklengyel.com> wrote:
> > > --- a/tools/tests/xen-access/xen-access.c
> > > +++ b/tools/tests/xen-access/xen-access.c
> > > @@ -1,4 +1,3 @@
> > > -/*

This line -- you seemed to have removed "/*". I think that's what Jan
meant.

Wei.

> > >   * xen-access.c
> > >   *
> > >   * Exercises the basic per-page access mechanisms
> >
> > Are you saying this still builds with such a change?
> >
> 
> It builds fine, so not sure what you mean.
> 
> Tamas

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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 17:33       ` Wei Liu
@ 2016-05-04 17:42         ` Tamas K Lengyel
  0 siblings, 0 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 17:42 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, Ian Jackson, Jan Beulich, Razvan Cojocaru


[-- Attachment #1.1: Type: text/plain, Size: 536 bytes --]

On May 4, 2016 11:34, "Wei Liu" <wei.liu2@citrix.com> wrote:
>
> On Wed, May 04, 2016 at 11:16:24AM -0600, Tamas K Lengyel wrote:
> > On May 4, 2016 09:35, "Jan Beulich" <JBeulich@suse.com> wrote:
> > >
> > > >>> On 04.05.16 at 16:51, <tamas@tklengyel.com> wrote:
> > > > --- a/tools/tests/xen-access/xen-access.c
> > > > +++ b/tools/tests/xen-access/xen-access.c
> > > > @@ -1,4 +1,3 @@
> > > > -/*
>
> This line -- you seemed to have removed "/*". I think that's what Jan
> meant.
>

Oops, that must be a typo from --annotate.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 912 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call
  2016-05-04 14:51 ` [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
@ 2016-05-04 20:08   ` Konrad Rzeszutek Wilk
  2016-05-04 22:12     ` Tamas K Lengyel
  0 siblings, 1 reply; 31+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-04 20:08 UTC (permalink / raw)
  To: Tamas K Lengyel; +Cc: xen-devel, Ian Jackson

> +/*
> ++ * Local variables:
> ++ * mode: C
> ++ * c-file-style: "BSD"
> ++ * c-basic-offset: 4
> ++ * tab-width: 4
> ++ * indent-tabs-mode: nil
> ++ * End:
> ++ */

++ ?

Why the extra + ?

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

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

* Re: [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call
  2016-05-04 20:08   ` Konrad Rzeszutek Wilk
@ 2016-05-04 22:12     ` Tamas K Lengyel
  0 siblings, 0 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-04 22:12 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Xen-devel, Ian Jackson

On Wed, May 4, 2016 at 2:08 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
>> +/*
>> ++ * Local variables:
>> ++ * mode: C
>> ++ * c-file-style: "BSD"
>> ++ * c-basic-offset: 4
>> ++ * tab-width: 4
>> ++ * indent-tabs-mode: nil
>> ++ * End:
>> ++ */
>
> ++ ?
>
> Why the extra + ?

Will fix.

Thanks,
Tamas

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

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

* Re: [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common
  2016-05-04 14:51 ` [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common Tamas K Lengyel
@ 2016-05-05  9:34   ` Razvan Cojocaru
  2016-05-16  9:48   ` Julien Grall
  1 sibling, 0 replies; 31+ messages in thread
From: Razvan Cojocaru @ 2016-05-05  9:34 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel
  Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich

On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> The prototype of vm_event_fill_regs will differ on x86 and ARM so in this patch
> we move components from common to arch-specific that use this function. As
> part of this patch we rename and relocate vm_event_monitor_guest_request as
> monitor_guest_request from vm_event to monitor.
> 
> Signed-off-by: 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>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> ---
>  xen/arch/arm/Makefile      |  1 +
>  xen/arch/arm/hvm.c         |  4 ++--
>  xen/arch/arm/monitor.c     | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/hvm/event.c   |  3 +++
>  xen/arch/x86/hvm/hvm.c     |  3 ++-
>  xen/arch/x86/monitor.c     | 18 +++++++++++++++++
>  xen/common/vm_event.c      | 17 ----------------
>  xen/include/xen/monitor.h  |  1 +
>  xen/include/xen/vm_event.h |  2 --
>  9 files changed, 75 insertions(+), 22 deletions(-)
>  create mode 100644 xen/arch/arm/monitor.c

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

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

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

* Re: [PATCH v3 3/9] monitor: ARM SMC events
  2016-05-04 14:51 ` [PATCH v3 3/9] monitor: ARM SMC events Tamas K Lengyel
@ 2016-05-05  9:36   ` Razvan Cojocaru
  2016-05-16  9:56   ` Julien Grall
  1 sibling, 0 replies; 31+ messages in thread
From: Razvan Cojocaru @ 2016-05-05  9:36 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel; +Cc: Julien Grall, Stefano Stabellini

On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> Add support for monitoring ARM SMC events. This patch only adds the required
> bits to enable/disable monitoring and forwarding the event through vm_event.
> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> 
> v3: Split parts off as separate patches
>     Union for arm32/64 register structs in vm_event
>     Cosmetic fixes
> ---
>  xen/arch/arm/monitor.c        | 49 +++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c          | 16 ++++++++++++--
>  xen/include/asm-arm/domain.h  |  5 +++++
>  xen/include/asm-arm/monitor.h | 24 ++++++---------------
>  xen/include/public/domctl.h   |  1 +
>  xen/include/public/vm_event.h |  2 ++
>  6 files changed, 77 insertions(+), 20 deletions(-)

The interface part looks alright, but I can't comment on the
ARM-specific bits. With that:

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

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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 14:51 ` [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
  2016-05-04 15:35   ` Jan Beulich
@ 2016-05-05  9:37   ` Razvan Cojocaru
  1 sibling, 0 replies; 31+ messages in thread
From: Razvan Cojocaru @ 2016-05-05  9:37 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel; +Cc: Wei Liu, Ian Jackson

On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/tests/xen-access/xen-access.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
> index f26e723..db65846 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -1,4 +1,3 @@
> -/*
>   * xen-access.c

With the above fixed as per Jan's comment:

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

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

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

* Re: [PATCH v3 7/9] x86/hvm: Rename hvm/event to hvm/monitor
  2016-05-04 14:51 ` [PATCH v3 7/9] x86/hvm: Rename hvm/event to hvm/monitor Tamas K Lengyel
@ 2016-05-05  9:39   ` Razvan Cojocaru
  0 siblings, 0 replies; 31+ messages in thread
From: Razvan Cojocaru @ 2016-05-05  9:39 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel; +Cc: Andrew Cooper, Jun Nakajima, Jan Beulich

On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> Mechanical renaming to better describe that the code in hvm/event is part of
> the monitor subsystem.
> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> ---
>  xen/arch/x86/hvm/Makefile                      |  2 +-
>  xen/arch/x86/hvm/hvm.c                         | 12 +++++------
>  xen/arch/x86/hvm/{event.c => monitor.c}        | 17 ++++++++-------
>  xen/arch/x86/hvm/vmx/vmx.c                     | 13 +++++------
>  xen/include/asm-x86/hvm/{event.h => monitor.h} | 30 +++++++++++++-------------
>  5 files changed, 38 insertions(+), 36 deletions(-)
>  rename xen/arch/x86/hvm/{event.c => monitor.c} (88%)
>  rename xen/include/asm-x86/hvm/{event.h => monitor.h} (60%)

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

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

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

* Re: [PATCH v3 9/9] MAINTAINERS: Update monitor/vm_event covered code
  2016-05-04 14:51 ` [PATCH v3 9/9] MAINTAINERS: Update monitor/vm_event covered code Tamas K Lengyel
@ 2016-05-05  9:53   ` Razvan Cojocaru
  0 siblings, 0 replies; 31+ messages in thread
From: Razvan Cojocaru @ 2016-05-05  9:53 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan

On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> Add headers to the covered list.
> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  MAINTAINERS | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

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

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

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

* Re: [PATCH v3 8/9] x86/hvm: Add debug exception vm_events
  2016-05-04 14:51 ` [PATCH v3 8/9] x86/hvm: Add debug exception vm_events Tamas K Lengyel
@ 2016-05-05  9:56   ` Razvan Cojocaru
  0 siblings, 0 replies; 31+ messages in thread
From: Razvan Cojocaru @ 2016-05-05  9:56 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel
  Cc: Kevin Tian, Wei Liu, Jan Beulich, Andrew Cooper, Ian Jackson,
	Jun Nakajima

On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> Since in-guest debug exceptions are now unconditionally trapped to Xen, adding
> a hook for vm_event subscribers to tap into this new always-on guest event. We
> rename along the way hvm_event_breakpoint_type to hvm_event_type to better
> match the various events that can be passed with it. We also introduce the
> necessary monitor_op domctl's to enable subscribing to the events.
> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> 
> v3: Increment vm_event interface version
> v2: Rename hvm_monitor_event to hvm_monitor_debug
> ---
>  tools/libxc/include/xenctrl.h       |  3 +-
>  tools/libxc/xc_monitor.c            | 15 +++++++++
>  tools/tests/xen-access/xen-access.c | 61 ++++++++++++++++++++++++++++++++-----
>  xen/arch/x86/hvm/monitor.c          | 26 ++++++++++++++--
>  xen/arch/x86/hvm/vmx/vmx.c          | 26 +++++++++++++---
>  xen/arch/x86/monitor.c              | 16 ++++++++++
>  xen/include/asm-x86/domain.h        |  2 ++
>  xen/include/asm-x86/hvm/monitor.h   |  7 +++--
>  xen/include/asm-x86/monitor.h       |  3 +-
>  xen/include/public/domctl.h         |  6 ++++
>  xen/include/public/vm_event.h       | 14 +++++++--
>  11 files changed, 157 insertions(+), 22 deletions(-)

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

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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
  2016-05-04 17:16     ` Tamas K Lengyel
  2016-05-04 17:33       ` Wei Liu
@ 2016-05-05 16:25       ` Jan Beulich
       [not found]         ` <CABfawh=gWOs3AtsTdYaDj61ph2jumjX6Q=0uFVeahPH99DY9qg@mail.gmail.com>
  1 sibling, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2016-05-05 16:25 UTC (permalink / raw)
  To: tamas; +Cc: ian.jackson, wei.liu2, rcojocaru, xen-devel


i.A. Jan Beulich
Software Engineering Consultant
Attachmate Group
Nördlicher Zubringer 9-11
40470 Düsseldorf
jbeulich@suse.com
SUSE
 
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

>>> Tamas K Lengyel <tamas@tklengyel.com> 05/04/16 7:18 PM >>>
>On May 4, 2016 09:35, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>> >>> On 04.05.16 at 16:51, <tamas@tklengyel.com> wrote:
>> > --- a/tools/tests/xen-access/xen-access.c
>> > +++ b/tools/tests/xen-access/xen-access.c
>> > @@ -1,4 +1,3 @@
>> > -/*
>> >   * xen-access.c
>> >   *
>> >   * Exercises the basic per-page access mechanisms
>>
>> Are you saying this still builds with such a change?
>
>It builds fine, so not sure what you mean.

Perhaps the patch here isn't what you test? I can't see how a file with the
beginning of a comment removed can build.

Jan


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

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

* Re: [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC
       [not found]           ` <CABfawhknB62vZJFvcJv6VAGzw0toZUCXBHyEnzm99+N1ZLBYEg@mail.gmail.com>
@ 2016-05-05 18:25             ` Tamas K Lengyel
  0 siblings, 0 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-05 18:25 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Xen-devel, wei.liu2, ian.jackson, rcojocaru


[-- Attachment #1.1: Type: text/plain, Size: 1116 bytes --]

On May 5, 2016 10:29, "Jan Beulich" <jbeulich@suse.com> wrote:
>
>
> i.A. Jan Beulich
> Software Engineering Consultant
> Attachmate Group
> Nördlicher Zubringer 9-11
> 40470 Düsseldorf
> jbeulich@suse.com
> SUSE
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nürnberg)
>
> >>> Tamas K Lengyel <tamas@tklengyel.com> 05/04/16 7:18 PM >>>
> >On May 4, 2016 09:35, "Jan Beulich" <JBeulich@suse.com> wrote:
> >>
> >> >>> On 04.05.16 at 16:51, <tamas@tklengyel.com> wrote:
> >> > --- a/tools/tests/xen-access/xen-access.c
> >> > +++ b/tools/tests/xen-access/xen-access.c
> >> > @@ -1,4 +1,3 @@
> >> > -/*
> >> >   * xen-access.c
> >> >   *
> >> >   * Exercises the basic per-page access mechanisms
> >>
> >> Are you saying this still builds with such a change?
> >
> >It builds fine, so not sure what you mean.
>
> Perhaps the patch here isn't what you test? I can't see how a file with
the
> beginning of a comment removed can build.
>
> Jan

Yea, the typo was introduced somewhere midway in the process.. Oh well,
will resend.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 1776 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common
  2016-05-04 14:51 ` [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common Tamas K Lengyel
  2016-05-05  9:34   ` Razvan Cojocaru
@ 2016-05-16  9:48   ` Julien Grall
  2016-05-27 18:58     ` Tamas K Lengyel
  1 sibling, 1 reply; 31+ messages in thread
From: Julien Grall @ 2016-05-16  9:48 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel
  Cc: Andrew Cooper, Stefano Stabellini, Razvan Cojocaru, Jan Beulich

Hi Tamas,

On 04/05/16 15:51, Tamas K Lengyel wrote:
> The prototype of vm_event_fill_regs will differ on x86 and ARM so in this patch
> we move components from common to arch-specific that use this function. As
> part of this patch we rename and relocate vm_event_monitor_guest_request as
> monitor_guest_request from vm_event to monitor.

Would not it be possible to find a common prototype between ARM and x86?

 From patch #4, the ARM prototype is:

void vm_event_fill_regs(vm_event_request_t *req,
                         const struct cpu_user_regs *regs,
                         struct domain *d)

and the x86 one is

void vm_event_fill_regs(vm_event_request_t *req);

The parameter "regs" will always be equal to guest_cpu_user_regs(). And 
the domain will always be current->domain.

So, IHMO, there is no need to differ between ARM and x86. This would 
also keep the code simple.

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3 3/9] monitor: ARM SMC events
  2016-05-04 14:51 ` [PATCH v3 3/9] monitor: ARM SMC events Tamas K Lengyel
  2016-05-05  9:36   ` Razvan Cojocaru
@ 2016-05-16  9:56   ` Julien Grall
  1 sibling, 0 replies; 31+ messages in thread
From: Julien Grall @ 2016-05-16  9:56 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel; +Cc: Stefano Stabellini, Razvan Cojocaru

Hi Tamas,

On 04/05/16 15:51, Tamas K Lengyel wrote:
> Add support for monitoring ARM SMC events. This patch only adds the required
> bits to enable/disable monitoring and forwarding the event through vm_event.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
>
> v3: Split parts off as separate patches
>      Union for arm32/64 register structs in vm_event
>      Cosmetic fixes
> ---
>   xen/arch/arm/monitor.c        | 49 +++++++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/traps.c          | 16 ++++++++++++--
>   xen/include/asm-arm/domain.h  |  5 +++++
>   xen/include/asm-arm/monitor.h | 24 ++++++---------------
>   xen/include/public/domctl.h   |  1 +
>   xen/include/public/vm_event.h |  2 ++
>   6 files changed, 77 insertions(+), 20 deletions(-)
>
> diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
> index f957257..9c481ac 100644
> --- a/xen/arch/arm/monitor.c
> +++ b/xen/arch/arm/monitor.c
> @@ -22,6 +22,55 @@
>   #include <asm/vm_event.h>
>   #include <public/vm_event.h>
>
> +int arch_monitor_domctl_event(struct domain *d,
> +                              struct xen_domctl_monitor_op *mop)
> +{
> +    struct arch_domain *ad = &d->arch;
> +    bool_t requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op);
> +
> +    switch ( mop->event )
> +    {
> +    case XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL:
> +    {
> +        bool_t old_status = ad->monitor.privileged_call_enabled;
> +
> +        if ( unlikely(old_status == requested_status) )
> +            return -EEXIST;
> +
> +        domain_pause(d);
> +        ad->monitor.privileged_call_enabled = requested_status;
> +        domain_unpause(d);
> +        break;
> +    }
> +
> +    default:
> +        /*
> +         * Should not be reached unless arch_monitor_get_capabilities() is
> +         * not properly implemented.
> +         */
> +        ASSERT_UNREACHABLE();
> +        return -EOPNOTSUPP;
> +    }
> +
> +    return 0;
> +}
> +
> +bool_t monitor_smc(const struct cpu_user_regs *regs) {

Coding style, the brace should be on a separate line.

> +    struct vcpu *curr = current;
> +    vm_event_request_t req = { 0 };
> +
> +    if ( !curr->domain->arch.monitor.privileged_call_enabled )
> +        return 0;
> +
> +    req.reason = VM_EVENT_REASON_PRIVILEGED_CALL;
> +    req.vcpu_id = curr->vcpu_id;
> +
> +    if ( vm_event_monitor_traps(curr, 1, &req) <= 0 )
> +        return 0;
> +    else
> +        return 1;
> +}
> +
>   void monitor_guest_request(void)
>   {
>       struct vcpu *curr = current;
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 9abfc3c..f26e12e 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -41,6 +41,7 @@
>   #include <asm/mmio.h>
>   #include <asm/cpufeature.h>
>   #include <asm/flushtlb.h>
> +#include <asm/monitor.h>
>
>   #include "decode.h"
>   #include "vtimer.h"
> @@ -2491,6 +2492,17 @@ bad_data_abort:
>       inject_dabt_exception(regs, info.gva, hsr.len);
>   }
>
> +static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> +    bool_t handled = 0;
> +
> +    if ( current->domain->arch.monitor.privileged_call_enabled )
> +        handled = monitor_smc(regs);
> +
> +    if ( handled != 1 )

handled is a boolean. So if ( !handled )

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3 4/9] arm/vm_event: get/set registers
  2016-05-04 14:51 ` [PATCH v3 4/9] arm/vm_event: get/set registers Tamas K Lengyel
@ 2016-05-16 10:14   ` Julien Grall
  2016-05-16 15:37     ` Tamas K Lengyel
  0 siblings, 1 reply; 31+ messages in thread
From: Julien Grall @ 2016-05-16 10:14 UTC (permalink / raw)
  To: Tamas K Lengyel, xen-devel; +Cc: Stefano Stabellini, Razvan Cojocaru

Hi Tamas,

On 04/05/16 15:51, Tamas K Lengyel wrote:
> diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
> index a3fc4ce..814d0da 100644
> --- a/xen/include/asm-arm/vm_event.h
> +++ b/xen/include/asm-arm/vm_event.h
> @@ -48,15 +48,10 @@ void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
>       /* Not supported on ARM. */
>   }
>
> -static inline
> -void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
> -{
> -    /* Not supported on ARM. */
> -}
> +void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
>
> -static inline void vm_event_fill_regs(vm_event_request_t *req)
> -{
> -    /* Not supported on ARM. */
> -}
> +void vm_event_fill_regs(vm_event_request_t *req,
> +                        const struct cpu_user_regs *regs,
> +                        struct domain *d);
>
>   #endif /* __ASM_ARM_VM_EVENT_H__ */
> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
> index 3acf217..fabeee8 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -129,8 +129,8 @@
>   #define VM_EVENT_X86_XCR0   3
>
>   /*
> - * Using a custom struct (not hvm_hw_cpu) so as to not fill
> - * the vm_event ring buffer too quickly.
> + * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM

You may want to rework this sentence as hvm_hw_cpu does not exist on 
ARM/ARM64.

> + * so as to not fill the vm_event ring buffer too quickly.
>    */
>   struct vm_event_regs_x86 {
>       uint64_t rax;
> @@ -168,6 +168,54 @@ struct vm_event_regs_x86 {
>       uint32_t _pad;
>   };
>
> +struct vm_event_regs_arm32 {
> +    uint32_t r0_usr;
> +    uint32_t r1_usr;
> +    uint32_t r2_usr;
> +    uint32_t r3_usr;
> +    uint32_t r4_usr;
> +    uint32_t r5_usr;
> +    uint32_t r6_usr;
> +    uint32_t r7_usr;
> +    uint32_t r8_usr;
> +    uint32_t r9_usr;
> +    uint32_t r10_usr;
> +    uint32_t r12_usr;
> +    uint32_t lr_usr;
> +    uint32_t sp_usr;
> +    uint32_t sp_svc;
> +    uint32_t spsr_svc;
> +    uint32_t fp;
> +    uint32_t pc;
> +    uint32_t cpsr;
> +    uint32_t ttbr0;
> +    uint32_t ttbr1;
> +};
> +
> +struct vm_event_regs_arm64 {
> +    uint64_t x0;
> +    uint64_t x1;
> +    uint64_t x2;
> +    uint64_t x3;
> +    uint64_t x4;
> +    uint64_t x5;
> +    uint64_t x6;
> +    uint64_t x7;
> +    uint64_t x8;
> +    uint64_t x9;
> +    uint64_t x10;
> +    uint64_t x16;
> +    uint64_t lr;
> +    uint64_t sp_el0;
> +    uint64_t sp_el1;
> +    uint32_t spsr_el1;
> +    uint64_t fp;
> +    uint64_t pc;
> +    uint32_t cpsr;
> +    uint64_t ttbr0;
> +    uint64_t ttbr1;
> +};

By defining 2 distinct structures, it will be more difficult for the 
introspection tools to inspect registers of an Aarch64 domain running in 
AArch32 mode. They wouldn't be able to re-use code for AArch32 domain 
because the structure fields are different.

The ARM ARM (see D1.20.1 in ARM DDI 0487A.i) provides a mapping between 
AArch32 state and AArch64 state. If you use it to define the layout of a 
common structure, the support of AArch32 state for AArch64 domain will 
come free.

> +
>   /*
>    * mem_access flag definitions
>    *
> @@ -256,6 +304,8 @@ typedef struct vm_event_st {
>       union {
>           union {
>               struct vm_event_regs_x86 x86;
> +            struct vm_event_regs_arm32 arm32;
> +            struct vm_event_regs_arm64 arm64;
>           } regs;
>
>           struct vm_event_emul_read_data emul_read_data;
>

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3 4/9] arm/vm_event: get/set registers
  2016-05-16 10:14   ` Julien Grall
@ 2016-05-16 15:37     ` Tamas K Lengyel
  2016-05-16 15:58       ` Julien Grall
  0 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-16 15:37 UTC (permalink / raw)
  To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini, Razvan Cojocaru


[-- Attachment #1.1: Type: text/plain, Size: 3722 bytes --]

On May 16, 2016 04:14, "Julien Grall" <julien.grall@arm.com> wrote:
>
> Hi Tamas,
>
>
> On 04/05/16 15:51, Tamas K Lengyel wrote:
>>
>> diff --git a/xen/include/asm-arm/vm_event.h
b/xen/include/asm-arm/vm_event.h
>> index a3fc4ce..814d0da 100644
>> --- a/xen/include/asm-arm/vm_event.h
>> +++ b/xen/include/asm-arm/vm_event.h
>> @@ -48,15 +48,10 @@ void vm_event_register_write_resume(struct vcpu *v,
vm_event_response_t *rsp)
>>       /* Not supported on ARM. */
>>   }
>>
>> -static inline
>> -void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
>> -{
>> -    /* Not supported on ARM. */
>> -}
>> +void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
>>
>> -static inline void vm_event_fill_regs(vm_event_request_t *req)
>> -{
>> -    /* Not supported on ARM. */
>> -}
>> +void vm_event_fill_regs(vm_event_request_t *req,
>> +                        const struct cpu_user_regs *regs,
>> +                        struct domain *d);
>>
>>   #endif /* __ASM_ARM_VM_EVENT_H__ */
>> diff --git a/xen/include/public/vm_event.h
b/xen/include/public/vm_event.h
>> index 3acf217..fabeee8 100644
>> --- a/xen/include/public/vm_event.h
>> +++ b/xen/include/public/vm_event.h
>> @@ -129,8 +129,8 @@
>>   #define VM_EVENT_X86_XCR0   3
>>
>>   /*
>> - * Using a custom struct (not hvm_hw_cpu) so as to not fill
>> - * the vm_event ring buffer too quickly.
>> + * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM
>
>
> You may want to rework this sentence as hvm_hw_cpu does not exist on
ARM/ARM64.

IMHO the point gets across even if we don't name the ARM structs
specifically.

>
>> + * so as to not fill the vm_event ring buffer too quickly.
>>    */
>>   struct vm_event_regs_x86 {
>>       uint64_t rax;
>> @@ -168,6 +168,54 @@ struct vm_event_regs_x86 {
>>       uint32_t _pad;
>>   };
>>
>> +struct vm_event_regs_arm32 {
>> +    uint32_t r0_usr;
>> +    uint32_t r1_usr;
>> +    uint32_t r2_usr;
>> +    uint32_t r3_usr;
>> +    uint32_t r4_usr;
>> +    uint32_t r5_usr;
>> +    uint32_t r6_usr;
>> +    uint32_t r7_usr;
>> +    uint32_t r8_usr;
>> +    uint32_t r9_usr;
>> +    uint32_t r10_usr;
>> +    uint32_t r12_usr;
>> +    uint32_t lr_usr;
>> +    uint32_t sp_usr;
>> +    uint32_t sp_svc;
>> +    uint32_t spsr_svc;
>> +    uint32_t fp;
>> +    uint32_t pc;
>> +    uint32_t cpsr;
>> +    uint32_t ttbr0;
>> +    uint32_t ttbr1;
>> +};
>> +
>> +struct vm_event_regs_arm64 {
>> +    uint64_t x0;
>> +    uint64_t x1;
>> +    uint64_t x2;
>> +    uint64_t x3;
>> +    uint64_t x4;
>> +    uint64_t x5;
>> +    uint64_t x6;
>> +    uint64_t x7;
>> +    uint64_t x8;
>> +    uint64_t x9;
>> +    uint64_t x10;
>> +    uint64_t x16;
>> +    uint64_t lr;
>> +    uint64_t sp_el0;
>> +    uint64_t sp_el1;
>> +    uint32_t spsr_el1;
>> +    uint64_t fp;
>> +    uint64_t pc;
>> +    uint32_t cpsr;
>> +    uint64_t ttbr0;
>> +    uint64_t ttbr1;
>> +};
>
>
> By defining 2 distinct structures, it will be more difficult for the
introspection tools to inspect registers of an Aarch64 domain running in
AArch32 mode. They wouldn't be able to re-use code for AArch32 domain
because the structure fields are different.
>
> The ARM ARM (see D1.20.1 in ARM DDI 0487A.i) provides a mapping between
AArch32 state and AArch64 state. If you use it to define the layout of a
common structure, the support of AArch32 state for AArch64 domain will come
free.
>

If the guest is running in 32-bit mode Xen will fill the 32-bit struct, so
doing a common struct is not strictly necessary. It also requires a bunch
if union declarations to match the names between that I would prefer to
avoid. IMHO it's cleaner to do the struct definitions separately and then
do a union on top.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 5131 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH v3 4/9] arm/vm_event: get/set registers
  2016-05-16 15:37     ` Tamas K Lengyel
@ 2016-05-16 15:58       ` Julien Grall
  2016-05-16 16:26         ` Tamas K Lengyel
  0 siblings, 1 reply; 31+ messages in thread
From: Julien Grall @ 2016-05-16 15:58 UTC (permalink / raw)
  To: Tamas K Lengyel; +Cc: Xen-devel, Stefano Stabellini, Razvan Cojocaru

Hi Tamas,

On 16/05/16 16:37, Tamas K Lengyel wrote:
>
> On May 16, 2016 04:14, "Julien Grall" <julien.grall@arm.com
> <mailto:julien.grall@arm.com>> wrote:
>  > On 04/05/16 15:51, Tamas K Lengyel wrote:
>  >>
>  >> diff --git a/xen/include/asm-arm/vm_event.h
> b/xen/include/asm-arm/vm_event.h
>  >> index a3fc4ce..814d0da 100644
>  >> --- a/xen/include/asm-arm/vm_event.h
>  >> +++ b/xen/include/asm-arm/vm_event.h
>  >> @@ -48,15 +48,10 @@ void vm_event_register_write_resume(struct vcpu
> *v, vm_event_response_t *rsp)
>  >>       /* Not supported on ARM. */
>  >>   }
>  >>
>  >> -static inline
>  >> -void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
>  >> -{
>  >> -    /* Not supported on ARM. */
>  >> -}
>  >> +void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
>  >>
>  >> -static inline void vm_event_fill_regs(vm_event_request_t *req)
>  >> -{
>  >> -    /* Not supported on ARM. */
>  >> -}
>  >> +void vm_event_fill_regs(vm_event_request_t *req,
>  >> +                        const struct cpu_user_regs *regs,
>  >> +                        struct domain *d);
>  >>
>  >>   #endif /* __ASM_ARM_VM_EVENT_H__ */
>  >> diff --git a/xen/include/public/vm_event.h
> b/xen/include/public/vm_event.h
>  >> index 3acf217..fabeee8 100644
>  >> --- a/xen/include/public/vm_event.h
>  >> +++ b/xen/include/public/vm_event.h
>  >> @@ -129,8 +129,8 @@
>  >>   #define VM_EVENT_X86_XCR0   3
>  >>
>  >>   /*
>  >> - * Using a custom struct (not hvm_hw_cpu) so as to not fill
>  >> - * the vm_event ring buffer too quickly.
>  >> + * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM
>  >
>  >
>  > You may want to rework this sentence as hvm_hw_cpu does not exist on
> ARM/ARM64.
>
> IMHO the point gets across even if we don't name the ARM structs
> specifically.

It was more for more correctness from an ARM point of view. But fair enough.

>
>  >
>  >> + * so as to not fill the vm_event ring buffer too quickly.
>  >>    */
>  >>   struct vm_event_regs_x86 {
>  >>       uint64_t rax;
>  >> @@ -168,6 +168,54 @@ struct vm_event_regs_x86 {
>  >>       uint32_t _pad;
>  >>   };
>  >>
>  >> +struct vm_event_regs_arm32 {
>  >> +    uint32_t r0_usr;
>  >> +    uint32_t r1_usr;
>  >> +    uint32_t r2_usr;
>  >> +    uint32_t r3_usr;
>  >> +    uint32_t r4_usr;
>  >> +    uint32_t r5_usr;
>  >> +    uint32_t r6_usr;
>  >> +    uint32_t r7_usr;
>  >> +    uint32_t r8_usr;
>  >> +    uint32_t r9_usr;
>  >> +    uint32_t r10_usr;
>  >> +    uint32_t r12_usr;
>  >> +    uint32_t lr_usr;
>  >> +    uint32_t sp_usr;
>  >> +    uint32_t sp_svc;
>  >> +    uint32_t spsr_svc;
>  >> +    uint32_t fp;
>  >> +    uint32_t pc;
>  >> +    uint32_t cpsr;
>  >> +    uint32_t ttbr0;
>  >> +    uint32_t ttbr1;
>  >> +};
>  >> +
>  >> +struct vm_event_regs_arm64 {
>  >> +    uint64_t x0;
>  >> +    uint64_t x1;
>  >> +    uint64_t x2;
>  >> +    uint64_t x3;
>  >> +    uint64_t x4;
>  >> +    uint64_t x5;
>  >> +    uint64_t x6;
>  >> +    uint64_t x7;
>  >> +    uint64_t x8;
>  >> +    uint64_t x9;
>  >> +    uint64_t x10;
>  >> +    uint64_t x16;
>  >> +    uint64_t lr;
>  >> +    uint64_t sp_el0;
>  >> +    uint64_t sp_el1;
>  >> +    uint32_t spsr_el1;
>  >> +    uint64_t fp;
>  >> +    uint64_t pc;
>  >> +    uint32_t cpsr;
>  >> +    uint64_t ttbr0;
>  >> +    uint64_t ttbr1;
>  >> +};
>  >
>  >
>  > By defining 2 distinct structures, it will be more difficult for the
> introspection tools to inspect registers of an Aarch64 domain running in
> AArch32 mode. They wouldn't be able to re-use code for AArch32 domain
> because the structure fields are different.
>  >
>  > The ARM ARM (see D1.20.1 in ARM DDI 0487A.i) provides a mapping
> between AArch32 state and AArch64 state. If you use it to define the
> layout of a common structure, the support of AArch32 state for AArch64
> domain will come free.
>  >
>
> If the guest is running in 32-bit mode Xen will fill the 32-bit struct,
> so doing a common struct is not strictly necessary. It also requires a
> bunch if union declarations to match the names between that I would
> prefer to avoid. IMHO it's cleaner to do the struct definitions
> separately and then do a union on top.

That is not true. is_domain_32bit will check if the domain is configured 
to run 32-bit or 64-bit in EL1 (i.e the kernel level).

So if you have a guest with 64-bit kernel and 32-bit userspace, Xen will 
always fill the 64-bit structure, even when the userspace is running.

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3 4/9] arm/vm_event: get/set registers
  2016-05-16 15:58       ` Julien Grall
@ 2016-05-16 16:26         ` Tamas K Lengyel
  2016-05-16 17:18           ` Julien Grall
  0 siblings, 1 reply; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-16 16:26 UTC (permalink / raw)
  To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini, Razvan Cojocaru


[-- Attachment #1.1: Type: text/plain, Size: 5023 bytes --]

On May 16, 2016 09:59, "Julien Grall" <julien.grall@arm.com> wrote:
>
> Hi Tamas,
>
>
> On 16/05/16 16:37, Tamas K Lengyel wrote:
>>
>>
>> On May 16, 2016 04:14, "Julien Grall" <julien.grall@arm.com
>> <mailto:julien.grall@arm.com>> wrote:
>>  > On 04/05/16 15:51, Tamas K Lengyel wrote:
>>  >>
>>  >> diff --git a/xen/include/asm-arm/vm_event.h
>> b/xen/include/asm-arm/vm_event.h
>>  >> index a3fc4ce..814d0da 100644
>>  >> --- a/xen/include/asm-arm/vm_event.h
>>  >> +++ b/xen/include/asm-arm/vm_event.h
>>  >> @@ -48,15 +48,10 @@ void vm_event_register_write_resume(struct vcpu
>> *v, vm_event_response_t *rsp)
>>  >>       /* Not supported on ARM. */
>>  >>   }
>>  >>
>>  >> -static inline
>>  >> -void vm_event_set_registers(struct vcpu *v, vm_event_response_t
*rsp)
>>  >> -{
>>  >> -    /* Not supported on ARM. */
>>  >> -}
>>  >> +void vm_event_set_registers(struct vcpu *v, vm_event_response_t
*rsp);
>>  >>
>>  >> -static inline void vm_event_fill_regs(vm_event_request_t *req)
>>  >> -{
>>  >> -    /* Not supported on ARM. */
>>  >> -}
>>  >> +void vm_event_fill_regs(vm_event_request_t *req,
>>  >> +                        const struct cpu_user_regs *regs,
>>  >> +                        struct domain *d);
>>  >>
>>  >>   #endif /* __ASM_ARM_VM_EVENT_H__ */
>>  >> diff --git a/xen/include/public/vm_event.h
>> b/xen/include/public/vm_event.h
>>  >> index 3acf217..fabeee8 100644
>>  >> --- a/xen/include/public/vm_event.h
>>  >> +++ b/xen/include/public/vm_event.h
>>  >> @@ -129,8 +129,8 @@
>>  >>   #define VM_EVENT_X86_XCR0   3
>>  >>
>>  >>   /*
>>  >> - * Using a custom struct (not hvm_hw_cpu) so as to not fill
>>  >> - * the vm_event ring buffer too quickly.
>>  >> + * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and
ARM
>>  >
>>  >
>>  > You may want to rework this sentence as hvm_hw_cpu does not exist on
>> ARM/ARM64.
>>
>> IMHO the point gets across even if we don't name the ARM structs
>> specifically.
>
>
> It was more for more correctness from an ARM point of view. But fair
enough.
>
>
>>
>>  >
>>  >> + * so as to not fill the vm_event ring buffer too quickly.
>>  >>    */
>>  >>   struct vm_event_regs_x86 {
>>  >>       uint64_t rax;
>>  >> @@ -168,6 +168,54 @@ struct vm_event_regs_x86 {
>>  >>       uint32_t _pad;
>>  >>   };
>>  >>
>>  >> +struct vm_event_regs_arm32 {
>>  >> +    uint32_t r0_usr;
>>  >> +    uint32_t r1_usr;
>>  >> +    uint32_t r2_usr;
>>  >> +    uint32_t r3_usr;
>>  >> +    uint32_t r4_usr;
>>  >> +    uint32_t r5_usr;
>>  >> +    uint32_t r6_usr;
>>  >> +    uint32_t r7_usr;
>>  >> +    uint32_t r8_usr;
>>  >> +    uint32_t r9_usr;
>>  >> +    uint32_t r10_usr;
>>  >> +    uint32_t r12_usr;
>>  >> +    uint32_t lr_usr;
>>  >> +    uint32_t sp_usr;
>>  >> +    uint32_t sp_svc;
>>  >> +    uint32_t spsr_svc;
>>  >> +    uint32_t fp;
>>  >> +    uint32_t pc;
>>  >> +    uint32_t cpsr;
>>  >> +    uint32_t ttbr0;
>>  >> +    uint32_t ttbr1;
>>  >> +};
>>  >> +
>>  >> +struct vm_event_regs_arm64 {
>>  >> +    uint64_t x0;
>>  >> +    uint64_t x1;
>>  >> +    uint64_t x2;
>>  >> +    uint64_t x3;
>>  >> +    uint64_t x4;
>>  >> +    uint64_t x5;
>>  >> +    uint64_t x6;
>>  >> +    uint64_t x7;
>>  >> +    uint64_t x8;
>>  >> +    uint64_t x9;
>>  >> +    uint64_t x10;
>>  >> +    uint64_t x16;
>>  >> +    uint64_t lr;
>>  >> +    uint64_t sp_el0;
>>  >> +    uint64_t sp_el1;
>>  >> +    uint32_t spsr_el1;
>>  >> +    uint64_t fp;
>>  >> +    uint64_t pc;
>>  >> +    uint32_t cpsr;
>>  >> +    uint64_t ttbr0;
>>  >> +    uint64_t ttbr1;
>>  >> +};
>>  >
>>  >
>>  > By defining 2 distinct structures, it will be more difficult for the
>> introspection tools to inspect registers of an Aarch64 domain running in
>> AArch32 mode. They wouldn't be able to re-use code for AArch32 domain
>> because the structure fields are different.
>>  >
>>  > The ARM ARM (see D1.20.1 in ARM DDI 0487A.i) provides a mapping
>> between AArch32 state and AArch64 state. If you use it to define the
>> layout of a common structure, the support of AArch32 state for AArch64
>> domain will come free.
>>  >
>>
>> If the guest is running in 32-bit mode Xen will fill the 32-bit struct,
>> so doing a common struct is not strictly necessary. It also requires a
>> bunch if union declarations to match the names between that I would
>> prefer to avoid. IMHO it's cleaner to do the struct definitions
>> separately and then do a union on top.
>
>
> That is not true. is_domain_32bit will check if the domain is configured
to run 32-bit or 64-bit in EL1 (i.e the kernel level).
>
> So if you have a guest with 64-bit kernel and 32-bit userspace, Xen will
always fill the 64-bit structure, even when the userspace is running.
>

Hm fair point. It complicates things a bit though either way as the event
subscriber wouldn't know which mode it received the event from without
doing some extra checks. So I guess Xen should transmit that information
too, at which point it could also just pick the right struct to fill with
the current setup.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 7602 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

* Re: [PATCH v3 4/9] arm/vm_event: get/set registers
  2016-05-16 16:26         ` Tamas K Lengyel
@ 2016-05-16 17:18           ` Julien Grall
  0 siblings, 0 replies; 31+ messages in thread
From: Julien Grall @ 2016-05-16 17:18 UTC (permalink / raw)
  To: Tamas K Lengyel; +Cc: Xen-devel, Stefano Stabellini, Razvan Cojocaru



On 16/05/16 17:26, Tamas K Lengyel wrote:
>
> On May 16, 2016 09:59, "Julien Grall" <julien.grall@arm.com
> <mailto:julien.grall@arm.com>> wrote:
>  >>  > By defining 2 distinct structures, it will be more difficult for the
>  >> introspection tools to inspect registers of an Aarch64 domain running in
>  >> AArch32 mode. They wouldn't be able to re-use code for AArch32 domain
>  >> because the structure fields are different.
>  >>  >
>  >>  > The ARM ARM (see D1.20.1 in ARM DDI 0487A.i) provides a mapping
>  >> between AArch32 state and AArch64 state. If you use it to define the
>  >> layout of a common structure, the support of AArch32 state for AArch64
>  >> domain will come free.
>  >>  >
>  >>
>  >> If the guest is running in 32-bit mode Xen will fill the 32-bit struct,
>  >> so doing a common struct is not strictly necessary. It also requires a
>  >> bunch if union declarations to match the names between that I would
>  >> prefer to avoid. IMHO it's cleaner to do the struct definitions
>  >> separately and then do a union on top.
>  >
>  >
>  > That is not true. is_domain_32bit will check if the domain is
> configured to run 32-bit or 64-bit in EL1 (i.e the kernel level).
>  >
>  > So if you have a guest with 64-bit kernel and 32-bit userspace, Xen
> will always fill the 64-bit structure, even when the userspace is running.
>  >
>
> Hm fair point. It complicates things a bit though either way as the
> event subscriber wouldn't know which mode it received the event from
> without doing some extra checks. So I guess Xen should transmit that
> information too, at which point it could also just pick the right struct
> to fill with the current setup.

Not really, CPSR.M[4] tells you if the execution state were AArch32 or 
AArch64. vm_event_regs_* contains the cpsr fields, so the event
subscriber can determine the mode.

Actually, not all the registers in vm_event_regs_arm32 makes sense when 
the guest is a 64-bit domain. For instance, spsr_svc is for the EL1 (i.e 
kernel).

Hence the suggestion to use a structure very similar to 
vcpu_guest_core_regs.

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common
  2016-05-16  9:48   ` Julien Grall
@ 2016-05-27 18:58     ` Tamas K Lengyel
  0 siblings, 0 replies; 31+ messages in thread
From: Tamas K Lengyel @ 2016-05-27 18:58 UTC (permalink / raw)
  To: Julien Grall
  Cc: Xen-devel, Stefano Stabellini, Razvan Cojocaru, Jan Beulich,
	Andrew Cooper

On Mon, May 16, 2016 at 3:48 AM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Tamas,
>
> On 04/05/16 15:51, Tamas K Lengyel wrote:
>>
>> The prototype of vm_event_fill_regs will differ on x86 and ARM so in this
>> patch
>> we move components from common to arch-specific that use this function. As
>> part of this patch we rename and relocate vm_event_monitor_guest_request
>> as
>> monitor_guest_request from vm_event to monitor.
>
>
> Would not it be possible to find a common prototype between ARM and x86?
>
> From patch #4, the ARM prototype is:
>
> void vm_event_fill_regs(vm_event_request_t *req,
>                         const struct cpu_user_regs *regs,
>                         struct domain *d)
>
> and the x86 one is
>
> void vm_event_fill_regs(vm_event_request_t *req);
>
> The parameter "regs" will always be equal to guest_cpu_user_regs(). And the
> domain will always be current->domain.
>
> So, IHMO, there is no need to differ between ARM and x86. This would also
> keep the code simple.
>

Yeap, you are right, I'll get rid of this patch.

Thanks,
Tamas

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

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

end of thread, other threads:[~2016-05-27 18:58 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-04 14:51 [PATCH v3 1/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
2016-05-04 14:51 ` [PATCH v3 2/9] monitor: Don't call vm_event_fill_regs from common Tamas K Lengyel
2016-05-05  9:34   ` Razvan Cojocaru
2016-05-16  9:48   ` Julien Grall
2016-05-27 18:58     ` Tamas K Lengyel
2016-05-04 14:51 ` [PATCH v3 3/9] monitor: ARM SMC events Tamas K Lengyel
2016-05-05  9:36   ` Razvan Cojocaru
2016-05-16  9:56   ` Julien Grall
2016-05-04 14:51 ` [PATCH v3 4/9] arm/vm_event: get/set registers Tamas K Lengyel
2016-05-16 10:14   ` Julien Grall
2016-05-16 15:37     ` Tamas K Lengyel
2016-05-16 15:58       ` Julien Grall
2016-05-16 16:26         ` Tamas K Lengyel
2016-05-16 17:18           ` Julien Grall
2016-05-04 14:51 ` [PATCH v3 5/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
2016-05-04 20:08   ` Konrad Rzeszutek Wilk
2016-05-04 22:12     ` Tamas K Lengyel
2016-05-04 14:51 ` [PATCH v3 6/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
2016-05-04 15:35   ` Jan Beulich
2016-05-04 17:16     ` Tamas K Lengyel
2016-05-04 17:33       ` Wei Liu
2016-05-04 17:42         ` Tamas K Lengyel
2016-05-05 16:25       ` Jan Beulich
     [not found]         ` <CABfawh=gWOs3AtsTdYaDj61ph2jumjX6Q=0uFVeahPH99DY9qg@mail.gmail.com>
     [not found]           ` <CABfawhknB62vZJFvcJv6VAGzw0toZUCXBHyEnzm99+N1ZLBYEg@mail.gmail.com>
2016-05-05 18:25             ` Tamas K Lengyel
2016-05-05  9:37   ` Razvan Cojocaru
2016-05-04 14:51 ` [PATCH v3 7/9] x86/hvm: Rename hvm/event to hvm/monitor Tamas K Lengyel
2016-05-05  9:39   ` Razvan Cojocaru
2016-05-04 14:51 ` [PATCH v3 8/9] x86/hvm: Add debug exception vm_events Tamas K Lengyel
2016-05-05  9:56   ` Razvan Cojocaru
2016-05-04 14:51 ` [PATCH v3 9/9] MAINTAINERS: Update monitor/vm_event covered code Tamas K Lengyel
2016-05-05  9:53   ` Razvan Cojocaru

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).