* [PATCH] x86/MSI: fix error handling
@ 2015-03-25 16:23 Jan Beulich
2015-03-25 18:37 ` Andrew Cooper
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2015-03-25 16:23 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 3669 bytes --]
__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 <jbeulich@suse.com>
--- 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 )
[-- Attachment #2: x86-MSI-error-paths.patch --]
[-- Type: text/plain, Size: 3694 bytes --]
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 <jbeulich@suse.com>
--- 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 )
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] x86/MSI: fix error handling
2015-03-25 16:23 [PATCH] x86/MSI: fix error handling Jan Beulich
@ 2015-03-25 18:37 ` Andrew Cooper
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2015-03-25 18:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
On 25/03/15 16:23, Jan Beulich wrote:
> __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 <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-03-25 18:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-25 16:23 [PATCH] x86/MSI: fix error handling Jan Beulich
2015-03-25 18:37 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.