[PATCH v5 00/13] x86: refactor trap handling code

[PATCH v5 00/13] x86: refactor trap handling code

[Wei Liu]

This series can also be found on my xenbits/xen.git wip.move-traps-v5

Wei Liu (13):
  x86: move callback_op code to pv/callback.c
  x86: move the compat callback ops next to the non-compat variant
  x86: move do_set_trap_table to pv/callback.c
  x86: move compat_set_trap_table along side the non-compat variant
  x86: remove the now empty x86_64/compat/traps.c
  x86: simplify guest_has_trap_callback
  x86/traps: simplify and rename send_guest_trap
  x86/traps: factor out pv_trap_init
  xen: move do_nmi_op and make it x86 only
  x86/traps: move {un,}register_guest_nmi_callback to pv/callback.c
  x86/callback.c: slightly change {un,}register_guest_nmi_callback
  x86/traps: move some PV specific functions to pv/traps.c
  x86/traps.h: remove unused declaration of cpu_user_regs

 xen/arch/x86/cpu/mcheck/vmce.c     |  11 +-
 xen/arch/x86/irq.c                 |   9 +-
 xen/arch/x86/nmi.c                 |   2 +-
 xen/arch/x86/oprofile/nmi_int.c    |   2 +-
 xen/arch/x86/pv/Makefile           |   1 +
 xen/arch/x86/pv/callback.c         | 510 +++++++++++++++++++++++++++++++++++++
 xen/arch/x86/pv/traps.c            | 104 ++++++++
 xen/arch/x86/traps.c               | 198 +-------------
 xen/arch/x86/x86_64/compat/traps.c | 202 ---------------
 xen/arch/x86/x86_64/entry.S        |   4 +
 xen/arch/x86/x86_64/traps.c        | 152 +----------
 xen/common/compat/kernel.c         |   5 -
 xen/common/kernel.c                |  26 --
 xen/include/asm-arm/nmi.h          |  15 --
 xen/include/asm-x86/nmi.h          |  15 --
 xen/include/asm-x86/pv/traps.h     |  12 +
 xen/include/asm-x86/traps.h        |  25 --
 xen/include/xen/nmi.h              |  14 -
 18 files changed, 654 insertions(+), 653 deletions(-)
 create mode 100644 xen/arch/x86/pv/callback.c
 delete mode 100644 xen/arch/x86/x86_64/compat/traps.c
 delete mode 100644 xen/include/asm-arm/nmi.h
 delete mode 100644 xen/include/xen/nmi.h

-- 
2.11.0


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

[PATCH v5 01/13] x86: move callback_op code to pv/callback.c

[Wei Liu]

Take the chance to change v to curr.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/Makefile    |   1 +
 xen/arch/x86/pv/callback.c  | 183 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/x86_64/traps.c | 148 -----------------------------------
 3 files changed, 184 insertions(+), 148 deletions(-)
 create mode 100644 xen/arch/x86/pv/callback.c

diff --git a/xen/arch/x86/pv/Makefile b/xen/arch/x86/pv/Makefile
index 7e3da332d8..bd1a7081fc 100644
--- a/xen/arch/x86/pv/Makefile
+++ b/xen/arch/x86/pv/Makefile
@@ -1,6 +1,7 @@
 obj-y += hypercall.o
 obj-y += traps.o
 
+obj-y += callback.o
 obj-bin-y += dom0_build.init.o
 obj-y += domain.o
 obj-y += emulate.o
diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
new file mode 100644
index 0000000000..cbcdeac2da
--- /dev/null
+++ b/xen/arch/x86/pv/callback.c
@@ -0,0 +1,183 @@
+/*
+ * pv/callback.c
+ *
+ * hypercall handles and helper functions for guest callback
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, 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/guest_access.h>
+#include <xen/lib.h>
+#include <xen/sched.h>
+
+#include <asm/current.h>
+#include <asm/nmi.h>
+#include <asm/traps.h>
+
+#include <public/callback.h>
+
+static long register_guest_callback(struct callback_register *reg)
+{
+    long ret = 0;
+    struct vcpu *curr = current;
+
+    if ( !is_canonical_address(reg->address) )
+        return -EINVAL;
+
+    switch ( reg->type )
+    {
+    case CALLBACKTYPE_event:
+        curr->arch.pv_vcpu.event_callback_eip    = reg->address;
+        break;
+
+    case CALLBACKTYPE_failsafe:
+        curr->arch.pv_vcpu.failsafe_callback_eip = reg->address;
+        if ( reg->flags & CALLBACKF_mask_events )
+            set_bit(_VGCF_failsafe_disables_events,
+                    &curr->arch.vgc_flags);
+        else
+            clear_bit(_VGCF_failsafe_disables_events,
+                      &curr->arch.vgc_flags);
+        break;
+
+    case CALLBACKTYPE_syscall:
+        curr->arch.pv_vcpu.syscall_callback_eip  = reg->address;
+        if ( reg->flags & CALLBACKF_mask_events )
+            set_bit(_VGCF_syscall_disables_events,
+                    &curr->arch.vgc_flags);
+        else
+            clear_bit(_VGCF_syscall_disables_events,
+                      &curr->arch.vgc_flags);
+        break;
+
+    case CALLBACKTYPE_syscall32:
+        curr->arch.pv_vcpu.syscall32_callback_eip = reg->address;
+        curr->arch.pv_vcpu.syscall32_disables_events =
+            !!(reg->flags & CALLBACKF_mask_events);
+        break;
+
+    case CALLBACKTYPE_sysenter:
+        curr->arch.pv_vcpu.sysenter_callback_eip = reg->address;
+        curr->arch.pv_vcpu.sysenter_disables_events =
+            !!(reg->flags & CALLBACKF_mask_events);
+        break;
+
+    case CALLBACKTYPE_nmi:
+        ret = register_guest_nmi_callback(reg->address);
+        break;
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
+static long unregister_guest_callback(struct callback_unregister *unreg)
+{
+    long ret;
+
+    switch ( unreg->type )
+    {
+    case CALLBACKTYPE_event:
+    case CALLBACKTYPE_failsafe:
+    case CALLBACKTYPE_syscall:
+    case CALLBACKTYPE_syscall32:
+    case CALLBACKTYPE_sysenter:
+        ret = -EINVAL;
+        break;
+
+    case CALLBACKTYPE_nmi:
+        ret = unregister_guest_nmi_callback();
+        break;
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
+long do_callback_op(int cmd, XEN_GUEST_HANDLE_PARAM(const_void) arg)
+{
+    long ret;
+
+    switch ( cmd )
+    {
+    case CALLBACKOP_register:
+    {
+        struct callback_register reg;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&reg, arg, 1) )
+            break;
+
+        ret = register_guest_callback(&reg);
+    }
+    break;
+
+    case CALLBACKOP_unregister:
+    {
+        struct callback_unregister unreg;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&unreg, arg, 1) )
+            break;
+
+        ret = unregister_guest_callback(&unreg);
+    }
+    break;
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
+long do_set_callbacks(unsigned long event_address,
+                      unsigned long failsafe_address,
+                      unsigned long syscall_address)
+{
+    struct callback_register event = {
+        .type = CALLBACKTYPE_event,
+        .address = event_address,
+    };
+    struct callback_register failsafe = {
+        .type = CALLBACKTYPE_failsafe,
+        .address = failsafe_address,
+    };
+    struct callback_register syscall = {
+        .type = CALLBACKTYPE_syscall,
+        .address = syscall_address,
+    };
+
+    register_guest_callback(&event);
+    register_guest_callback(&failsafe);
+    register_guest_callback(&syscall);
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index c87746ef1c..79bfc4d3f0 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -23,7 +23,6 @@
 #include <asm/shared.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/support.h>
-#include <public/callback.h>
 
 
 static void print_xen_info(void)
@@ -336,153 +335,6 @@ void subarch_percpu_traps_init(void)
     wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
 }
 
-static long register_guest_callback(struct callback_register *reg)
-{
-    long ret = 0;
-    struct vcpu *v = current;
-
-    if ( !is_canonical_address(reg->address) )
-        return -EINVAL;
-
-    switch ( reg->type )
-    {
-    case CALLBACKTYPE_event:
-        v->arch.pv_vcpu.event_callback_eip    = reg->address;
-        break;
-
-    case CALLBACKTYPE_failsafe:
-        v->arch.pv_vcpu.failsafe_callback_eip = reg->address;
-        if ( reg->flags & CALLBACKF_mask_events )
-            set_bit(_VGCF_failsafe_disables_events,
-                    &v->arch.vgc_flags);
-        else
-            clear_bit(_VGCF_failsafe_disables_events,
-                      &v->arch.vgc_flags);
-        break;
-
-    case CALLBACKTYPE_syscall:
-        v->arch.pv_vcpu.syscall_callback_eip  = reg->address;
-        if ( reg->flags & CALLBACKF_mask_events )
-            set_bit(_VGCF_syscall_disables_events,
-                    &v->arch.vgc_flags);
-        else
-            clear_bit(_VGCF_syscall_disables_events,
-                      &v->arch.vgc_flags);
-        break;
-
-    case CALLBACKTYPE_syscall32:
-        v->arch.pv_vcpu.syscall32_callback_eip = reg->address;
-        v->arch.pv_vcpu.syscall32_disables_events =
-            !!(reg->flags & CALLBACKF_mask_events);
-        break;
-
-    case CALLBACKTYPE_sysenter:
-        v->arch.pv_vcpu.sysenter_callback_eip = reg->address;
-        v->arch.pv_vcpu.sysenter_disables_events =
-            !!(reg->flags & CALLBACKF_mask_events);
-        break;
-
-    case CALLBACKTYPE_nmi:
-        ret = register_guest_nmi_callback(reg->address);
-        break;
-
-    default:
-        ret = -ENOSYS;
-        break;
-    }
-
-    return ret;
-}
-
-static long unregister_guest_callback(struct callback_unregister *unreg)
-{
-    long ret;
-
-    switch ( unreg->type )
-    {
-    case CALLBACKTYPE_event:
-    case CALLBACKTYPE_failsafe:
-    case CALLBACKTYPE_syscall:
-    case CALLBACKTYPE_syscall32:
-    case CALLBACKTYPE_sysenter:
-        ret = -EINVAL;
-        break;
-
-    case CALLBACKTYPE_nmi:
-        ret = unregister_guest_nmi_callback();
-        break;
-
-    default:
-        ret = -ENOSYS;
-        break;
-    }
-
-    return ret;
-}
-
-
-long do_callback_op(int cmd, XEN_GUEST_HANDLE_PARAM(const_void) arg)
-{
-    long ret;
-
-    switch ( cmd )
-    {
-    case CALLBACKOP_register:
-    {
-        struct callback_register reg;
-
-        ret = -EFAULT;
-        if ( copy_from_guest(&reg, arg, 1) )
-            break;
-
-        ret = register_guest_callback(&reg);
-    }
-    break;
-
-    case CALLBACKOP_unregister:
-    {
-        struct callback_unregister unreg;
-
-        ret = -EFAULT;
-        if ( copy_from_guest(&unreg, arg, 1) )
-            break;
-
-        ret = unregister_guest_callback(&unreg);
-    }
-    break;
-
-    default:
-        ret = -ENOSYS;
-        break;
-    }
-
-    return ret;
-}
-
-long do_set_callbacks(unsigned long event_address,
-                      unsigned long failsafe_address,
-                      unsigned long syscall_address)
-{
-    struct callback_register event = {
-        .type = CALLBACKTYPE_event,
-        .address = event_address,
-    };
-    struct callback_register failsafe = {
-        .type = CALLBACKTYPE_failsafe,
-        .address = failsafe_address,
-    };
-    struct callback_register syscall = {
-        .type = CALLBACKTYPE_syscall,
-        .address = syscall_address,
-    };
-
-    register_guest_callback(&event);
-    register_guest_callback(&failsafe);
-    register_guest_callback(&syscall);
-
-    return 0;
-}
-
 #include "compat/traps.c"
 
 void hypercall_page_initialise(struct domain *d, void *hypercall_page)
-- 
2.11.0


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

[PATCH v5 02/13] x86: move the compat callback ops next to the non-compat variant

[Wei Liu]

Take the chance to change v to curr.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/callback.c         | 142 ++++++++++++++++++++++++++++++++++++
 xen/arch/x86/x86_64/compat/traps.c | 143 -------------------------------------
 2 files changed, 142 insertions(+), 143 deletions(-)

diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
index cbcdeac2da..d811f48110 100644
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -19,6 +19,7 @@
 #include <xen/guest_access.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
+#include <compat/callback.h>
 
 #include <asm/current.h>
 #include <asm/nmi.h>
@@ -172,6 +173,147 @@ long do_set_callbacks(unsigned long event_address,
     return 0;
 }
 
+static long compat_register_guest_callback(struct compat_callback_register *reg)
+{
+    long ret = 0;
+    struct vcpu *curr = current;
+
+    fixup_guest_code_selector(curr->domain, reg->address.cs);
+
+    switch ( reg->type )
+    {
+    case CALLBACKTYPE_event:
+        curr->arch.pv_vcpu.event_callback_cs     = reg->address.cs;
+        curr->arch.pv_vcpu.event_callback_eip    = reg->address.eip;
+        break;
+
+    case CALLBACKTYPE_failsafe:
+        curr->arch.pv_vcpu.failsafe_callback_cs  = reg->address.cs;
+        curr->arch.pv_vcpu.failsafe_callback_eip = reg->address.eip;
+        if ( reg->flags & CALLBACKF_mask_events )
+            set_bit(_VGCF_failsafe_disables_events,
+                    &curr->arch.vgc_flags);
+        else
+            clear_bit(_VGCF_failsafe_disables_events,
+                      &curr->arch.vgc_flags);
+        break;
+
+    case CALLBACKTYPE_syscall32:
+        curr->arch.pv_vcpu.syscall32_callback_cs     = reg->address.cs;
+        curr->arch.pv_vcpu.syscall32_callback_eip    = reg->address.eip;
+        curr->arch.pv_vcpu.syscall32_disables_events =
+            (reg->flags & CALLBACKF_mask_events) != 0;
+        break;
+
+    case CALLBACKTYPE_sysenter:
+        curr->arch.pv_vcpu.sysenter_callback_cs     = reg->address.cs;
+        curr->arch.pv_vcpu.sysenter_callback_eip    = reg->address.eip;
+        curr->arch.pv_vcpu.sysenter_disables_events =
+            (reg->flags & CALLBACKF_mask_events) != 0;
+        break;
+
+    case CALLBACKTYPE_nmi:
+        ret = register_guest_nmi_callback(reg->address.eip);
+        break;
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
+static long compat_unregister_guest_callback(
+    struct compat_callback_unregister *unreg)
+{
+    long ret;
+
+    switch ( unreg->type )
+    {
+    case CALLBACKTYPE_event:
+    case CALLBACKTYPE_failsafe:
+    case CALLBACKTYPE_syscall32:
+    case CALLBACKTYPE_sysenter:
+        ret = -EINVAL;
+        break;
+
+    case CALLBACKTYPE_nmi:
+        ret = unregister_guest_nmi_callback();
+        break;
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
+long compat_callback_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+{
+    long ret;
+
+    switch ( cmd )
+    {
+    case CALLBACKOP_register:
+    {
+        struct compat_callback_register reg;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&reg, arg, 1) )
+            break;
+
+        ret = compat_register_guest_callback(&reg);
+    }
+    break;
+
+    case CALLBACKOP_unregister:
+    {
+        struct compat_callback_unregister unreg;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&unreg, arg, 1) )
+            break;
+
+        ret = compat_unregister_guest_callback(&unreg);
+    }
+    break;
+
+    default:
+        ret = -EINVAL;
+        break;
+    }
+
+    return ret;
+}
+
+long compat_set_callbacks(unsigned long event_selector,
+                          unsigned long event_address,
+                          unsigned long failsafe_selector,
+                          unsigned long failsafe_address)
+{
+    struct compat_callback_register event = {
+        .type = CALLBACKTYPE_event,
+        .address = {
+            .cs = event_selector,
+            .eip = event_address
+        }
+    };
+    struct compat_callback_register failsafe = {
+        .type = CALLBACKTYPE_failsafe,
+        .address = {
+            .cs = failsafe_selector,
+            .eip = failsafe_address
+        }
+    };
+
+    compat_register_guest_callback(&event);
+    compat_register_guest_callback(&failsafe);
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/x86_64/compat/traps.c b/xen/arch/x86/x86_64/compat/traps.c
index 4594a32100..9e1ccba646 100644
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ b/xen/arch/x86/x86_64/compat/traps.c
@@ -3,149 +3,6 @@
 #include <compat/callback.h>
 #include <compat/arch-x86_32.h>
 
-static long compat_register_guest_callback(
-    struct compat_callback_register *reg)
-{
-    long ret = 0;
-    struct vcpu *v = current;
-
-    fixup_guest_code_selector(v->domain, reg->address.cs);
-
-    switch ( reg->type )
-    {
-    case CALLBACKTYPE_event:
-        v->arch.pv_vcpu.event_callback_cs     = reg->address.cs;
-        v->arch.pv_vcpu.event_callback_eip    = reg->address.eip;
-        break;
-
-    case CALLBACKTYPE_failsafe:
-        v->arch.pv_vcpu.failsafe_callback_cs  = reg->address.cs;
-        v->arch.pv_vcpu.failsafe_callback_eip = reg->address.eip;
-        if ( reg->flags & CALLBACKF_mask_events )
-            set_bit(_VGCF_failsafe_disables_events,
-                    &v->arch.vgc_flags);
-        else
-            clear_bit(_VGCF_failsafe_disables_events,
-                      &v->arch.vgc_flags);
-        break;
-
-    case CALLBACKTYPE_syscall32:
-        v->arch.pv_vcpu.syscall32_callback_cs     = reg->address.cs;
-        v->arch.pv_vcpu.syscall32_callback_eip    = reg->address.eip;
-        v->arch.pv_vcpu.syscall32_disables_events =
-            (reg->flags & CALLBACKF_mask_events) != 0;
-        break;
-
-    case CALLBACKTYPE_sysenter:
-        v->arch.pv_vcpu.sysenter_callback_cs     = reg->address.cs;
-        v->arch.pv_vcpu.sysenter_callback_eip    = reg->address.eip;
-        v->arch.pv_vcpu.sysenter_disables_events =
-            (reg->flags & CALLBACKF_mask_events) != 0;
-        break;
-
-    case CALLBACKTYPE_nmi:
-        ret = register_guest_nmi_callback(reg->address.eip);
-        break;
-
-    default:
-        ret = -ENOSYS;
-        break;
-    }
-
-    return ret;
-}
-
-static long compat_unregister_guest_callback(
-    struct compat_callback_unregister *unreg)
-{
-    long ret;
-
-    switch ( unreg->type )
-    {
-    case CALLBACKTYPE_event:
-    case CALLBACKTYPE_failsafe:
-    case CALLBACKTYPE_syscall32:
-    case CALLBACKTYPE_sysenter:
-        ret = -EINVAL;
-        break;
-
-    case CALLBACKTYPE_nmi:
-        ret = unregister_guest_nmi_callback();
-        break;
-
-    default:
-        ret = -ENOSYS;
-        break;
-    }
-
-    return ret;
-}
-
-
-long compat_callback_op(int cmd, XEN_GUEST_HANDLE(void) arg)
-{
-    long ret;
-
-    switch ( cmd )
-    {
-    case CALLBACKOP_register:
-    {
-        struct compat_callback_register reg;
-
-        ret = -EFAULT;
-        if ( copy_from_guest(&reg, arg, 1) )
-            break;
-
-        ret = compat_register_guest_callback(&reg);
-    }
-    break;
-
-    case CALLBACKOP_unregister:
-    {
-        struct compat_callback_unregister unreg;
-
-        ret = -EFAULT;
-        if ( copy_from_guest(&unreg, arg, 1) )
-            break;
-
-        ret = compat_unregister_guest_callback(&unreg);
-    }
-    break;
-
-    default:
-        ret = -EINVAL;
-        break;
-    }
-
-    return ret;
-}
-
-long compat_set_callbacks(unsigned long event_selector,
-                          unsigned long event_address,
-                          unsigned long failsafe_selector,
-                          unsigned long failsafe_address)
-{
-    struct compat_callback_register event = {
-        .type = CALLBACKTYPE_event,
-        .address = {
-            .cs = event_selector,
-            .eip = event_address
-        }
-    };
-    struct compat_callback_register failsafe = {
-        .type = CALLBACKTYPE_failsafe,
-        .address = {
-            .cs = failsafe_selector,
-            .eip = failsafe_address
-        }
-    };
-
-    compat_register_guest_callback(&event);
-    compat_register_guest_callback(&failsafe);
-
-    return 0;
-}
-
 int compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
 {
     struct compat_trap_info cur;
-- 
2.11.0


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

[PATCH v5 03/13] x86: move do_set_trap_table to pv/callback.c

[Wei Liu]

That hypercall is used to set guest callbacks for traps.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/callback.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/traps.c       | 50 ----------------------------------------------
 2 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
index d811f48110..0d7641df43 100644
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -16,6 +16,7 @@
  * License along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <xen/event.h>
 #include <xen/guest_access.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
@@ -314,6 +315,55 @@ long compat_set_callbacks(unsigned long event_selector,
     return 0;
 }
 
+long do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
+{
+    struct trap_info cur;
+    struct vcpu *curr = current;
+    struct trap_info *dst = curr->arch.pv_vcpu.trap_ctxt;
+    long rc = 0;
+
+    /* If no table is presented then clear the entire virtual IDT. */
+    if ( guest_handle_is_null(traps) )
+    {
+        memset(dst, 0, NR_VECTORS * sizeof(*dst));
+        init_int80_direct_trap(curr);
+        return 0;
+    }
+
+    for ( ; ; )
+    {
+        if ( copy_from_guest(&cur, traps, 1) )
+        {
+            rc = -EFAULT;
+            break;
+        }
+
+        if ( cur.address == 0 )
+            break;
+
+        if ( !is_canonical_address(cur.address) )
+            return -EINVAL;
+
+        fixup_guest_code_selector(curr->domain, cur.cs);
+
+        memcpy(&dst[cur.vector], &cur, sizeof(cur));
+
+        if ( cur.vector == 0x80 )
+            init_int80_direct_trap(curr);
+
+        guest_handle_add_offset(traps, 1);
+
+        if ( hypercall_preempt_check() )
+        {
+            rc = hypercall_create_continuation(
+                __HYPERVISOR_set_trap_table, "h", traps);
+            break;
+        }
+    }
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 8caa9255ed..c43f3f1095 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2073,56 +2073,6 @@ int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
     return -EIO;
 }
 
-
-long do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
-{
-    struct trap_info cur;
-    struct vcpu *curr = current;
-    struct trap_info *dst = curr->arch.pv_vcpu.trap_ctxt;
-    long rc = 0;
-
-    /* If no table is presented then clear the entire virtual IDT. */
-    if ( guest_handle_is_null(traps) )
-    {
-        memset(dst, 0, NR_VECTORS * sizeof(*dst));
-        init_int80_direct_trap(curr);
-        return 0;
-    }
-
-    for ( ; ; )
-    {
-        if ( copy_from_guest(&cur, traps, 1) )
-        {
-            rc = -EFAULT;
-            break;
-        }
-
-        if ( cur.address == 0 )
-            break;
-
-        if ( !is_canonical_address(cur.address) )
-            return -EINVAL;
-
-        fixup_guest_code_selector(curr->domain, cur.cs);
-
-        memcpy(&dst[cur.vector], &cur, sizeof(cur));
-
-        if ( cur.vector == 0x80 )
-            init_int80_direct_trap(curr);
-
-        guest_handle_add_offset(traps, 1);
-
-        if ( hypercall_preempt_check() )
-        {
-            rc = hypercall_create_continuation(
-                __HYPERVISOR_set_trap_table, "h", traps);
-            break;
-        }
-    }
-
-    return rc;
-}
-
 void activate_debugregs(const struct vcpu *curr)
 {
     ASSERT(curr == current);
-- 
2.11.0


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

[PATCH v5 04/13] x86: move compat_set_trap_table along side the non-compat variant

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/callback.c         | 47 ++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/x86_64/compat/traps.c | 44 -----------------------------------
 2 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
index 0d7641df43..b9dd39bf0e 100644
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -17,6 +17,7 @@
  */
 
 #include <xen/event.h>
+#include <xen/hypercall.h>
 #include <xen/guest_access.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
@@ -364,6 +365,52 @@ long do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
     return rc;
 }
 
+int compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
+{
+    struct vcpu *curr = current;
+    struct compat_trap_info cur;
+    struct trap_info *dst = curr->arch.pv_vcpu.trap_ctxt;
+    long rc = 0;
+
+    /* If no table is presented then clear the entire virtual IDT. */
+    if ( guest_handle_is_null(traps) )
+    {
+        memset(dst, 0, NR_VECTORS * sizeof(*dst));
+        init_int80_direct_trap(curr);
+        return 0;
+    }
+
+    for ( ; ; )
+    {
+        if ( copy_from_guest(&cur, traps, 1) )
+        {
+            rc = -EFAULT;
+            break;
+        }
+
+        if ( cur.address == 0 )
+            break;
+
+        fixup_guest_code_selector(curr->domain, cur.cs);
+
+        XLAT_trap_info(dst + cur.vector, &cur);
+
+        if ( cur.vector == 0x80 )
+            init_int80_direct_trap(curr);
+
+        guest_handle_add_offset(traps, 1);
+
+        if ( hypercall_preempt_check() )
+        {
+            rc = hypercall_create_continuation(
+                __HYPERVISOR_set_trap_table, "h", traps);
+            break;
+        }
+    }
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/x86_64/compat/traps.c b/xen/arch/x86/x86_64/compat/traps.c
index 9e1ccba646..cade7f2923 100644
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ b/xen/arch/x86/x86_64/compat/traps.c
@@ -3,50 +3,6 @@
 #include <compat/callback.h>
 #include <compat/arch-x86_32.h>
 
-int compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
-{
-    struct compat_trap_info cur;
-    struct trap_info *dst = current->arch.pv_vcpu.trap_ctxt;
-    long rc = 0;
-
-    /* If no table is presented then clear the entire virtual IDT. */
-    if ( guest_handle_is_null(traps) )
-    {
-        memset(dst, 0, NR_VECTORS * sizeof(*dst));
-        init_int80_direct_trap(current);
-        return 0;
-    }
-
-    for ( ; ; )
-    {
-        if ( copy_from_guest(&cur, traps, 1) )
-        {
-            rc = -EFAULT;
-            break;
-        }
-
-        if ( cur.address == 0 )
-            break;
-
-        fixup_guest_code_selector(current->domain, cur.cs);
-
-        XLAT_trap_info(dst + cur.vector, &cur);
-
-        if ( cur.vector == 0x80 )
-            init_int80_direct_trap(current);
-
-        guest_handle_add_offset(traps, 1);
-
-        if ( hypercall_preempt_check() )
-        {
-            rc = hypercall_create_continuation(
-                __HYPERVISOR_set_trap_table, "h", traps);
-            break;
-        }
-    }
-
-    return rc;
-}
 
 /*
  * Local variables:
-- 
2.11.0


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

[PATCH v5 05/13] x86: remove the now empty x86_64/compat/traps.c

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/x86_64/compat/traps.c | 15 ---------------
 xen/arch/x86/x86_64/traps.c        |  2 --
 2 files changed, 17 deletions(-)
 delete mode 100644 xen/arch/x86/x86_64/compat/traps.c

diff --git a/xen/arch/x86/x86_64/compat/traps.c b/xen/arch/x86/x86_64/compat/traps.c
deleted file mode 100644
index cade7f2923..0000000000
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ /dev/null
@@ -1,15 +0,0 @@
-#include <xen/event.h>
-#include <asm/regs.h>
-#include <compat/callback.h>
-#include <compat/arch-x86_32.h>
-
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 79bfc4d3f0..a15231ca0c 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -335,8 +335,6 @@ void subarch_percpu_traps_init(void)
     wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
 }
 
-#include "compat/traps.c"
-
 void hypercall_page_initialise(struct domain *d, void *hypercall_page)
 {
     memset(hypercall_page, 0xCC, PAGE_SIZE);
-- 
2.11.0


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

[PATCH v5 06/13] x86: simplify guest_has_trap_callback

[Wei Liu]

There is only one caller for that function. Simplify the function,
move it close to the caller and rename it.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/cpu/mcheck/vmce.c | 11 ++++++++++-
 xen/arch/x86/traps.c           | 18 ------------------
 xen/include/asm-x86/traps.h    |  8 --------
 3 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index d591d31600..368285810a 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -359,6 +359,15 @@ static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
                           vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
 
+static inline bool pv_callback_registered(const struct vcpu *v, uint8_t vector)
+{
+#ifdef CONFIG_PV
+    return v->arch.pv_vcpu.trap_ctxt[vector].address;
+#else
+    return false;
+#endif
+}
+
 /*
  * for Intel MCE, broadcast vMCE to all vcpus
  * for AMD MCE, only inject vMCE to vcpu0
@@ -383,7 +392,7 @@ int inject_vmce(struct domain *d, int vcpu)
             continue;
 
         if ( (is_hvm_domain(d) ||
-              guest_has_trap_callback(d, v->vcpu_id, TRAP_machine_check)) &&
+              pv_callback_registered(v, TRAP_machine_check)) &&
              !test_and_set_bool(v->mce_pending) )
         {
             mce_printk(MCE_VERBOSE, "MCE: inject vMCE to %pv\n", v);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index c43f3f1095..f3c5de6f2c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2008,24 +2008,6 @@ long unregister_guest_nmi_callback(void)
     return 0;
 }
 
-int guest_has_trap_callback(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
-{
-    struct vcpu *v;
-    struct trap_info *t;
-
-    BUG_ON(d == NULL);
-    BUG_ON(vcpuid >= d->max_vcpus);
-
-    /* Sanity check - XXX should be more fine grained. */
-    BUG_ON(trap_nr >= NR_VECTORS);
-
-    v = d->vcpu[vcpuid];
-    t = &v->arch.pv_vcpu.trap_ctxt[trap_nr];
-
-    return (t->address != 0);
-}
-
-
 int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
 {
     struct vcpu *v;
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 893b4dbe77..cc1d7d01d9 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -29,14 +29,6 @@ struct cpu_user_regs;
 
 void async_exception_cleanup(struct vcpu *);
  
-/**
- * guest_has_trap_callback
- *
- * returns true (non-zero) if guest registered a trap handler
- */
-extern int guest_has_trap_callback(struct domain *d, uint16_t vcpuid,
-				unsigned int trap_nr);
-
 /**
  * send_guest_trap
  *
-- 
2.11.0


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

[PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap

[Wei Liu]

Rename it to pv_raise_interrupt. Simplify the code by using the vcpu
structure already  at hand in the caller.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/traps.c           | 13 ++++---------
 xen/include/asm-x86/pv/traps.h |  8 ++++++++
 xen/include/asm-x86/traps.h    |  9 ---------
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index f3c5de6f2c..7e3cba0ffe 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1620,7 +1620,7 @@ static void nmi_hwdom_report(unsigned int reason_idx)
 
     set_bit(reason_idx, nmi_reason(d));
 
-    send_guest_trap(d, 0, TRAP_nmi);
+    pv_raise_interrupt(d->vcpu[0], TRAP_nmi);
 }
 
 static void pci_serr_error(const struct cpu_user_regs *regs)
@@ -2008,21 +2008,16 @@ long unregister_guest_nmi_callback(void)
     return 0;
 }
 
-int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
+int pv_raise_interrupt(struct vcpu *v, uint8_t trap_nr)
 {
-    struct vcpu *v;
     struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
 
-    BUG_ON(d == NULL);
-    BUG_ON(vcpuid >= d->max_vcpus);
-    v = d->vcpu[vcpuid];
-
     switch (trap_nr) {
     case TRAP_nmi:
         if ( cmpxchgptr(&st->vcpu, NULL, v) )
             return -EBUSY;
         if ( !test_and_set_bool(v->nmi_pending) ) {
-               st->domain = d;
+               st->domain = v->domain;
                st->processor = v->processor;
 
                /* not safe to wake up a vcpu here */
@@ -2040,7 +2035,7 @@ int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
          * on the physical CPU that reported a machine check error. */
 
         if ( !test_and_set_bool(v->mce_pending) ) {
-                st->domain = d;
+                st->domain = v->domain;
                 st->processor = v->processor;
 
                 /* not safe to wake up a vcpu here */
diff --git a/xen/include/asm-x86/pv/traps.h b/xen/include/asm-x86/pv/traps.h
index f9d44ab989..72e54a09a6 100644
--- a/xen/include/asm-x86/pv/traps.h
+++ b/xen/include/asm-x86/pv/traps.h
@@ -25,12 +25,20 @@
 
 #include <public/xen.h>
 
+/* Deliver interrupt to PV guest. Return 0 on success. */
+int pv_raise_interrupt(struct vcpu *v, uint8_t vector);
+
 int pv_emulate_privileged_op(struct cpu_user_regs *regs);
 void pv_emulate_gate_op(struct cpu_user_regs *regs);
 bool pv_emulate_invalid_op(struct cpu_user_regs *regs);
 
 #else  /* !CONFIG_PV */
 
+#include <xen/errno.h>
+
+/* Deliver interrupt to PV guest. Return 0 on success. */
+static int pv_raise_interrupt(struct vcpu *v, uint8_t vector) { return -EOPNOTSUPP; }
+
 static inline int pv_emulate_privileged_op(struct cpu_user_regs *regs) { return 0; }
 static inline void pv_emulate_gate_op(struct cpu_user_regs *regs) {}
 static inline bool pv_emulate_invalid_op(struct cpu_user_regs *regs) { return true; }
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index cc1d7d01d9..1e3f9c7fad 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -28,15 +28,6 @@ struct softirq_trap {
 struct cpu_user_regs;
 
 void async_exception_cleanup(struct vcpu *);
- 
-/**
- * send_guest_trap
- *
- * delivers trap to guest analogous to send_guest_global_virq
- * return 0 on successful delivery
- */
-extern int send_guest_trap(struct domain *d, uint16_t vcpuid,
-                           unsigned int trap_nr);
 
 uint32_t guest_io_read(unsigned int port, unsigned int bytes,
                        struct domain *);
-- 
2.11.0


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

[PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Wei Liu]

Factor out pv_trap_init and call it at the beginning of trap_init. We
then need to tune the code to generate stub handlers in entry.S. Take
the chance to tune init_irq_data so that 0x80 and 0x82 can be used in
!CONFIG_PV case.

While at it, fix some coding style issues in init_irq_data and replace
0x80 with LEGACY_SYSCALL_VECTOR in pv_trap_init.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/irq.c             |  9 ++++++---
 xen/arch/x86/traps.c           | 23 +++++++++++++++--------
 xen/arch/x86/x86_64/entry.S    |  4 ++++
 xen/include/asm-x86/pv/traps.h |  4 ++++
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 523d089006..b2709f4988 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -350,7 +350,7 @@ int __init init_irq_data(void)
     struct irq_desc *desc;
     int irq, vector;
 
-    for (vector = 0; vector < NR_VECTORS; ++vector)
+    for ( vector = 0; vector < NR_VECTORS; ++vector )
         this_cpu(vector_irq)[vector] = INT_MIN;
 
     irq_desc = xzalloc_array(struct irq_desc, nr_irqs);
@@ -358,17 +358,20 @@ int __init init_irq_data(void)
     if ( !irq_desc )
         return -ENOMEM;
 
-    for (irq = 0; irq < nr_irqs_gsi; irq++) {
+    for ( irq = 0; irq < nr_irqs_gsi; irq++ )
+    {
         desc = irq_to_desc(irq);
         desc->irq = irq;
         init_one_irq_desc(desc);
     }
-    for (; irq < nr_irqs; irq++)
+    for ( ; irq < nr_irqs; irq++ )
         irq_to_desc(irq)->irq = irq;
 
+#ifdef CONFIG_PV
     /* Never allocate the hypercall vector or Linux/BSD fast-trap vector. */
     set_bit(LEGACY_SYSCALL_VECTOR, used_vectors);
     set_bit(HYPERCALL_VECTOR, used_vectors);
+#endif
     
     /* IRQ_MOVE_CLEANUP_VECTOR used for clean up vectors */
     set_bit(IRQ_MOVE_CLEANUP_VECTOR, used_vectors);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 7e3cba0ffe..d89409ff05 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
     this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
 }
 
+void __init pv_trap_init(void)
+{
+    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
+    _set_gate(idt_table + HYPERCALL_VECTOR,
+              SYS_DESC_trap_gate, 1, entry_int82);
+
+    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
+    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
+              &int80_direct_trap);
+
+    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
+}
+
 extern void (*const autogen_entrypoints[NR_VECTORS])(void);
 void __init trap_init(void)
 {
     unsigned int vector;
 
+    pv_trap_init();
+
     /* Replace early pagefault with real pagefault handler. */
     set_intr_gate(TRAP_page_fault, &page_fault);
 
-    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
-    _set_gate(idt_table + HYPERCALL_VECTOR,
-              SYS_DESC_trap_gate, 1, entry_int82);
-
-    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
-    _set_gate(idt_table + 0x80, SYS_DESC_trap_gate, 3, &int80_direct_trap);
-
     for ( vector = 0; vector < NR_VECTORS; ++vector )
     {
         if ( autogen_entrypoints[vector] )
@@ -1968,7 +1976,6 @@ void __init trap_init(void)
 
     cpu_init();
 
-    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
     open_softirq(PCI_SERR_SOFTIRQ, pci_serr_softirq);
 }
 
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 65c771f979..b6e2397e84 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -706,7 +706,11 @@ autogen_stubs: /* Automatically generated stubs. */
         .rept NR_VECTORS
 
         /* Common interrupts, heading towards do_IRQ(). */
+#ifdef CONFIG_PV
         .if vec >= FIRST_DYNAMIC_VECTOR && vec != HYPERCALL_VECTOR && vec != LEGACY_SYSCALL_VECTOR
+#else
+        .if vec >= FIRST_DYNAMIC_VECTOR
+#endif
 
         ALIGN
 1:      pushq $0
diff --git a/xen/include/asm-x86/pv/traps.h b/xen/include/asm-x86/pv/traps.h
index 72e54a09a6..e64c713700 100644
--- a/xen/include/asm-x86/pv/traps.h
+++ b/xen/include/asm-x86/pv/traps.h
@@ -25,6 +25,8 @@
 
 #include <public/xen.h>
 
+void pv_trap_init(void);
+
 /* Deliver interrupt to PV guest. Return 0 on success. */
 int pv_raise_interrupt(struct vcpu *v, uint8_t vector);
 
@@ -36,6 +38,8 @@ bool pv_emulate_invalid_op(struct cpu_user_regs *regs);
 
 #include <xen/errno.h>
 
+static inline void pv_trap_init(void) {}
+
 /* Deliver interrupt to PV guest. Return 0 on success. */
 static int pv_raise_interrupt(struct vcpu *v, uint8_t vector) { return -EOPNOTSUPP; }
 
-- 
2.11.0


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

[PATCH v5 09/13] xen: move do_nmi_op and make it x86 only

[Wei Liu]

Since ARM doesn't need do_nmi_op, move the hypercall handler from
common/kernel.c to pv/callback.c. Drop the stubs in ARM. Delete the
common and ARM nmi.h and adjust header inclusions in various files.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>

ARM build test passed.
---
 xen/arch/x86/nmi.c              |  2 +-
 xen/arch/x86/oprofile/nmi_int.c |  2 +-
 xen/arch/x86/pv/callback.c      | 49 +++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/traps.c            |  2 +-
 xen/arch/x86/x86_64/traps.c     |  2 +-
 xen/common/compat/kernel.c      |  5 -----
 xen/common/kernel.c             | 26 ----------------------
 xen/include/asm-arm/nmi.h       | 15 -------------
 xen/include/xen/nmi.h           | 14 ------------
 9 files changed, 53 insertions(+), 64 deletions(-)
 delete mode 100644 xen/include/asm-arm/nmi.h
 delete mode 100644 xen/include/xen/nmi.h

diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index 410cfa1f94..ced61fd17e 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -17,7 +17,6 @@
 #include <xen/lib.h>
 #include <xen/mm.h>
 #include <xen/irq.h>
-#include <xen/nmi.h>
 #include <xen/delay.h>
 #include <xen/time.h>
 #include <xen/sched.h>
@@ -29,6 +28,7 @@
 #include <asm/mc146818rtc.h>
 #include <asm/msr.h>
 #include <asm/mpspec.h>
+#include <asm/nmi.h>
 #include <asm/debugger.h>
 #include <asm/div64.h>
 #include <asm/apic.h>
diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index 13534d4914..126f7a8d9f 100644
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -15,7 +15,6 @@
 #include <xen/types.h>
 #include <xen/errno.h>
 #include <xen/init.h>
-#include <xen/nmi.h>
 #include <xen/string.h>
 #include <xen/delay.h>
 #include <xen/xenoprof.h>
@@ -24,6 +23,7 @@
 #include <asm/apic.h>
 #include <asm/regs.h>
 #include <asm/current.h>
+#include <asm/nmi.h>
 
 #include "op_counter.h"
 #include "op_x86_model.h"
diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
index b9dd39bf0e..5317ae8f05 100644
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -22,6 +22,7 @@
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <compat/callback.h>
+#include <compat/nmi.h>
 
 #include <asm/current.h>
 #include <asm/nmi.h>
@@ -411,6 +412,54 @@ int compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
     return rc;
 }
 
+long do_nmi_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    struct xennmi_callback cb;
+    long rc = 0;
+
+    switch ( cmd )
+    {
+    case XENNMI_register_callback:
+        rc = -EFAULT;
+        if ( copy_from_guest(&cb, arg, 1) )
+            break;
+        rc = register_guest_nmi_callback(cb.handler_address);
+        break;
+    case XENNMI_unregister_callback:
+        rc = unregister_guest_nmi_callback();
+        break;
+    default:
+        rc = -ENOSYS;
+        break;
+    }
+
+    return rc;
+}
+
+int compat_nmi_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    struct compat_nmi_callback cb;
+    int rc = 0;
+
+    switch ( cmd )
+    {
+    case XENNMI_register_callback:
+        rc = -EFAULT;
+        if ( copy_from_guest(&cb, arg, 1) )
+            break;
+        rc = register_guest_nmi_callback(cb.handler_address);
+        break;
+    case XENNMI_unregister_callback:
+        rc = unregister_guest_nmi_callback();
+        break;
+    default:
+        rc = -ENOSYS;
+        break;
+    }
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d89409ff05..58f52926d9 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -43,7 +43,6 @@
 #include <xen/domain_page.h>
 #include <xen/symbols.h>
 #include <xen/iocap.h>
-#include <xen/nmi.h>
 #include <xen/version.h>
 #include <xen/kexec.h>
 #include <xen/trace.h>
@@ -64,6 +63,7 @@
 #include <asm/xstate.h>
 #include <asm/debugger.h>
 #include <asm/msr.h>
+#include <asm/nmi.h>
 #include <asm/shared.h>
 #include <asm/x86_emulate.h>
 #include <asm/traps.h>
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index a15231ca0c..41ec78f8fc 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -10,7 +10,6 @@
 #include <xen/console.h>
 #include <xen/sched.h>
 #include <xen/shutdown.h>
-#include <xen/nmi.h>
 #include <xen/guest_access.h>
 #include <xen/watchdog.h>
 #include <xen/hypercall.h>
@@ -18,6 +17,7 @@
 #include <asm/flushtlb.h>
 #include <asm/traps.h>
 #include <asm/event.h>
+#include <asm/nmi.h>
 #include <asm/msr.h>
 #include <asm/page.h>
 #include <asm/shared.h>
diff --git a/xen/common/compat/kernel.c b/xen/common/compat/kernel.c
index df93fdd89c..64232669d2 100644
--- a/xen/common/compat/kernel.c
+++ b/xen/common/compat/kernel.c
@@ -9,11 +9,9 @@ asm(".file \"" __FILE__ "\"");
 #include <xen/errno.h>
 #include <xen/version.h>
 #include <xen/sched.h>
-#include <xen/nmi.h>
 #include <xen/guest_access.h>
 #include <asm/current.h>
 #include <compat/xen.h>
-#include <compat/nmi.h>
 #include <compat/version.h>
 
 extern xen_commandline_t saved_cmdline;
@@ -39,9 +37,6 @@ CHECK_TYPE(capabilities_info);
 
 CHECK_TYPE(domain_handle);
 
-#define xennmi_callback compat_nmi_callback
-#define xennmi_callback_t compat_nmi_callback_t
-
 #ifdef COMPAT_VM_ASSIST_VALID
 #undef VM_ASSIST_VALID
 #define VM_ASSIST_VALID COMPAT_VM_ASSIST_VALID
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index e1ebb0b412..ce7cb8adb5 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -10,12 +10,10 @@
 #include <xen/version.h>
 #include <xen/sched.h>
 #include <xen/paging.h>
-#include <xen/nmi.h>
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
 #include <xsm/xsm.h>
 #include <asm/current.h>
-#include <public/nmi.h>
 #include <public/version.h>
 
 #ifndef COMPAT
@@ -431,30 +429,6 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     return -ENOSYS;
 }
 
-DO(nmi_op)(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    struct xennmi_callback cb;
-    long rc = 0;
-
-    switch ( cmd )
-    {
-    case XENNMI_register_callback:
-        rc = -EFAULT;
-        if ( copy_from_guest(&cb, arg, 1) )
-            break;
-        rc = register_guest_nmi_callback(cb.handler_address);
-        break;
-    case XENNMI_unregister_callback:
-        rc = unregister_guest_nmi_callback();
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
-
 #ifdef VM_ASSIST_VALID
 DO(vm_assist)(unsigned int cmd, unsigned int type)
 {
diff --git a/xen/include/asm-arm/nmi.h b/xen/include/asm-arm/nmi.h
deleted file mode 100644
index a60587e7d1..0000000000
--- a/xen/include/asm-arm/nmi.h
+++ /dev/null
@@ -1,15 +0,0 @@
-#ifndef ASM_NMI_H
-#define ASM_NMI_H
-
-#define register_guest_nmi_callback(a)  (-ENOSYS)
-#define unregister_guest_nmi_callback() (-ENOSYS)
-
-#endif /* ASM_NMI_H */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/xen/nmi.h b/xen/include/xen/nmi.h
deleted file mode 100644
index e526b6ab6f..0000000000
--- a/xen/include/xen/nmi.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/******************************************************************************
- * nmi.h
- *
- * Register and unregister NMI callbacks.
- *
- * Copyright (c) 2006, Ian Campbell <ian.campbell@xensource.com>
- */
-
-#ifndef __XEN_NMI_H__
-#define __XEN_NMI_H__
-
-#include <asm/nmi.h>
-
-#endif /* __XEN_NMI_H__ */
-- 
2.11.0


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

[PATCH v5 10/13] x86/traps: move {un, }register_guest_nmi_callback to pv/callback.c

[Wei Liu]

Move these helper functions along side their users. Now all users of
these functions are within the same file, make them static.

Take the chance to change v to curr and remove some unneeded
parentheses.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/callback.c | 37 +++++++++++++++++++++++++++++++++++++
 xen/arch/x86/traps.c       | 36 ------------------------------------
 xen/include/asm-x86/nmi.h  | 15 ---------------
 3 files changed, 37 insertions(+), 51 deletions(-)

diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
index 5317ae8f05..739ccb4d06 100644
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -26,10 +26,47 @@
 
 #include <asm/current.h>
 #include <asm/nmi.h>
+#include <asm/shared.h>
 #include <asm/traps.h>
 
 #include <public/callback.h>
 
+static long register_guest_nmi_callback(unsigned long address)
+{
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
+    struct trap_info *t = &curr->arch.pv_vcpu.trap_ctxt[TRAP_nmi];
+
+    if ( !is_canonical_address(address) )
+        return -EINVAL;
+
+    t->vector  = TRAP_nmi;
+    t->flags   = 0;
+    t->cs      = (is_pv_32bit_domain(d) ?
+                  FLAT_COMPAT_KERNEL_CS : FLAT_KERNEL_CS);
+    t->address = address;
+    TI_SET_IF(t, 1);
+
+    /*
+     * If no handler was registered we can 'lose the NMI edge'. Re-assert it
+     * now.
+     */
+    if ( curr->vcpu_id == 0 && arch_get_nmi_reason(d) != 0 )
+        curr->nmi_pending = 1;
+
+    return 0;
+}
+
+static long unregister_guest_nmi_callback(void)
+{
+    struct vcpu *curr = current;
+    struct trap_info *t = &curr->arch.pv_vcpu.trap_ctxt[TRAP_nmi];
+
+    memset(t, 0, sizeof(*t));
+
+    return 0;
+}
+
 static long register_guest_callback(struct callback_register *reg)
 {
     long ret = 0;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 58f52926d9..f12a52032a 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1979,42 +1979,6 @@ void __init trap_init(void)
     open_softirq(PCI_SERR_SOFTIRQ, pci_serr_softirq);
 }
 
-long register_guest_nmi_callback(unsigned long address)
-{
-    struct vcpu *v = current;
-    struct domain *d = v->domain;
-    struct trap_info *t = &v->arch.pv_vcpu.trap_ctxt[TRAP_nmi];
-
-    if ( !is_canonical_address(address) )
-        return -EINVAL;
-
-    t->vector  = TRAP_nmi;
-    t->flags   = 0;
-    t->cs      = (is_pv_32bit_domain(d) ?
-                  FLAT_COMPAT_KERNEL_CS : FLAT_KERNEL_CS);
-    t->address = address;
-    TI_SET_IF(t, 1);
-
-    /*
-     * If no handler was registered we can 'lose the NMI edge'. Re-assert it
-     * now.
-     */
-    if ( (v->vcpu_id == 0) && (arch_get_nmi_reason(d) != 0) )
-        v->nmi_pending = 1;
-
-    return 0;
-}
-
-long unregister_guest_nmi_callback(void)
-{
-    struct vcpu *v = current;
-    struct trap_info *t = &v->arch.pv_vcpu.trap_ctxt[TRAP_nmi];
-
-    memset(t, 0, sizeof(*t));
-
-    return 0;
-}
-
 int pv_raise_interrupt(struct vcpu *v, uint8_t trap_nr)
 {
     struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
diff --git a/xen/include/asm-x86/nmi.h b/xen/include/asm-x86/nmi.h
index fb0f57aa09..cb66e4b3f2 100644
--- a/xen/include/asm-x86/nmi.h
+++ b/xen/include/asm-x86/nmi.h
@@ -29,19 +29,4 @@ nmi_callback_t *set_nmi_callback(nmi_callback_t *callback);
  */
 void unset_nmi_callback(void);
  
-/**
- * register_guest_nmi_callback
- *
- * The default NMI handler passes the NMI to a guest callback. This
- * function registers the address of that callback.
- */
-long register_guest_nmi_callback(unsigned long address);
-
-/**
- * unregister_guest_nmi_callback
- *
- * Unregister a guest NMI handler.
- */
-long unregister_guest_nmi_callback(void);
-
 #endif /* ASM_NMI_H */
-- 
2.11.0


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

[PATCH v5 11/13] x86/callback.c: slightly change {un, }register_guest_nmi_callback

[Wei Liu]

Make register_guest_nmi_callback return int and make
unregister_guest_nmi_callback void. Adjust the callers where
necessary.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Can be squashed into previous patch.
---
 xen/arch/x86/pv/callback.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
index 739ccb4d06..5957cb5085 100644
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -31,7 +31,7 @@
 
 #include <public/callback.h>
 
-static long register_guest_nmi_callback(unsigned long address)
+static int register_guest_nmi_callback(unsigned long address)
 {
     struct vcpu *curr = current;
     struct domain *d = curr->domain;
@@ -57,14 +57,12 @@ static long register_guest_nmi_callback(unsigned long address)
     return 0;
 }
 
-static long unregister_guest_nmi_callback(void)
+static void unregister_guest_nmi_callback(void)
 {
     struct vcpu *curr = current;
     struct trap_info *t = &curr->arch.pv_vcpu.trap_ctxt[TRAP_nmi];
 
     memset(t, 0, sizeof(*t));
-
-    return 0;
 }
 
 static long register_guest_callback(struct callback_register *reg)
@@ -140,7 +138,8 @@ static long unregister_guest_callback(struct callback_unregister *unreg)
         break;
 
     case CALLBACKTYPE_nmi:
-        ret = unregister_guest_nmi_callback();
+        unregister_guest_nmi_callback();
+        ret = 0;
         break;
 
     default:
@@ -279,7 +278,8 @@ static long compat_unregister_guest_callback(
         break;
 
     case CALLBACKTYPE_nmi:
-        ret = unregister_guest_nmi_callback();
+        unregister_guest_nmi_callback();
+        ret = 0;
         break;
 
     default:
@@ -463,7 +463,8 @@ long do_nmi_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         rc = register_guest_nmi_callback(cb.handler_address);
         break;
     case XENNMI_unregister_callback:
-        rc = unregister_guest_nmi_callback();
+        unregister_guest_nmi_callback();
+        rc = 0;
         break;
     default:
         rc = -ENOSYS;
@@ -487,7 +488,8 @@ int compat_nmi_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         rc = register_guest_nmi_callback(cb.handler_address);
         break;
     case XENNMI_unregister_callback:
-        rc = unregister_guest_nmi_callback();
+        unregister_guest_nmi_callback();
+        rc = 0;
         break;
     default:
         rc = -ENOSYS;
-- 
2.11.0


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

[PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c

[Wei Liu]

Those functions must be moved at the same time. Also move softirq_trap
because it is only used there.

Fix some coding style issues while moving code.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/traps.c     | 104 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/traps.c        |  88 -------------------------------------
 xen/include/asm-x86/traps.h |   6 ---
 3 files changed, 104 insertions(+), 94 deletions(-)

diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 7d2f9aa638..1fce7df0c0 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -19,9 +19,11 @@
  * Copyright (c) 2017 Citrix Systems Ltd.
  */
 
+#include <xen/event.h>
 #include <xen/hypercall.h>
 #include <xen/lib.h>
 #include <xen/trace.h>
+#include <xen/softirq.h>
 
 #include <asm/apic.h>
 #include <asm/shared.h>
@@ -148,6 +150,108 @@ void init_int80_direct_trap(struct vcpu *v)
         tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
 }
 
+struct softirq_trap {
+    struct domain *domain; /* domain to inject trap */
+    struct vcpu *vcpu;     /* vcpu to inject trap */
+    int processor;         /* physical cpu to inject trap */
+};
+
+static DEFINE_PER_CPU(struct softirq_trap, softirq_trap);
+
+static void nmi_mce_softirq(void)
+{
+    int cpu = smp_processor_id();
+    struct softirq_trap *st = &per_cpu(softirq_trap, cpu);
+
+    BUG_ON(st->vcpu == NULL);
+
+    /*
+     * Set the tmp value unconditionally, so that the check in the iret
+     * hypercall works.
+     */
+    cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
+                 st->vcpu->cpu_hard_affinity);
+
+    if ( (cpu != st->processor) ||
+         (st->processor != st->vcpu->processor) )
+    {
+
+        /*
+	 * We are on a different physical cpu.  Make sure to wakeup the vcpu on
+	 * the specified processor.
+         */
+        vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
+
+        /* Affinity is restored in the iret hypercall. */
+    }
+
+    /*
+     * Only used to defer wakeup of domain/vcpu to a safe (non-NMI/MCE)
+     * context.
+     */
+    vcpu_kick(st->vcpu);
+    st->vcpu = NULL;
+}
+
+void __init pv_trap_init(void)
+{
+    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
+    _set_gate(idt_table + HYPERCALL_VECTOR,
+              SYS_DESC_trap_gate, 1, entry_int82);
+
+    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
+    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
+              &int80_direct_trap);
+
+    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
+}
+
+int pv_raise_interrupt(struct vcpu *v, uint8_t trap_nr)
+{
+    struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
+
+    switch ( trap_nr )
+    {
+    case TRAP_nmi:
+        if ( cmpxchgptr(&st->vcpu, NULL, v) )
+            return -EBUSY;
+        if ( !test_and_set_bool(v->nmi_pending) )
+        {
+            st->domain = v->domain;
+            st->processor = v->processor;
+
+            /* Not safe to wake up a vcpu here */
+            raise_softirq(NMI_MCE_SOFTIRQ);
+            return 0;
+        }
+        st->vcpu = NULL;
+        break;
+
+    case TRAP_machine_check:
+        if ( cmpxchgptr(&st->vcpu, NULL, v) )
+            return -EBUSY;
+
+        /*
+	 * We are called by the machine check (exception or polling) handlers
+	 * on the physical CPU that reported a machine check error.
+         */
+        if ( !test_and_set_bool(v->mce_pending) )
+        {
+            st->domain = v->domain;
+            st->processor = v->processor;
+
+            /* not safe to wake up a vcpu here */
+            raise_softirq(NMI_MCE_SOFTIRQ);
+            return 0;
+        }
+        st->vcpu = NULL;
+        break;
+    }
+
+    /* Delivery failed */
+    return -EIO;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index f12a52032a..4d6f42d168 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1540,39 +1540,6 @@ void do_general_protection(struct cpu_user_regs *regs)
     panic("GENERAL PROTECTION FAULT\n[error_code=%04x]", regs->error_code);
 }
 
-static DEFINE_PER_CPU(struct softirq_trap, softirq_trap);
-
-static void nmi_mce_softirq(void)
-{
-    int cpu = smp_processor_id();
-    struct softirq_trap *st = &per_cpu(softirq_trap, cpu);
-
-    BUG_ON(st->vcpu == NULL);
-
-    /* Set the tmp value unconditionally, so that
-     * the check in the iret hypercall works. */
-    cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
-                 st->vcpu->cpu_hard_affinity);
-
-    if ((cpu != st->processor)
-       || (st->processor != st->vcpu->processor))
-    {
-        /* We are on a different physical cpu.
-         * Make sure to wakeup the vcpu on the
-         * specified processor.
-         */
-        vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
-
-        /* Affinity is restored in the iret hypercall. */
-    }
-
-    /* Only used to defer wakeup of domain/vcpu to
-     * a safe (non-NMI/MCE) context.
-     */
-    vcpu_kick(st->vcpu);
-    st->vcpu = NULL;
-}
-
 static void pci_serr_softirq(void)
 {
     printk("\n\nNMI - PCI system error (SERR)\n");
@@ -1934,19 +1901,6 @@ void __init init_idt_traps(void)
     this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
 }
 
-void __init pv_trap_init(void)
-{
-    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
-    _set_gate(idt_table + HYPERCALL_VECTOR,
-              SYS_DESC_trap_gate, 1, entry_int82);
-
-    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
-    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
-              &int80_direct_trap);
-
-    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
-}
-
 extern void (*const autogen_entrypoints[NR_VECTORS])(void);
 void __init trap_init(void)
 {
@@ -1979,48 +1933,6 @@ void __init trap_init(void)
     open_softirq(PCI_SERR_SOFTIRQ, pci_serr_softirq);
 }
 
-int pv_raise_interrupt(struct vcpu *v, uint8_t trap_nr)
-{
-    struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
-
-    switch (trap_nr) {
-    case TRAP_nmi:
-        if ( cmpxchgptr(&st->vcpu, NULL, v) )
-            return -EBUSY;
-        if ( !test_and_set_bool(v->nmi_pending) ) {
-               st->domain = v->domain;
-               st->processor = v->processor;
-
-               /* not safe to wake up a vcpu here */
-               raise_softirq(NMI_MCE_SOFTIRQ);
-               return 0;
-        }
-        st->vcpu = NULL;
-        break;
-
-    case TRAP_machine_check:
-        if ( cmpxchgptr(&st->vcpu, NULL, v) )
-            return -EBUSY;
-
-        /* We are called by the machine check (exception or polling) handlers
-         * on the physical CPU that reported a machine check error. */
-
-        if ( !test_and_set_bool(v->mce_pending) ) {
-                st->domain = v->domain;
-                st->processor = v->processor;
-
-                /* not safe to wake up a vcpu here */
-                raise_softirq(NMI_MCE_SOFTIRQ);
-                return 0;
-        }
-        st->vcpu = NULL;
-        break;
-    }
-
-    /* delivery failed */
-    return -EIO;
-}
-
 void activate_debugregs(const struct vcpu *curr)
 {
     ASSERT(curr == current);
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 1e3f9c7fad..8d903ec91b 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -19,12 +19,6 @@
 #ifndef ASM_TRAP_H
 #define ASM_TRAP_H
 
-struct softirq_trap {
-	struct domain *domain;  /* domain to inject trap */
-	struct vcpu *vcpu;	/* vcpu to inject trap */
-	int processor;		/* physical cpu to inject trap */
-};
-
 struct cpu_user_regs;
 
 void async_exception_cleanup(struct vcpu *);
-- 
2.11.0


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

[PATCH v5 13/13] x86/traps.h: remove unused declaration of cpu_user_regs

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/asm-x86/traps.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 8d903ec91b..bed25290d7 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -19,8 +19,6 @@
 #ifndef ASM_TRAP_H
 #define ASM_TRAP_H
 
-struct cpu_user_regs;
-
 void async_exception_cleanup(struct vcpu *);
 
 uint32_t guest_io_read(unsigned int port, unsigned int bytes,
-- 
2.11.0


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

Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Take the chance to change v to curr.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 02/13] x86: move the compat callback ops next to the non-compat variant

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Take the chance to change v to curr.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 03/13] x86: move do_set_trap_table to pv/callback.c

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> That hypercall is used to set guest callbacks for traps.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>

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

Re: [PATCH v5 04/13] x86: move compat_set_trap_table along side the non-compat variant

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 05/13] x86: remove the now empty x86_64/compat/traps.c

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 06/13] x86: simplify guest_has_trap_callback

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> There is only one caller for that function. Simplify the function,
> move it close to the caller and rename it.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Good improvement.  In principle, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>, although...

> ---
>  xen/arch/x86/cpu/mcheck/vmce.c | 11 ++++++++++-
>  xen/arch/x86/traps.c           | 18 ------------------
>  xen/include/asm-x86/traps.h    |  8 --------
>  3 files changed, 10 insertions(+), 27 deletions(-)
>
> diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
> index d591d31600..368285810a 100644
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -359,6 +359,15 @@ static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
>  HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
>                            vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
>  
> +static inline bool pv_callback_registered(const struct vcpu *v, uint8_t vector)
> +{
> +#ifdef CONFIG_PV
> +    return v->arch.pv_vcpu.trap_ctxt[vector].address;
> +#else
> +    return false;
> +#endif
> +}
> +

Isn't there a header file this would be better living in?  Its certainly
not vmce-specific.

~Andrew

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

Re: [PATCH v5 06/13] x86: simplify guest_has_trap_callback

[Jan Beulich]

>>> Andrew Cooper <andrew.cooper3@citrix.com> 06/26/17 6:58 PM >>>
>On 26/06/17 17:28, Wei Liu wrote:
>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>> @@ -359,6 +359,15 @@ static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
>>  HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
>>                            vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
>>  
>> +static inline bool pv_callback_registered(const struct vcpu *v, uint8_t vector)
>> +{
>> +#ifdef CONFIG_PV
>> +    return v->arch.pv_vcpu.trap_ctxt[vector].address;
>> +#else
>> +    return false;
>> +#endif
>> +}
>> +
>
>Isn't there a header file this would be better living in?  Its certainly
>not vmce-specific.

We certainly have the equivalent of this check in assembly code (which
iirc we mean to convert to C eventually), so pv/traps.h would seem to be
the right place. The function name would need to change a little though,
as we're talking about exception callbacks here, not the things we call
callbacks in the public interface. pv_trap_callback_registered() perhaps?

Jan



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

Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>--- a/xen/arch/x86/pv/Makefile
>+++ b/xen/arch/x86/pv/Makefile
>@@ -1,6 +1,7 @@
 >obj-y += hypercall.o
 >obj-y += traps.o
 >
>+obj-y += callback.o
 >obj-bin-y += dom0_build.init.o
 >obj-y += domain.o
 >obj-y += emulate.o

Not something to be dealt with in this patch, but - is there a reason we
have two groups of object files here? I see none, and hence would have
expected this to be a single sorted list instead of two.

Jan


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

Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c

[Wei Liu]

On Tue, Jun 27, 2017 at 12:13:19AM -0600, Jan Beulich wrote:
> >>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
> >--- a/xen/arch/x86/pv/Makefile
> >+++ b/xen/arch/x86/pv/Makefile
> >@@ -1,6 +1,7 @@
>  >obj-y += hypercall.o
>  >obj-y += traps.o
>  >
> >+obj-y += callback.o
>  >obj-bin-y += dom0_build.init.o
>  >obj-y += domain.o
>  >obj-y += emulate.o
> 
> Not something to be dealt with in this patch, but - is there a reason we
> have two groups of object files here? I see none, and hence would have
> expected this to be a single sorted list instead of two.
> 

No. It just so happened I added a newline at some point.

I will submit a patch to fix this at some point.

> Jan
> 

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

Re: [PATCH v5 06/13] x86: simplify guest_has_trap_callback

[Wei Liu]

On Tue, Jun 27, 2017 at 12:09:14AM -0600, Jan Beulich wrote:
> >>> Andrew Cooper <andrew.cooper3@citrix.com> 06/26/17 6:58 PM >>>
> >On 26/06/17 17:28, Wei Liu wrote:
> >> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> >> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> >> @@ -359,6 +359,15 @@ static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> >>  HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
> >>                            vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
> >>  
> >> +static inline bool pv_callback_registered(const struct vcpu *v, uint8_t vector)
> >> +{
> >> +#ifdef CONFIG_PV
> >> +    return v->arch.pv_vcpu.trap_ctxt[vector].address;
> >> +#else
> >> +    return false;
> >> +#endif
> >> +}
> >> +
> >
> >Isn't there a header file this would be better living in?  Its certainly
> >not vmce-specific.
> 
> We certainly have the equivalent of this check in assembly code (which
> iirc we mean to convert to C eventually), so pv/traps.h would seem to be
> the right place. The function name would need to change a little though,
> as we're talking about exception callbacks here, not the things we call
> callbacks in the public interface. pv_trap_callback_registered() perhaps?

I agree pv/traps.h is the right place and I've rename it to
pv_trap_callback_registered.

I kept Andrew's Rb because those are just cosmetic changes.

---8<---
From d211f6a7aea2e8497bddf72df084f37588234e2d Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Fri, 23 Jun 2017 18:34:41 +0100
Subject: [PATCH] x86: simplify guest_has_trap_callback

Simplify that function, rename it to pv_trap_callback_registered and
move it to pv/traps.h. Adjust vmce.c accordingly.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/cpu/mcheck/vmce.c |  3 ++-
 xen/arch/x86/traps.c           | 18 ------------------
 xen/include/asm-x86/pv/traps.h | 11 +++++++++++
 xen/include/asm-x86/traps.h    |  8 --------
 4 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index d591d31600..1356f611ab 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -32,6 +32,7 @@
 #include <asm/system.h>
 #include <asm/msr.h>
 #include <asm/p2m.h>
+#include <asm/pv/traps.h>
 
 #include "mce.h"
 #include "x86_mca.h"
@@ -383,7 +384,7 @@ int inject_vmce(struct domain *d, int vcpu)
             continue;
 
         if ( (is_hvm_domain(d) ||
-              guest_has_trap_callback(d, v->vcpu_id, TRAP_machine_check)) &&
+              pv_trap_callback_registered(v, TRAP_machine_check)) &&
              !test_and_set_bool(v->mce_pending) )
         {
             mce_printk(MCE_VERBOSE, "MCE: inject vMCE to %pv\n", v);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index c43f3f1095..f3c5de6f2c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2008,24 +2008,6 @@ long unregister_guest_nmi_callback(void)
     return 0;
 }
 
-int guest_has_trap_callback(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
-{
-    struct vcpu *v;
-    struct trap_info *t;
-
-    BUG_ON(d == NULL);
-    BUG_ON(vcpuid >= d->max_vcpus);
-
-    /* Sanity check - XXX should be more fine grained. */
-    BUG_ON(trap_nr >= NR_VECTORS);
-
-    v = d->vcpu[vcpuid];
-    t = &v->arch.pv_vcpu.trap_ctxt[trap_nr];
-
-    return (t->address != 0);
-}
-
-
 int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
 {
     struct vcpu *v;
diff --git a/xen/include/asm-x86/pv/traps.h b/xen/include/asm-x86/pv/traps.h
index f9d44ab989..c32c38c372 100644
--- a/xen/include/asm-x86/pv/traps.h
+++ b/xen/include/asm-x86/pv/traps.h
@@ -29,12 +29,23 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs);
 void pv_emulate_gate_op(struct cpu_user_regs *regs);
 bool pv_emulate_invalid_op(struct cpu_user_regs *regs);
 
+static inline bool pv_trap_callback_registered(const struct vcpu *v,
+                                               uint8_t vector)
+{
+    return v->arch.pv_vcpu.trap_ctxt[vector].address;
+}
+
 #else  /* !CONFIG_PV */
 
 static inline int pv_emulate_privileged_op(struct cpu_user_regs *regs) { return 0; }
 static inline void pv_emulate_gate_op(struct cpu_user_regs *regs) {}
 static inline bool pv_emulate_invalid_op(struct cpu_user_regs *regs) { return true; }
 
+static inline bool pv_trap_callback_registered(const struct vcpu *v,
+                                               uint8_t vector)
+{
+    return false;
+}
 #endif /* CONFIG_PV */
 
 #endif /* __X86_PV_TRAPS_H__ */
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 893b4dbe77..cc1d7d01d9 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -29,14 +29,6 @@ struct cpu_user_regs;
 
 void async_exception_cleanup(struct vcpu *);
  
-/**
- * guest_has_trap_callback
- *
- * returns true (non-zero) if guest registered a trap handler
- */
-extern int guest_has_trap_callback(struct domain *d, uint16_t vcpuid,
-				unsigned int trap_nr);
-
 /**
  * send_guest_trap
  *
-- 
2.11.0


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

Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c

[Andrew Cooper]

On 27/06/17 09:48, Wei Liu wrote:
> On Tue, Jun 27, 2017 at 12:13:19AM -0600, Jan Beulich wrote:
>>>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>>> --- a/xen/arch/x86/pv/Makefile
>>> +++ b/xen/arch/x86/pv/Makefile
>>> @@ -1,6 +1,7 @@
>>  >obj-y += hypercall.o
>>  >obj-y += traps.o
>>  >
>>> +obj-y += callback.o
>>  >obj-bin-y += dom0_build.init.o
>>  >obj-y += domain.o
>>  >obj-y += emulate.o
>>
>> Not something to be dealt with in this patch, but - is there a reason we
>> have two groups of object files here? I see none, and hence would have
>> expected this to be a single sorted list instead of two.
>>
> No. It just so happened I added a newline at some point.
>
> I will submit a patch to fix this at some point.

I'd suggest splitting the obj-y and obj-bin-y lists, as it will make the
Makefile easier to read.  That appears to have been the original
intention behind the space in the first place.

~Andrew

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

Re: [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Rename it to pv_raise_interrupt. Simplify the code by using the vcpu
> structure already  at hand in the caller.

double space.

>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Factor out pv_trap_init and call it at the beginning of trap_init. We
> then need to tune the code to generate stub handlers in entry.S. Take
> the chance to tune init_irq_data so that 0x80 and 0x82 can be used in
> !CONFIG_PV case.

"used for regular interrupts in ..." ?

> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 7e3cba0ffe..d89409ff05 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
>      this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
>  }
>  
> +void __init pv_trap_init(void)
> +{
> +    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
> +    _set_gate(idt_table + HYPERCALL_VECTOR,

&idt_table[HYPERCALL_VECTOR]

> +              SYS_DESC_trap_gate, 1, entry_int82);
> +
> +    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> +    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,

&idt_table[LEGACY_SYSCALL_VECTOR]

Otherwise, Reivewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +              &int80_direct_trap);
> +
> +    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
> +}
> +
>  extern void (*const autogen_entrypoints[NR_VECTORS])(void);
>  void __init trap_init(void)
>  {
>      unsigned int vector;
>  
> +    pv_trap_init();
> +
>      /* Replace early pagefault with real pagefault handler. */
>      set_intr_gate(TRAP_page_fault, &page_fault);
>  
> -    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
> -    _set_gate(idt_table + HYPERCALL_VECTOR,
> -              SYS_DESC_trap_gate, 1, entry_int82);
> -
> -    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> -    _set_gate(idt_table + 0x80, SYS_DESC_trap_gate, 3, &int80_direct_trap);
> -
>      for ( vector = 0; vector < NR_VECTORS; ++vector )
>      {
>          if ( autogen_entrypoints[vector] )


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

Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Since ARM doesn't need do_nmi_op, move the hypercall handler from
> common/kernel.c to pv/callback.c. Drop the stubs in ARM. Delete the
> common and ARM nmi.h and adjust header inclusions in various files.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 10/13] x86/traps: move {un, }register_guest_nmi_callback to pv/callback.c

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Move these helper functions along side their users. Now all users of
> these functions are within the same file, make them static.
>
> Take the chance to change v to curr and remove some unneeded
> parentheses.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 11/13] x86/callback.c: slightly change {un, }register_guest_nmi_callback

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Make register_guest_nmi_callback return int and make
> unregister_guest_nmi_callback void. Adjust the callers where
> necessary.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> @@ -148,6 +150,108 @@ void init_int80_direct_trap(struct vcpu *v)
>          tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
>  }
>  
> +struct softirq_trap {
> +    struct domain *domain; /* domain to inject trap */
> +    struct vcpu *vcpu;     /* vcpu to inject trap */
> +    int processor;         /* physical cpu to inject trap */
> +};
> +
> +static DEFINE_PER_CPU(struct softirq_trap, softirq_trap);
> +
> +static void nmi_mce_softirq(void)
> +{
> +    int cpu = smp_processor_id();
> +    struct softirq_trap *st = &per_cpu(softirq_trap, cpu);
> +
> +    BUG_ON(st->vcpu == NULL);
> +
> +    /*
> +     * Set the tmp value unconditionally, so that the check in the iret
> +     * hypercall works.
> +     */
> +    cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
> +                 st->vcpu->cpu_hard_affinity);
> +
> +    if ( (cpu != st->processor) ||
> +         (st->processor != st->vcpu->processor) )
> +    {
> +
> +        /*
> +	 * We are on a different physical cpu.  Make sure to wakeup the vcpu on
> +	 * the specified processor.
> +         */

You have some stray tabs here.

> +        vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
> +
> +        /* Affinity is restored in the iret hypercall. */
> +    }
> +
> +    /*
> +     * Only used to defer wakeup of domain/vcpu to a safe (non-NMI/MCE)
> +     * context.
> +     */
> +    vcpu_kick(st->vcpu);
> +    st->vcpu = NULL;
> +}
> +
> +void __init pv_trap_init(void)
> +{
> +    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
> +    _set_gate(idt_table + HYPERCALL_VECTOR,
> +              SYS_DESC_trap_gate, 1, entry_int82);
> +
> +    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> +    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
> +              &int80_direct_trap);
> +
> +    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
> +}
> +
> +int pv_raise_interrupt(struct vcpu *v, uint8_t trap_nr)

Please s/trap_nr/vector/ as you are moving it.  Unlike
guest_has_trap_callback(), the uint8_t here isn't important.

> +{
> +    struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
> +
> +    switch ( trap_nr )
> +    {
> +    case TRAP_nmi:
> +        if ( cmpxchgptr(&st->vcpu, NULL, v) )
> +            return -EBUSY;
> +        if ( !test_and_set_bool(v->nmi_pending) )
> +        {
> +            st->domain = v->domain;
> +            st->processor = v->processor;
> +
> +            /* Not safe to wake up a vcpu here */
> +            raise_softirq(NMI_MCE_SOFTIRQ);
> +            return 0;
> +        }
> +        st->vcpu = NULL;
> +        break;
> +
> +    case TRAP_machine_check:
> +        if ( cmpxchgptr(&st->vcpu, NULL, v) )
> +            return -EBUSY;
> +
> +        /*
> +	 * We are called by the machine check (exception or polling) handlers
> +	 * on the physical CPU that reported a machine check error.
> +         */

And more tabs here.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 13/13] x86/traps.h: remove unused declaration of cpu_user_regs

[Andrew Cooper]

On 26/06/17 17:28, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Jan Beulich]

>>> Andrew Cooper <andrew.cooper3@citrix.com> 06/27/17 8:06 PM >>>
>On 26/06/17 17:28, Wei Liu wrote:
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
>>      this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
>>  }
>>  
>> +void __init pv_trap_init(void)
>> +{
>> +    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
>> +    _set_gate(idt_table + HYPERCALL_VECTOR,
>
>&idt_table[HYPERCALL_VECTOR]

I don't think we should require this. Personally I prefer the form Wei used,
and iirc there's nothing in our coding style guidelines that mandates the
form you suggest.

Jan


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

Re: [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>Rename it to pv_raise_interrupt.

Is "interrupt" really a suitable term here? These are all exceptions being
raised, not (external) interrupts.

Jan


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

Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>--- a/xen/arch/x86/traps.c
>+++ b/xen/arch/x86/traps.c
>@@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
     >this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
 >}
 >
>+void __init pv_trap_init(void)
>+{
>+    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
>+    _set_gate(idt_table + HYPERCALL_VECTOR,
>+              SYS_DESC_trap_gate, 1, entry_int82);
>+
>+    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
>+    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
>+              &int80_direct_trap);
>+
>+    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
>+}

Any reason you don't move this into pv/traps.c right away?

>void __init trap_init(void)
 >{
     >unsigned int vector;
 >
>+    pv_trap_init();

I think this would better be called after ...

>/* Replace early pagefault with real pagefault handler. */
     >set_intr_gate(TRAP_page_fault, &page_fault);
 
... this, i.e. in line with where most of the code was before.

Jan


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

Re: [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap

[Andrew Cooper]

On 27/06/17 19:21, Jan Beulich wrote:
>>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>> Rename it to pv_raise_interrupt.
> Is "interrupt" really a suitable term here? These are all exceptions being
> raised, not (external) interrupts.

This function is really NMIs and MCEs only, and it only gets used for
the former.  (on a technicality, NMIs are classified as Interrupts
rather than Exceptions).

Furthermore, it is only actually used for latching an NMI as pending
with dom0 from NMI context.  MCEs take a different path.

I've been considering whether this mechanism can be done any better.  I
expect it can, but I haven't got a better plan yet.

~Andrew

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

Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>Since ARM doesn't need do_nmi_op, move the hypercall handler from
>common/kernel.c to pv/callback.c.

There are two handlers you actually move, and while their neither large nor
likely to change, I still somewhat dislike the code duplication you introduce.
But I guess not enough to put under question the R-b you've already got
from Andrew.

Jan


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

Re: [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:46 PM >>>
>@@ -148,6 +150,108 @@ void init_int80_direct_trap(struct vcpu *v)
         >tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
 >}
 >
>+struct softirq_trap {
>+    struct domain *domain; /* domain to inject trap */
>+    struct vcpu *vcpu;     /* vcpu to inject trap */
>+    int processor;         /* physical cpu to inject trap */

unsigned int please as you go.

>+static void nmi_mce_softirq(void)
>+{
>+    int cpu = smp_processor_id();

Same here.

Jan


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

Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only

[Andrew Cooper]

On 27/06/17 19:31, Jan Beulich wrote:
>>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
>> Since ARM doesn't need do_nmi_op, move the hypercall handler from
>> common/kernel.c to pv/callback.c.
> There are two handlers you actually move, and while their neither large nor
> likely to change, I still somewhat dislike the code duplication you introduce.
> But I guess not enough to put under question the R-b you've already got
> from Andrew.

If its not obvious already, I am taking a strong stance against any code
obfuscation, because obfuscation the source code is entirely detrimental
to the project.

Macros such as DO() fall into the obfuscation category, so I will
support all changes which reduce/remove its usage.

~Andrew

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

Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only

[Stefano Stabellini]

On Mon, 26 Jun 2017, Wei Liu wrote:
> Since ARM doesn't need do_nmi_op, move the hypercall handler from
> common/kernel.c to pv/callback.c. Drop the stubs in ARM. Delete the
> common and ARM nmi.h and adjust header inclusions in various files.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> 
> ARM build test passed.
> ---
>  xen/arch/x86/nmi.c              |  2 +-
>  xen/arch/x86/oprofile/nmi_int.c |  2 +-
>  xen/arch/x86/pv/callback.c      | 49 +++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/traps.c            |  2 +-
>  xen/arch/x86/x86_64/traps.c     |  2 +-
>  xen/common/compat/kernel.c      |  5 -----
>  xen/common/kernel.c             | 26 ----------------------
>  xen/include/asm-arm/nmi.h       | 15 -------------
>  xen/include/xen/nmi.h           | 14 ------------
>  9 files changed, 53 insertions(+), 64 deletions(-)
>  delete mode 100644 xen/include/asm-arm/nmi.h
>  delete mode 100644 xen/include/xen/nmi.h
> 
> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
> index 410cfa1f94..ced61fd17e 100644
> --- a/xen/arch/x86/nmi.c
> +++ b/xen/arch/x86/nmi.c
> @@ -17,7 +17,6 @@
>  #include <xen/lib.h>
>  #include <xen/mm.h>
>  #include <xen/irq.h>
> -#include <xen/nmi.h>
>  #include <xen/delay.h>
>  #include <xen/time.h>
>  #include <xen/sched.h>
> @@ -29,6 +28,7 @@
>  #include <asm/mc146818rtc.h>
>  #include <asm/msr.h>
>  #include <asm/mpspec.h>
> +#include <asm/nmi.h>
>  #include <asm/debugger.h>
>  #include <asm/div64.h>
>  #include <asm/apic.h>
> diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
> index 13534d4914..126f7a8d9f 100644
> --- a/xen/arch/x86/oprofile/nmi_int.c
> +++ b/xen/arch/x86/oprofile/nmi_int.c
> @@ -15,7 +15,6 @@
>  #include <xen/types.h>
>  #include <xen/errno.h>
>  #include <xen/init.h>
> -#include <xen/nmi.h>
>  #include <xen/string.h>
>  #include <xen/delay.h>
>  #include <xen/xenoprof.h>
> @@ -24,6 +23,7 @@
>  #include <asm/apic.h>
>  #include <asm/regs.h>
>  #include <asm/current.h>
> +#include <asm/nmi.h>
>  
>  #include "op_counter.h"
>  #include "op_x86_model.h"
> diff --git a/xen/arch/x86/pv/callback.c b/xen/arch/x86/pv/callback.c
> index b9dd39bf0e..5317ae8f05 100644
> --- a/xen/arch/x86/pv/callback.c
> +++ b/xen/arch/x86/pv/callback.c
> @@ -22,6 +22,7 @@
>  #include <xen/lib.h>
>  #include <xen/sched.h>
>  #include <compat/callback.h>
> +#include <compat/nmi.h>
>  
>  #include <asm/current.h>
>  #include <asm/nmi.h>
> @@ -411,6 +412,54 @@ int compat_set_trap_table(XEN_GUEST_HANDLE(trap_info_compat_t) traps)
>      return rc;
>  }
>  
> +long do_nmi_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    struct xennmi_callback cb;
> +    long rc = 0;
> +
> +    switch ( cmd )
> +    {
> +    case XENNMI_register_callback:
> +        rc = -EFAULT;
> +        if ( copy_from_guest(&cb, arg, 1) )
> +            break;
> +        rc = register_guest_nmi_callback(cb.handler_address);
> +        break;
> +    case XENNMI_unregister_callback:
> +        rc = unregister_guest_nmi_callback();
> +        break;
> +    default:
> +        rc = -ENOSYS;
> +        break;
> +    }
> +
> +    return rc;
> +}
> +
> +int compat_nmi_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    struct compat_nmi_callback cb;
> +    int rc = 0;
> +
> +    switch ( cmd )
> +    {
> +    case XENNMI_register_callback:
> +        rc = -EFAULT;
> +        if ( copy_from_guest(&cb, arg, 1) )
> +            break;
> +        rc = register_guest_nmi_callback(cb.handler_address);
> +        break;
> +    case XENNMI_unregister_callback:
> +        rc = unregister_guest_nmi_callback();
> +        break;
> +    default:
> +        rc = -ENOSYS;
> +        break;
> +    }
> +
> +    return rc;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index d89409ff05..58f52926d9 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -43,7 +43,6 @@
>  #include <xen/domain_page.h>
>  #include <xen/symbols.h>
>  #include <xen/iocap.h>
> -#include <xen/nmi.h>
>  #include <xen/version.h>
>  #include <xen/kexec.h>
>  #include <xen/trace.h>
> @@ -64,6 +63,7 @@
>  #include <asm/xstate.h>
>  #include <asm/debugger.h>
>  #include <asm/msr.h>
> +#include <asm/nmi.h>
>  #include <asm/shared.h>
>  #include <asm/x86_emulate.h>
>  #include <asm/traps.h>
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index a15231ca0c..41ec78f8fc 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -10,7 +10,6 @@
>  #include <xen/console.h>
>  #include <xen/sched.h>
>  #include <xen/shutdown.h>
> -#include <xen/nmi.h>
>  #include <xen/guest_access.h>
>  #include <xen/watchdog.h>
>  #include <xen/hypercall.h>
> @@ -18,6 +17,7 @@
>  #include <asm/flushtlb.h>
>  #include <asm/traps.h>
>  #include <asm/event.h>
> +#include <asm/nmi.h>
>  #include <asm/msr.h>
>  #include <asm/page.h>
>  #include <asm/shared.h>
> diff --git a/xen/common/compat/kernel.c b/xen/common/compat/kernel.c
> index df93fdd89c..64232669d2 100644
> --- a/xen/common/compat/kernel.c
> +++ b/xen/common/compat/kernel.c
> @@ -9,11 +9,9 @@ asm(".file \"" __FILE__ "\"");
>  #include <xen/errno.h>
>  #include <xen/version.h>
>  #include <xen/sched.h>
> -#include <xen/nmi.h>
>  #include <xen/guest_access.h>
>  #include <asm/current.h>
>  #include <compat/xen.h>
> -#include <compat/nmi.h>
>  #include <compat/version.h>
>  
>  extern xen_commandline_t saved_cmdline;
> @@ -39,9 +37,6 @@ CHECK_TYPE(capabilities_info);
>  
>  CHECK_TYPE(domain_handle);
>  
> -#define xennmi_callback compat_nmi_callback
> -#define xennmi_callback_t compat_nmi_callback_t
> -
>  #ifdef COMPAT_VM_ASSIST_VALID
>  #undef VM_ASSIST_VALID
>  #define VM_ASSIST_VALID COMPAT_VM_ASSIST_VALID
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index e1ebb0b412..ce7cb8adb5 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -10,12 +10,10 @@
>  #include <xen/version.h>
>  #include <xen/sched.h>
>  #include <xen/paging.h>
> -#include <xen/nmi.h>
>  #include <xen/guest_access.h>
>  #include <xen/hypercall.h>
>  #include <xsm/xsm.h>
>  #include <asm/current.h>
> -#include <public/nmi.h>
>  #include <public/version.h>
>  
>  #ifndef COMPAT
> @@ -431,30 +429,6 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      return -ENOSYS;
>  }
>  
> -DO(nmi_op)(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> -{
> -    struct xennmi_callback cb;
> -    long rc = 0;
> -
> -    switch ( cmd )
> -    {
> -    case XENNMI_register_callback:
> -        rc = -EFAULT;
> -        if ( copy_from_guest(&cb, arg, 1) )
> -            break;
> -        rc = register_guest_nmi_callback(cb.handler_address);
> -        break;
> -    case XENNMI_unregister_callback:
> -        rc = unregister_guest_nmi_callback();
> -        break;
> -    default:
> -        rc = -ENOSYS;
> -        break;
> -    }
> -
> -    return rc;
> -}
> -
>  #ifdef VM_ASSIST_VALID
>  DO(vm_assist)(unsigned int cmd, unsigned int type)
>  {
> diff --git a/xen/include/asm-arm/nmi.h b/xen/include/asm-arm/nmi.h
> deleted file mode 100644
> index a60587e7d1..0000000000
> --- a/xen/include/asm-arm/nmi.h
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -#ifndef ASM_NMI_H
> -#define ASM_NMI_H
> -
> -#define register_guest_nmi_callback(a)  (-ENOSYS)
> -#define unregister_guest_nmi_callback() (-ENOSYS)
> -
> -#endif /* ASM_NMI_H */
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */
> diff --git a/xen/include/xen/nmi.h b/xen/include/xen/nmi.h
> deleted file mode 100644
> index e526b6ab6f..0000000000
> --- a/xen/include/xen/nmi.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/******************************************************************************
> - * nmi.h
> - *
> - * Register and unregister NMI callbacks.
> - *
> - * Copyright (c) 2006, Ian Campbell <ian.campbell@xensource.com>
> - */
> -
> -#ifndef __XEN_NMI_H__
> -#define __XEN_NMI_H__
> -
> -#include <asm/nmi.h>
> -
> -#endif /* __XEN_NMI_H__ */
> -- 
> 2.11.0
> 

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

Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Wei Liu]

On Tue, Jun 27, 2017 at 12:26:52PM -0600, Jan Beulich wrote:
> >>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
> >--- a/xen/arch/x86/traps.c
> >+++ b/xen/arch/x86/traps.c
> >@@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
>      >this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
>  >}
>  >
> >+void __init pv_trap_init(void)
> >+{
> >+    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
> >+    _set_gate(idt_table + HYPERCALL_VECTOR,
> >+              SYS_DESC_trap_gate, 1, entry_int82);
> >+
> >+    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> >+    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
> >+              &int80_direct_trap);
> >+
> >+    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
> >+}
> 
> Any reason you don't move this into pv/traps.c right away?

Because nmi_mce_softirq needs to be moved at the same time. That can't
be undone until I refactor the code further in later patch(es).

> 
> >void __init trap_init(void)
>  >{
>      >unsigned int vector;
>  >
> >+    pv_trap_init();
> 
> I think this would better be called after ...
> 
> >/* Replace early pagefault with real pagefault handler. */
>      >set_intr_gate(TRAP_page_fault, &page_fault);
>  
> ... this, i.e. in line with where most of the code was before.
> 

NP.

> Jan
> 

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

Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only

[Wei Liu]

On Tue, Jun 27, 2017 at 12:31:20PM -0600, Jan Beulich wrote:
> >>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
> >Since ARM doesn't need do_nmi_op, move the hypercall handler from
> >common/kernel.c to pv/callback.c.
> 
> There are two handlers you actually move, and while their neither large nor
> likely to change, I still somewhat dislike the code duplication you introduce.
> But I guess not enough to put under question the R-b you've already got
> from Andrew.
> 

I'm with Andrew here. I deliberately unrolled all macros because the
resulting code is cleaner.

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

Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Wei Liu]

On Wed, Jun 28, 2017 at 11:06:58AM +0100, Wei Liu wrote:
> On Tue, Jun 27, 2017 at 12:26:52PM -0600, Jan Beulich wrote:
> > >>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
> > >--- a/xen/arch/x86/traps.c
> > >+++ b/xen/arch/x86/traps.c
> > >@@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
> >      >this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
> >  >}
> >  >
> > >+void __init pv_trap_init(void)
> > >+{
> > >+    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
> > >+    _set_gate(idt_table + HYPERCALL_VECTOR,
> > >+              SYS_DESC_trap_gate, 1, entry_int82);
> > >+
> > >+    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> > >+    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
> > >+              &int80_direct_trap);
> > >+
> > >+    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
> > >+}
> > 
> > Any reason you don't move this into pv/traps.c right away?
> 
> Because nmi_mce_softirq needs to be moved at the same time. That can't
> be undone until I refactor the code further in later patch(es).

undone -> done

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

Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c

[Wei Liu]

On Tue, Jun 27, 2017 at 06:57:52PM +0100, Andrew Cooper wrote:
> On 27/06/17 09:48, Wei Liu wrote:
> > On Tue, Jun 27, 2017 at 12:13:19AM -0600, Jan Beulich wrote:
> >>>>> Wei Liu <wei.liu2@citrix.com> 06/26/17 6:29 PM >>>
> >>> --- a/xen/arch/x86/pv/Makefile
> >>> +++ b/xen/arch/x86/pv/Makefile
> >>> @@ -1,6 +1,7 @@
> >>  >obj-y += hypercall.o
> >>  >obj-y += traps.o
> >>  >
> >>> +obj-y += callback.o
> >>  >obj-bin-y += dom0_build.init.o
> >>  >obj-y += domain.o
> >>  >obj-y += emulate.o
> >>
> >> Not something to be dealt with in this patch, but - is there a reason we
> >> have two groups of object files here? I see none, and hence would have
> >> expected this to be a single sorted list instead of two.
> >>
> > No. It just so happened I added a newline at some point.
> >
> > I will submit a patch to fix this at some point.
> 
> I'd suggest splitting the obj-y and obj-bin-y lists, as it will make the
> Makefile easier to read.  That appears to have been the original
> intention behind the space in the first place.
> 

That's a good suggestion.

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

Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init

[Andrew Cooper]

On 27/06/17 19:19, Jan Beulich wrote:
>>>> Andrew Cooper <andrew.cooper3@citrix.com> 06/27/17 8:06 PM >>>
>> On 26/06/17 17:28, Wei Liu wrote:
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -1934,21 +1934,29 @@ void __init init_idt_traps(void)
>>>      this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
>>>  }
>>>  
>>> +void __init pv_trap_init(void)
>>> +{
>>> +    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
>>> +    _set_gate(idt_table + HYPERCALL_VECTOR,
>> &idt_table[HYPERCALL_VECTOR]
> I don't think we should require this. Personally I prefer the form Wei used,
> and iirc there's nothing in our coding style guidelines that mandates the
> form you suggest.

One form is obviously an array access, while one is not obvious.

I'm not overly fussed, but I would prefer the former.

~Andrew

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