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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 1fec412..1be058a 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -20,6 +20,7 @@
  */

 #include <asm/monitor.h>
+#include <asm/vm_event.h>
 #include <public/vm_event.h>

 int arch_monitor_domctl_event(struct domain *d,
@@ -77,25 +78,25 @@ int arch_monitor_domctl_event(struct domain *d,

     case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR:
     {
-        bool_t old_status = ad->monitor.mov_to_msr_enabled;
+        bool_t old_status;
+        int rc;
+        u32 msr = mop->u.mov_to_msr.msr;

-        if ( unlikely(old_status == requested_status) )
-            return -EEXIST;
+        domain_pause(d);

-        if ( requested_status && mop->u.mov_to_msr.extended_capture &&
-             !hvm_enable_msr_exit_interception(d) )
-            return -EOPNOTSUPP;
+        old_status = vm_event_is_msr_enabled(d, msr);

-        domain_pause(d);
+        if ( unlikely(old_status == requested_status) )
+            return -EEXIST;

Moving this test after the domain gets paused will leave the guest paused on error condition. Any reason why this rearrangement is necessary?

 

-        if ( requested_status && mop->u.mov_to_msr.extended_capture )
-            ad->monitor.mov_to_msr_extended = 1;
+        if ( requested_status )
+            rc = vm_event_enable_msr(d, msr);
         else
-            ad->monitor.mov_to_msr_extended = 0;
+            rc = vm_event_disable_msr(d, msr);

-        ad->monitor.mov_to_msr_enabled = requested_status;
         domain_unpause(d);
-        break;
+
+        return rc;
     }

     case XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP:
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 5635603..b851e39 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -27,6 +27,13 @@ int vm_event_init_domain(struct domain *d)
 {
     struct vcpu *v;

+    d->arch.monitor_msr_bitmap = alloc_xenheap_page();
+
+    if ( !d->arch.monitor_msr_bitmap )
+        return -ENOMEM;
+
+    memset(d->arch.monitor_msr_bitmap, 0, PAGE_SIZE);
+
     for_each_vcpu ( d, v )
     {
         if ( v->arch.vm_event )
@@ -55,11 +62,66 @@ void vm_event_cleanup_domain(struct domain *d)
         v->arch.vm_event = NULL;
     }

+    free_xenheap_page(d->arch.monitor_msr_bitmap);
+    d->arch.monitor_msr_bitmap = NULL;
+
     d->arch.mem_access_emulate_each_rep = 0;
     memset(&d->arch.monitor, 0, sizeof(d->arch.monitor));
     memset(&d->monitor, 0, sizeof(d->monitor));
 }

+int vm_event_enable_msr(struct domain *d, u32 msr)

This function..

 

+{
+    if ( !d->arch.monitor_msr_bitmap )
+        return -EINVAL;
+
+    if ( msr <= 0x1fff )
+        set_bit(msr, d->arch.monitor_msr_bitmap + 0x000/BYTES_PER_LONG);
+    else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
+    {
+        msr &= 0x1fff;
+        set_bit(msr, d->arch.monitor_msr_bitmap + 0x400/BYTES_PER_LONG);
+    }
+
+    hvm_enable_msr_interception(d, msr);
+
+    return 0;
+}
+
+int vm_event_disable_msr(struct domain *d, u32 msr)

..and this..

 

+{
+    if ( !d->arch.monitor_msr_bitmap )
+        return -EINVAL;
+
+    if ( msr <= 0x1fff )
+        clear_bit(msr, d->arch.monitor_msr_bitmap + 0x000/BYTES_PER_LONG);
+    else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
+    {
+        msr &= 0x1fff;
+        clear_bit(msr, d->arch.monitor_msr_bitmap + 0x400/BYTES_PER_LONG);
+    }
+
+    return 0;
+}
+
+bool_t vm_event_is_msr_enabled(const struct domain *d, u32 msr)

 

..and this one has nothing to do with vm_event really. These belong in the monitor.c side.

 

+{
+    bool_t rc = 0;
+
+    if ( !d->arch.monitor_msr_bitmap )
+        return 0;
+
+    if ( msr <= 0x1fff )
+        rc = test_bit(msr, d->arch.monitor_msr_bitmap + 0x000/BYTES_PER_LONG);
+    else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
+    {
+        msr &= 0x1fff;
+        rc = test_bit(msr, d->arch.monitor_msr_bitmap + 0x400/BYTES_PER_LONG);
+    }
+
+    return rc;
+}
+
 void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
 {
     if ( !is_hvm_domain(d) || !atomic_read(&v->vm_event_pause_count) )

Cheers,

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel