* alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
@ 2017-09-19 2:00 Guenter Roeck
2017-09-19 9:12 ` Lorenzo Pieralisi
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2017-09-19 2:00 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-kernel, Bjorn Helgaas, Richard Henderson, Ivan Kokshaysky,
Matt Turner, alpha
Hi,
I see the following runtime warnings in mainline when running alpha images in qemu.
Floppy drive(s): fd0 is 2.88M
ide0: disabled, no IRQ
ide0: failed to initialize IDE interface
ide0: disabling port
cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
cmd64x 0000:00:02.0: can't reserve resources
CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
sysfs: cannot create duplicate filename '/class/ide_port/ide0'
...
Trace:
[<fffffc00003308a0>] __warn+0x160/0x190
[<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
[<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
[<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
[<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
[<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
[<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
[<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
[<fffffc00005b9d64>] device_add+0x2a4/0x7c0
[<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
[<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
[<fffffc00005ba690>] device_create+0x50/0x70
[<fffffc00005df36c>] ide_host_register+0x48c/0xa00
[<fffffc00005df330>] ide_host_register+0x450/0xa00
[<fffffc00005ba2a0>] device_register+0x20/0x50
[<fffffc00005df330>] ide_host_register+0x450/0xa00
[<fffffc00005df944>] ide_host_add+0x64/0xe0
[<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
[<fffffc0000310288>] do_one_initcall+0x68/0x260
[<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
[<fffffc00007b13a0>] kernel_init+0x0/0x1a0
[<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
[<fffffc00007b13a0>] kernel_init+0x0/0x1a0
---[ end trace 24a70433c3e4d374 ]---
ide0: disabling port
[ multiple times ]
A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
Prior to the offending commit, the kernel log looks as follows.
...
Uniform Multi-Platform E-IDE driver
cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
cmd64x 0000:00:02.0: IDE port disabled
cmd64x 0000:00:02.0: 100% native mode on irq 28
PCI: Setting latency timer of device 0000:00:02.0 to 64
ide0: BM-DMA at 0x8040-0x8047
Floppy drive(s): fd0 is 2.88M
ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
ide2 at 0x170-0x177,0x376 on irq 15
ide-gd driver 1.18
ide-cd driver 5.00
...
Reverting the commit is not possible due to context changes.
Bisect log is attached.
Guenter
----------------------
# bad: [ebb2c2437d8008d46796902ff390653822af6cc4] Merge tag 'mmc-v4.14-2' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc
# good: [5969d1bb3082b41eba8fd2c826559abe38ccb6df] Merge branch 'gperf-removal'
git bisect start 'HEAD' '5969d1bb3082'
# bad: [ae46654bcff303b33facbbd04a3ad9c21d303f9b] Merge tag 'armsoc-drivers' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect bad ae46654bcff303b33facbbd04a3ad9c21d303f9b
# bad: [126e76ffbf78d9e948b641aadb265d16c57f5a3d] Merge branch 'for-4.14/block-postmerge' of git://git.kernel.dk/linux-block
git bisect bad 126e76ffbf78d9e948b641aadb265d16c57f5a3d
# bad: [0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c] Merge tag 'pci-v4.14-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci
git bisect bad 0d519f2d1ed1f11e49abc88cfcf6cf13b83ba14c
# good: [cf5f9cc8e4e5e8e0ecc35f1c904d98f889be2c0f] Merge branch 'pci/hotplug' into next
git bisect good cf5f9cc8e4e5e8e0ecc35f1c904d98f889be2c0f
# good: [5f54c8b2d4fad95d1f8ecbe023ebe6038e6d3760] Merge branch 'kvm-ppc-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc
git bisect good 5f54c8b2d4fad95d1f8ecbe023ebe6038e6d3760
# good: [369130b63178e0e2f863a2da2a5ad0238ded6d9d] selftests: Enhance kselftest_harness.h to print which assert failed
git bisect good 369130b63178e0e2f863a2da2a5ad0238ded6d9d
# bad: [d872694bac212f76ca13fd20a85e5c1bdb53a945] Merge branch 'pci/pm' into next
git bisect bad d872694bac212f76ca13fd20a85e5c1bdb53a945
# bad: [d4fdf844c9c3debc080aea1be8b71d9d0aaa01dc] Merge branch 'pci/irq-fixups' into next
git bisect bad d4fdf844c9c3debc080aea1be8b71d9d0aaa01dc
# bad: [04c81c7293df875ca6a46e2c9a272c7ec72e5145] MIPS: PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks
git bisect bad 04c81c7293df875ca6a46e2c9a272c7ec72e5145
# good: [20d693225ab78f0651b0e116b74196aaf8a950bb] sh/PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks
git bisect good 20d693225ab78f0651b0e116b74196aaf8a950bb
# bad: [19cc4c843f40c6110dd07270414586e7fe4121b2] m68k/PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks
git bisect bad 19cc4c843f40c6110dd07270414586e7fe4121b2
# bad: [0e4c2eeb758a91e68b9eaf7a4bee9bd5ed97ff2b] alpha/PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks
git bisect bad 0e4c2eeb758a91e68b9eaf7a4bee9bd5ed97ff2b
# first bad commit: [0e4c2eeb758a91e68b9eaf7a4bee9bd5ed97ff2b] alpha/PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-19 2:00 alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...") Guenter Roeck
@ 2017-09-19 9:12 ` Lorenzo Pieralisi
2017-09-19 18:02 ` Guenter Roeck
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Pieralisi @ 2017-09-19 9:12 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kernel, Bjorn Helgaas, Richard Henderson, Ivan Kokshaysky,
Matt Turner, alpha
On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> Hi,
>
> I see the following runtime warnings in mainline when running alpha images in qemu.
>
>
> Floppy drive(s): fd0 is 2.88M
> ide0: disabled, no IRQ
> ide0: failed to initialize IDE interface
> ide0: disabling port
> cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> cmd64x 0000:00:02.0: can't reserve resources
> CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> ...
>
> Trace:
> [<fffffc00003308a0>] __warn+0x160/0x190
> [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> [<fffffc00005ba690>] device_create+0x50/0x70
> [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> [<fffffc00005df330>] ide_host_register+0x450/0xa00
> [<fffffc00005ba2a0>] device_register+0x20/0x50
> [<fffffc00005df330>] ide_host_register+0x450/0xa00
> [<fffffc00005df944>] ide_host_add+0x64/0xe0
> [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> [<fffffc0000310288>] do_one_initcall+0x68/0x260
> [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
>
> ---[ end trace 24a70433c3e4d374 ]---
> ide0: disabling port
>
> [ multiple times ]
>
> A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
>
> Prior to the offending commit, the kernel log looks as follows.
>
> ...
> Uniform Multi-Platform E-IDE driver
> cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> cmd64x 0000:00:02.0: IDE port disabled
> cmd64x 0000:00:02.0: 100% native mode on irq 28
> PCI: Setting latency timer of device 0000:00:02.0 to 64
> ide0: BM-DMA at 0x8040-0x8047
> Floppy drive(s): fd0 is 2.88M
> ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> ide2 at 0x170-0x177,0x376 on irq 15
> ide-gd driver 1.18
> ide-cd driver 5.00
> ...
>
> Reverting the commit is not possible due to context changes.
>
> Bisect log is attached.
Ok thanks. Can you please check if the diff below fixes the issue ?
I think the problem is that now we allocate the IRQ at device_probe
instead of device_add time, if the patch below fixes the issue the
question is whether we want to move pci_assign_irq() to pci_device_add()
for ALL PCI devices or just fix this for IDE subsytem.
Lorenzo
-- >8 --
diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
index 112d2fe..94ca9cc 100644
--- a/drivers/ide/setup-pci.c
+++ b/drivers/ide/setup-pci.c
@@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
{
int pciirq, ret;
+ pci_assign_irq(dev);
+
/*
* Can we trust the reported IRQ?
*/
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-19 9:12 ` Lorenzo Pieralisi
@ 2017-09-19 18:02 ` Guenter Roeck
2017-09-20 11:31 ` Lorenzo Pieralisi
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2017-09-19 18:02 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-kernel, Bjorn Helgaas, Richard Henderson, Ivan Kokshaysky,
Matt Turner, alpha
On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > Hi,
> >
> > I see the following runtime warnings in mainline when running alpha images in qemu.
> >
> >
> > Floppy drive(s): fd0 is 2.88M
> > ide0: disabled, no IRQ
> > ide0: failed to initialize IDE interface
> > ide0: disabling port
> > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > cmd64x 0000:00:02.0: can't reserve resources
> > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > ...
> >
> > Trace:
> > [<fffffc00003308a0>] __warn+0x160/0x190
> > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > [<fffffc00005ba690>] device_create+0x50/0x70
> > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> >
> > ---[ end trace 24a70433c3e4d374 ]---
> > ide0: disabling port
> >
> > [ multiple times ]
> >
> > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> >
> > Prior to the offending commit, the kernel log looks as follows.
> >
> > ...
> > Uniform Multi-Platform E-IDE driver
> > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > cmd64x 0000:00:02.0: IDE port disabled
> > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > ide0: BM-DMA at 0x8040-0x8047
> > Floppy drive(s): fd0 is 2.88M
> > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > ide2 at 0x170-0x177,0x376 on irq 15
> > ide-gd driver 1.18
> > ide-cd driver 5.00
> > ...
> >
> > Reverting the commit is not possible due to context changes.
> >
> > Bisect log is attached.
>
> Ok thanks. Can you please check if the diff below fixes the issue ?
>
It does. Feel free to add
Tested-by: Guenter Roeck <linux@roeck-us.net>
to the actual patch.
> I think the problem is that now we allocate the IRQ at device_probe
> instead of device_add time, if the patch below fixes the issue the
> question is whether we want to move pci_assign_irq() to pci_device_add()
> for ALL PCI devices or just fix this for IDE subsytem.
>
That I can not answer ...
Guenter
> Lorenzo
>
> -- >8 --
> diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> index 112d2fe..94ca9cc 100644
> --- a/drivers/ide/setup-pci.c
> +++ b/drivers/ide/setup-pci.c
> @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> {
> int pciirq, ret;
>
> + pci_assign_irq(dev);
> +
> /*
> * Can we trust the reported IRQ?
> */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-19 18:02 ` Guenter Roeck
@ 2017-09-20 11:31 ` Lorenzo Pieralisi
2017-09-27 10:30 ` Lorenzo Pieralisi
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Pieralisi @ 2017-09-20 11:31 UTC (permalink / raw)
To: Guenter Roeck, Bjorn Helgaas
Cc: linux-kernel, Richard Henderson, Ivan Kokshaysky, Matt Turner, alpha
On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > Hi,
> > >
> > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > >
> > >
> > > Floppy drive(s): fd0 is 2.88M
> > > ide0: disabled, no IRQ
> > > ide0: failed to initialize IDE interface
> > > ide0: disabling port
> > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > cmd64x 0000:00:02.0: can't reserve resources
> > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > ...
> > >
> > > Trace:
> > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > >
> > > ---[ end trace 24a70433c3e4d374 ]---
> > > ide0: disabling port
> > >
> > > [ multiple times ]
> > >
> > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > >
> > > Prior to the offending commit, the kernel log looks as follows.
> > >
> > > ...
> > > Uniform Multi-Platform E-IDE driver
> > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > cmd64x 0000:00:02.0: IDE port disabled
> > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > ide0: BM-DMA at 0x8040-0x8047
> > > Floppy drive(s): fd0 is 2.88M
> > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > ide2 at 0x170-0x177,0x376 on irq 15
> > > ide-gd driver 1.18
> > > ide-cd driver 5.00
> > > ...
> > >
> > > Reverting the commit is not possible due to context changes.
> > >
> > > Bisect log is attached.
> >
> > Ok thanks. Can you please check if the diff below fixes the issue ?
> >
> It does. Feel free to add
>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
>
> to the actual patch.
>
> > I think the problem is that now we allocate the IRQ at device_probe
> > instead of device_add time, if the patch below fixes the issue the
> > question is whether we want to move pci_assign_irq() to pci_device_add()
> > for ALL PCI devices or just fix this for IDE subsytem.
> >
>
> That I can not answer ...
It is not a clean-cut answer. Moving pci_assign_irq() from
pci_device_probe()
to
pci_device_add()
would fix this issue too and just map/swizzle() irqs for ALL PCI devices
earlier (even ones that are not even probed); it should have no side
effects (famous last words) and would be a cleaner fix.
Fix below is fine but I am not a big fan of it, I would like to
get Bjorn's opinion before I send a fix out.
Thanks,
Lorenzo
> Guenter
>
> > Lorenzo
> >
> > -- >8 --
> > diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> > index 112d2fe..94ca9cc 100644
> > --- a/drivers/ide/setup-pci.c
> > +++ b/drivers/ide/setup-pci.c
> > @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> > {
> > int pciirq, ret;
> >
> > + pci_assign_irq(dev);
> > +
> > /*
> > * Can we trust the reported IRQ?
> > */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-20 11:31 ` Lorenzo Pieralisi
@ 2017-09-27 10:30 ` Lorenzo Pieralisi
2017-09-27 19:55 ` Bjorn Helgaas
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Pieralisi @ 2017-09-27 10:30 UTC (permalink / raw)
To: Guenter Roeck, Bjorn Helgaas
Cc: linux-kernel, Richard Henderson, Ivan Kokshaysky, Matt Turner, alpha
Bjorn, Guenter,
On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > Hi,
> > > >
> > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > >
> > > >
> > > > Floppy drive(s): fd0 is 2.88M
> > > > ide0: disabled, no IRQ
> > > > ide0: failed to initialize IDE interface
> > > > ide0: disabling port
> > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > > cmd64x 0000:00:02.0: can't reserve resources
> > > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > ------------[ cut here ]------------
> > > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > > ...
> > > >
> > > > Trace:
> > > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > >
> > > > ---[ end trace 24a70433c3e4d374 ]---
> > > > ide0: disabling port
> > > >
> > > > [ multiple times ]
> > > >
> > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > >
> > > > Prior to the offending commit, the kernel log looks as follows.
> > > >
> > > > ...
> > > > Uniform Multi-Platform E-IDE driver
> > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > ide0: BM-DMA at 0x8040-0x8047
> > > > Floppy drive(s): fd0 is 2.88M
> > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > ide-gd driver 1.18
> > > > ide-cd driver 5.00
> > > > ...
> > > >
> > > > Reverting the commit is not possible due to context changes.
> > > >
> > > > Bisect log is attached.
> > >
> > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > >
> > It does. Feel free to add
> >
> > Tested-by: Guenter Roeck <linux@roeck-us.net>
> >
> > to the actual patch.
> >
> > > I think the problem is that now we allocate the IRQ at device_probe
> > > instead of device_add time, if the patch below fixes the issue the
> > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > for ALL PCI devices or just fix this for IDE subsytem.
> > >
> >
> > That I can not answer ...
>
> It is not a clean-cut answer. Moving pci_assign_irq() from
>
> pci_device_probe()
>
> to
>
> pci_device_add()
>
> would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> earlier (even ones that are not even probed); it should have no side
> effects (famous last words) and would be a cleaner fix.
>
> Fix below is fine but I am not a big fan of it, I would like to
> get Bjorn's opinion before I send a fix out.
I think that for now the fix below should be ok, I would like to
send it out before -rc3, the only question is related to the
"Fixes:" tag, I am reluctant to add:
Fixes: 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge mapping hooks")
since this is not necessarily just an alpha/PCI bug (ie it is related to
IDE legacy IRQ routing - which Guenter hit on Alpha), the tag can be
misleading in that sense.
Please let me know what you think, I will send out the patch then.
Thanks,
Lorenzo
> Thanks,
> Lorenzo
>
> > Guenter
> >
> > > Lorenzo
> > >
> > > -- >8 --
> > > diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> > > index 112d2fe..94ca9cc 100644
> > > --- a/drivers/ide/setup-pci.c
> > > +++ b/drivers/ide/setup-pci.c
> > > @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> > > {
> > > int pciirq, ret;
> > >
> > > + pci_assign_irq(dev);
> > > +
> > > /*
> > > * Can we trust the reported IRQ?
> > > */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-27 10:30 ` Lorenzo Pieralisi
@ 2017-09-27 19:55 ` Bjorn Helgaas
2017-09-28 9:25 ` Lorenzo Pieralisi
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2017-09-27 19:55 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Guenter Roeck, Bjorn Helgaas, linux-kernel, Richard Henderson,
Ivan Kokshaysky, Matt Turner, alpha, linux-pci, David S. Miller,
linux-ide
[+cc linux-pci, linux-ide, DaveM]
On Wed, Sep 27, 2017 at 11:30:34AM +0100, Lorenzo Pieralisi wrote:
> Bjorn, Guenter,
>
> On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > > Hi,
> > > > >
> > > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > > >
> > > > >
> > > > > Floppy drive(s): fd0 is 2.88M
> > > > > ide0: disabled, no IRQ
> > > > > ide0: failed to initialize IDE interface
> > > > > ide0: disabling port
> > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > > > cmd64x 0000:00:02.0: can't reserve resources
> > > > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > ------------[ cut here ]------------
> > > > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > > > ...
> > > > >
> > > > > Trace:
> > > > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > >
> > > > > ---[ end trace 24a70433c3e4d374 ]---
> > > > > ide0: disabling port
> > > > >
> > > > > [ multiple times ]
> > > > >
> > > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > > >
> > > > > Prior to the offending commit, the kernel log looks as follows.
> > > > >
> > > > > ...
> > > > > Uniform Multi-Platform E-IDE driver
> > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > > ide0: BM-DMA at 0x8040-0x8047
> > > > > Floppy drive(s): fd0 is 2.88M
> > > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > > ide-gd driver 1.18
> > > > > ide-cd driver 5.00
> > > > > ...
> > > > >
> > > > > Reverting the commit is not possible due to context changes.
> > > > >
> > > > > Bisect log is attached.
> > > >
> > > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > > >
> > > It does. Feel free to add
> > >
> > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > >
> > > to the actual patch.
> > >
> > > > I think the problem is that now we allocate the IRQ at device_probe
> > > > instead of device_add time, if the patch below fixes the issue the
> > > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > > for ALL PCI devices or just fix this for IDE subsytem.
> > > >
> > >
> > > That I can not answer ...
> >
> > It is not a clean-cut answer. Moving pci_assign_irq() from
> >
> > pci_device_probe()
> >
> > to
> >
> > pci_device_add()
> >
> > would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> > earlier (even ones that are not even probed); it should have no side
> > effects (famous last words) and would be a cleaner fix.
Hmmm, this is ugly. I *think* this is related to ide_pci_init_two().
We already call pci_assign_irq() from pci_device_probe(), where we try
to bind one device to a driver. But ide_pci_init_two() deals with
*two* devices: the one pci_device_probe() is trying to bind, and
another random one found via pci_get_slot(). We have not called
pci_assign_irq() for this second device.
That breaks the model the PCI core expects, where we call the driver
probe method for device X, and that method deals only with device X.
We can fix this by doing the pci_assign_irq() either in the PCI core's
device add path or in the driver's probe path (ide_pci_init_two(), as
in the patch below). Doing it in the PCI core is sort of ugly because
there's no obvious reason why we should map/swizzle the IRQ if there's
no driver.
Doing it in the driver is also sort of ugly because this is something
that should be confined to the core (pci_assign_irq() probably should
be declared in drivers/pci/pci.h so it's not available outside the
core).
I think the patch below means we call pci_assign_irq() *twice* on the
device we're binding: once from pci_device_probe() and again from
do_ide_setup_pci_device(). And I guess we might call it twice for the
second device, too: once from do_ide_setup_pci_device() and again if
pci_device_probe() tries to bind it. It's probably idempotent, but it
does seem a little ugly.
I guess I agree that calling pci_assign_irq() from pci_device_add() is
the lesser evil. That will do it unnecessarily in cases where a
device doesn't have a driver, but it should be safe. It sets
pdev->irq but probably doesn't touch the device itself.
We do more IRQ setup at pci_enable_device()-time (or probe-time for
arm64) via acpi_pci_irq_enable(). This also updates pdev->irq. I'd
rather not do that part at device add-time, because it does things
like allocate and enable interrupt links, and we shouldn't do that
until we know we have a driver for the device.
But this is potentially another problem for this "second-device" thing
IDE does. We've called pci_assign_irq() but not acpi_pci_irq_enable()
for the second device, so we may still get the wrong IRQ for it.
> > Fix below is fine but I am not a big fan of it, I would like to
> > get Bjorn's opinion before I send a fix out.
>
> I think that for now the fix below should be ok, I would like to
> send it out before -rc3, the only question is related to the
> "Fixes:" tag, I am reluctant to add:
>
> Fixes: 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge mapping hooks")
>
> since this is not necessarily just an alpha/PCI bug (ie it is related to
> IDE legacy IRQ routing - which Guenter hit on Alpha), the tag can be
> misleading in that sense.
>
> Please let me know what you think, I will send out the patch then.
>
> Thanks,
> Lorenzo
>
> > Thanks,
> > Lorenzo
> >
> > > Guenter
> > >
> > > > Lorenzo
> > > >
> > > > -- >8 --
> > > > diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> > > > index 112d2fe..94ca9cc 100644
> > > > --- a/drivers/ide/setup-pci.c
> > > > +++ b/drivers/ide/setup-pci.c
> > > > @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> > > > {
> > > > int pciirq, ret;
> > > >
> > > > + pci_assign_irq(dev);
> > > > +
> > > > /*
> > > > * Can we trust the reported IRQ?
> > > > */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-27 19:55 ` Bjorn Helgaas
@ 2017-09-28 9:25 ` Lorenzo Pieralisi
2017-09-28 17:53 ` Bjorn Helgaas
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Pieralisi @ 2017-09-28 9:25 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Guenter Roeck, Bjorn Helgaas, linux-kernel, Richard Henderson,
Ivan Kokshaysky, Matt Turner, alpha, linux-pci, David S. Miller,
linux-ide
On Wed, Sep 27, 2017 at 02:55:02PM -0500, Bjorn Helgaas wrote:
> [+cc linux-pci, linux-ide, DaveM]
>
> On Wed, Sep 27, 2017 at 11:30:34AM +0100, Lorenzo Pieralisi wrote:
> > Bjorn, Guenter,
> >
> > On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> > > On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > > > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > > > >
> > > > > >
> > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > ide0: disabled, no IRQ
> > > > > > ide0: failed to initialize IDE interface
> > > > > > ide0: disabling port
> > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > > > > cmd64x 0000:00:02.0: can't reserve resources
> > > > > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > ------------[ cut here ]------------
> > > > > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > > > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > > > > ...
> > > > > >
> > > > > > Trace:
> > > > > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > > > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > > > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > > > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > > > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > > > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > > > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > > > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > > > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > >
> > > > > > ---[ end trace 24a70433c3e4d374 ]---
> > > > > > ide0: disabling port
> > > > > >
> > > > > > [ multiple times ]
> > > > > >
> > > > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > > > >
> > > > > > Prior to the offending commit, the kernel log looks as follows.
> > > > > >
> > > > > > ...
> > > > > > Uniform Multi-Platform E-IDE driver
> > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > > > ide0: BM-DMA at 0x8040-0x8047
> > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > > > ide-gd driver 1.18
> > > > > > ide-cd driver 5.00
> > > > > > ...
> > > > > >
> > > > > > Reverting the commit is not possible due to context changes.
> > > > > >
> > > > > > Bisect log is attached.
> > > > >
> > > > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > > > >
> > > > It does. Feel free to add
> > > >
> > > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > > >
> > > > to the actual patch.
> > > >
> > > > > I think the problem is that now we allocate the IRQ at device_probe
> > > > > instead of device_add time, if the patch below fixes the issue the
> > > > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > > > for ALL PCI devices or just fix this for IDE subsytem.
> > > > >
> > > >
> > > > That I can not answer ...
> > >
> > > It is not a clean-cut answer. Moving pci_assign_irq() from
> > >
> > > pci_device_probe()
> > >
> > > to
> > >
> > > pci_device_add()
> > >
> > > would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> > > earlier (even ones that are not even probed); it should have no side
> > > effects (famous last words) and would be a cleaner fix.
>
> Hmmm, this is ugly. I *think* this is related to ide_pci_init_two().
> We already call pci_assign_irq() from pci_device_probe(), where we try
> to bind one device to a driver. But ide_pci_init_two() deals with
> *two* devices: the one pci_device_probe() is trying to bind, and
> another random one found via pci_get_slot(). We have not called
> pci_assign_irq() for this second device.
>
> That breaks the model the PCI core expects, where we call the driver
> probe method for device X, and that method deals only with device X.
>
> We can fix this by doing the pci_assign_irq() either in the PCI core's
> device add path or in the driver's probe path (ide_pci_init_two(), as
> in the patch below). Doing it in the PCI core is sort of ugly because
> there's no obvious reason why we should map/swizzle the IRQ if there's
> no driver.
>
> Doing it in the driver is also sort of ugly because this is something
> that should be confined to the core (pci_assign_irq() probably should
> be declared in drivers/pci/pci.h so it's not available outside the
> core).
>
> I think the patch below means we call pci_assign_irq() *twice* on the
> device we're binding: once from pci_device_probe() and again from
> do_ide_setup_pci_device(). And I guess we might call it twice for the
> second device, too: once from do_ide_setup_pci_device() and again if
> pci_device_probe() tries to bind it. It's probably idempotent, but it
> does seem a little ugly.
>
> I guess I agree that calling pci_assign_irq() from pci_device_add() is
> the lesser evil. That will do it unnecessarily in cases where a
> device doesn't have a driver, but it should be safe. It sets
> pdev->irq but probably doesn't touch the device itself.
I will do that, actually this would reinstate the exact boot stage at
which pci_fixup_irqs() was called (except that pci_assign_irq() does
that on a per host-bridge basis and in core code) because
pci_fixup_irqs() was called after enumeration before PCI devices were
added (ie before calling pci_bus_add_devices()), it *should* be safe.
> We do more IRQ setup at pci_enable_device()-time (or probe-time for
> arm64) via acpi_pci_irq_enable(). This also updates pdev->irq. I'd
> rather not do that part at device add-time, because it does things
> like allocate and enable interrupt links, and we shouldn't do that
> until we know we have a driver for the device.
>
> But this is potentially another problem for this "second-device" thing
> IDE does. We've called pci_assign_irq() but not acpi_pci_irq_enable()
> for the second device, so we may still get the wrong IRQ for it.
I have not touched the ACPI irq assignment path with my patches on
purpose. Put it differently, PNP0A03 host bridges do not have any
{map/swizzle}_irq() hook (yet) so pci_assign_irq() does nothing on
them. ACPI based platforms never used pci_fixup_irqs() so I changed
nothing on them (and I doubt I can given how complicated x86 legacy
IRQ assignment code is in arch code).
I will send the fix shortly - even though I am not sure I can add
a sensible Fixes: tag to it, I think:
Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in
pci_device_probe()")
is what makes most sense.
Thanks,
Lorenzo
> > > Fix below is fine but I am not a big fan of it, I would like to
> > > get Bjorn's opinion before I send a fix out.
> >
> > I think that for now the fix below should be ok, I would like to
> > send it out before -rc3, the only question is related to the
> > "Fixes:" tag, I am reluctant to add:
> >
> > Fixes: 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge mapping hooks")
> >
> > since this is not necessarily just an alpha/PCI bug (ie it is related to
> > IDE legacy IRQ routing - which Guenter hit on Alpha), the tag can be
> > misleading in that sense.
> >
> > Please let me know what you think, I will send out the patch then.
> >
> > Thanks,
> > Lorenzo
> >
> > > Thanks,
> > > Lorenzo
> > >
> > > > Guenter
> > > >
> > > > > Lorenzo
> > > > >
> > > > > -- >8 --
> > > > > diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> > > > > index 112d2fe..94ca9cc 100644
> > > > > --- a/drivers/ide/setup-pci.c
> > > > > +++ b/drivers/ide/setup-pci.c
> > > > > @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> > > > > {
> > > > > int pciirq, ret;
> > > > >
> > > > > + pci_assign_irq(dev);
> > > > > +
> > > > > /*
> > > > > * Can we trust the reported IRQ?
> > > > > */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
2017-09-28 9:25 ` Lorenzo Pieralisi
@ 2017-09-28 17:53 ` Bjorn Helgaas
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2017-09-28 17:53 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Guenter Roeck, Bjorn Helgaas, linux-kernel, Richard Henderson,
Ivan Kokshaysky, Matt Turner, alpha, linux-pci, David S. Miller,
linux-ide
On Thu, Sep 28, 2017 at 10:25:33AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Sep 27, 2017 at 02:55:02PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 27, 2017 at 11:30:34AM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> > > > On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > > > > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > > > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > > > > >
> > > > > > >
> > > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > > ide0: disabled, no IRQ
> > > > > > > ide0: failed to initialize IDE interface
> > > > > > > ide0: disabling port
> > > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > > > > > cmd64x 0000:00:02.0: can't reserve resources
> > > > > > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > > ------------[ cut here ]------------
> > > > > > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > > > > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > > > > > ...
> > > > > > >
> > > > > > > Trace:
> > > > > > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > > > > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > > > > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > > > > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > > > > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > > > > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > > > > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > > > > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > > > > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > > >
> > > > > > > ---[ end trace 24a70433c3e4d374 ]---
> > > > > > > ide0: disabling port
> > > > > > >
> > > > > > > [ multiple times ]
> > > > > > >
> > > > > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > > > > >
> > > > > > > Prior to the offending commit, the kernel log looks as follows.
> > > > > > >
> > > > > > > ...
> > > > > > > Uniform Multi-Platform E-IDE driver
> > > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > > > > ide0: BM-DMA at 0x8040-0x8047
> > > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > > > > ide-gd driver 1.18
> > > > > > > ide-cd driver 5.00
> > > > > > > ...
> > > > > > >
> > > > > > > Reverting the commit is not possible due to context changes.
> > > > > > >
> > > > > > > Bisect log is attached.
> > > > > >
> > > > > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > > > > >
> > > > > It does. Feel free to add
> > > > >
> > > > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > > > >
> > > > > to the actual patch.
> > > > >
> > > > > > I think the problem is that now we allocate the IRQ at device_probe
> > > > > > instead of device_add time, if the patch below fixes the issue the
> > > > > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > > > > for ALL PCI devices or just fix this for IDE subsytem.
> > > > > >
> > > > >
> > > > > That I can not answer ...
> > > >
> > > > It is not a clean-cut answer. Moving pci_assign_irq() from
> > > >
> > > > pci_device_probe()
> > > >
> > > > to
> > > >
> > > > pci_device_add()
> > > >
> > > > would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> > > > earlier (even ones that are not even probed); it should have no side
> > > > effects (famous last words) and would be a cleaner fix.
> >
> > Hmmm, this is ugly. I *think* this is related to ide_pci_init_two().
> > We already call pci_assign_irq() from pci_device_probe(), where we try
> > to bind one device to a driver. But ide_pci_init_two() deals with
> > *two* devices: the one pci_device_probe() is trying to bind, and
> > another random one found via pci_get_slot(). We have not called
> > pci_assign_irq() for this second device.
> >
> > That breaks the model the PCI core expects, where we call the driver
> > probe method for device X, and that method deals only with device X.
> >
> > We can fix this by doing the pci_assign_irq() either in the PCI core's
> > device add path or in the driver's probe path (ide_pci_init_two(), as
> > in the patch below). Doing it in the PCI core is sort of ugly because
> > there's no obvious reason why we should map/swizzle the IRQ if there's
> > no driver.
> >
> > Doing it in the driver is also sort of ugly because this is something
> > that should be confined to the core (pci_assign_irq() probably should
> > be declared in drivers/pci/pci.h so it's not available outside the
> > core).
> >
> > I think the patch below means we call pci_assign_irq() *twice* on the
> > device we're binding: once from pci_device_probe() and again from
> > do_ide_setup_pci_device(). And I guess we might call it twice for the
> > second device, too: once from do_ide_setup_pci_device() and again if
> > pci_device_probe() tries to bind it. It's probably idempotent, but it
> > does seem a little ugly.
> >
> > I guess I agree that calling pci_assign_irq() from pci_device_add() is
> > the lesser evil. That will do it unnecessarily in cases where a
> > device doesn't have a driver, but it should be safe. It sets
> > pdev->irq but probably doesn't touch the device itself.
>
> I will do that, actually this would reinstate the exact boot stage at
> which pci_fixup_irqs() was called (except that pci_assign_irq() does
> that on a per host-bridge basis and in core code) because
> pci_fixup_irqs() was called after enumeration before PCI devices were
> added (ie before calling pci_bus_add_devices()), it *should* be safe.
>
> > We do more IRQ setup at pci_enable_device()-time (or probe-time for
> > arm64) via acpi_pci_irq_enable(). This also updates pdev->irq. I'd
> > rather not do that part at device add-time, because it does things
> > like allocate and enable interrupt links, and we shouldn't do that
> > until we know we have a driver for the device.
> >
> > But this is potentially another problem for this "second-device" thing
> > IDE does. We've called pci_assign_irq() but not acpi_pci_irq_enable()
> > for the second device, so we may still get the wrong IRQ for it.
>
> I have not touched the ACPI irq assignment path with my patches on
> purpose. Put it differently, PNP0A03 host bridges do not have any
> {map/swizzle}_irq() hook (yet) so pci_assign_irq() does nothing on
> them. ACPI based platforms never used pci_fixup_irqs() so I changed
> nothing on them (and I doubt I can given how complicated x86 legacy
> IRQ assignment code is in arch code).
Right. This isn't a problem with your patches; it's just a problem
with the ide_pci_init_two() strategy of looking at dev->irq for a
device for which the core hasn't called the probe path.
Bjorn
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-28 17:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 2:00 alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...") Guenter Roeck
2017-09-19 9:12 ` Lorenzo Pieralisi
2017-09-19 18:02 ` Guenter Roeck
2017-09-20 11:31 ` Lorenzo Pieralisi
2017-09-27 10:30 ` Lorenzo Pieralisi
2017-09-27 19:55 ` Bjorn Helgaas
2017-09-28 9:25 ` Lorenzo Pieralisi
2017-09-28 17:53 ` 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).