Emulation and active (valid) interrupts

* Emulation and active (valid) interrupts
@ 2018-08-08 14:26 Razvan Cojocaru
  2018-08-08 15:01 ` Razvan Cojocaru
  2018-08-09  7:54 ` Jan Beulich
  0 siblings, 2 replies; 15+ messages in thread
From: Razvan Cojocaru @ 2018-08-08 14:26 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper

[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]

Hello,

I'm seeing rare occasions where this backtrace occurs at a point in
__vmx_inject_exception() where there's already a valid interrupt set up
in VM_ENTRY_INTR_INFO (that is, once the value is __vmread(), intr_info
& INTR_INFO_VALID_MASK is non-zero):

Xen call trace:
   [<ffff82d0801fe9c7>] vmx.c#__vmx_inject_exception+0x47/0xd9
   [<ffff82d080201c6a>] vmx.c#vmx_inject_trap+0x1e1/0x29f
   [<ffff82d0801d6b8a>] hvm_inject_trap+0xb0/0xb5
   [<ffff82d0801d6be7>] hvm_inject_page_fault+0x2a/0x2c
   [<ffff82d0801d6cc6>] hvm.c#__hvm_copy+0xdd/0x37f
   [<ffff82d0801d88fa>] hvm_copy_from_guest_virt+0x14/0x16
   [<ffff82d0801d28e3>] emulate.c#__hvmemul_read+0x12f/0x19f
   [<ffff82d0801d2a2c>] emulate.c#hvmemul_read+0x28/0x2a
   [<ffff82d0801a931c>] x86_emulate.c#read_ulong+0x13/0x15
   [<ffff82d0801ab412>] x86_emulate+0x16b1/0x11405
   [<ffff82d0801d180a>] emulate.c#_hvm_emulate_one+0x188/0x2bc
   [<ffff82d0801d1a34>] hvm_emulate_one+0x10/0x12
   [<ffff82d0801d2d39>] hvm_mem_access_emulate_one+0x7a/0xdd
   [<ffff82d0801dbe50>] hvm_do_resume+0x246/0x3d3
   [<ffff82d0801fbddb>] vmx_do_resume+0x102/0x119
   [<ffff82d080170ba3>] context_switch+0xf52/0xfad
   [<ffff82d08013182c>] schedule.c#schedule+0x753/0x792
   [<ffff82d080134f05>] softirq.c#__do_softirq+0x85/0x90
   [<ffff82d080134f5a>] do_softirq+0x13/0x15
   [<ffff82d08016b5f2>] domain.c#idle_loop+0x61/0x6e

(this is a Xen 4.7.5 trace, but it applies to staging as well).

This was the initial culprit:

[<ffff82d08031b55d>] vmx.c#__vmx_inject_exception+0xa1/0xda
[<ffff82d08031eb5c>] vmx_inject_extint+0x94/0x9f
[<ffff82d080315a0a>] vmx_intr_assist+0x4ee/0x5ad
[<ffff82d0803258ff>] vmx_asm_vmexit_handler+0xff/0x270

and I thought I'd fixed it by treating the emulation in a similar manner
to the single-step case: have vmx_intr_assist() block interrupts for the
duration of the emulation (please see the attached patch for staging).
However, a bit more rarely this time, we still end up overwriting an
interrupt via the above code path.

Obviously this works only if nothing has been written in
VM_ENTRY_INTR_INFO _before_ we block (return) in vmx_intr_assist().

My questions are:

1. Is it possible to already have a valid interrupt written in
VM_ENTRY_INTR_INFO at EXIT_REASON_EPT_VIOLATION-time in
vmx_vmexit_handler()?

2. Is it possible that something else in that path writes into
VM_ENTRY_INTR_INFO (which the vmx_intr_assist() logic can't possibly
prevent)? For example, in my Xen 4.7.5 sources, there's a
pt_restore_timer(v); call in hvm_do_resume() before the vm_event
emulation code.


Thanks,
Razvan

[-- Attachment #2: emulate_intr_block.patch --]
[-- Type: text/x-patch, Size: 2549 bytes --]

diff --git a/xen/arch/x86/hvm/vm_event.c b/xen/arch/x86/hvm/vm_event.c
index 28d08a6..8e7c737 100644
--- a/xen/arch/x86/hvm/vm_event.c
+++ b/xen/arch/x86/hvm/vm_event.c
@@ -124,6 +124,8 @@ void hvm_vm_event_do_resume(struct vcpu *v)
 
         w->do_write.msr = 0;
     }
+
+    v->arch.vm_event->intr_block = false;
 }
 
 /*
diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index eb9b288..97cecbf 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -22,6 +22,7 @@
 #include <xen/errno.h>
 #include <xen/trace.h>
 #include <xen/event.h>
+#include <xen/vm_event.h>
 #include <asm/apicdef.h>
 #include <asm/current.h>
 #include <asm/cpufeature.h>
@@ -37,6 +38,7 @@
 #include <asm/hvm/nestedhvm.h>
 #include <public/hvm/ioreq.h>
 #include <asm/hvm/trace.h>
+#include <asm/vm_event.h>
 
 /*
  * A few notes on virtual NMI and INTR delivery, and interactions with
@@ -231,6 +233,11 @@ void vmx_intr_assist(void)
     enum hvm_intblk intblk;
     int pt_vector;
 
+    if ( unlikely(v->arch.vm_event) &&
+         vm_event_check_ring(v->domain->vm_event_monitor) &&
+         v->arch.vm_event->intr_block )
+        return;
+
     /* Block event injection when single step with MTF. */
     if ( unlikely(v->arch.hvm_vcpu.single_step) )
     {
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index c606683..4263929 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -113,6 +113,12 @@ int monitor_traps(struct vcpu *v, bool sync, vm_event_request_t *req)
     if ( sync )
     {
         req->flags |= VM_EVENT_FLAG_VCPU_PAUSED;
+        /*
+         * It only makes sense to block interrupts for the duration of
+         * processing blocking vm_events, since that is the only case where
+         * emulating the current instruction really applies.
+         */
+        v->arch.vm_event->intr_block = true;
         vm_event_vcpu_pause(v);
         rc = 1;
     }
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 39e73c8..2b36614 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -34,6 +34,12 @@ struct arch_vm_event {
     struct monitor_write_data write_data;
     struct vm_event_regs_x86 gprs;
     bool set_gprs;
+    /*
+     * Block interrupts until this vm_event is done handling (after the
+     * fashion of single-step). Meant for the cases where the vm_event
+     * reply asks for emulation of the current instruction.
+     */
+    bool intr_block;
 };
 
 int vm_event_init_domain(struct domain *d);

[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread