From 022aa5fea4fcd347a1657baac68eae8d24fe3b9f Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Sun, 18 Sep 2011 00:00:26 +0100 Subject: [PATCH] Clear IRQ_GUEST in irq_desc->status when setting action to NULL. Looking more closely at usage of action field with relation to IRQ_GUEST flag. It appears that set IRQ_GUEST implies that action is not NULL. As result it is not safe to set action to NULL and leave IRQ_GUEST set. Hence IRQ_GUEST should be cleared in dynamic_irq_cleanup where action is set to NULL. An addition remove BUGON at __pirq_guest_unbind that appears to be bogus and not needed anymore. Thanks Paolo Bonzini for NACKing previous patch, and pointing at the correct solution. Signed-off-by: Igor Mammedov Reinstate the BUG_ON, but after the action==NULL check. Since we then go and start interpreting action as an irq_guest_action_t, the BUG_ON is relevant here. More generally, the brute-force nature of dynamic_irq_cleanup() looks a bit worrying. Possibly there should be more integratioin with pirq_guest_unbind() logic, for cleaning up un-acked EOIs and the like. Signed-off-by: Keir Fraser --- xen/arch/x86/irq.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index 9ea29bac76..228b112084 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -192,6 +192,7 @@ static void dynamic_irq_cleanup(unsigned int irq) spin_lock_irqsave(&desc->lock, flags); desc->status |= IRQ_DISABLED; + desc->status &= ~IRQ_GUEST; desc->handler->shutdown(irq); action = desc->action; desc->action = NULL; @@ -1465,8 +1466,6 @@ static irq_guest_action_t *__pirq_guest_unbind( cpumask_t cpu_eoi_map; int i; - BUG_ON(!(desc->status & IRQ_GUEST)); - action = (irq_guest_action_t *)desc->action; irq = desc - irq_desc; @@ -1477,6 +1476,8 @@ static irq_guest_action_t *__pirq_guest_unbind( return NULL; } + BUG_ON(!(desc->status & IRQ_GUEST)); + for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ ) continue; BUG_ON(i == action->nr_guests); -- 2.39.5