All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V6 0/3] Vm_event memory introspection helpers
@ 2015-07-15  8:45 Razvan Cojocaru
  2015-07-15  8:45 ` [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Razvan Cojocaru @ 2015-07-15  8:45 UTC (permalink / raw)
  To: xen-devel
  Cc: jun.nakajima, wei.liu2, kevin.tian, keir, ian.campbell,
	stefano.stabellini, george.dunlap, andrew.cooper3, eddie.dong,
	Aravind.Gopalakrishnan, jbeulich, tlengyel, suravee.suthikulpanit,
	boris.ostrovsky, ian.jackson

This series addresses reviews addressed to V5. All patches have
at least one ack, and the modifications are minor.

Patch 2/3 has not been modified at all, and the only modification
in patch 3/3 is that it now uses vzalloc() / vfree() instead of
xzalloc_array() / xfree(), and both patch 3/3 and 1/3 now set
the allocated data to NULL after freeing it on domain destruction
paths.

Patch 1/3 does has slightly more modifications, however they are
mostly cosmetic (the only non-cosmetic one is that the patch now
bypasses a hvm_copy_from_guest_phys() call that did no harm but
was unnecessary).

As discussed, I've kept the better-safe-than-sorry approach of
freeing allocated data on both domain destruction paths and
vm_event_cleanup(), based on the comments in
shadow_final_teardown(), which imply that it is theoretically
possible to end up on a domain destruction path without
domain_kill() being called (and domain_kill() does the
vm_event_cleanup()).

[PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
[PATCH V6 2/3] xen/vm_event: Support for guest-requested events
[PATCH V6 3/3] xen/vm_event: Deny register writes if refused by
vm_event reply


Thanks in advance for your reviews,
Razvan

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

* [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
  2015-07-15  8:45 [PATCH V6 0/3] Vm_event memory introspection helpers Razvan Cojocaru
@ 2015-07-15  8:45 ` Razvan Cojocaru
  2015-07-15 10:10   ` George Dunlap
  2015-07-15  8:45 ` [PATCH V6 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Razvan Cojocaru @ 2015-07-15  8:45 UTC (permalink / raw)
  To: xen-devel
  Cc: jun.nakajima, wei.liu2, kevin.tian, keir, ian.campbell,
	Razvan Cojocaru, stefano.stabellini, george.dunlap,
	andrew.cooper3, eddie.dong, Aravind.Gopalakrishnan, jbeulich,
	tlengyel, suravee.suthikulpanit, boris.ostrovsky, ian.jackson

This patch adds support for memory-content hiding, by modifying the
value returned by emulated instructions that read certain memory
addresses that contain sensitive data. The patch only applies to
cases where VM_FLAG_ACCESS_EMULATE has been set to a vm_event
response.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Tamas K Lengyel <tlengyel@novetta.com>

---
Changes since V5:
 - Renamed set_context_data()'s bytes parameter to size.
 - Inverted if() condition in set_context_data().
 - Removed memcpy() conditional from set_context_data().
 - Removed label from hvmemul_rep_outs_set_context().
 - Now bypassing hvm_copy_from_guest_phys() in hvmemul_rep_movs() if
   hvmemul_ctxt->set_context is true.
 - Fixed for_each_vcpu() coding style (blank before the opening
   parenthesis).
 - Added comments about the serialization status of
   vm_event_init_domain() and vm_event_cleanup_domain().
 - Setting v->arch.vm_event.emul_read_data to NULL after xfree() in
   vcpu_destroy() for safety.
---
 tools/tests/xen-access/xen-access.c |    2 +-
 xen/arch/x86/domain.c               |    3 +
 xen/arch/x86/hvm/emulate.c          |  117 ++++++++++++++++++++++++++++++++---
 xen/arch/x86/hvm/event.c            |   50 +++++++--------
 xen/arch/x86/mm/p2m.c               |   92 +++++++++++++++------------
 xen/arch/x86/vm_event.c             |   35 +++++++++++
 xen/common/vm_event.c               |    8 +++
 xen/include/asm-arm/vm_event.h      |   13 ++++
 xen/include/asm-x86/domain.h        |    1 +
 xen/include/asm-x86/hvm/emulate.h   |   10 ++-
 xen/include/asm-x86/vm_event.h      |    4 ++
 xen/include/public/vm_event.h       |   35 ++++++++---
 12 files changed, 287 insertions(+), 83 deletions(-)

diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 12ab921..e6ca9ba 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -530,7 +530,7 @@ int main(int argc, char *argv[])
                 break;
             case VM_EVENT_REASON_SOFTWARE_BREAKPOINT:
                 printf("Breakpoint: rip=%016"PRIx64", gfn=%"PRIx64" (vcpu %d)\n",
-                       req.regs.x86.rip,
+                       req.data.regs.x86.rip,
                        req.u.software_breakpoint.gfn,
                        req.vcpu_id);
 
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 34ecd7c..1ef9fad 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -511,6 +511,9 @@ int vcpu_initialise(struct vcpu *v)
 
 void vcpu_destroy(struct vcpu *v)
 {
+    xfree(v->arch.vm_event.emul_read_data);
+    v->arch.vm_event.emul_read_data = NULL;
+
     if ( is_pv_32bit_vcpu(v) )
     {
         free_compat_arg_xlat(v);
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 795321c..2766919 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -67,6 +67,24 @@ static int null_write(const struct hvm_io_handler *handler,
     return X86EMUL_OKAY;
 }
 
+static int set_context_data(void *buffer, unsigned int size)
+{
+    struct vcpu *curr = current;
+
+    if ( curr->arch.vm_event.emul_read_data )
+    {
+        unsigned int safe_size =
+            min(size, curr->arch.vm_event.emul_read_data->size);
+
+        memcpy(buffer, curr->arch.vm_event.emul_read_data->data, safe_size);
+        memset(buffer + safe_size, 0, size - safe_size);
+    }
+    else
+        return X86EMUL_UNHANDLEABLE;
+
+    return X86EMUL_OKAY;
+}
+
 static const struct hvm_io_ops null_ops = {
     .read = null_read,
     .write = null_write
@@ -771,6 +789,12 @@ static int hvmemul_read(
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
+    struct hvm_emulate_ctxt *hvmemul_ctxt =
+        container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
+
+    if ( unlikely(hvmemul_ctxt->set_context) )
+        return set_context_data(p_data, bytes);
+
     return __hvmemul_read(
         seg, offset, p_data, bytes, hvm_access_read,
         container_of(ctxt, struct hvm_emulate_ctxt, ctxt));
@@ -963,6 +987,17 @@ static int hvmemul_cmpxchg(
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
+    struct hvm_emulate_ctxt *hvmemul_ctxt =
+        container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
+
+    if ( unlikely(hvmemul_ctxt->set_context) )
+    {
+        int rc = set_context_data(p_new, bytes);
+
+        if ( rc != X86EMUL_OKAY )
+            return rc;
+    }
+
     /* Fix this in case the guest is really relying on r-m-w atomicity. */
     return hvmemul_write(seg, offset, p_new, bytes, ctxt);
 }
@@ -1005,6 +1040,38 @@ static int hvmemul_rep_ins(
                                !!(ctxt->regs->eflags & X86_EFLAGS_DF), gpa);
 }
 
+static int hvmemul_rep_outs_set_context(
+    enum x86_segment src_seg,
+    unsigned long src_offset,
+    uint16_t dst_port,
+    unsigned int bytes_per_rep,
+    unsigned long *reps,
+    struct x86_emulate_ctxt *ctxt)
+{
+    unsigned int bytes = *reps * bytes_per_rep;
+    char *buf;
+    int rc;
+
+    buf = xmalloc_array(char, bytes);
+
+    if ( buf == NULL )
+        return X86EMUL_UNHANDLEABLE;
+
+    rc = set_context_data(buf, bytes);
+
+    if ( rc != X86EMUL_OKAY )
+    {
+        xfree(buf);
+        return rc;
+    }
+
+    rc = hvmemul_do_pio_buffer(dst_port, bytes, IOREQ_WRITE, buf);
+
+    xfree(buf);
+
+    return rc;
+}
+
 static int hvmemul_rep_outs(
     enum x86_segment src_seg,
     unsigned long src_offset,
@@ -1021,6 +1088,10 @@ static int hvmemul_rep_outs(
     p2m_type_t p2mt;
     int rc;
 
+    if ( unlikely(hvmemul_ctxt->set_context) )
+        return hvmemul_rep_outs_set_context(src_seg, src_offset, dst_port,
+                                            bytes_per_rep, reps, ctxt);
+
     rc = hvmemul_virtual_to_linear(
         src_seg, src_offset, bytes_per_rep, reps, hvm_access_read,
         hvmemul_ctxt, &addr);
@@ -1127,11 +1198,26 @@ static int hvmemul_rep_movs(
     if ( buf == NULL )
         return X86EMUL_UNHANDLEABLE;
 
-    /*
-     * We do a modicum of checking here, just for paranoia's sake and to
-     * definitely avoid copying an unitialised buffer into guest address space.
-     */
-    rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
+    if ( unlikely(hvmemul_ctxt->set_context) )
+    {
+        rc = set_context_data(buf, bytes);
+
+        if ( rc != X86EMUL_OKAY)
+        {
+            xfree(buf);
+            return rc;
+        }
+
+        rc = HVMCOPY_okay;
+    }
+    else
+        /*
+         * We do a modicum of checking here, just for paranoia's sake and to
+         * definitely avoid copying an unitialised buffer into guest address
+         * space.
+         */
+        rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
+
     if ( rc == HVMCOPY_okay )
         rc = hvm_copy_to_guest_phys(dgpa, buf, bytes);
 
@@ -1292,7 +1378,14 @@ static int hvmemul_read_io(
     unsigned long *val,
     struct x86_emulate_ctxt *ctxt)
 {
+    struct hvm_emulate_ctxt *hvmemul_ctxt =
+        container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
+
     *val = 0;
+
+    if ( unlikely(hvmemul_ctxt->set_context) )
+        return set_context_data(val, bytes);
+
     return hvmemul_do_pio_buffer(port, bytes, IOREQ_READ, val);
 }
 
@@ -1677,7 +1770,7 @@ int hvm_emulate_one_no_write(
     return _hvm_emulate_one(hvmemul_ctxt, &hvm_emulate_ops_no_write);
 }
 
-void hvm_mem_access_emulate_one(bool_t nowrite, unsigned int trapnr,
+void hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int trapnr,
     unsigned int errcode)
 {
     struct hvm_emulate_ctxt ctx = {{ 0 }};
@@ -1685,10 +1778,17 @@ void hvm_mem_access_emulate_one(bool_t nowrite, unsigned int trapnr,
 
     hvm_emulate_prepare(&ctx, guest_cpu_user_regs());
 
-    if ( nowrite )
+    switch ( kind )
+    {
+    case EMUL_KIND_NOWRITE:
         rc = hvm_emulate_one_no_write(&ctx);
-    else
+        break;
+    case EMUL_KIND_SET_CONTEXT:
+        ctx.set_context = 1;
+        /* Intentional fall-through. */
+    default:
         rc = hvm_emulate_one(&ctx);
+    }
 
     switch ( rc )
     {
@@ -1722,6 +1822,7 @@ void hvm_emulate_prepare(
     hvmemul_ctxt->ctxt.force_writeback = 1;
     hvmemul_ctxt->seg_reg_accessed = 0;
     hvmemul_ctxt->seg_reg_dirty = 0;
+    hvmemul_ctxt->set_context = 0;
     hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt);
     hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt);
 }
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 53b9ca4..5341937 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -30,31 +30,31 @@ static void hvm_event_fill_regs(vm_event_request_t *req)
     const struct cpu_user_regs *regs = guest_cpu_user_regs();
     const struct vcpu *curr = current;
 
-    req->regs.x86.rax = regs->eax;
-    req->regs.x86.rcx = regs->ecx;
-    req->regs.x86.rdx = regs->edx;
-    req->regs.x86.rbx = regs->ebx;
-    req->regs.x86.rsp = regs->esp;
-    req->regs.x86.rbp = regs->ebp;
-    req->regs.x86.rsi = regs->esi;
-    req->regs.x86.rdi = regs->edi;
-
-    req->regs.x86.r8  = regs->r8;
-    req->regs.x86.r9  = regs->r9;
-    req->regs.x86.r10 = regs->r10;
-    req->regs.x86.r11 = regs->r11;
-    req->regs.x86.r12 = regs->r12;
-    req->regs.x86.r13 = regs->r13;
-    req->regs.x86.r14 = regs->r14;
-    req->regs.x86.r15 = regs->r15;
-
-    req->regs.x86.rflags = regs->eflags;
-    req->regs.x86.rip    = regs->eip;
-
-    req->regs.x86.msr_efer = curr->arch.hvm_vcpu.guest_efer;
-    req->regs.x86.cr0 = curr->arch.hvm_vcpu.guest_cr[0];
-    req->regs.x86.cr3 = curr->arch.hvm_vcpu.guest_cr[3];
-    req->regs.x86.cr4 = curr->arch.hvm_vcpu.guest_cr[4];
+    req->data.regs.x86.rax = regs->eax;
+    req->data.regs.x86.rcx = regs->ecx;
+    req->data.regs.x86.rdx = regs->edx;
+    req->data.regs.x86.rbx = regs->ebx;
+    req->data.regs.x86.rsp = regs->esp;
+    req->data.regs.x86.rbp = regs->ebp;
+    req->data.regs.x86.rsi = regs->esi;
+    req->data.regs.x86.rdi = regs->edi;
+
+    req->data.regs.x86.r8  = regs->r8;
+    req->data.regs.x86.r9  = regs->r9;
+    req->data.regs.x86.r10 = regs->r10;
+    req->data.regs.x86.r11 = regs->r11;
+    req->data.regs.x86.r12 = regs->r12;
+    req->data.regs.x86.r13 = regs->r13;
+    req->data.regs.x86.r14 = regs->r14;
+    req->data.regs.x86.r15 = regs->r15;
+
+    req->data.regs.x86.rflags = regs->eflags;
+    req->data.regs.x86.rip    = regs->eip;
+
+    req->data.regs.x86.msr_efer = curr->arch.hvm_vcpu.guest_efer;
+    req->data.regs.x86.cr0 = curr->arch.hvm_vcpu.guest_cr[0];
+    req->data.regs.x86.cr3 = curr->arch.hvm_vcpu.guest_cr[3];
+    req->data.regs.x86.cr4 = curr->arch.hvm_vcpu.guest_cr[4];
 }
 
 static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index f180cee..6fe6387 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1369,49 +1369,49 @@ static void p2m_vm_event_fill_regs(vm_event_request_t *req)
     /* Architecture-specific vmcs/vmcb bits */
     hvm_funcs.save_cpu_ctxt(curr, &ctxt);
 
-    req->regs.x86.rax = regs->eax;
-    req->regs.x86.rcx = regs->ecx;
-    req->regs.x86.rdx = regs->edx;
-    req->regs.x86.rbx = regs->ebx;
-    req->regs.x86.rsp = regs->esp;
-    req->regs.x86.rbp = regs->ebp;
-    req->regs.x86.rsi = regs->esi;
-    req->regs.x86.rdi = regs->edi;
-
-    req->regs.x86.r8  = regs->r8;
-    req->regs.x86.r9  = regs->r9;
-    req->regs.x86.r10 = regs->r10;
-    req->regs.x86.r11 = regs->r11;
-    req->regs.x86.r12 = regs->r12;
-    req->regs.x86.r13 = regs->r13;
-    req->regs.x86.r14 = regs->r14;
-    req->regs.x86.r15 = regs->r15;
-
-    req->regs.x86.rflags = regs->eflags;
-    req->regs.x86.rip    = regs->eip;
-
-    req->regs.x86.dr7 = curr->arch.debugreg[7];
-    req->regs.x86.cr0 = ctxt.cr0;
-    req->regs.x86.cr2 = ctxt.cr2;
-    req->regs.x86.cr3 = ctxt.cr3;
-    req->regs.x86.cr4 = ctxt.cr4;
-
-    req->regs.x86.sysenter_cs = ctxt.sysenter_cs;
-    req->regs.x86.sysenter_esp = ctxt.sysenter_esp;
-    req->regs.x86.sysenter_eip = ctxt.sysenter_eip;
-
-    req->regs.x86.msr_efer = ctxt.msr_efer;
-    req->regs.x86.msr_star = ctxt.msr_star;
-    req->regs.x86.msr_lstar = ctxt.msr_lstar;
+    req->data.regs.x86.rax = regs->eax;
+    req->data.regs.x86.rcx = regs->ecx;
+    req->data.regs.x86.rdx = regs->edx;
+    req->data.regs.x86.rbx = regs->ebx;
+    req->data.regs.x86.rsp = regs->esp;
+    req->data.regs.x86.rbp = regs->ebp;
+    req->data.regs.x86.rsi = regs->esi;
+    req->data.regs.x86.rdi = regs->edi;
+
+    req->data.regs.x86.r8  = regs->r8;
+    req->data.regs.x86.r9  = regs->r9;
+    req->data.regs.x86.r10 = regs->r10;
+    req->data.regs.x86.r11 = regs->r11;
+    req->data.regs.x86.r12 = regs->r12;
+    req->data.regs.x86.r13 = regs->r13;
+    req->data.regs.x86.r14 = regs->r14;
+    req->data.regs.x86.r15 = regs->r15;
+
+    req->data.regs.x86.rflags = regs->eflags;
+    req->data.regs.x86.rip    = regs->eip;
+
+    req->data.regs.x86.dr7 = curr->arch.debugreg[7];
+    req->data.regs.x86.cr0 = ctxt.cr0;
+    req->data.regs.x86.cr2 = ctxt.cr2;
+    req->data.regs.x86.cr3 = ctxt.cr3;
+    req->data.regs.x86.cr4 = ctxt.cr4;
+
+    req->data.regs.x86.sysenter_cs = ctxt.sysenter_cs;
+    req->data.regs.x86.sysenter_esp = ctxt.sysenter_esp;
+    req->data.regs.x86.sysenter_eip = ctxt.sysenter_eip;
+
+    req->data.regs.x86.msr_efer = ctxt.msr_efer;
+    req->data.regs.x86.msr_star = ctxt.msr_star;
+    req->data.regs.x86.msr_lstar = ctxt.msr_lstar;
 
     hvm_get_segment_register(curr, x86_seg_fs, &seg);
-    req->regs.x86.fs_base = seg.base;
+    req->data.regs.x86.fs_base = seg.base;
 
     hvm_get_segment_register(curr, x86_seg_gs, &seg);
-    req->regs.x86.gs_base = seg.base;
+    req->data.regs.x86.gs_base = seg.base;
 
     hvm_get_segment_register(curr, x86_seg_cs, &seg);
-    req->regs.x86.cs_arbytes = seg.attr.bytes;
+    req->data.regs.x86.cs_arbytes = seg.attr.bytes;
 }
 
 void p2m_mem_access_emulate_check(struct vcpu *v,
@@ -1466,6 +1466,10 @@ void p2m_mem_access_emulate_check(struct vcpu *v,
         }
 
         v->arch.vm_event.emulate_flags = violation ? rsp->flags : 0;
+
+        if ( (rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA) &&
+             v->arch.vm_event.emul_read_data )
+            *v->arch.vm_event.emul_read_data = rsp->data.emul_read_data;
     }
 }
 
@@ -1552,9 +1556,17 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
 
     if ( v->arch.vm_event.emulate_flags )
     {
-        hvm_mem_access_emulate_one((v->arch.vm_event.emulate_flags &
-                                    VM_EVENT_FLAG_EMULATE_NOWRITE) != 0,
-                                   TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);
+        enum emul_kind kind = EMUL_KIND_NORMAL;
+
+        if ( v->arch.vm_event.emulate_flags &
+             VM_EVENT_FLAG_SET_EMUL_READ_DATA )
+            kind = EMUL_KIND_SET_CONTEXT;
+        else if ( v->arch.vm_event.emulate_flags &
+                  VM_EVENT_FLAG_EMULATE_NOWRITE )
+            kind = EMUL_KIND_NOWRITE;
+
+        hvm_mem_access_emulate_one(kind, TRAP_invalid_op,
+                                   HVM_DELIVER_NO_ERROR_CODE);
 
         v->arch.vm_event.emulate_flags = 0;
         return 1;
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index c390225..ec856fb 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -23,6 +23,41 @@
 #include <xen/sched.h>
 #include <asm/hvm/hvm.h>
 
+/* Implicitly serialized by the domctl lock. */
+int vm_event_init_domain(struct domain *d)
+{
+    struct vcpu *v;
+
+    for_each_vcpu ( d, v )
+    {
+        if ( v->arch.vm_event.emul_read_data )
+            continue;
+
+        v->arch.vm_event.emul_read_data =
+            xzalloc(struct vm_event_emul_read_data);
+
+        if ( !v->arch.vm_event.emul_read_data )
+            return -ENOMEM;
+    }
+
+    return 0;
+}
+
+/*
+ * Implicitly serialized by the domctl lock,
+ * or on domain cleanup paths only.
+ */
+void vm_event_cleanup_domain(struct domain *d)
+{
+    struct vcpu *v;
+
+    for_each_vcpu ( d, v )
+    {
+        xfree(v->arch.vm_event.emul_read_data);
+        v->arch.vm_event.emul_read_data = NULL;
+    }
+}
+
 void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
 {
     if ( !is_hvm_domain(d) || !atomic_read(&v->vm_event_pause_count) )
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index a4b9c36..0007d70 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -64,6 +64,11 @@ static int vm_event_enable(
     vm_event_ring_lock_init(ved);
     vm_event_ring_lock(ved);
 
+    rc = vm_event_init_domain(d);
+
+    if ( rc < 0 )
+        goto err;
+
     rc = prepare_ring_for_helper(d, ring_gfn, &ved->ring_pg_struct,
                                     &ved->ring_page);
     if ( rc < 0 )
@@ -226,6 +231,9 @@ static int vm_event_disable(struct domain *d, struct vm_event_domain *ved)
 
         destroy_ring_for_helper(&ved->ring_page,
                                 ved->ring_pg_struct);
+
+        vm_event_cleanup_domain(d);
+
         vm_event_ring_unlock(ved);
     }
 
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index a517495..20469a8 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -23,6 +23,19 @@
 #include <xen/sched.h>
 
 static inline
+int vm_event_init_domain(struct domain *d)
+{
+    /* Not supported on ARM. */
+    return 0;
+}
+
+static inline
+void vm_event_cleanup_domain(struct domain *d)
+{
+    /* Not supported on ARM. */
+}
+
+static inline
 void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
 {
     /* Not supported on ARM. */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 201436d..c5ad1cb 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -512,6 +512,7 @@ struct arch_vcpu
         uint32_t emulate_flags;
         unsigned long gpa;
         unsigned long eip;
+        struct vm_event_emul_read_data *emul_read_data;
     } vm_event;
 
 };
diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
index 594be38..49134b5 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -32,13 +32,21 @@ struct hvm_emulate_ctxt {
     struct hvm_trap trap;
 
     uint32_t intr_shadow;
+
+    bool_t set_context;
+};
+
+enum emul_kind {
+    EMUL_KIND_NORMAL,
+    EMUL_KIND_NOWRITE,
+    EMUL_KIND_SET_CONTEXT
 };
 
 int hvm_emulate_one(
     struct hvm_emulate_ctxt *hvmemul_ctxt);
 int hvm_emulate_one_no_write(
     struct hvm_emulate_ctxt *hvmemul_ctxt);
-void hvm_mem_access_emulate_one(bool_t nowrite,
+void hvm_mem_access_emulate_one(enum emul_kind kind,
     unsigned int trapnr,
     unsigned int errcode);
 void hvm_emulate_prepare(
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 7cc3a3d..3881783 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -22,6 +22,10 @@
 
 #include <xen/sched.h>
 
+int vm_event_init_domain(struct domain *d);
+
+void vm_event_cleanup_domain(struct domain *d);
+
 void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v);
 
 #endif /* __ASM_X86_VM_EVENT_H__ */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index c756c7c..4d89c38 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -44,9 +44,9 @@
  *  paused
  * VCPU_PAUSED in a response signals to unpause the vCPU
  */
-#define VM_EVENT_FLAG_VCPU_PAUSED       (1 << 0)
+#define VM_EVENT_FLAG_VCPU_PAUSED        (1 << 0)
 /* Flags to aid debugging vm_event */
-#define VM_EVENT_FLAG_FOREIGN           (1 << 1)
+#define VM_EVENT_FLAG_FOREIGN            (1 << 1)
 /*
  * The following flags can be set in response to a mem_access event.
  *
@@ -54,17 +54,26 @@
  * This will allow the guest to continue execution without lifting the page
  * access restrictions.
  */
-#define VM_EVENT_FLAG_EMULATE           (1 << 2)
+#define VM_EVENT_FLAG_EMULATE            (1 << 2)
 /*
- * Same as MEM_ACCESS_EMULATE, but with write operations or operations
+ * Same as VM_EVENT_FLAG_EMULATE, but with write operations or operations
  * potentially having side effects (like memory mapped or port I/O) disabled.
  */
-#define VM_EVENT_FLAG_EMULATE_NOWRITE   (1 << 3)
+#define VM_EVENT_FLAG_EMULATE_NOWRITE    (1 << 3)
 /*
  * Toggle singlestepping on vm_event response.
  * Requires the vCPU to be paused already (synchronous events only).
  */
-#define VM_EVENT_FLAG_TOGGLE_SINGLESTEP (1 << 4)
+#define VM_EVENT_FLAG_TOGGLE_SINGLESTEP  (1 << 4)
+/*
+ * Data is being sent back to the hypervisor in the event response, to be
+ * returned by the read function when emulating an instruction.
+ * This flag is only useful when combined with VM_EVENT_FLAG_EMULATE
+ * and takes precedence if combined with VM_EVENT_FLAG_EMULATE_NOWRITE
+ * (i.e. if both VM_EVENT_FLAG_EMULATE_NOWRITE and
+ * VM_EVENT_FLAG_SET_EMUL_READ_DATA are set, only the latter will be honored).
+ */
+#define VM_EVENT_FLAG_SET_EMUL_READ_DATA (1 << 5)
 
 /*
  * Reasons for the vm event request
@@ -194,6 +203,12 @@ struct vm_event_sharing {
     uint32_t _pad;
 };
 
+struct vm_event_emul_read_data {
+    uint32_t size;
+    /* The struct is used in a union with vm_event_regs_x86. */
+    uint8_t  data[sizeof(struct vm_event_regs_x86) - sizeof(uint32_t)];
+};
+
 typedef struct vm_event_st {
     uint32_t version;   /* VM_EVENT_INTERFACE_VERSION */
     uint32_t flags;     /* VM_EVENT_FLAG_* */
@@ -211,8 +226,12 @@ typedef struct vm_event_st {
     } u;
 
     union {
-        struct vm_event_regs_x86 x86;
-    } regs;
+        union {
+            struct vm_event_regs_x86 x86;
+        } regs;
+
+        struct vm_event_emul_read_data emul_read_data;
+    } data;
 } vm_event_request_t, vm_event_response_t;
 
 DEFINE_RING_TYPES(vm_event, vm_event_request_t, vm_event_response_t);
-- 
1.7.9.5

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

* [PATCH V6 2/3] xen/vm_event: Support for guest-requested events
  2015-07-15  8:45 [PATCH V6 0/3] Vm_event memory introspection helpers Razvan Cojocaru
  2015-07-15  8:45 ` [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
@ 2015-07-15  8:45 ` Razvan Cojocaru
  2015-07-15  8:45 ` [PATCH V6 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
  2015-07-15 10:21 ` [PATCH V6 0/3] Vm_event memory introspection helpers Wei Liu
  3 siblings, 0 replies; 10+ messages in thread
From: Razvan Cojocaru @ 2015-07-15  8:45 UTC (permalink / raw)
  To: xen-devel
  Cc: jun.nakajima, wei.liu2, kevin.tian, keir, ian.campbell,
	Razvan Cojocaru, stefano.stabellini, george.dunlap,
	andrew.cooper3, eddie.dong, Aravind.Gopalakrishnan, jbeulich,
	tlengyel, suravee.suthikulpanit, boris.ostrovsky, ian.jackson

Added support for a new class of vm_events: VM_EVENT_REASON_REQUEST,
sent via HVMOP_request_vm_event. The guest can request that a
generic vm_event (containing only the vm_event-filled guest registers
as information) be sent to userspace by setting up the correct
registers and doing a VMCALL. For example, for a 32-bit guest, this
means: EAX = 34 (hvmop), EBX = 24 (HVMOP_guest_request_vm_event),
ECX = 0 (NULL required for the hypercall parameter, reserved).

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

---
Changes since V5:
 - None.
---
 tools/libxc/include/xenctrl.h   |    2 ++
 tools/libxc/xc_monitor.c        |   15 +++++++++++++++
 xen/arch/x86/hvm/event.c        |   16 ++++++++++++++++
 xen/arch/x86/hvm/hvm.c          |    8 +++++++-
 xen/arch/x86/monitor.c          |   19 ++++++++++++++++++-
 xen/include/asm-x86/domain.h    |   16 +++++++++-------
 xen/include/asm-x86/hvm/event.h |    1 +
 xen/include/public/domctl.h     |    6 ++++++
 xen/include/public/hvm/hvm_op.h |    2 ++
 xen/include/public/vm_event.h   |    2 ++
 10 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0bbae2a..ce9029c 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2378,6 +2378,8 @@ int xc_monitor_mov_to_msr(xc_interface *xch, domid_t domain_id, bool enable,
 int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, bool enable);
 int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
                                    bool enable);
+int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
+                             bool enable, bool sync);
 
 /***
  * Memory sharing operations.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b64bce3..d5f87da 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -129,3 +129,18 @@ int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id,
 
     return do_domctl(xch, &domctl);
 }
+
+int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
+                             bool sync)
+{
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_monitor_op;
+    domctl.domain = domain_id;
+    domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+                                    : XEN_DOMCTL_MONITOR_OP_DISABLE;
+    domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST;
+    domctl.u.monitor_op.u.guest_request.sync = sync;
+
+    return do_domctl(xch, &domctl);
+}
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 5341937..17638ea 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -126,6 +126,22 @@ void hvm_event_msr(unsigned int msr, uint64_t value)
         hvm_event_traps(1, &req);
 }
 
+void hvm_event_guest_request(void)
+{
+    struct vcpu *curr = current;
+    struct arch_domain *currad = &curr->domain->arch;
+
+    if ( currad->monitor.guest_request_enabled )
+    {
+        vm_event_request_t req = {
+            .reason = VM_EVENT_REASON_GUEST_REQUEST,
+            .vcpu_id = curr->vcpu_id,
+        };
+
+        hvm_event_traps(currad->monitor.guest_request_sync, &req);
+    }
+}
+
 int hvm_event_int3(unsigned long gla)
 {
     int rc = 0;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8a10111..22dbab1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5999,7 +5999,6 @@ static int hvmop_get_param(
 #define HVMOP_op_mask 0xff
 
 long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
-
 {
     unsigned long start_iter, mask;
     long rc = 0;
@@ -6413,6 +6412,13 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 
+    case HVMOP_guest_request_vm_event:
+        if ( guest_handle_is_null(arg) )
+            hvm_event_guest_request();
+        else
+            rc = -EINVAL;
+        break;
+
     default:
     {
         gdprintk(XENLOG_DEBUG, "Bad HVM op %ld.\n", op);
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 0da855e..d35907b 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -55,7 +55,8 @@ static inline uint32_t get_capabilities(struct domain *d)
 
     capabilities = (1 << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
                    (1 << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
-                   (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT);
+                   (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+                   (1 << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
 
     /* Since we know this is on VMX, we can just call the hvm func */
     if ( hvm_is_singlestep_supported() )
@@ -184,6 +185,22 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
         break;
     }
 
+    case XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST:
+    {
+        bool_t status = ad->monitor.guest_request_enabled;
+
+        rc = status_check(mop, status);
+        if ( rc )
+            return rc;
+
+        ad->monitor.guest_request_sync = mop->u.guest_request.sync;
+
+        domain_pause(d);
+        ad->monitor.guest_request_enabled = !status;
+        domain_unpause(d);
+        break;
+    }
+
     default:
         return -EOPNOTSUPP;
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c5ad1cb..9fbbdd9 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -347,13 +347,15 @@ struct arch_domain
 
     /* Monitor options */
     struct {
-        uint16_t write_ctrlreg_enabled       : 4;
-        uint16_t write_ctrlreg_sync          : 4;
-        uint16_t write_ctrlreg_onchangeonly  : 4;
-        uint16_t mov_to_msr_enabled          : 1;
-        uint16_t mov_to_msr_extended         : 1;
-        uint16_t singlestep_enabled          : 1;
-        uint16_t software_breakpoint_enabled : 1;
+        unsigned int write_ctrlreg_enabled       : 4;
+        unsigned int write_ctrlreg_sync          : 4;
+        unsigned int write_ctrlreg_onchangeonly  : 4;
+        unsigned int mov_to_msr_enabled          : 1;
+        unsigned int mov_to_msr_extended         : 1;
+        unsigned int singlestep_enabled          : 1;
+        unsigned int software_breakpoint_enabled : 1;
+        unsigned int guest_request_enabled       : 1;
+        unsigned int guest_request_sync          : 1;
     } monitor;
 
     /* Mem_access emulation control */
diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h
index 819ef96..ab5abd0 100644
--- a/xen/include/asm-x86/hvm/event.h
+++ b/xen/include/asm-x86/hvm/event.h
@@ -26,6 +26,7 @@ void hvm_event_msr(unsigned int msr, uint64_t value);
 /* Called for current VCPU: returns -1 if no listener */
 int hvm_event_int3(unsigned long gla);
 int hvm_event_single_step(unsigned long gla);
+void hvm_event_guest_request(void);
 
 #endif /* __ASM_X86_HVM_EVENT_H__ */
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 8b1d6b4..631935a 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1009,6 +1009,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
 #define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR            1
 #define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP            2
 #define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT   3
+#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST         4
 
 struct xen_domctl_monitor_op {
     uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
@@ -1039,6 +1040,11 @@ struct xen_domctl_monitor_op {
             /* Enable the capture of an extended set of MSRs */
             uint8_t extended_capture;
         } mov_to_msr;
+
+        struct {
+            /* Pause vCPU until response */
+            uint8_t sync;
+        } guest_request;
     } u;
 };
 typedef struct xen_domctl_monitor_op xen_domctl_monitor_op_t;
diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index 9b84e84..d053db9 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -396,6 +396,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_evtchn_upcall_vector_t);
 
 #endif /* defined(__i386__) || defined(__x86_64__) */
 
+#define HVMOP_guest_request_vm_event 24
+
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */
 
 /*
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 4d89c38..f889139 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -95,6 +95,8 @@
 #define VM_EVENT_REASON_SOFTWARE_BREAKPOINT     6
 /* Single-step (e.g. MTF) */
 #define VM_EVENT_REASON_SINGLESTEP              7
+/* An event has been requested via HVMOP_guest_request_vm_event. */
+#define VM_EVENT_REASON_GUEST_REQUEST           8
 
 /* Supported values for the vm_event_write_ctrlreg index. */
 #define VM_EVENT_X86_CR0    0
-- 
1.7.9.5

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

* [PATCH V6 3/3] xen/vm_event: Deny register writes if refused by vm_event reply
  2015-07-15  8:45 [PATCH V6 0/3] Vm_event memory introspection helpers Razvan Cojocaru
  2015-07-15  8:45 ` [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
  2015-07-15  8:45 ` [PATCH V6 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
@ 2015-07-15  8:45 ` Razvan Cojocaru
  2015-07-15 10:21 ` [PATCH V6 0/3] Vm_event memory introspection helpers Wei Liu
  3 siblings, 0 replies; 10+ messages in thread
From: Razvan Cojocaru @ 2015-07-15  8:45 UTC (permalink / raw)
  To: xen-devel
  Cc: jun.nakajima, wei.liu2, kevin.tian, keir, ian.campbell,
	Razvan Cojocaru, stefano.stabellini, george.dunlap,
	andrew.cooper3, eddie.dong, Aravind.Gopalakrishnan, jbeulich,
	tlengyel, suravee.suthikulpanit, boris.ostrovsky, ian.jackson

Deny register writes if a vm_client subscribed to mov_to_msr or
control register write events forbids them. Currently supported for
MSR, CR0, CR3 and CR4 events.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tamas K Lengyel <tlengyel@novetta.com>

---
Changes since V5:
 - Now using vzalloc() / vfree() for d->arch.event_write_data,
   and setting it to NULL after releasing it in arch_domain_destroy()
   for safety.
---
 xen/arch/x86/domain.c             |    3 +
 xen/arch/x86/hvm/emulate.c        |    8 +--
 xen/arch/x86/hvm/event.c          |    5 +-
 xen/arch/x86/hvm/hvm.c            |  118 ++++++++++++++++++++++++++++++++-----
 xen/arch/x86/hvm/svm/nestedsvm.c  |   14 ++---
 xen/arch/x86/hvm/svm/svm.c        |    2 +-
 xen/arch/x86/hvm/vmx/vmx.c        |   15 +++--
 xen/arch/x86/hvm/vmx/vvmx.c       |   18 +++---
 xen/arch/x86/vm_event.c           |   43 ++++++++++++++
 xen/common/vm_event.c             |    4 ++
 xen/include/asm-arm/vm_event.h    |    7 +++
 xen/include/asm-x86/domain.h      |   18 +++++-
 xen/include/asm-x86/hvm/event.h   |    9 ++-
 xen/include/asm-x86/hvm/support.h |    9 +--
 xen/include/asm-x86/vm_event.h    |    3 +
 xen/include/public/vm_event.h     |    5 ++
 16 files changed, 235 insertions(+), 46 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 1ef9fad..045f6ff 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -668,6 +668,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
 
 void arch_domain_destroy(struct domain *d)
 {
+    vfree(d->arch.event_write_data);
+    d->arch.event_write_data = NULL;
+
     if ( has_hvm_container_domain(d) )
         hvm_domain_destroy(d);
 
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 2766919..bc7514a 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1428,14 +1428,14 @@ static int hvmemul_write_cr(
     switch ( reg )
     {
     case 0:
-        return hvm_set_cr0(val);
+        return hvm_set_cr0(val, 1);
     case 2:
         current->arch.hvm_vcpu.guest_cr[2] = val;
         return X86EMUL_OKAY;
     case 3:
-        return hvm_set_cr3(val);
+        return hvm_set_cr3(val, 1);
     case 4:
-        return hvm_set_cr4(val);
+        return hvm_set_cr4(val, 1);
     default:
         break;
     }
@@ -1456,7 +1456,7 @@ static int hvmemul_write_msr(
     uint64_t val,
     struct x86_emulate_ctxt *ctxt)
 {
-    return hvm_msr_write_intercept(reg, val);
+    return hvm_msr_write_intercept(reg, val, 1);
 }
 
 static int hvmemul_wbinvd(
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 17638ea..042e583 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -90,7 +90,7 @@ static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
     return 1;
 }
 
-void hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
+bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
 {
     struct arch_domain *currad = &current->domain->arch;
     unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(index);
@@ -109,7 +109,10 @@ void hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
 
         hvm_event_traps(currad->monitor.write_ctrlreg_sync & ctrlreg_bitmask,
                         &req);
+        return 1;
     }
+
+    return 0;
 }
 
 void hvm_event_msr(unsigned int msr, uint64_t value)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 22dbab1..c07e3ef 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -52,6 +52,7 @@
 #include <asm/traps.h>
 #include <asm/mc146818rtc.h>
 #include <asm/mce.h>
+#include <asm/monitor.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/vpt.h>
 #include <asm/hvm/support.h>
@@ -519,6 +520,35 @@ void hvm_do_resume(struct vcpu *v)
         break;
     }
 
+    if ( unlikely(d->arch.event_write_data) )
+    {
+        struct monitor_write_data *w = &d->arch.event_write_data[v->vcpu_id];
+
+        if ( w->do_write.msr )
+        {
+            hvm_msr_write_intercept(w->msr, w->value, 0);
+            w->do_write.msr = 0;
+        }
+
+        if ( w->do_write.cr0 )
+        {
+            hvm_set_cr0(w->cr0, 0);
+            w->do_write.cr0 = 0;
+        }
+
+        if ( w->do_write.cr4 )
+        {
+            hvm_set_cr4(w->cr4, 0);
+            w->do_write.cr4 = 0;
+        }
+
+        if ( w->do_write.cr3 )
+        {
+            hvm_set_cr3(w->cr3, 0);
+            w->do_write.cr3 = 0;
+        }
+    }
+
     /* Inject pending hw/sw trap */
     if ( v->arch.hvm_vcpu.inject_trap.vector != -1 ) 
     {
@@ -3123,13 +3153,13 @@ int hvm_mov_to_cr(unsigned int cr, unsigned int gpr)
     switch ( cr )
     {
     case 0:
-        return hvm_set_cr0(val);
+        return hvm_set_cr0(val, 1);
 
     case 3:
-        return hvm_set_cr3(val);
+        return hvm_set_cr3(val, 1);
 
     case 4:
-        return hvm_set_cr4(val);
+        return hvm_set_cr4(val, 1);
 
     case 8:
         vlapic_set_reg(vcpu_vlapic(curr), APIC_TASKPRI, ((val & 0x0f) << 4));
@@ -3226,12 +3256,13 @@ static void hvm_update_cr(struct vcpu *v, unsigned int cr, unsigned long value)
     hvm_update_guest_cr(v, cr);
 }
 
-int hvm_set_cr0(unsigned long value)
+int hvm_set_cr0(unsigned long value, bool_t may_defer)
 {
     struct vcpu *v = current;
     struct domain *d = v->domain;
     unsigned long gfn, old_value = v->arch.hvm_vcpu.guest_cr[0];
     struct page_info *page;
+    struct arch_domain *currad = &v->domain->arch;
 
     HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value);
 
@@ -3261,6 +3292,22 @@ int hvm_set_cr0(unsigned long value)
         goto gpf;
     }
 
+    if ( may_defer && unlikely(currad->monitor.write_ctrlreg_enabled &
+                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR0)) &&
+         value != old_value )
+    {
+        ASSERT(currad->event_write_data != NULL);
+
+        if ( hvm_event_crX(CR0, value, old_value) )
+        {
+            /* The actual write will occur in hvm_do_resume(), if permitted. */
+            currad->event_write_data[v->vcpu_id].do_write.cr0 = 1;
+            currad->event_write_data[v->vcpu_id].cr0 = value;
+
+            return X86EMUL_OKAY;
+        }
+    }
+
     if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
     {
         if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
@@ -3327,7 +3374,6 @@ int hvm_set_cr0(unsigned long value)
         hvm_funcs.handle_cd(v, value);
 
     hvm_update_cr(v, 0, value);
-    hvm_event_crX(CR0, value, old_value);
 
     if ( (value ^ old_value) & X86_CR0_PG ) {
         if ( !nestedhvm_vmswitch_in_progress(v) && nestedhvm_vcpu_in_guestmode(v) )
@@ -3343,11 +3389,28 @@ int hvm_set_cr0(unsigned long value)
     return X86EMUL_EXCEPTION;
 }
 
-int hvm_set_cr3(unsigned long value)
+int hvm_set_cr3(unsigned long value, bool_t may_defer)
 {
     struct vcpu *v = current;
     struct page_info *page;
-    unsigned long old;
+    unsigned long old = v->arch.hvm_vcpu.guest_cr[3];
+    struct arch_domain *currad = &v->domain->arch;
+
+    if ( may_defer && unlikely(currad->monitor.write_ctrlreg_enabled &
+                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3)) &&
+         value != old )
+    {
+        ASSERT(currad->event_write_data != NULL);
+
+        if ( hvm_event_crX(CR3, value, old) )
+        {
+            /* The actual write will occur in hvm_do_resume(), if permitted. */
+            currad->event_write_data[v->vcpu_id].do_write.cr3 = 1;
+            currad->event_write_data[v->vcpu_id].cr3 = value;
+
+            return X86EMUL_OKAY;
+        }
+    }
 
     if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) &&
          (value != v->arch.hvm_vcpu.guest_cr[3]) )
@@ -3365,10 +3428,8 @@ int hvm_set_cr3(unsigned long value)
         HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR3 value = %lx", value);
     }
 
-    old=v->arch.hvm_vcpu.guest_cr[3];
     v->arch.hvm_vcpu.guest_cr[3] = value;
     paging_update_cr3(v);
-    hvm_event_crX(CR3, value, old);
     return X86EMUL_OKAY;
 
  bad_cr3:
@@ -3377,10 +3438,11 @@ int hvm_set_cr3(unsigned long value)
     return X86EMUL_UNHANDLEABLE;
 }
 
-int hvm_set_cr4(unsigned long value)
+int hvm_set_cr4(unsigned long value, bool_t may_defer)
 {
     struct vcpu *v = current;
     unsigned long old_cr;
+    struct arch_domain *currad = &v->domain->arch;
 
     if ( value & hvm_cr4_guest_reserved_bits(v, 0) )
     {
@@ -3408,8 +3470,23 @@ int hvm_set_cr4(unsigned long value)
         goto gpf;
     }
 
+    if ( may_defer && unlikely(currad->monitor.write_ctrlreg_enabled &
+                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR4)) &&
+         value != old_cr )
+    {
+        ASSERT(currad->event_write_data != NULL);
+
+        if ( hvm_event_crX(CR4, value, old_cr) )
+        {
+            /* The actual write will occur in hvm_do_resume(), if permitted. */
+            currad->event_write_data[v->vcpu_id].do_write.cr4 = 1;
+            currad->event_write_data[v->vcpu_id].cr4 = value;
+
+            return X86EMUL_OKAY;
+        }
+    }
+
     hvm_update_cr(v, 4, value);
-    hvm_event_crX(CR4, value, old_cr);
 
     /*
      * Modifying CR4.{PSE,PAE,PGE,SMEP}, or clearing CR4.PCIDE
@@ -3873,7 +3950,7 @@ void hvm_task_switch(
         goto out;
 
 
-    if ( hvm_set_cr3(tss.cr3) )
+    if ( hvm_set_cr3(tss.cr3, 1) )
         goto out;
 
     regs->eip    = tss.eip;
@@ -4575,12 +4652,14 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
     goto out;
 }
 
-int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
+int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
+                            bool_t may_defer)
 {
     struct vcpu *v = current;
     bool_t mtrr;
     unsigned int edx, index;
     int ret = X86EMUL_OKAY;
+    struct arch_domain *currad = &current->domain->arch;
 
     HVMTRACE_3D(MSR_WRITE, msr,
                (uint32_t)msr_content, (uint32_t)(msr_content >> 32));
@@ -4588,7 +4667,18 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
     hvm_cpuid(1, NULL, NULL, NULL, &edx);
     mtrr = !!(edx & cpufeat_mask(X86_FEATURE_MTRR));
 
-    hvm_event_msr(msr, msr_content);
+    if ( may_defer && unlikely(currad->monitor.mov_to_msr_enabled) )
+    {
+        ASSERT(currad->event_write_data != NULL);
+
+        /* The actual write will occur in hvm_do_resume() (if permitted). */
+        currad->event_write_data[v->vcpu_id].do_write.msr = 1;
+        currad->event_write_data[v->vcpu_id].msr = msr;
+        currad->event_write_data[v->vcpu_id].value = msr_content;
+
+        hvm_event_msr(msr, msr_content);
+        return X86EMUL_OKAY;
+    }
 
     switch ( msr )
     {
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 2653bc1..f22b7d1 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -274,7 +274,7 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs)
 
     /* CR4 */
     v->arch.hvm_vcpu.guest_cr[4] = n1vmcb->_cr4;
-    rc = hvm_set_cr4(n1vmcb->_cr4);
+    rc = hvm_set_cr4(n1vmcb->_cr4, 1);
     if (rc != X86EMUL_OKAY)
         gdprintk(XENLOG_ERR, "hvm_set_cr4 failed, rc: %u\n", rc);
 
@@ -283,7 +283,7 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs)
         svm->ns_cr0, v->arch.hvm_vcpu.guest_cr[0]);
     v->arch.hvm_vcpu.guest_cr[0] = n1vmcb->_cr0 | X86_CR0_PE;
     n1vmcb->rflags &= ~X86_EFLAGS_VM;
-    rc = hvm_set_cr0(n1vmcb->_cr0 | X86_CR0_PE);
+    rc = hvm_set_cr0(n1vmcb->_cr0 | X86_CR0_PE, 1);
     if (rc != X86EMUL_OKAY)
         gdprintk(XENLOG_ERR, "hvm_set_cr0 failed, rc: %u\n", rc);
     svm->ns_cr0 = v->arch.hvm_vcpu.guest_cr[0];
@@ -309,7 +309,7 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs)
         v->arch.guest_table = pagetable_null();
         /* hvm_set_cr3() below sets v->arch.hvm_vcpu.guest_cr[3] for us. */
     }
-    rc = hvm_set_cr3(n1vmcb->_cr3);
+    rc = hvm_set_cr3(n1vmcb->_cr3, 1);
     if (rc != X86EMUL_OKAY)
         gdprintk(XENLOG_ERR, "hvm_set_cr3 failed, rc: %u\n", rc);
 
@@ -534,7 +534,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
 
     /* CR4 */
     v->arch.hvm_vcpu.guest_cr[4] = ns_vmcb->_cr4;
-    rc = hvm_set_cr4(ns_vmcb->_cr4);
+    rc = hvm_set_cr4(ns_vmcb->_cr4, 1);
     if (rc != X86EMUL_OKAY)
         gdprintk(XENLOG_ERR, "hvm_set_cr4 failed, rc: %u\n", rc);
 
@@ -542,7 +542,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
     svm->ns_cr0 = v->arch.hvm_vcpu.guest_cr[0];
     cr0 = nestedsvm_fpu_vmentry(svm->ns_cr0, ns_vmcb, n1vmcb, n2vmcb);
     v->arch.hvm_vcpu.guest_cr[0] = ns_vmcb->_cr0;
-    rc = hvm_set_cr0(cr0);
+    rc = hvm_set_cr0(cr0, 1);
     if (rc != X86EMUL_OKAY)
         gdprintk(XENLOG_ERR, "hvm_set_cr0 failed, rc: %u\n", rc);
 
@@ -558,7 +558,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         nestedsvm_vmcb_set_nestedp2m(v, ns_vmcb, n2vmcb);
 
         /* hvm_set_cr3() below sets v->arch.hvm_vcpu.guest_cr[3] for us. */
-        rc = hvm_set_cr3(ns_vmcb->_cr3);
+        rc = hvm_set_cr3(ns_vmcb->_cr3, 1);
         if (rc != X86EMUL_OKAY)
             gdprintk(XENLOG_ERR, "hvm_set_cr3 failed, rc: %u\n", rc);
     } else if (paging_mode_hap(v->domain)) {
@@ -570,7 +570,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
          * we assume it intercepts page faults.
          */
         /* hvm_set_cr3() below sets v->arch.hvm_vcpu.guest_cr[3] for us. */
-        rc = hvm_set_cr3(ns_vmcb->_cr3);
+        rc = hvm_set_cr3(ns_vmcb->_cr3, 1);
         if (rc != X86EMUL_OKAY)
             gdprintk(XENLOG_ERR, "hvm_set_cr3 failed, rc: %u\n", rc);
     } else {
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 70de49e..b8bba71 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1949,7 +1949,7 @@ static void svm_do_msr_access(struct cpu_user_regs *regs)
         if ( (inst_len = __get_instruction_length(v, INSTR_WRMSR)) == 0 )
             return;
         msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
-        rc = hvm_msr_write_intercept(regs->ecx, msr_content);
+        rc = hvm_msr_write_intercept(regs->ecx, msr_content, 1);
     }
 
     if ( rc == X86EMUL_OKAY )
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index bc3212f..d3183a8 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2016,9 +2016,16 @@ static int vmx_cr_access(unsigned long exit_qualification)
     }
     case VMX_CONTROL_REG_ACCESS_TYPE_CLTS: {
         unsigned long old = curr->arch.hvm_vcpu.guest_cr[0];
-        curr->arch.hvm_vcpu.guest_cr[0] &= ~X86_CR0_TS;
+        unsigned long value = old & ~X86_CR0_TS;
+
+        /*
+         * Special case unlikely to be interesting to a
+         * VM_EVENT_FLAG_DENY-capable application, so the hvm_event_crX()
+         * return value is ignored for now.
+         */
+        hvm_event_crX(CR0, value, old);
+        curr->arch.hvm_vcpu.guest_cr[0] = value;
         vmx_update_guest_cr(curr, 0);
-        hvm_event_crX(CR0, curr->arch.hvm_vcpu.guest_cr[0], old);
         HVMTRACE_0D(CLTS);
         break;
     }
@@ -2030,7 +2037,7 @@ static int vmx_cr_access(unsigned long exit_qualification)
                 (VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) &
                  (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS));
         HVMTRACE_LONG_1D(LMSW, value);
-        return hvm_set_cr0(value);
+        return hvm_set_cr0(value, 1);
     }
     default:
         BUG();
@@ -3053,7 +3060,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
     {
         uint64_t msr_content;
         msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
-        if ( hvm_msr_write_intercept(regs->ecx, msr_content) == X86EMUL_OKAY )
+        if ( hvm_msr_write_intercept(regs->ecx, msr_content, 1) == X86EMUL_OKAY )
             update_guest_eip(); /* Safe: WRMSR */
         break;
     }
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 72dd9c8..555fdfa 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1034,15 +1034,16 @@ static void load_shadow_guest_state(struct vcpu *v)
 
     nvcpu->guest_cr[0] = __get_vvmcs(vvmcs, CR0_READ_SHADOW);
     nvcpu->guest_cr[4] = __get_vvmcs(vvmcs, CR4_READ_SHADOW);
-    hvm_set_cr0(__get_vvmcs(vvmcs, GUEST_CR0));
-    hvm_set_cr4(__get_vvmcs(vvmcs, GUEST_CR4));
-    hvm_set_cr3(__get_vvmcs(vvmcs, GUEST_CR3));
+    hvm_set_cr0(__get_vvmcs(vvmcs, GUEST_CR0), 1);
+    hvm_set_cr4(__get_vvmcs(vvmcs, GUEST_CR4), 1);
+    hvm_set_cr3(__get_vvmcs(vvmcs, GUEST_CR3), 1);
 
     control = __get_vvmcs(vvmcs, VM_ENTRY_CONTROLS);
     if ( control & VM_ENTRY_LOAD_GUEST_PAT )
         hvm_set_guest_pat(v, __get_vvmcs(vvmcs, GUEST_PAT));
     if ( control & VM_ENTRY_LOAD_PERF_GLOBAL_CTRL )
-        hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL, __get_vvmcs(vvmcs, GUEST_PERF_GLOBAL_CTRL));
+        hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL,
+                                __get_vvmcs(vvmcs, GUEST_PERF_GLOBAL_CTRL), 0);
 
     hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset, 0);
 
@@ -1235,15 +1236,16 @@ static void load_vvmcs_host_state(struct vcpu *v)
         __vmwrite(vmcs_h2g_field[i].guest_field, r);
     }
 
-    hvm_set_cr0(__get_vvmcs(vvmcs, HOST_CR0));
-    hvm_set_cr4(__get_vvmcs(vvmcs, HOST_CR4));
-    hvm_set_cr3(__get_vvmcs(vvmcs, HOST_CR3));
+    hvm_set_cr0(__get_vvmcs(vvmcs, HOST_CR0), 1);
+    hvm_set_cr4(__get_vvmcs(vvmcs, HOST_CR4), 1);
+    hvm_set_cr3(__get_vvmcs(vvmcs, HOST_CR3), 1);
 
     control = __get_vvmcs(vvmcs, VM_EXIT_CONTROLS);
     if ( control & VM_EXIT_LOAD_HOST_PAT )
         hvm_set_guest_pat(v, __get_vvmcs(vvmcs, HOST_PAT));
     if ( control & VM_EXIT_LOAD_PERF_GLOBAL_CTRL )
-        hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL, __get_vvmcs(vvmcs, HOST_PERF_GLOBAL_CTRL));
+        hvm_msr_write_intercept(MSR_CORE_PERF_GLOBAL_CTRL,
+                                __get_vvmcs(vvmcs, HOST_PERF_GLOBAL_CTRL), 1);
 
     hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset, 0);
 
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index ec856fb..5fba6b7 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -22,12 +22,20 @@
 
 #include <xen/sched.h>
 #include <asm/hvm/hvm.h>
+#include <asm/vm_event.h>
 
 /* Implicitly serialized by the domctl lock. */
 int vm_event_init_domain(struct domain *d)
 {
     struct vcpu *v;
 
+    if ( !d->arch.event_write_data )
+        d->arch.event_write_data =
+            vzalloc(sizeof(struct monitor_write_data) * d->max_vcpus);
+
+    if ( !d->arch.event_write_data )
+        return -ENOMEM;
+
     for_each_vcpu ( d, v )
     {
         if ( v->arch.vm_event.emul_read_data )
@@ -51,6 +59,9 @@ void vm_event_cleanup_domain(struct domain *d)
 {
     struct vcpu *v;
 
+    vfree(d->arch.event_write_data);
+    d->arch.event_write_data = NULL;
+
     for_each_vcpu ( d, v )
     {
         xfree(v->arch.vm_event.emul_read_data);
@@ -66,6 +77,38 @@ void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
     hvm_toggle_singlestep(v);
 }
 
+void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
+{
+    if ( rsp->flags & VM_EVENT_FLAG_DENY )
+    {
+        struct monitor_write_data *w =
+            &v->domain->arch.event_write_data[v->vcpu_id];
+
+        ASSERT(v->domain->arch.event_write_data != NULL);
+
+        switch ( rsp->reason )
+        {
+        case VM_EVENT_REASON_MOV_TO_MSR:
+            w->do_write.msr = 0;
+            break;
+        case VM_EVENT_REASON_WRITE_CTRLREG:
+            switch ( rsp->u.write_ctrlreg.index )
+            {
+            case VM_EVENT_X86_CR0:
+                w->do_write.cr0 = 0;
+                break;
+            case VM_EVENT_X86_CR3:
+                w->do_write.cr3 = 0;
+                break;
+            case VM_EVENT_X86_CR4:
+                w->do_write.cr4 = 0;
+                break;
+            }
+            break;
+        }
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 0007d70..4c6bf98 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -393,6 +393,10 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
          */
         switch ( rsp.reason )
         {
+        case VM_EVENT_REASON_MOV_TO_MSR:
+        case VM_EVENT_REASON_WRITE_CTRLREG:
+            vm_event_register_write_resume(v, &rsp);
+            break;
 
 #ifdef HAS_MEM_ACCESS
         case VM_EVENT_REASON_MEM_ACCESS:
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 20469a8..0833a65 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -21,6 +21,7 @@
 #define __ASM_ARM_VM_EVENT_H__
 
 #include <xen/sched.h>
+#include <xen/vm_event.h>
 
 static inline
 int vm_event_init_domain(struct domain *d)
@@ -41,4 +42,10 @@ void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
     /* Not supported on ARM. */
 }
 
+static inline
+void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
+{
+    /* Not supported on ARM. */
+}
+
 #endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 9fbbdd9..7a9e96f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -247,6 +247,21 @@ struct pv_domain
     struct mapcache_domain mapcache;
 };
 
+struct monitor_write_data {
+    struct {
+        unsigned int msr : 1;
+        unsigned int cr0 : 1;
+        unsigned int cr3 : 1;
+        unsigned int cr4 : 1;
+    } do_write;
+
+    uint32_t msr;
+    uint64_t value;
+    uint64_t cr0;
+    uint64_t cr3;
+    uint64_t cr4;
+};
+
 struct arch_domain
 {
     struct page_info *perdomain_l3_pg;
@@ -360,6 +375,8 @@ struct arch_domain
 
     /* Mem_access emulation control */
     bool_t mem_access_emulate_enabled;
+
+    struct monitor_write_data *event_write_data;
 } __cacheline_aligned;
 
 #define has_arch_pdevs(d)    (!list_empty(&(d)->arch.pdev_list))
@@ -516,7 +533,6 @@ struct arch_vcpu
         unsigned long eip;
         struct vm_event_emul_read_data *emul_read_data;
     } vm_event;
-
 };
 
 smap_check_policy_t smap_policy_change(struct vcpu *v,
diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h
index ab5abd0..c082c20 100644
--- a/xen/include/asm-x86/hvm/event.h
+++ b/xen/include/asm-x86/hvm/event.h
@@ -18,8 +18,13 @@
 #ifndef __ASM_X86_HVM_EVENT_H__
 #define __ASM_X86_HVM_EVENT_H__
 
-/* Called for current VCPU on crX/MSR changes by guest */
-void hvm_event_cr(unsigned int index, unsigned long value, unsigned long old);
+/*
+ * Called for current VCPU on crX/MSR changes by guest.
+ * The event might not fire if the client has subscribed to it in onchangeonly
+ * mode, hence the bool_t return type for control register write events.
+ */
+bool_t hvm_event_cr(unsigned int index, unsigned long value,
+                    unsigned long old);
 #define hvm_event_crX(what, new, old) \
     hvm_event_cr(VM_EVENT_X86_##what, new, old)
 void hvm_event_msr(unsigned int msr, uint64_t value);
diff --git a/xen/include/asm-x86/hvm/support.h b/xen/include/asm-x86/hvm/support.h
index 05ef5c5..95d3bb2 100644
--- a/xen/include/asm-x86/hvm/support.h
+++ b/xen/include/asm-x86/hvm/support.h
@@ -124,11 +124,12 @@ void hvm_shadow_handle_cd(struct vcpu *v, unsigned long value);
 
 /* These functions all return X86EMUL return codes. */
 int hvm_set_efer(uint64_t value);
-int hvm_set_cr0(unsigned long value);
-int hvm_set_cr3(unsigned long value);
-int hvm_set_cr4(unsigned long value);
+int hvm_set_cr0(unsigned long value, bool_t may_defer);
+int hvm_set_cr3(unsigned long value, bool_t may_defer);
+int hvm_set_cr4(unsigned long value, bool_t may_defer);
 int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
-int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content);
+int hvm_msr_write_intercept(
+    unsigned int msr, uint64_t msr_content, bool_t may_defer);
 int hvm_mov_to_cr(unsigned int cr, unsigned int gpr);
 int hvm_mov_from_cr(unsigned int cr, unsigned int gpr);
 
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 3881783..2bcfe26 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -21,6 +21,7 @@
 #define __ASM_X86_VM_EVENT_H__
 
 #include <xen/sched.h>
+#include <xen/vm_event.h>
 
 int vm_event_init_domain(struct domain *d);
 
@@ -28,4 +29,6 @@ void vm_event_cleanup_domain(struct domain *d);
 
 void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v);
 
+void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp);
+
 #endif /* __ASM_X86_VM_EVENT_H__ */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index f889139..fbc76b2 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -74,6 +74,11 @@
  * VM_EVENT_FLAG_SET_EMUL_READ_DATA are set, only the latter will be honored).
  */
 #define VM_EVENT_FLAG_SET_EMUL_READ_DATA (1 << 5)
+ /*
+  * Deny completion of the operation that triggered the event.
+  * Currently only useful for MSR, CR0, CR3 and CR4 write events.
+  */
+#define VM_EVENT_FLAG_DENY               (1 << 6)
 
 /*
  * Reasons for the vm event request
-- 
1.7.9.5

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

* Re: [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
  2015-07-15  8:45 ` [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
@ 2015-07-15 10:10   ` George Dunlap
  2015-07-15 10:21     ` Jan Beulich
  2015-07-15 10:21     ` Razvan Cojocaru
  0 siblings, 2 replies; 10+ messages in thread
From: George Dunlap @ 2015-07-15 10:10 UTC (permalink / raw)
  To: Razvan Cojocaru, xen-devel
  Cc: kevin.tian, keir, eddie.dong, stefano.stabellini, jun.nakajima,
	andrew.cooper3, ian.jackson, Aravind.Gopalakrishnan, jbeulich,
	tlengyel, wei.liu2, boris.ostrovsky, suravee.suthikulpanit,
	ian.campbell

On 07/15/2015 09:45 AM, Razvan Cojocaru wrote:
> This patch adds support for memory-content hiding, by modifying the
> value returned by emulated instructions that read certain memory
> addresses that contain sensitive data. The patch only applies to
> cases where VM_FLAG_ACCESS_EMULATE has been set to a vm_event
> response.
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Acked-by: Tamas K Lengyel <tlengyel@novetta.com>

BTW I've looked at an earlier version of this and acked it, and I
haven't seen any changes I want to review; so when the rest of it is
acked/reviewed I'll take another look through and send my ack.

 -George

> 
> ---
> Changes since V5:
>  - Renamed set_context_data()'s bytes parameter to size.
>  - Inverted if() condition in set_context_data().
>  - Removed memcpy() conditional from set_context_data().
>  - Removed label from hvmemul_rep_outs_set_context().
>  - Now bypassing hvm_copy_from_guest_phys() in hvmemul_rep_movs() if
>    hvmemul_ctxt->set_context is true.
>  - Fixed for_each_vcpu() coding style (blank before the opening
>    parenthesis).
>  - Added comments about the serialization status of
>    vm_event_init_domain() and vm_event_cleanup_domain().
>  - Setting v->arch.vm_event.emul_read_data to NULL after xfree() in
>    vcpu_destroy() for safety.
> ---
>  tools/tests/xen-access/xen-access.c |    2 +-
>  xen/arch/x86/domain.c               |    3 +
>  xen/arch/x86/hvm/emulate.c          |  117 ++++++++++++++++++++++++++++++++---
>  xen/arch/x86/hvm/event.c            |   50 +++++++--------
>  xen/arch/x86/mm/p2m.c               |   92 +++++++++++++++------------
>  xen/arch/x86/vm_event.c             |   35 +++++++++++
>  xen/common/vm_event.c               |    8 +++
>  xen/include/asm-arm/vm_event.h      |   13 ++++
>  xen/include/asm-x86/domain.h        |    1 +
>  xen/include/asm-x86/hvm/emulate.h   |   10 ++-
>  xen/include/asm-x86/vm_event.h      |    4 ++
>  xen/include/public/vm_event.h       |   35 ++++++++---
>  12 files changed, 287 insertions(+), 83 deletions(-)
> 
> diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
> index 12ab921..e6ca9ba 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -530,7 +530,7 @@ int main(int argc, char *argv[])
>                  break;
>              case VM_EVENT_REASON_SOFTWARE_BREAKPOINT:
>                  printf("Breakpoint: rip=%016"PRIx64", gfn=%"PRIx64" (vcpu %d)\n",
> -                       req.regs.x86.rip,
> +                       req.data.regs.x86.rip,
>                         req.u.software_breakpoint.gfn,
>                         req.vcpu_id);
>  
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 34ecd7c..1ef9fad 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -511,6 +511,9 @@ int vcpu_initialise(struct vcpu *v)
>  
>  void vcpu_destroy(struct vcpu *v)
>  {
> +    xfree(v->arch.vm_event.emul_read_data);
> +    v->arch.vm_event.emul_read_data = NULL;
> +
>      if ( is_pv_32bit_vcpu(v) )
>      {
>          free_compat_arg_xlat(v);
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 795321c..2766919 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -67,6 +67,24 @@ static int null_write(const struct hvm_io_handler *handler,
>      return X86EMUL_OKAY;
>  }
>  
> +static int set_context_data(void *buffer, unsigned int size)
> +{
> +    struct vcpu *curr = current;
> +
> +    if ( curr->arch.vm_event.emul_read_data )
> +    {
> +        unsigned int safe_size =
> +            min(size, curr->arch.vm_event.emul_read_data->size);
> +
> +        memcpy(buffer, curr->arch.vm_event.emul_read_data->data, safe_size);
> +        memset(buffer + safe_size, 0, size - safe_size);
> +    }
> +    else
> +        return X86EMUL_UNHANDLEABLE;
> +
> +    return X86EMUL_OKAY;
> +}
> +
>  static const struct hvm_io_ops null_ops = {
>      .read = null_read,
>      .write = null_write
> @@ -771,6 +789,12 @@ static int hvmemul_read(
>      unsigned int bytes,
>      struct x86_emulate_ctxt *ctxt)
>  {
> +    struct hvm_emulate_ctxt *hvmemul_ctxt =
> +        container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> +
> +    if ( unlikely(hvmemul_ctxt->set_context) )
> +        return set_context_data(p_data, bytes);
> +
>      return __hvmemul_read(
>          seg, offset, p_data, bytes, hvm_access_read,
>          container_of(ctxt, struct hvm_emulate_ctxt, ctxt));
> @@ -963,6 +987,17 @@ static int hvmemul_cmpxchg(
>      unsigned int bytes,
>      struct x86_emulate_ctxt *ctxt)
>  {
> +    struct hvm_emulate_ctxt *hvmemul_ctxt =
> +        container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> +
> +    if ( unlikely(hvmemul_ctxt->set_context) )
> +    {
> +        int rc = set_context_data(p_new, bytes);
> +
> +        if ( rc != X86EMUL_OKAY )
> +            return rc;
> +    }
> +
>      /* Fix this in case the guest is really relying on r-m-w atomicity. */
>      return hvmemul_write(seg, offset, p_new, bytes, ctxt);
>  }
> @@ -1005,6 +1040,38 @@ static int hvmemul_rep_ins(
>                                 !!(ctxt->regs->eflags & X86_EFLAGS_DF), gpa);
>  }
>  
> +static int hvmemul_rep_outs_set_context(
> +    enum x86_segment src_seg,
> +    unsigned long src_offset,
> +    uint16_t dst_port,
> +    unsigned int bytes_per_rep,
> +    unsigned long *reps,
> +    struct x86_emulate_ctxt *ctxt)
> +{
> +    unsigned int bytes = *reps * bytes_per_rep;
> +    char *buf;
> +    int rc;
> +
> +    buf = xmalloc_array(char, bytes);
> +
> +    if ( buf == NULL )
> +        return X86EMUL_UNHANDLEABLE;
> +
> +    rc = set_context_data(buf, bytes);
> +
> +    if ( rc != X86EMUL_OKAY )
> +    {
> +        xfree(buf);
> +        return rc;
> +    }
> +
> +    rc = hvmemul_do_pio_buffer(dst_port, bytes, IOREQ_WRITE, buf);
> +
> +    xfree(buf);
> +
> +    return rc;
> +}
> +
>  static int hvmemul_rep_outs(
>      enum x86_segment src_seg,
>      unsigned long src_offset,
> @@ -1021,6 +1088,10 @@ static int hvmemul_rep_outs(
>      p2m_type_t p2mt;
>      int rc;
>  
> +    if ( unlikely(hvmemul_ctxt->set_context) )
> +        return hvmemul_rep_outs_set_context(src_seg, src_offset, dst_port,
> +                                            bytes_per_rep, reps, ctxt);
> +
>      rc = hvmemul_virtual_to_linear(
>          src_seg, src_offset, bytes_per_rep, reps, hvm_access_read,
>          hvmemul_ctxt, &addr);
> @@ -1127,11 +1198,26 @@ static int hvmemul_rep_movs(
>      if ( buf == NULL )
>          return X86EMUL_UNHANDLEABLE;
>  
> -    /*
> -     * We do a modicum of checking here, just for paranoia's sake and to
> -     * definitely avoid copying an unitialised buffer into guest address space.
> -     */
> -    rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
> +    if ( unlikely(hvmemul_ctxt->set_context) )
> +    {
> +        rc = set_context_data(buf, bytes);
> +
> +        if ( rc != X86EMUL_OKAY)
> +        {
> +            xfree(buf);
> +            return rc;
> +        }
> +
> +        rc = HVMCOPY_okay;
> +    }
> +    else
> +        /*
> +         * We do a modicum of checking here, just for paranoia's sake and to
> +         * definitely avoid copying an unitialised buffer into guest address
> +         * space.
> +         */
> +        rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
> +
>      if ( rc == HVMCOPY_okay )
>          rc = hvm_copy_to_guest_phys(dgpa, buf, bytes);
>  
> @@ -1292,7 +1378,14 @@ static int hvmemul_read_io(
>      unsigned long *val,
>      struct x86_emulate_ctxt *ctxt)
>  {
> +    struct hvm_emulate_ctxt *hvmemul_ctxt =
> +        container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> +
>      *val = 0;
> +
> +    if ( unlikely(hvmemul_ctxt->set_context) )
> +        return set_context_data(val, bytes);
> +
>      return hvmemul_do_pio_buffer(port, bytes, IOREQ_READ, val);
>  }
>  
> @@ -1677,7 +1770,7 @@ int hvm_emulate_one_no_write(
>      return _hvm_emulate_one(hvmemul_ctxt, &hvm_emulate_ops_no_write);
>  }
>  
> -void hvm_mem_access_emulate_one(bool_t nowrite, unsigned int trapnr,
> +void hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int trapnr,
>      unsigned int errcode)
>  {
>      struct hvm_emulate_ctxt ctx = {{ 0 }};
> @@ -1685,10 +1778,17 @@ void hvm_mem_access_emulate_one(bool_t nowrite, unsigned int trapnr,
>  
>      hvm_emulate_prepare(&ctx, guest_cpu_user_regs());
>  
> -    if ( nowrite )
> +    switch ( kind )
> +    {
> +    case EMUL_KIND_NOWRITE:
>          rc = hvm_emulate_one_no_write(&ctx);
> -    else
> +        break;
> +    case EMUL_KIND_SET_CONTEXT:
> +        ctx.set_context = 1;
> +        /* Intentional fall-through. */
> +    default:
>          rc = hvm_emulate_one(&ctx);
> +    }
>  
>      switch ( rc )
>      {
> @@ -1722,6 +1822,7 @@ void hvm_emulate_prepare(
>      hvmemul_ctxt->ctxt.force_writeback = 1;
>      hvmemul_ctxt->seg_reg_accessed = 0;
>      hvmemul_ctxt->seg_reg_dirty = 0;
> +    hvmemul_ctxt->set_context = 0;
>      hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt);
>      hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt);
>  }
> diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> index 53b9ca4..5341937 100644
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -30,31 +30,31 @@ static void hvm_event_fill_regs(vm_event_request_t *req)
>      const struct cpu_user_regs *regs = guest_cpu_user_regs();
>      const struct vcpu *curr = current;
>  
> -    req->regs.x86.rax = regs->eax;
> -    req->regs.x86.rcx = regs->ecx;
> -    req->regs.x86.rdx = regs->edx;
> -    req->regs.x86.rbx = regs->ebx;
> -    req->regs.x86.rsp = regs->esp;
> -    req->regs.x86.rbp = regs->ebp;
> -    req->regs.x86.rsi = regs->esi;
> -    req->regs.x86.rdi = regs->edi;
> -
> -    req->regs.x86.r8  = regs->r8;
> -    req->regs.x86.r9  = regs->r9;
> -    req->regs.x86.r10 = regs->r10;
> -    req->regs.x86.r11 = regs->r11;
> -    req->regs.x86.r12 = regs->r12;
> -    req->regs.x86.r13 = regs->r13;
> -    req->regs.x86.r14 = regs->r14;
> -    req->regs.x86.r15 = regs->r15;
> -
> -    req->regs.x86.rflags = regs->eflags;
> -    req->regs.x86.rip    = regs->eip;
> -
> -    req->regs.x86.msr_efer = curr->arch.hvm_vcpu.guest_efer;
> -    req->regs.x86.cr0 = curr->arch.hvm_vcpu.guest_cr[0];
> -    req->regs.x86.cr3 = curr->arch.hvm_vcpu.guest_cr[3];
> -    req->regs.x86.cr4 = curr->arch.hvm_vcpu.guest_cr[4];
> +    req->data.regs.x86.rax = regs->eax;
> +    req->data.regs.x86.rcx = regs->ecx;
> +    req->data.regs.x86.rdx = regs->edx;
> +    req->data.regs.x86.rbx = regs->ebx;
> +    req->data.regs.x86.rsp = regs->esp;
> +    req->data.regs.x86.rbp = regs->ebp;
> +    req->data.regs.x86.rsi = regs->esi;
> +    req->data.regs.x86.rdi = regs->edi;
> +
> +    req->data.regs.x86.r8  = regs->r8;
> +    req->data.regs.x86.r9  = regs->r9;
> +    req->data.regs.x86.r10 = regs->r10;
> +    req->data.regs.x86.r11 = regs->r11;
> +    req->data.regs.x86.r12 = regs->r12;
> +    req->data.regs.x86.r13 = regs->r13;
> +    req->data.regs.x86.r14 = regs->r14;
> +    req->data.regs.x86.r15 = regs->r15;
> +
> +    req->data.regs.x86.rflags = regs->eflags;
> +    req->data.regs.x86.rip    = regs->eip;
> +
> +    req->data.regs.x86.msr_efer = curr->arch.hvm_vcpu.guest_efer;
> +    req->data.regs.x86.cr0 = curr->arch.hvm_vcpu.guest_cr[0];
> +    req->data.regs.x86.cr3 = curr->arch.hvm_vcpu.guest_cr[3];
> +    req->data.regs.x86.cr4 = curr->arch.hvm_vcpu.guest_cr[4];
>  }
>  
>  static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index f180cee..6fe6387 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1369,49 +1369,49 @@ static void p2m_vm_event_fill_regs(vm_event_request_t *req)
>      /* Architecture-specific vmcs/vmcb bits */
>      hvm_funcs.save_cpu_ctxt(curr, &ctxt);
>  
> -    req->regs.x86.rax = regs->eax;
> -    req->regs.x86.rcx = regs->ecx;
> -    req->regs.x86.rdx = regs->edx;
> -    req->regs.x86.rbx = regs->ebx;
> -    req->regs.x86.rsp = regs->esp;
> -    req->regs.x86.rbp = regs->ebp;
> -    req->regs.x86.rsi = regs->esi;
> -    req->regs.x86.rdi = regs->edi;
> -
> -    req->regs.x86.r8  = regs->r8;
> -    req->regs.x86.r9  = regs->r9;
> -    req->regs.x86.r10 = regs->r10;
> -    req->regs.x86.r11 = regs->r11;
> -    req->regs.x86.r12 = regs->r12;
> -    req->regs.x86.r13 = regs->r13;
> -    req->regs.x86.r14 = regs->r14;
> -    req->regs.x86.r15 = regs->r15;
> -
> -    req->regs.x86.rflags = regs->eflags;
> -    req->regs.x86.rip    = regs->eip;
> -
> -    req->regs.x86.dr7 = curr->arch.debugreg[7];
> -    req->regs.x86.cr0 = ctxt.cr0;
> -    req->regs.x86.cr2 = ctxt.cr2;
> -    req->regs.x86.cr3 = ctxt.cr3;
> -    req->regs.x86.cr4 = ctxt.cr4;
> -
> -    req->regs.x86.sysenter_cs = ctxt.sysenter_cs;
> -    req->regs.x86.sysenter_esp = ctxt.sysenter_esp;
> -    req->regs.x86.sysenter_eip = ctxt.sysenter_eip;
> -
> -    req->regs.x86.msr_efer = ctxt.msr_efer;
> -    req->regs.x86.msr_star = ctxt.msr_star;
> -    req->regs.x86.msr_lstar = ctxt.msr_lstar;
> +    req->data.regs.x86.rax = regs->eax;
> +    req->data.regs.x86.rcx = regs->ecx;
> +    req->data.regs.x86.rdx = regs->edx;
> +    req->data.regs.x86.rbx = regs->ebx;
> +    req->data.regs.x86.rsp = regs->esp;
> +    req->data.regs.x86.rbp = regs->ebp;
> +    req->data.regs.x86.rsi = regs->esi;
> +    req->data.regs.x86.rdi = regs->edi;
> +
> +    req->data.regs.x86.r8  = regs->r8;
> +    req->data.regs.x86.r9  = regs->r9;
> +    req->data.regs.x86.r10 = regs->r10;
> +    req->data.regs.x86.r11 = regs->r11;
> +    req->data.regs.x86.r12 = regs->r12;
> +    req->data.regs.x86.r13 = regs->r13;
> +    req->data.regs.x86.r14 = regs->r14;
> +    req->data.regs.x86.r15 = regs->r15;
> +
> +    req->data.regs.x86.rflags = regs->eflags;
> +    req->data.regs.x86.rip    = regs->eip;
> +
> +    req->data.regs.x86.dr7 = curr->arch.debugreg[7];
> +    req->data.regs.x86.cr0 = ctxt.cr0;
> +    req->data.regs.x86.cr2 = ctxt.cr2;
> +    req->data.regs.x86.cr3 = ctxt.cr3;
> +    req->data.regs.x86.cr4 = ctxt.cr4;
> +
> +    req->data.regs.x86.sysenter_cs = ctxt.sysenter_cs;
> +    req->data.regs.x86.sysenter_esp = ctxt.sysenter_esp;
> +    req->data.regs.x86.sysenter_eip = ctxt.sysenter_eip;
> +
> +    req->data.regs.x86.msr_efer = ctxt.msr_efer;
> +    req->data.regs.x86.msr_star = ctxt.msr_star;
> +    req->data.regs.x86.msr_lstar = ctxt.msr_lstar;
>  
>      hvm_get_segment_register(curr, x86_seg_fs, &seg);
> -    req->regs.x86.fs_base = seg.base;
> +    req->data.regs.x86.fs_base = seg.base;
>  
>      hvm_get_segment_register(curr, x86_seg_gs, &seg);
> -    req->regs.x86.gs_base = seg.base;
> +    req->data.regs.x86.gs_base = seg.base;
>  
>      hvm_get_segment_register(curr, x86_seg_cs, &seg);
> -    req->regs.x86.cs_arbytes = seg.attr.bytes;
> +    req->data.regs.x86.cs_arbytes = seg.attr.bytes;
>  }
>  
>  void p2m_mem_access_emulate_check(struct vcpu *v,
> @@ -1466,6 +1466,10 @@ void p2m_mem_access_emulate_check(struct vcpu *v,
>          }
>  
>          v->arch.vm_event.emulate_flags = violation ? rsp->flags : 0;
> +
> +        if ( (rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA) &&
> +             v->arch.vm_event.emul_read_data )
> +            *v->arch.vm_event.emul_read_data = rsp->data.emul_read_data;
>      }
>  }
>  
> @@ -1552,9 +1556,17 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>  
>      if ( v->arch.vm_event.emulate_flags )
>      {
> -        hvm_mem_access_emulate_one((v->arch.vm_event.emulate_flags &
> -                                    VM_EVENT_FLAG_EMULATE_NOWRITE) != 0,
> -                                   TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);
> +        enum emul_kind kind = EMUL_KIND_NORMAL;
> +
> +        if ( v->arch.vm_event.emulate_flags &
> +             VM_EVENT_FLAG_SET_EMUL_READ_DATA )
> +            kind = EMUL_KIND_SET_CONTEXT;
> +        else if ( v->arch.vm_event.emulate_flags &
> +                  VM_EVENT_FLAG_EMULATE_NOWRITE )
> +            kind = EMUL_KIND_NOWRITE;
> +
> +        hvm_mem_access_emulate_one(kind, TRAP_invalid_op,
> +                                   HVM_DELIVER_NO_ERROR_CODE);
>  
>          v->arch.vm_event.emulate_flags = 0;
>          return 1;
> diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
> index c390225..ec856fb 100644
> --- a/xen/arch/x86/vm_event.c
> +++ b/xen/arch/x86/vm_event.c
> @@ -23,6 +23,41 @@
>  #include <xen/sched.h>
>  #include <asm/hvm/hvm.h>
>  
> +/* Implicitly serialized by the domctl lock. */
> +int vm_event_init_domain(struct domain *d)
> +{
> +    struct vcpu *v;
> +
> +    for_each_vcpu ( d, v )
> +    {
> +        if ( v->arch.vm_event.emul_read_data )
> +            continue;
> +
> +        v->arch.vm_event.emul_read_data =
> +            xzalloc(struct vm_event_emul_read_data);
> +
> +        if ( !v->arch.vm_event.emul_read_data )
> +            return -ENOMEM;
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * Implicitly serialized by the domctl lock,
> + * or on domain cleanup paths only.
> + */
> +void vm_event_cleanup_domain(struct domain *d)
> +{
> +    struct vcpu *v;
> +
> +    for_each_vcpu ( d, v )
> +    {
> +        xfree(v->arch.vm_event.emul_read_data);
> +        v->arch.vm_event.emul_read_data = NULL;
> +    }
> +}
> +
>  void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
>  {
>      if ( !is_hvm_domain(d) || !atomic_read(&v->vm_event_pause_count) )
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index a4b9c36..0007d70 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -64,6 +64,11 @@ static int vm_event_enable(
>      vm_event_ring_lock_init(ved);
>      vm_event_ring_lock(ved);
>  
> +    rc = vm_event_init_domain(d);
> +
> +    if ( rc < 0 )
> +        goto err;
> +
>      rc = prepare_ring_for_helper(d, ring_gfn, &ved->ring_pg_struct,
>                                      &ved->ring_page);
>      if ( rc < 0 )
> @@ -226,6 +231,9 @@ static int vm_event_disable(struct domain *d, struct vm_event_domain *ved)
>  
>          destroy_ring_for_helper(&ved->ring_page,
>                                  ved->ring_pg_struct);
> +
> +        vm_event_cleanup_domain(d);
> +
>          vm_event_ring_unlock(ved);
>      }
>  
> diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
> index a517495..20469a8 100644
> --- a/xen/include/asm-arm/vm_event.h
> +++ b/xen/include/asm-arm/vm_event.h
> @@ -23,6 +23,19 @@
>  #include <xen/sched.h>
>  
>  static inline
> +int vm_event_init_domain(struct domain *d)
> +{
> +    /* Not supported on ARM. */
> +    return 0;
> +}
> +
> +static inline
> +void vm_event_cleanup_domain(struct domain *d)
> +{
> +    /* Not supported on ARM. */
> +}
> +
> +static inline
>  void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
>  {
>      /* Not supported on ARM. */
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 201436d..c5ad1cb 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -512,6 +512,7 @@ struct arch_vcpu
>          uint32_t emulate_flags;
>          unsigned long gpa;
>          unsigned long eip;
> +        struct vm_event_emul_read_data *emul_read_data;
>      } vm_event;
>  
>  };
> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
> index 594be38..49134b5 100644
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -32,13 +32,21 @@ struct hvm_emulate_ctxt {
>      struct hvm_trap trap;
>  
>      uint32_t intr_shadow;
> +
> +    bool_t set_context;
> +};
> +
> +enum emul_kind {
> +    EMUL_KIND_NORMAL,
> +    EMUL_KIND_NOWRITE,
> +    EMUL_KIND_SET_CONTEXT
>  };
>  
>  int hvm_emulate_one(
>      struct hvm_emulate_ctxt *hvmemul_ctxt);
>  int hvm_emulate_one_no_write(
>      struct hvm_emulate_ctxt *hvmemul_ctxt);
> -void hvm_mem_access_emulate_one(bool_t nowrite,
> +void hvm_mem_access_emulate_one(enum emul_kind kind,
>      unsigned int trapnr,
>      unsigned int errcode);
>  void hvm_emulate_prepare(
> diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
> index 7cc3a3d..3881783 100644
> --- a/xen/include/asm-x86/vm_event.h
> +++ b/xen/include/asm-x86/vm_event.h
> @@ -22,6 +22,10 @@
>  
>  #include <xen/sched.h>
>  
> +int vm_event_init_domain(struct domain *d);
> +
> +void vm_event_cleanup_domain(struct domain *d);
> +
>  void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v);
>  
>  #endif /* __ASM_X86_VM_EVENT_H__ */
> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
> index c756c7c..4d89c38 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -44,9 +44,9 @@
>   *  paused
>   * VCPU_PAUSED in a response signals to unpause the vCPU
>   */
> -#define VM_EVENT_FLAG_VCPU_PAUSED       (1 << 0)
> +#define VM_EVENT_FLAG_VCPU_PAUSED        (1 << 0)
>  /* Flags to aid debugging vm_event */
> -#define VM_EVENT_FLAG_FOREIGN           (1 << 1)
> +#define VM_EVENT_FLAG_FOREIGN            (1 << 1)
>  /*
>   * The following flags can be set in response to a mem_access event.
>   *
> @@ -54,17 +54,26 @@
>   * This will allow the guest to continue execution without lifting the page
>   * access restrictions.
>   */
> -#define VM_EVENT_FLAG_EMULATE           (1 << 2)
> +#define VM_EVENT_FLAG_EMULATE            (1 << 2)
>  /*
> - * Same as MEM_ACCESS_EMULATE, but with write operations or operations
> + * Same as VM_EVENT_FLAG_EMULATE, but with write operations or operations
>   * potentially having side effects (like memory mapped or port I/O) disabled.
>   */
> -#define VM_EVENT_FLAG_EMULATE_NOWRITE   (1 << 3)
> +#define VM_EVENT_FLAG_EMULATE_NOWRITE    (1 << 3)
>  /*
>   * Toggle singlestepping on vm_event response.
>   * Requires the vCPU to be paused already (synchronous events only).
>   */
> -#define VM_EVENT_FLAG_TOGGLE_SINGLESTEP (1 << 4)
> +#define VM_EVENT_FLAG_TOGGLE_SINGLESTEP  (1 << 4)
> +/*
> + * Data is being sent back to the hypervisor in the event response, to be
> + * returned by the read function when emulating an instruction.
> + * This flag is only useful when combined with VM_EVENT_FLAG_EMULATE
> + * and takes precedence if combined with VM_EVENT_FLAG_EMULATE_NOWRITE
> + * (i.e. if both VM_EVENT_FLAG_EMULATE_NOWRITE and
> + * VM_EVENT_FLAG_SET_EMUL_READ_DATA are set, only the latter will be honored).
> + */
> +#define VM_EVENT_FLAG_SET_EMUL_READ_DATA (1 << 5)
>  
>  /*
>   * Reasons for the vm event request
> @@ -194,6 +203,12 @@ struct vm_event_sharing {
>      uint32_t _pad;
>  };
>  
> +struct vm_event_emul_read_data {
> +    uint32_t size;
> +    /* The struct is used in a union with vm_event_regs_x86. */
> +    uint8_t  data[sizeof(struct vm_event_regs_x86) - sizeof(uint32_t)];
> +};
> +
>  typedef struct vm_event_st {
>      uint32_t version;   /* VM_EVENT_INTERFACE_VERSION */
>      uint32_t flags;     /* VM_EVENT_FLAG_* */
> @@ -211,8 +226,12 @@ typedef struct vm_event_st {
>      } u;
>  
>      union {
> -        struct vm_event_regs_x86 x86;
> -    } regs;
> +        union {
> +            struct vm_event_regs_x86 x86;
> +        } regs;
> +
> +        struct vm_event_emul_read_data emul_read_data;
> +    } data;
>  } vm_event_request_t, vm_event_response_t;
>  
>  DEFINE_RING_TYPES(vm_event, vm_event_request_t, vm_event_response_t);
> 

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

* Re: [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
  2015-07-15 10:10   ` George Dunlap
@ 2015-07-15 10:21     ` Jan Beulich
  2015-07-15 10:38       ` George Dunlap
  2015-07-15 10:21     ` Razvan Cojocaru
  1 sibling, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2015-07-15 10:21 UTC (permalink / raw)
  To: George Dunlap
  Cc: kevin.tian, wei.liu2, ian.campbell, Razvan Cojocaru,
	stefano.stabellini, jun.nakajima, andrew.cooper3, ian.jackson,
	xen-devel, eddie.dong, Aravind.Gopalakrishnan,
	suravee.suthikulpanit, tlengyel, keir, boris.ostrovsky

>>> On 15.07.15 at 12:10, <george.dunlap@eu.citrix.com> wrote:
> On 07/15/2015 09:45 AM, Razvan Cojocaru wrote:
>> This patch adds support for memory-content hiding, by modifying the
>> value returned by emulated instructions that read certain memory
>> addresses that contain sensitive data. The patch only applies to
>> cases where VM_FLAG_ACCESS_EMULATE has been set to a vm_event
>> response.
>> 
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>> Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
> 
> BTW I've looked at an earlier version of this and acked it, and I
> haven't seen any changes I want to review; so when the rest of it is
> acked/reviewed I'll take another look through and send my ack.

As just said on IRC, I think the series is ready to go in once you
gave your ack.

Jan

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

* Re: [PATCH V6 0/3] Vm_event memory introspection helpers
  2015-07-15  8:45 [PATCH V6 0/3] Vm_event memory introspection helpers Razvan Cojocaru
                   ` (2 preceding siblings ...)
  2015-07-15  8:45 ` [PATCH V6 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
@ 2015-07-15 10:21 ` Wei Liu
  3 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2015-07-15 10:21 UTC (permalink / raw)
  To: Razvan Cojocaru
  Cc: jun.nakajima, wei.liu2, kevin.tian, keir, ian.campbell,
	stefano.stabellini, george.dunlap, andrew.cooper3, eddie.dong,
	xen-devel, Aravind.Gopalakrishnan, jbeulich, tlengyel,
	suravee.suthikulpanit, boris.ostrovsky, ian.jackson

On Wed, Jul 15, 2015 at 11:45:46AM +0300, Razvan Cojocaru wrote:
> This series addresses reviews addressed to V5. All patches have
> at least one ack, and the modifications are minor.
> 
> Patch 2/3 has not been modified at all, and the only modification
> in patch 3/3 is that it now uses vzalloc() / vfree() instead of
> xzalloc_array() / xfree(), and both patch 3/3 and 1/3 now set
> the allocated data to NULL after freeing it on domain destruction
> paths.
> 
> Patch 1/3 does has slightly more modifications, however they are
> mostly cosmetic (the only non-cosmetic one is that the patch now
> bypasses a hvm_copy_from_guest_phys() call that did no harm but
> was unnecessary).
> 
> As discussed, I've kept the better-safe-than-sorry approach of
> freeing allocated data on both domain destruction paths and
> vm_event_cleanup(), based on the comments in
> shadow_final_teardown(), which imply that it is theoretically
> possible to end up on a domain destruction path without
> domain_kill() being called (and domain_kill() does the
> vm_event_cleanup()).
> 
> [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
> [PATCH V6 2/3] xen/vm_event: Support for guest-requested events
> [PATCH V6 3/3] xen/vm_event: Deny register writes if refused by
> vm_event reply
> 
> 
> Thanks in advance for your reviews,
> Razvan

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Subject to all concerns from maintainers addressed.

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

* Re: [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
  2015-07-15 10:10   ` George Dunlap
  2015-07-15 10:21     ` Jan Beulich
@ 2015-07-15 10:21     ` Razvan Cojocaru
  1 sibling, 0 replies; 10+ messages in thread
From: Razvan Cojocaru @ 2015-07-15 10:21 UTC (permalink / raw)
  To: George Dunlap, xen-devel
  Cc: kevin.tian, keir, eddie.dong, stefano.stabellini, jun.nakajima,
	andrew.cooper3, ian.jackson, Aravind.Gopalakrishnan, jbeulich,
	tlengyel, wei.liu2, boris.ostrovsky, suravee.suthikulpanit,
	ian.campbell

On 07/15/2015 01:10 PM, George Dunlap wrote:
> On 07/15/2015 09:45 AM, Razvan Cojocaru wrote:
>> This patch adds support for memory-content hiding, by modifying the
>> value returned by emulated instructions that read certain memory
>> addresses that contain sensitive data. The patch only applies to
>> cases where VM_FLAG_ACCESS_EMULATE has been set to a vm_event
>> response.
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>> Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
> 
> BTW I've looked at an earlier version of this and acked it, and I
> haven't seen any changes I want to review; so when the rest of it is
> acked/reviewed I'll take another look through and send my ack.

The mm bits have remained unchanged since your ack, so if it only
concerned those then I believe the ack still applies, but I was unsure
so it seemed better to not make that assumption.


Thanks,
Razvan

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

* Re: [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
  2015-07-15 10:21     ` Jan Beulich
@ 2015-07-15 10:38       ` George Dunlap
  2015-07-15 10:41         ` Razvan Cojocaru
  0 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2015-07-15 10:38 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Tian, Kevin, Wei Liu, Ian Campbell, Razvan Cojocaru,
	Stefano Stabellini, Andrew Cooper, Ian Jackson,
	xen-devel@lists.xen.org, Dong, Eddie, Aravind Gopalakrishnan,
	Jun Nakajima, tlengyel, Keir Fraser, Boris Ostrovsky,
	Suravee Suthikulpanit

On Wed, Jul 15, 2015 at 11:21 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 15.07.15 at 12:10, <george.dunlap@eu.citrix.com> wrote:
>> On 07/15/2015 09:45 AM, Razvan Cojocaru wrote:
>>> This patch adds support for memory-content hiding, by modifying the
>>> value returned by emulated instructions that read certain memory
>>> addresses that contain sensitive data. The patch only applies to
>>> cases where VM_FLAG_ACCESS_EMULATE has been set to a vm_event
>>> response.
>>>
>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>> Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
>>
>> BTW I've looked at an earlier version of this and acked it, and I
>> haven't seen any changes I want to review; so when the rest of it is
>> acked/reviewed I'll take another look through and send my ack.
>
> As just said on IRC, I think the series is ready to go in once you
> gave your ack.

I haven't seen a hypervisor maintainer Ack / Review yet, which is what
I was waiting for. :-)

(And for some reason my IRC client didn't catch your mention of my nick...)

Anyway, re the mm bits:

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

 -George

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

* Re: [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding
  2015-07-15 10:38       ` George Dunlap
@ 2015-07-15 10:41         ` Razvan Cojocaru
  0 siblings, 0 replies; 10+ messages in thread
From: Razvan Cojocaru @ 2015-07-15 10:41 UTC (permalink / raw)
  To: George Dunlap, Jan Beulich
  Cc: Tian, Kevin, Wei Liu, Ian Campbell, Stefano Stabellini,
	Andrew Cooper, Ian Jackson, xen-devel@lists.xen.org, Dong, Eddie,
	Aravind Gopalakrishnan, Jun Nakajima, tlengyel, Keir Fraser,
	Boris Ostrovsky, Suravee Suthikulpanit

On 07/15/2015 01:38 PM, George Dunlap wrote:
> On Wed, Jul 15, 2015 at 11:21 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 15.07.15 at 12:10, <george.dunlap@eu.citrix.com> wrote:
>>> On 07/15/2015 09:45 AM, Razvan Cojocaru wrote:
>>>> This patch adds support for memory-content hiding, by modifying the
>>>> value returned by emulated instructions that read certain memory
>>>> addresses that contain sensitive data. The patch only applies to
>>>> cases where VM_FLAG_ACCESS_EMULATE has been set to a vm_event
>>>> response.
>>>>
>>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>>> Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
>>>
>>> BTW I've looked at an earlier version of this and acked it, and I
>>> haven't seen any changes I want to review; so when the rest of it is
>>> acked/reviewed I'll take another look through and send my ack.
>>
>> As just said on IRC, I think the series is ready to go in once you
>> gave your ack.
> 
> I haven't seen a hypervisor maintainer Ack / Review yet, which is what
> I was waiting for. :-)
> 
> (And for some reason my IRC client didn't catch your mention of my nick...)
> 
> Anyway, re the mm bits:
> 
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

Great news! Thank you to all reviewers for your time and help!


Thanks,
Razvan

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

end of thread, other threads:[~2015-07-15 10:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-15  8:45 [PATCH V6 0/3] Vm_event memory introspection helpers Razvan Cojocaru
2015-07-15  8:45 ` [PATCH V6 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
2015-07-15 10:10   ` George Dunlap
2015-07-15 10:21     ` Jan Beulich
2015-07-15 10:38       ` George Dunlap
2015-07-15 10:41         ` Razvan Cojocaru
2015-07-15 10:21     ` Razvan Cojocaru
2015-07-15  8:45 ` [PATCH V6 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
2015-07-15  8:45 ` [PATCH V6 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
2015-07-15 10:21 ` [PATCH V6 0/3] Vm_event memory introspection helpers Wei Liu

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