call _OSC support during root bridge discovery
v2 cover call _OSC support during root bridge discovery
This is v2 of the "call _OSC support during root bridge discovery" patchset. At Bjorn Helgaas's suggestion, I have merged all the MSI handling patches into one patch. Kenji Kaneshige also noted that I was not checking aspm_disabled before setting ASPM _OSC capabilities. This has been fixed.
Kenji also noted that the pci_msi_enable can be changed in various quirks after root bridge discovery, resulting in the incorrect setting of the MSI _OSC capability. I do not yet have a solution for this problem.
I still need Matthew Wilcox to sign off on the first two patches.
I recently reported a problem where a machine with close to a 100 PCIe root bridges was taking half an hour to just call _OSC. The root cause is the AER code calling:
pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
for each bridge. The pcie_osc_support_set() function also iterates over each bridge, so _OSC is called a quadratic number of times.
One solution to this problem would be to just move the call to pcie_osc_support_set() to aer_service_init(), so _OSC support is only called on each bridge once.
Matthew Wilcox came up with a better solution. He says "Why should a driver be calling pcie_osc_support_set() anyway? This is something the PCI core should be taking care of for it."
Matthew provided a patch for this solution that I massaged into the following patch series.
It creates 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.
Some work that still needs to be done (provided by Matthew):
o 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.
o 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.
o 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.
o We also need to re-evaluate _OSC when coming out of S4.
This patch series duplicates currently functionality while removing our quadratic problem. I believe that it can be applied now while the above new functionality can be implemented some time in the future.
This patch series applies to the linux-next branch of pci-2.6 git repository. I expect it will also work with linux-next.
Diff stats:
drivers/acpi/pci_root.c | 12 ++++++++++++ drivers/pci/msi.c | 31 +++++++++++------------------- drivers/pci/pci-acpi.c | 37 +++++++++++++----------------------- drivers/pci/pci.c | 2 -- drivers/pci/pci.h | 2 -- drivers/pci/pcie/aer/aerdrv_acpi.c | 1 - drivers/pci/pcie/aspm.c | 27 +++++++++----------------- include/linux/pci-acpi.h | 14 +++----------- include/linux/pci.h | 14 ++++++++++++++ 9 files changed, 62 insertions(+), 78 deletions(-)
Commits:
- Include missing acpi.h file in pci-acpi.h.
- Call _OSC support during root bridge discovery.
- PCIe ASPM _OSC support capabilities called when root bridge added.
- PCIe AER _OSC support capabilities called when root bridge added.
- PCI MSI _OSC support capabilities called when root bridge added.
- Remove obsolete _OSC capability support functions.
Signature · 1 line
Andrew Patterson
PATCH v2 1/6 PCI, ACPI: include missing acpi.h file in pci-acpi.h.
PCI, ACPI: include missing acpi.h file in pci-acpi.h.
The pci-acpi.h file will not compile without including linux/acpi.h.
---
include/linux/pci-acpi.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
include/linux/pci-acpi.h +2 -0
@@ -8,6 +8,8 @@#ifndef _PCI_ACPI_H_#define _PCI_ACPI_H_ +#include <linux/acpi.h> +#define OSC_QUERY_TYPE 0#define OSC_SUPPORT_TYPE 1#define OSC_CONTROL_TYPE 2
PATCH v2 2/6 ACPI, PCI: call _OSC support during root bridge discovery
In reply to Andrew PattersonACPI, PCI: call _OSC support during root bridge discovery
Added pci_acpi_osc_support() which is called when a PCI bridge is added, so individual PCI root bridge drivers do not have to call _OSC support for every root bridge in their probe functions.
---
drivers/acpi/pci_root.c | 7 +++++++ drivers/pci/pci-acpi.c | 24 +++++++++++++++++++----- include/linux/pci-acpi.h | 1 + 3 files changed, 27 insertions(+), 5 deletions(-)
drivers/acpi/pci_root.c +7 -0
@@ -31,6 +31,7 @@#include <linux/spinlock.h> #include <linux/pm.h> #include <linux/pci.h> +#include <linux/pci-acpi.h> #include <linux/acpi.h> #include <acpi/acpi_bus.h> #include <acpi/acpi_drivers.h>
@@ -193,6 +194,7 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)unsigned long long value =0 ;acpi_handle handle =NULL ;struct acpi_device *child; +u32 flags;if (!device)
@@ -210,6 +212,11 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) device->ops.bind = acpi_pci_bind; + flags = (OSC_EXT_PCI_CONFIG_SUPPORT | +OSC_PCI_SEGMENT_GROUPS_SUPPORT | +0 ); +pci_acpi_osc_support (device->handle, flags); +/* * Segment * -------
drivers/pci/pci-acpi.c +19 -5
@@ -139,28 +139,42 @@ static acpi_status __acpi_query_osc(u32 flags, struct acpi_osc_data *osc_data,return status; } -static acpi_status acpi_query_osc (acpi_handle handle, -u32 level,void *context,void **retval) +/* +* pci_acpi_osc_support: Invoke _OSC indicating support for the given feature +* @flags: Bitmask of flags to support +* +* See the ACPI spec for the definition of the flags +*/ +int pci_acpi_osc_support (acpi_handle handle,u32 flags) { +u32 dummy;acpi_status status; -struct acpi_osc_data *osc_data; -u32 flags = (unsigned long )context, dummy;acpi_handle tmp; +struct acpi_osc_data *osc_data; +int rc =0 ; status =acpi_get_handle (handle,"_OSC" , &tmp);if (ACPI_FAILURE (status)) -return AE_OK ; +return -ENOTTY ;mutex_lock (&pci_acpi_lock); osc_data =acpi_get_osc_data (handle);if (!osc_data) {printk (KERN_ERR "acpi osc data array is full\n" ); + rc = -ENOMEM ; goto out; }__acpi_query_osc (flags, osc_data, &dummy); out:mutex_unlock (&pci_acpi_lock); +return rc; +} + +static acpi_status acpi_query_osc (acpi_handle handle,u32 level, +void *context,void **retval) +{ +pci_acpi_osc_support (handle, (unsigned long )context);return AE_OK ; }
include/linux/pci-acpi.h +1 -0
@@ -51,6 +51,7 @@#ifdef CONFIG_ACPI extern acpi_status pci_osc_control_set (acpi_handle handle,u32 flags);extern acpi_status __pci_osc_support_set (u32 flags,const char *hid); +int pci_acpi_osc_support (acpi_handle handle,u32 flags);static inline acpi_status pci_osc_support_set (u32 flags) {return __pci_osc_support_set (flags,PCI_ROOT_HID_STRING );
PATCH v2 3/6 ACPI, PCI: PCIe ASPM _OSC support capabilities called when root bridge added
In reply to Andrew PattersonACPI, PCI: PCIe ASPM _OSC support capabilities called when root bridge added
The _OSC capabilities OSC_ACTIVE_STATE_PWR_SUPPORT and OSC_CLOCK_PWR_CAPABILITY_SUPPORT are set when the root bridge is added with pci_acpi_osc_support(), so we no longer need to do it in the ASPM driver.
Added the function pcie_aspm_enabled, which returns true if pcie_aspm=off is not on the kernel command-line.
---
drivers/acpi/pci_root.c | 3 +++ drivers/pci/pcie/aspm.c | 27 +++++++++------------------ include/linux/pci.h | 9 +++++++++ 3 files changed, 21 insertions(+), 18 deletions(-)
drivers/acpi/pci_root.c +3 -0
@@ -215,6 +215,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) flags = (OSC_EXT_PCI_CONFIG_SUPPORT |OSC_PCI_SEGMENT_GROUPS_SUPPORT |0 ); +if (pcie_aspm_enabled ()) + flags |=OSC_ACTIVE_STATE_PWR_SUPPORT | +OSC_CLOCK_PWR_CAPABILITY_SUPPORT ;pci_acpi_osc_support (device->handle, flags);/*
drivers/pci/pcie/aspm.c +9 -18
@@ -834,24 +834,15 @@ void pcie_no_aspm(void) aspm_disabled =1 ; } -#ifdef CONFIG_ACPI -#include <acpi/acpi_bus.h> -#include <linux/pci-acpi.h> -static void pcie_aspm_platform_init (void ) -{ -pcie_osc_support_set (OSC_ACTIVE_STATE_PWR_SUPPORT | -OSC_CLOCK_PWR_CAPABILITY_SUPPORT ); -} -#else -static inline void pcie_aspm_platform_init (void ) { } -#endif - -static int __init pcie_aspm_init (void ) +/** +* pcie_aspm_enabled - is PCIe ASPM enabled? +* +* Returns true if ASPM has not been disabled by the command-line option +* pcie_aspm=off. +**/ +int pcie_aspm_enabled (void ) { -if (aspm_disabled) -return 0 ; -pcie_aspm_platform_init (); -return 0 ; +return !aspm_disabled; } +EXPORT_SYMBOL (pcie_aspm_enabled); -fs_initcall (pcie_aspm_init);
include/linux/pci.h +9 -0
@@ -779,6 +779,15 @@ extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);extern void pci_restore_msi_state (struct pci_dev *dev);#endif +#ifndef CONFIG_PCIEASPM +static inline int pcie_aspm_enabled (void ) +{ +return 0 ; +} +#else +extern int pcie_aspm_enabled (void ); +#endif +#ifdef CONFIG_HT_IRQ /* The functions a driver should call */ int ht_create_irq (struct pci_dev *dev,int idx);
PATCH v2 4/6 ACPI, PCI: PCIe AER _OSC support capabilities called when root bridge added
In reply to Andrew PattersonACPI, PCI: PCIe AER _OSC support capabilities called when root bridge added
The _OSC capability OSC_EXT_PCI_CONFIG_SUPPORT is set when the root bridge is added with pci_acpi_osc_support(), so we no longer need to do it in the PCIe AER driver.
---
drivers/pci/pcie/aer/aerdrv_acpi.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/pci/pcie/aer/aerdrv_acpi.c +0 -1
@@ -38,7 +38,6 @@ int aer_osc_setup(struct pcie_device *pciedev) handle =acpi_find_root_bridge_handle (pdev);if (handle) { -pcie_osc_support_set (OSC_EXT_PCI_CONFIG_SUPPORT ); status =pci_osc_control_set (handle,OSC_PCI_EXPRESS_AER_CONTROL |OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL );
PATCH v2 5/6 ACPI, PCI: PCI MSI _OSC support capabilities called when root bridge added
In reply to Andrew PattersonACPI, PCI: PCI MSI _OSC support capabilities called when root bridge added
The _OSC capabilityy OSC_MSI_SUPPORT is set when the root bridge is added with pci_acpi_osc_support(), so we no longer need to do it in the PCI MSI driver.
Added the function pci_msi_enabled, which returns true if pci=nomsi is not on the kernel command-line.
---
drivers/acpi/pci_root.c | 2 ++ drivers/pci/msi.c | 31 +++++++++++-------------------- drivers/pci/pci.c | 2 -- drivers/pci/pci.h | 2 -- include/linux/pci.h | 5 +++++ 5 files changed, 18 insertions(+), 24 deletions(-)
drivers/acpi/pci_root.c +2 -0
@@ -218,6 +218,8 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)if (pcie_aspm_enabled ()) flags |=OSC_ACTIVE_STATE_PWR_SUPPORT |OSC_CLOCK_PWR_CAPABILITY_SUPPORT ; +if (pci_msi_enabled ()) + flags |=OSC_MSI_SUPPORT ;pci_acpi_osc_support (device->handle, flags);/*
drivers/pci/msi.c +11 -20
@@ -755,28 +755,19 @@ void pci_no_msi(void) pci_msi_enable =0 ; } -void pci_msi_init_pci_dev (struct pci_dev *dev) -{ -INIT_LIST_HEAD (&dev->msi_list); -} - -#ifdef CONFIG_ACPI -#include <linux/acpi.h> -#include <linux/pci-acpi.h> -static void __devinit msi_acpi_init (void ) +/** +* pci_msi_enabled - is MSI enabled? +* +* Returns true if MSI has not been disabled by the command-line option +* pci=nomsi. +**/ +int pci_msi_enabled (void ) { -if (acpi_pci_disabled) -return ; -pci_osc_support_set (OSC_MSI_SUPPORT ); -pcie_osc_support_set (OSC_MSI_SUPPORT ); +return pci_msi_enable; } -#else -static inline void msi_acpi_init (void ) { } -#endif /* CONFIG_ACPI */ +EXPORT_SYMBOL (pci_msi_enabled); -void __devinit msi_init (void ) +void pci_msi_init_pci_dev (struct pci_dev *dev) { -if (!pci_msi_enable) -return ; -msi_acpi_init (); +INIT_LIST_HEAD (&dev->msi_list); }
drivers/pci/pci.c +0 -2
@@ -2032,8 +2032,6 @@ static int __devinit pci_init(void)pci_fixup_device (pci_fixup_final, dev); } -msi_init (); -return 0 ; }
drivers/pci/pci.h +0 -2
@@ -98,11 +98,9 @@ extern unsigned int pci_pm_d3_delay;#ifdef CONFIG_PCI_MSI void pci_no_msi (void );extern void pci_msi_init_pci_dev (struct pci_dev *dev); -extern void __devinit msi_init (void );#else static inline void pci_no_msi (void ) { }static inline void pci_msi_init_pci_dev (struct pci_dev *dev) { } -static inline void msi_init (void ) { }#endif #ifdef CONFIG_PCIEAER
include/linux/pci.h +5 -0
@@ -767,6 +767,10 @@ static inline void msi_remove_pci_irq_vectors(struct pci_dev *dev)static inline void pci_restore_msi_state (struct pci_dev *dev) { } +static inline int pci_msi_enabled (void ) +{ +return 0 ; +}#else extern int pci_enable_msi (struct pci_dev *dev);extern void pci_msi_shutdown (struct pci_dev *dev);
@@ -777,6 +781,7 @@ extern void pci_msix_shutdown(struct pci_dev *dev);extern void pci_disable_msix (struct pci_dev *dev);extern void msi_remove_pci_irq_vectors (struct pci_dev *dev);extern void pci_restore_msi_state (struct pci_dev *dev); +extern int pci_msi_enabled (void );#endif #ifndef CONFIG_PCIEASPM
PATCH v2 6/6 PCI, ACPI: remove obsolete _OSC capability support functions
In reply to Andrew PattersonPCI, ACPI: remove obsolete _OSC capability support functions
The acpi_query_osc, __pci_osc_support_set, pci_osc_support_set, and pcie_osc_support_set functions have been obsoleted in favor of setting these capabilities during root bridge discovery with pci_acpi_osc_support. There are no longer any callers of these functions, so they are removed.
---
drivers/pci/pci-acpi.c | 25 ------------------------- include/linux/pci-acpi.h | 11 ----------- 2 files changed, 0 insertions(+), 36 deletions(-)
drivers/pci/pci-acpi.c +0 -25
@@ -171,31 +171,6 @@ out:return rc; } -static acpi_status acpi_query_osc (acpi_handle handle,u32 level, -void *context,void **retval) -{ -pci_acpi_osc_support (handle, (unsigned long )context); -return AE_OK ; -} - -/** -* __pci_osc_support_set - register OS support to Firmware -* @flags: OS support bits -* @hid: hardware ID -* -* Update OS support fields and doing a _OSC Query to obtain an update -* from Firmware on supported control bits. -**/ -acpi_status __pci_osc_support_set (u32 flags,const char *hid) -{ -if (!(flags &OSC_SUPPORT_MASKS )) -return AE_TYPE ; - -acpi_get_devices (hid, acpi_query_osc, - (void *)(unsigned long )flags,NULL ); -return AE_OK ; -} -/** * pci_osc_control_set - commit requested control to Firmware * @handle: acpi_handle for the target ACPI object
include/linux/pci-acpi.h +0 -11
@@ -50,16 +50,7 @@#ifdef CONFIG_ACPI extern acpi_status pci_osc_control_set (acpi_handle handle,u32 flags); -extern acpi_status __pci_osc_support_set (u32 flags,const char *hid);int pci_acpi_osc_support (acpi_handle handle,u32 flags); -static inline acpi_status pci_osc_support_set (u32 flags) -{ -return __pci_osc_support_set (flags,PCI_ROOT_HID_STRING ); -} -static inline acpi_status pcie_osc_support_set (u32 flags) -{ -return __pci_osc_support_set (flags,PCI_EXPRESS_ROOT_HID_STRING ); -}static inline acpi_handle acpi_find_root_bridge_handle (struct pci_dev *pdev) {/* Find root host bridge */
@@ -76,8 +67,6 @@ typedef u32 acpi_status;#endif static inline acpi_status pci_osc_control_set (acpi_handle handle,u32 flags) {return AE_ERROR ;} -static inline acpi_status pci_osc_support_set (u32 flags) {return AE_ERROR ;} -static inline acpi_status pcie_osc_support_set (u32 flags) {return AE_ERROR ;}static inline acpi_handle acpi_find_root_bridge_handle (struct pci_dev *pdev) {return NULL ; }#endif
Quoted 10 lines · Andrew Patterson
This is v2 of the "call
_OSCsupport during root bridge discovery" patchset. At Bjorn Helgaas's suggestion, I have merged all the MSI handling patches into one patch. Kenji Kaneshige also noted that I was not checkingaspm_disabledbefore setting ASPM_OSCcapabilities. This has been fixed.Kenji also noted that the
pci_msi_enablecan be changed in various quirks after root bridge discovery, resulting in the incorrect setting of the MSI_OSCcapability. I do not yet have a solution for this problem.
Unfortunately, I don't have good idea about it yet too. In addition, I don't understand how to handle the case if a feature is supported but disabled by OS, from the PCI firmware spec.
If we should not set _OSC capabilities when it is supported but disabled by OS, maybe we need also consider what we should do for OSC_EXT_PCI_CONFIG_SUPPORT and OSC_PCI_SEGMENT_GROUPS_SUPPORT if mmconfig is disabled, as well as aspm_disabled and pci_msi_enable.
By the way, Taku Izumi plan to post some patches to reduce _OSC evaluation for _OSC control soon. I'm appreciate it if you will try it on your big server.
Thanks,
Kenji Kaneshige
Quoted 74 lines
I still need Matthew Wilcox to sign off on the first two patches.
I recently reported a problem where a machine with close to a 100 PCIe root bridges was taking half an hour to just call
_OSC. The root cause is the AER code calling:pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);for each bridge. The
pcie_osc_support_set()function also iterates over each bridge, so_OSCis called a quadratic number of times.One solution to this problem would be to just move the call to
pcie_osc_support_set()toaer_service_init(), so_OSCsupport is only called on each bridge once.Matthew Wilcox came up with a better solution. He says "Why should a driver be calling
pcie_osc_support_set()anyway? This is something the PCI core should be taking care of for it."Matthew provided a patch for this solution that I massaged into the following patch series.
It creates a new function (
pci_acpi_osc_support()) which is called frompci_root.cbefore 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.Some work that still needs to be done (provided by Matthew):
o All the existing
_OSCcode should be moved frompci-acpi.ctopci_root.c. I also think theacpi_osc_datashould be made part of theacpi_pci_rootstruct, eliminating a separate allocation.o We could also use an interface that iterates over all existing busses calling
_OSCwith new flags. Shouldn't be hard, once it's integrated intopci_root.c.o 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.
o We also need to re-evaluate
_OSCwhen coming out of S4.This patch series duplicates currently functionality while removing our quadratic problem. I believe that it can be applied now while the above new functionality can be implemented some time in the future.
This patch series applies to the linux-next branch of pci-2.6 git repository. I expect it will also work with linux-next.
Diff stats:
drivers/acpi/pci_root.c | 12 ++++++++++++ drivers/pci/msi.c | 31 +++++++++++------------------- drivers/pci/pci-acpi.c | 37 +++++++++++++----------------------- drivers/pci/pci.c | 2 -- drivers/pci/pci.h | 2 -- drivers/pci/pcie/aer/aerdrv_acpi.c | 1 - drivers/pci/pcie/aspm.c | 27 +++++++++----------------- include/linux/pci-acpi.h | 14 +++----------- include/linux/pci.h | 14 ++++++++++++++ 9 files changed, 62 insertions(+), 78 deletions(-)Commits:
- Include missing
acpi.hfile inpci-acpi.h.
- Call_OSCsupport during root bridge discovery.
- PCIe ASPM_OSCsupport capabilities called when root bridge added.
- PCIe AER_OSCsupport capabilities called when root bridge added.
- PCI MSI_OSCsupport capabilities called when root bridge added.
- Remove obsolete_OSCcapability support functions.
Quoted 17 lines · Kenji Kaneshige
Quoted 10 lines · Andrew Patterson
This is v2 of the "call
_OSCsupport during root bridge discovery" patchset. At Bjorn Helgaas's suggestion, I have merged all the MSI handling patches into one patch. Kenji Kaneshige also noted that I was not checkingaspm_disabledbefore setting ASPM_OSCcapabilities. This has been fixed.Kenji also noted that the
pci_msi_enablecan be changed in various quirks after root bridge discovery, resulting in the incorrect setting of the MSI_OSCcapability. I do not yet have a solution for this problem.Unfortunately, I don't have good idea about it yet too. In addition, I don't understand how to handle the case if a feature is supported but disabled by OS, from the PCI firmware spec.
Quoted 5 lines
If we should not set
_OSCcapabilities when it is supported but disabled by OS, maybe we need also consider what we should do forOSC_EXT_PCI_CONFIG_SUPPORTandOSC_PCI_SEGMENT_GROUPS_SUPPORTif mmconfig is disabled, as well asaspm_disabledandpci_msi_enable.
A good idea. I try and come up with something.
Quoted 4 lines
By the way, Taku Izumi plan to post some patches to reduce
_OSCevaluation for_OSCcontrol soon. I'm appreciate it if you will try it on your big server.
I would be happy to help. Getting time on this server is sometimes difficult, so it may take a while before I can run a test.
Andrew
Quoted 6 lines
Thanks,
Kenji Kaneshige