* [PATCH] genirq/msi: fix populating multiple interrupts
@ 2017-09-05 17:12 John Keeping
2017-09-06 8:52 ` Marc Zyngier
0 siblings, 1 reply; 4+ messages in thread
From: John Keeping @ 2017-09-05 17:12 UTC (permalink / raw)
To: Marc Zyngier; +Cc: Thomas Gleixner, linux-kernel, John Keeping
Use the correct variable to set up each interrupt in turn rather than
configuring the first interrupt "nvec" times.
Signed-off-by: John Keeping <john@metanate.com>
---
kernel/irq/msi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 48eadf416c24..3fa4bd59f569 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -315,11 +315,12 @@ int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
ops->set_desc(arg, desc);
/* Assumes the domain mutex is held! */
- ret = irq_domain_alloc_irqs_hierarchy(domain, virq, 1, arg);
+ ret = irq_domain_alloc_irqs_hierarchy(domain, desc->irq, 1,
+ arg);
if (ret)
break;
- irq_set_msi_desc_off(virq, 0, desc);
+ irq_set_msi_desc_off(desc->irq, 0, desc);
}
if (ret) {
--
2.14.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] genirq/msi: fix populating multiple interrupts
2017-09-05 17:12 [PATCH] genirq/msi: fix populating multiple interrupts John Keeping
@ 2017-09-06 8:52 ` Marc Zyngier
2017-09-06 9:35 ` [PATCH v2] genirq/msi: Fix " John Keeping
0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2017-09-06 8:52 UTC (permalink / raw)
To: John Keeping; +Cc: Thomas Gleixner, linux-kernel
Hi John,
On 05/09/17 18:12, John Keeping wrote:
> Use the correct variable to set up each interrupt in turn rather than
> configuring the first interrupt "nvec" times.
Thanks for addressing this. I think this bug deserves a slightly better
write-up. How about something like:
<quote>
On allocating the interrupts routed via to a wire-to-MSI bridge, we
iterate over the MSI descriptors to build the hierarchy, but fail to use
the descriptor interrupt number, and instead use the base number,
generating the wrong IRQ domain mappings.
The fix is to use the MSI descriptor interrupt number when setting up
the interrupt instead of the base interrupt for the allocation range.
The only saving grace is that although the MSI descriptors are allocated
in bulk, the wired interrupts are only allocated one by one (so
desc->irq == virq) and the bug goes unnoticed.
</quote>
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> kernel/irq/msi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 48eadf416c24..3fa4bd59f569 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -315,11 +315,12 @@ int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
>
> ops->set_desc(arg, desc);
> /* Assumes the domain mutex is held! */
> - ret = irq_domain_alloc_irqs_hierarchy(domain, virq, 1, arg);
> + ret = irq_domain_alloc_irqs_hierarchy(domain, desc->irq, 1,
> + arg);
> if (ret)
> break;
>
> - irq_set_msi_desc_off(virq, 0, desc);
> + irq_set_msi_desc_off(desc->irq, 0, desc);
> }
>
> if (ret) {
>
Fixes: 2145ac9310b60 ("genirq/msi: Add msi_domain_populate_irqs")
Cc: stable@vger.kernel.org #v4.5+
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] genirq/msi: Fix populating multiple interrupts
2017-09-06 8:52 ` Marc Zyngier
@ 2017-09-06 9:35 ` John Keeping
2017-09-06 9:46 ` [tip:irq/urgent] " tip-bot for John Keeping
0 siblings, 1 reply; 4+ messages in thread
From: John Keeping @ 2017-09-06 9:35 UTC (permalink / raw)
To: Marc Zyngier; +Cc: Thomas Gleixner, linux-kernel
On allocating the interrupts routed via to a wire-to-MSI bridge, we
iterate over the MSI descriptors to build the hierarchy, but fail to use
the descriptor interrupt number, and instead use the base number,
generating the wrong IRQ domain mappings.
The fix is to use the MSI descriptor interrupt number when setting up
the interrupt instead of the base interrupt for the allocation range.
The only saving grace is that although the MSI descriptors are allocated
in bulk, the wired interrupts are only allocated one by one (so
desc->irq == virq) and the bug goes unnoticed.
Fixes: 2145ac9310b60 ("genirq/msi: Add msi_domain_populate_irqs")
Cc: stable@vger.kernel.org #v4.5+
Signed-off-by: John Keeping <john@metanate.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
On Wed, 6 Sep 2017 09:52:43 +0100, Marc Zyngier wrote:
> On 05/09/17 18:12, John Keeping wrote:
> > Use the correct variable to set up each interrupt in turn rather than
> > configuring the first interrupt "nvec" times.
>
> Thanks for addressing this. I think this bug deserves a slightly better
> write-up. How about something like:
Much better, thanks. I've copied and pasted your suggestion for v2.
Changes in v2:
- Use Marc's text for the commit message
- Add Fixes, Cc, Reviewed-by tags
kernel/irq/msi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 48eadf416c24..3fa4bd59f569 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -315,11 +315,12 @@ int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
ops->set_desc(arg, desc);
/* Assumes the domain mutex is held! */
- ret = irq_domain_alloc_irqs_hierarchy(domain, virq, 1, arg);
+ ret = irq_domain_alloc_irqs_hierarchy(domain, desc->irq, 1,
+ arg);
if (ret)
break;
- irq_set_msi_desc_off(virq, 0, desc);
+ irq_set_msi_desc_off(desc->irq, 0, desc);
}
if (ret) {
--
2.14.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip:irq/urgent] genirq/msi: Fix populating multiple interrupts
2017-09-06 9:35 ` [PATCH v2] genirq/msi: Fix " John Keeping
@ 2017-09-06 9:46 ` tip-bot for John Keeping
0 siblings, 0 replies; 4+ messages in thread
From: tip-bot for John Keeping @ 2017-09-06 9:46 UTC (permalink / raw)
To: linux-tip-commits; +Cc: mingo, tglx, linux-kernel, hpa, john, marc.zyngier
Commit-ID: 596a7a1d0989c621c3ae49be73a1d1f9de22eb5a
Gitweb: http://git.kernel.org/tip/596a7a1d0989c621c3ae49be73a1d1f9de22eb5a
Author: John Keeping <john@metanate.com>
AuthorDate: Wed, 6 Sep 2017 10:35:40 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 6 Sep 2017 11:41:20 +0200
genirq/msi: Fix populating multiple interrupts
On allocating the interrupts routed via a wire-to-MSI bridge, the allocator
iterates over the MSI descriptors to build the hierarchy, but fails to use
the descriptor interrupt number, and instead uses the base number,
generating the wrong IRQ domain mappings.
The fix is to use the MSI descriptor interrupt number when setting up
the interrupt instead of the base interrupt for the allocation range.
The only saving grace is that although the MSI descriptors are allocated
in bulk, the wired interrupts are only allocated one by one (so
desc->irq == virq) and the bug went unnoticed so far.
Fixes: 2145ac9310b60 ("genirq/msi: Add msi_domain_populate_irqs")
Signed-off-by: John Keeping <john@metanate.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20170906103540.373864a2.john@metanate.com
---
kernel/irq/msi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 48eadf4..3fa4bd5 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -315,11 +315,12 @@ int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
ops->set_desc(arg, desc);
/* Assumes the domain mutex is held! */
- ret = irq_domain_alloc_irqs_hierarchy(domain, virq, 1, arg);
+ ret = irq_domain_alloc_irqs_hierarchy(domain, desc->irq, 1,
+ arg);
if (ret)
break;
- irq_set_msi_desc_off(virq, 0, desc);
+ irq_set_msi_desc_off(desc->irq, 0, desc);
}
if (ret) {
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-06 9:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 17:12 [PATCH] genirq/msi: fix populating multiple interrupts John Keeping
2017-09-06 8:52 ` Marc Zyngier
2017-09-06 9:35 ` [PATCH v2] genirq/msi: Fix " John Keeping
2017-09-06 9:46 ` [tip:irq/urgent] " tip-bot for John Keeping
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).