* [PATCH v5 01/13] x86: move callback_op code to pv/callback.c
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-26 16:44 ` Andrew Cooper
2017-06-27 6:13 ` Jan Beulich
2017-06-26 16:28 ` [PATCH v5 02/13] x86: move the compat callback ops next to the non-compat variant Wei Liu
` (11 subsequent siblings)
12 siblings, 2 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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(®, arg, 1) )
+ break;
+
+ ret = register_guest_callback(®);
+ }
+ 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(®, arg, 1) )
- break;
-
- ret = register_guest_callback(®);
- }
- 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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c
2017-06-26 16:28 ` [PATCH v5 01/13] x86: move callback_op code to pv/callback.c Wei Liu
@ 2017-06-26 16:44 ` Andrew Cooper
2017-06-27 6:13 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-26 16:44 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c
2017-06-26 16:28 ` [PATCH v5 01/13] x86: move callback_op code to pv/callback.c Wei Liu
2017-06-26 16:44 ` Andrew Cooper
@ 2017-06-27 6:13 ` Jan Beulich
2017-06-27 8:48 ` Wei Liu
1 sibling, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 6:13 UTC (permalink / raw)
To: wei.liu2; +Cc: andrew.cooper3, xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c
2017-06-27 6:13 ` Jan Beulich
@ 2017-06-27 8:48 ` Wei Liu
2017-06-27 17:57 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-27 8:48 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c
2017-06-27 8:48 ` Wei Liu
@ 2017-06-27 17:57 ` Andrew Cooper
2017-06-28 10:14 ` Wei Liu
0 siblings, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 17:57 UTC (permalink / raw)
To: Wei Liu, Jan Beulich; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 01/13] x86: move callback_op code to pv/callback.c
2017-06-27 17:57 ` Andrew Cooper
@ 2017-06-28 10:14 ` Wei Liu
0 siblings, 0 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-28 10:14 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu, Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v5 02/13] x86: move the compat callback ops next to the non-compat variant
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
2017-06-26 16:28 ` [PATCH v5 01/13] x86: move callback_op code to pv/callback.c Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-26 16:51 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 03/13] x86: move do_set_trap_table to pv/callback.c Wei Liu
` (10 subsequent siblings)
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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(®, arg, 1) )
+ break;
+
+ ret = compat_register_guest_callback(®);
+ }
+ 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(®, arg, 1) )
- break;
-
- ret = compat_register_guest_callback(®);
- }
- 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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v5 03/13] x86: move do_set_trap_table to pv/callback.c
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
2017-06-26 16:28 ` [PATCH v5 01/13] x86: move callback_op code to pv/callback.c Wei Liu
2017-06-26 16:28 ` [PATCH v5 02/13] x86: move the compat callback ops next to the non-compat variant Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-26 16:53 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 04/13] x86: move compat_set_trap_table along side the non-compat variant Wei Liu
` (9 subsequent siblings)
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v5 04/13] x86: move compat_set_trap_table along side the non-compat variant
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (2 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 03/13] x86: move do_set_trap_table to pv/callback.c Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-26 16:53 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 05/13] x86: remove the now empty x86_64/compat/traps.c Wei Liu
` (8 subsequent siblings)
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v5 05/13] x86: remove the now empty x86_64/compat/traps.c
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (3 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 04/13] x86: move compat_set_trap_table along side the non-compat variant Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-26 16:54 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 06/13] x86: simplify guest_has_trap_callback Wei Liu
` (7 subsequent siblings)
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v5 06/13] x86: simplify guest_has_trap_callback
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (4 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 05/13] x86: remove the now empty x86_64/compat/traps.c Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-26 16:57 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap Wei Liu
` (6 subsequent siblings)
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v5 06/13] x86: simplify guest_has_trap_callback
2017-06-26 16:28 ` [PATCH v5 06/13] x86: simplify guest_has_trap_callback Wei Liu
@ 2017-06-26 16:57 ` Andrew Cooper
2017-06-27 6:09 ` Jan Beulich
0 siblings, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2017-06-26 16:57 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 06/13] x86: simplify guest_has_trap_callback
2017-06-26 16:57 ` Andrew Cooper
@ 2017-06-27 6:09 ` Jan Beulich
2017-06-27 9:02 ` Wei Liu
0 siblings, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 6:09 UTC (permalink / raw)
To: andrew.cooper3, wei.liu2; +Cc: xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 06/13] x86: simplify guest_has_trap_callback
2017-06-27 6:09 ` Jan Beulich
@ 2017-06-27 9:02 ` Wei Liu
0 siblings, 0 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-27 9:02 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
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
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (5 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 06/13] x86: simplify guest_has_trap_callback Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:01 ` Andrew Cooper
2017-06-27 18:21 ` Jan Beulich
2017-06-26 16:28 ` [PATCH v5 08/13] x86/traps: factor out pv_trap_init Wei Liu
` (5 subsequent siblings)
12 siblings, 2 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap
2017-06-26 16:28 ` [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap Wei Liu
@ 2017-06-27 18:01 ` Andrew Cooper
2017-06-27 18:21 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 18:01 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap
2017-06-26 16:28 ` [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap Wei Liu
2017-06-27 18:01 ` Andrew Cooper
@ 2017-06-27 18:21 ` Jan Beulich
2017-06-27 18:28 ` Andrew Cooper
1 sibling, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 18:21 UTC (permalink / raw)
To: wei.liu2; +Cc: andrew.cooper3, xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap
2017-06-27 18:21 ` Jan Beulich
@ 2017-06-27 18:28 ` Andrew Cooper
0 siblings, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 18:28 UTC (permalink / raw)
To: Jan Beulich, wei.liu2; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (6 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 07/13] x86/traps: simplify and rename send_guest_trap Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:05 ` Andrew Cooper
2017-06-27 18:26 ` Jan Beulich
2017-06-26 16:28 ` [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only Wei Liu
` (4 subsequent siblings)
12 siblings, 2 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-26 16:28 ` [PATCH v5 08/13] x86/traps: factor out pv_trap_init Wei Liu
@ 2017-06-27 18:05 ` Andrew Cooper
2017-06-27 18:19 ` Jan Beulich
2017-06-27 18:26 ` Jan Beulich
1 sibling, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 18:05 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-27 18:05 ` Andrew Cooper
@ 2017-06-27 18:19 ` Jan Beulich
2017-06-28 11:43 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 18:19 UTC (permalink / raw)
To: andrew.cooper3, wei.liu2; +Cc: xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-27 18:19 ` Jan Beulich
@ 2017-06-28 11:43 ` Andrew Cooper
0 siblings, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-28 11:43 UTC (permalink / raw)
To: Jan Beulich, wei.liu2; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-26 16:28 ` [PATCH v5 08/13] x86/traps: factor out pv_trap_init Wei Liu
2017-06-27 18:05 ` Andrew Cooper
@ 2017-06-27 18:26 ` Jan Beulich
2017-06-28 10:06 ` Wei Liu
1 sibling, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 18:26 UTC (permalink / raw)
To: wei.liu2; +Cc: andrew.cooper3, xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-27 18:26 ` Jan Beulich
@ 2017-06-28 10:06 ` Wei Liu
2017-06-28 10:13 ` Wei Liu
0 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-28 10:06 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 08/13] x86/traps: factor out pv_trap_init
2017-06-28 10:06 ` Wei Liu
@ 2017-06-28 10:13 ` Wei Liu
0 siblings, 0 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-28 10:13 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (7 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 08/13] x86/traps: factor out pv_trap_init Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:08 ` Andrew Cooper
` (2 more replies)
2017-06-26 16:28 ` [PATCH v5 10/13] x86/traps: move {un, }register_guest_nmi_callback to pv/callback.c Wei Liu
` (3 subsequent siblings)
12 siblings, 3 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Wei Liu,
Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only
2017-06-26 16:28 ` [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only Wei Liu
@ 2017-06-27 18:08 ` Andrew Cooper
2017-06-27 18:31 ` Jan Beulich
2017-06-27 20:34 ` Stefano Stabellini
2 siblings, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 18:08 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only
2017-06-26 16:28 ` [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only Wei Liu
2017-06-27 18:08 ` Andrew Cooper
@ 2017-06-27 18:31 ` Jan Beulich
2017-06-27 18:50 ` Andrew Cooper
2017-06-28 10:12 ` Wei Liu
2017-06-27 20:34 ` Stefano Stabellini
2 siblings, 2 replies; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 18:31 UTC (permalink / raw)
To: wei.liu2; +Cc: andrew.cooper3, julien.grall, sstabellini, xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only
2017-06-27 18:31 ` Jan Beulich
@ 2017-06-27 18:50 ` Andrew Cooper
2017-06-28 10:12 ` Wei Liu
1 sibling, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 18:50 UTC (permalink / raw)
To: Jan Beulich, wei.liu2; +Cc: xen-devel, julien.grall, sstabellini
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only
2017-06-27 18:31 ` Jan Beulich
2017-06-27 18:50 ` Andrew Cooper
@ 2017-06-28 10:12 ` Wei Liu
1 sibling, 0 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-28 10:12 UTC (permalink / raw)
To: Jan Beulich
Cc: andrew.cooper3, julien.grall, sstabellini, wei.liu2, xen-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only
2017-06-26 16:28 ` [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only Wei Liu
2017-06-27 18:08 ` Andrew Cooper
2017-06-27 18:31 ` Jan Beulich
@ 2017-06-27 20:34 ` Stefano Stabellini
2 siblings, 0 replies; 45+ messages in thread
From: Stefano Stabellini @ 2017-06-27 20:34 UTC (permalink / raw)
To: Wei Liu
Cc: Xen-devel, Julien Grall, Stefano Stabellini, Jan Beulich,
Andrew Cooper
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v5 10/13] x86/traps: move {un, }register_guest_nmi_callback to pv/callback.c
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (8 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 09/13] xen: move do_nmi_op and make it x86 only Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:09 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 11/13] x86/callback.c: slightly change {un, }register_guest_nmi_callback Wei Liu
` (2 subsequent siblings)
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v5 11/13] x86/callback.c: slightly change {un, }register_guest_nmi_callback
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (9 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 10/13] x86/traps: move {un, }register_guest_nmi_callback to pv/callback.c Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:10 ` Andrew Cooper
2017-06-26 16:28 ` [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c Wei Liu
2017-06-26 16:28 ` [PATCH v5 13/13] x86/traps.h: remove unused declaration of cpu_user_regs Wei Liu
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (10 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 11/13] x86/callback.c: slightly change {un, }register_guest_nmi_callback Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:12 ` Andrew Cooper
2017-06-27 18:34 ` Jan Beulich
2017-06-26 16:28 ` [PATCH v5 13/13] x86/traps.h: remove unused declaration of cpu_user_regs Wei Liu
12 siblings, 2 replies; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c
2017-06-26 16:28 ` [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c Wei Liu
@ 2017-06-27 18:12 ` Andrew Cooper
2017-06-27 18:34 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2017-06-27 18:12 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Jan Beulich
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c
2017-06-26 16:28 ` [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c Wei Liu
2017-06-27 18:12 ` Andrew Cooper
@ 2017-06-27 18:34 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Jan Beulich @ 2017-06-27 18:34 UTC (permalink / raw)
To: wei.liu2; +Cc: andrew.cooper3, xen-devel
>>> 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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v5 13/13] x86/traps.h: remove unused declaration of cpu_user_regs
2017-06-26 16:28 [PATCH v5 00/13] x86: refactor trap handling code Wei Liu
` (11 preceding siblings ...)
2017-06-26 16:28 ` [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c Wei Liu
@ 2017-06-26 16:28 ` Wei Liu
2017-06-27 18:13 ` Andrew Cooper
12 siblings, 1 reply; 45+ messages in thread
From: Wei Liu @ 2017-06-26 16:28 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 45+ messages in thread