[Xen-devel] xc_hvm_inject_trap() races

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

Hello,

We've stumbled across the following scenario: we're injecting a #PF to
try to bring a swapped page back, but Xen already have a pending
interrupt, and the two collide.

I've logged what happens in hvm_do_resume() at the point of injection,
and stumbled across this:

(XEN) [  252.878389] vector: 14, type: 3, error_code: 0,
VM_ENTRY_INTR_INFO: 0x00000000800000e1

VM_ENTRY_INTR_INFO does have INTR_INFO_VALID_MASK set here.

This obviously leads to all sorts of unpleasantness with the guest.
Ideally, the injected #PF should take precedence, and the other
interrupt should not be lost as well. Suggestions on how best to solve
this are welcome and appreciated.

This problem would also occur with reinjected breakpoint events (please
see xen-access.c), which also use xc_hvm_inject_trap().

The easiest approach would be to simply ignore the injected trap while
VM_ENTRY_INTR_INFO & INTR_INFO_VALID_MASK, like this:

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 2d78e45..cfe04b4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -532,8 +532,16 @@ void hvm_do_resume(struct vcpu *v)
     /* Inject pending hw/sw trap */
     if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
     {
-        hvm_inject_trap(&v->arch.hvm_vcpu.inject_trap);
-        v->arch.hvm_vcpu.inject_trap.vector = -1;
+        unsigned long ev;
+
+        __vmread(VM_ENTRY_INTR_INFO, &ev);
+
+        /* Check for already pending interrupts (races). */
+        if ( !(ev & INTR_INFO_VALID_MASK) )
+        {
+            hvm_inject_trap(&v->arch.hvm_vcpu.inject_trap);
+            v->arch.hvm_vcpu.inject_trap.vector = -1;
+        }
     }
 }

However, this does not even guarantee delayed delivery of the trap,
since by the time the next hvm_do_resume() call happens where there's no
other pending interrupt we could have called xc_hvm_inject_trap() with
different values, and have overwritten v->arch.hvm_vcpu.inject_trap.
More importantly, the context might be different then and the injection
would fail (or mess things up) because of that.

So we'd be better off simply discarding the trap in this case (as has
been suggested by Andrew), but here again the xc_hvm_inject_trap()
caller has no clue whether the call succeeded or failed. This is
inefficient (and thus comes with a performance impact) since the guest
would be put in a loop until injection finally succeeds, and finally
makes client applications significantly more cumbersome to implement,
because they would need to guess when the injection failed (since
xc_hvm_inject_trap() won't tell us), and handle duplicates for these
corner cases.

Is there an elegant way to fix this that keeps both interrupts but makes
the injected one higher priority?


Thanks,
Razvan

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