linux-kernel - Re: [PATCH] PCI: Workaround missing pci_set

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]

Message-ID: <1383569048.9623.16.camel@x220.thuisdomein>
Date:	Mon, 04 Nov 2013 13:44:08 +0100
From:	Paul Bolle <pebolle@...cali.nl>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	Yinghai Lu <yinghai@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

On Fri, 2013-10-04 at 09:55 -0600, Bjorn Helgaas wrote:
> On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu <yinghai@...nel.org> wrote:
> > On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> >> On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> >>> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
> >>>
> >>>       pci_enable_bridge(dev->bus->self);
> >>>
> >>> -     if (pci_is_enabled(dev))
> >>> +     if (pci_is_enabled(dev)) {
> >>> +             if (!dev->is_busmaster) {
> >>> +                     dev_warn(&dev->dev, "driver skip pci_set_master, fix it!\n");
> >>
> >> I know this is already in Linus' tree, but if we're going to enable
> >> bus mastering here, what's the point of the warning?  If somebody
> >> fixes the driver by adding a pci_set_master() call there, does that
> >> improve something?
> >
> > Help us to catch other offender and fix them.
> 
> What is improved by doing it in the driver instead of here?

After booting v3.12 for the first time on a laptop I noticed two new
warnings: 
    <4>[    4.427959] pcieport 0000:00:1c.4: driver skip pci_set_master, fix it!
    <4>[    4.448630] pcieport 0000:00:1c.1: driver skip pci_set_master, fix it!

These warnings aren't entirely clear, but luckily they are easily
greppable. It turns out they can be traced back to this patch.

So some further grepping, looking at the code, etc. suggests these
warnings could be silenced by calling pci_set_master() before calling
pci_enable_device(). Ie, reverse the current order of those calls in
drivers/pci/pcie/portdrv_core.c:pcie_port_device_register(). Is that
correct? But what should then be done if pci_enable_device() fails?

And Bjorn's question - what's the point of this warning if
pci_set_master() will be called anyway - also came up when I looked at
that code segment for the first time. But I'm not familiar with the PCI
code.


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/