Re: Doing _OSC right

[Andrew Patterson <andrew.patterson@hp.com>]

On Mon, 2008-10-27 at 16:12 +0000, Andrew Patterson wrote:
> On Fri, 2008-10-24 at 23:30 -0600, Matthew Wilcox wrote:
> > Andrew Patterson recently reported a problem where a machine with dozens
> > of PCIe root bridges would take half an hour just calling _OSC.  The
> > root cause is the AER code calling:
> > 
> > 	pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
> > 
> > for each bridge.  pcie_osc_support_set() also iterates over each bridge,
> > so _OSC is called a quadratic number of times.  Obviously, this isn't
> > hard to fix -- just moving the call to pcie_osc_support_set() to
> > aer_service_init() would do.
> > 
> > But why should a driver be calling pcie_osc_support_set() anyway?  This
> > is something the PCI core should be taking care of for it.
> > 
> > The patch below illustrates one possible solution.  I create a new
> > function (pci_acpi_osc_support()) which is called from pci_root.c before
> > we call any other methods on the device (as recommended by the ACPI spec).
> > Since we know what capabilities the system supports, individual modules
> > do not now need to inform the core of their support for various
> > capabilities.
> > 
> > I do not think this patch is perfect.  One defect is that there are
> > now no callers of pci_osc_support_set nor pcie_osc_support_set, so they
> > could be deleted.
> > 
> > Another problem is that if, for example, MSI is disabled at boot time,
> > we will incorrectly inform ACPI that we support MSI.  So something more
> > flexible is needed.
> 
> Check pci_msi_enable? After exposing it.
> 
> > 
> > I think all the existing _OSC code should be moved from pci-acpi.c to
> > pci_root.c.  I also think the acpi_osc_data should be made part of the
> > acpi_pci_root struct, eliminating a separate allocation.
> > 
> > We could also use an interface that iterates over all existing busses
> > calling _OSC with new flags.  Shouldn't be hard, once it's integrated
> > into pci_root.c.
> > 
> 
> Need to think about root hotplug at some point.
> 
> > Further ahead, we don't actually check that the bits we asked for (in
> > 'control') were actually granted to us.  The PCI firmware spec is quite
> > explicit about interdependencies amongst the bits.
> 
> Do we need to do this in addition to the typical
> pci_find_capability(dev, PCI_CAP_ID_MSIX) check? Perhaps we can write a
> function that does both?
> 
> > 
> > We also need to re-evaluate _OSC when coming out of S4.  I'm not much of
> > a power-management maven so I don't know where to put this call.
> > 
> > Thoughts on the road ahead?
> > 
> 
> This patch does not compile on pci-2.6/linux-next with ia-64.

My bad.  Chopped off the last diff when removing the sym2 patches.

Andrew