XEN_DOMCTL_destroydomain creates a continuation if domain_kill -ERESTARTS.
In that scenario, it is possible to receive multiple _pirq_guest_unbind
calls for the same pirq from domain_kill, if the pirq has not yet been
removed from the domain's pirq_tree, as:
domain_kill()
-> domain_relinquish_resources()
-> pci_release_devices()
-> pci_clean_dpci_irq()
-> pirq_guest_unbind()
-> __pirq_guest_unbind()
For a shared pirq (nr_guests > 1), the first call would zap the current
domain from the pirq's guests[] list, but the action handler is never freed
as there are other guests using this pirq. As a result, on the second call,
__pirq_guest_unbind searches for the current domain which has been removed
from the guests[] list, and hits a BUG_ON.
Make __pirq_guest_unbind safe to be called multiple times by letting xen
continue if a shared pirq has already been unbound from this guest. The
PIRQ will be cleaned up from the domain's pirq_tree during the destruction
in complete_domain_destroy anyways.
Signed-off-by: Varad Gautam <vrd@xxxxxxxxx>
Reviewed-by: Paul Durrant <paul@xxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
v2: Split the check on action->nr_guests > 0 and make it an ASSERT.
v3: Style fixups.
---
xen/arch/x86/irq.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 310ac00..4b172eb 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1681,7 +1681,20 @@ static irq_guest_action_t *__pirq_guest_unbind(
for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ )
continue;
- BUG_ON(i == action->nr_guests);
+ if ( i == action->nr_guests )
+ {
+ ASSERT(action->nr_guests > 0);
+ /*
+ * In case the pirq was shared, unbound for this domain in an earlier
+ * call, but still existed on the domain's pirq_tree, we still reach
+ * here if there are any later unbind calls on the same pirq. Return
+ * if such an unbind happens.
+ */
+ if ( action->shareable )
+ return NULL;
+ BUG();