linux-kernel - Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 03 Oct 2017 14:10:11 +0200
From:   Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Guenter Roeck <linux@...ck-us.net>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-ide@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
        Richard Henderson <rth@...ddle.net>,
        Ivan Kokshaysky <ink@...assic.park.msu.ru>,
        Matt Turner <mattst88@...il.com>
Subject: Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment


On Tuesday, October 03, 2017 11:39:55 AM Bartlomiej Zolnierkiewicz wrote:
> On Monday, October 02, 2017 11:52:47 AM Lorenzo Pieralisi wrote:
> > Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now
> > possible to define IRQ mapping functions on a per PCI host bridge basis.
> > 
> > Actual IRQ allocation is carried out by the pci_assign_irq() function in
> > pci_device_probe() - to make sure a device is assigned an IRQ only if it
> > is probed (ie match a driver); it retrieves a struct pci_host_bridge*
> > for a given PCI device and through {map/swizzle}_irq() it carries out
> > the PCI IRQ allocation.
> > 
> > The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks
> > and pci_assign_irq() to deal with legacy IRQs in
> > 
> > commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in
> > pci_device_probe()")
> > 
> > did not take into account that the IDE subsystem relies on a special
> > purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force
> > devices probe ordering by sidestepping the core PCI bus layer entirely
> > (and therefore pci_device_probe()) by executing the registered IDE PCI
> > drivers probe routines (ie registered with ide_pci_register_driver())
> > through ide_scan_pcidev().
> > 
> > Since this IDE PCI specific probe mechanism bypasses the PCI core
> > code generic probing (ie pci_device_probe()) it also misses the
> > pci_assign_irq() call (among other PCI initialization functions);
> > this triggers IDE PCI devices initialization failures caused
> > by the missing IRQ allocation:
> > 
> > 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
> > 
> > Fix the IRQ allocation issue by adding a pci_assign_irq() call in the
> > ide_scan_pcidev() function before probing the IDE PCI drivers, so that
> > IRQs for a given PCI device are allocated for the IDE PCI drivers to
> > use them for device configuration.
> > 
> > Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()")
> > Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> > Reported-by: Guenter Roeck <linux@...ck-us.net>
> > Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> > Cc: Richard Henderson <rth@...ddle.net>
> > Cc: Ivan Kokshaysky <ink@...assic.park.msu.ru>
> > Cc: David S. Miller <davem@...emloft.net>
> > Cc: Guenter Roeck <linux@...ck-us.net>
> > Cc: Matt Turner <mattst88@...il.com>
> 
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>

IDE specific problems uncovered by pci_assign_irq() change have been
addressed in separate patches, please see:

http://patchwork.ozlabs.org/patch/820859/
http://patchwork.ozlabs.org/patch/820870/

Guenter, could you please verify them (I have only compile tested
them)? [ Just revert Lorenzo's fix and apply both my patches. ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics