All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Paolo Pisati <p.pisati@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Andrew Lunn <andrew@lunn.ch>, Jason Cooper <jason@lakedaemon.net>,
	Joerg Roedel <joro@8bytes.org>,
	Hiroshi.DOYU@nokia.com, linux-kernel@vger.kernel.org,
	iommu@lists.linux-foundation.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Gregory CLEMENT <gregory.clement@free-electrons.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	Florian Vaussard <florian.vaussard@epfl.ch>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: 3.16rc3 multiplatform, Armada 370 and IOMMU: unbootable kernel
Date: Mon, 7 Jul 2014 20:37:58 -0300	[thread overview]
Message-ID: <20140707233758.GA1456@arch.cereza> (raw)
In-Reply-To: <20140707183002.GA5947@kroah.com>

On 07 Jul 11:30 AM, Greg Kroah-Hartman wrote:
> On Mon, Jul 07, 2014 at 07:58:18AM -0300, Ezequiel Garcia wrote:
[..]
> > 
> > I guess I snipped the thread and lost most of the information about the panic.
> > Here's the original bug report:
> > 
> > http://www.spinics.net/lists/arm-kernel/msg344059.html
> > 
> > The problem reported involves enabling OMAP IOMMU driver and not any other IOMMU
> > driver. Doing some tracing and adding a few prints, we found that
> > omap_iommu_init() sets a bus notifier for the platform bus type:
> > 
> > omap_iommu_init -> bus_set_iommu -> iommu_bus_init:
> > 
> > static void iommu_bus_init(struct bus_type *bus, struct iommu_ops *ops)
> > {
> >         bus_register_notifier(bus, &iommu_bus_nb);
> >         bus_for_each_dev(bus, NULL, ops, add_iommu_group);
> > }
> > 
> > But the iommu bus notifier gets called for the 'pci' bus type, which
> > has the iommu_ops field NULL (since it hasn't been set for iommu).
> 
> So this is what needs to be figured out, how is the notifier being
> called with a PCI device?  Who else called iommu_bus_init() for the PCI
> bus?
> 

Nobody. The only caller of iommu_bus_init() the above OMAP IOMMU.

However, I found something interesting. Tried to bisect this without much
luck; I did two bisects and ended up in the same merge commit:

# good: [0f16aa3c24a216d14d7f0587e1cbd2c1b51a38f3] Merge tag 'samsung-dt-3' of git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung into next/dt
# first bad commit: [755a9ba7bf24a45b6dbf8bb15a5a56c8ed12461a] Merge tag 'dt-for-3.16' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc into next

So, after doing a few diff's between that good and bad and searching for
"bus_notifier" changes, saw something strange in arch/arm/mach-mvebu/coherency.c.

It seems bus_register_notifier() is been called for platform and pci devices
with the *same* notifier block. Haven't looked close enough, but you mentioned
that could cause trouble?

This patch fixes the issue here:

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 477202f..2bdc323 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -292,6 +292,10 @@ static struct notifier_block mvebu_hwcc_nb = {
 	.notifier_call = mvebu_hwcc_notifier,
 };
 
+static struct notifier_block mvebu_hwcc_pci_nb = {
+	.notifier_call = mvebu_hwcc_notifier,
+};
+
 static void __init armada_370_coherency_init(struct device_node *np)
 {
 	struct resource res;
@@ -427,7 +431,7 @@ static int __init coherency_pci_init(void)
 {
 	if (coherency_available())
 		bus_register_notifier(&pci_bus_type,
-				       &mvebu_hwcc_nb);
+				       &mvebu_hwcc_pci_nb);
 	return 0;
 }
 
Paolo, can you apply it and confirm it fixes the problem?

Greg, can you confirm using the same notifier block pointer
for two different bus types makes the bus notifier go nuts?

Thanks for the help,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: ezequiel.garcia@free-electrons.com (Ezequiel Garcia)
To: linux-arm-kernel@lists.infradead.org
Subject: 3.16rc3 multiplatform, Armada 370 and IOMMU: unbootable kernel
Date: Mon, 7 Jul 2014 20:37:58 -0300	[thread overview]
Message-ID: <20140707233758.GA1456@arch.cereza> (raw)
In-Reply-To: <20140707183002.GA5947@kroah.com>

On 07 Jul 11:30 AM, Greg Kroah-Hartman wrote:
> On Mon, Jul 07, 2014 at 07:58:18AM -0300, Ezequiel Garcia wrote:
[..]
> > 
> > I guess I snipped the thread and lost most of the information about the panic.
> > Here's the original bug report:
> > 
> > http://www.spinics.net/lists/arm-kernel/msg344059.html
> > 
> > The problem reported involves enabling OMAP IOMMU driver and not any other IOMMU
> > driver. Doing some tracing and adding a few prints, we found that
> > omap_iommu_init() sets a bus notifier for the platform bus type:
> > 
> > omap_iommu_init -> bus_set_iommu -> iommu_bus_init:
> > 
> > static void iommu_bus_init(struct bus_type *bus, struct iommu_ops *ops)
> > {
> >         bus_register_notifier(bus, &iommu_bus_nb);
> >         bus_for_each_dev(bus, NULL, ops, add_iommu_group);
> > }
> > 
> > But the iommu bus notifier gets called for the 'pci' bus type, which
> > has the iommu_ops field NULL (since it hasn't been set for iommu).
> 
> So this is what needs to be figured out, how is the notifier being
> called with a PCI device?  Who else called iommu_bus_init() for the PCI
> bus?
> 

Nobody. The only caller of iommu_bus_init() the above OMAP IOMMU.

However, I found something interesting. Tried to bisect this without much
luck; I did two bisects and ended up in the same merge commit:

# good: [0f16aa3c24a216d14d7f0587e1cbd2c1b51a38f3] Merge tag 'samsung-dt-3' of git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung into next/dt
# first bad commit: [755a9ba7bf24a45b6dbf8bb15a5a56c8ed12461a] Merge tag 'dt-for-3.16' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc into next

So, after doing a few diff's between that good and bad and searching for
"bus_notifier" changes, saw something strange in arch/arm/mach-mvebu/coherency.c.

It seems bus_register_notifier() is been called for platform and pci devices
with the *same* notifier block. Haven't looked close enough, but you mentioned
that could cause trouble?

This patch fixes the issue here:

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 477202f..2bdc323 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -292,6 +292,10 @@ static struct notifier_block mvebu_hwcc_nb = {
 	.notifier_call = mvebu_hwcc_notifier,
 };
 
+static struct notifier_block mvebu_hwcc_pci_nb = {
+	.notifier_call = mvebu_hwcc_notifier,
+};
+
 static void __init armada_370_coherency_init(struct device_node *np)
 {
 	struct resource res;
@@ -427,7 +431,7 @@ static int __init coherency_pci_init(void)
 {
 	if (coherency_available())
 		bus_register_notifier(&pci_bus_type,
-				       &mvebu_hwcc_nb);
+				       &mvebu_hwcc_pci_nb);
 	return 0;
 }
 
Paolo, can you apply it and confirm it fixes the problem?

Greg, can you confirm using the same notifier block pointer
for two different bus types makes the bus notifier go nuts?

Thanks for the help,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

  reply	other threads:[~2014-07-07 23:38 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 13:51 3.16rc3 multiplatform, Armada 370 and IOMMU: unbootable kernel Paolo Pisati
2014-07-03 14:27 ` Thomas Petazzoni
2014-07-03 14:57   ` Paolo Pisati
2014-07-03 20:22   ` Gregory CLEMENT
2014-07-03 20:52     ` Thomas Petazzoni
2014-07-03 20:57       ` Gregory CLEMENT
2014-07-03 21:01         ` Thomas Petazzoni
2014-07-03 21:07           ` Gregory CLEMENT
2014-07-03 21:24             ` Gregory CLEMENT
     [not found]               ` <53B5CA26.7050405-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-07-04  8:47                 ` Laurent Pinchart
2014-07-04  8:47                   ` Laurent Pinchart
2014-07-05 15:03                   ` Ezequiel Garcia
2014-07-05 15:03                     ` Ezequiel Garcia
2014-07-05 20:59                     ` Greg Kroah-Hartman
2014-07-05 20:59                       ` Greg Kroah-Hartman
2014-07-05 20:59                       ` Greg Kroah-Hartman
2014-07-07 10:58                       ` Ezequiel Garcia
2014-07-07 10:58                         ` Ezequiel Garcia
2014-07-07 18:30                         ` Greg Kroah-Hartman
2014-07-07 18:30                           ` Greg Kroah-Hartman
2014-07-07 18:30                           ` Greg Kroah-Hartman
2014-07-07 23:37                           ` Ezequiel Garcia [this message]
2014-07-07 23:37                             ` Ezequiel Garcia
2014-07-07 23:47                             ` Greg Kroah-Hartman
2014-07-07 23:47                               ` Greg Kroah-Hartman
2014-07-07 23:47                               ` Greg Kroah-Hartman
2014-07-08  7:41                             ` Thomas Petazzoni
2014-07-08  7:41                               ` Thomas Petazzoni
2014-07-08  9:20                               ` Paolo Pisati
2014-07-08  9:20                                 ` Paolo Pisati
2014-07-08  9:20                                 ` Paolo Pisati
2014-07-08 12:01                             ` Jason Cooper
2014-07-08 12:01                               ` Jason Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140707233758.GA1456@arch.cereza \
    --to=ezequiel.garcia@free-electrons.com \
    --cc=Hiroshi.DOYU@nokia.com \
    --cc=andrew@lunn.ch \
    --cc=florian.vaussard@epfl.ch \
    --cc=gregkh@linuxfoundation.org \
    --cc=gregory.clement@free-electrons.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jason@lakedaemon.net \
    --cc=joro@8bytes.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.pisati@gmail.com \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.