linux-kernel - Re: [RESEND PATCH v3 0/2] Pass resources to pci_create

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: <20111102141410.4d44a4eb@jbarnes-desktop>
Date:	Wed, 2 Nov 2011 14:14:10 -0700
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	"Zhu, DengCheng" <dczhu@...s.com>,
	"ralf@...ux-mips.org" <ralf@...ux-mips.org>,
	"monstr@...str.eu" <monstr@...str.eu>,
	"paulus@...ba.org" <paulus@...ba.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
	"Barzilay, Eyal" <eyal@...s.com>,
	"Fortuna, Zenon" <zenon@...s.com>,
	"dengcheng.zhu@...il.com" <dengcheng.zhu@...il.com>
Subject: Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and
 fix MIPS PCI resources

On Tue, 11 Oct 2011 10:15:34 -0600
Bjorn Helgaas <bhelgaas@...gle.com> wrote:

> On Tue, Oct 11, 2011 at 1:48 AM, Benjamin Herrenschmidt
> <benh@...nel.crashing.org> wrote:
> 
> > I must admit I don't completely understand what this patch is about,
> > other than it will most probably break the way we do resource management
> > on powerpc :-)
> >
> > I don't understand the point about conflicts in scan_slot and I don't
> > see what you win by "settling down early". Also keep in mind that the
> > resources read from the device need to be remapped on some archs like
> > powerpc which we do from a header quirk at the moment.
> 
> These patches only deal with root bus resources, i.e., the
> non-architected PCI host bridge windows.  They don't have anything to
> do with normal PCI BARs.
> 
> MIPS sets up root buses differently than powerpc, so it has a problem
> that powerpc doesn't have.  Here's the original MIPS flow (before this
> series):
> 
>               pci_scan_bus
>                 pci_scan_bus_parented
>                   pci_create_bus        <-- A create root bus
>                   pci_scan_child_bus
>                     pci_scan_slot
>                       pci_scan_single_device
>                         pci_scan_device
>                           pci_setup_device
>                             pci_fixup_device (pci_fixup_early)  <-- B
>                         pci_device_add
>                           pci_fixup_device (pci_fixup_header)   <-- C
>                     pcibios_fixup_bus   <-- D fill in root bus resources
> 
> At point A, we allocate a struct pci_bus for the root bus.
> pci_create_bus() fills in defaults for the resources available on that
> bus: ioport_resource and iomem_resource, which cover all possible
> address space.  Later at point D, we replace those defaults with the
> correct resources (hose->io_resource and hose->mem_resource in this
> MIPS case).
> 
> The problem is that the root bus resources are wrong during the
> interval between A and D.  Anything that looks at them may break.  In
> the case Deng-Cheng found, the quirk_piix4_acpi() fixup at point C
> claimed a region, which incorrectly became the child of
> ioport_resource instead of host->io_resource.
> 
> Deng-Cheng's patches close this window by basically combining the
> fixup at D with the root bus creation at A.
> 
> Powerpc doesn't have the same problem because it calls
> pci_create_bus() directly so it can fix the root bus resources with
> pcibios_setup_phb_resources() *before* it scans the bus.
> 
> Even though powerpc and many other architectures don't have the MIPS
> problem, I think it's worth changing the code because the existing
> pattern is poor.  In almost all cases, we know what the host bridge
> apertures are before we create the root bus.  It's error-prone to have
> pci_create_bus() fill in default resources, then rely on the
> architecture to fix that up later.  I think it's better to supply the
> resources up front.

Ben, with the above explained are you ok with this change?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)