* [PATCH V4 0/3] Vm_event memory introspection helpers
@ 2015-07-08 10:22 Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-08 10:22 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
Version 4 of the series addresses V3 reviews, and consists of:
[PATCH 1/3] xen/mem_access: Support for memory-content hiding
[PATCH 2/3] xen/vm_event: Support for guest-requested events
[PATCH 3/3] xen/vm_event: Deny register writes if refused by
vm_event reply
All the patches in this version have been acked by at least one
person. For [PATCH 3/3], Tamas has suggested that I move the
DENY logic from p2m.c to dedicated files, which I've done here.
Since this is simply a trivial move without any modifications
to the logic itself, I've kept both acks received for the patch;
George's ack should in any case not be an issue, as it only
concerned the mm parts which are unchanged, but if I shouldn't
have kept Jan's ack then please disregard it.
This version of the series assumes the patch "vm_event: Rename
MEM_ACCESS_EMULATE and MEM_ACCESS_EMULATE_NOWRITE" that I've
submitted yesterday. I've not added that patch to this series
because I wanted it to be available for Tamas as well, as he's
working on a parallel series and I had hoped that this way
would be better than him having to wait for this whole series
to go in.
Thank you,
Razvan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding
2015-07-08 10:22 [PATCH V4 0/3] Vm_event memory introspection helpers Razvan Cojocaru
@ 2015-07-08 10:22 ` Razvan Cojocaru
2015-07-08 12:24 ` Lengyel, Tamas
2015-07-10 12:41 ` Jan Beulich
2015-07-08 10:22 ` [PATCH V4 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
2 siblings, 2 replies; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-08 10:22 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 MEM_ACCESS_EMULATE or MEM_ACCESS_EMULATE_NOWRITE have
been set to a vm_event response.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
---
Changes since V3:
- Renamed MEM_ACCESS_SET_EMUL_READ_DATA to
VM_EVENT_FLAG_SET_EMUL_READ_DATA and updated its comment.
- Removed xfree(v->arch.vm_event.emul_read_data) from
free_vcpu_struct().
- Returning X86EMUL_UNHANDLEABLE from hvmemul_cmpxchg() when
!curr->arch.vm_event.emul_read_data.
- Replaced in xmalloc_bytes() with xmalloc_array() in
hvmemul_rep_outs_set_context().
- Setting the rest of the buffer to zero in hvmemul_rep_movs()
(no longer leaking heap contents).
- No longer memset()ing the whole buffer before copy (just zeroing
out the rest).
- Moved hvmemul_ctxt->set_context = 0 to hvm_emulate_prepare() and
removed hvm_emulate_one_set_context().
---
tools/tests/xen-access/xen-access.c | 2 +-
xen/arch/x86/hvm/emulate.c | 138 ++++++++++++++++++++++++++++++++++-
xen/arch/x86/hvm/event.c | 50 ++++++-------
xen/arch/x86/mm/p2m.c | 92 +++++++++++++----------
xen/common/domain.c | 2 +
xen/common/vm_event.c | 23 ++++++
xen/include/asm-x86/domain.h | 2 +
xen/include/asm-x86/hvm/emulate.h | 10 ++-
xen/include/public/vm_event.h | 31 ++++++--
9 files changed, 274 insertions(+), 76 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/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index fe5661d..c6ccb1f 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -653,6 +653,31 @@ 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) )
+ {
+ struct vcpu *curr = current;
+ unsigned int safe_bytes;
+
+ if ( !curr->arch.vm_event.emul_read_data )
+ return X86EMUL_UNHANDLEABLE;
+
+ safe_bytes = min_t(unsigned int,
+ bytes, curr->arch.vm_event.emul_read_data->size);
+
+ if ( safe_bytes )
+ {
+ memcpy(p_data, curr->arch.vm_event.emul_read_data->data, safe_bytes);
+
+ if ( bytes > safe_bytes )
+ memset(p_data + safe_bytes, 0, bytes - safe_bytes);
+ }
+
+ return X86EMUL_OKAY;
+ }
+
return __hvmemul_read(
seg, offset, p_data, bytes, hvm_access_read,
container_of(ctxt, struct hvm_emulate_ctxt, ctxt));
@@ -893,6 +918,28 @@ 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) )
+ {
+ struct vcpu *curr = current;
+
+ if ( curr->arch.vm_event.emul_read_data )
+ {
+ unsigned int safe_bytes = min_t(unsigned int, bytes,
+ curr->arch.vm_event.emul_read_data->size);
+
+ memcpy(p_new, curr->arch.vm_event.emul_read_data->data,
+ safe_bytes);
+
+ if ( bytes > safe_bytes )
+ memset(p_new + safe_bytes, 0, bytes - safe_bytes);
+ }
+ else
+ return X86EMUL_UNHANDLEABLE;
+ }
+
/* Fix this in case the guest is really relying on r-m-w atomicity. */
return hvmemul_write(seg, offset, p_new, bytes, ctxt);
}
@@ -935,6 +982,43 @@ 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;
+ struct vcpu *curr = current;
+ unsigned int safe_bytes;
+ char *buf = NULL;
+ int rc;
+
+ if ( !curr->arch.vm_event.emul_read_data )
+ return X86EMUL_UNHANDLEABLE;
+
+ buf = xmalloc_array(char, bytes);
+
+ if ( buf == NULL )
+ return X86EMUL_UNHANDLEABLE;
+
+ safe_bytes = min_t(unsigned int, bytes,
+ curr->arch.vm_event.emul_read_data->size);
+
+ memcpy(buf, curr->arch.vm_event.emul_read_data->data, safe_bytes);
+
+ if ( bytes > safe_bytes )
+ memset(buf + safe_bytes, 0, bytes - safe_bytes);
+
+ 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,
@@ -951,6 +1035,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);
@@ -1063,7 +1151,23 @@ static int hvmemul_rep_movs(
*/
rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
if ( rc == HVMCOPY_okay )
+ {
+ struct vcpu *curr = current;
+
+ if ( unlikely(hvmemul_ctxt->set_context) &&
+ curr->arch.vm_event.emul_read_data )
+ {
+ unsigned int safe_bytes = min_t(unsigned int, bytes,
+ curr->arch.vm_event.emul_read_data->size);
+
+ memcpy(buf, curr->arch.vm_event.emul_read_data->data, safe_bytes);
+
+ if ( bytes > safe_bytes )
+ memset(buf + safe_bytes, 0, bytes - safe_bytes);
+ }
+
rc = hvm_copy_to_guest_phys(dgpa, buf, bytes);
+ }
xfree(buf);
@@ -1222,7 +1326,27 @@ 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) )
+ {
+ struct vcpu *curr = current;
+ unsigned int safe_bytes;
+
+ if ( !curr->arch.vm_event.emul_read_data )
+ return X86EMUL_UNHANDLEABLE;
+
+ safe_bytes = min_t(unsigned int, bytes,
+ curr->arch.vm_event.emul_read_data->size);
+
+ memcpy(val, curr->arch.vm_event.emul_read_data->data, safe_bytes);
+
+ return X86EMUL_OKAY;
+ }
+
return hvmemul_do_pio_buffer(port, bytes, IOREQ_READ, val);
}
@@ -1608,7 +1732,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 }};
@@ -1616,10 +1740,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 )
{
@@ -1653,6 +1784,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 4c49369..2109024 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/common/domain.c b/xen/common/domain.c
index 3bc52e6..ff86266 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -166,6 +166,7 @@ struct vcpu *alloc_vcpu(
free_cpumask_var(v->cpu_hard_affinity_saved);
free_cpumask_var(v->cpu_soft_affinity);
free_cpumask_var(v->vcpu_dirty_cpumask);
+ xfree(v->arch.vm_event.emul_read_data);
free_vcpu_struct(v);
return NULL;
}
@@ -821,6 +822,7 @@ static void complete_domain_destroy(struct rcu_head *head)
free_cpumask_var(v->cpu_hard_affinity_saved);
free_cpumask_var(v->cpu_soft_affinity);
free_cpumask_var(v->vcpu_dirty_cpumask);
+ xfree(v->arch.vm_event.emul_read_data);
free_vcpu_struct(v);
}
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 120a78a..11438da 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -48,6 +48,7 @@ static int vm_event_enable(
{
int rc;
unsigned long ring_gfn = d->arch.hvm_domain.params[param];
+ struct vcpu *v;
/* Only one helper at a time. If the helper crashed,
* the ring is in an undefined state and so is the guest.
@@ -63,6 +64,21 @@ static int vm_event_enable(
vm_event_ring_lock_init(ved);
vm_event_ring_lock(ved);
+ for_each_vcpu( d, v )
+ {
+ if ( v->arch.vm_event.emul_read_data )
+ continue;
+
+ v->arch.vm_event.emul_read_data =
+ xmalloc(struct vm_event_emul_read_data);
+
+ if ( !v->arch.vm_event.emul_read_data )
+ {
+ rc = -ENOMEM;
+ goto err;
+ }
+ }
+
rc = prepare_ring_for_helper(d, ring_gfn, &ved->ring_pg_struct,
&ved->ring_page);
if ( rc < 0 )
@@ -225,6 +241,13 @@ static int vm_event_disable(struct domain *d, struct vm_event_domain *ved)
destroy_ring_for_helper(&ved->ring_page,
ved->ring_pg_struct);
+
+ for_each_vcpu( d, v )
+ {
+ xfree(v->arch.vm_event.emul_read_data);
+ v->arch.vm_event.emul_read_data = NULL;
+ }
+
vm_event_ring_unlock(ved);
}
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 96bde65..7908844 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -10,6 +10,7 @@
#include <asm/mce.h>
#include <public/vcpu.h>
#include <public/hvm/hvm_info_table.h>
+#include <public/vm_event.h>
#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo)
#define is_pv_32bit_domain(d) ((d)->arch.is_32bit_pv)
@@ -508,6 +509,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/public/vm_event.h b/xen/include/public/vm_event.h
index aa22052..c6f69af 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 mem_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,12 +54,21 @@
* 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
* 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)
+/*
+ * 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 << 4)
/*
* Reasons for the vm event request
@@ -189,6 +198,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_* */
@@ -206,8 +221,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] 12+ messages in thread
* [PATCH V4 2/3] xen/vm_event: Support for guest-requested events
2015-07-08 10:22 [PATCH V4 0/3] Vm_event memory introspection helpers Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
@ 2015-07-08 10:22 ` Razvan Cojocaru
2015-07-08 12:41 ` George Dunlap
2015-07-08 10:22 ` [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
2 siblings, 1 reply; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-08 10:22 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>
---
Changes since V3:
- None, just addded acks.
---
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 | 16 ++++++++++++++++
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, 76 insertions(+), 8 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index d1d2ab3..4ce519a 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2384,6 +2384,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 63013de..d979122 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -105,3 +105,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 535d622..536d1c8 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5974,7 +5974,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;
@@ -6388,6 +6387,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 896acf7..f8df7d2 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -161,6 +161,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 7908844..f712caa 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -346,13 +346,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 bc45ea5..6812016 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1012,6 +1012,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_* */
@@ -1034,6 +1035,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 c6f69af..11e65c4 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -90,6 +90,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] 12+ messages in thread
* [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply
2015-07-08 10:22 [PATCH V4 0/3] Vm_event memory introspection helpers Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
@ 2015-07-08 10:22 ` Razvan Cojocaru
2015-07-08 12:18 ` Lengyel, Tamas
2 siblings, 1 reply; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-08 10:22 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>
---
Changes since V3:
- Renamed MEM_ACCESS_FLAG_DENY to VM_EVENT_FLAG_DENY (and fixed
the bit shift appropriately).
- Moved the DENY vm_event response logic from p2m.c to newly
added dedicated files for vm_event handling, as suggested
by Tamas Lengyel.
---
MAINTAINERS | 1 +
xen/arch/x86/Makefile | 1 +
xen/arch/x86/domain.c | 2 +
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 | 33 +++++++++++
xen/common/vm_event.c | 9 +++
xen/include/asm-arm/vm_event.h | 12 ++++
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 | 8 +++
xen/include/public/vm_event.h | 6 ++
18 files changed, 242 insertions(+), 46 deletions(-)
create mode 100644 xen/arch/x86/vm_event.c
create mode 100644 xen/include/asm-arm/vm_event.h
create mode 100644 xen/include/asm-x86/vm_event.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 6b1068e..59c0822 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -383,6 +383,7 @@ F: xen/common/vm_event.c
F: xen/common/mem_access.c
F: xen/arch/x86/hvm/event.c
F: xen/arch/x86/monitor.c
+F: xen/arch/x86/vm_event.c
XENTRACE
M: George Dunlap <george.dunlap@eu.citrix.com>
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 37e547c..5f24951 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -60,6 +60,7 @@ obj-y += machine_kexec.o
obj-y += crash.o
obj-y += tboot.o
obj-y += hpet.o
+obj-y += vm_event.o
obj-y += xstate.o
obj-$(crash_debug) += gdbstub.o
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a8fe046..c688ab9 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -678,6 +678,8 @@ void arch_domain_destroy(struct domain *d)
cleanup_domain_irq_mapping(d);
psr_free_rmid(d);
+
+ xfree(d->arch.event_write_data);
}
void arch_domain_shutdown(struct domain *d)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index c6ccb1f..780adb4 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1389,14 +1389,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;
}
@@ -1417,7 +1417,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 = ¤t->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 536d1c8..abfca33 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>
@@ -468,6 +469,35 @@ void hvm_do_resume(struct vcpu *v)
}
}
+ 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 )
{
@@ -3099,13 +3129,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));
@@ -3202,12 +3232,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);
@@ -3237,6 +3268,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 )
@@ -3303,7 +3350,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) )
@@ -3319,11 +3365,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]) )
@@ -3341,10 +3404,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:
@@ -3353,10 +3414,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) )
{
@@ -3384,8 +3446,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
@@ -3849,7 +3926,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;
@@ -4551,12 +4628,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 = ¤t->domain->arch;
HVMTRACE_3D(MSR_WRITE, msr,
(uint32_t)msr_content, (uint32_t)(msr_content >> 32));
@@ -4564,7 +4643,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 be5797a..07e3cad 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -274,7 +274,7 @@ 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 @@ 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 @@ 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 a02f983..5e39b88 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1945,7 +1945,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 fc29b89..96c55ec 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2010,9 +2010,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;
}
@@ -2024,7 +2031,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();
@@ -3035,7 +3042,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 ac6e3b3..52bf39c 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1048,15 +1048,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);
@@ -1249,15 +1250,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
new file mode 100644
index 0000000..aca731d
--- /dev/null
+++ b/xen/arch/x86/vm_event.c
@@ -0,0 +1,33 @@
+#include <asm/vm_event.h>
+
+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;
+ }
+ }
+}
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 11438da..eea035c 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -26,6 +26,7 @@
#include <xen/wait.h>
#include <xen/vm_event.h>
#include <xen/mem_access.h>
+#include <asm/vm_event.h>
#include <asm/p2m.h>
#include <xsm/xsm.h>
@@ -79,6 +80,10 @@ static int vm_event_enable(
}
}
+ if ( !d->arch.event_write_data )
+ d->arch.event_write_data = xzalloc_array(struct monitor_write_data,
+ d->max_vcpus);
+
rc = prepare_ring_for_helper(d, ring_gfn, &ved->ring_pg_struct,
&ved->ring_page);
if ( rc < 0 )
@@ -407,6 +412,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
new file mode 100644
index 0000000..5205ee8
--- /dev/null
+++ b/xen/include/asm-arm/vm_event.h
@@ -0,0 +1,12 @@
+#ifndef __ASM_ARM_VM_EVENT_H__
+#define __ASM_ARM_VM_EVENT_H__
+
+#include <xen/vm_event.h>
+
+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 f712caa..8990277 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -249,6 +249,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;
@@ -359,6 +374,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))
@@ -513,7 +530,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
new file mode 100644
index 0000000..bc6e9f7
--- /dev/null
+++ b/xen/include/asm-x86/vm_event.h
@@ -0,0 +1,8 @@
+#ifndef __ASM_X86_VM_EVENT_H__
+#define __ASM_X86_VM_EVENT_H__
+
+#include <xen/vm_event.h>
+
+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 11e65c4..0e175cc 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -69,6 +69,12 @@
* VM_EVENT_FLAG_SET_EMUL_READ_DATA are set, only the latter will be honored).
*/
#define VM_EVENT_FLAG_SET_EMUL_READ_DATA (1 << 4)
+ /*
+ * 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 << 5)
+
/*
* Reasons for the vm event request
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply
2015-07-08 10:22 ` [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
@ 2015-07-08 12:18 ` Lengyel, Tamas
2015-07-08 12:26 ` Razvan Cojocaru
0 siblings, 1 reply; 12+ messages in thread
From: Lengyel, Tamas @ 2015-07-08 12:18 UTC (permalink / raw
To: Razvan Cojocaru
Cc: Jun Nakajima, Wei Liu, kevin.tian, keir, Ian Campbell,
Stefano Stabellini, George Dunlap, Andrew Cooper, eddie.dong,
Xen-devel, Aravind.Gopalakrishnan, Jan Beulich,
suravee.suthikulpanit, boris.ostrovsky, Ian Jackson
[-- Attachment #1.1: Type: text/plain, Size: 989 bytes --]
On Wed, Jul 8, 2015 at 6:22 AM, Razvan Cojocaru <rcojocaru@bitdefender.com>
wrote:
> 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>
>
> ---
> Changes since V3:
> - Renamed MEM_ACCESS_FLAG_DENY to VM_EVENT_FLAG_DENY (and fixed
> the bit shift appropriately).
> - Moved the DENY vm_event response logic from p2m.c to newly
> added dedicated files for vm_event handling, as suggested
> by Tamas Lengyel.
>
This looks good to me. It will have to be rebased on staging once the other
series is merged as couple things will conflict. If this series lands first
however, the newly added asm/vm_event files lack the required license
header.
With that:
Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
[-- Attachment #1.2: Type: text/html, Size: 1655 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding
2015-07-08 10:22 ` [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
@ 2015-07-08 12:24 ` Lengyel, Tamas
2015-07-10 12:41 ` Jan Beulich
1 sibling, 0 replies; 12+ messages in thread
From: Lengyel, Tamas @ 2015-07-08 12:24 UTC (permalink / raw
To: Razvan Cojocaru
Cc: Jun Nakajima, Wei Liu, kevin.tian, keir, Ian Campbell,
Stefano Stabellini, George Dunlap, Andrew Cooper, eddie.dong,
Xen-devel, Aravind.Gopalakrishnan, Jan Beulich,
suravee.suthikulpanit, boris.ostrovsky, Ian Jackson
[-- Attachment #1.1: Type: text/plain, Size: 1869 bytes --]
On Wed, Jul 8, 2015 at 6:22 AM, Razvan Cojocaru <rcojocaru@bitdefender.com>
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 MEM_ACCESS_EMULATE or MEM_ACCESS_EMULATE_NOWRITE have
> been set to a vm_event response.
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> ---
> Changes since V3:
> - Renamed MEM_ACCESS_SET_EMUL_READ_DATA to
> VM_EVENT_FLAG_SET_EMUL_READ_DATA and updated its comment.
> - Removed xfree(v->arch.vm_event.emul_read_data) from
> free_vcpu_struct().
> - Returning X86EMUL_UNHANDLEABLE from hvmemul_cmpxchg() when
> !curr->arch.vm_event.emul_read_data.
> - Replaced in xmalloc_bytes() with xmalloc_array() in
> hvmemul_rep_outs_set_context().
> - Setting the rest of the buffer to zero in hvmemul_rep_movs()
> (no longer leaking heap contents).
> - No longer memset()ing the whole buffer before copy (just zeroing
> out the rest).
> - Moved hvmemul_ctxt->set_context = 0 to hvm_emulate_prepare() and
> removed hvm_emulate_one_set_context().
> ---
> tools/tests/xen-access/xen-access.c | 2 +-
> xen/arch/x86/hvm/emulate.c | 138
> ++++++++++++++++++++++++++++++++++-
> xen/arch/x86/hvm/event.c | 50 ++++++-------
> xen/arch/x86/mm/p2m.c | 92 +++++++++++++----------
> xen/common/domain.c | 2 +
> xen/common/vm_event.c | 23 ++++++
> xen/include/asm-x86/domain.h | 2 +
> xen/include/asm-x86/hvm/emulate.h | 10 ++-
> xen/include/public/vm_event.h | 31 ++++++--
> 9 files changed, 274 insertions(+), 76 deletions(-)
>
Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
[-- Attachment #1.2: Type: text/html, Size: 2530 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply
2015-07-08 12:18 ` Lengyel, Tamas
@ 2015-07-08 12:26 ` Razvan Cojocaru
2015-07-08 12:39 ` Lengyel, Tamas
0 siblings, 1 reply; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-08 12:26 UTC (permalink / raw
To: Lengyel, Tamas
Cc: Jun Nakajima, Wei Liu, kevin.tian, keir, Ian Campbell,
Stefano Stabellini, George Dunlap, Andrew Cooper, eddie.dong,
Xen-devel, Aravind.Gopalakrishnan, Jan Beulich,
suravee.suthikulpanit, boris.ostrovsky, Ian Jackson
On 07/08/2015 03:18 PM, Lengyel, Tamas wrote:
>
>
> On Wed, Jul 8, 2015 at 6:22 AM, Razvan Cojocaru
> <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote:
>
> 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
> <mailto:rcojocaru@bitdefender.com>>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com
> <mailto:george.dunlap@eu.citrix.com>>
> Acked-by: Jan Beulich <jbeulich@suse.com <mailto:jbeulich@suse.com>>
>
> ---
> Changes since V3:
> - Renamed MEM_ACCESS_FLAG_DENY to VM_EVENT_FLAG_DENY (and fixed
> the bit shift appropriately).
> - Moved the DENY vm_event response logic from p2m.c to newly
> added dedicated files for vm_event handling, as suggested
> by Tamas Lengyel.
>
>
> This looks good to me. It will have to be rebased on staging once the
> other series is merged as couple things will conflict. If this series
> lands first however, the newly added asm/vm_event files lack the
> required license header.
>
> With that:
> Acked-by: Tamas K Lengyel <tlengyel@novetta.com>
Thanks Tamas!
Are the license headers required? I just tried to make the change as
small as possible, and looking at the other headers (for example in
xen/include/asm-arm), at least half of them have no license header. I'm
guessing this is something we'd now like to start correcting in new patches?
Thanks,
Razvan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply
2015-07-08 12:26 ` Razvan Cojocaru
@ 2015-07-08 12:39 ` Lengyel, Tamas
2015-07-08 12:52 ` Razvan Cojocaru
0 siblings, 1 reply; 12+ messages in thread
From: Lengyel, Tamas @ 2015-07-08 12:39 UTC (permalink / raw
To: Razvan Cojocaru
Cc: Jun Nakajima, Wei Liu, kevin.tian, keir, Ian Campbell,
Stefano Stabellini, George Dunlap, Andrew Cooper, eddie.dong,
Xen-devel, Aravind.Gopalakrishnan, Jan Beulich,
suravee.suthikulpanit, boris.ostrovsky, Ian Jackson
[-- Attachment #1.1: Type: text/plain, Size: 793 bytes --]
>
> Are the license headers required? I just tried to make the change as
> small as possible, and looking at the other headers (for example in
> xen/include/asm-arm), at least half of them have no license header. I'm
> guessing this is something we'd now like to start correcting in new
> patches?
>
>
> Thanks,
> Razvan
>
The wiki's definition that goes with the Signed-off-by tag goes: "The
contribution was created in whole or in part by me and I have the right to
submit it under the open source license indicated in the file;". So, the
open source license should be indicated in the file. If it's a new file
being created, I would say it's the creators responsibility to add the
license. But I haven't seen any discussion/documentation on the matter so
I'm just guessing.
Cheers,
Tamas
[-- Attachment #1.2: Type: text/html, Size: 1190 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 2/3] xen/vm_event: Support for guest-requested events
2015-07-08 10:22 ` [PATCH V4 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
@ 2015-07-08 12:41 ` George Dunlap
0 siblings, 0 replies; 12+ messages in thread
From: George Dunlap @ 2015-07-08 12:41 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/08/2015 11:22 AM, Razvan Cojocaru wrote:
> 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>
>
> ---
> Changes since V3:
> - None, just addded acks.
And Razvan answered my question, so FWIW:
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> 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 | 16 ++++++++++++++++
> 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, 76 insertions(+), 8 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index d1d2ab3..4ce519a 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2384,6 +2384,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 63013de..d979122 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -105,3 +105,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 535d622..536d1c8 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5974,7 +5974,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;
> @@ -6388,6 +6387,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 896acf7..f8df7d2 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -161,6 +161,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 7908844..f712caa 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -346,13 +346,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 bc45ea5..6812016 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1012,6 +1012,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_* */
> @@ -1034,6 +1035,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 c6f69af..11e65c4 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -90,6 +90,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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply
2015-07-08 12:39 ` Lengyel, Tamas
@ 2015-07-08 12:52 ` Razvan Cojocaru
0 siblings, 0 replies; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-08 12:52 UTC (permalink / raw
To: Lengyel, Tamas
Cc: Jun Nakajima, Wei Liu, kevin.tian, keir, Ian Campbell,
Stefano Stabellini, George Dunlap, Andrew Cooper, eddie.dong,
Xen-devel, Aravind.Gopalakrishnan, Jan Beulich,
suravee.suthikulpanit, boris.ostrovsky, Ian Jackson
On 07/08/2015 03:39 PM, Lengyel, Tamas wrote:
>
>
> Are the license headers required? I just tried to make the change as
> small as possible, and looking at the other headers (for example in
> xen/include/asm-arm), at least half of them have no license header. I'm
> guessing this is something we'd now like to start correcting in new
> patches?
>
>
> Thanks,
> Razvan
>
>
> The wiki's definition that goes with the Signed-off-by tag goes: "The
> contribution was created in whole or in part by me and I have the right
> to submit it under the open source license indicated in the file;". So,
> the open source license should be indicated in the file. If it's a new
> file being created, I would say it's the creators responsibility to add
> the license. But I haven't seen any discussion/documentation on the
> matter so I'm just guessing.
That makes sense, thanks for pointing it out. I'm happy to resubmit with
the license header if it is required. I'm also obviously OK with the
license header you've provided, so if this series does land first and
you don't mind, feel free to add the original license text to the files.
Not a problem either way.
Thanks,
Razvan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding
2015-07-08 10:22 ` [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
2015-07-08 12:24 ` Lengyel, Tamas
@ 2015-07-10 12:41 ` Jan Beulich
2015-07-10 14:00 ` Razvan Cojocaru
1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2015-07-10 12:41 UTC (permalink / raw
To: Razvan Cojocaru
Cc: jun.nakajima, kevin.tian, wei.liu2, ian.campbell,
stefano.stabellini, george.dunlap, andrew.cooper3, ian.jackson,
xen-devel, eddie.dong, Aravind.Gopalakrishnan,
suravee.suthikulpanit, tlengyel, keir, boris.ostrovsky
>>> On 08.07.15 at 12:22, <rcojocaru@bitdefender.com> wrote:
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -653,6 +653,31 @@ 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) )
> + {
> + struct vcpu *curr = current;
> + unsigned int safe_bytes;
> +
> + if ( !curr->arch.vm_event.emul_read_data )
> + return X86EMUL_UNHANDLEABLE;
> +
> + safe_bytes = min_t(unsigned int,
> + bytes, curr->arch.vm_event.emul_read_data->size);
Afaict min() would work here.
> + if ( safe_bytes )
> + {
> + memcpy(p_data, curr->arch.vm_event.emul_read_data->data, safe_bytes);
Why is this conditional? Doesn't ->size being zero mean all data
should be zeroed? Otherwise this isn't really consistent behavior...
Also - long line.
> +
> + if ( bytes > safe_bytes )
> + memset(p_data + safe_bytes, 0, bytes - safe_bytes);
No need for the conditional here.
> @@ -893,6 +918,28 @@ 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) )
> + {
> + struct vcpu *curr = current;
> +
> + if ( curr->arch.vm_event.emul_read_data )
> + {
> + unsigned int safe_bytes = min_t(unsigned int, bytes,
> + curr->arch.vm_event.emul_read_data->size);
> +
> + memcpy(p_new, curr->arch.vm_event.emul_read_data->data,
> + safe_bytes);
> +
> + if ( bytes > safe_bytes )
> + memset(p_new + safe_bytes, 0, bytes - safe_bytes);
> + }
> + else
> + return X86EMUL_UNHANDLEABLE;
Please make this look as similar to hvmemul_read() as possible (invert
the if() condition etc). Once done, you'll be seeing that this would
better be factored out...
> @@ -935,6 +982,43 @@ 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;
> + struct vcpu *curr = current;
> + unsigned int safe_bytes;
> + char *buf = NULL;
Pointless initializer.
> @@ -1063,7 +1151,23 @@ static int hvmemul_rep_movs(
> */
> rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
> if ( rc == HVMCOPY_okay )
> + {
> + struct vcpu *curr = current;
> +
> + if ( unlikely(hvmemul_ctxt->set_context) &&
> + curr->arch.vm_event.emul_read_data )
Factoring out will also make obvious that here you're _still_ not
handling things consistently with the other cases.
> @@ -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 &&
Please parenthesize the &.
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -166,6 +166,7 @@ struct vcpu *alloc_vcpu(
> free_cpumask_var(v->cpu_hard_affinity_saved);
> free_cpumask_var(v->cpu_soft_affinity);
> free_cpumask_var(v->vcpu_dirty_cpumask);
> + xfree(v->arch.vm_event.emul_read_data);
> free_vcpu_struct(v);
> return NULL;
> }
> @@ -821,6 +822,7 @@ static void complete_domain_destroy(struct rcu_head *head)
> free_cpumask_var(v->cpu_hard_affinity_saved);
> free_cpumask_var(v->cpu_soft_affinity);
> free_cpumask_var(v->vcpu_dirty_cpumask);
> + xfree(v->arch.vm_event.emul_read_data);
Common code fiddling with arch-specific bits. I can't see how this
would build on ARM.
> @@ -63,6 +64,21 @@ static int vm_event_enable(
> vm_event_ring_lock_init(ved);
> vm_event_ring_lock(ved);
>
> + for_each_vcpu( d, v )
> + {
> + if ( v->arch.vm_event.emul_read_data )
> + continue;
> +
> + v->arch.vm_event.emul_read_data =
> + xmalloc(struct vm_event_emul_read_data);
> +
> + if ( !v->arch.vm_event.emul_read_data )
> + {
> + rc = -ENOMEM;
> + goto err;
> + }
> + }
Same here. You need to decide whether this is to be a generic
feature (then the pointer needs to move to struct vcpu) or x86
specific (then the code needs to move to x86-specific files).
> @@ -189,6 +198,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)];
Again, albeit here I can see no good alternative at this point.
Jan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding
2015-07-10 12:41 ` Jan Beulich
@ 2015-07-10 14:00 ` Razvan Cojocaru
0 siblings, 0 replies; 12+ messages in thread
From: Razvan Cojocaru @ 2015-07-10 14:00 UTC (permalink / raw
To: Jan Beulich
Cc: jun.nakajima, kevin.tian, wei.liu2, ian.campbell,
stefano.stabellini, george.dunlap, andrew.cooper3, ian.jackson,
xen-devel, eddie.dong, Aravind.Gopalakrishnan,
suravee.suthikulpanit, tlengyel, keir, boris.ostrovsky
On 07/10/2015 03:41 PM, Jan Beulich wrote:
>>>> On 08.07.15 at 12:22, <rcojocaru@bitdefender.com> wrote:
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -653,6 +653,31 @@ 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) )
>> + {
>> + struct vcpu *curr = current;
>> + unsigned int safe_bytes;
>> +
>> + if ( !curr->arch.vm_event.emul_read_data )
>> + return X86EMUL_UNHANDLEABLE;
>> +
>> + safe_bytes = min_t(unsigned int,
>> + bytes, curr->arch.vm_event.emul_read_data->size);
>
> Afaict min() would work here.
Ack.
>> + if ( safe_bytes )
>> + {
>> + memcpy(p_data, curr->arch.vm_event.emul_read_data->data, safe_bytes);
>
> Why is this conditional? Doesn't ->size being zero mean all data
> should be zeroed? Otherwise this isn't really consistent behavior...
>
> Also - long line.
Ack.
>> +
>> + if ( bytes > safe_bytes )
>> + memset(p_data + safe_bytes, 0, bytes - safe_bytes);
>
> No need for the conditional here.
Ack.
>> @@ -893,6 +918,28 @@ 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) )
>> + {
>> + struct vcpu *curr = current;
>> +
>> + if ( curr->arch.vm_event.emul_read_data )
>> + {
>> + unsigned int safe_bytes = min_t(unsigned int, bytes,
>> + curr->arch.vm_event.emul_read_data->size);
>> +
>> + memcpy(p_new, curr->arch.vm_event.emul_read_data->data,
>> + safe_bytes);
>> +
>> + if ( bytes > safe_bytes )
>> + memset(p_new + safe_bytes, 0, bytes - safe_bytes);
>> + }
>> + else
>> + return X86EMUL_UNHANDLEABLE;
>
> Please make this look as similar to hvmemul_read() as possible (invert
> the if() condition etc). Once done, you'll be seeing that this would
> better be factored out...
Very true, I should have factored it out from the beginning, I only have
not done so because there's been a somewhat convoluted trip to the
current state of the code. Ack.
>> @@ -935,6 +982,43 @@ 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;
>> + struct vcpu *curr = current;
>> + unsigned int safe_bytes;
>> + char *buf = NULL;
>
> Pointless initializer.
Ack.
>> @@ -1063,7 +1151,23 @@ static int hvmemul_rep_movs(
>> */
>> rc = hvm_copy_from_guest_phys(buf, sgpa, bytes);
>> if ( rc == HVMCOPY_okay )
>> + {
>> + struct vcpu *curr = current;
>> +
>> + if ( unlikely(hvmemul_ctxt->set_context) &&
>> + curr->arch.vm_event.emul_read_data )
>
> Factoring out will also make obvious that here you're _still_ not
> handling things consistently with the other cases.
True. Will fix it.
>> @@ -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 &&
>
> Please parenthesize the &.
Ack.
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -166,6 +166,7 @@ struct vcpu *alloc_vcpu(
>> free_cpumask_var(v->cpu_hard_affinity_saved);
>> free_cpumask_var(v->cpu_soft_affinity);
>> free_cpumask_var(v->vcpu_dirty_cpumask);
>> + xfree(v->arch.vm_event.emul_read_data);
>> free_vcpu_struct(v);
>> return NULL;
>> }
>> @@ -821,6 +822,7 @@ static void complete_domain_destroy(struct rcu_head *head)
>> free_cpumask_var(v->cpu_hard_affinity_saved);
>> free_cpumask_var(v->cpu_soft_affinity);
>> free_cpumask_var(v->vcpu_dirty_cpumask);
>> + xfree(v->arch.vm_event.emul_read_data);
>
> Common code fiddling with arch-specific bits. I can't see how this
> would build on ARM.
Indeed, I should have been more careful with the xfree() move.
>
>> @@ -63,6 +64,21 @@ static int vm_event_enable(
>> vm_event_ring_lock_init(ved);
>> vm_event_ring_lock(ved);
>>
>> + for_each_vcpu( d, v )
>> + {
>> + if ( v->arch.vm_event.emul_read_data )
>> + continue;
>> +
>> + v->arch.vm_event.emul_read_data =
>> + xmalloc(struct vm_event_emul_read_data);
>> +
>> + if ( !v->arch.vm_event.emul_read_data )
>> + {
>> + rc = -ENOMEM;
>> + goto err;
>> + }
>> + }
>
> Same here. You need to decide whether this is to be a generic
> feature (then the pointer needs to move to struct vcpu) or x86
> specific (then the code needs to move to x86-specific files).
I think the best bet is probably to move it to struct vcpu, especially
since Tamas intends to use the feature in new scenarios, so it's
probably best to not restrict it to x86. Tamas, what do you think?
Thanks,
Razvan
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-10 14:00 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-08 10:22 [PATCH V4 0/3] Vm_event memory introspection helpers Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 1/3] xen/mem_access: Support for memory-content hiding Razvan Cojocaru
2015-07-08 12:24 ` Lengyel, Tamas
2015-07-10 12:41 ` Jan Beulich
2015-07-10 14:00 ` Razvan Cojocaru
2015-07-08 10:22 ` [PATCH V4 2/3] xen/vm_event: Support for guest-requested events Razvan Cojocaru
2015-07-08 12:41 ` George Dunlap
2015-07-08 10:22 ` [PATCH V4 3/3] xen/vm_event: Deny register writes if refused by vm_event reply Razvan Cojocaru
2015-07-08 12:18 ` Lengyel, Tamas
2015-07-08 12:26 ` Razvan Cojocaru
2015-07-08 12:39 ` Lengyel, Tamas
2015-07-08 12:52 ` Razvan Cojocaru
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.