Re: [RESEND PATCH v3 0/2] Pass resources to pci_create_bus() and fix MIPS PCI resources

[Jesse Barnes <jbarnes@virtuousgeek.org>]

[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]

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

> On Tue, Oct 11, 2011 at 1:48 AM, Benjamin Herrenschmidt
> <benh@kernel.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]