[PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

PCIe port service drivers ask the BIOS, through _OSC, for control of
the services they handle.  Unfortunately, each of them individually
asks for control of the PCIe capability structure and if that is
granted, some BIOSes expect that the other PCIe port services will be
configured and handled by the kernel as well.  If that is not the
case (eg. one of the PCIe port service drivers is not loaded), the
BIOS may be confused and may cause the system as a whole to misbehave
(eg. on one of such systems enabling the native PCIe PME service
without loading the native PCIe hot-plug service driver causes a
storm of ACPI notify requests to appear).

For this reason rework the PCIe port driver so that (1) it checks
which native PCIe port services can be enabled, according to the
BIOS, and (2) it requests control of all these services
simultaneously.  In particular, this causes pcie_portdrv_probe() to
fail if the BIOS refuses to grant control of the PCIe capability
structure, which means that no native PCIe port services can be
enabled for the PCIe root complex the given port belongs to.

Make it possible to override this behavior using a new command line
switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
default), 'native' (use the PCIe native services regardless of the
BIOS response to the control request), or 'compat' (do not use the
PCIe native services at all).

Accordingly, rework the existing PCIe port service drivers so that
they don't request control of the services directly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/kernel-parameters.txt  |   17 ++++--
 drivers/acpi/pci_root.c              |   46 ++++++++++++++++--
 drivers/pci/hotplug/acpi_pcihp.c     |    4 -
 drivers/pci/hotplug/pciehp.h         |   12 ----
 drivers/pci/hotplug/pciehp_acpi.c    |    4 -
 drivers/pci/hotplug/pciehp_core.c    |    4 -
 drivers/pci/pcie/Makefile            |    3 -
 drivers/pci/pcie/aer/aerdrv_acpi.c   |   36 --------------
 drivers/pci/pcie/aer/aerdrv_core.c   |   14 -----
 drivers/pci/pcie/pme/Makefile        |    8 ---
 drivers/pci/pcie/pme/pcie_pme.c      |   64 ++-----------------------
 drivers/pci/pcie/pme/pcie_pme.h      |   28 -----------
 drivers/pci/pcie/pme/pcie_pme_acpi.c |   54 ---------------------
 drivers/pci/pcie/portdrv.h           |   22 ++++++++
 drivers/pci/pcie/portdrv_acpi.c      |   87 +++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv_core.c      |   37 +++++++++++++-
 drivers/pci/pcie/portdrv_pci.c       |   37 ++++++++++++--
 include/linux/acpi.h                 |    1 
 include/linux/pcieport_if.h          |    5 ++
 19 files changed, 240 insertions(+), 243 deletions(-)

Index: linux-2.6/drivers/pci/pcie/portdrv_pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/portdrv_pci.c
+++ linux-2.6/drivers/pci/pcie/portdrv_pci.c
@@ -29,6 +29,32 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
+/* If this switch is set, PCIe port native services should not be enabled. */
+bool pcie_ports_disabled;
+
+/*
+ * If this switch is set, ACPI _OSC will be used to determine whether or not to
+ * enable PCIe port native services.
+ */
+bool pcie_ports_auto = true;
+
+static int __init pcie_port_setup(char *str)
+{
+	if (!strncmp(str, "auto", 4)) {
+		pcie_ports_disabled = false;
+		pcie_ports_auto = true;
+	} else if (!strncmp(str, "native", 6)) {
+		pcie_ports_disabled = false;
+		pcie_ports_auto = false;
+	} else if (!strncmp(str, "compat", 6)) {
+		pcie_ports_disabled = true;
+		pcie_ports_auto = false;
+	}
+
+	return 1;
+}
+__setup("pcie_ports=", pcie_port_setup);
+
 /* global data */
 
 static int pcie_portdrv_restore_config(struct pci_dev *dev)
@@ -82,6 +108,7 @@ static int __devinit pcie_portdrv_probe(
 		dev_warn(&dev->dev, "device [%04x:%04x] has invalid IRQ; "
 			 "check vendor BIOS\n", dev->vendor, dev->device);
 	}
+
 	status = pcie_port_device_register(dev);
 	if (status)
 		return status;
@@ -301,6 +328,9 @@ static int __init pcie_portdrv_init(void
 {
 	int retval;
 
+	if (pcie_ports_disabled)
+		return -EACCES;
+
 	dmi_check_system(pcie_portdrv_dmi_table);
 
 	retval = pcie_port_bus_register();
@@ -315,11 +345,4 @@ static int __init pcie_portdrv_init(void
 	return retval;
 }
 
-static void __exit pcie_portdrv_exit(void)
-{
-	pci_unregister_driver(&pcie_portdriver);
-	pcie_port_bus_unregister();
-}
-
 module_init(pcie_portdrv_init);
-module_exit(pcie_portdrv_exit);
Index: linux-2.6/drivers/pci/pcie/portdrv_acpi.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/pci/pcie/portdrv_acpi.c
@@ -0,0 +1,87 @@
+/*
+ * PCIe Port Native Services Support, ACPI-Related Part
+ *
+ * Copyright (C) 2010 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License V2.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/pci.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/acpi.h>
+#include <linux/pci-acpi.h>
+#include <linux/pcieport_if.h>
+
+#include "aer/aerdrv.h"
+
+/**
+ * pcie_port_acpi_setup - Request the BIOS to release control of PCIe services.
+ * @port: PCIe Port service for a root port or event collector.
+ * @srv_mask: Bit mask of services that can be enabled for @port.
+ *
+ * Invoked when @port is identified as a PCIe port device.  To avoid conflicts
+ * with the BIOS PCIe port native services support requires the BIOS to yield
+ * control of these services to the kernel.  The mask of services that the BIOS
+ * allows to be enabled for @port is written to @srv_mask.
+ *
+ * NOTE: It turns out that we cannot do that for individual port services
+ * separately, because that would make some systems work incorrectly.
+ */
+int pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
+{
+	acpi_status status;
+	acpi_handle handle;
+	u32 flags, request;
+
+	if (acpi_pci_disabled)
+		return 0;
+
+	handle = acpi_find_root_bridge_handle(port);
+	if (!handle)
+		return -EINVAL;
+
+	status = acpi_pci_osc_control_get(handle, &flags);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&port->dev, "ACPI _OSC query failed (code %d)\n",
+			status);
+		return -ENODEV;
+	}
+
+	if (!(flags & OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)) {
+		dev_dbg(&port->dev, "BIOS refuses to grant control of PCIe "
+			"Capability Structure\n");
+		return -EACCES;
+	}
+
+	request = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL;
+	*srv_mask = PCIE_PORT_SERVICE_VC;
+	if (flags & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL) {
+		request |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
+		*srv_mask |= PCIE_PORT_SERVICE_HP;
+	}
+	if (flags & OSC_PCI_EXPRESS_PME_CONTROL) {
+		request |= OSC_PCI_EXPRESS_PME_CONTROL;
+		*srv_mask |= PCIE_PORT_SERVICE_PME;
+	}
+
+	if (pcie_aer_get_firmware_first(port)) {
+		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
+	} else if (flags & OSC_PCI_EXPRESS_AER_CONTROL) {
+		request |= OSC_PCI_EXPRESS_AER_CONTROL;
+		*srv_mask |= PCIE_PORT_SERVICE_AER;
+	}
+
+	status = acpi_pci_osc_control_set(handle, request);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&port->dev, "ACPI _OSC request failed (code %d)\n",
+			status);
+		return -ENODEV;
+	}
+
+	dev_info(&port->dev, "ACPI _OSC control granted for 0x%02x\n", request);
+
+	return 0;
+}
Index: linux-2.6/drivers/pci/pcie/portdrv.h
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/portdrv.h
+++ linux-2.6/drivers/pci/pcie/portdrv.h
@@ -20,6 +20,9 @@
 
 #define get_descriptor_id(type, service) (((type - 4) << 4) | service)
 
+extern bool pcie_ports_disabled;
+extern bool pcie_ports_auto;
+
 extern struct bus_type pcie_port_bus_type;
 extern int pcie_port_device_register(struct pci_dev *dev);
 #ifdef CONFIG_PM
@@ -30,6 +33,8 @@ extern void pcie_port_device_remove(stru
 extern int __must_check pcie_port_bus_register(void);
 extern void pcie_port_bus_unregister(void);
 
+struct pci_dev;
+
 #ifdef CONFIG_PCIE_PME
 extern bool pcie_pme_msi_disabled;
 
@@ -42,9 +47,26 @@ static inline bool pcie_pme_no_msi(void)
 {
 	return pcie_pme_msi_disabled;
 }
+
+extern void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable);
 #else /* !CONFIG_PCIE_PME */
 static inline void pcie_pme_disable_msi(void) {}
 static inline bool pcie_pme_no_msi(void) { return false; }
+static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {}
 #endif /* !CONFIG_PCIE_PME */
 
+#ifdef CONFIG_ACPI
+extern int pcie_port_acpi_setup(struct pci_dev *port, int *mask);
+
+static inline int pcie_port_platform_notify(struct pci_dev *port, int *mask)
+{
+	return pcie_port_acpi_setup(port, mask);
+}
+#else /* !CONFIG_ACPI */
+static inline int pcie_port_platform_notify(struct pci_dev *port, int *mask)
+{
+	return 0;
+}
+#endif /* !CONFIG_ACPI */
+
 #endif /* _PORTDRV_H_ */
Index: linux-2.6/drivers/pci/pcie/Makefile
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/Makefile
+++ linux-2.6/drivers/pci/pcie/Makefile
@@ -6,10 +6,11 @@
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
 pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
 
 # Build PCI Express AER if needed
 obj-$(CONFIG_PCIEAER)		+= aer/
 
-obj-$(CONFIG_PCIE_PME) += pme/
+obj-$(CONFIG_PCIE_PME) += pme/pcie_pme.o
Index: linux-2.6/drivers/pci/pcie/portdrv_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2.6/drivers/pci/pcie/portdrv_core.c
@@ -14,6 +14,7 @@
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/pcieport_if.h>
+#include <linux/aer.h>
 
 #include "../pci.h"
 #include "portdrv.h"
@@ -236,24 +237,49 @@ static int get_port_device_capability(st
 	int services = 0, pos;
 	u16 reg16;
 	u32 reg32;
+	int cap_mask;
+	int err;
+
+	err = pcie_port_platform_notify(dev, &cap_mask);
+	if (pcie_ports_auto) {
+		if (err)
+			return 0;
+	} else {
+		cap_mask = PCIE_PORT_SERVICE_MASK;
+	}
 
 	pos = pci_pcie_cap(dev);
 	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
 	/* Hot-Plug Capable */
-	if (reg16 & PCI_EXP_FLAGS_SLOT) {
+	if ((cap_mask & PCIE_PORT_SERVICE_HP) && (reg16 & PCI_EXP_FLAGS_SLOT)) {
 		pci_read_config_dword(dev, pos + PCI_EXP_SLTCAP, &reg32);
-		if (reg32 & PCI_EXP_SLTCAP_HPC)
+		if (reg32 & PCI_EXP_SLTCAP_HPC) {
 			services |= PCIE_PORT_SERVICE_HP;
+			/*
+			 * Disable hot-plug interrupts in case the hot-plug
+			 * service driver is not loaded.
+			 */
+			pos += PCI_EXP_SLTCTL;
+			pci_read_config_word(dev, pos, &reg16);
+			reg16 &= ~(PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
+			pci_write_config_word(dev, pos, reg16);
+		}
 	}
 	/* AER capable */
-	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
+	if ((cap_mask & PCIE_PORT_SERVICE_AER)
+	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR)) {
 		services |= PCIE_PORT_SERVICE_AER;
+		pci_disable_pcie_error_reporting(dev);
+	}
 	/* VC support */
 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
 		services |= PCIE_PORT_SERVICE_VC;
 	/* Root ports are capable of generating PME too */
-	if (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
+	if ((cap_mask & PCIE_PORT_SERVICE_PME)
+	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
 		services |= PCIE_PORT_SERVICE_PME;
+		pcie_pme_interrupt_enable(dev, false);
+	}
 
 	return services;
 }
@@ -494,6 +520,9 @@ static void pcie_port_shutdown_service(s
  */
 int pcie_port_service_register(struct pcie_port_service_driver *new)
 {
+	if (pcie_ports_disabled)
+		return -ENODEV;
+
 	new->driver.name = (char *)new->name;
 	new->driver.bus = &pcie_port_bus_type;
 	new->driver.probe = pcie_port_probe_service;
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -364,6 +364,40 @@ out:
 EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
 
 /**
+ * acpi_pci_osc_control_get - Get the _OSC bits the BIOS will grant control of.
+ * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
+ * @mask: Address to store the _OSC bits mask at.
+ **/
+acpi_status acpi_pci_osc_control_get(acpi_handle handle, u32 *mask)
+{
+	acpi_status status;
+	acpi_handle tmp;
+	struct acpi_pci_root *root;
+
+	root = acpi_pci_find_root(handle);
+	if (!root)
+		return AE_NOT_EXIST;
+
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	mutex_lock(&osc_lock);
+
+	if (!root->osc_queried) {
+		status = acpi_pci_query_osc(root, root->osc_support_set);
+		if (ACPI_FAILURE(status))
+			goto out;
+	}
+
+	*mask = root->osc_control_qry;
+
+ out:
+	mutex_unlock(&osc_lock);
+	return status;
+}
+
+/**
  * acpi_pci_osc_control_set - commit requested control to Firmware
  * @handle: acpi_handle for the target ACPI object
  * @flags: driver's requested control bits
@@ -377,10 +411,6 @@ acpi_status acpi_pci_osc_control_set(acp
 	acpi_handle tmp;
 	struct acpi_pci_root *root;
 
-	status = acpi_get_handle(handle, "_OSC", &tmp);
-	if (ACPI_FAILURE(status))
-		return status;
-
 	control_req = (flags & OSC_PCI_CONTROL_MASKS);
 	if (!control_req)
 		return AE_TYPE;
@@ -389,6 +419,10 @@ acpi_status acpi_pci_osc_control_set(acp
 	if (!root)
 		return AE_NOT_EXIST;
 
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
+
 	mutex_lock(&osc_lock);
 	/* No need to evaluate _OSC if the control was already granted. */
 	if ((root->osc_control_set & control_req) == control_req)
@@ -401,8 +435,8 @@ acpi_status acpi_pci_osc_control_set(acp
 			goto out;
 	}
 	if ((root->osc_control_qry & control_req) != control_req) {
-		printk(KERN_DEBUG
-		       "Firmware did not grant requested _OSC control\n");
+		pr_info("ACPI: Requested _OSC control for unsupported mask "
+			"0x%02x\n", control_req);
 		status = AE_SUPPORT;
 		goto out;
 	}
Index: linux-2.6/drivers/pci/pcie/pme/pcie_pme.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/pme/pcie_pme.c
+++ linux-2.6/drivers/pci/pcie/pme/pcie_pme.c
@@ -24,37 +24,12 @@
 #include <linux/pm_runtime.h>
 
 #include "../../pci.h"
-#include "pcie_pme.h"
+#include "../portdrv.h"
 
 #define PCI_EXP_RTSTA_PME	0x10000 /* PME status */
 #define PCI_EXP_RTSTA_PENDING	0x20000 /* PME pending */
 
 /*
- * If set, this switch will prevent the PCIe root port PME service driver from
- * being registered.  Consequently, the interrupt-based PCIe PME signaling will
- * not be used by any PCIe root ports in that case.
- */
-static bool pcie_pme_disabled = true;
-
-/*
- * The PCI Express Base Specification 2.0, Section 6.1.8, states the following:
- * "In order to maintain compatibility with non-PCI Express-aware system
- * software, system power management logic must be configured by firmware to use
- * the legacy mechanism of signaling PME by default.  PCI Express-aware system
- * software must notify the firmware prior to enabling native, interrupt-based
- * PME signaling."  However, if the platform doesn't provide us with a suitable
- * notification mechanism or the notification fails, it is not clear whether or
- * not we are supposed to use the interrupt-based PCIe PME signaling.  The
- * switch below can be used to indicate the desired behaviour.  When set, it
- * will make the kernel use the interrupt-based PCIe PME signaling regardless of
- * the platform notification status, although the kernel will attempt to notify
- * the platform anyway.  When unset, it will prevent the kernel from using the
- * the interrupt-based PCIe PME signaling if the platform notification fails,
- * which is the default.
- */
-static bool pcie_pme_force_enable;
-
-/*
  * If this switch is set, MSI will not be used for PCIe PME signaling.  This
  * causes the PCIe port driver to use INTx interrupts only, but it turns out
  * that using MSI for PCIe PME signaling doesn't play well with PCIe PME-based
@@ -64,38 +39,13 @@ bool pcie_pme_msi_disabled;
 
 static int __init pcie_pme_setup(char *str)
 {
-	if (!strncmp(str, "auto", 4))
-		pcie_pme_disabled = false;
-	else if (!strncmp(str, "force", 5))
-		pcie_pme_force_enable = true;
-
-	str = strchr(str, ',');
-	if (str) {
-		str++;
-		str += strspn(str, " \t");
-		if (*str && !strcmp(str, "nomsi"))
-			pcie_pme_msi_disabled = true;
-	}
+	if (!strncmp(str, "nomsi", 5))
+		pcie_pme_msi_disabled = true;
 
 	return 1;
 }
 __setup("pcie_pme=", pcie_pme_setup);
 
-/**
- * pcie_pme_platform_setup - Ensure that the kernel controls the PCIe PME.
- * @srv: PCIe PME root port service to use for carrying out the check.
- *
- * Notify the platform that the native PCIe PME is going to be used and return
- * 'true' if the control of the PCIe PME registers has been acquired from the
- * platform.
- */
-static bool pcie_pme_platform_setup(struct pcie_device *srv)
-{
-	if (!pcie_pme_platform_notify(srv))
-		return true;
-	return pcie_pme_force_enable;
-}
-
 struct pcie_pme_service_data {
 	spinlock_t lock;
 	struct pcie_device *srv;
@@ -108,7 +58,7 @@ struct pcie_pme_service_data {
  * @dev: PCIe root port or event collector.
  * @enable: Enable or disable the interrupt.
  */
-static void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable)
+void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable)
 {
 	int rtctl_pos;
 	u16 rtctl;
@@ -417,9 +367,6 @@ static int pcie_pme_probe(struct pcie_de
 	struct pcie_pme_service_data *data;
 	int ret;
 
-	if (!pcie_pme_platform_setup(srv))
-		return -EACCES;
-
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -509,8 +456,7 @@ static struct pcie_port_service_driver p
  */
 static int __init pcie_pme_service_init(void)
 {
-	return pcie_pme_disabled ?
-		-ENODEV : pcie_port_service_register(&pcie_pme_driver);
+	return pcie_port_service_register(&pcie_pme_driver);
 }
 
 module_init(pcie_pme_service_init);
Index: linux-2.6/drivers/pci/pcie/pme/pcie_pme.h
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/pme/pcie_pme.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * drivers/pci/pcie/pme/pcie_pme.h
- *
- * PCI Express Root Port PME signaling support
- *
- * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
- */
-
-#ifndef _PCIE_PME_H_
-#define _PCIE_PME_H_
-
-struct pcie_device;
-
-#ifdef CONFIG_ACPI
-extern int pcie_pme_acpi_setup(struct pcie_device *srv);
-
-static inline int pcie_pme_platform_notify(struct pcie_device *srv)
-{
-	return pcie_pme_acpi_setup(srv);
-}
-#else /* !CONFIG_ACPI */
-static inline int pcie_pme_platform_notify(struct pcie_device *srv)
-{
-	return 0;
-}
-#endif /* !CONFIG_ACPI */
-
-#endif
Index: linux-2.6/drivers/pci/pcie/pme/pcie_pme_acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/pme/pcie_pme_acpi.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * PCIe Native PME support, ACPI-related part
- *
- * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License V2.  See the file "COPYING" in the main directory of this archive
- * for more details.
- */
-
-#include <linux/pci.h>
-#include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/acpi.h>
-#include <linux/pci-acpi.h>
-#include <linux/pcieport_if.h>
-
-/**
- * pcie_pme_acpi_setup - Request the ACPI BIOS to release control over PCIe PME.
- * @srv - PCIe PME service for a root port or event collector.
- *
- * Invoked when the PCIe bus type loads PCIe PME service driver.  To avoid
- * conflict with the BIOS PCIe support requires the BIOS to yield PCIe PME
- * control to the kernel.
- */
-int pcie_pme_acpi_setup(struct pcie_device *srv)
-{
-	acpi_status status = AE_NOT_FOUND;
-	struct pci_dev *port = srv->port;
-	acpi_handle handle;
-	int error = 0;
-
-	if (acpi_pci_disabled)
-		return -ENOSYS;
-
-	dev_info(&port->dev, "Requesting control of PCIe PME from ACPI BIOS\n");
-
-	handle = acpi_find_root_bridge_handle(port);
-	if (!handle)
-		return -EINVAL;
-
-	status = acpi_pci_osc_control_set(handle,
-			OSC_PCI_EXPRESS_PME_CONTROL |
-			OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-	if (ACPI_FAILURE(status)) {
-		dev_info(&port->dev,
-			"Failed to receive control of PCIe PME service: %s\n",
-			(status == AE_SUPPORT || status == AE_NOT_FOUND) ?
-			"no _OSC support" : "ACPI _OSC failed");
-		error = -ENODEV;
-	}
-
-	return error;
-}
Index: linux-2.6/drivers/pci/pcie/pme/Makefile
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/pme/Makefile
+++ /dev/null
@@ -1,8 +0,0 @@
-#
-# Makefile for PCI-Express Root Port PME signaling driver
-#
-
-obj-$(CONFIG_PCIE_PME) += pmedriver.o
-
-pmedriver-objs := pcie_pme.o
-pmedriver-$(CONFIG_ACPI) += pcie_pme_acpi.o
Index: linux-2.6/drivers/pci/pcie/aer/aerdrv_acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ linux-2.6/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -19,42 +19,6 @@
 #include <acpi/apei.h>
 #include "aerdrv.h"
 
-/**
- * aer_osc_setup - run ACPI _OSC method
- * @pciedev: pcie_device which AER is being enabled on
- *
- * @return: Zero on success. Nonzero otherwise.
- *
- * Invoked when PCIe bus loads AER service driver. To avoid conflict with
- * BIOS AER support requires BIOS to yield AER control to OS native driver.
- **/
-int aer_osc_setup(struct pcie_device *pciedev)
-{
-	acpi_status status = AE_NOT_FOUND;
-	struct pci_dev *pdev = pciedev->port;
-	acpi_handle handle = NULL;
-
-	if (acpi_pci_disabled)
-		return -1;
-
-	handle = acpi_find_root_bridge_handle(pdev);
-	if (handle) {
-		status = acpi_pci_osc_control_set(handle,
-					OSC_PCI_EXPRESS_AER_CONTROL |
-					OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-	}
-
-	if (ACPI_FAILURE(status)) {
-		dev_printk(KERN_DEBUG, &pciedev->device, "AER service couldn't "
-			   "init device: %s\n",
-			   (status == AE_SUPPORT || status == AE_NOT_FOUND) ?
-			   "no _OSC support" : "_OSC failed");
-		return -1;
-	}
-
-	return 0;
-}
-
 #ifdef CONFIG_ACPI_APEI
 static inline int hest_match_pci(struct acpi_hest_aer_common *p,
 				 struct pci_dev *pci)
Index: linux-2.6/drivers/pci/pcie/aer/aerdrv_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/aer/aerdrv_core.c
+++ linux-2.6/drivers/pci/pcie/aer/aerdrv_core.c
@@ -771,22 +771,10 @@ void aer_isr(struct work_struct *work)
  */
 int aer_init(struct pcie_device *dev)
 {
-	if (pcie_aer_get_firmware_first(dev->port)) {
-		dev_printk(KERN_DEBUG, &dev->device,
-			   "PCIe errors handled by platform firmware.\n");
-		goto out;
-	}
-
-	if (aer_osc_setup(dev))
-		goto out;
-
-	return 0;
-out:
 	if (forceload) {
 		dev_printk(KERN_DEBUG, &dev->device,
 			   "aerdrv forceload requested.\n");
 		pcie_aer_force_firmware_first(dev->port, 0);
-		return 0;
 	}
-	return -ENXIO;
+	return 0;
 }
Index: linux-2.6/drivers/pci/hotplug/acpi_pcihp.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/acpi_pcihp.c
+++ linux-2.6/drivers/pci/hotplug/acpi_pcihp.c
@@ -338,9 +338,7 @@ int acpi_get_hp_hw_control_from_firmware
 	acpi_handle chandle, handle;
 	struct acpi_buffer string = { ACPI_ALLOCATE_BUFFER, NULL };
 
-	flags &= (OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
-		  OSC_SHPC_NATIVE_HP_CONTROL |
-		  OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
+	flags &= OSC_SHPC_NATIVE_HP_CONTROL;
 	if (!flags) {
 		err("Invalid flags %u specified!\n", flags);
 		return -EINVAL;
Index: linux-2.6/drivers/pci/hotplug/pciehp.h
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp.h
+++ linux-2.6/drivers/pci/hotplug/pciehp.h
@@ -176,19 +176,7 @@ static inline void pciehp_firmware_init(
 {
 	pciehp_acpi_slot_detection_init();
 }
-
-static inline int pciehp_get_hp_hw_control_from_firmware(struct pci_dev *dev)
-{
-	int retval;
-	u32 flags = (OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
-		     OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-	retval = acpi_get_hp_hw_control_from_firmware(dev, flags);
-	if (retval)
-		return retval;
-	return pciehp_acpi_slot_detection_check(dev);
-}
 #else
 #define pciehp_firmware_init()				do {} while (0)
-#define pciehp_get_hp_hw_control_from_firmware(dev) 	0
 #endif 				/* CONFIG_ACPI */
 #endif				/* _PCIEHP_H */
Index: linux-2.6/drivers/pci/hotplug/pciehp_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_core.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_core.c
@@ -59,7 +59,7 @@ module_param(pciehp_force, bool, 0644);
 MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
 MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
 MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
-MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if _OSC and OSHP are missing");
+MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if OSHP is missing");
 
 #define PCIE_MODULE_NAME "pciehp"
 
@@ -235,7 +235,7 @@ static int pciehp_probe(struct pcie_devi
 		dev_info(&dev->device,
 			 "Bypassing BIOS check for pciehp use on %s\n",
 			 pci_name(dev->port));
-	else if (pciehp_get_hp_hw_control_from_firmware(dev->port))
+	else if (pciehp_acpi_slot_detection_check(dev->port))
 		goto err_out_none;
 
 	ctrl = pcie_init(dev);
Index: linux-2.6/drivers/pci/hotplug/pciehp_acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_acpi.c
+++ linux-2.6/drivers/pci/hotplug/pciehp_acpi.c
@@ -85,9 +85,7 @@ static int __init dummy_probe(struct pci
 	acpi_handle handle;
 	struct dummy_slot *slot, *tmp;
 	struct pci_dev *pdev = dev->port;
-	/* Note: pciehp_detect_mode != PCIEHP_DETECT_ACPI here */
-	if (pciehp_get_hp_hw_control_from_firmware(pdev))
-		return -ENODEV;
+
 	pos = pci_pcie_cap(pdev);
 	if (!pos)
 		return -ENODEV;
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -2047,15 +2047,18 @@ and is between 256 and 4096 characters. 
 		force	Enable ASPM even on devices that claim not to support it.
 			WARNING: Forcing ASPM on may cause system lockups.
 
+	pcie_ports=	[PCIE] PCIe ports handling:
+		auto	Ask the BIOS whether or not to use native PCIe services
+			associated with PCIe ports (PME, hot-plug, AER).  Use
+			them only if that is allowed by the BIOS.
+		native	Use native PCIe services associated with PCIe ports
+			unconditionally.
+		compat	Treat PCIe ports as PCI-to-PCI bridges, disable the PCIe
+			ports driver.
+
 	pcie_pme=	[PCIE,PM] Native PCIe PME signaling options:
-			Format: {auto|force}[,nomsi]
-		auto	Use native PCIe PME signaling if the BIOS allows the
-			kernel to control PCIe config registers of root ports.
-		force	Use native PCIe PME signaling even if the BIOS refuses
-			to allow the kernel to control the relevant PCIe config
-			registers.
 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
-			all PCIe root ports use INTx for everything).
+			all PCIe root ports use INTx for all services).
 
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
Index: linux-2.6/include/linux/acpi.h
===================================================================
--- linux-2.6.orig/include/linux/acpi.h
+++ linux-2.6/include/linux/acpi.h
@@ -305,6 +305,7 @@ acpi_status acpi_run_osc(acpi_handle han
 				OSC_PCI_EXPRESS_AER_CONTROL |		\
 				OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)
 
+extern acpi_status acpi_pci_osc_control_get(acpi_handle handle, u32 *mask);
 extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags);
 extern void acpi_early_init(void);
 
Index: linux-2.6/include/linux/pcieport_if.h
===================================================================
--- linux-2.6.orig/include/linux/pcieport_if.h
+++ linux-2.6/include/linux/pcieport_if.h
@@ -22,6 +22,11 @@
 #define PCIE_PORT_SERVICE_VC_SHIFT	3	/* Virtual Channel */
 #define PCIE_PORT_SERVICE_VC		(1 << PCIE_PORT_SERVICE_VC_SHIFT)
 
+#define PCIE_PORT_SERVICE_MASK		(PCIE_PORT_SERVICE_PME \
+					| PCIE_PORT_SERVICE_AER \
+					| PCIE_PORT_SERVICE_HP \
+					| PCIE_PORT_SERVICE_VC)
+
 struct pcie_device {
 	int 		irq;	    /* Service IRQ/MSI/MSI-X Vector */
 	struct pci_dev *port;	    /* Root/Upstream/Downstream Port */

Re: [PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Jesse Barnes]

On Wed, 28 Jul 2010 23:23:56 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> PCIe port service drivers ask the BIOS, through _OSC, for control of
> the services they handle.  Unfortunately, each of them individually
> asks for control of the PCIe capability structure and if that is
> granted, some BIOSes expect that the other PCIe port services will be
> configured and handled by the kernel as well.  If that is not the
> case (eg. one of the PCIe port service drivers is not loaded), the
> BIOS may be confused and may cause the system as a whole to misbehave
> (eg. on one of such systems enabling the native PCIe PME service
> without loading the native PCIe hot-plug service driver causes a
> storm of ACPI notify requests to appear).
> 
> For this reason rework the PCIe port driver so that (1) it checks
> which native PCIe port services can be enabled, according to the
> BIOS, and (2) it requests control of all these services
> simultaneously.  In particular, this causes pcie_portdrv_probe() to
> fail if the BIOS refuses to grant control of the PCIe capability
> structure, which means that no native PCIe port services can be
> enabled for the PCIe root complex the given port belongs to.
> 
> Make it possible to override this behavior using a new command line
> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
> default), 'native' (use the PCIe native services regardless of the
> BIOS response to the control request), or 'compat' (do not use the
> PCIe native services at all).
> 
> Accordingly, rework the existing PCIe port service drivers so that
> they don't request control of the services directly.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---

Kenji-san, are you ok with this version?  I would like to get your ack
(and ideally your tested-by) for this one since it affects
functionality you need.

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

Re: [PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Kenji Kaneshige]

(2010/07/29 6:43), Jesse Barnes wrote:
> On Wed, 28 Jul 2010 23:23:56 +0200
> "Rafael J. Wysocki"<rjw@sisk.pl>  wrote:
>
>> From: Rafael J. Wysocki<rjw@sisk.pl>
>>
>> PCIe port service drivers ask the BIOS, through _OSC, for control of
>> the services they handle.  Unfortunately, each of them individually
>> asks for control of the PCIe capability structure and if that is
>> granted, some BIOSes expect that the other PCIe port services will be
>> configured and handled by the kernel as well.  If that is not the
>> case (eg. one of the PCIe port service drivers is not loaded), the
>> BIOS may be confused and may cause the system as a whole to misbehave
>> (eg. on one of such systems enabling the native PCIe PME service
>> without loading the native PCIe hot-plug service driver causes a
>> storm of ACPI notify requests to appear).
>>
>> For this reason rework the PCIe port driver so that (1) it checks
>> which native PCIe port services can be enabled, according to the
>> BIOS, and (2) it requests control of all these services
>> simultaneously.  In particular, this causes pcie_portdrv_probe() to
>> fail if the BIOS refuses to grant control of the PCIe capability
>> structure, which means that no native PCIe port services can be
>> enabled for the PCIe root complex the given port belongs to.
>>
>> Make it possible to override this behavior using a new command line
>> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
>> default), 'native' (use the PCIe native services regardless of the
>> BIOS response to the control request), or 'compat' (do not use the
>> PCIe native services at all).
>>
>> Accordingly, rework the existing PCIe port service drivers so that
>> they don't request control of the services directly.
>>
>> Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl>
>> ---
>
> Kenji-san, are you ok with this version?  I would like to get your ack
> (and ideally your tested-by) for this one since it affects
> functionality you need.
>

Hi Jesse, Rafael,

I've just started reviewing the latest version of the patch. I have good
impression about this version so far. Please give me a few days for deeper
review. And I've booked the test machine for this patch in next week. I'll
try to test the patch and send you the result as soon as I can.

Here are two comments I have so far.

Though it is not directly related to Rafael's patch, I found one problem
in _OSC query handling while reviewing this patch. Currently, all the
_OSC controls are queried at the same time in acpi_pci_osc_support() and
the result is preserved for later acpi_pci_osc_control_set() call. But
query result can vary depending on the combination of requested controls.
So I think query result must not be preserved. Since Rafael's patch uses
query result, this problem should be fixed at the same time. I'll try to
make a patch for this and update Rafael's patch if needed.


I think the following changes should be done in the separated patch. I
guess this change is for the BIOS which enables hotplug interrupt at when
_OSC is evaluated with native hot-plug control. Right? Anyway, it should
be separated patch with appropriate description.

+			/*
+			 * Disable hot-plug interrupts in case the hot-plug
+			 * service driver is not loaded.
+			 */
+			pos += PCI_EXP_SLTCTL;
+			pci_read_config_word(dev, pos, &reg16);
+			reg16 &= ~(PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
+			pci_write_config_word(dev, pos, reg16);

Thanks,
Kenji Kaneshige

Re: [PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Rafael J. Wysocki]

On Thursday, July 29, 2010, Kenji Kaneshige wrote:
> (2010/07/29 6:43), Jesse Barnes wrote:
> > On Wed, 28 Jul 2010 23:23:56 +0200
> > "Rafael J. Wysocki"<rjw@sisk.pl>  wrote:
> >
> >> From: Rafael J. Wysocki<rjw@sisk.pl>
> >>
> >> PCIe port service drivers ask the BIOS, through _OSC, for control of
> >> the services they handle.  Unfortunately, each of them individually
> >> asks for control of the PCIe capability structure and if that is
> >> granted, some BIOSes expect that the other PCIe port services will be
> >> configured and handled by the kernel as well.  If that is not the
> >> case (eg. one of the PCIe port service drivers is not loaded), the
> >> BIOS may be confused and may cause the system as a whole to misbehave
> >> (eg. on one of such systems enabling the native PCIe PME service
> >> without loading the native PCIe hot-plug service driver causes a
> >> storm of ACPI notify requests to appear).
> >>
> >> For this reason rework the PCIe port driver so that (1) it checks
> >> which native PCIe port services can be enabled, according to the
> >> BIOS, and (2) it requests control of all these services
> >> simultaneously.  In particular, this causes pcie_portdrv_probe() to
> >> fail if the BIOS refuses to grant control of the PCIe capability
> >> structure, which means that no native PCIe port services can be
> >> enabled for the PCIe root complex the given port belongs to.
> >>
> >> Make it possible to override this behavior using a new command line
> >> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
> >> default), 'native' (use the PCIe native services regardless of the
> >> BIOS response to the control request), or 'compat' (do not use the
> >> PCIe native services at all).
> >>
> >> Accordingly, rework the existing PCIe port service drivers so that
> >> they don't request control of the services directly.
> >>
> >> Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl>
> >> ---
> >
> > Kenji-san, are you ok with this version?  I would like to get your ack
> > (and ideally your tested-by) for this one since it affects
> > functionality you need.
> >
> 
> Hi Jesse, Rafael,
> 
> I've just started reviewing the latest version of the patch. I have good
> impression about this version so far. Please give me a few days for deeper
> review. And I've booked the test machine for this patch in next week. I'll
> try to test the patch and send you the result as soon as I can.
> 
> Here are two comments I have so far.
> 
> Though it is not directly related to Rafael's patch, I found one problem
> in _OSC query handling while reviewing this patch. Currently, all the
> _OSC controls are queried at the same time in acpi_pci_osc_support() and
> the result is preserved for later acpi_pci_osc_control_set() call. But
> query result can vary depending on the combination of requested controls.

In principle it can, but acpi_pci_query_osc() asks for all aplicable control
bits, in which case the formware can only clear those bits in the response if
the corresponding features are unsupported.  Doing otherwise would be in
violation of Section 6.2.9.1. of ACPI spec 3.0b, the following in particular:

"If any bits in the Control Field are returned cleared (masked to zero) by the _OSC control method,
the respective feature is designated unsupported by the platform and must not be enabled by the OS."

> So I think query result must not be preserved. Since Rafael's patch uses
> query result, this problem should be fixed at the same time. I'll try to
> make a patch for this and update Rafael's patch if needed.

It would be sufficient to change acpi_pci_osc_control_get(), introduced
by my patch, to call acpi_pci_query_osc() unconditionally, but as I said above,
I don't think it's necessary.

> I think the following changes should be done in the separated patch. I
> guess this change is for the BIOS which enables hotplug interrupt at when
> _OSC is evaluated with native hot-plug control. Right?

Yes, it is.

> Anyway, it should be separated patch with appropriate description.

But the $subject patch does analogous things for AER and PCIe PME.  Do you
think they also should be introduced by separate patches?

The rationale is that since we've told the BIOS, by requesting
OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL,  that we're going to take the
responsibility for the PCIe native services, we should disable them to a
minimum to prevent spurious events from occurring (the BIOS might have done
something to these bits before we called _OSC and it cannot legitimately do
anything else to them after it's granted control to us).

> +			/*
> +			 * Disable hot-plug interrupts in case the hot-plug
> +			 * service driver is not loaded.
> +			 */
> +			pos += PCI_EXP_SLTCTL;
> +			pci_read_config_word(dev, pos, &reg16);
> +			reg16 &= ~(PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
> +			pci_write_config_word(dev, pos, reg16);

Thanks,
Rafael

Re: [PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Kenji Kaneshige]

(2010/07/30 0:45), Rafael J. Wysocki wrote:
> On Thursday, July 29, 2010, Kenji Kaneshige wrote:
>> (2010/07/29 6:43), Jesse Barnes wrote:
>>> On Wed, 28 Jul 2010 23:23:56 +0200
>>> "Rafael J. Wysocki"<rjw@sisk.pl>   wrote:
>>>
>>>> From: Rafael J. Wysocki<rjw@sisk.pl>
>>>>
>>>> PCIe port service drivers ask the BIOS, through _OSC, for control of
>>>> the services they handle.  Unfortunately, each of them individually
>>>> asks for control of the PCIe capability structure and if that is
>>>> granted, some BIOSes expect that the other PCIe port services will be
>>>> configured and handled by the kernel as well.  If that is not the
>>>> case (eg. one of the PCIe port service drivers is not loaded), the
>>>> BIOS may be confused and may cause the system as a whole to misbehave
>>>> (eg. on one of such systems enabling the native PCIe PME service
>>>> without loading the native PCIe hot-plug service driver causes a
>>>> storm of ACPI notify requests to appear).
>>>>
>>>> For this reason rework the PCIe port driver so that (1) it checks
>>>> which native PCIe port services can be enabled, according to the
>>>> BIOS, and (2) it requests control of all these services
>>>> simultaneously.  In particular, this causes pcie_portdrv_probe() to
>>>> fail if the BIOS refuses to grant control of the PCIe capability
>>>> structure, which means that no native PCIe port services can be
>>>> enabled for the PCIe root complex the given port belongs to.
>>>>
>>>> Make it possible to override this behavior using a new command line
>>>> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
>>>> default), 'native' (use the PCIe native services regardless of the
>>>> BIOS response to the control request), or 'compat' (do not use the
>>>> PCIe native services at all).
>>>>
>>>> Accordingly, rework the existing PCIe port service drivers so that
>>>> they don't request control of the services directly.
>>>>
>>>> Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl>
>>>> ---
>>>
>>> Kenji-san, are you ok with this version?  I would like to get your ack
>>> (and ideally your tested-by) for this one since it affects
>>> functionality you need.
>>>
>>
>> Hi Jesse, Rafael,
>>
>> I've just started reviewing the latest version of the patch. I have good
>> impression about this version so far. Please give me a few days for deeper
>> review. And I've booked the test machine for this patch in next week. I'll
>> try to test the patch and send you the result as soon as I can.
>>
>> Here are two comments I have so far.
>>
>> Though it is not directly related to Rafael's patch, I found one problem
>> in _OSC query handling while reviewing this patch. Currently, all the
>> _OSC controls are queried at the same time in acpi_pci_osc_support() and
>> the result is preserved for later acpi_pci_osc_control_set() call. But
>> query result can vary depending on the combination of requested controls.
>
> In principle it can, but acpi_pci_query_osc() asks for all aplicable control
> bits, in which case the formware can only clear those bits in the response if
> the corresponding features are unsupported.  Doing otherwise would be in
> violation of Section 6.2.9.1. of ACPI spec 3.0b, the following in particular:
>
> "If any bits in the Control Field are returned cleared (masked to zero) by the _OSC control method,
> the respective feature is designated unsupported by the platform and must not be enabled by the OS."
>

My concern is as follows.

For easy, assume as follows:
- all the _OSC controls are A, B, C
-B depends on C.
- When requesting A, B and C, all A, B and C are granted to OS.
- When requesting A, B, only A is granted to OS.

Current linux queries all A, B and C at the boot time and preserve the
result (all A, B and C can be granted to OS). Here, if a service driver
requests A and B using acpi_pci_osc_control_set(), acpi_pci_osc_control_set()
checks if both of those controls can be granted to OS. The expected result
here is only A can be granted to OS, but current linux judges both of them
can be granted because linux does this check by seeing preserved result.
As a result, acpi_pci_osc_control_set() evaluates _OSC with query flag
cleared to request A and B. Since the control B returned cleared,
acpi_pci_osc_control() returns error to the service driver. As a result,
control A is granted to OS even though there is no driver to handle the
control A.


>> So I think query result must not be preserved. Since Rafael's patch uses
>> query result, this problem should be fixed at the same time. I'll try to
>> make a patch for this and update Rafael's patch if needed.
>
> It would be sufficient to change acpi_pci_osc_control_get(), introduced
> by my patch, to call acpi_pci_query_osc() unconditionally, but as I said above,
> I don't think it's necessary.

I made a patch for this and updated your patch slightly. I'll send them soon.

>
>> I think the following changes should be done in the separated patch. I
>> guess this change is for the BIOS which enables hotplug interrupt at when
>> _OSC is evaluated with native hot-plug control. Right?
>
> Yes, it is.
>
>> Anyway, it should be separated patch with appropriate description.
>
> But the $subject patch does analogous things for AER and PCIe PME.  Do you
> think they also should be introduced by separate patches?

I think the following changes in your patch should be introduced by
separate patches.

  - optimizing the checks in acpi_pci_osc_control_set()
  - Disabling hot-plug interrupt
  - Removing module_exit()

Thanks,
Kenji Kaneshige

Re: [PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Kenji Kaneshige]

(2010/07/30 15:00), Kenji Kaneshige wrote:
> (2010/07/30 0:45), Rafael J. Wysocki wrote:
>> On Thursday, July 29, 2010, Kenji Kaneshige wrote:
>>> (2010/07/29 6:43), Jesse Barnes wrote:
>>>> On Wed, 28 Jul 2010 23:23:56 +0200
>>>> "Rafael J. Wysocki"<rjw@sisk.pl> wrote:
>>>>
>>>>> From: Rafael J. Wysocki<rjw@sisk.pl>
>>>>>
>>>>> PCIe port service drivers ask the BIOS, through _OSC, for control of
>>>>> the services they handle. Unfortunately, each of them individually
>>>>> asks for control of the PCIe capability structure and if that is
>>>>> granted, some BIOSes expect that the other PCIe port services will be
>>>>> configured and handled by the kernel as well. If that is not the
>>>>> case (eg. one of the PCIe port service drivers is not loaded), the
>>>>> BIOS may be confused and may cause the system as a whole to misbehave
>>>>> (eg. on one of such systems enabling the native PCIe PME service
>>>>> without loading the native PCIe hot-plug service driver causes a
>>>>> storm of ACPI notify requests to appear).
>>>>>
>>>>> For this reason rework the PCIe port driver so that (1) it checks
>>>>> which native PCIe port services can be enabled, according to the
>>>>> BIOS, and (2) it requests control of all these services
>>>>> simultaneously. In particular, this causes pcie_portdrv_probe() to
>>>>> fail if the BIOS refuses to grant control of the PCIe capability
>>>>> structure, which means that no native PCIe port services can be
>>>>> enabled for the PCIe root complex the given port belongs to.
>>>>>
>>>>> Make it possible to override this behavior using a new command line
>>>>> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
>>>>> default), 'native' (use the PCIe native services regardless of the
>>>>> BIOS response to the control request), or 'compat' (do not use the
>>>>> PCIe native services at all).
>>>>>
>>>>> Accordingly, rework the existing PCIe port service drivers so that
>>>>> they don't request control of the services directly.
>>>>>
>>>>> Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl>
>>>>> ---
>>>>
>>>> Kenji-san, are you ok with this version? I would like to get your ack
>>>> (and ideally your tested-by) for this one since it affects
>>>> functionality you need.
>>>>
>>>
>>> Hi Jesse, Rafael,
>>>
>>> I've just started reviewing the latest version of the patch. I have good
>>> impression about this version so far. Please give me a few days for
>>> deeper
>>> review. And I've booked the test machine for this patch in next week.
>>> I'll
>>> try to test the patch and send you the result as soon as I can.
>>>
>>> Here are two comments I have so far.
>>>
>>> Though it is not directly related to Rafael's patch, I found one problem
>>> in _OSC query handling while reviewing this patch. Currently, all the
>>> _OSC controls are queried at the same time in acpi_pci_osc_support() and
>>> the result is preserved for later acpi_pci_osc_control_set() call. But
>>> query result can vary depending on the combination of requested
>>> controls.
>>
>> In principle it can, but acpi_pci_query_osc() asks for all aplicable
>> control
>> bits, in which case the formware can only clear those bits in the
>> response if
>> the corresponding features are unsupported. Doing otherwise would be in
>> violation of Section 6.2.9.1. of ACPI spec 3.0b, the following in
>> particular:
>>
>> "If any bits in the Control Field are returned cleared (masked to
>> zero) by the _OSC control method,
>> the respective feature is designated unsupported by the platform and
>> must not be enabled by the OS."
>>
>
> My concern is as follows.
>
> For easy, assume as follows:
> - all the _OSC controls are A, B, C
> -B depends on C.
> - When requesting A, B and C, all A, B and C are granted to OS.
> - When requesting A, B, only A is granted to OS.
>
> Current linux queries all A, B and C at the boot time and preserve the
> result (all A, B and C can be granted to OS). Here, if a service driver
> requests A and B using acpi_pci_osc_control_set(),
> acpi_pci_osc_control_set()
> checks if both of those controls can be granted to OS. The expected result
> here is only A can be granted to OS, but current linux judges both of them
> can be granted because linux does this check by seeing preserved result.
> As a result, acpi_pci_osc_control_set() evaluates _OSC with query flag
> cleared to request A and B. Since the control B returned cleared,
> acpi_pci_osc_control() returns error to the service driver. As a result,
> control A is granted to OS even though there is no driver to handle the
> control A.
>
>
>>> So I think query result must not be preserved. Since Rafael's patch uses
>>> query result, this problem should be fixed at the same time. I'll try to
>>> make a patch for this and update Rafael's patch if needed.
>>
>> It would be sufficient to change acpi_pci_osc_control_get(), introduced
>> by my patch, to call acpi_pci_query_osc() unconditionally, but as I
>> said above,
>> I don't think it's necessary.
>
> I made a patch for this and updated your patch slightly. I'll send them
> soon.
>
>>
>>> I think the following changes should be done in the separated patch. I
>>> guess this change is for the BIOS which enables hotplug interrupt at
>>> when
>>> _OSC is evaluated with native hot-plug control. Right?
>>
>> Yes, it is.
>>
>>> Anyway, it should be separated patch with appropriate description.
>>
>> But the $subject patch does analogous things for AER and PCIe PME. Do you
>> think they also should be introduced by separate patches?
>
> I think the following changes in your patch should be introduced by
> separate patches.
>
> - optimizing the checks in acpi_pci_osc_control_set()
> - Disabling hot-plug interrupt
> - Removing module_exit()
>
> Thanks,
> Kenji Kaneshige

Hi Rafael,

Here is an updated version of your patch. The set of patches consists
of six patches. The [PATCH 1/6] and [PATCH 2/6] is to change _OSC
query handling as I explained in the previous e-mail. Remaining
patches are based on your change (I split the original one into some
pieces). Please see below about the summary of changes.

NOTE: I have not tested it at all yet (compile only). This is request
       for comment.

- [PATCH 1/5] ACPI/PCI: cleanup acpi_pci_run_osc
   Cleanup acpi_pci_run_osc() to make the following patch cleaner.

- [PATCH 2/5] ACPI/PCI: do not preserve query result
   Fix the possible problem that some of the requested _OSC controls
   are granted to OS even though acpi_pci_osc_control_set() fails. That
   is, some of the requested controls for this service is granted to OS
   even though OS doesn't handle the service.

- [PATCH 3/5] ACPI/PCI: optimize checks in acpi_pci_osc_control_set()
   Make the following optimization in Rafael's v.4 patch as a separated
   patch here.
   ================================================================
   -       status = acpi_get_handle(handle, "_OSC", &tmp);
   -       if (ACPI_FAILURE(status))
   -               return status;
   -
           control_req = (flags & OSC_PCI_CONTROL_MASKS);
           if (!control_req)
               return AE_TYPE;
   @@ -389,6 +419,10 @@ acpi_status acpi_pci_osc_control_set(acp
           if (!root)
                   return AE_NOT_EXIST;

   +       status = acpi_get_handle(handle, "_OSC", &tmp);
   +       if (ACPI_FAILURE(status))
   +               return status;
   +
     ================================================================

- [PATCH 4/5] ACPI/PCI: ask bios for control of all native services at once
   ChangeLog from Rafael's v.4:
   ============================
   * Rewrote acpi_pci_osc_control_get() becasuse _OSC query handling is
     changed by [PATCH 2/5]. And renamed acpi_pci_osc_control_get() to
     acpi_pci_osc_control_query() because latter one is more
     straightforward.
   * Moved the optimization for acpi_pci_osc_control_set() into the
     separated patch ([PATCH 3/6]).
   * Moved the change to disable native hot-plug interrupt into the
     separated patch ([PATCH 5/6]).
   * Moved the change to remove module_exit() into the separated patch
     ([PATCH 6/6]).

- [PATCH 5/6] PCI: portdrv: disable native hot-plug interrupt
   Make the change to disable native hot-plug interrupt as a separated
   patch here.

- [PATCH 6/6] PCI: portdrv: remove module_exit
   Make the change to remove module_exit() as a separated patch here.

Regards,
Kenji Kaneshige

[PATCH 1/6] ACPI/PCI: cleanup acpi_pci_run_osc

[Kenji Kaneshige]

Make capabilitis buffer in acpi_pci_run_osc() instead of caller of
this function. This makes the code a little cleaner. This has no
functional changes.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_root.c |   34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
+++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
@@ -206,9 +206,10 @@ static void acpi_pci_bridge_scan(struct 
 
 static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
 
-static acpi_status acpi_pci_run_osc(acpi_handle handle,
-				    const u32 *capbuf, u32 *retval)
+static acpi_status acpi_pci_run_osc(acpi_handle handle, u32 support,
+				    u32 control, bool query, u32 *retval)
 {
+	u32 capbuf[3];
 	struct acpi_osc_context context = {
 		.uuid_str = pci_osc_uuid_str,
 		.rev = 1,
@@ -217,6 +218,10 @@ static acpi_status acpi_pci_run_osc(acpi
 	};
 	acpi_status status;
 
+	capbuf[OSC_QUERY_TYPE] = (query == true) ? OSC_QUERY_ENABLE : 0;
+	capbuf[OSC_SUPPORT_TYPE] = support;
+	capbuf[OSC_CONTROL_TYPE] = control;
+
 	status = acpi_run_osc(handle, &context);
 	if (ACPI_SUCCESS(status)) {
 		*retval = *((u32 *)(context.ret.pointer + 8));
@@ -228,17 +233,16 @@ static acpi_status acpi_pci_run_osc(acpi
 static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 flags)
 {
 	acpi_status status;
-	u32 support_set, result, capbuf[3];
+	u32 result;
 
 	/* do _OSC query for all possible controls */
-	support_set = root->osc_support_set | (flags & OSC_PCI_SUPPORT_MASKS);
-	capbuf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
-	capbuf[OSC_SUPPORT_TYPE] = support_set;
-	capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
-
-	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
+	flags &= OSC_PCI_SUPPORT_MASKS;
+	status = acpi_pci_run_osc(root->device->handle,
+				  root->osc_support_set | flags,
+				  OSC_PCI_CONTROL_MASKS,
+				  true, &result);
 	if (ACPI_SUCCESS(status)) {
-		root->osc_support_set = support_set;
+		root->osc_support_set |= flags;
 		root->osc_control_qry = result;
 		root->osc_queried = 1;
 	}
@@ -373,7 +377,7 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
 acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags)
 {
 	acpi_status status;
-	u32 control_req, result, capbuf[3];
+	u32 control_req, result;
 	acpi_handle tmp;
 	struct acpi_pci_root *root;
 
@@ -407,10 +411,10 @@ acpi_status acpi_pci_osc_control_set(acp
 		goto out;
 	}
 
-	capbuf[OSC_QUERY_TYPE] = 0;
-	capbuf[OSC_SUPPORT_TYPE] = root->osc_support_set;
-	capbuf[OSC_CONTROL_TYPE] = root->osc_control_set | control_req;
-	status = acpi_pci_run_osc(handle, capbuf, &result);
+	status = acpi_pci_run_osc(handle,
+				  root->osc_support_set,
+				  root->osc_control_set | control_req,
+				  false, &result);
 	if (ACPI_SUCCESS(status))
 		root->osc_control_set = result;
 out:

[PATCH 2/6] ACPI/PCI: do not preserve query result

[Kenji Kaneshige]

Currently, all the _OSC controls are queried at the same time in
acpi_pci_osc_support() and the result is preserved for later
acpi_pci_osc_control_set() call. But query result can vary depending
on the combination of requested controls. Therefore, query result must
not be preserved.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_root.c |   49 ++++++++++++++++++++++++++----------------------
 include/acpi/acpi_bus.h |    3 --
 2 files changed, 27 insertions(+), 25 deletions(-)

Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
+++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
@@ -230,35 +230,42 @@ static acpi_status acpi_pci_run_osc(acpi
 	return status;
 }
 
-static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 flags)
+static acpi_status acpi_pci_osc_control_query(struct acpi_pci_root *root,
+					      u32 flags, u32 *result)
 {
-	acpi_status status;
-	u32 result;
-
-	/* do _OSC query for all possible controls */
-	flags &= OSC_PCI_SUPPORT_MASKS;
-	status = acpi_pci_run_osc(root->device->handle,
-				  root->osc_support_set | flags,
-				  OSC_PCI_CONTROL_MASKS,
-				  true, &result);
-	if (ACPI_SUCCESS(status)) {
-		root->osc_support_set |= flags;
-		root->osc_control_qry = result;
-		root->osc_queried = 1;
+	/* No need to run _OSC if requested control bits are already granted */
+	flags &= OSC_PCI_CONTROL_MASKS;
+	if ((root->osc_control_set & flags) == flags) {
+		*result = root->osc_control_set;
+		return AE_OK;
 	}
-	return status;
+	return acpi_pci_run_osc(root->device->handle,
+				root->osc_support_set,
+				root->osc_control_set | flags,
+				true, result);
 }
 
 static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
 {
 	acpi_status status;
 	acpi_handle tmp;
+	u32 result;
 
 	status = acpi_get_handle(root->device->handle, "_OSC", &tmp);
 	if (ACPI_FAILURE(status))
 		return status;
 	mutex_lock(&osc_lock);
-	status = acpi_pci_query_osc(root, flags);
+	/* No need to run _OSC if requested support bits are already set */
+	flags &= OSC_PCI_SUPPORT_MASKS;
+	if ((root->osc_support_set & flags) == flags)
+		goto out;
+	status = acpi_pci_run_osc(root->device->handle,
+				  root->osc_support_set | flags,
+				  root->osc_control_set,
+				  true, &result);
+	if (ACPI_SUCCESS(status))
+		root->osc_support_set |= flags;
+out:
 	mutex_unlock(&osc_lock);
 	return status;
 }
@@ -399,12 +406,10 @@ acpi_status acpi_pci_osc_control_set(acp
 		goto out;
 
 	/* Need to query controls first before requesting them */
-	if (!root->osc_queried) {
-		status = acpi_pci_query_osc(root, root->osc_support_set);
-		if (ACPI_FAILURE(status))
-			goto out;
-	}
-	if ((root->osc_control_qry & control_req) != control_req) {
+	status = acpi_pci_osc_control_query(root, control_req, &result);
+	if (ACPI_FAILURE(status))
+		goto out;
+	if ((result & control_req) != control_req) {
 		printk(KERN_DEBUG
 		       "Firmware did not grant requested _OSC control\n");
 		status = AE_SUPPORT;
Index: linux-2.6.35-rc6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.35-rc6.orig/include/acpi/acpi_bus.h
+++ linux-2.6.35-rc6/include/acpi/acpi_bus.h
@@ -377,9 +377,6 @@ struct acpi_pci_root {
 
 	u32 osc_support_set;	/* _OSC state of support bits */
 	u32 osc_control_set;	/* _OSC state of control bits */
-	u32 osc_control_qry;	/* the latest _OSC query result */
-
-	u32 osc_queried:1;	/* has _OSC control been queried? */
 };
 
 /* helper */

[PATCH 3/6] ACPI/PCI: optimize checks in acpi_pci_osc_control_set()

[Kenji Kaneshige]

Check for ACPI _OSC object is heavier than other checks in
acpi_pci_osc_control_set(). So move it after the other checks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_root.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
+++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
@@ -388,10 +388,6 @@ acpi_status acpi_pci_osc_control_set(acp
 	acpi_handle tmp;
 	struct acpi_pci_root *root;
 
-	status = acpi_get_handle(handle, "_OSC", &tmp);
-	if (ACPI_FAILURE(status))
-		return status;
-
 	control_req = (flags & OSC_PCI_CONTROL_MASKS);
 	if (!control_req)
 		return AE_TYPE;
@@ -400,6 +396,10 @@ acpi_status acpi_pci_osc_control_set(acp
 	if (!root)
 		return AE_NOT_EXIST;
 
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
+
 	mutex_lock(&osc_lock);
 	/* No need to evaluate _OSC if the control was already granted. */
 	if ((root->osc_control_set & control_req) == control_req)

[PATCH 4/6] ACPI/PCI: ask bios for control of all native services at once

[Kenji Kaneshige]

PCIe port service drivers ask the BIOS, through _OSC, for control of
the services they handle.  Unfortunately, each of them individually
asks for control of the PCIe capability structure and if that is
granted, some BIOSes expect that the other PCIe port services will be
configured and handled by the kernel as well.  If that is not the
case (eg. one of the PCIe port service drivers is not loaded), the
BIOS may be confused and may cause the system as a whole to misbehave
(eg. on one of such systems enabling the native PCIe PME service
without loading the native PCIe hot-plug service driver causes a
storm of ACPI notify requests to appear).

For this reason rework the PCIe port driver so that (1) it checks
which native PCIe port services can be enabled, according to the
BIOS, and (2) it requests control of all these services
simultaneously.  In particular, this causes pcie_portdrv_probe() to
fail if the BIOS refuses to grant control of the PCIe capability
structure, which means that no native PCIe port services can be
enabled for the PCIe root complex the given port belongs to.

Make it possible to override this behavior using a new command line
switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
default), 'native' (use the PCIe native services regardless of the
BIOS response to the control request), or 'compat' (do not use the
PCIe native services at all).

Accordingly, rework the existing PCIe port service drivers so that
they don't request control of the services directly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Tested-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 Documentation/kernel-parameters.txt  |   17 +++---
 drivers/acpi/pci_root.c              |   44 +++++++++++++++--
 drivers/pci/hotplug/acpi_pcihp.c     |    4 -
 drivers/pci/hotplug/pciehp.h         |   12 ----
 drivers/pci/hotplug/pciehp_acpi.c    |    4 -
 drivers/pci/hotplug/pciehp_core.c    |    4 -
 drivers/pci/pcie/Makefile            |    3 -
 drivers/pci/pcie/aer/aerdrv_acpi.c   |   36 --------------
 drivers/pci/pcie/aer/aerdrv_core.c   |   14 -----
 drivers/pci/pcie/pme/Makefile        |    8 ---
 drivers/pci/pcie/pme/pcie_pme.c      |   64 +------------------------
 drivers/pci/pcie/pme/pcie_pme.h      |   28 -----------
 drivers/pci/pcie/pme/pcie_pme_acpi.c |   54 ---------------------
 drivers/pci/pcie/portdrv.h           |   22 ++++++++
 drivers/pci/pcie/portdrv_acpi.c      |   88 +++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv_core.c      |   26 +++++++++-
 drivers/pci/pcie/portdrv_pci.c       |   30 +++++++++++
 include/linux/acpi.h                 |    2 
 include/linux/pcieport_if.h          |    5 +
 19 files changed, 231 insertions(+), 234 deletions(-)

Index: linux-2.6.35-rc6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.35-rc6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6.35-rc6/Documentation/kernel-parameters.txt
@@ -2047,15 +2047,18 @@ and is between 256 and 4096 characters. 
 		force	Enable ASPM even on devices that claim not to support it.
 			WARNING: Forcing ASPM on may cause system lockups.
 
+	pcie_ports=	[PCIE] PCIe ports handling:
+		auto	Ask the BIOS whether or not to use native PCIe services
+			associated with PCIe ports (PME, hot-plug, AER).  Use
+			them only if that is allowed by the BIOS.
+		native	Use native PCIe services associated with PCIe ports
+			unconditionally.
+		compat	Treat PCIe ports as PCI-to-PCI bridges, disable the PCIe
+			ports driver.
+
 	pcie_pme=	[PCIE,PM] Native PCIe PME signaling options:
-			Format: {auto|force}[,nomsi]
-		auto	Use native PCIe PME signaling if the BIOS allows the
-			kernel to control PCIe config registers of root ports.
-		force	Use native PCIe PME signaling even if the BIOS refuses
-			to allow the kernel to control the relevant PCIe config
-			registers.
 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
-			all PCIe root ports use INTx for everything).
+			all PCIe root ports use INTx for all services).
 
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
+++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
@@ -230,8 +230,8 @@ static acpi_status acpi_pci_run_osc(acpi
 	return status;
 }
 
-static acpi_status acpi_pci_osc_control_query(struct acpi_pci_root *root,
-					      u32 flags, u32 *result)
+static acpi_status __acpi_pci_osc_control_query(struct acpi_pci_root *root,
+						u32 flags, u32 *result)
 {
 	/* No need to run _OSC if requested control bits are already granted */
 	flags &= OSC_PCI_CONTROL_MASKS;
@@ -406,12 +406,12 @@ acpi_status acpi_pci_osc_control_set(acp
 		goto out;
 
 	/* Need to query controls first before requesting them */
-	status = acpi_pci_osc_control_query(root, control_req, &result);
+	status = __acpi_pci_osc_control_query(root, control_req, &result);
 	if (ACPI_FAILURE(status))
 		goto out;
 	if ((result & control_req) != control_req) {
-		printk(KERN_DEBUG
-		       "Firmware did not grant requested _OSC control\n");
+		pr_info("ACPI: Requested _OSC control for unsupported mask "
+			"0x%02x\n", control_req);
 		status = AE_SUPPORT;
 		goto out;
 	}
@@ -428,6 +428,40 @@ out:
 }
 EXPORT_SYMBOL(acpi_pci_osc_control_set);
 
+/**
+ * acpi_pci_osc_control_query - query requested control to firmware
+ * @handle: acpi_handle for the target ACPI object
+ * @flags: driver's requested control bits
+ * @result: pointer to the buffer to store the result
+ *
+ * Query requested controls to firmware and store the result to the
+ * buffer. If the requested control can be granted to the OS,
+ * corresponding bit in @result is set. Note that control bits already
+ * granted are set in @result regardless of @flags. So you can see set
+ * of control bits that are already granted by specifying zero to @flags.
+ */
+acpi_status acpi_pci_osc_control_query(acpi_handle handle,
+				       u32 flags, u32 *result)
+{
+	acpi_status status;
+	acpi_handle tmp;
+	struct acpi_pci_root *root;
+
+	root = acpi_pci_find_root(handle);
+	if (!root)
+		return AE_NOT_EXIST;
+
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	mutex_lock(&osc_lock);
+	status = __acpi_pci_osc_control_query(root, flags, result);
+	mutex_unlock(&osc_lock);
+
+	return status;
+}
+
 static int __devinit acpi_pci_root_add(struct acpi_device *device)
 {
 	unsigned long long segment, bus;
Index: linux-2.6.35-rc6/drivers/pci/hotplug/acpi_pcihp.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/hotplug/acpi_pcihp.c
+++ linux-2.6.35-rc6/drivers/pci/hotplug/acpi_pcihp.c
@@ -338,9 +338,7 @@ int acpi_get_hp_hw_control_from_firmware
 	acpi_handle chandle, handle;
 	struct acpi_buffer string = { ACPI_ALLOCATE_BUFFER, NULL };
 
-	flags &= (OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
-		  OSC_SHPC_NATIVE_HP_CONTROL |
-		  OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
+	flags &= OSC_SHPC_NATIVE_HP_CONTROL;
 	if (!flags) {
 		err("Invalid flags %u specified!\n", flags);
 		return -EINVAL;
Index: linux-2.6.35-rc6/drivers/pci/hotplug/pciehp.h
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/hotplug/pciehp.h
+++ linux-2.6.35-rc6/drivers/pci/hotplug/pciehp.h
@@ -176,19 +176,7 @@ static inline void pciehp_firmware_init(
 {
 	pciehp_acpi_slot_detection_init();
 }
-
-static inline int pciehp_get_hp_hw_control_from_firmware(struct pci_dev *dev)
-{
-	int retval;
-	u32 flags = (OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
-		     OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-	retval = acpi_get_hp_hw_control_from_firmware(dev, flags);
-	if (retval)
-		return retval;
-	return pciehp_acpi_slot_detection_check(dev);
-}
 #else
 #define pciehp_firmware_init()				do {} while (0)
-#define pciehp_get_hp_hw_control_from_firmware(dev) 	0
 #endif 				/* CONFIG_ACPI */
 #endif				/* _PCIEHP_H */
Index: linux-2.6.35-rc6/drivers/pci/hotplug/pciehp_acpi.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/hotplug/pciehp_acpi.c
+++ linux-2.6.35-rc6/drivers/pci/hotplug/pciehp_acpi.c
@@ -85,9 +85,7 @@ static int __init dummy_probe(struct pci
 	acpi_handle handle;
 	struct dummy_slot *slot, *tmp;
 	struct pci_dev *pdev = dev->port;
-	/* Note: pciehp_detect_mode != PCIEHP_DETECT_ACPI here */
-	if (pciehp_get_hp_hw_control_from_firmware(pdev))
-		return -ENODEV;
+
 	pos = pci_pcie_cap(pdev);
 	if (!pos)
 		return -ENODEV;
Index: linux-2.6.35-rc6/drivers/pci/hotplug/pciehp_core.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/hotplug/pciehp_core.c
+++ linux-2.6.35-rc6/drivers/pci/hotplug/pciehp_core.c
@@ -59,7 +59,7 @@ module_param(pciehp_force, bool, 0644);
 MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
 MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
 MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
-MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if _OSC and OSHP are missing");
+MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if OSHP is missing");
 
 #define PCIE_MODULE_NAME "pciehp"
 
@@ -235,7 +235,7 @@ static int pciehp_probe(struct pcie_devi
 		dev_info(&dev->device,
 			 "Bypassing BIOS check for pciehp use on %s\n",
 			 pci_name(dev->port));
-	else if (pciehp_get_hp_hw_control_from_firmware(dev->port))
+	else if (pciehp_acpi_slot_detection_check(dev->port))
 		goto err_out_none;
 
 	ctrl = pcie_init(dev);
Index: linux-2.6.35-rc6/drivers/pci/pcie/Makefile
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/Makefile
+++ linux-2.6.35-rc6/drivers/pci/pcie/Makefile
@@ -6,10 +6,11 @@
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
 pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
 
 # Build PCI Express AER if needed
 obj-$(CONFIG_PCIEAER)		+= aer/
 
-obj-$(CONFIG_PCIE_PME) += pme/
+obj-$(CONFIG_PCIE_PME) += pme/pcie_pme.o
Index: linux-2.6.35-rc6/drivers/pci/pcie/aer/aerdrv_acpi.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -19,42 +19,6 @@
 #include <acpi/apei.h>
 #include "aerdrv.h"
 
-/**
- * aer_osc_setup - run ACPI _OSC method
- * @pciedev: pcie_device which AER is being enabled on
- *
- * @return: Zero on success. Nonzero otherwise.
- *
- * Invoked when PCIe bus loads AER service driver. To avoid conflict with
- * BIOS AER support requires BIOS to yield AER control to OS native driver.
- **/
-int aer_osc_setup(struct pcie_device *pciedev)
-{
-	acpi_status status = AE_NOT_FOUND;
-	struct pci_dev *pdev = pciedev->port;
-	acpi_handle handle = NULL;
-
-	if (acpi_pci_disabled)
-		return -1;
-
-	handle = acpi_find_root_bridge_handle(pdev);
-	if (handle) {
-		status = acpi_pci_osc_control_set(handle,
-					OSC_PCI_EXPRESS_AER_CONTROL |
-					OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-	}
-
-	if (ACPI_FAILURE(status)) {
-		dev_printk(KERN_DEBUG, &pciedev->device, "AER service couldn't "
-			   "init device: %s\n",
-			   (status == AE_SUPPORT || status == AE_NOT_FOUND) ?
-			   "no _OSC support" : "_OSC failed");
-		return -1;
-	}
-
-	return 0;
-}
-
 #ifdef CONFIG_ACPI_APEI
 static inline int hest_match_pci(struct acpi_hest_aer_common *p,
 				 struct pci_dev *pci)
Index: linux-2.6.35-rc6/drivers/pci/pcie/aer/aerdrv_core.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/aer/aerdrv_core.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/aer/aerdrv_core.c
@@ -771,22 +771,10 @@ void aer_isr(struct work_struct *work)
  */
 int aer_init(struct pcie_device *dev)
 {
-	if (pcie_aer_get_firmware_first(dev->port)) {
-		dev_printk(KERN_DEBUG, &dev->device,
-			   "PCIe errors handled by platform firmware.\n");
-		goto out;
-	}
-
-	if (aer_osc_setup(dev))
-		goto out;
-
-	return 0;
-out:
 	if (forceload) {
 		dev_printk(KERN_DEBUG, &dev->device,
 			   "aerdrv forceload requested.\n");
 		pcie_aer_force_firmware_first(dev->port, 0);
-		return 0;
 	}
-	return -ENXIO;
+	return 0;
 }
Index: linux-2.6.35-rc6/drivers/pci/pcie/pme/Makefile
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/pme/Makefile
+++ /dev/null
@@ -1,8 +0,0 @@
-#
-# Makefile for PCI-Express Root Port PME signaling driver
-#
-
-obj-$(CONFIG_PCIE_PME) += pmedriver.o
-
-pmedriver-objs := pcie_pme.o
-pmedriver-$(CONFIG_ACPI) += pcie_pme_acpi.o
Index: linux-2.6.35-rc6/drivers/pci/pcie/pme/pcie_pme.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/pme/pcie_pme.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/pme/pcie_pme.c
@@ -24,37 +24,12 @@
 #include <linux/pm_runtime.h>
 
 #include "../../pci.h"
-#include "pcie_pme.h"
+#include "../portdrv.h"
 
 #define PCI_EXP_RTSTA_PME	0x10000 /* PME status */
 #define PCI_EXP_RTSTA_PENDING	0x20000 /* PME pending */
 
 /*
- * If set, this switch will prevent the PCIe root port PME service driver from
- * being registered.  Consequently, the interrupt-based PCIe PME signaling will
- * not be used by any PCIe root ports in that case.
- */
-static bool pcie_pme_disabled = true;
-
-/*
- * The PCI Express Base Specification 2.0, Section 6.1.8, states the following:
- * "In order to maintain compatibility with non-PCI Express-aware system
- * software, system power management logic must be configured by firmware to use
- * the legacy mechanism of signaling PME by default.  PCI Express-aware system
- * software must notify the firmware prior to enabling native, interrupt-based
- * PME signaling."  However, if the platform doesn't provide us with a suitable
- * notification mechanism or the notification fails, it is not clear whether or
- * not we are supposed to use the interrupt-based PCIe PME signaling.  The
- * switch below can be used to indicate the desired behaviour.  When set, it
- * will make the kernel use the interrupt-based PCIe PME signaling regardless of
- * the platform notification status, although the kernel will attempt to notify
- * the platform anyway.  When unset, it will prevent the kernel from using the
- * the interrupt-based PCIe PME signaling if the platform notification fails,
- * which is the default.
- */
-static bool pcie_pme_force_enable;
-
-/*
  * If this switch is set, MSI will not be used for PCIe PME signaling.  This
  * causes the PCIe port driver to use INTx interrupts only, but it turns out
  * that using MSI for PCIe PME signaling doesn't play well with PCIe PME-based
@@ -64,38 +39,13 @@ bool pcie_pme_msi_disabled;
 
 static int __init pcie_pme_setup(char *str)
 {
-	if (!strncmp(str, "auto", 4))
-		pcie_pme_disabled = false;
-	else if (!strncmp(str, "force", 5))
-		pcie_pme_force_enable = true;
-
-	str = strchr(str, ',');
-	if (str) {
-		str++;
-		str += strspn(str, " \t");
-		if (*str && !strcmp(str, "nomsi"))
-			pcie_pme_msi_disabled = true;
-	}
+	if (!strncmp(str, "nomsi", 5))
+		pcie_pme_msi_disabled = true;
 
 	return 1;
 }
 __setup("pcie_pme=", pcie_pme_setup);
 
-/**
- * pcie_pme_platform_setup - Ensure that the kernel controls the PCIe PME.
- * @srv: PCIe PME root port service to use for carrying out the check.
- *
- * Notify the platform that the native PCIe PME is going to be used and return
- * 'true' if the control of the PCIe PME registers has been acquired from the
- * platform.
- */
-static bool pcie_pme_platform_setup(struct pcie_device *srv)
-{
-	if (!pcie_pme_platform_notify(srv))
-		return true;
-	return pcie_pme_force_enable;
-}
-
 struct pcie_pme_service_data {
 	spinlock_t lock;
 	struct pcie_device *srv;
@@ -108,7 +58,7 @@ struct pcie_pme_service_data {
  * @dev: PCIe root port or event collector.
  * @enable: Enable or disable the interrupt.
  */
-static void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable)
+void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable)
 {
 	int rtctl_pos;
 	u16 rtctl;
@@ -414,9 +364,6 @@ static int pcie_pme_probe(struct pcie_de
 	struct pcie_pme_service_data *data;
 	int ret;
 
-	if (!pcie_pme_platform_setup(srv))
-		return -EACCES;
-
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -506,8 +453,7 @@ static struct pcie_port_service_driver p
  */
 static int __init pcie_pme_service_init(void)
 {
-	return pcie_pme_disabled ?
-		-ENODEV : pcie_port_service_register(&pcie_pme_driver);
+	return pcie_port_service_register(&pcie_pme_driver);
 }
 
 module_init(pcie_pme_service_init);
Index: linux-2.6.35-rc6/drivers/pci/pcie/pme/pcie_pme.h
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/pme/pcie_pme.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * drivers/pci/pcie/pme/pcie_pme.h
- *
- * PCI Express Root Port PME signaling support
- *
- * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
- */
-
-#ifndef _PCIE_PME_H_
-#define _PCIE_PME_H_
-
-struct pcie_device;
-
-#ifdef CONFIG_ACPI
-extern int pcie_pme_acpi_setup(struct pcie_device *srv);
-
-static inline int pcie_pme_platform_notify(struct pcie_device *srv)
-{
-	return pcie_pme_acpi_setup(srv);
-}
-#else /* !CONFIG_ACPI */
-static inline int pcie_pme_platform_notify(struct pcie_device *srv)
-{
-	return 0;
-}
-#endif /* !CONFIG_ACPI */
-
-#endif
Index: linux-2.6.35-rc6/drivers/pci/pcie/pme/pcie_pme_acpi.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/pme/pcie_pme_acpi.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * PCIe Native PME support, ACPI-related part
- *
- * Copyright (C) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License V2.  See the file "COPYING" in the main directory of this archive
- * for more details.
- */
-
-#include <linux/pci.h>
-#include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/acpi.h>
-#include <linux/pci-acpi.h>
-#include <linux/pcieport_if.h>
-
-/**
- * pcie_pme_acpi_setup - Request the ACPI BIOS to release control over PCIe PME.
- * @srv - PCIe PME service for a root port or event collector.
- *
- * Invoked when the PCIe bus type loads PCIe PME service driver.  To avoid
- * conflict with the BIOS PCIe support requires the BIOS to yield PCIe PME
- * control to the kernel.
- */
-int pcie_pme_acpi_setup(struct pcie_device *srv)
-{
-	acpi_status status = AE_NOT_FOUND;
-	struct pci_dev *port = srv->port;
-	acpi_handle handle;
-	int error = 0;
-
-	if (acpi_pci_disabled)
-		return -ENOSYS;
-
-	dev_info(&port->dev, "Requesting control of PCIe PME from ACPI BIOS\n");
-
-	handle = acpi_find_root_bridge_handle(port);
-	if (!handle)
-		return -EINVAL;
-
-	status = acpi_pci_osc_control_set(handle,
-			OSC_PCI_EXPRESS_PME_CONTROL |
-			OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-	if (ACPI_FAILURE(status)) {
-		dev_info(&port->dev,
-			"Failed to receive control of PCIe PME service: %s\n",
-			(status == AE_SUPPORT || status == AE_NOT_FOUND) ?
-			"no _OSC support" : "ACPI _OSC failed");
-		error = -ENODEV;
-	}
-
-	return error;
-}
Index: linux-2.6.35-rc6/drivers/pci/pcie/portdrv.h
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/portdrv.h
+++ linux-2.6.35-rc6/drivers/pci/pcie/portdrv.h
@@ -20,6 +20,9 @@
 
 #define get_descriptor_id(type, service) (((type - 4) << 4) | service)
 
+extern bool pcie_ports_disabled;
+extern bool pcie_ports_auto;
+
 extern struct bus_type pcie_port_bus_type;
 extern int pcie_port_device_register(struct pci_dev *dev);
 #ifdef CONFIG_PM
@@ -30,6 +33,8 @@ extern void pcie_port_device_remove(stru
 extern int __must_check pcie_port_bus_register(void);
 extern void pcie_port_bus_unregister(void);
 
+struct pci_dev;
+
 #ifdef CONFIG_PCIE_PME
 extern bool pcie_pme_msi_disabled;
 
@@ -42,9 +47,26 @@ static inline bool pcie_pme_no_msi(void)
 {
 	return pcie_pme_msi_disabled;
 }
+
+extern void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable);
 #else /* !CONFIG_PCIE_PME */
 static inline void pcie_pme_disable_msi(void) {}
 static inline bool pcie_pme_no_msi(void) { return false; }
+static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {}
 #endif /* !CONFIG_PCIE_PME */
 
+#ifdef CONFIG_ACPI
+extern int pcie_port_acpi_setup(struct pci_dev *port, int *mask);
+
+static inline int pcie_port_platform_notify(struct pci_dev *port, int *mask)
+{
+	return pcie_port_acpi_setup(port, mask);
+}
+#else /* !CONFIG_ACPI */
+static inline int pcie_port_platform_notify(struct pci_dev *port, int *mask)
+{
+	return 0;
+}
+#endif /* !CONFIG_ACPI */
+
 #endif /* _PORTDRV_H_ */
Index: linux-2.6.35-rc6/drivers/pci/pcie/portdrv_acpi.c
===================================================================
--- /dev/null
+++ linux-2.6.35-rc6/drivers/pci/pcie/portdrv_acpi.c
@@ -0,0 +1,88 @@
+/*
+ * PCIe Port Native Services Support, ACPI-Related Part
+ *
+ * Copyright (C) 2010 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License V2.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/pci.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/acpi.h>
+#include <linux/pci-acpi.h>
+#include <linux/pcieport_if.h>
+
+#include "aer/aerdrv.h"
+
+/**
+ * pcie_port_acpi_setup - Request the BIOS to release control of PCIe services.
+ * @port: PCIe Port service for a root port or event collector.
+ * @srv_mask: Bit mask of services that can be enabled for @port.
+ *
+ * Invoked when @port is identified as a PCIe port device.  To avoid conflicts
+ * with the BIOS PCIe port native services support requires the BIOS to yield
+ * control of these services to the kernel.  The mask of services that the BIOS
+ * allows to be enabled for @port is written to @srv_mask.
+ *
+ * NOTE: It turns out that we cannot do that for individual port services
+ * separately, because that would make some systems work incorrectly.
+ */
+int pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
+{
+	acpi_status status;
+	acpi_handle handle;
+	u32 flags, request;
+
+	if (acpi_pci_disabled)
+		return 0;
+
+	handle = acpi_find_root_bridge_handle(port);
+	if (!handle)
+		return -EINVAL;
+
+	request = (OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL |
+		   OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
+		   OSC_PCI_EXPRESS_PME_CONTROL |
+		   OSC_PCI_EXPRESS_AER_CONTROL);
+
+	if (pcie_aer_get_firmware_first(port)) {
+		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
+		request &= ~OSC_PCI_EXPRESS_AER_CONTROL;
+	}
+
+	status = acpi_pci_osc_control_query(handle, request, &flags);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&port->dev, "ACPI _OSC query failed (code %d)\n",
+			status);
+		return -ENODEV;
+	}
+
+	if (!(flags & OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)) {
+		dev_dbg(&port->dev, "BIOS refuses to grant control of PCIe "
+			"Capability Structure\n");
+		return -EACCES;
+	}
+
+	request &= flags;
+	*srv_mask = PCIE_PORT_SERVICE_VC;
+	if (request & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
+		*srv_mask |= PCIE_PORT_SERVICE_HP;
+	if (request & OSC_PCI_EXPRESS_PME_CONTROL)
+		*srv_mask |= PCIE_PORT_SERVICE_PME;
+	if (request & OSC_PCI_EXPRESS_AER_CONTROL)
+		*srv_mask |= PCIE_PORT_SERVICE_AER;
+
+	status = acpi_pci_osc_control_set(handle, request);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&port->dev, "ACPI _OSC request failed (code %d)\n",
+			status);
+		return -ENODEV;
+	}
+
+	dev_info(&port->dev, "ACPI _OSC control granted for 0x%02x\n", request);
+
+	return 0;
+}
Index: linux-2.6.35-rc6/drivers/pci/pcie/portdrv_core.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/portdrv_core.c
@@ -14,6 +14,7 @@
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/pcieport_if.h>
+#include <linux/aer.h>
 
 #include "../pci.h"
 #include "portdrv.h"
@@ -236,24 +237,40 @@ static int get_port_device_capability(st
 	int services = 0, pos;
 	u16 reg16;
 	u32 reg32;
+	int cap_mask;
+	int err;
+
+	err = pcie_port_platform_notify(dev, &cap_mask);
+	if (pcie_ports_auto) {
+		if (err)
+			return 0;
+	} else {
+		cap_mask = PCIE_PORT_SERVICE_MASK;
+	}
 
 	pos = pci_pcie_cap(dev);
 	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
 	/* Hot-Plug Capable */
-	if (reg16 & PCI_EXP_FLAGS_SLOT) {
+	if ((cap_mask & PCIE_PORT_SERVICE_HP) && (reg16 & PCI_EXP_FLAGS_SLOT)) {
 		pci_read_config_dword(dev, pos + PCI_EXP_SLTCAP, &reg32);
 		if (reg32 & PCI_EXP_SLTCAP_HPC)
 			services |= PCIE_PORT_SERVICE_HP;
 	}
 	/* AER capable */
-	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
+	if ((cap_mask & PCIE_PORT_SERVICE_AER)
+	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR)) {
 		services |= PCIE_PORT_SERVICE_AER;
+		pci_disable_pcie_error_reporting(dev);
+	}
 	/* VC support */
 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
 		services |= PCIE_PORT_SERVICE_VC;
 	/* Root ports are capable of generating PME too */
-	if (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
+	if ((cap_mask & PCIE_PORT_SERVICE_PME)
+	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
 		services |= PCIE_PORT_SERVICE_PME;
+		pcie_pme_interrupt_enable(dev, false);
+	}
 
 	return services;
 }
@@ -494,6 +511,9 @@ static void pcie_port_shutdown_service(s
  */
 int pcie_port_service_register(struct pcie_port_service_driver *new)
 {
+	if (pcie_ports_disabled)
+		return -ENODEV;
+
 	new->driver.name = (char *)new->name;
 	new->driver.bus = &pcie_port_bus_type;
 	new->driver.probe = pcie_port_probe_service;
Index: linux-2.6.35-rc6/drivers/pci/pcie/portdrv_pci.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/portdrv_pci.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/portdrv_pci.c
@@ -29,6 +29,32 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
+/* If this switch is set, PCIe port native services should not be enabled. */
+bool pcie_ports_disabled;
+
+/*
+ * If this switch is set, ACPI _OSC will be used to determine whether or not to
+ * enable PCIe port native services.
+ */
+bool pcie_ports_auto = true;
+
+static int __init pcie_port_setup(char *str)
+{
+	if (!strncmp(str, "auto", 4)) {
+		pcie_ports_disabled = false;
+		pcie_ports_auto = true;
+	} else if (!strncmp(str, "native", 6)) {
+		pcie_ports_disabled = false;
+		pcie_ports_auto = false;
+	} else if (!strncmp(str, "compat", 6)) {
+		pcie_ports_disabled = true;
+		pcie_ports_auto = false;
+	}
+
+	return 1;
+}
+__setup("pcie_ports=", pcie_port_setup);
+
 /* global data */
 
 static int pcie_portdrv_restore_config(struct pci_dev *dev)
@@ -82,6 +108,7 @@ static int __devinit pcie_portdrv_probe(
 		dev_warn(&dev->dev, "device [%04x:%04x] has invalid IRQ; "
 			 "check vendor BIOS\n", dev->vendor, dev->device);
 	}
+
 	status = pcie_port_device_register(dev);
 	if (status)
 		return status;
@@ -301,6 +328,9 @@ static int __init pcie_portdrv_init(void
 {
 	int retval;
 
+	if (pcie_ports_disabled)
+		return -EACCES;
+
 	dmi_check_system(pcie_portdrv_dmi_table);
 
 	retval = pcie_port_bus_register();
Index: linux-2.6.35-rc6/include/linux/pcieport_if.h
===================================================================
--- linux-2.6.35-rc6.orig/include/linux/pcieport_if.h
+++ linux-2.6.35-rc6/include/linux/pcieport_if.h
@@ -22,6 +22,11 @@
 #define PCIE_PORT_SERVICE_VC_SHIFT	3	/* Virtual Channel */
 #define PCIE_PORT_SERVICE_VC		(1 << PCIE_PORT_SERVICE_VC_SHIFT)
 
+#define PCIE_PORT_SERVICE_MASK		(PCIE_PORT_SERVICE_PME \
+					| PCIE_PORT_SERVICE_AER \
+					| PCIE_PORT_SERVICE_HP \
+					| PCIE_PORT_SERVICE_VC)
+
 struct pcie_device {
 	int 		irq;	    /* Service IRQ/MSI/MSI-X Vector */
 	struct pci_dev *port;	    /* Root/Upstream/Downstream Port */
Index: linux-2.6.35-rc6/include/linux/acpi.h
===================================================================
--- linux-2.6.35-rc6.orig/include/linux/acpi.h
+++ linux-2.6.35-rc6/include/linux/acpi.h
@@ -306,6 +306,8 @@ acpi_status acpi_run_osc(acpi_handle han
 				OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)
 
 extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags);
+extern acpi_status acpi_pci_osc_control_query(acpi_handle handle,
+					      u32 flags, u32 *result);
 extern void acpi_early_init(void);
 
 #else	/* !CONFIG_ACPI */

[PATCH 5/6] PCI: portdrv: disable native hot-plug interrupt

[Kenji Kaneshige]

Some BIOS enable PCIe native hot-plug interrupt on _OSC evaluation.
But it should not be enabled until hot-plug service driver is loaded,
otherwise something wrong (ex. interrupt storm) can happen. So disable
hot-plug interrupt at port service probe time.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/pcie/portdrv_core.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Index: linux-2.6.35-rc6/drivers/pci/pcie/portdrv_core.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/portdrv_core.c
@@ -253,8 +253,17 @@ static int get_port_device_capability(st
 	/* Hot-Plug Capable */
 	if ((cap_mask & PCIE_PORT_SERVICE_HP) && (reg16 & PCI_EXP_FLAGS_SLOT)) {
 		pci_read_config_dword(dev, pos + PCI_EXP_SLTCAP, &reg32);
-		if (reg32 & PCI_EXP_SLTCAP_HPC)
+		if (reg32 & PCI_EXP_SLTCAP_HPC) {
 			services |= PCIE_PORT_SERVICE_HP;
+			/*
+			 * Disable hot-plug interrupts in case the hot-plug
+			 * service driver is not loaded.
+			 */
+			pos += PCI_EXP_SLTCTL;
+			pci_read_config_word(dev, pos, &reg16);
+			reg16 &= ~(PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
+			pci_write_config_word(dev, pos, reg16);
+		}
 	}
 	/* AER capable */
 	if ((cap_mask & PCIE_PORT_SERVICE_AER)

[PATCH 6/6] PCI: portdrv: remove module_exit

[Kenji Kaneshige]

Remove unnecessary pcie_portdrv_exit().

The PCIe port bus driver is built-in kernel module. So the module will
never be unloaded.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/pcie/portdrv_pci.c |    7 -------
 1 file changed, 7 deletions(-)

Index: linux-2.6.35-rc6/drivers/pci/pcie/portdrv_pci.c
===================================================================
--- linux-2.6.35-rc6.orig/drivers/pci/pcie/portdrv_pci.c
+++ linux-2.6.35-rc6/drivers/pci/pcie/portdrv_pci.c
@@ -345,11 +345,4 @@ static int __init pcie_portdrv_init(void
 	return retval;
 }
 
-static void __exit pcie_portdrv_exit(void)
-{
-	pci_unregister_driver(&pcie_portdriver);
-	pcie_port_bus_unregister();
-}
-
 module_init(pcie_portdrv_init);
-module_exit(pcie_portdrv_exit);

Re: [PATCH 4/6] ACPI/PCI: ask bios for control of all native services at once

[Hidetoshi Seto]

Hi,

I have a concern.

(2010/07/30 15:23), Kenji Kaneshige wrote:
(snip)
> +/**
> + * pcie_port_acpi_setup - Request the BIOS to release control of PCIe services.
> + * @port: PCIe Port service for a root port or event collector.
> + * @srv_mask: Bit mask of services that can be enabled for @port.
> + *
> + * Invoked when @port is identified as a PCIe port device.  To avoid conflicts
> + * with the BIOS PCIe port native services support requires the BIOS to yield
> + * control of these services to the kernel.  The mask of services that the BIOS
> + * allows to be enabled for @port is written to @srv_mask.
> + *
> + * NOTE: It turns out that we cannot do that for individual port services
> + * separately, because that would make some systems work incorrectly.
> + */
> +int pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
> +{
> +	acpi_status status;
> +	acpi_handle handle;
> +	u32 flags, request;
> +
> +	if (acpi_pci_disabled)
> +		return 0;
> +
> +	handle = acpi_find_root_bridge_handle(port);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	request = (OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL |
> +		   OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
> +		   OSC_PCI_EXPRESS_PME_CONTROL |
> +		   OSC_PCI_EXPRESS_AER_CONTROL);
> +
> +	if (pcie_aer_get_firmware_first(port)) {
> +		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
> +		request &= ~OSC_PCI_EXPRESS_AER_CONTROL;
> +	}

We should drop OSC_PCI_EXPRESS_AER_CONTROL from request when AER is not
configured, and also when AER is disabled for some reason (e.g. pci=noaer).
Otherwise no one own AER errors, it will mean that unhandled error might
cause unexpected system behavior.

> +
> +	status = acpi_pci_osc_control_query(handle, request, &flags);
> +	if (ACPI_FAILURE(status)) {
> +		dev_dbg(&port->dev, "ACPI _OSC query failed (code %d)\n",
> +			status);
> +		return -ENODEV;
> +	}
> +
> +	if (!(flags & OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)) {
> +		dev_dbg(&port->dev, "BIOS refuses to grant control of PCIe "
> +			"Capability Structure\n");
> +		return -EACCES;
> +	}
> +
> +	request &= flags;
> +	*srv_mask = PCIE_PORT_SERVICE_VC;
> +	if (request & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
> +		*srv_mask |= PCIE_PORT_SERVICE_HP;
> +	if (request & OSC_PCI_EXPRESS_PME_CONTROL)
> +		*srv_mask |= PCIE_PORT_SERVICE_PME;
> +	if (request & OSC_PCI_EXPRESS_AER_CONTROL)
> +		*srv_mask |= PCIE_PORT_SERVICE_AER;
> +
> +	status = acpi_pci_osc_control_set(handle, request);
> +	if (ACPI_FAILURE(status)) {
> +		dev_dbg(&port->dev, "ACPI _OSC request failed (code %d)\n",
> +			status);
> +		return -ENODEV;
> +	}
> +
> +	dev_info(&port->dev, "ACPI _OSC control granted for 0x%02x\n", request);
> +
> +	return 0;
> +}

I'll post a fix for this soon.


Thanks,
H.Seto

[PATCH] portdrv: Don't take control of AER if not required

[Hidetoshi Seto]

OS cannot handle AER when:
 - !CONFIG_PCIEAER
 - option pci=noaer is specified
 - MSIs are globally disabled (by quirk for the chipset etc.)

In such cases don't request AER control via _OSC.

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
---
 drivers/pci/pci.h               |    2 ++
 drivers/pci/pcie/aer/aerdrv.c   |    9 ++++++---
 drivers/pci/pcie/portdrv_acpi.c |    5 ++++-
 drivers/pci/pcie/portdrv_core.c |    2 ++
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f8077b3..d09d4f3 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -130,8 +130,10 @@ static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
 
 #ifdef CONFIG_PCIEAER
 void pci_no_aer(void);
+bool pci_aer_available(void);
 #else
 static inline void pci_no_aer(void) { }
+static inline bool pci_aer_available(void) { return false; }
 #endif
 
 static inline int pci_no_d1d2(struct pci_dev *dev)
diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index 484cc55..4fbec91 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -67,6 +67,11 @@ static struct pcie_port_service_driver aerdriver = {
 
 static int pcie_aer_disable;
 
+bool pci_aer_available(void)
+{
+	return !pcie_aer_disable && pci_msi_enabled();
+}
+
 void pci_no_aer(void)
 {
 	pcie_aer_disable = 1;	/* has priority over 'forceload' */
@@ -411,9 +416,7 @@ static void aer_error_resume(struct pci_dev *dev)
  */
 static int __init aer_service_init(void)
 {
-	if (pcie_aer_disable)
-		return -ENXIO;
-	if (!pci_msi_enabled())
+	if (!pci_aer_available())
 		return -ENXIO;
 	return pcie_port_service_register(&aerdriver);
 }
diff --git a/drivers/pci/pcie/portdrv_acpi.c b/drivers/pci/pcie/portdrv_acpi.c
index a957de2..c8814b9 100644
--- a/drivers/pci/pcie/portdrv_acpi.c
+++ b/drivers/pci/pcie/portdrv_acpi.c
@@ -16,6 +16,7 @@
 #include <linux/pcieport_if.h>
 
 #include "aer/aerdrv.h"
+#include "../pci.h"
 
 /**
  * pcie_port_acpi_setup - Request the BIOS to release control of PCIe services.
@@ -48,7 +49,9 @@ int pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
 		   OSC_PCI_EXPRESS_PME_CONTROL |
 		   OSC_PCI_EXPRESS_AER_CONTROL);
 
-	if (pcie_aer_get_firmware_first(port)) {
+	if (!pci_aer_available()) {
+		request &= ~OSC_PCI_EXPRESS_AER_CONTROL;
+	} else if (pcie_aer_get_firmware_first(port)) {
 		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
 		request &= ~OSC_PCI_EXPRESS_AER_CONTROL;
 	}
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 8e96bd4..8e26025 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -246,6 +246,8 @@ static int get_port_device_capability(struct pci_dev *dev)
 			return 0;
 	} else {
 		cap_mask = PCIE_PORT_SERVICE_MASK;
+		if (!pci_aer_available())
+			cap_mask &= ~PCIE_PORT_SERVICE_AER;
 	}
 
 	pos = pci_pcie_cap(dev);
-- 
1.7.2

Re: [PATCH] PCI / PCIe: Ask BIOS for control of all native services at once (v4)

[Rafael J. Wysocki]

On Friday, July 30, 2010, Kenji Kaneshige wrote:
> (2010/07/30 0:45), Rafael J. Wysocki wrote:
> > On Thursday, July 29, 2010, Kenji Kaneshige wrote:
> >> (2010/07/29 6:43), Jesse Barnes wrote:
> >>> On Wed, 28 Jul 2010 23:23:56 +0200
> >>> "Rafael J. Wysocki"<rjw@sisk.pl>   wrote:
> >>>
> >>>> From: Rafael J. Wysocki<rjw@sisk.pl>
> >>>>
> >>>> PCIe port service drivers ask the BIOS, through _OSC, for control of
> >>>> the services they handle.  Unfortunately, each of them individually
> >>>> asks for control of the PCIe capability structure and if that is
> >>>> granted, some BIOSes expect that the other PCIe port services will be
> >>>> configured and handled by the kernel as well.  If that is not the
> >>>> case (eg. one of the PCIe port service drivers is not loaded), the
> >>>> BIOS may be confused and may cause the system as a whole to misbehave
> >>>> (eg. on one of such systems enabling the native PCIe PME service
> >>>> without loading the native PCIe hot-plug service driver causes a
> >>>> storm of ACPI notify requests to appear).
> >>>>
> >>>> For this reason rework the PCIe port driver so that (1) it checks
> >>>> which native PCIe port services can be enabled, according to the
> >>>> BIOS, and (2) it requests control of all these services
> >>>> simultaneously.  In particular, this causes pcie_portdrv_probe() to
> >>>> fail if the BIOS refuses to grant control of the PCIe capability
> >>>> structure, which means that no native PCIe port services can be
> >>>> enabled for the PCIe root complex the given port belongs to.
> >>>>
> >>>> Make it possible to override this behavior using a new command line
> >>>> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
> >>>> default), 'native' (use the PCIe native services regardless of the
> >>>> BIOS response to the control request), or 'compat' (do not use the
> >>>> PCIe native services at all).
> >>>>
> >>>> Accordingly, rework the existing PCIe port service drivers so that
> >>>> they don't request control of the services directly.
> >>>>
> >>>> Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl>
> >>>> ---
> >>>
> >>> Kenji-san, are you ok with this version?  I would like to get your ack
> >>> (and ideally your tested-by) for this one since it affects
> >>> functionality you need.
> >>>
> >>
> >> Hi Jesse, Rafael,
> >>
> >> I've just started reviewing the latest version of the patch. I have good
> >> impression about this version so far. Please give me a few days for deeper
> >> review. And I've booked the test machine for this patch in next week. I'll
> >> try to test the patch and send you the result as soon as I can.
> >>
> >> Here are two comments I have so far.
> >>
> >> Though it is not directly related to Rafael's patch, I found one problem
> >> in _OSC query handling while reviewing this patch. Currently, all the
> >> _OSC controls are queried at the same time in acpi_pci_osc_support() and
> >> the result is preserved for later acpi_pci_osc_control_set() call. But
> >> query result can vary depending on the combination of requested controls.
> >
> > In principle it can, but acpi_pci_query_osc() asks for all aplicable control
> > bits, in which case the formware can only clear those bits in the response if
> > the corresponding features are unsupported.  Doing otherwise would be in
> > violation of Section 6.2.9.1. of ACPI spec 3.0b, the following in particular:
> >
> > "If any bits in the Control Field are returned cleared (masked to zero) by the _OSC control method,
> > the respective feature is designated unsupported by the platform and must not be enabled by the OS."
> >
> 
> My concern is as follows.
> 
> For easy, assume as follows:
> - all the _OSC controls are A, B, C
> -B depends on C.
> - When requesting A, B and C, all A, B and C are granted to OS.
> - When requesting A, B, only A is granted to OS.

That's why we should always ask for A, B, C at the same time.

> Current linux queries all A, B and C at the boot time and preserve the
> result (all A, B and C can be granted to OS). Here, if a service driver
> requests A and B using acpi_pci_osc_control_set(),

Service drivers should not use acpi_pci_osc_control_set().  That's why the
current code is wrong.

> acpi_pci_osc_control_set() checks if both of those controls can be granted
> to OS. The expected result here is only A can be granted to OS, but current
> linux judges both of them can be granted because linux does this check by
> seeing preserved result.
> As a result, acpi_pci_osc_control_set() evaluates _OSC with query flag
> cleared to request A and B. Since the control B returned cleared,
> acpi_pci_osc_control() returns error to the service driver. As a result,
> control A is granted to OS even though there is no driver to handle the
> control A.

That indeed is a bug in the current code.  With the $subject patch, though
(without your modifications), that will never happen.

> >> So I think query result must not be preserved. Since Rafael's patch uses
> >> query result, this problem should be fixed at the same time. I'll try to
> >> make a patch for this and update Rafael's patch if needed.
> >
> > It would be sufficient to change acpi_pci_osc_control_get(), introduced
> > by my patch, to call acpi_pci_query_osc() unconditionally, but as I said above,
> > I don't think it's necessary.
> 
> I made a patch for this and updated your patch slightly. I'll send them soon.
> 
> >
> >> I think the following changes should be done in the separated patch. I
> >> guess this change is for the BIOS which enables hotplug interrupt at when
> >> _OSC is evaluated with native hot-plug control. Right?
> >
> > Yes, it is.
> >
> >> Anyway, it should be separated patch with appropriate description.
> >
> > But the $subject patch does analogous things for AER and PCIe PME.  Do you
> > think they also should be introduced by separate patches?
> 
> I think the following changes in your patch should be introduced by
> separate patches.
> 
>   - optimizing the checks in acpi_pci_osc_control_set()

I don't think that is necessary.

>   - Disabling hot-plug interrupt

OK, but what exactly is the difference between that and disabling the PME
interrupt?  It is done for the same reason.

>   - Removing module_exit()

OK

Rafael

Re: [PATCH 1/6] ACPI/PCI: cleanup acpi_pci_run_osc

[Rafael J. Wysocki]

On Friday, July 30, 2010, Kenji Kaneshige wrote:
> Make capabilitis buffer in acpi_pci_run_osc() instead of caller of
> this function. This makes the code a little cleaner. This has no
> functional changes.
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

Well, I'm not sure if that really is an improvement.  With the patch
acpi_pci_run_osc() simply has more arguments that doesn't really
make it more readable IMHO.

> ---
>  drivers/acpi/pci_root.c |   34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
> +++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
> @@ -206,9 +206,10 @@ static void acpi_pci_bridge_scan(struct 
>  
>  static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
>  
> -static acpi_status acpi_pci_run_osc(acpi_handle handle,
> -				    const u32 *capbuf, u32 *retval)
> +static acpi_status acpi_pci_run_osc(acpi_handle handle, u32 support,
> +				    u32 control, bool query, u32 *retval)
>  {
> +	u32 capbuf[3];
>  	struct acpi_osc_context context = {
>  		.uuid_str = pci_osc_uuid_str,
>  		.rev = 1,
> @@ -217,6 +218,10 @@ static acpi_status acpi_pci_run_osc(acpi
>  	};
>  	acpi_status status;
>  
> +	capbuf[OSC_QUERY_TYPE] = (query == true) ? OSC_QUERY_ENABLE : 0;
> +	capbuf[OSC_SUPPORT_TYPE] = support;
> +	capbuf[OSC_CONTROL_TYPE] = control;
> +
>  	status = acpi_run_osc(handle, &context);
>  	if (ACPI_SUCCESS(status)) {
>  		*retval = *((u32 *)(context.ret.pointer + 8));
> @@ -228,17 +233,16 @@ static acpi_status acpi_pci_run_osc(acpi
>  static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 flags)
>  {
>  	acpi_status status;
> -	u32 support_set, result, capbuf[3];
> +	u32 result;
>  
>  	/* do _OSC query for all possible controls */
> -	support_set = root->osc_support_set | (flags & OSC_PCI_SUPPORT_MASKS);
> -	capbuf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
> -	capbuf[OSC_SUPPORT_TYPE] = support_set;
> -	capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
> -
> -	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
> +	flags &= OSC_PCI_SUPPORT_MASKS;
> +	status = acpi_pci_run_osc(root->device->handle,
> +				  root->osc_support_set | flags,
> +				  OSC_PCI_CONTROL_MASKS,
> +				  true, &result);
>  	if (ACPI_SUCCESS(status)) {
> -		root->osc_support_set = support_set;
> +		root->osc_support_set |= flags;

To me, the old code is cleaner.  I would do

+	flags &= OSC_PCI_SUPPORT_MASKS;
+	flags |= root->osc_support_set;
+	status = acpi_pci_run_osc(root->device->handle,
+				  flags,
+				  OSC_PCI_CONTROL_MASKS,
+				  true, &result);
 	if (ACPI_SUCCESS(status)) {
-		root->osc_support_set = support_set;
+		root->osc_support_set = flags;

but that's almost the same as the old code.

>  		root->osc_control_qry = result;
>  		root->osc_queried = 1;
>  	}
> @@ -373,7 +377,7 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
>  acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags)
>  {
>  	acpi_status status;
> -	u32 control_req, result, capbuf[3];
> +	u32 control_req, result;
>  	acpi_handle tmp;
>  	struct acpi_pci_root *root;
>  
> @@ -407,10 +411,10 @@ acpi_status acpi_pci_osc_control_set(acp
>  		goto out;
>  	}
>  
> -	capbuf[OSC_QUERY_TYPE] = 0;
> -	capbuf[OSC_SUPPORT_TYPE] = root->osc_support_set;
> -	capbuf[OSC_CONTROL_TYPE] = root->osc_control_set | control_req;
> -	status = acpi_pci_run_osc(handle, capbuf, &result);
> +	status = acpi_pci_run_osc(handle,
> +				  root->osc_support_set,
> +				  root->osc_control_set | control_req,
> +				  false, &result);
>  	if (ACPI_SUCCESS(status))
>  		root->osc_control_set = result;
>  out:

Overall, I don't like this change, sorry.

Rafael

Re: [PATCH 2/6] ACPI/PCI: do not preserve query result

[Rafael J. Wysocki]

On Friday, July 30, 2010, Kenji Kaneshige wrote:
> Currently, all the _OSC controls are queried at the same time in
> acpi_pci_osc_support() and the result is preserved for later
> acpi_pci_osc_control_set() call. But query result can vary depending
> on the combination of requested controls. Therefore, query result must
> not be preserved.
> 
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

IMHO it would be _much_ simpler to change the definition of
acpi_pci_osc_control_get() in my original patch so that it executes
acp_pci_query_osc() unconditionally.

Rafael


> ---
>  drivers/acpi/pci_root.c |   49 ++++++++++++++++++++++++++----------------------
>  include/acpi/acpi_bus.h |    3 --
>  2 files changed, 27 insertions(+), 25 deletions(-)
> 
> Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
> +++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
> @@ -230,35 +230,42 @@ static acpi_status acpi_pci_run_osc(acpi
>  	return status;
>  }
>  
> -static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 flags)
> +static acpi_status acpi_pci_osc_control_query(struct acpi_pci_root *root,
> +					      u32 flags, u32 *result)
>  {
> -	acpi_status status;
> -	u32 result;
> -
> -	/* do _OSC query for all possible controls */
> -	flags &= OSC_PCI_SUPPORT_MASKS;
> -	status = acpi_pci_run_osc(root->device->handle,
> -				  root->osc_support_set | flags,
> -				  OSC_PCI_CONTROL_MASKS,
> -				  true, &result);
> -	if (ACPI_SUCCESS(status)) {
> -		root->osc_support_set |= flags;
> -		root->osc_control_qry = result;
> -		root->osc_queried = 1;
> +	/* No need to run _OSC if requested control bits are already granted */
> +	flags &= OSC_PCI_CONTROL_MASKS;
> +	if ((root->osc_control_set & flags) == flags) {
> +		*result = root->osc_control_set;
> +		return AE_OK;
>  	}
> -	return status;
> +	return acpi_pci_run_osc(root->device->handle,
> +				root->osc_support_set,
> +				root->osc_control_set | flags,
> +				true, result);
>  }
>  
>  static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
>  {
>  	acpi_status status;
>  	acpi_handle tmp;
> +	u32 result;
>  
>  	status = acpi_get_handle(root->device->handle, "_OSC", &tmp);
>  	if (ACPI_FAILURE(status))
>  		return status;
>  	mutex_lock(&osc_lock);
> -	status = acpi_pci_query_osc(root, flags);
> +	/* No need to run _OSC if requested support bits are already set */
> +	flags &= OSC_PCI_SUPPORT_MASKS;
> +	if ((root->osc_support_set & flags) == flags)
> +		goto out;
> +	status = acpi_pci_run_osc(root->device->handle,
> +				  root->osc_support_set | flags,
> +				  root->osc_control_set,
> +				  true, &result);
> +	if (ACPI_SUCCESS(status))
> +		root->osc_support_set |= flags;
> +out:
>  	mutex_unlock(&osc_lock);
>  	return status;
>  }
> @@ -399,12 +406,10 @@ acpi_status acpi_pci_osc_control_set(acp
>  		goto out;
>  
>  	/* Need to query controls first before requesting them */
> -	if (!root->osc_queried) {
> -		status = acpi_pci_query_osc(root, root->osc_support_set);
> -		if (ACPI_FAILURE(status))
> -			goto out;
> -	}
> -	if ((root->osc_control_qry & control_req) != control_req) {
> +	status = acpi_pci_osc_control_query(root, control_req, &result);
> +	if (ACPI_FAILURE(status))
> +		goto out;
> +	if ((result & control_req) != control_req) {
>  		printk(KERN_DEBUG
>  		       "Firmware did not grant requested _OSC control\n");
>  		status = AE_SUPPORT;
> Index: linux-2.6.35-rc6/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-2.6.35-rc6.orig/include/acpi/acpi_bus.h
> +++ linux-2.6.35-rc6/include/acpi/acpi_bus.h
> @@ -377,9 +377,6 @@ struct acpi_pci_root {
>  
>  	u32 osc_support_set;	/* _OSC state of support bits */
>  	u32 osc_control_set;	/* _OSC state of control bits */
> -	u32 osc_control_qry;	/* the latest _OSC query result */
> -
> -	u32 osc_queried:1;	/* has _OSC control been queried? */
>  };
>  
>  /* helper */
> 
> 
> 

Re: [PATCH 3/6] ACPI/PCI: optimize checks in acpi_pci_osc_control_set()

[Rafael J. Wysocki]

On Friday, July 30, 2010, Kenji Kaneshige wrote:
> Check for ACPI _OSC object is heavier than other checks in
> acpi_pci_osc_control_set(). So move it after the other checks.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

OK

Rafael

> ---
>  drivers/acpi/pci_root.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6.35-rc6/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-2.6.35-rc6.orig/drivers/acpi/pci_root.c
> +++ linux-2.6.35-rc6/drivers/acpi/pci_root.c
> @@ -388,10 +388,6 @@ acpi_status acpi_pci_osc_control_set(acp
>  	acpi_handle tmp;
>  	struct acpi_pci_root *root;
>  
> -	status = acpi_get_handle(handle, "_OSC", &tmp);
> -	if (ACPI_FAILURE(status))
> -		return status;
> -
>  	control_req = (flags & OSC_PCI_CONTROL_MASKS);
>  	if (!control_req)
>  		return AE_TYPE;
> @@ -400,6 +396,10 @@ acpi_status acpi_pci_osc_control_set(acp
>  	if (!root)
>  		return AE_NOT_EXIST;
>  
> +	status = acpi_get_handle(handle, "_OSC", &tmp);
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
>  	mutex_lock(&osc_lock);
>  	/* No need to evaluate _OSC if the control was already granted. */
>  	if ((root->osc_control_set & control_req) == control_req)
> 
> 
> 

Re: [PATCH 4/6] ACPI/PCI: ask bios for control of all native services at once

[Rafael J. Wysocki]

On Friday, July 30, 2010, Kenji Kaneshige wrote:
> PCIe port service drivers ask the BIOS, through _OSC, for control of
> the services they handle.  Unfortunately, each of them individually
> asks for control of the PCIe capability structure and if that is
> granted, some BIOSes expect that the other PCIe port services will be
> configured and handled by the kernel as well.  If that is not the
> case (eg. one of the PCIe port service drivers is not loaded), the
> BIOS may be confused and may cause the system as a whole to misbehave
> (eg. on one of such systems enabling the native PCIe PME service
> without loading the native PCIe hot-plug service driver causes a
> storm of ACPI notify requests to appear).
> 
> For this reason rework the PCIe port driver so that (1) it checks
> which native PCIe port services can be enabled, according to the
> BIOS, and (2) it requests control of all these services
> simultaneously.  In particular, this causes pcie_portdrv_probe() to
> fail if the BIOS refuses to grant control of the PCIe capability
> structure, which means that no native PCIe port services can be
> enabled for the PCIe root complex the given port belongs to.
> 
> Make it possible to override this behavior using a new command line
> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the
> default), 'native' (use the PCIe native services regardless of the
> BIOS response to the control request), or 'compat' (do not use the
> PCIe native services at all).
> 
> Accordingly, rework the existing PCIe port service drivers so that
> they don't request control of the services directly.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> Tested-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

OK, I think I see what you want it to look like.

I'll send my new version later today.

Thanks,
Rafael