x86/dpci: do not remove pirqs from domain tree on unbind

A fix for a previous issue removed the pirqs from the domain tree when
they are unbound in order to prevent shared pirqs from triggering a
BUG_ON in __pirq_guest_unbind if they are unbound multiple times. That
caused free_domain_pirqs to no longer unmap the pirqs because they
are gone from the domain pirq tree, thus leaving stale unbound pirqs
after domain destruction if the domain had mapped dpci pirqs after
shutdown.

Take a different approach to fix the original issue, instead of
removing the pirq from d->pirq_tree clear the flags of the dpci pirq
struct to signal that the pirq is now unbound. This prevents calling
pirq_guest_unbind multiple times for the same pirq without having to
remove it from the domain pirq tree.

This is XSA-360.

Fixes: 5b58dad089 ('x86/pass-through: avoid double IRQ unbind during domain cleanup')
Reported-by: Samuel Verschelde <samuel.verschelde@vates.fr>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 21881a1345532392a12964c9b504908a194f6e81..032fe821675048fdd2ca7a23ed7ba42a8e9b80bc 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1349,7 +1349,7 @@ void (pirq_cleanup_check)(struct pirq *pirq, struct domain *d)
     }
 
     if ( radix_tree_delete(&d->pirq_tree, pirq->pirq) != pirq )
-        BUG_ON(!d->is_dying);
+        BUG();
 }
 
 /* Flush all ready EOIs from the top of this CPU's pending-EOI stack. */

diff --git a/xen/drivers/passthrough/x86/hvm.c b/xen/drivers/passthrough/x86/hvm.c
index edc8059518a5d2b6f07aa1a2ca3c0fd3e8fe60eb..a6e2863c1441945aa63219010cf9c79766073095 100644 (file)
--- a/xen/drivers/passthrough/x86/hvm.c
+++ b/xen/drivers/passthrough/x86/hvm.c
@@ -1036,6 +1036,10 @@ static int pci_clean_dpci_irq(struct domain *d,
 {
     struct dev_intx_gsi_link *digl, *tmp;
 
+    if ( !pirq_dpci->flags )
+        /* Already processed. */
+        return 0;
+
     pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
 
     if ( pt_irq_need_timer(pirq_dpci->flags) )
@@ -1046,15 +1050,10 @@ static int pci_clean_dpci_irq(struct domain *d,
         list_del(&digl->list);
         xfree(digl);
     }
+    /* Note the pirq is now unbound. */
+    pirq_dpci->flags = 0;
 
-    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
-
-    if ( !pt_pirq_softirq_active(pirq_dpci) )
-        return 0;
-
-    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
-
-    return -ERESTART;
+    return pt_pirq_softirq_active(pirq_dpci) ? -ERESTART : 0;
 }
 
 int arch_pci_clean_pirqs(struct domain *d)
@@ -1071,18 +1070,8 @@ int arch_pci_clean_pirqs(struct domain *d)
     hvm_irq_dpci = domain_get_irq_dpci(d);
     if ( hvm_irq_dpci != NULL )
     {
-        int ret = 0;
-
-        if ( hvm_irq_dpci->pending_pirq_dpci )
-        {
-            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
-                 ret = -ERESTART;
-            else
-                 hvm_irq_dpci->pending_pirq_dpci = NULL;
-        }
+        int ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
 
-        if ( !ret )
-            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
         if ( ret )
         {
             spin_unlock(&d->event_lock);

diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
index 532880d497e6d9d67c85b79246f6f69d2fced4b8..7f76f6c4378e176146b113ac15f6f682c74dbb02 100644 (file)
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -160,8 +160,6 @@ struct hvm_irq_dpci {
     DECLARE_BITMAP(isairq_map, NR_ISAIRQS);
     /* Record of mapped Links */
     uint8_t link_cnt[NR_LINK];
-    /* Clean up: Entry with a softirq invocation pending / in progress. */
-    struct hvm_pirq_dpci *pending_pirq_dpci;
 };
 
 /* Machine IRQ to guest device/intx mapping. */