[PATCH V2 0/2] Fix vm_event resume path race condition

[PATCH V2 0/2] Fix vm_event resume path race condition

[Razvan Cojocaru]

This small series first creates hvm/vm_event.{h,c}, in order to bring
under vm_event maintainership the code that has previously lived in
hvm_do_resume(), and then fixes a __context_switch()-related race
condition when attempting to set registers via vm_event reply.

Previously this has been a single patch, "x86/vm_event: fix race
between vmx_vmexit_handler() and vm_event_resume()".

[PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h,c}
[PATCH V2 2/2] x86/vm_event: fix race between __context_switch() and


Thanks,
Razvan

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

[PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h,c}

[Razvan Cojocaru]

Created arch/x86/hvm/vm_event.c and include/asm-x86/hvm/vm_event.h,
where HVM-specific vm_event-related code will live. This cleans up
hvm_do_resume() and ensures that the vm_event maintainers are
responsible for changes to that code.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 MAINTAINERS                        |   2 +
 xen/arch/x86/hvm/Makefile          |   1 +
 xen/arch/x86/hvm/hvm.c             |  64 +----------------------
 xen/arch/x86/hvm/vm_event.c        | 101 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/vm_event.h |  34 +++++++++++++
 5 files changed, 140 insertions(+), 62 deletions(-)
 create mode 100644 xen/arch/x86/hvm/vm_event.c
 create mode 100644 xen/include/asm-x86/hvm/vm_event.h

diff --git a/MAINTAINERS b/MAINTAINERS
index c345a50..10863bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -404,6 +404,7 @@ S:	Supported
 F:	tools/tests/xen-access
 F:	xen/arch/*/monitor.c
 F:	xen/arch/*/vm_event.c
+F:	xen/arch/*/hvm/vm_event.c
 F:	xen/arch/arm/mem_access.c
 F:	xen/arch/x86/mm/mem_access.c
 F:	xen/arch/x86/hvm/monitor.c
@@ -413,6 +414,7 @@ F:	xen/common/vm_event.c
 F:	xen/include/*/mem_access.h
 F:	xen/include/*/monitor.h
 F:	xen/include/*/vm_event.h
+F:	xen/include/*/hvm/vm_event.h
 F:	xen/include/asm-x86/hvm/monitor.h
 
 VTPM
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 0a3d0f4..02f0add 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -24,6 +24,7 @@ obj-y += stdvga.o
 obj-y += vioapic.o
 obj-y += viridian.o
 obj-y += vlapic.o
+obj-y += vm_event.o
 obj-y += vmsi.o
 obj-y += vpic.o
 obj-y += vpt.o
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a441955..f010394 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -34,7 +34,6 @@
 #include <xen/wait.h>
 #include <xen/mem_access.h>
 #include <xen/rangeset.h>
-#include <xen/vm_event.h>
 #include <xen/monitor.h>
 #include <xen/warning.h>
 #include <asm/shadow.h>
@@ -61,6 +60,7 @@
 #include <asm/hvm/nestedhvm.h>
 #include <asm/hvm/monitor.h>
 #include <asm/hvm/ioreq.h>
+#include <asm/hvm/vm_event.h>
 #include <asm/altp2m.h>
 #include <asm/mtrr.h>
 #include <asm/apic.h>
@@ -483,67 +483,7 @@ void hvm_do_resume(struct vcpu *v)
     if ( !handle_hvm_io_completion(v) )
         return;
 
-    if ( unlikely(v->arch.vm_event) )
-    {
-        struct monitor_write_data *w = &v->arch.vm_event->write_data;
-
-        if ( unlikely(v->arch.vm_event->emulate_flags) )
-        {
-            enum emul_kind kind = EMUL_KIND_NORMAL;
-
-            /*
-             * Please observ the order here to match the flag descriptions
-             * provided in public/vm_event.h
-             */
-            if ( v->arch.vm_event->emulate_flags &
-                 VM_EVENT_FLAG_SET_EMUL_READ_DATA )
-                kind = EMUL_KIND_SET_CONTEXT_DATA;
-            else if ( v->arch.vm_event->emulate_flags &
-                      VM_EVENT_FLAG_EMULATE_NOWRITE )
-                kind = EMUL_KIND_NOWRITE;
-            else if ( v->arch.vm_event->emulate_flags &
-                      VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
-                kind = EMUL_KIND_SET_CONTEXT_INSN;
-
-            hvm_emulate_one_vm_event(kind, TRAP_invalid_op,
-                                     X86_EVENT_NO_EC);
-
-            v->arch.vm_event->emulate_flags = 0;
-        }
-
-        if ( w->do_write.msr )
-        {
-            if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
-                 X86EMUL_EXCEPTION )
-                hvm_inject_hw_exception(TRAP_gp_fault, 0);
-
-            w->do_write.msr = 0;
-        }
-
-        if ( w->do_write.cr0 )
-        {
-            if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION )
-                hvm_inject_hw_exception(TRAP_gp_fault, 0);
-
-            w->do_write.cr0 = 0;
-        }
-
-        if ( w->do_write.cr4 )
-        {
-            if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION )
-                hvm_inject_hw_exception(TRAP_gp_fault, 0);
-
-            w->do_write.cr4 = 0;
-        }
-
-        if ( w->do_write.cr3 )
-        {
-            if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION )
-                hvm_inject_hw_exception(TRAP_gp_fault, 0);
-
-            w->do_write.cr3 = 0;
-        }
-    }
+    hvm_vm_event_do_resume(v);
 
     /* Inject pending hw/sw event */
     if ( v->arch.hvm_vcpu.inject_event.vector >= 0 )
diff --git a/xen/arch/x86/hvm/vm_event.c b/xen/arch/x86/hvm/vm_event.c
new file mode 100644
index 0000000..b35ee12
--- /dev/null
+++ b/xen/arch/x86/hvm/vm_event.c
@@ -0,0 +1,101 @@
+/*
+ * arch/x86/hvm/vm_event.c
+ *
+ * HVM vm_event handling routines
+ *
+ * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/sched.h>
+#include <xen/vm_event.h>
+#include <asm/hvm/support.h>
+#include <asm/vm_event.h>
+
+void hvm_vm_event_do_resume(struct vcpu *v)
+{
+    struct monitor_write_data *w;
+
+    if ( likely(!v->arch.vm_event) )
+        return;
+
+    w = &v->arch.vm_event->write_data;
+
+    if ( unlikely(v->arch.vm_event->emulate_flags) )
+    {
+        enum emul_kind kind = EMUL_KIND_NORMAL;
+
+        /*
+         * Please observe the order here to match the flag descriptions
+         * provided in public/vm_event.h
+         */
+        if ( v->arch.vm_event->emulate_flags &
+             VM_EVENT_FLAG_SET_EMUL_READ_DATA )
+            kind = EMUL_KIND_SET_CONTEXT_DATA;
+        else if ( v->arch.vm_event->emulate_flags &
+                  VM_EVENT_FLAG_EMULATE_NOWRITE )
+            kind = EMUL_KIND_NOWRITE;
+        else if ( v->arch.vm_event->emulate_flags &
+                  VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
+            kind = EMUL_KIND_SET_CONTEXT_INSN;
+
+        hvm_emulate_one_vm_event(kind, TRAP_invalid_op,
+                                 X86_EVENT_NO_EC);
+
+        v->arch.vm_event->emulate_flags = 0;
+    }
+
+    if ( w->do_write.cr0 )
+    {
+        if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION )
+            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+
+        w->do_write.cr0 = 0;
+    }
+
+    if ( w->do_write.cr4 )
+    {
+        if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION )
+            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+
+        w->do_write.cr4 = 0;
+    }
+
+    if ( w->do_write.cr3 )
+    {
+        if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION )
+            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+
+        w->do_write.cr3 = 0;
+    }
+
+    if ( w->do_write.msr )
+    {
+        if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
+             X86EMUL_EXCEPTION )
+            hvm_inject_hw_exception(TRAP_gp_fault, 0);
+
+        w->do_write.msr = 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/include/asm-x86/hvm/vm_event.h b/xen/include/asm-x86/hvm/vm_event.h
new file mode 100644
index 0000000..515fa85
--- /dev/null
+++ b/xen/include/asm-x86/hvm/vm_event.h
@@ -0,0 +1,34 @@
+/*
+ * include/asm-x86/hvm/vm_event.h
+ *
+ * Hardware virtual machine vm_event abstractions.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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/>.
+ */
+
+#ifndef __ASM_X86_HVM_VM_EVENT_H__
+#define __ASM_X86_HVM_VM_EVENT_H__
+
+void hvm_vm_event_do_resume(struct vcpu *v);
+
+#endif /* __ASM_X86_HVM_MONITOR_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1


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

[PATCH V2 2/2] x86/vm_event: fix race between __context_switch() and vm_event_resume()

[Razvan Cojocaru]

The introspection agent can reply to a vm_event faster than
vmx_vmexit_handler() can complete in some cases, where it is then
not safe for vm_event_set_registers() to modify v->arch.user_regs.
In the test scenario, we were stepping over an INT3 breakpoint by
setting RIP += 1. The quick reply tended to complete before the VCPU
triggering the introspection event had properly paused and been
descheduled. If the reply occurs before __context_switch() happens,
__context_switch() clobbers the reply by overwriting
v->arch.user_regs from the stack. If the reply occurs after
__context_switch(), we don't pass through __context_switch() when
transitioning to idle.

This patch ensures that vm_event_resume() code only sets per-VCPU
data to be used for the actual setting of registers later in
hvm_do_resume() (similar to the model used to control setting of CRs
and MSRs).

The patch additionally removes the sync_vcpu_execstate(v) call from
vm_event_resume(), which is no longer necessary, which removes the
associated broadcast TLB flush (read: performance improvement).

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/vm_event.c    | 35 +++++++++++++++++++++++++++++++++++
 xen/arch/x86/vm_event.c        | 22 ++--------------------
 xen/common/vm_event.c          | 17 ++++++++++-------
 xen/include/asm-x86/vm_event.h |  2 ++
 4 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/hvm/vm_event.c b/xen/arch/x86/hvm/vm_event.c
index b35ee12..bfb95a2 100644
--- a/xen/arch/x86/hvm/vm_event.c
+++ b/xen/arch/x86/hvm/vm_event.c
@@ -23,6 +23,39 @@
 #include <asm/hvm/support.h>
 #include <asm/vm_event.h>
 
+static void hvm_vm_event_set_registers(const struct vcpu *v)
+{
+    ASSERT(v == current);
+
+    if ( unlikely(v->arch.vm_event->set_gprs) )
+    {
+        struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+        regs->rax = v->arch.vm_event->gprs.rax;
+        regs->rbx = v->arch.vm_event->gprs.rbx;
+        regs->rcx = v->arch.vm_event->gprs.rcx;
+        regs->rdx = v->arch.vm_event->gprs.rdx;
+        regs->rsp = v->arch.vm_event->gprs.rsp;
+        regs->rbp = v->arch.vm_event->gprs.rbp;
+        regs->rsi = v->arch.vm_event->gprs.rsi;
+        regs->rdi = v->arch.vm_event->gprs.rdi;
+
+        regs->r8 = v->arch.vm_event->gprs.r8;
+        regs->r9 = v->arch.vm_event->gprs.r9;
+        regs->r10 = v->arch.vm_event->gprs.r10;
+        regs->r11 = v->arch.vm_event->gprs.r11;
+        regs->r12 = v->arch.vm_event->gprs.r12;
+        regs->r13 = v->arch.vm_event->gprs.r13;
+        regs->r14 = v->arch.vm_event->gprs.r14;
+        regs->r15 = v->arch.vm_event->gprs.r15;
+
+        regs->rflags = v->arch.vm_event->gprs.rflags;
+        regs->rip = v->arch.vm_event->gprs.rip;
+
+        v->arch.vm_event->set_gprs = false;
+    }
+}
+
 void hvm_vm_event_do_resume(struct vcpu *v)
 {
     struct monitor_write_data *w;
@@ -30,6 +63,8 @@ void hvm_vm_event_do_resume(struct vcpu *v)
     if ( likely(!v->arch.vm_event) )
         return;
 
+    hvm_vm_event_set_registers(v);
+
     w = &v->arch.vm_event->write_data;
 
     if ( unlikely(v->arch.vm_event->emulate_flags) )
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 502ccc2..a6ea42c 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -113,26 +113,8 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
 {
     ASSERT(atomic_read(&v->vm_event_pause_count));
 
-    v->arch.user_regs.rax = rsp->data.regs.x86.rax;
-    v->arch.user_regs.rbx = rsp->data.regs.x86.rbx;
-    v->arch.user_regs.rcx = rsp->data.regs.x86.rcx;
-    v->arch.user_regs.rdx = rsp->data.regs.x86.rdx;
-    v->arch.user_regs.rsp = rsp->data.regs.x86.rsp;
-    v->arch.user_regs.rbp = rsp->data.regs.x86.rbp;
-    v->arch.user_regs.rsi = rsp->data.regs.x86.rsi;
-    v->arch.user_regs.rdi = rsp->data.regs.x86.rdi;
-
-    v->arch.user_regs.r8 = rsp->data.regs.x86.r8;
-    v->arch.user_regs.r9 = rsp->data.regs.x86.r9;
-    v->arch.user_regs.r10 = rsp->data.regs.x86.r10;
-    v->arch.user_regs.r11 = rsp->data.regs.x86.r11;
-    v->arch.user_regs.r12 = rsp->data.regs.x86.r12;
-    v->arch.user_regs.r13 = rsp->data.regs.x86.r13;
-    v->arch.user_regs.r14 = rsp->data.regs.x86.r14;
-    v->arch.user_regs.r15 = rsp->data.regs.x86.r15;
-
-    v->arch.user_regs.rflags = rsp->data.regs.x86.rflags;
-    v->arch.user_regs.rip = rsp->data.regs.x86.rip;
+    v->arch.vm_event->gprs = rsp->data.regs.x86;
+    v->arch.vm_event->set_gprs = true;
 }
 
 void vm_event_monitor_next_interrupt(struct vcpu *v)
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 0fe9a53..9291db6 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -357,6 +357,16 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
 {
     vm_event_response_t rsp;
 
+    /*
+     * vm_event_resume() runs in either XEN_DOMCTL_VM_EVENT_OP_*, or
+     * EVTCHN_send context from the introspection consumer. Both contexts
+     * are guaranteed not to be the subject of vm_event responses.
+     * While we could ASSERT(v != current) for each VCPU in d in the loop
+     * below, this covers the case where we would need to iterate over all
+     * of them more succintly.
+     */
+    ASSERT(d != current->domain);
+
     /* Pull all responses off the ring. */
     while ( vm_event_get_response(d, ved, &rsp) )
     {
@@ -375,13 +385,6 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
         v = d->vcpu[rsp.vcpu_id];
 
         /*
-         * Make sure the vCPU state has been synchronized for the custom
-         * handlers.
-         */
-        if ( atomic_read(&v->vm_event_pause_count) )
-            sync_vcpu_execstate(v);
-
-        /*
          * In some cases the response type needs extra handling, so here
          * we call the appropriate handlers.
          */
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index ca73f99..39e73c8 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -32,6 +32,8 @@ struct arch_vm_event {
         struct vm_event_emul_insn_data insn;
     } emul;
     struct monitor_write_data write_data;
+    struct vm_event_regs_x86 gprs;
+    bool set_gprs;
 };
 
 int vm_event_init_domain(struct domain *d);
-- 
1.9.1


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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Jan Beulich]

>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote:
> @@ -483,67 +483,7 @@ void hvm_do_resume(struct vcpu *v)
>      if ( !handle_hvm_io_completion(v) )
>          return;
>  
> -    if ( unlikely(v->arch.vm_event) )
> -    {
> -        struct monitor_write_data *w = &v->arch.vm_event->write_data;
> -
> -        if ( unlikely(v->arch.vm_event->emulate_flags) )
> -        {
> -            enum emul_kind kind = EMUL_KIND_NORMAL;
> -
> -            /*
> -             * Please observ the order here to match the flag descriptions
> -             * provided in public/vm_event.h
> -             */
> -            if ( v->arch.vm_event->emulate_flags &
> -                 VM_EVENT_FLAG_SET_EMUL_READ_DATA )
> -                kind = EMUL_KIND_SET_CONTEXT_DATA;
> -            else if ( v->arch.vm_event->emulate_flags &
> -                      VM_EVENT_FLAG_EMULATE_NOWRITE )
> -                kind = EMUL_KIND_NOWRITE;
> -            else if ( v->arch.vm_event->emulate_flags &
> -                      VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
> -                kind = EMUL_KIND_SET_CONTEXT_INSN;
> -
> -            hvm_emulate_one_vm_event(kind, TRAP_invalid_op,
> -                                     X86_EVENT_NO_EC);
> -
> -            v->arch.vm_event->emulate_flags = 0;
> -        }
> -
> -        if ( w->do_write.msr )
> -        {
> -            if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
> -                 X86EMUL_EXCEPTION )
> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
> -
> -            w->do_write.msr = 0;
> -        }
> -
> -        if ( w->do_write.cr0 )
> -        {
> -            if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION )
> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
> -
> -            w->do_write.cr0 = 0;
> -        }
> -
> -        if ( w->do_write.cr4 )
> -        {
> -            if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION )
> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
> -
> -            w->do_write.cr4 = 0;
> -        }
> -
> -        if ( w->do_write.cr3 )
> -        {
> -            if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION )
> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
> -
> -            w->do_write.cr3 = 0;
> -        }
> -    }
> +    hvm_vm_event_do_resume(v);

As indicated before, I think we want to keep

    if ( unlikely(v->arch.vm_event) )

here of in an inline wrapper, to avoid the actual function call in the
common case.

> --- /dev/null
> +++ b/xen/arch/x86/hvm/vm_event.c
> @@ -0,0 +1,101 @@
> +/*
> + * arch/x86/hvm/vm_event.c
> + *
> + * HVM vm_event handling routines
> + *
> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com)

I'm notoriously bad when it comes to copyrights, but you just
moving code makes me wonder whether this is appropriate.

> +void hvm_vm_event_do_resume(struct vcpu *v)
> +{
> +    struct monitor_write_data *w;
> +
> +    if ( likely(!v->arch.vm_event) )
> +        return;
> +
> +    w = &v->arch.vm_event->write_data;
> +
> +    if ( unlikely(v->arch.vm_event->emulate_flags) )
> +    {
> +        enum emul_kind kind = EMUL_KIND_NORMAL;
> +
> +        /*
> +         * Please observe the order here to match the flag descriptions
> +         * provided in public/vm_event.h
> +         */
> +        if ( v->arch.vm_event->emulate_flags &
> +             VM_EVENT_FLAG_SET_EMUL_READ_DATA )
> +            kind = EMUL_KIND_SET_CONTEXT_DATA;
> +        else if ( v->arch.vm_event->emulate_flags &
> +                  VM_EVENT_FLAG_EMULATE_NOWRITE )
> +            kind = EMUL_KIND_NOWRITE;
> +        else if ( v->arch.vm_event->emulate_flags &
> +                  VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
> +            kind = EMUL_KIND_SET_CONTEXT_INSN;
> +
> +        hvm_emulate_one_vm_event(kind, TRAP_invalid_op,
> +                                 X86_EVENT_NO_EC);
> +
> +        v->arch.vm_event->emulate_flags = 0;
> +    }
> +
> +    if ( w->do_write.cr0 )
> +    {
> +        if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION )
> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +        w->do_write.cr0 = 0;
> +    }
> +
> +    if ( w->do_write.cr4 )
> +    {
> +        if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION )
> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +        w->do_write.cr4 = 0;
> +    }
> +
> +    if ( w->do_write.cr3 )
> +    {
> +        if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION )
> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +        w->do_write.cr3 = 0;
> +    }
> +
> +    if ( w->do_write.msr )
> +    {
> +        if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
> +             X86EMUL_EXCEPTION )
> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +
> +        w->do_write.msr = 0;
> +    }

I wonder whether all of these outer if()-s wouldn't better have
unlikely() too.

Jan

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

Re: [PATCH V2 2/2] x86/vm_event: fix race between __context_switch() and vm_event_resume()

[Jan Beulich]

>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote:
> The introspection agent can reply to a vm_event faster than
> vmx_vmexit_handler() can complete in some cases, where it is then
> not safe for vm_event_set_registers() to modify v->arch.user_regs.
> In the test scenario, we were stepping over an INT3 breakpoint by
> setting RIP += 1. The quick reply tended to complete before the VCPU
> triggering the introspection event had properly paused and been
> descheduled. If the reply occurs before __context_switch() happens,
> __context_switch() clobbers the reply by overwriting
> v->arch.user_regs from the stack. If the reply occurs after
> __context_switch(), we don't pass through __context_switch() when
> transitioning to idle.

This last sentence still looks to be wrong (and even self-contradictory).
The reply can't occur after __context_switch() if we don't make it there
in the first place. How about "If we don't pass through
__context_switch() (due to switching to the idle vCPU), reply data
wouldn't be picked up when switching back straight to the original
vCPU"?

> This patch ensures that vm_event_resume() code only sets per-VCPU
> data to be used for the actual setting of registers later in
> hvm_do_resume() (similar to the model used to control setting of CRs
> and MSRs).
> 
> The patch additionally removes the sync_vcpu_execstate(v) call from
> vm_event_resume(), which is no longer necessary, which removes the
> associated broadcast TLB flush (read: performance improvement).
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Code changes themselves
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Razvan Cojocaru]

On 05/03/17 12:51, Jan Beulich wrote:
>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote:
>> @@ -483,67 +483,7 @@ void hvm_do_resume(struct vcpu *v)
>>      if ( !handle_hvm_io_completion(v) )
>>          return;
>>  
>> -    if ( unlikely(v->arch.vm_event) )
>> -    {
>> -        struct monitor_write_data *w = &v->arch.vm_event->write_data;
>> -
>> -        if ( unlikely(v->arch.vm_event->emulate_flags) )
>> -        {
>> -            enum emul_kind kind = EMUL_KIND_NORMAL;
>> -
>> -            /*
>> -             * Please observ the order here to match the flag descriptions
>> -             * provided in public/vm_event.h
>> -             */
>> -            if ( v->arch.vm_event->emulate_flags &
>> -                 VM_EVENT_FLAG_SET_EMUL_READ_DATA )
>> -                kind = EMUL_KIND_SET_CONTEXT_DATA;
>> -            else if ( v->arch.vm_event->emulate_flags &
>> -                      VM_EVENT_FLAG_EMULATE_NOWRITE )
>> -                kind = EMUL_KIND_NOWRITE;
>> -            else if ( v->arch.vm_event->emulate_flags &
>> -                      VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
>> -                kind = EMUL_KIND_SET_CONTEXT_INSN;
>> -
>> -            hvm_emulate_one_vm_event(kind, TRAP_invalid_op,
>> -                                     X86_EVENT_NO_EC);
>> -
>> -            v->arch.vm_event->emulate_flags = 0;
>> -        }
>> -
>> -        if ( w->do_write.msr )
>> -        {
>> -            if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
>> -                 X86EMUL_EXCEPTION )
>> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> -
>> -            w->do_write.msr = 0;
>> -        }
>> -
>> -        if ( w->do_write.cr0 )
>> -        {
>> -            if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION )
>> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> -
>> -            w->do_write.cr0 = 0;
>> -        }
>> -
>> -        if ( w->do_write.cr4 )
>> -        {
>> -            if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION )
>> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> -
>> -            w->do_write.cr4 = 0;
>> -        }
>> -
>> -        if ( w->do_write.cr3 )
>> -        {
>> -            if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION )
>> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> -
>> -            w->do_write.cr3 = 0;
>> -        }
>> -    }
>> +    hvm_vm_event_do_resume(v);
> 
> As indicated before, I think we want to keep
> 
>     if ( unlikely(v->arch.vm_event) )
> 
> here of in an inline wrapper, to avoid the actual function call in the
> common case.

Will do.

>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/vm_event.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * arch/x86/hvm/vm_event.c
>> + *
>> + * HVM vm_event handling routines
>> + *
>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com)
> 
> I'm notoriously bad when it comes to copyrights, but you just
> moving code makes me wonder whether this is appropriate.

To be honest I quite agree with you, and in the beginning I just meant
to have no Copyright line in there at all - but I remembered a
discussion a while back where a patch was I believe rejected because it
lacked one. So I've just copied Tamas' file (vm_event.c) and only
changed the copyright line because I didn't really know what else to put
there.

I'm quite happy to remove it altogether. Will that do?

>> +void hvm_vm_event_do_resume(struct vcpu *v)
>> +{
>> +    struct monitor_write_data *w;
>> +
>> +    if ( likely(!v->arch.vm_event) )
>> +        return;
>> +
>> +    w = &v->arch.vm_event->write_data;
>> +
>> +    if ( unlikely(v->arch.vm_event->emulate_flags) )
>> +    {
>> +        enum emul_kind kind = EMUL_KIND_NORMAL;
>> +
>> +        /*
>> +         * Please observe the order here to match the flag descriptions
>> +         * provided in public/vm_event.h
>> +         */
>> +        if ( v->arch.vm_event->emulate_flags &
>> +             VM_EVENT_FLAG_SET_EMUL_READ_DATA )
>> +            kind = EMUL_KIND_SET_CONTEXT_DATA;
>> +        else if ( v->arch.vm_event->emulate_flags &
>> +                  VM_EVENT_FLAG_EMULATE_NOWRITE )
>> +            kind = EMUL_KIND_NOWRITE;
>> +        else if ( v->arch.vm_event->emulate_flags &
>> +                  VM_EVENT_FLAG_SET_EMUL_INSN_DATA )
>> +            kind = EMUL_KIND_SET_CONTEXT_INSN;
>> +
>> +        hvm_emulate_one_vm_event(kind, TRAP_invalid_op,
>> +                                 X86_EVENT_NO_EC);
>> +
>> +        v->arch.vm_event->emulate_flags = 0;
>> +    }
>> +
>> +    if ( w->do_write.cr0 )
>> +    {
>> +        if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION )
>> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +        w->do_write.cr0 = 0;
>> +    }
>> +
>> +    if ( w->do_write.cr4 )
>> +    {
>> +        if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION )
>> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +        w->do_write.cr4 = 0;
>> +    }
>> +
>> +    if ( w->do_write.cr3 )
>> +    {
>> +        if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION )
>> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +        w->do_write.cr3 = 0;
>> +    }
>> +
>> +    if ( w->do_write.msr )
>> +    {
>> +        if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
>> +             X86EMUL_EXCEPTION )
>> +            hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +
>> +        w->do_write.msr = 0;
>> +    }
> 
> I wonder whether all of these outer if()-s wouldn't better have
> unlikely() too.

It can't hurt, unless anyone objects I'll wrap them in unlikely()s.


Thanks,
Razvan

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

Re: [PATCH V2 2/2] x86/vm_event: fix race between __context_switch() and vm_event_resume()

[Razvan Cojocaru]

On 05/03/17 13:01, Jan Beulich wrote:
>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote:
>> The introspection agent can reply to a vm_event faster than
>> vmx_vmexit_handler() can complete in some cases, where it is then
>> not safe for vm_event_set_registers() to modify v->arch.user_regs.
>> In the test scenario, we were stepping over an INT3 breakpoint by
>> setting RIP += 1. The quick reply tended to complete before the VCPU
>> triggering the introspection event had properly paused and been
>> descheduled. If the reply occurs before __context_switch() happens,
>> __context_switch() clobbers the reply by overwriting
>> v->arch.user_regs from the stack. If the reply occurs after
>> __context_switch(), we don't pass through __context_switch() when
>> transitioning to idle.
> 
> This last sentence still looks to be wrong (and even self-contradictory).
> The reply can't occur after __context_switch() if we don't make it there
> in the first place. How about "If we don't pass through
> __context_switch() (due to switching to the idle vCPU), reply data
> wouldn't be picked up when switching back straight to the original
> vCPU"?

Quite right, it's very convoluted. I'll update the comment.


Thanks,
Razvan


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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Jan Beulich]

>>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com> wrote:
> On 05/03/17 12:51, Jan Beulich wrote:
>>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/x86/hvm/vm_event.c
>>> @@ -0,0 +1,101 @@
>>> +/*
>>> + * arch/x86/hvm/vm_event.c
>>> + *
>>> + * HVM vm_event handling routines
>>> + *
>>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com)
>> 
>> I'm notoriously bad when it comes to copyrights, but you just
>> moving code makes me wonder whether this is appropriate.
> 
> To be honest I quite agree with you, and in the beginning I just meant
> to have no Copyright line in there at all - but I remembered a
> discussion a while back where a patch was I believe rejected because it
> lacked one. So I've just copied Tamas' file (vm_event.c) and only
> changed the copyright line because I didn't really know what else to put
> there.
> 
> I'm quite happy to remove it altogether. Will that do?

Afaic - sure. But as said, I'm quite bad at such things ...

Jan


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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Tamas K Lengyel]

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

On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com> wrote:
> > On 05/03/17 12:51, Jan Beulich wrote:
> >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote:
> >>> --- /dev/null
> >>> +++ b/xen/arch/x86/hvm/vm_event.c
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * arch/x86/hvm/vm_event.c
> >>> + *
> >>> + * HVM vm_event handling routines
> >>> + *
> >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com)
> >>
> >> I'm notoriously bad when it comes to copyrights, but you just
> >> moving code makes me wonder whether this is appropriate.
> >
> > To be honest I quite agree with you, and in the beginning I just meant
> > to have no Copyright line in there at all - but I remembered a
> > discussion a while back where a patch was I believe rejected because it
> > lacked one. So I've just copied Tamas' file (vm_event.c) and only
> > changed the copyright line because I didn't really know what else to put
> > there.
> >
> > I'm quite happy to remove it altogether. Will that do?
>
> Afaic - sure. But as said, I'm quite bad at such things ...
>

Since this is just code-movement from hvm.c to a separate file I would say
it should retain the copyright lines from hvm.c. Other then that it looks
good to me.

Tamas

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

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

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

Re: [PATCH V2 2/2] x86/vm_event: fix race between __context_switch() and vm_event_resume()

[Tamas K Lengyel]

On Wed, May 3, 2017 at 5:10 AM, Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
>
> The introspection agent can reply to a vm_event faster than
> vmx_vmexit_handler() can complete in some cases, where it is then
> not safe for vm_event_set_registers() to modify v->arch.user_regs.
> In the test scenario, we were stepping over an INT3 breakpoint by
> setting RIP += 1. The quick reply tended to complete before the VCPU
> triggering the introspection event had properly paused and been
> descheduled. If the reply occurs before __context_switch() happens,
> __context_switch() clobbers the reply by overwriting
> v->arch.user_regs from the stack. If the reply occurs after
> __context_switch(), we don't pass through __context_switch() when
> transitioning to idle.
>
> This patch ensures that vm_event_resume() code only sets per-VCPU
> data to be used for the actual setting of registers later in
> hvm_do_resume() (similar to the model used to control setting of CRs
> and MSRs).
>
> The patch additionally removes the sync_vcpu_execstate(v) call from
> vm_event_resume(), which is no longer necessary, which removes the
> associated broadcast TLB flush (read: performance improvement).
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Tamas K Lengyel <tamas@tklengyel.com>

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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Razvan Cojocaru]

On 05/03/2017 11:05 PM, Tamas K Lengyel wrote:
> 
> 
> On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com
> <mailto:JBeulich@suse.com>> wrote:
> 
>     >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>     > On 05/03/17 12:51, Jan Beulich wrote:
>     >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>     >>> --- /dev/null
>     >>> +++ b/xen/arch/x86/hvm/vm_event.c
>     >>> @@ -0,0 +1,101 @@
>     >>> +/*
>     >>> + * arch/x86/hvm/vm_event.c
>     >>> + *
>     >>> + * HVM vm_event handling routines
>     >>> + *
>     >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>)
>     >>
>     >> I'm notoriously bad when it comes to copyrights, but you just
>     >> moving code makes me wonder whether this is appropriate.
>     >
>     > To be honest I quite agree with you, and in the beginning I just meant
>     > to have no Copyright line in there at all - but I remembered a
>     > discussion a while back where a patch was I believe rejected because it
>     > lacked one. So I've just copied Tamas' file (vm_event.c) and only
>     > changed the copyright line because I didn't really know what else to put
>     > there.
>     >
>     > I'm quite happy to remove it altogether. Will that do?
> 
>     Afaic - sure. But as said, I'm quite bad at such things ...
> 
> 
> Since this is just code-movement from hvm.c to a separate file I would
> say it should retain the copyright lines from hvm.c. Other then that it
> looks good to me.

Actually the funny part about that is that while this is indeed only
moved code, I have written all of that code in the first place, so I've
moved my own code. :)

But I have no problem with either removing the copyright line altogether
or using the lines in hvm.c as suggested.


Thanks,
Razvan

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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Tamas K Lengyel]

On Wed, May 3, 2017 at 4:16 PM, Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
> On 05/03/2017 11:05 PM, Tamas K Lengyel wrote:
>>
>>
>> On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com
>> <mailto:JBeulich@suse.com>> wrote:
>>
>>     >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>>     > On 05/03/17 12:51, Jan Beulich wrote:
>>     >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>>     >>> --- /dev/null
>>     >>> +++ b/xen/arch/x86/hvm/vm_event.c
>>     >>> @@ -0,0 +1,101 @@
>>     >>> +/*
>>     >>> + * arch/x86/hvm/vm_event.c
>>     >>> + *
>>     >>> + * HVM vm_event handling routines
>>     >>> + *
>>     >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>)
>>     >>
>>     >> I'm notoriously bad when it comes to copyrights, but you just
>>     >> moving code makes me wonder whether this is appropriate.
>>     >
>>     > To be honest I quite agree with you, and in the beginning I just meant
>>     > to have no Copyright line in there at all - but I remembered a
>>     > discussion a while back where a patch was I believe rejected because it
>>     > lacked one. So I've just copied Tamas' file (vm_event.c) and only
>>     > changed the copyright line because I didn't really know what else to put
>>     > there.
>>     >
>>     > I'm quite happy to remove it altogether. Will that do?
>>
>>     Afaic - sure. But as said, I'm quite bad at such things ...
>>
>>
>> Since this is just code-movement from hvm.c to a separate file I would
>> say it should retain the copyright lines from hvm.c. Other then that it
>> looks good to me.
>
> Actually the funny part about that is that while this is indeed only
> moved code, I have written all of that code in the first place, so I've
> moved my own code. :)

Well, I've also added like two lines to it with
VM_EVENT_FLAG_SET_EMUL_INSN_DATA ;)

>
> But I have no problem with either removing the copyright line altogether
> or using the lines in hvm.c as suggested.

Yeap, I'm fine with either route but the safe bet is to just use the
one from hvm.c (and add yourself to it if you feel like).

Tamas

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

Re: [PATCH V2 1/2] x86/vm_event: added hvm/vm_event.{h, c}

[Razvan Cojocaru]

On 05/03/2017 11:32 PM, Tamas K Lengyel wrote:
> On Wed, May 3, 2017 at 4:16 PM, Razvan Cojocaru
> <rcojocaru@bitdefender.com> wrote:
>> On 05/03/2017 11:05 PM, Tamas K Lengyel wrote:
>>>
>>>
>>> On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com
>>> <mailto:JBeulich@suse.com>> wrote:
>>>
>>>     >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>>>     > On 05/03/17 12:51, Jan Beulich wrote:
>>>     >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>>>     >>> --- /dev/null
>>>     >>> +++ b/xen/arch/x86/hvm/vm_event.c
>>>     >>> @@ -0,0 +1,101 @@
>>>     >>> +/*
>>>     >>> + * arch/x86/hvm/vm_event.c
>>>     >>> + *
>>>     >>> + * HVM vm_event handling routines
>>>     >>> + *
>>>     >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>)
>>>     >>
>>>     >> I'm notoriously bad when it comes to copyrights, but you just
>>>     >> moving code makes me wonder whether this is appropriate.
>>>     >
>>>     > To be honest I quite agree with you, and in the beginning I just meant
>>>     > to have no Copyright line in there at all - but I remembered a
>>>     > discussion a while back where a patch was I believe rejected because it
>>>     > lacked one. So I've just copied Tamas' file (vm_event.c) and only
>>>     > changed the copyright line because I didn't really know what else to put
>>>     > there.
>>>     >
>>>     > I'm quite happy to remove it altogether. Will that do?
>>>
>>>     Afaic - sure. But as said, I'm quite bad at such things ...
>>>
>>>
>>> Since this is just code-movement from hvm.c to a separate file I would
>>> say it should retain the copyright lines from hvm.c. Other then that it
>>> looks good to me.
>>
>> Actually the funny part about that is that while this is indeed only
>> moved code, I have written all of that code in the first place, so I've
>> moved my own code. :)
> 
> Well, I've also added like two lines to it with
> VM_EVENT_FLAG_SET_EMUL_INS_DATA ;)

Right, my bad. :)

>> But I have no problem with either removing the copyright line altogether
>> or using the lines in hvm.c as suggested.
> 
> Yeap, I'm fine with either route but the safe bet is to just use the
> one from hvm.c (and add yourself to it if you feel like).

Alright, I'll just copy it from hvm.c.


Thanks,
Razvan

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