As suggested by Andrew Cooper, this patch attempts to remove some redundancy and allow for an easier time when adding vm_events for new control registers in the future, by having a single VM_EVENT_REASON_WRITE_CTRLREG vm_event type, meant to serve CR0, CR3, CR4 and (newly introduced) XCR0. The actual control register will be deduced by the new .index field in vm_event_write_ctrlreg (renamed from vm_event_mov_to_cr). The patch has also modified the xen-access.c test - it is now able to log CR3 events.
Signed-off-by: Razvan Cojocaru <rcojoc...@bitdefender.com> --- Changes since V1: - VM_EVENT_REASON_MOV_TO_CR became VM_EVENT_REASON_WRITE_CTRLREG. - xc_monitor_mov_to_cr() became xc_monitor_write_ctrlreg(). - XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR became XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG. - Plugged in an actual event for VM_EVENT_X86_XCR0. - X86_CR0 became VM_EVENT_X86_CR0, and so on. --- tools/libxc/include/xenctrl.h | 9 ++---- tools/libxc/xc_monitor.c | 40 +++-------------------- tools/tests/xen-access/xen-access.c | 30 ++++++++++++++++-- xen/arch/x86/hvm/event.c | 50 +++++++---------------------- xen/arch/x86/hvm/hvm.c | 9 ++++-- xen/arch/x86/hvm/vmx/vmx.c | 4 +-- xen/arch/x86/monitor.c | 60 ++++++++++++----------------------- xen/include/asm-x86/domain.h | 20 ++++-------- xen/include/asm-x86/hvm/event.h | 4 +-- xen/include/public/domctl.h | 12 +++---- xen/include/public/vm_event.h | 27 +++++++++------- 11 files changed, 105 insertions(+), 160 deletions(-) diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index a689caf..d6008ff 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2355,12 +2355,9 @@ int xc_mem_access_disable_emulate(xc_interface *xch, domid_t domain_id); void *xc_monitor_enable(xc_interface *xch, domid_t domain_id, uint32_t *port); int xc_monitor_disable(xc_interface *xch, domid_t domain_id); int xc_monitor_resume(xc_interface *xch, domid_t domain_id); -int xc_monitor_mov_to_cr0(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly); -int xc_monitor_mov_to_cr3(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly); -int xc_monitor_mov_to_cr4(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly); +int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, + uint16_t index, bool enable, bool sync, + bool onchangeonly); int xc_monitor_mov_to_msr(xc_interface *xch, domid_t domain_id, bool enable, bool extended_capture); int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, bool enable); diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c index 87ad968..63013de 100644 --- a/tools/libxc/xc_monitor.c +++ b/tools/libxc/xc_monitor.c @@ -45,8 +45,9 @@ int xc_monitor_resume(xc_interface *xch, domid_t domain_id) NULL); } -int xc_monitor_mov_to_cr0(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly) +int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, + uint16_t index, bool enable, bool sync, + bool onchangeonly) { DECLARE_DOMCTL; @@ -54,39 +55,8 @@ int xc_monitor_mov_to_cr0(xc_interface *xch, domid_t domain_id, bool enable, 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_MOV_TO_CR0; - domctl.u.monitor_op.u.mov_to_cr.sync = sync; - domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly; - - return do_domctl(xch, &domctl); -} - -int xc_monitor_mov_to_cr3(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly) -{ - 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_MOV_TO_CR3; - domctl.u.monitor_op.u.mov_to_cr.sync = sync; - domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly; - - return do_domctl(xch, &domctl); -} - -int xc_monitor_mov_to_cr4(xc_interface *xch, domid_t domain_id, bool enable, - bool sync, bool onchangeonly) -{ - 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_MOV_TO_CR4; + domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG; + domctl.u.monitor_op.u.mov_to_cr.index = index; domctl.u.monitor_op.u.mov_to_cr.sync = sync; domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly; diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c index 12ab921..909aba6 100644 --- a/tools/tests/xen-access/xen-access.c +++ b/tools/tests/xen-access/xen-access.c @@ -318,9 +318,9 @@ static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp) void usage(char* progname) { fprintf(stderr, - "Usage: %s [-m] <domain_id> write|exec|breakpoint\n" + "Usage: %s [-m] <domain_id> write|exec|breakpoint|cr3\n" "\n" - "Logs first page writes, execs, or breakpoint traps that occur on the domain.\n" + "Logs first page writes, execs, CR3 writes or breakpoint traps that occur on the domain.\n" "\n" "-m requires this program to run, or else the domain may pause\n", progname); @@ -341,6 +341,7 @@ int main(int argc, char *argv[]) int required = 0; int breakpoint = 0; int shutting_down = 0; + int cr3 = 0; char* progname = argv[0]; argv++; @@ -383,6 +384,10 @@ int main(int argc, char *argv[]) { breakpoint = 1; } + else if ( !strcmp(argv[0], "cr3") ) + { + cr3 = 1; + } else { usage(argv[0]); @@ -443,6 +448,16 @@ int main(int argc, char *argv[]) } } + if ( cr3 ) + { + rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR3, 1, 1, 1); + if ( rc < 0 ) + { + ERROR("Error %d requesting CR3 monitoring with vm_event\n", rc); + goto exit; + } + } + /* Wait for access */ for (;;) { @@ -455,6 +470,7 @@ int main(int argc, char *argv[]) rc = xc_set_mem_access(xch, domain_id, XENMEM_access_rwx, START_PFN, (xenaccess->max_gpfn - START_PFN) ); rc = xc_monitor_software_breakpoint(xch, domain_id, 0); + rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR3, 0, 1, 1); shutting_down = 1; } @@ -546,6 +562,16 @@ int main(int argc, char *argv[]) } break; + case VM_EVENT_REASON_WRITE_CTRLREG: + if ( req.u.write_ctrlreg.index == VM_EVENT_X86_CR3 ) + printf("CR3: old 0x%016lx new 0x%016lx\n", + req.u.write_ctrlreg.new_value, + req.u.write_ctrlreg.old_value); + else + printf("Non-CR3 control register write event: 0x%08lx\n", + req.u.write_ctrlreg.index); + + break; default: fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason); } diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c index 9d5f9f3..cff855a 100644 --- a/xen/arch/x86/hvm/event.c +++ b/xen/arch/x86/hvm/event.c @@ -88,55 +88,29 @@ static int hvm_event_traps(uint8_t sync, vm_event_request_t *req) return 1; } -static inline -void hvm_event_cr(uint32_t reason, unsigned long value, - unsigned long old, bool_t onchangeonly, bool_t sync) +void hvm_event_cr(unsigned short index, unsigned long value, unsigned long old) { - if ( onchangeonly && value == old ) + struct arch_domain *currad = ¤t->domain->arch; + + if ( !(currad->monitor.write_ctrlreg_enabled & index) ) + return; + + if ( (currad->monitor.write_ctrlreg_onchangeonly & index) && value == old ) return; else { vm_event_request_t req = { - .reason = reason, + .reason = VM_EVENT_REASON_WRITE_CTRLREG, .vcpu_id = current->vcpu_id, - .u.mov_to_cr.new_value = value, - .u.mov_to_cr.old_value = old + .u.write_ctrlreg.index = index, + .u.write_ctrlreg.new_value = value, + .u.write_ctrlreg.old_value = old }; - hvm_event_traps(sync, &req); + hvm_event_traps(currad->monitor.write_ctrlreg_sync & index, &req); } } -void hvm_event_cr0(unsigned long value, unsigned long old) -{ - struct arch_domain *currad = ¤t->domain->arch; - - if ( currad->monitor.mov_to_cr0_enabled ) - hvm_event_cr(VM_EVENT_REASON_MOV_TO_CR0, value, old, - currad->monitor.mov_to_cr0_onchangeonly, - currad->monitor.mov_to_cr0_sync); -} - -void hvm_event_cr3(unsigned long value, unsigned long old) -{ - struct arch_domain *currad = ¤t->domain->arch; - - if ( currad->monitor.mov_to_cr3_enabled ) - hvm_event_cr(VM_EVENT_REASON_MOV_TO_CR3, value, old, - currad->monitor.mov_to_cr3_onchangeonly, - currad->monitor.mov_to_cr3_sync); -} - -void hvm_event_cr4(unsigned long value, unsigned long old) -{ - struct arch_domain *currad = ¤t->domain->arch; - - if ( currad->monitor.mov_to_cr4_enabled ) - hvm_event_cr(VM_EVENT_REASON_MOV_TO_CR4, value, old, - currad->monitor.mov_to_cr4_onchangeonly, - currad->monitor.mov_to_cr4_sync); -} - void hvm_event_msr(unsigned int msr, uint64_t value) { struct vcpu *curr = current; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 689e402..df83948 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2965,11 +2965,14 @@ out: int hvm_handle_xsetbv(u32 index, u64 new_bv) { struct segment_register sreg; + struct vcpu *curr = current; hvm_get_segment_register(current, x86_seg_ss, &sreg); if ( sreg.attr.fields.dpl != 0 ) goto err; + hvm_event_cr(VM_EVENT_X86_XCR0, new_bv, curr->arch.xcr0); + if ( handle_xsetbv(index, new_bv) ) goto err; @@ -3267,7 +3270,7 @@ int hvm_set_cr0(unsigned long value) hvm_funcs.handle_cd(v, value); hvm_update_cr(v, 0, value); - hvm_event_cr0(value, old_value); + hvm_event_cr(VM_EVENT_X86_CR0, value, old_value); if ( (value ^ old_value) & X86_CR0_PG ) { if ( !nestedhvm_vmswitch_in_progress(v) && nestedhvm_vcpu_in_guestmode(v) ) @@ -3308,7 +3311,7 @@ int hvm_set_cr3(unsigned long value) old=v->arch.hvm_vcpu.guest_cr[3]; v->arch.hvm_vcpu.guest_cr[3] = value; paging_update_cr3(v); - hvm_event_cr3(value, old); + hvm_event_cr(VM_EVENT_X86_CR3, value, old); return X86EMUL_OKAY; bad_cr3: @@ -3349,7 +3352,7 @@ int hvm_set_cr4(unsigned long value) } hvm_update_cr(v, 4, value); - hvm_event_cr4(value, old_cr); + hvm_event_cr(VM_EVENT_X86_CR4, value, old_cr); /* * Modifying CR4.{PSE,PAE,PGE,SMEP}, or clearing CR4.PCIDE diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 74f563f..9a7b924 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1262,7 +1262,7 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr) v->arch.hvm_vmx.exec_control |= cr3_ctls; /* Trap CR3 updates if CR3 memory events are enabled. */ - if ( v->domain->arch.monitor.mov_to_cr3_enabled ) + if ( v->domain->arch.monitor.write_ctrlreg_enabled & VM_EVENT_X86_CR3 ) v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING; vmx_update_cpu_exec_control(v); @@ -2010,7 +2010,7 @@ static int vmx_cr_access(unsigned long exit_qualification) unsigned long old = curr->arch.hvm_vcpu.guest_cr[0]; curr->arch.hvm_vcpu.guest_cr[0] &= ~X86_CR0_TS; vmx_update_guest_cr(curr, 0); - hvm_event_cr0(curr->arch.hvm_vcpu.guest_cr[0], old); + hvm_event_cr(VM_EVENT_X86_CR0, curr->arch.hvm_vcpu.guest_cr[0], old); HVMTRACE_0D(CLTS); break; } diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c index d7b1c18..6fdacbd 100644 --- a/xen/arch/x86/monitor.c +++ b/xen/arch/x86/monitor.c @@ -67,59 +67,39 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop) switch ( mop->event ) { - case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR0: + case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG: { - bool_t status = ad->monitor.mov_to_cr0_enabled; - - rc = status_check(mop, status); - if ( rc ) - return rc; - - ad->monitor.mov_to_cr0_sync = mop->u.mov_to_cr.sync; - ad->monitor.mov_to_cr0_onchangeonly = mop->u.mov_to_cr.onchangeonly; - - domain_pause(d); - ad->monitor.mov_to_cr0_enabled = !status; - domain_unpause(d); - break; - } - - case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR3: - { - bool_t status = ad->monitor.mov_to_cr3_enabled; + bool_t status = ad->monitor.write_ctrlreg_enabled & mop->u.mov_to_cr.index; struct vcpu *v; rc = status_check(mop, status); if ( rc ) return rc; - ad->monitor.mov_to_cr3_sync = mop->u.mov_to_cr.sync; - ad->monitor.mov_to_cr3_onchangeonly = mop->u.mov_to_cr.onchangeonly; + if ( mop->u.mov_to_cr.sync ) + ad->monitor.write_ctrlreg_sync |= mop->u.mov_to_cr.index; + else + ad->monitor.write_ctrlreg_sync &= ~mop->u.mov_to_cr.index; - domain_pause(d); - ad->monitor.mov_to_cr3_enabled = !status; - domain_unpause(d); + if ( mop->u.mov_to_cr.onchangeonly ) + ad->monitor.write_ctrlreg_onchangeonly |= mop->u.mov_to_cr.index; + else + ad->monitor.write_ctrlreg_onchangeonly &= mop->u.mov_to_cr.index; - /* Latches new CR3 mask through CR0 code */ - for_each_vcpu ( d, v ) - hvm_funcs.update_guest_cr(v, 0); - break; - } + domain_pause(d); - case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR4: - { - bool_t status = ad->monitor.mov_to_cr4_enabled; + if ( !status ) + ad->monitor.write_ctrlreg_enabled |= mop->u.mov_to_cr.index; + else + ad->monitor.write_ctrlreg_enabled &= ~mop->u.mov_to_cr.index; - rc = status_check(mop, status); - if ( rc ) - return rc; + domain_unpause(d); - ad->monitor.mov_to_cr4_sync = mop->u.mov_to_cr.sync; - ad->monitor.mov_to_cr4_onchangeonly = mop->u.mov_to_cr.onchangeonly; + if ( mop->u.mov_to_cr.index == VM_EVENT_X86_CR3 ) + /* Latches new CR3 mask through CR0 code */ + for_each_vcpu ( d, v ) + hvm_funcs.update_guest_cr(v, 0); - domain_pause(d); - ad->monitor.mov_to_cr4_enabled = !status; - domain_unpause(d); break; } diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index 45b5283..1dd49dd 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -341,19 +341,13 @@ struct arch_domain /* Monitor options */ struct { - uint16_t mov_to_cr0_enabled : 1; - uint16_t mov_to_cr0_sync : 1; - uint16_t mov_to_cr0_onchangeonly : 1; - uint16_t mov_to_cr3_enabled : 1; - uint16_t mov_to_cr3_sync : 1; - uint16_t mov_to_cr3_onchangeonly : 1; - uint16_t mov_to_cr4_enabled : 1; - uint16_t mov_to_cr4_sync : 1; - uint16_t mov_to_cr4_onchangeonly : 1; - 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; + uint32_t write_ctrlreg_enabled : 8; + uint32_t write_ctrlreg_sync : 8; + uint32_t write_ctrlreg_onchangeonly : 8; + uint32_t mov_to_msr_enabled : 1; + uint32_t mov_to_msr_extended : 1; + uint32_t singlestep_enabled : 1; + uint32_t software_breakpoint_enabled : 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 bb757a1..7d327f4 100644 --- a/xen/include/asm-x86/hvm/event.h +++ b/xen/include/asm-x86/hvm/event.h @@ -19,9 +19,7 @@ #define __ASM_X86_HVM_EVENT_H__ /* Called for current VCPU on crX/MSR changes by guest */ -void hvm_event_cr0(unsigned long value, unsigned long old); -void hvm_event_cr3(unsigned long value, unsigned long old); -void hvm_event_cr4(unsigned long value, unsigned long old); +void hvm_event_cr(unsigned short index, unsigned long value, unsigned long old); 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); diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 0c0ea4a..32629f9 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1034,12 +1034,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t); #define XEN_DOMCTL_MONITOR_OP_ENABLE 0 #define XEN_DOMCTL_MONITOR_OP_DISABLE 1 -#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR0 0 -#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR3 1 -#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_CR4 2 -#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 3 -#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 4 -#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 5 +#define XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG 0 +#define XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR 1 +#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2 +#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3 struct xen_domctl_monitor_op { uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */ @@ -1050,6 +1048,8 @@ struct xen_domctl_monitor_op { */ union { struct { + /* Which control register */ + uint8_t index; /* Pause vCPU until response */ uint8_t sync; /* Send event only on a change of value */ diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index c7426de..54560b4 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -60,22 +60,24 @@ #define VM_EVENT_REASON_MEM_SHARING 2 /* Memory paging event */ #define VM_EVENT_REASON_MEM_PAGING 3 -/* CR0 was updated */ -#define VM_EVENT_REASON_MOV_TO_CR0 4 -/* CR3 was updated */ -#define VM_EVENT_REASON_MOV_TO_CR3 5 -/* CR4 was updated */ -#define VM_EVENT_REASON_MOV_TO_CR4 6 +/* A control register was updated */ +#define VM_EVENT_REASON_WRITE_CTRLREG 4 /* An MSR was updated. */ -#define VM_EVENT_REASON_MOV_TO_MSR 7 +#define VM_EVENT_REASON_MOV_TO_MSR 5 /* Debug operation executed (e.g. int3) */ -#define VM_EVENT_REASON_SOFTWARE_BREAKPOINT 8 +#define VM_EVENT_REASON_SOFTWARE_BREAKPOINT 6 /* Single-step (e.g. MTF) */ -#define VM_EVENT_REASON_SINGLESTEP 9 +#define VM_EVENT_REASON_SINGLESTEP 7 + +/* Supported values for the vm_event_write_ctrlreg index. */ +#define VM_EVENT_X86_CR0 (1 << 0) +#define VM_EVENT_X86_CR3 (1 << 1) +#define VM_EVENT_X86_CR4 (1 << 2) +#define VM_EVENT_X86_XCR0 (1 << 3) /* * Using a custom struct (not hvm_hw_cpu) so as to not fill - * the mem_event ring buffer too quickly. + * the vm_event ring buffer too quickly. */ struct vm_event_regs_x86 { uint64_t rax; @@ -156,7 +158,8 @@ struct vm_event_mem_access { uint32_t _pad; }; -struct vm_event_mov_to_cr { +struct vm_event_write_ctrlreg { + uint64_t index; uint64_t new_value; uint64_t old_value; }; @@ -196,7 +199,7 @@ typedef struct vm_event_st { struct vm_event_paging mem_paging; struct vm_event_sharing mem_sharing; struct vm_event_mem_access mem_access; - struct vm_event_mov_to_cr mov_to_cr; + struct vm_event_write_ctrlreg write_ctrlreg; struct vm_event_mov_to_msr mov_to_msr; struct vm_event_debug software_breakpoint; struct vm_event_debug singlestep; -- 1.7.9.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel