x86/MSI: fix error handling __setup_msi_irq() needs to undo what it did before calling write_msi_msg() in case that returned an error. map_domain_pirq() needs to get rid of the MSI descriptor it (implicitly) allocated. The case of a setup_msi_irq() failure on a non-initial multi-vector-MSI interrupt needs special handling: While the initial IRQ will get freed by the caller (who also passed it to us), we need to undo the effect setup_msi_irq() had on it. (As a benefit from the added call to msi_free_irq() we no longer need to explicitly call destroy_irq() on the non-initial slots.) Signed-off-by: Jan Beulich --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -1974,6 +1974,8 @@ int map_domain_pirq( dprintk(XENLOG_G_ERR, "dom%d: irq %d in use\n", d->domain_id, irq); pci_disable_msi(msi_desc); + msi_desc->irq = -1; + msi_free_irq(msi_desc); ret = -EBUSY; goto done; } @@ -2028,22 +2030,29 @@ int map_domain_pirq( if ( ret ) { spin_unlock_irqrestore(&desc->lock, flags); + pci_disable_msi(msi_desc); + if ( nr ) + { + ASSERT(msi_desc->irq >= 0); + desc = irq_to_desc(msi_desc->irq); + spin_lock_irqsave(&desc->lock, flags); + desc->handler = &no_irq_type; + desc->msi_desc = NULL; + spin_unlock_irqrestore(&desc->lock, flags); + } while ( nr-- ) { - if ( irq >= 0 ) - { - if ( irq_deny_access(d, irq) ) - printk(XENLOG_G_ERR - "dom%d: could not revoke access to IRQ%d (pirq %d)\n", - d->domain_id, irq, pirq); - destroy_irq(irq); - } + if ( irq >= 0 && irq_deny_access(d, irq) ) + printk(XENLOG_G_ERR + "dom%d: could not revoke access to IRQ%d (pirq %d)\n", + d->domain_id, irq, pirq); if ( info ) cleanup_domain_irq_pirq(d, irq, info); info = pirq_info(d, pirq + nr); irq = info->arch.irq; } - pci_disable_msi(msi_desc); + msi_desc->irq = -1; + msi_free_irq(msi_desc); goto done; } --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -561,6 +561,7 @@ static struct msi_desc *alloc_msi_entry( while ( nr-- ) { entry[nr].dev = NULL; + entry[nr].irq = -1; entry[nr].remap_index = -1; } @@ -578,11 +579,19 @@ int __setup_msi_irq(struct irq_desc *des hw_irq_controller *handler) { struct msi_msg msg; + int ret; desc->msi_desc = msidesc; desc->handler = handler; msi_compose_msg(desc->arch.vector, desc->arch.cpu_mask, &msg); - return write_msi_msg(msidesc, &msg); + ret = write_msi_msg(msidesc, &msg); + if ( unlikely(ret) ) + { + desc->handler = &no_irq_type; + desc->msi_desc = NULL; + } + + return ret; } int msi_free_irq(struct msi_desc *entry) @@ -592,7 +601,8 @@ int msi_free_irq(struct msi_desc *entry) while ( nr-- ) { - destroy_irq(entry[nr].irq); + if ( entry[nr].irq >= 0 ) + destroy_irq(entry[nr].irq); /* Free the unused IRTE if intr remap enabled */ if ( iommu_intremap )