* [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU
@ 2014-03-10 9:56 Adrian Huang12
2014-03-10 15:45 ` Matthew Garrett
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Huang12 @ 2014-03-10 9:56 UTC (permalink / raw
To: Rafael J. Wysocki, Len Brown; +Cc: Adrian Huang12, linux-acpi@vger.kernel.org
On the Haswell-based server (Grantley platform), there are two PCI
root bridges. One is the PCI-express root bridge (HID: PNP0A08)
for processor IIO devices, and the other is the PCI root bridge
(HID: PNP0A03) for the processor uncore devices.
Some BIOSes do not implement the _OSC object for PCI root bridge
(PNP0A03) since it is the optional object described on the page 286
of ACPI spec 5.0. This causes the variable no_aspm is set to 1 when
evaluating the undefined _OSC object of the PCI root bridge. It
turns out that the ASPM policy cannot be changed even though the
PCI-express root bridge on the Haswell-based server platform is
evaluated successfully. Here is the example:
$ cat /sys/module/pcie_aspm/parameters/policy
[default] performance powersave
$ echo performance | sudo tee /sys/module/pcie_aspm/parameters/policy
performance
tee: /sys/module/pcie_aspm/parameters/policy: Operation not permitted
To fix the above-mentioned issue, this patch does not set the
variable no_aspm to 1 if the returned status is AE_NOT_FOUND and
the bridge is the PCI root bridge.
After applying this patch, the ASPM policy can be changed correctly.
And, the corresponding reaction works correctly after changing a
policy.
Tested on the Grantley platform and v3.14-rc6.
Signed-off-by: Adrian Huang <ahuang12@lenovo.com>
---
drivers/acpi/pci_root.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c1c4102..cfb3bca 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -446,6 +446,13 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
decode_osc_support(root, "OS supports", support);
status = acpi_pci_osc_support(root, support);
if (ACPI_FAILURE(status)) {
+ if (status == AE_NOT_FOUND &&
+ !strcmp(acpi_device_hid(device), PCI_ROOT_HID_STRING)) {
+ dev_info(&device->dev,
+ "No _OSC; skip ASPM configuration\n");
+ return;
+ }
+
dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
acpi_format_exception(status));
*no_aspm = 1;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU
2014-03-10 9:56 [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU Adrian Huang12
@ 2014-03-10 15:45 ` Matthew Garrett
2014-03-10 16:17 ` Adrian Huang12
0 siblings, 1 reply; 5+ messages in thread
From: Matthew Garrett @ 2014-03-10 15:45 UTC (permalink / raw
To: Adrian Huang12; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi@vger.kernel.org
On Mon, Mar 10, 2014 at 09:56:25AM +0000, Adrian Huang12 wrote:
> + if (status == AE_NOT_FOUND &&
Why limit it to not found? I suspect that we should never be basing our
ASPM policy on the behaviour of PCI (rather than PCIe) bridges.
I'd recommend changing the patch subject line, too - there's nothing
Haswell specific about this.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU
2014-03-10 15:45 ` Matthew Garrett
@ 2014-03-10 16:17 ` Adrian Huang12
2014-03-10 16:24 ` Matthew Garrett
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Huang12 @ 2014-03-10 16:17 UTC (permalink / raw
To: Matthew Garrett
Cc: Adrian Huang12, Rafael J. Wysocki, Len Brown,
linux-acpi@vger.kernel.org
On Mon, 2014-03-10 at 15:45 +0000, Matthew Garrett wrote:
> On Mon, Mar 10, 2014 at 09:56:25AM +0000, Adrian Huang12 wrote:
> > + if (status == AE_NOT_FOUND &&
>
> Why limit it to not found?
Just for the undefined _OSC object in order to follow ACPI5.0.
Looks like another approach should be implemented to address
this issue. Is this what you were thinking of: we should never
evaluate the _OSC object if it is the PCI root bridge?
> I suspect that we should never be basing our
> ASPM policy on the behaviour of PCI (rather than PCIe) bridges.
Yes, agree since the ASPM functionality is supported only for PCIe.
Do you agree we should never evaluate the _OSC object if it is
the PCI root bridge?
> I'd recommend changing the patch subject line, too - there's nothing
> Haswell specific about this.
Thanks. I will change the subject line. :-)
--
Adrian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU
2014-03-10 16:17 ` Adrian Huang12
@ 2014-03-10 16:24 ` Matthew Garrett
2014-03-13 12:22 ` Adrian Huang12
0 siblings, 1 reply; 5+ messages in thread
From: Matthew Garrett @ 2014-03-10 16:24 UTC (permalink / raw
To: Adrian Huang12; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi@vger.kernel.org
On Mon, Mar 10, 2014 at 04:17:05PM +0000, Adrian Huang12 wrote:
> On Mon, 2014-03-10 at 15:45 +0000, Matthew Garrett wrote:
> > On Mon, Mar 10, 2014 at 09:56:25AM +0000, Adrian Huang12 wrote:
> > > + if (status == AE_NOT_FOUND &&
> >
> > Why limit it to not found?
>
> Just for the undefined _OSC object in order to follow ACPI5.0.
> Looks like another approach should be implemented to address
> this issue. Is this what you were thinking of: we should never
> evaluate the _OSC object if it is the PCI root bridge?
No, I meant we should never set no_aspm because _OSC fails for any
reason on a non-PCIe root bridge. But thinking about it, I suspect that
the whole way we handle _OSC in this case is wrong. If a PCI host bridge
does implement _OSC then there's still a good chance that it'll refuse
to grant us control over ASPM, and so we may still end up with failure
cases.
> > I suspect that we should never be basing our
> > ASPM policy on the behaviour of PCI (rather than PCIe) bridges.
>
> Yes, agree since the ASPM functionality is supported only for PCIe.
> Do you agree we should never evaluate the _OSC object if it is
> the PCI root bridge?
Just skipping _OSC entirely in that case would certainly fix the issue,
but it doesn't sound like the best fix. I think we need to revisit some
assumptions in this code.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU
2014-03-10 16:24 ` Matthew Garrett
@ 2014-03-13 12:22 ` Adrian Huang12
0 siblings, 0 replies; 5+ messages in thread
From: Adrian Huang12 @ 2014-03-13 12:22 UTC (permalink / raw
To: Matthew Garrett
Cc: Adrian Huang12, Rafael J. Wysocki, Len Brown,
linux-acpi@vger.kernel.org
On Mon, 2014-03-10 at 16:24 +0000, Matthew Garrett wrote:
> On Mon, Mar 10, 2014 at 04:17:05PM +0000, Adrian Huang12 wrote:
> > On Mon, 2014-03-10 at 15:45 +0000, Matthew Garrett wrote:
> > > On Mon, Mar 10, 2014 at 09:56:25AM +0000, Adrian Huang12 wrote:
>
> Just skipping _OSC entirely in that case would certainly fix the issue,
> but it doesn't sound like the best fix. I think we need to revisit some
> assumptions in this code.
>
Thanks for the comments. I'll try to figure out the best fix.
-- Adrian
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-13 12:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-10 9:56 [PATCH 1/1] ACPI: Fix the incorrect behavior of the disabled ASPM on Haswell CPU Adrian Huang12
2014-03-10 15:45 ` Matthew Garrett
2014-03-10 16:17 ` Adrian Huang12
2014-03-10 16:24 ` Matthew Garrett
2014-03-13 12:22 ` Adrian Huang12
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.