Re: [PATCH] PCI/ASPM: Print correct ASPM status when _OSC failed

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Jun 02, 2020 at 01:50:37PM -0400, Sinan Kaya wrote:
> Bjorn,
> 
> On 6/1/2020 9:57 PM, Yicong Yang wrote:
> > well, Sinan's words make sense to me. But I'm still confused that, the message
> > says we're "disabling ASPM" but ASPM maybe enabled if we designate
> > pcie_aspm=force as I mentioned in the commit message. Will it be possible if
> > we replace "disabling" to "disabled" or we can do something else to make
> > the message reflect the real status of ASPM?
> 
> What do you think?

ASPM is a mess in general, and the whole "no_aspm" dance for delaying
setting of aspm_disabled is ... well, it's confusing at best.

These "_OSC failed" messages are confusing to users as well.  They
lead to bug reports against Linux (when it's usually a BIOS problem)
and users booting with "pcie_aspm=force" (which is a poor user
experience and potentially dangerous since the platform hasn't granted
us control of the PCIe Capability).

And it's not even specific to ASPM; when _OSC fails, we don't take
over *any* PCIe features.  At least, we're not *supposed* to -- I
don't think we're very careful about random things in the PCIe
capability.

What if we just removed the ASPM text from the message completely,
e.g., something like this:

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 800a3d26d24b..49fdb07061b1 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -453,9 +453,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;
 	}
 
@@ -516,7 +515,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));
 
 		/*