[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V3] vm_event: Allow subscribing to write events for specific MSR-s

On 15/04/16 18:12, Tamas K Lengyel wrote:

On Fri, Apr 15, 2016 at 3:02 AM, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> wrote:
Previously, subscribing to MSR write events was an all-or-none
approach, with special cases for introspection MSR-s. This patch
allows the vm_event consumer to specify exactly what MSR-s it is
interested in, and as a side-effect gets rid of the
vmx_introspection_force_enabled_msrs[] special case.
This replaces the previously posted "xen: Filter out MSR write
events" patch.

Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>

Changes since V2:
 - Introduced struct monitor_msr_bitmap as recommended by Andrew
   Cooper, which allowed removing some pointer arithmetic magic.
 - Removed arch_ prefix from monitor functions, as recommended
   by Tamas Lengyel.
 - Replaced the page allocation code with xzalloc() / xfree() for
   struct monitor_msr_bitmap.
 - Now returning -ENXIO instead of -EINVAL from the monitor
   functions, as recommended by Konrad Rzeszutek Wilk.
 tools/libxc/include/xenctrl.h      |  4 +-
 tools/libxc/xc_monitor.c           |  6 +--
 xen/arch/x86/hvm/event.c           |  3 +-
 xen/arch/x86/hvm/hvm.c             |  3 +-
 xen/arch/x86/hvm/vmx/vmcs.c        | 26 ++---------
 xen/arch/x86/hvm/vmx/vmx.c         | 10 ++--
 xen/arch/x86/monitor.c             | 95 +++++++++++++++++++++++++++++++++-----
 xen/arch/x86/vm_event.c            |  9 ++++
 xen/include/asm-x86/domain.h       |  4 +-
 xen/include/asm-x86/hvm/hvm.h      |  8 ++--
 xen/include/asm-x86/hvm/vmx/vmcs.h |  7 ---
 xen/include/asm-x86/monitor.h      |  8 ++++
 xen/include/public/domctl.h        |  5 +-
 13 files changed, 121 insertions(+), 67 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index f5a034a..9698d46 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2183,8 +2183,8 @@ 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);
-int xc_monitor_mov_to_msr(xc_interface *xch, domid_t domain_id, bool enable,
-                          bool extended_capture);
+int xc_monitor_mov_to_msr(xc_interface *xch, domid_t domain_id, uint32_t msr,
+                          bool enable);
So my only concern with this approach here is that the MSR index definitions that are supposed to be passed are never exported via a public header, are only defined in asm-x86/msr-index.h. Should that also be moved to be a public header as part of this patch?

The MSRs are specified by the Intel/AMD manuals.  Furthermore, for the non-architectural ones, it is quite possible that the same index maps to different MSRs on different hardware.  It is definitely the case that different hadware has the same MSR at different indices.  (The Intel cpuid masking MSRs have this propery across different CPU generations).

I expect the monitoring application to know the current hardware, and which MSRs are applicable.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.