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: <CAErSpo5rU8aa=-joApUTYbsQrbrRz9x-7VA3V3H9gDg7k7nj+w@mail.gmail.com>
Date:	Tue, 11 Oct 2011 10:15:34 -0600
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:	"Zhu, DengCheng" <dczhu@...s.com>,
	"jbarnes@...tuousgeek.org" <jbarnes@...tuousgeek.org>,
	"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, 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.

Bjorn
--
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/