[Xen-devel] [PATCH 1/2] x86/monitor: Introduce a boolean to suppress nested monitoring events

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

When applying vm_event actions, monitoring events can nest and effectively
livelock the vcpu.  Introduce a boolean which is set for a specific portion of
the hvm_do_resume() path, which causes the hvm_monitor_* helpers to exit
early.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>

RFC - This probably wants wiring up on ARM as well, but all I see is
monitor_smc() and no equivalent parts to hvm_do_resume() where we may inject
an exception from a vm_event.  Should this be included in non-x86 monitor
functions for consistency at this point?
---
 xen/arch/x86/hvm/hvm.c     |  8 ++++++++
 xen/arch/x86/hvm/monitor.c | 24 ++++++++++++++++++++++--
 xen/include/xen/sched.h    |  8 ++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 9bc8078..4b4d9d6 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -512,6 +512,12 @@ void hvm_do_resume(struct vcpu *v)
     if ( !handle_hvm_io_completion(v) )
         return;
 
+    /*
+     * We are about to apply actions requested by the introspection
+     * agent.  Don't trigger further monitoring.
+     */
+    v->monitor.suppress = true;
+
     if ( unlikely(v->arch.vm_event) )
         hvm_vm_event_do_resume(v);
 
@@ -526,6 +532,8 @@ void hvm_do_resume(struct vcpu *v)
         v->arch.hvm.inject_event.vector = HVM_EVENT_VECTOR_UNSET;
     }
 
+    v->monitor.suppress = false;
+
     if ( unlikely(v->arch.vm_event) && v->arch.monitor.next_interrupt_enabled )
     {
         struct x86_event info;
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 2a41ccc..f1a196f 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -36,6 +36,9 @@ bool hvm_monitor_cr(unsigned int index, unsigned long value, 
unsigned long old)
     struct arch_domain *ad = &curr->domain->arch;
     unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(index);
 
+    if ( curr->monitor.suppress )
+        return 0;
+
     if ( index == VM_EVENT_X86_CR3 && hvm_pcid_enabled(curr) )
         value &= ~X86_CR3_NOFLUSH; /* Clear the noflush bit. */
 
@@ -73,6 +76,9 @@ bool hvm_monitor_emul_unimplemented(void)
         .vcpu_id  = curr->vcpu_id,
     };
 
+    if ( curr->monitor.suppress )
+        return false;
+
     return curr->domain->arch.monitor.emul_unimplemented_enabled &&
         monitor_traps(curr, true, &req) == 1;
 }
@@ -81,6 +87,9 @@ void hvm_monitor_msr(unsigned int msr, uint64_t new_value, 
uint64_t old_value)
 {
     struct vcpu *curr = current;
 
+    if ( curr->monitor.suppress )
+        return;
+
     if ( monitored_msr(curr->domain, msr) &&
          (!monitored_msr_onchangeonly(curr->domain, msr) ||
            new_value != old_value) )
@@ -100,12 +109,16 @@ void hvm_monitor_descriptor_access(uint64_t exit_info,
                                    uint64_t vmx_exit_qualification,
                                    uint8_t descriptor, bool is_write)
 {
+    struct vcpu *curr = current;
     vm_event_request_t req = {
         .reason = VM_EVENT_REASON_DESCRIPTOR_ACCESS,
         .u.desc_access.descriptor = descriptor,
         .u.desc_access.is_write = is_write,
     };
 
+    if ( curr->monitor.suppress )
+        return;
+
     if ( cpu_has_vmx )
     {
         req.u.desc_access.arch.vmx.instr_info = exit_info;
@@ -116,7 +129,7 @@ void hvm_monitor_descriptor_access(uint64_t exit_info,
         req.u.desc_access.arch.svm.exitinfo = exit_info;
     }
 
-    monitor_traps(current, true, &req);
+    monitor_traps(curr, true, &req);
 }
 
 static inline unsigned long gfn_of_rip(unsigned long rip)
@@ -146,6 +159,9 @@ int hvm_monitor_debug(unsigned long rip, enum 
hvm_monitor_debug_type type,
     vm_event_request_t req = {};
     bool sync;
 
+    if ( curr->monitor.suppress )
+        return 0;
+
     switch ( type )
     {
     case HVM_MONITOR_SOFTWARE_BREAKPOINT:
@@ -204,6 +220,7 @@ int hvm_monitor_cpuid(unsigned long insn_length, unsigned 
int leaf,
 void hvm_monitor_interrupt(unsigned int vector, unsigned int type,
                            unsigned int err, uint64_t cr2)
 {
+    struct vcpu *curr = current;
     vm_event_request_t req = {
         .reason = VM_EVENT_REASON_INTERRUPT,
         .u.interrupt.x86.vector = vector,
@@ -212,7 +229,10 @@ void hvm_monitor_interrupt(unsigned int vector, unsigned 
int type,
         .u.interrupt.x86.cr2 = cr2,
     };
 
-    monitor_traps(current, 1, &req);
+    if ( curr->monitor.suppress )
+        return;
+
+    monitor_traps(curr, 1, &req);
 }
 
 /*
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3171eab..b32089a 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -268,6 +268,14 @@ struct vcpu
 
     struct evtchn_fifo_vcpu *evtchn_fifo;
 
+    struct {
+        /*
+         * Suppress monitoring events.  Used to prevent vm_event-generated
+         * actions causing new monitoring events, and livelock the vcpu.
+         */
+        bool suppress;
+    } monitor;
+
     /* vPCI per-vCPU area, used to store data for long running operations. */
     struct vpci_vcpu vpci;
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel