[PATCH 0/3] PCI/ACPI: _OSC cleanups

[PATCH 0/3] PCI/ACPI: _OSC cleanups

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Clean up a few _OSC-related things.

We talked about the _OSC failure message in the last patch long ago, and I
just dropped the ball, sorry about that.  Previous discussion:
https://lore.kernel.org/linux-pci/20200602223618.GA845676@bjorn-Precision-5520/

I'm happy to merge these given your ack, Rafael, or you can take them if
they look good to you.

Bjorn Helgaas (3):
  PCI/ACPI: Make acpi_pci_osc_control_set() static
  PCI/ACPI: Remove unnecessary osc_lock
  PCI/ACPI: Clarify message about _OSC failure

 drivers/acpi/pci_root.c | 40 ++++++++++++++--------------------------
 include/linux/acpi.h    |  3 ---
 2 files changed, 14 insertions(+), 29 deletions(-)

-- 
2.25.1

[PATCH 1/3] PCI/ACPI: Make acpi_pci_osc_control_set() static

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

acpi_pci_osc_control_set() is only called from pci_root.c, so stop
exporting it and make it static.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/acpi/pci_root.c | 3 +--
 include/linux/acpi.h    | 3 ---
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index ff7882afeb29..51dec352b8b8 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -353,7 +353,7 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
  * _OSC bits the BIOS has granted control of, but its contents are meaningless
  * on failure.
  **/
-acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
+static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 {
 	struct acpi_pci_root *root;
 	acpi_status status = AE_OK;
@@ -406,7 +406,6 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 	mutex_unlock(&osc_lock);
 	return status;
 }
-EXPORT_SYMBOL(acpi_pci_osc_control_set);
 
 static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
 				 bool is_pcie)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 053bf05fb1f7..4703daafcce9 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -581,9 +581,6 @@ extern bool osc_pc_lpi_support_confirmed;
 #define ACPI_GSB_ACCESS_ATTRIB_RAW_BYTES	0x0000000E
 #define ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS	0x0000000F
 
-extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
-					     u32 *mask, u32 req);
-
 /* Enable _OST when all relevant hotplug operations are enabled */
 #if defined(CONFIG_ACPI_HOTPLUG_CPU) &&			\
 	defined(CONFIG_ACPI_HOTPLUG_MEMORY) &&		\
-- 
2.25.1

[PATCH 2/3] PCI/ACPI: Remove unnecessary osc_lock

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

9778c14b4ca2 ("ACPI/PCI: Fix possible race condition on _OSC evaluation")
added locking around _OSC calls to protect the acpi_osc_data_list that
stored the results.

63f10f0f6df4 ("PCI/ACPI: move _OSC code to pci_root.c") moved the results
from acpi_osc_data_list to the struct acpi_pci_root, where it no longer
needs locking, but did not remove the lock.

Remove the unnecessary locking around _OSC calls.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/acpi/pci_root.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 51dec352b8b8..a001f8f56b4b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -56,8 +56,6 @@ static struct acpi_scan_handler pci_root_handler = {
 	},
 };
 
-static DEFINE_MUTEX(osc_lock);
-
 /**
  * acpi_is_root_bridge - determine whether an ACPI CA node is a PCI root bridge
  * @handle:  the ACPI CA node in question.
@@ -223,12 +221,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
 
 static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
 {
-	acpi_status status;
-
-	mutex_lock(&osc_lock);
-	status = acpi_pci_query_osc(root, flags, NULL);
-	mutex_unlock(&osc_lock);
-	return status;
+	return acpi_pci_query_osc(root, flags, NULL);
 }
 
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
@@ -356,7 +349,7 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
 static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 {
 	struct acpi_pci_root *root;
-	acpi_status status = AE_OK;
+	acpi_status status;
 	u32 ctrl, capbuf[3];
 
 	if (!mask)
@@ -370,18 +363,16 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 r
 	if (!root)
 		return AE_NOT_EXIST;
 
-	mutex_lock(&osc_lock);
-
 	*mask = ctrl | root->osc_control_set;
 	/* No need to evaluate _OSC if the control was already granted. */
 	if ((root->osc_control_set & ctrl) == ctrl)
-		goto out;
+		return AE_OK;
 
 	/* Need to check the available controls bits before requesting them. */
 	while (*mask) {
 		status = acpi_pci_query_osc(root, root->osc_support_set, mask);
 		if (ACPI_FAILURE(status))
-			goto out;
+			return status;
 		if (ctrl == *mask)
 			break;
 		decode_osc_control(root, "platform does not support",
@@ -392,19 +383,18 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 r
 	if ((ctrl & req) != req) {
 		decode_osc_control(root, "not requesting control; platform does not support",
 				   req & ~(ctrl));
-		status = AE_SUPPORT;
-		goto out;
+		return AE_SUPPORT;
 	}
 
 	capbuf[OSC_QUERY_DWORD] = 0;
 	capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
 	capbuf[OSC_CONTROL_DWORD] = ctrl;
 	status = acpi_pci_run_osc(handle, capbuf, mask);
-	if (ACPI_SUCCESS(status))
-		root->osc_control_set = *mask;
-out:
-	mutex_unlock(&osc_lock);
-	return status;
+	if (ACPI_FAILURE(status))
+		return status;
+
+	root->osc_control_set = *mask;
+	return AE_OK;
 }
 
 static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
-- 
2.25.1

[PATCH 3/3] PCI/ACPI: Clarify message about _OSC failure

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

This message:

  acpi PNP0A08:02: _OSC failed (AE_NOT_FOUND); disabling ASPM

is alarming and slightly misleading.  Per the PCI Firmware Spec, r3.2, sec
4.5.1, _OSC is required for PCIe hierarchies.  If _OSC is absent or fails,
the OS must not attempt to use any of the features defined for _OSC.  That
includes native hotplug, native PME, AER, and other things as well as ASPM.

Rephrase the message to:

  acpi PNP0A08:02: _OSC: platform retains control of PCIe features (AE_NOT_FOUND)

Previous discussion at https://lore.kernel.org/r/20200602223618.GA845676@bjorn-Precision-5520/

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Sinan Kaya <okaya@kernel.org>
Cc: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/acpi/pci_root.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index a001f8f56b4b..6950a35a5ac7 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -441,9 +441,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
 		if ((status == AE_NOT_FOUND) && !is_pcie)
 			return;
 
-		dev_info(&device->dev, "_OSC failed (%s)%s\n",
-			 acpi_format_exception(status),
-			 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
+		dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
+			 acpi_format_exception(status));
 		return;
 	}
 
@@ -499,7 +498,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
 	} else {
 		decode_osc_control(root, "OS requested", requested);
 		decode_osc_control(root, "platform willing to grant", control);
-		dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
+		dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
 			acpi_format_exception(status));
 
 		/*
-- 
2.25.1

Re: [PATCH 0/3] PCI/ACPI: _OSC cleanups

[Rafael J. Wysocki]

On Wed, Jan 27, 2021 at 1:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Clean up a few _OSC-related things.
>
> We talked about the _OSC failure message in the last patch long ago, and I
> just dropped the ball, sorry about that.  Previous discussion:
> https://lore.kernel.org/linux-pci/20200602223618.GA845676@bjorn-Precision-5520/
>
> I'm happy to merge these given your ack, Rafael, or you can take them if
> they look good to you.

They do look good to me, so

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

for the whole series.

I don't think it really matters which way they go in and I'm guessing
that it will be somewhat more convenient to you to apply them, so
please route them through the PCI tree.

Cheers!

Re: [PATCH 0/3] PCI/ACPI: _OSC cleanups

[Bjorn Helgaas]

On Wed, Jan 27, 2021 at 02:55:07PM +0100, Rafael J. Wysocki wrote:
> On Wed, Jan 27, 2021 at 1:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Clean up a few _OSC-related things.
> >
> > We talked about the _OSC failure message in the last patch long ago, and I
> > just dropped the ball, sorry about that.  Previous discussion:
> > https://lore.kernel.org/linux-pci/20200602223618.GA845676@bjorn-Precision-5520/
> >
> > I'm happy to merge these given your ack, Rafael, or you can take them if
> > they look good to you.
> 
> They do look good to me, so
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> for the whole series.
> 
> I don't think it really matters which way they go in and I'm guessing
> that it will be somewhat more convenient to you to apply them, so
> please route them through the PCI tree.

Thanks, Rafael, I put these on my pci/enumeration branch for v5.12.