[PATCH 0/7] ACPI / PCI / PCIe: Rework _OSC handling

[PATCH 0/7] ACPI / PCI / PCIe: Rework _OSC handling

[Rafael J. Wysocki]

Hi,

The following patchset is based on
https://patchwork.kernel.org/patch/114917/ and the comments I received on that
patch.  Hopefully, I took all of the comments into account.

[1/7] - Remove the last argument of acpi_pci_osc_run(); this is a cleanup, but
        IMO it makes sense.

[2/7] - Introduce acpi_pci_root_osc_query() allowing the caller to get a
        mask of _OSC control bits the BIOS allows the kernel to control
        for a given PCI root bridge.

[3/7] - Rework the PCIe port driver to request _OSC control for all serives at
        once.

[4/7] - Disable PCIe port services (that might be enabled by the BIOS) during
        initialization.

[5/7] - Remove the PCIe port driver modules exit function.

[6/7] - Rework acpi_pci_osc_control_set() so that it doesn't use cached
        result of a query and remove the fields of struct acpi_pci_root that
       aren't used any more.

[7/7] - Reorder checks in acpi_pci_osc_control_set()

Comments (and testing :-)) welcome.

Thanks,
Rafael

[PATCH 1/7] ACPI / PCI: Make acpi_pci_run_osc() use capbuf to return the result

[Rafael J. Wysocki]

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

There is no reason why acpi_pci_run_osc() should use a separate
argument to return the _OSC result, since there is a place for it
in the buffer passed via capbuf, as none of the callers needs to
preserve the original contents of that buffer, so drop the third
argument of acpi_pci_run_osc() and modify the callers accordingly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/pci_root.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

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
@@ -206,8 +206,7 @@ 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 *capbuf)
 {
 	struct acpi_osc_context context = {
 		.uuid_str = pci_osc_uuid_str,
@@ -219,7 +218,7 @@ static acpi_status acpi_pci_run_osc(acpi
 
 	status = acpi_run_osc(handle, &context);
 	if (ACPI_SUCCESS(status)) {
-		*retval = *((u32 *)(context.ret.pointer + 8));
+		capbuf[OSC_CONTROL_TYPE] = ((u32 *)context.ret.pointer)[2];
 		kfree(context.ret.pointer);
 	}
 	return status;
@@ -228,7 +227,7 @@ 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 support_set, capbuf[3];
 
 	/* do _OSC query for all possible controls */
 	support_set = root->osc_support_set | (flags & OSC_PCI_SUPPORT_MASKS);
@@ -236,10 +235,10 @@ static acpi_status acpi_pci_query_osc(st
 	capbuf[OSC_SUPPORT_TYPE] = support_set;
 	capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
 
-	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
+	status = acpi_pci_run_osc(root->device->handle, capbuf);
 	if (ACPI_SUCCESS(status)) {
 		root->osc_support_set = support_set;
-		root->osc_control_qry = result;
+		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
 		root->osc_queried = 1;
 	}
 	return status;
@@ -373,7 +372,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, capbuf[3];
 	acpi_handle tmp;
 	struct acpi_pci_root *root;
 
@@ -410,9 +409,9 @@ acpi_status acpi_pci_osc_control_set(acp
 	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, capbuf);
 	if (ACPI_SUCCESS(status))
-		root->osc_control_set = result;
+		root->osc_control_set = capbuf[OSC_CONTROL_TYPE];
 out:
 	mutex_unlock(&osc_lock);
 	return status;

Re: [PATCH 1/7] ACPI / PCI: Make acpi_pci_run_osc() use capbuf to return the result

[Hidetoshi Seto]

(2010/07/31 7:24), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> There is no reason why acpi_pci_run_osc() should use a separate
> argument to return the _OSC result, since there is a place for it
> in the buffer passed via capbuf, as none of the callers needs to
> preserve the original contents of that buffer, so drop the third
> argument of acpi_pci_run_osc() and modify the callers accordingly.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---

I think this change is not significant for later patches.
Can we skip?

Thanks,
H.Seto

>  drivers/acpi/pci_root.c |   17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> 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
> @@ -206,8 +206,7 @@ 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 *capbuf)
>  {
>  	struct acpi_osc_context context = {
>  		.uuid_str = pci_osc_uuid_str,
> @@ -219,7 +218,7 @@ static acpi_status acpi_pci_run_osc(acpi
>  
>  	status = acpi_run_osc(handle, &context);
>  	if (ACPI_SUCCESS(status)) {
> -		*retval = *((u32 *)(context.ret.pointer + 8));
> +		capbuf[OSC_CONTROL_TYPE] = ((u32 *)context.ret.pointer)[2];
>  		kfree(context.ret.pointer);
>  	}
>  	return status;
> @@ -228,7 +227,7 @@ 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 support_set, capbuf[3];
>  
>  	/* do _OSC query for all possible controls */
>  	support_set = root->osc_support_set | (flags & OSC_PCI_SUPPORT_MASKS);
> @@ -236,10 +235,10 @@ static acpi_status acpi_pci_query_osc(st
>  	capbuf[OSC_SUPPORT_TYPE] = support_set;
>  	capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
>  
> -	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
> +	status = acpi_pci_run_osc(root->device->handle, capbuf);
>  	if (ACPI_SUCCESS(status)) {
>  		root->osc_support_set = support_set;
> -		root->osc_control_qry = result;
> +		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
>  		root->osc_queried = 1;
>  	}
>  	return status;
> @@ -373,7 +372,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, capbuf[3];
>  	acpi_handle tmp;
>  	struct acpi_pci_root *root;
>  
> @@ -410,9 +409,9 @@ acpi_status acpi_pci_osc_control_set(acp
>  	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, capbuf);
>  	if (ACPI_SUCCESS(status))
> -		root->osc_control_set = result;
> +		root->osc_control_set = capbuf[OSC_CONTROL_TYPE];
>  out:
>  	mutex_unlock(&osc_lock);
>  	return status;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 1/7] ACPI / PCI: Make acpi_pci_run_osc() use capbuf to return the result

[Rafael J. Wysocki]

On Monday, August 02, 2010, Hidetoshi Seto wrote:
> (2010/07/31 7:24), Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > There is no reason why acpi_pci_run_osc() should use a separate
> > argument to return the _OSC result, since there is a place for it
> > in the buffer passed via capbuf, as none of the callers needs to
> > preserve the original contents of that buffer, so drop the third
> > argument of acpi_pci_run_osc() and modify the callers accordingly.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> 
> I think this change is not significant for later patches.
> Can we skip?

Yes, we can, but that's going to make the next patches slightly less readable.

Thanks,
Rafael

[PATCH 2/7] ACPI / PCI: Introduce acpi_pci_root_osc_query()

[Rafael J. Wysocki]

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

Introduce a function allowing the caller to obtain a mask of _OSC
control bits the BIOS will allow the kernel to control for a given
PCI root bridge.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/pci_root.c |   56 +++++++++++++++++++++++++++++++++++++++++-------
 include/linux/acpi.h    |    1 
 2 files changed, 49 insertions(+), 8 deletions(-)

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
@@ -224,21 +224,32 @@ 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_query_osc(struct acpi_pci_root *root,
+					u32 support,
+					u32 *control)
 {
 	acpi_status status;
-	u32 support_set, capbuf[3];
+	u32 capbuf[3];
+
+	support &= OSC_PCI_SUPPORT_MASKS;
+	support |= root->osc_support_set;
 
 	/* 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;
+	capbuf[OSC_SUPPORT_TYPE] = support;
+	if (control) {
+		*control &= OSC_PCI_CONTROL_MASKS;
+		capbuf[OSC_CONTROL_TYPE] = *control;
+	} else {
+		capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
+	}
 
 	status = acpi_pci_run_osc(root->device->handle, capbuf);
 	if (ACPI_SUCCESS(status)) {
-		root->osc_support_set = support_set;
+		root->osc_support_set = support;
 		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
+		if (control)
+			*control = capbuf[OSC_CONTROL_TYPE];
 		root->osc_queried = 1;
 	}
 	return status;
@@ -253,7 +264,7 @@ static acpi_status acpi_pci_osc_support(
 	if (ACPI_FAILURE(status))
 		return status;
 	mutex_lock(&osc_lock);
-	status = acpi_pci_query_osc(root, flags);
+	status = acpi_pci_query_osc(root, flags, NULL);
 	mutex_unlock(&osc_lock);
 	return status;
 }
@@ -362,6 +373,35 @@ out:
 }
 EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
 
+ /**
+ * acpi_pci_root_osc_query - Get the _OSC bits the kernel can control.
+ * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
+ * @ctrl_mask: Mask of _OSC bits to query and the place to put the result into.
+ **/
+acpi_status acpi_pci_root_osc_query(acpi_handle handle, u32 *ctrl_mask)
+{
+	acpi_status status;
+	acpi_handle tmp;
+	struct acpi_pci_root *root;
+
+	if (!ctrl_mask || !(*ctrl_mask & OSC_PCI_CONTROL_MASKS))
+		return AE_BAD_PARAMETER;
+
+	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_query_osc(root, root->osc_support_set, ctrl_mask);
+	mutex_unlock(&osc_lock);
+
+	return status;
+}
+
 /**
  * acpi_pci_osc_control_set - commit requested control to Firmware
  * @handle: acpi_handle for the target ACPI object
@@ -395,7 +435,7 @@ acpi_status acpi_pci_osc_control_set(acp
 
 	/* Need to query controls first before requesting them */
 	if (!root->osc_queried) {
-		status = acpi_pci_query_osc(root, root->osc_support_set);
+		status = acpi_pci_query_osc(root, root->osc_support_set, NULL);
 		if (ACPI_FAILURE(status))
 			goto out;
 	}
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_root_osc_query(acpi_handle handle, u32 *ctrl_mask);
 extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags);
 extern void acpi_early_init(void);
 

Re: [PATCH 2/7] ACPI / PCI: Introduce acpi_pci_root_osc_query()

[Hidetoshi Seto]

(2010/07/31 7:26), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Introduce a function allowing the caller to obtain a mask of _OSC
> control bits the BIOS will allow the kernel to control for a given
> PCI root bridge.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/pci_root.c |   56 +++++++++++++++++++++++++++++++++++++++++-------
>  include/linux/acpi.h    |    1 
>  2 files changed, 49 insertions(+), 8 deletions(-)
> 
> 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
> @@ -224,21 +224,32 @@ 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_query_osc(struct acpi_pci_root *root,
> +					u32 support,
> +					u32 *control)
>  {
>  	acpi_status status;
> -	u32 support_set, capbuf[3];
> +	u32 capbuf[3];
> +
> +	support &= OSC_PCI_SUPPORT_MASKS;
> +	support |= root->osc_support_set;
>  
>  	/* 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;
> +	capbuf[OSC_SUPPORT_TYPE] = support;
> +	if (control) {
> +		*control &= OSC_PCI_CONTROL_MASKS;
> +		capbuf[OSC_CONTROL_TYPE] = *control;
> +	} else {
> +		capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
> +	}
>  
>  	status = acpi_pci_run_osc(root->device->handle, capbuf);
>  	if (ACPI_SUCCESS(status)) {
> -		root->osc_support_set = support_set;
> +		root->osc_support_set = support;
>  		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
> +		if (control)
> +			*control = capbuf[OSC_CONTROL_TYPE];
>  		root->osc_queried = 1;
>  	}
>  	return status;
> @@ -253,7 +264,7 @@ static acpi_status acpi_pci_osc_support(
>  	if (ACPI_FAILURE(status))
>  		return status;
>  	mutex_lock(&osc_lock);
> -	status = acpi_pci_query_osc(root, flags);
> +	status = acpi_pci_query_osc(root, flags, NULL);
>  	mutex_unlock(&osc_lock);
>  	return status;
>  }
> @@ -362,6 +373,35 @@ out:
>  }
>  EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
>  
> + /**
> + * acpi_pci_root_osc_query - Get the _OSC bits the kernel can control.
> + * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
> + * @ctrl_mask: Mask of _OSC bits to query and the place to put the result into.
> + **/
> +acpi_status acpi_pci_root_osc_query(acpi_handle handle, u32 *ctrl_mask)
> +{
> +	acpi_status status;
> +	acpi_handle tmp;
> +	struct acpi_pci_root *root;
> +
> +	if (!ctrl_mask || !(*ctrl_mask & OSC_PCI_CONTROL_MASKS))
> +		return AE_BAD_PARAMETER;
> +
> +	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_query_osc(root, root->osc_support_set, ctrl_mask);
> +	mutex_unlock(&osc_lock);
> +
> +	return status;
> +}
> +
>  /**
>   * acpi_pci_osc_control_set - commit requested control to Firmware
>   * @handle: acpi_handle for the target ACPI object
> @@ -395,7 +435,7 @@ acpi_status acpi_pci_osc_control_set(acp
>  
>  	/* Need to query controls first before requesting them */
>  	if (!root->osc_queried) {
> -		status = acpi_pci_query_osc(root, root->osc_support_set);
> +		status = acpi_pci_query_osc(root, root->osc_support_set, NULL);
>  		if (ACPI_FAILURE(status))
>  			goto out;
>  	}
> 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_root_osc_query(acpi_handle handle, u32 *ctrl_mask);
>  extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags);
>  extern void acpi_early_init(void);

IMHO acpi_pci_osc_control_query() is better name for new function here,
to make it be paired with acpi_pci_osc_control_set(), and to avoid mistakable
"*_get" that sounds like it takes controls via _OSC.

Thanks,
H.Seto

Re: [PATCH 2/7] ACPI / PCI: Introduce acpi_pci_root_osc_query()

[Rafael J. Wysocki]

On Monday, August 02, 2010, Hidetoshi Seto wrote:
> (2010/07/31 7:26), Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Introduce a function allowing the caller to obtain a mask of _OSC
> > control bits the BIOS will allow the kernel to control for a given
> > PCI root bridge.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/acpi/pci_root.c |   56 +++++++++++++++++++++++++++++++++++++++++-------
> >  include/linux/acpi.h    |    1 
> >  2 files changed, 49 insertions(+), 8 deletions(-)
> > 
> > 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
> > @@ -224,21 +224,32 @@ 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_query_osc(struct acpi_pci_root *root,
> > +					u32 support,
> > +					u32 *control)
> >  {
> >  	acpi_status status;
> > -	u32 support_set, capbuf[3];
> > +	u32 capbuf[3];
> > +
> > +	support &= OSC_PCI_SUPPORT_MASKS;
> > +	support |= root->osc_support_set;
> >  
> >  	/* 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;
> > +	capbuf[OSC_SUPPORT_TYPE] = support;
> > +	if (control) {
> > +		*control &= OSC_PCI_CONTROL_MASKS;
> > +		capbuf[OSC_CONTROL_TYPE] = *control;
> > +	} else {
> > +		capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS;
> > +	}
> >  
> >  	status = acpi_pci_run_osc(root->device->handle, capbuf);
> >  	if (ACPI_SUCCESS(status)) {
> > -		root->osc_support_set = support_set;
> > +		root->osc_support_set = support;
> >  		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
> > +		if (control)
> > +			*control = capbuf[OSC_CONTROL_TYPE];
> >  		root->osc_queried = 1;
> >  	}
> >  	return status;
> > @@ -253,7 +264,7 @@ static acpi_status acpi_pci_osc_support(
> >  	if (ACPI_FAILURE(status))
> >  		return status;
> >  	mutex_lock(&osc_lock);
> > -	status = acpi_pci_query_osc(root, flags);
> > +	status = acpi_pci_query_osc(root, flags, NULL);
> >  	mutex_unlock(&osc_lock);
> >  	return status;
> >  }
> > @@ -362,6 +373,35 @@ out:
> >  }
> >  EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> >  
> > + /**
> > + * acpi_pci_root_osc_query - Get the _OSC bits the kernel can control.
> > + * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
> > + * @ctrl_mask: Mask of _OSC bits to query and the place to put the result into.
> > + **/
> > +acpi_status acpi_pci_root_osc_query(acpi_handle handle, u32 *ctrl_mask)
> > +{
> > +	acpi_status status;
> > +	acpi_handle tmp;
> > +	struct acpi_pci_root *root;
> > +
> > +	if (!ctrl_mask || !(*ctrl_mask & OSC_PCI_CONTROL_MASKS))
> > +		return AE_BAD_PARAMETER;
> > +
> > +	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_query_osc(root, root->osc_support_set, ctrl_mask);
> > +	mutex_unlock(&osc_lock);
> > +
> > +	return status;
> > +}
> > +
> >  /**
> >   * acpi_pci_osc_control_set - commit requested control to Firmware
> >   * @handle: acpi_handle for the target ACPI object
> > @@ -395,7 +435,7 @@ acpi_status acpi_pci_osc_control_set(acp
> >  
> >  	/* Need to query controls first before requesting them */
> >  	if (!root->osc_queried) {
> > -		status = acpi_pci_query_osc(root, root->osc_support_set);
> > +		status = acpi_pci_query_osc(root, root->osc_support_set, NULL);
> >  		if (ACPI_FAILURE(status))
> >  			goto out;
> >  	}
> > 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_root_osc_query(acpi_handle handle, u32 *ctrl_mask);
> >  extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags);
> >  extern void acpi_early_init(void);
> 
> IMHO acpi_pci_osc_control_query() is better name for new function here,
> to make it be paired with acpi_pci_osc_control_set(), and to avoid mistakable
> "*_get" that sounds like it takes controls via _OSC.

OK, I'll change that, but IMHO we're entering the "splitting hairs" area here. :-)

Thanks,
Rafael

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

[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/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/pci.h                    |    2 
 drivers/pci/pcie/Makefile            |    3 -
 drivers/pci/pcie/aer/aerdrv.c        |    9 ++-
 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      |   25 ++++++++--
 drivers/pci/pcie/portdrv_pci.c       |   30 ++++++++++++
 18 files changed, 191 insertions(+), 232 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();
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"
+#include "../pci.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;
+
+	if (acpi_pci_disabled)
+		return 0;
+
+	handle = acpi_find_root_bridge_handle(port);
+	if (!handle)
+		return -EINVAL;
+
+	flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL
+		| OSC_PCI_EXPRESS_NATIVE_HP_CONTROL
+		| OSC_PCI_EXPRESS_PME_CONTROL;
+
+	if (pcie_aer_get_firmware_first(port))
+		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
+	else if (pci_aer_available())
+		flags |= OSC_PCI_EXPRESS_AER_CONTROL;
+
+	status = acpi_pci_root_osc_query(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;
+	}
+
+	status = acpi_pci_osc_control_set(handle, flags);
+	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", flags);
+
+	*srv_mask = PCIE_PORT_SERVICE_VC;
+	if (flags & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
+		*srv_mask |= PCIE_PORT_SERVICE_HP;
+	if (flags & OSC_PCI_EXPRESS_PME_CONTROL)
+		*srv_mask |= PCIE_PORT_SERVICE_PME;
+	if (flags & OSC_PCI_EXPRESS_AER_CONTROL)
+		*srv_mask |= PCIE_PORT_SERVICE_AER;
+
+	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,23 +237,38 @@ 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_PME | PCIE_PORT_SERVICE_HP
+				| PCIE_PORT_SERVICE_VC;
+		if (pci_aer_available())
+			cap_mask |= PCIE_PORT_SERVICE_AER;
+	}
 
 	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;
 	/* 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;
 
 	return services;
@@ -494,6 +510,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/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/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -131,8 +131,10 @@ static inline void pci_msi_init_pci_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)
Index: linux-2.6/drivers/pci/pcie/aer/aerdrv.c
===================================================================
--- linux-2.6.orig/drivers/pci/pcie/aer/aerdrv.c
+++ linux-2.6/drivers/pci/pcie/aer/aerdrv.c
@@ -72,6 +72,11 @@ void pci_no_aer(void)
 	pcie_aer_disable = 1;	/* has priority over 'forceload' */
 }
 
+bool pci_aer_available(void)
+{
+	return !pcie_aer_disable && pci_msi_enabled();
+}
+
 static int set_device_error_reporting(struct pci_dev *dev, void *data)
 {
 	bool enable = *((bool *)data);
@@ -411,9 +416,7 @@ static void aer_error_resume(struct pci_
  */
 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);
 }

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

[Hidetoshi Seto]

(2010/07/31 7:32), Rafael J. Wysocki 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>
> ---

It seems that this patch still have more than one logical changes.

I think that "pcie_ports=compat" has an independent function, so
it could be in a separated patch.

Maybe we can have 2 patches (at least) before this change:
 [1] aerdrv: introduce pci_aer_available()
 [2] portdrv: introduce pcie_ports=
      'compat' to disable all port services
 [3] (this patch)

>  Documentation/kernel-parameters.txt  |   17 ++++--
>  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/pci.h                    |    2 
>  drivers/pci/pcie/Makefile            |    3 -
>  drivers/pci/pcie/aer/aerdrv.c        |    9 ++-
>  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      |   25 ++++++++--
>  drivers/pci/pcie/portdrv_pci.c       |   30 ++++++++++++
>  18 files changed, 191 insertions(+), 232 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;

A blank line?

> @@ -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/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"
> +#include "../pci.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;
> +
> +	if (acpi_pci_disabled)
> +		return 0;
> +
> +	handle = acpi_find_root_bridge_handle(port);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL
> +		| OSC_PCI_EXPRESS_NATIVE_HP_CONTROL
> +		| OSC_PCI_EXPRESS_PME_CONTROL;
> +
> +	if (pcie_aer_get_firmware_first(port))
> +		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
> +	else if (pci_aer_available())
> +		flags |= OSC_PCI_EXPRESS_AER_CONTROL;

Debug message will not be interested when AER is not available.
(and of course when AER is disabled intentionally)
How about:

if (pci_aer_available()) {
	if (pcie_aer_get_firmware_first(port))
		dev_dbg(&port->dev, "PCIe errors handled by BIOS.\n");
	else
		flags |= OSC_PCI_EXPRESS_AER_CONTROL;
}

> +
> +	status = acpi_pci_root_osc_query(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;
> +	}
> +
> +	status = acpi_pci_osc_control_set(handle, flags);
> +	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", flags);
> +
> +	*srv_mask = PCIE_PORT_SERVICE_VC;
> +	if (flags & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
> +		*srv_mask |= PCIE_PORT_SERVICE_HP;
> +	if (flags & OSC_PCI_EXPRESS_PME_CONTROL)
> +		*srv_mask |= PCIE_PORT_SERVICE_PME;
> +	if (flags & OSC_PCI_EXPRESS_AER_CONTROL)
> +		*srv_mask |= PCIE_PORT_SERVICE_AER;
> +
> +	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,23 +237,38 @@ 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_PME | PCIE_PORT_SERVICE_HP
> +				| PCIE_PORT_SERVICE_VC;
> +		if (pci_aer_available())
> +			cap_mask |= PCIE_PORT_SERVICE_AER;
> +	}
>  
>  	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;
>  	/* 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;
>  
>  	return services;
> @@ -494,6 +510,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/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/drivers/pci/pci.h
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.h
> +++ linux-2.6/drivers/pci/pci.h
> @@ -131,8 +131,10 @@ static inline void pci_msi_init_pci_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)
> Index: linux-2.6/drivers/pci/pcie/aer/aerdrv.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pcie/aer/aerdrv.c
> +++ linux-2.6/drivers/pci/pcie/aer/aerdrv.c
> @@ -72,6 +72,11 @@ void pci_no_aer(void)
>  	pcie_aer_disable = 1;	/* has priority over 'forceload' */
>  }
>  
> +bool pci_aer_available(void)
> +{
> +	return !pcie_aer_disable && pci_msi_enabled();
> +}
> +
>  static int set_device_error_reporting(struct pci_dev *dev, void *data)
>  {
>  	bool enable = *((bool *)data);
> @@ -411,9 +416,7 @@ static void aer_error_resume(struct pci_
>   */
>  static int __init aer_service_init(void)
>  {
> -	if (pcie_aer_disable)
> -		return -ENXIO;
> -	if (!pci_msi_enabled())
> +	if (pci_aer_available())
>  		return -ENXIO;

Please continue initialization if aer is available.

if (!pci_aer_available())

Thanks,
H.Seto

>  	return pcie_port_service_register(&aerdriver);
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

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

[Rafael J. Wysocki]

On Monday, August 02, 2010, Hidetoshi Seto wrote:
> (2010/07/31 7:32), Rafael J. Wysocki 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>
> > ---
> 
> It seems that this patch still have more than one logical changes.
> 
> I think that "pcie_ports=compat" has an independent function, so
> it could be in a separated patch.
> 
> Maybe we can have 2 patches (at least) before this change:
>  [1] aerdrv: introduce pci_aer_available()
>  [2] portdrv: introduce pcie_ports=
>       'compat' to disable all port services
>  [3] (this patch)

I can do that, but I'm not sure if that really is going to make things better.

The introduction of pcie_ports=, for example, is only necessary because of the
other changes in the $subject patch, so why do you want it to be separate?

Thanks,
Rafael

[PATCH 4/7] PCI / PCIe: Disable PCIe port services during port initialization

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PCI / PCIe: Disable PCIe port services during port initialization

In principle PCIe port services may be enabled by the BIOS, so it's
better to disable them during port initialization to avoid spurious
events from being generated.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pcie/portdrv_core.c |   29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

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
@@ -256,20 +256,43 @@ 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 they have been
+			 * enabled by the BIOS and 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)
-	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
+	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR)) {
 		services |= PCIE_PORT_SERVICE_AER;
+		/*
+		 * Disable AER on this port in case it's been enabled by the
+		 * BIOS (the AER service driver will enable it when necessary).
+		 */
+		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 ((cap_mask & PCIE_PORT_SERVICE_PME)
-	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
+	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
 		services |= PCIE_PORT_SERVICE_PME;
+		/*
+		 * Disable PME interrupt on this port in case it's been enabled
+		 * by the BIOS (the PME service driver will enable it when
+		 * necessary).
+		 */
+		pcie_pme_interrupt_enable(dev, false);
+	}
 
 	return services;
 }

Re: [PATCH 4/7] PCI / PCIe: Disable PCIe port services during port initialization

[Hidetoshi Seto]

(2010/07/31 7:34), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PCI / PCIe: Disable PCIe port services during port initialization
> 
> In principle PCIe port services may be enabled by the BIOS, so it's
> better to disable them during port initialization to avoid spurious
> events from being generated.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/pci/pcie/portdrv_core.c |   29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> 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
> @@ -256,20 +256,43 @@ 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 they have been
> +			 * enabled by the BIOS and 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)
> -	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
> +	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR)) {
>  		services |= PCIE_PORT_SERVICE_AER;
> +		/*
> +		 * Disable AER on this port in case it's been enabled by the
> +		 * BIOS (the AER service driver will enable it when necessary).
> +		 */
> +		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 ((cap_mask & PCIE_PORT_SERVICE_PME)
> -	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
> +	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
>  		services |= PCIE_PORT_SERVICE_PME;
> +		/*
> +		 * Disable PME interrupt on this port in case it's been enabled
> +		 * by the BIOS (the PME service driver will enable it when
> +		 * necessary).
> +		 */
> +		pcie_pme_interrupt_enable(dev, false);
> +	}
>  
>  	return services;
>  }
> 

I believe that comments described well are always welcomed.

Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>

[PATCH 5/7] PCI / PCIe: Remove the port driver module exit routine

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PCI / PCIe: Remove the port driver module exit routine

The PCIe port driver's module exit routine is never used, so drop it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pcie/portdrv_pci.c |    7 -------
 1 file changed, 7 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
@@ -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 5/7] PCI / PCIe: Remove the port driver module exit routine

[Hidetoshi Seto]

(2010/07/31 7:35), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PCI / PCIe: Remove the port driver module exit routine
> 
> The PCIe port driver's module exit routine is never used, so drop it.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/pci/pcie/portdrv_pci.c |    7 -------
>  1 file changed, 7 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
> @@ -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);

This is exactly Kaneshige-san's [6/6].

And it seems that this patch can be applied alone.

Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>

Re: [PATCH 5/7] PCI / PCIe: Remove the port driver module exit routine

[Rafael J. Wysocki]

On Monday, August 02, 2010, Hidetoshi Seto wrote:
> (2010/07/31 7:35), Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: PCI / PCIe: Remove the port driver module exit routine
> > 
> > The PCIe port driver's module exit routine is never used, so drop it.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/pci/pcie/portdrv_pci.c |    7 -------
> >  1 file changed, 7 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
> > @@ -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);
> 
> This is exactly Kaneshige-san's [6/6].

You're right, sorry for not getting the sign-offs right.

> And it seems that this patch can be applied alone.
> 
> Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>

Thanks,
Rafael

[PATCH 6/7] ACPI / PCI: Do not preserve _OSC control bits returned by a query

[Rafael J. Wysocki]

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

All of the remaining callers of acpi_pci_osc_control_set() either
use acpi_pci_root_osc_query() right before calling it, like
pcie_port_acpi_setup(), or ask for control of one feature only,
like acpi_get_hp_hw_control_from_firmware().  Thus there is no
reason to preserve the _OSC control bits returned by an _OSC query
and the osc_control_qry and osc_queried fields of struct
acpi_pci_root are not necessary any more.  Remove them and modify the
code that uses them accordingly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/pci_root.c |   15 ---------------
 include/acpi/acpi_bus.h |    3 ---
 2 files changed, 18 deletions(-)

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
@@ -247,10 +247,8 @@ static acpi_status acpi_pci_query_osc(st
 	status = acpi_pci_run_osc(root->device->handle, capbuf);
 	if (ACPI_SUCCESS(status)) {
 		root->osc_support_set = support;
-		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
 		if (control)
 			*control = capbuf[OSC_CONTROL_TYPE];
-		root->osc_queried = 1;
 	}
 	return status;
 }
@@ -433,19 +431,6 @@ acpi_status acpi_pci_osc_control_set(acp
 	if ((root->osc_control_set & control_req) == control_req)
 		goto out;
 
-	/* Need to query controls first before requesting them */
-	if (!root->osc_queried) {
-		status = acpi_pci_query_osc(root, root->osc_support_set, NULL);
-		if (ACPI_FAILURE(status))
-			goto out;
-	}
-	if ((root->osc_control_qry & control_req) != control_req) {
-		printk(KERN_DEBUG
-		       "Firmware did not grant requested _OSC control\n");
-		status = AE_SUPPORT;
-		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;
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/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 6/7] ACPI / PCI: Do not preserve _OSC control bits returned by a query

[Hidetoshi Seto]

(2010/07/31 7:36), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> All of the remaining callers of acpi_pci_osc_control_set() either
> use acpi_pci_root_osc_query() right before calling it, like
> pcie_port_acpi_setup(), or ask for control of one feature only,
> like acpi_get_hp_hw_control_from_firmware().  Thus there is no
> reason to preserve the _OSC control bits returned by an _OSC query
> and the osc_control_qry and osc_queried fields of struct
> acpi_pci_root are not necessary any more.  Remove them and modify the
> code that uses them accordingly.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/pci_root.c |   15 ---------------
>  include/acpi/acpi_bus.h |    3 ---
>  2 files changed, 18 deletions(-)
> 
> 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
> @@ -247,10 +247,8 @@ static acpi_status acpi_pci_query_osc(st
>  	status = acpi_pci_run_osc(root->device->handle, capbuf);
>  	if (ACPI_SUCCESS(status)) {
>  		root->osc_support_set = support;
> -		root->osc_control_qry = capbuf[OSC_CONTROL_TYPE];
>  		if (control)
>  			*control = capbuf[OSC_CONTROL_TYPE];
> -		root->osc_queried = 1;
>  	}
>  	return status;
>  }
> @@ -433,19 +431,6 @@ acpi_status acpi_pci_osc_control_set(acp
>  	if ((root->osc_control_set & control_req) == control_req)
>  		goto out;
>  
> -	/* Need to query controls first before requesting them */
> -	if (!root->osc_queried) {
> -		status = acpi_pci_query_osc(root, root->osc_support_set, NULL);
> -		if (ACPI_FAILURE(status))
> -			goto out;
> -	}
> -	if ((root->osc_control_qry & control_req) != control_req) {
> -		printk(KERN_DEBUG
> -		       "Firmware did not grant requested _OSC control\n");
> -		status = AE_SUPPORT;
> -		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;
> Index: linux-2.6/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpi_bus.h
> +++ linux-2.6/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 */
> 

Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>

[PATCH 7/7] ACPI / PCI: Reorder checks in acpi_pci_osc_control_set()

[Rafael J. Wysocki]

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

Make acpi_pci_osc_control_set() attempt to find the handle of the
_OSC object under the given PCI root bridge object after verifying
that its second argument is correct and that there is a struct
acpi_pci_root object for the given root bridge handle.  This is
more logical than the old code and it matches the code ordering
in acpi_pci_root_osc_query().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/pci_root.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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
@@ -414,10 +414,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;
@@ -426,6 +422,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 7/7] ACPI / PCI: Reorder checks in acpi_pci_osc_control_set()

[Hidetoshi Seto]

(2010/07/31 7:37), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Make acpi_pci_osc_control_set() attempt to find the handle of the
> _OSC object under the given PCI root bridge object after verifying
> that its second argument is correct and that there is a struct
> acpi_pci_root object for the given root bridge handle.  This is
> more logical than the old code and it matches the code ordering
> in acpi_pci_root_osc_query().
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/pci_root.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 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
> @@ -414,10 +414,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;
> @@ -426,6 +422,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)
> 

Description is better than Kaneshige-san's [3/6].

And it seems that this patch can be applied alone too.

Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>