[Xen-devel] [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs APIC

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

It appears there is a case where Windows enables the APIC assist
enlightenment (see Microsoft Hypervisor Top Level Functional Spec. 5.0b,
section 10.3.5) but does not use it. This scenario is perfectly valid
according to the documentation, but causes the state machine in Xen to
become confused leading to a domain_crash() such as the following:

(XEN) d4: VIRIDIAN GUEST_OS_ID: vendor: 1 os: 4 major: 6 minor: 1 sp: 0
      build: 1db0
(XEN) d4: VIRIDIAN HYPERCALL: enabled: 1 pfn: 3ffff
(XEN) d4v0: VIRIDIAN VP_ASSIST_PAGE: enabled: 1 pfn: 3fffe
(XEN) domain_crash called from viridian.c:452
(XEN) Domain 4 (vcpu#0) crashed on cpu#1:

The following sequence of events is an example of how this can happen:

- On return to guest vlapic_has_pending_irq() finds a bit set in the IRR.
- vlapic_ack_pending_irq() calls viridian_start_apic_assist() which latches
  the vector, sets the bit in the ISR and clears it from the IRR.
- The guest then processes the interrupt but EOIs it normally, therefore
  clearing the bit in the ISR.
- On next return to guest vlapic_has_pending_irq() calls
  viridian_complete_apic_assist(), which discovers the assist bit still set
  in the shared page and therefore leaves the latched vector in place, but
  also finds another bit set in the IRR.
- vlapic_ack_pending_irq() is then called but, because the ISR is was
  cleared by the EOI, another call is made to viridian_start_apic_assist()
  and this then calls domain_crash() because it finds the latched vector
  has not been cleared.

This patch adds a call to viridian_abort_apic_assist() into vlapic EOI
handler to make sure any latched vector is cleared when the ISR is cleared.

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/arch/x86/hvm/vlapic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 50f53bdaef..3184970c6e 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -422,6 +422,13 @@ void vlapic_EOI_set(struct vlapic *vlapic)
     if ( vector == -1 )
         return;
 
+    /*
+     * It is possible that APIC assist has been enabled by the guest but
+     * it has chosen not to use it, by EOIing normally. It is therefore
+     * necessary to abort any APIC assist that may have been started
+     * to avoid confusing the state machine.
+     */
+    viridian_abort_apic_assist(vlapic_vcpu(vlapic));
     vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
 
     if ( hvm_funcs.handle_eoi )
-- 
2.11.0


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