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));
/*
