| Message ID | 1495806118-12223-2-git-send-email-ppircalabu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 26.05.17 at 15:41, <ppircalabu@bitdefender.com> wrote: > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -37,7 +37,7 @@ > #include "hvm/save.h" > #include "memory.h" > > -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d > +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e > > /* > * NB. xen_domctl.domain is an IN/OUT parameter for this operation. > @@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op { > uint8_t sync; > /* Send event only on a change of value */ > uint8_t onchangeonly; > + /* Send event only if the changed bit in the control register > + * is not masked > + */ > + unsigned long bitmask; You can't use "unsigned long" in public headers. Also beware of alignment issues between 32-bit and 64-bit callers (I didn't check whether there actually is any, I'm merely hinting towards the use of uint64_aligned_t). > --- a/xen/include/public/vm_event.h > +++ b/xen/include/public/vm_event.h > @@ -155,6 +155,15 @@ > #define VM_EVENT_X86_CR4 2 > #define VM_EVENT_X86_XCR0 3 > > +/* vm_event_write_ctrlreg default bit mask > + * If the changed bit in the control register is masked the event is > + * not triggered > + */ Comment style. > +#define VM_EVENT_X86_CR0_DEFAULT_MASK 0x00000000 > +#define VM_EVENT_X86_CR3_DEFAULT_MASK 0x00000000 > +#define VM_EVENT_X86_CR4_DEFAULT_MASK 0x00000080 > +#define VM_EVENT_X86_XCR0_DEFAULT_MASK 0x00000000 These are unused - what are they good for? And why would you want to special case CR4.PGE (and even without any comment)? Jan
On Lu, 2017-05-29 at 09:01 -0600, Jan Beulich wrote: > > > > > > > > > > > > > On 26.05.17 at 15:41, <ppircalabu@bitdefender.com> wrote: > > --- a/xen/include/public/domctl.h > > +++ b/xen/include/public/domctl.h > > @@ -37,7 +37,7 @@ > > #include "hvm/save.h" > > #include "memory.h" > > > > -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d > > +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e > > > > /* > > * NB. xen_domctl.domain is an IN/OUT parameter for this > > operation. > > @@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op { > > uint8_t sync; > > /* Send event only on a change of value */ > > uint8_t onchangeonly; > > + /* Send event only if the changed bit in the control > > register > > + * is not masked > > + */ > > + unsigned long bitmask; > You can't use "unsigned long" in public headers. Also beware of > alignment issues between 32-bit and 64-bit callers (I didn't check > whether there actually is any, I'm merely hinting towards the > use of uint64_aligned_t). > I will change it to uint64_aligned_t and resubmit the patch. > > > > --- a/xen/include/public/vm_event.h > > +++ b/xen/include/public/vm_event.h > > @@ -155,6 +155,15 @@ > > #define VM_EVENT_X86_CR4 2 > > #define VM_EVENT_X86_XCR0 3 > > > > +/* vm_event_write_ctrlreg default bit mask > > + * If the changed bit in the control register is masked the event > > is > > + * not triggered > > + */ > Comment style. > > > > > +#define VM_EVENT_X86_CR0_DEFAULT_MASK 0x00000000 > > +#define VM_EVENT_X86_CR3_DEFAULT_MASK 0x00000000 > > +#define VM_EVENT_X86_CR4_DEFAULT_MASK 0x00000080 > > +#define VM_EVENT_X86_XCR0_DEFAULT_MASK 0x00000000 > These are unused - what are they good for? And why would > you want to special case CR4.PGE (and even without any > comment)? > > Jan > The defaults were added because when the introspection engine requests CR4 events it will get too many as CR4.PGE is used for global TLB flushes. However, every client can set-up it's own event masking policy so these macros are not needed globally. > > ________________________ > This email was scanned by Bitdefender Many thanks for your comments, Petre ________________________ This email was scanned by Bitdefender
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 1629f41..8c26cb4 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -1999,7 +1999,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id, uint32_t *capabilities); int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, uint16_t index, bool enable, bool sync, - bool onchangeonly); + uint64_t bitmask, bool onchangeonly); /* * A list of MSR indices can usually be found in /usr/include/asm/msr-index.h. * Please consult the Intel/AMD manuals for more information on diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c index f99b6e3..70648d7 100644 --- a/tools/libxc/xc_monitor.c +++ b/tools/libxc/xc_monitor.c @@ -70,7 +70,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id, int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, uint16_t index, bool enable, bool sync, - bool onchangeonly) + uint64_t bitmask, bool onchangeonly) { DECLARE_DOMCTL; @@ -82,6 +82,7 @@ int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id, 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; + domctl.u.monitor_op.u.mov_to_cr.bitmask = bitmask; return do_domctl(xch, &domctl); } diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c index bde5fd0..a7ccfc4 100644 --- a/xen/arch/x86/hvm/monitor.c +++ b/xen/arch/x86/hvm/monitor.c @@ -38,7 +38,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) && (!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) || - value != old) ) + value != old) && + (!((value ^ old) & ad->monitor.write_ctrlreg_mask[index])) ) { bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask); diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c index 449c64c..d02d2ea 100644 --- a/xen/arch/x86/monitor.c +++ b/xen/arch/x86/monitor.c @@ -155,9 +155,15 @@ int arch_monitor_domctl_event(struct domain *d, ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask; if ( requested_status ) + { + ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask; ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask; + } else + { + ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0; ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask; + } if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index ) { diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index 924caac..27d80ee 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -406,6 +406,7 @@ struct arch_domain unsigned int cpuid_enabled : 1; unsigned int descriptor_access_enabled : 1; struct monitor_msr_bitmap *msr_bitmap; + uint64_t write_ctrlreg_mask[4]; } monitor; /* Mem_access emulation control */ diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index e6cf211..2bf5c5b 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -37,7 +37,7 @@ #include "hvm/save.h" #include "memory.h" -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e /* * NB. xen_domctl.domain is an IN/OUT parameter for this operation. @@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op { uint8_t sync; /* Send event only on a change of value */ uint8_t onchangeonly; + /* Send event only if the changed bit in the control register + * is not masked + */ + unsigned long bitmask; } mov_to_cr; struct { diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index f01e471..8a77d4d 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -155,6 +155,15 @@ #define VM_EVENT_X86_CR4 2 #define VM_EVENT_X86_XCR0 3 +/* vm_event_write_ctrlreg default bit mask + * If the changed bit in the control register is masked the event is + * not triggered + */ +#define VM_EVENT_X86_CR0_DEFAULT_MASK 0x00000000 +#define VM_EVENT_X86_CR3_DEFAULT_MASK 0x00000000 +#define VM_EVENT_X86_CR4_DEFAULT_MASK 0x00000080 +#define VM_EVENT_X86_XCR0_DEFAULT_MASK 0x00000000 + /* * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM * so as to not fill the vm_event ring buffer too quickly.
Add support for filtering out the write_ctrlreg monitor events if they are generated only by changing certains bits. A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg function in order to mask the event generation if the changed bits are set. Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com> --- tools/libxc/include/xenctrl.h | 2 +- tools/libxc/xc_monitor.c | 3 ++- xen/arch/x86/hvm/monitor.c | 3 ++- xen/arch/x86/monitor.c | 6 ++++++ xen/include/asm-x86/domain.h | 1 + xen/include/public/domctl.h | 6 +++++- xen/include/public/vm_event.h | 9 +++++++++ 7 files changed, 26 insertions(+), 4 deletions(-)