* [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical [not found] <1448288826-19058-1-git-send-email-marc.zyngier@arm.com> @ 2015-11-30 10:25 ` Phil Edworthy 2015-12-04 0:27 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Phil Edworthy @ 2015-11-30 10:25 UTC (permalink / raw) To: Thomas Gleixner, Jiang Liu, Bjorn Helgaas Cc: linux-kernel@vger.kernel.org, Wolfram Sang, linux-pci@vger.kernel.org Cc'd linux-pci ml On 23 November 2015 14:27, Marc Zyngier wrote: Since d8a1cb757550 ("PCI/MSI: Let pci_msi_get_domain use struct device::msi_domain"), we use the MSI domain associated to the PCI device. But finding a MSI domain doesn't mean that the domain is implemented using the generic MSI domain API, and a number of MSI controllers are still using the arch_setup_msi_irq/arch_teardown_msi_irqs. In order to avoid a firework on these systems, check that the domain we just obtained is hierarchical. If not, don't use the generic MSI stuff and stick with the old one. Not pretty, but reliable. Another insentive to rework those drivers and phase out this API. Reported-by: Phil Edworthy <phil.edworthy@renesas.com> Tested-by: Phil Edworthy <phil.edworthy@renesas.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- drivers/pci/msi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 53e4632..7eaa4c8 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -54,7 +54,7 @@ static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) struct irq_domain *domain; domain = pci_msi_get_domain(dev); - if (domain) + if (domain && irq_domain_is_hierarchy(domain)) return pci_msi_domain_alloc_irqs(domain, dev, nvec, type); return arch_setup_msi_irqs(dev, nvec, type); @@ -65,7 +65,7 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) struct irq_domain *domain; domain = pci_msi_get_domain(dev); - if (domain) + if (domain && irq_domain_is_hierarchy(domain)) pci_msi_domain_free_irqs(domain, dev); else arch_teardown_msi_irqs(dev); -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical 2015-11-30 10:25 ` [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical Phil Edworthy @ 2015-12-04 0:27 ` Bjorn Helgaas 2015-12-04 8:13 ` Marc Zyngier 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2015-12-04 0:27 UTC (permalink / raw) To: Phil Edworthy Cc: Thomas Gleixner, Jiang Liu, Bjorn Helgaas, linux-kernel@vger.kernel.org, Wolfram Sang, linux-pci@vger.kernel.org On Mon, Nov 30, 2015 at 10:25:34AM +0000, Phil Edworthy wrote: > Cc'd linux-pci ml > > On 23 November 2015 14:27, Marc Zyngier wrote: > > Since d8a1cb757550 ("PCI/MSI: Let pci_msi_get_domain use struct > device::msi_domain"), we use the MSI domain associated to the PCI device. > > But finding a MSI domain doesn't mean that the domain is implemented > using the generic MSI domain API, and a number of MSI controllers > are still using the arch_setup_msi_irq/arch_teardown_msi_irqs. > > In order to avoid a firework on these systems, check that the domain > we just obtained is hierarchical. If not, don't use the generic MSI > stuff and stick with the old one. Not pretty, but reliable. > > Another insentive to rework those drivers and phase out this API. > > Reported-by: Phil Edworthy <phil.edworthy@renesas.com> > Tested-by: Phil Edworthy <phil.edworthy@renesas.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> Thanks, I applied this with Thomas' ack to pci/msi for v4.5. It looks like d8a1cb757550 appeared in v4.3. Is this a fix for that commit? Does this need to be backported via a stable tag? > --- > drivers/pci/msi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index 53e4632..7eaa4c8 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -54,7 +54,7 @@ static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) > struct irq_domain *domain; > > domain = pci_msi_get_domain(dev); > - if (domain) > + if (domain && irq_domain_is_hierarchy(domain)) > return pci_msi_domain_alloc_irqs(domain, dev, nvec, type); > > return arch_setup_msi_irqs(dev, nvec, type); > @@ -65,7 +65,7 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) > struct irq_domain *domain; > > domain = pci_msi_get_domain(dev); > - if (domain) > + if (domain && irq_domain_is_hierarchy(domain)) > pci_msi_domain_free_irqs(domain, dev); > else > arch_teardown_msi_irqs(dev); > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical 2015-12-04 0:27 ` Bjorn Helgaas @ 2015-12-04 8:13 ` Marc Zyngier 2015-12-04 16:17 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2015-12-04 8:13 UTC (permalink / raw) To: Bjorn Helgaas Cc: Phil Edworthy, Thomas Gleixner, Jiang Liu, Bjorn Helgaas, linux-kernel@vger.kernel.org, Wolfram Sang, linux-pci@vger.kernel.org On Thu, 3 Dec 2015 18:27:59 -0600 Bjorn Helgaas <helgaas@kernel.org> wrote: > On Mon, Nov 30, 2015 at 10:25:34AM +0000, Phil Edworthy wrote: > > Cc'd linux-pci ml > > > > On 23 November 2015 14:27, Marc Zyngier wrote: > > > > Since d8a1cb757550 ("PCI/MSI: Let pci_msi_get_domain use struct > > device::msi_domain"), we use the MSI domain associated to the PCI device. > > > > But finding a MSI domain doesn't mean that the domain is implemented > > using the generic MSI domain API, and a number of MSI controllers > > are still using the arch_setup_msi_irq/arch_teardown_msi_irqs. > > > > In order to avoid a firework on these systems, check that the domain > > we just obtained is hierarchical. If not, don't use the generic MSI > > stuff and stick with the old one. Not pretty, but reliable. > > > > Another insentive to rework those drivers and phase out this API. > > > > Reported-by: Phil Edworthy <phil.edworthy@renesas.com> > > Tested-by: Phil Edworthy <phil.edworthy@renesas.com> > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > > Thanks, I applied this with Thomas' ack to pci/msi for v4.5. > > It looks like d8a1cb757550 appeared in v4.3. Is this a fix for that > commit? Does this need to be backported via a stable tag? Hi Bjorn, I think this really deserves to be queued as an immediate fix for 4.4 rather than 4.5, as some systems in mainline are affected by this bug: http://www.spinics.net/lists/arm-kernel/msg465792.html It would also deserve a stable tag for 4.3. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical 2015-12-04 8:13 ` Marc Zyngier @ 2015-12-04 16:17 ` Bjorn Helgaas 2015-12-04 17:07 ` Marc Zyngier 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2015-12-04 16:17 UTC (permalink / raw) To: Marc Zyngier Cc: Phil Edworthy, Thomas Gleixner, Jiang Liu, Bjorn Helgaas, linux-kernel@vger.kernel.org, Wolfram Sang, linux-pci@vger.kernel.org On Fri, Dec 04, 2015 at 08:13:50AM +0000, Marc Zyngier wrote: > On Thu, 3 Dec 2015 18:27:59 -0600 > Bjorn Helgaas <helgaas@kernel.org> wrote: > > > On Mon, Nov 30, 2015 at 10:25:34AM +0000, Phil Edworthy wrote: > > > Cc'd linux-pci ml > > > > > > On 23 November 2015 14:27, Marc Zyngier wrote: > > > > > > Since d8a1cb757550 ("PCI/MSI: Let pci_msi_get_domain use struct > > > device::msi_domain"), we use the MSI domain associated to the PCI device. > > > > > > But finding a MSI domain doesn't mean that the domain is implemented > > > using the generic MSI domain API, and a number of MSI controllers > > > are still using the arch_setup_msi_irq/arch_teardown_msi_irqs. > > > > > > In order to avoid a firework on these systems, check that the domain > > > we just obtained is hierarchical. If not, don't use the generic MSI > > > stuff and stick with the old one. Not pretty, but reliable. > > > > > > Another insentive to rework those drivers and phase out this API. > > > > > > Reported-by: Phil Edworthy <phil.edworthy@renesas.com> > > > Tested-by: Phil Edworthy <phil.edworthy@renesas.com> > > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > > > > Thanks, I applied this with Thomas' ack to pci/msi for v4.5. > > > > It looks like d8a1cb757550 appeared in v4.3. Is this a fix for that > > commit? Does this need to be backported via a stable tag? > > Hi Bjorn, > > I think this really deserves to be queued as an immediate fix for 4.4 > rather than 4.5, as some systems in mainline are affected by this bug: > > http://www.spinics.net/lists/arm-kernel/msg465792.html > > It would also deserve a stable tag for 4.3. OK, I can do that. It would save me a lot of time to get a hint when this is the case. I couldn't tell if this issue happened on mainline or with some still out-of-tree patches. I'd also like to know what machines are affected. I did look at d8a1cb757550, and the connection between that and this patch is not completely obvious; would you regard this as a fix to d8a1cb757550? Should this patch be backported to every kernel that includes d8a1cb757550? Or is this more closely tied to some other change? Bjorn ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical 2015-12-04 16:17 ` Bjorn Helgaas @ 2015-12-04 17:07 ` Marc Zyngier 2015-12-04 17:46 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2015-12-04 17:07 UTC (permalink / raw) To: Bjorn Helgaas Cc: Phil Edworthy, Thomas Gleixner, Jiang Liu, Bjorn Helgaas, linux-kernel@vger.kernel.org, Wolfram Sang, linux-pci@vger.kernel.org Hi Bjorn, On 04/12/15 16:17, Bjorn Helgaas wrote: > On Fri, Dec 04, 2015 at 08:13:50AM +0000, Marc Zyngier wrote: >> On Thu, 3 Dec 2015 18:27:59 -0600 >> Bjorn Helgaas <helgaas@kernel.org> wrote: >> >>> On Mon, Nov 30, 2015 at 10:25:34AM +0000, Phil Edworthy wrote: >>>> Cc'd linux-pci ml >>>> >>>> On 23 November 2015 14:27, Marc Zyngier wrote: >>>> >>>> Since d8a1cb757550 ("PCI/MSI: Let pci_msi_get_domain use struct >>>> device::msi_domain"), we use the MSI domain associated to the PCI device. >>>> >>>> But finding a MSI domain doesn't mean that the domain is implemented >>>> using the generic MSI domain API, and a number of MSI controllers >>>> are still using the arch_setup_msi_irq/arch_teardown_msi_irqs. >>>> >>>> In order to avoid a firework on these systems, check that the domain >>>> we just obtained is hierarchical. If not, don't use the generic MSI >>>> stuff and stick with the old one. Not pretty, but reliable. >>>> >>>> Another insentive to rework those drivers and phase out this API. >>>> >>>> Reported-by: Phil Edworthy <phil.edworthy@renesas.com> >>>> Tested-by: Phil Edworthy <phil.edworthy@renesas.com> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>> >>> Thanks, I applied this with Thomas' ack to pci/msi for v4.5. >>> >>> It looks like d8a1cb757550 appeared in v4.3. Is this a fix for that >>> commit? Does this need to be backported via a stable tag? >> >> Hi Bjorn, >> >> I think this really deserves to be queued as an immediate fix for 4.4 >> rather than 4.5, as some systems in mainline are affected by this bug: >> >> http://www.spinics.net/lists/arm-kernel/msg465792.html >> >> It would also deserve a stable tag for 4.3. > > OK, I can do that. It would save me a lot of time to get a hint when > this is the case. I couldn't tell if this issue happened on mainline > or with some still out-of-tree patches. I'd also like to know what > machines are affected. Sorry, I should have been more clear. At the moment, I've had reports of at least RCar, Tegra and Armada 370 being affected. Anything using the Designware driver will probably also fall over. > I did look at d8a1cb757550, and the connection between that and this > patch is not completely obvious; would you regard this as a fix to > d8a1cb757550? Should this patch be backported to every kernel that > includes d8a1cb757550? Or is this more closely tied to some other > change? d8a1cb757550 is indeed the source of the problem: we assumes that finding a domain identified by a given device is enough to decide that we're using the generic MSI layer. Unfortunately, this is not the case, and a number of ARM drivers are actually registering their own domain the same way, without using the generic MSI layer. I didn't spot this on arm64, as all my platforms are using this layer. So it seems better to merge this in 4.4, with a backport to 4.3 (which is the first kernel to contain this commit). Sorry for the mess - I'll make it clearer next time. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical 2015-12-04 17:07 ` Marc Zyngier @ 2015-12-04 17:46 ` Bjorn Helgaas 0 siblings, 0 replies; 6+ messages in thread From: Bjorn Helgaas @ 2015-12-04 17:46 UTC (permalink / raw) To: Marc Zyngier Cc: Phil Edworthy, Thomas Gleixner, Jiang Liu, Bjorn Helgaas, linux-kernel@vger.kernel.org, Wolfram Sang, linux-pci@vger.kernel.org On Fri, Dec 04, 2015 at 05:07:47PM +0000, Marc Zyngier wrote: > Hi Bjorn, > > On 04/12/15 16:17, Bjorn Helgaas wrote: > > On Fri, Dec 04, 2015 at 08:13:50AM +0000, Marc Zyngier wrote: > >> On Thu, 3 Dec 2015 18:27:59 -0600 > >> Bjorn Helgaas <helgaas@kernel.org> wrote: > >> > >>> On Mon, Nov 30, 2015 at 10:25:34AM +0000, Phil Edworthy wrote: > >>>> Cc'd linux-pci ml > >>>> > >>>> On 23 November 2015 14:27, Marc Zyngier wrote: > >>>> > >>>> Since d8a1cb757550 ("PCI/MSI: Let pci_msi_get_domain use struct > >>>> device::msi_domain"), we use the MSI domain associated to the PCI device. > >>>> > >>>> But finding a MSI domain doesn't mean that the domain is implemented > >>>> using the generic MSI domain API, and a number of MSI controllers > >>>> are still using the arch_setup_msi_irq/arch_teardown_msi_irqs. > >>>> > >>>> In order to avoid a firework on these systems, check that the domain > >>>> we just obtained is hierarchical. If not, don't use the generic MSI > >>>> stuff and stick with the old one. Not pretty, but reliable. > >>>> > >>>> Another insentive to rework those drivers and phase out this API. > >>>> > >>>> Reported-by: Phil Edworthy <phil.edworthy@renesas.com> > >>>> Tested-by: Phil Edworthy <phil.edworthy@renesas.com> > >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >>> > >>> Thanks, I applied this with Thomas' ack to pci/msi for v4.5. > >>> > >>> It looks like d8a1cb757550 appeared in v4.3. Is this a fix for that > >>> commit? Does this need to be backported via a stable tag? > >> > >> Hi Bjorn, > >> > >> I think this really deserves to be queued as an immediate fix for 4.4 > >> rather than 4.5, as some systems in mainline are affected by this bug: > >> > >> http://www.spinics.net/lists/arm-kernel/msg465792.html > >> > >> It would also deserve a stable tag for 4.3. > > > > OK, I can do that. It would save me a lot of time to get a hint when > > this is the case. I couldn't tell if this issue happened on mainline > > or with some still out-of-tree patches. I'd also like to know what > > machines are affected. > > Sorry, I should have been more clear. At the moment, I've had reports of > at least RCar, Tegra and Armada 370 being affected. Anything using the > Designware driver will probably also fall over. > > > I did look at d8a1cb757550, and the connection between that and this > > patch is not completely obvious; would you regard this as a fix to > > d8a1cb757550? Should this patch be backported to every kernel that > > includes d8a1cb757550? Or is this more closely tied to some other > > change? > > d8a1cb757550 is indeed the source of the problem: we assumes that > finding a domain identified by a given device is enough to decide that > we're using the generic MSI layer. Unfortunately, this is not the case, > and a number of ARM drivers are actually registering their own domain > the same way, without using the generic MSI layer. I didn't spot this on > arm64, as all my platforms are using this layer. > > So it seems better to merge this in 4.4, with a backport to 4.3 (which > is the first kernel to contain this commit). Thanks, Marc! I moved this to for-linus for v4.4, added info about the machines, and added a stable tag for v4.3. Bjorn ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-04 17:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1448288826-19058-1-git-send-email-marc.zyngier@arm.com>
2015-11-30 10:25 ` [PATCH] PCI: MSI: Only use the generic MSI layer when domain is hierarchical Phil Edworthy
2015-12-04 0:27 ` Bjorn Helgaas
2015-12-04 8:13 ` Marc Zyngier
2015-12-04 16:17 ` Bjorn Helgaas
2015-12-04 17:07 ` Marc Zyngier
2015-12-04 17:46 ` Bjorn Helgaas
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).