* [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC
@ 2026-07-31 9:09 KONDO KAZUMA(近藤 和真)
[not found] ` <20260731092147.B7C531F00A3F@smtp.kernel.org>
0 siblings, 1 reply; 4+ messages in thread
From: KONDO KAZUMA(近藤 和真) @ 2026-07-31 9:09 UTC (permalink / raw)
To: rafael, lenb, bhelgaas
Cc: jonathan.cameron, linux-acpi, linux-pci, linux-kernel
After commit 7d703df7f4f5 ("ACPI: bus: Split _OSC evaluation out of
acpi_run_osc()"), the _OSC evaluation path now returns AE_ERROR to
negotiate_os_control() instead of propagating AE_NOT_FOUND from
acpi_evaluate_object().
This has not caused any functional issues so far, but it produces
additional misleading messages for non-PCIe host bridges without _OSC on
some Intel servers:
kernel: ACPI: Enabled 2 GPEs in block 00 to 7F
kernel: ACPI: PCI Root Bridge [UNC0] (domain 0000 [bus fe])
kernel: acpi PNP0A03:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI EDR HPX-Type3]
+ kernel: acpi PNP0A03:00: _OSC: OS requested [PCIeHotplug SHPCHotplug PME AER PCIeCapability LTR DPC]
+ kernel: acpi PNP0A03:00: _OSC: platform willing to grant [PCIeHotplug SHPCHotplug PME AER PCIeCapability LTR DPC]
+ kernel: acpi PNP0A03:00: _OSC: platform retains control of PCIe features (AE_ERROR)
kernel: PCI host bridge to bus 0000:fe
Previously, negotiate_os_control() silently ignored AE_NOT_FOUND for
non-PCIe host bridges without _OSC, but after the above change, it no
longer does so.
As a result, negotiate_os_control() logs messages as if the OS had
negotiated with the platform via _OSC, even though the non-PCIe host
bridge has no _OSC method and no such negotiation actually occurs.
Skip _OSC negotiation for non-PCIe host bridges that do not define an
_OSC method before attempting to evaluate it.
Fixes: 7d703df7f4f5 ("ACPI: bus: Split _OSC evaluation out of acpi_run_osc()")
Link: https://lore.kernel.org/linux-acpi/d0be949d-0e21-472e-a44b-cedb1dd8695d@nec.com/
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Kazuma Kondo <kazuma-kondo@nec.com>
---
drivers/acpi/pci_root.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 4c06c3ffd0cb..3a24d018436b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -574,6 +574,12 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
return;
}
+ /* _OSC is optional for PCI host bridges */
+ if (!is_pcie(root) && !acpi_has_method(handle, "_OSC")) {
+ dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for non-PCIe host bridge)\n");
+ return;
+ }
+
support = calculate_support();
decode_osc_support(root, "OS supports", support);
@@ -615,10 +621,6 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
*/
*no_aspm = 1;
- /* _OSC is optional for PCI host bridges */
- if (status == AE_NOT_FOUND && !is_pcie(root))
- return;
-
if (control) {
decode_osc_control(root, "OS requested", requested);
decode_osc_control(root, "platform willing to grant", control);
--
2.49.0
^ permalink raw reply [flat|nested] 4+ messages in thread[parent not found: <20260731092147.B7C531F00A3F@smtp.kernel.org>]
* Re: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC [not found] ` <20260731092147.B7C531F00A3F@smtp.kernel.org> @ 2026-08-03 10:05 ` KONDO KAZUMA(近藤 和真) 2026-08-03 11:47 ` Rafael J. Wysocki (Intel) 0 siblings, 1 reply; 4+ messages in thread From: KONDO KAZUMA(近藤 和真) @ 2026-08-03 10:05 UTC (permalink / raw) To: rafael, lenb, bhelgaas; +Cc: linux-acpi, linux-pci, linux-kernel Hello, I received review comments from Sashiko: https://sashiko.dev/#/patchset/20260731090820.493064-1-kazuma-kondo@nec.com?part=1 On 2026/07/31 18:21, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Early return bypasses ASPM disablement for non-PCIe and CXL host bridges without _OSC, causing an undocumented functional change and potential instability > -- > > --- Patch [1]: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC --- > commit e68d29f653a1c34f8b7b08baaa548e2ff2d3663f > Author: KONDO KAZUMA( $B6aF#!!OB?? (B) <kazuma-kondo@nec.com> > > ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC > > This patch skips _OSC negotiation for non-PCIe host bridges that do not define > an _OSC method before attempting to evaluate it. It aims to prevent misleading > messages in the logs on some Intel servers where the OS appears to negotiate > with the platform despite the lack of an _OSC method. > > Link: https://lore.kernel.org/linux-acpi/d0be949d-0e21-472e-a44b-cedb1dd8695d@nec.com/ > >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >> index 4c06c3ffd0cbb..3a24d018436b4 100644 >> --- a/drivers/acpi/pci_root.c >> +++ b/drivers/acpi/pci_root.c >> @@ -574,6 +574,12 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) >> return; >> } >> >> + /* _OSC is optional for PCI host bridges */ >> + if (!is_pcie(root) && !acpi_has_method(handle, "_OSC")) { >> + dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for non-PCIe host bridge)\n"); >> + return; >> + } >> + > > [Severity: High] > Does this early return unintentionally skip disabling ASPM for these host > bridges? > > The commit message states this change produces no functional issues and only > removes misleading messages. However, before this patch, if the _OSC method > was missing, the function would proceed, fail the negotiation, and fall > through to the failure path where *no_aspm = 1 is unconditionally set. > > By returning early here, it looks like *no_aspm = 1 is completely bypassed > for legacy host bridges. > > Additionally, is_pcie() only checks for ACPI_BRIDGE_TYPE_PCIE. Could this > cause CXL host bridges (ACPI_BRIDGE_TYPE_CXL) to evaluate to false here? > If a CXL bridge lacks an _OSC method, it would take this early return and skip > ASPM disablement entirely, which could leave ASPM enabled inappropriately > without firmware negotiation. I think Sashiko's review is reasonable. According to [1], unless the platform explicitly grants the OS control over ASPM via _OSC, the OS should not change the ASPM state configured by the platform. Also, as described in [2], if CXL 1.1 hosts do not publish a CXL _OSC, the OS should fall back to the PCIe _OSC (just as acpi_pci_query_osc() does). So, negotiate_os_control() should skip _OSC negotiation only when a legacy PCI host bridge does not have _OSC. For example: /* _OSC is optional for PCI host bridges */ if (!is_pcie(root) && !is_cxl(root) && !acpi_has_method(handle, "_OSC")) { dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for legacy PCI host bridge)\n"); /* * Disable ASPM unless the OS has been granted control over it * via _OSC. */ *no_aspm = 1; return; } If I have misunderstood anything, please let me know. [1] https://lore.kernel.org/all/1320961113-5050-1-git-send-email-mjg@redhat.com/ [2] https://lore.kernel.org/all/20220413073618.291335-3-vishal.l.verma@intel.com/ Thanks, Kazuma > >> support = calculate_support(); >> >> decode_osc_support(root, "OS supports", support); > > [ ... ] > >> @@ -615,10 +621,6 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) >> */ >> *no_aspm = 1; >> >> - /* _OSC is optional for PCI host bridges */ >> - if (status == AE_NOT_FOUND && !is_pcie(root)) >> - return; >> - >> if (control) { >> decode_osc_control(root, "OS requested", requested); >> decode_osc_control(root, "platform willing to grant", control); > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC 2026-08-03 10:05 ` KONDO KAZUMA(近藤 和真) @ 2026-08-03 11:47 ` Rafael J. Wysocki (Intel) 2026-08-05 11:28 ` KONDO KAZUMA(近藤 和真) 0 siblings, 1 reply; 4+ messages in thread From: Rafael J. Wysocki (Intel) @ 2026-08-03 11:47 UTC (permalink / raw) To: KONDO KAZUMA(近藤 和真) Cc: bhelgaas, linux-acpi, linux-pci, linux-kernel On Mon, Aug 3, 2026 at 12:05 PM KONDO KAZUMA(近藤 和真) <kazuma-kondo@nec.com> wrote: > > Hello, > > I received review comments from Sashiko: > https://sashiko.dev/#/patchset/20260731090820.493064-1-kazuma-kondo@nec.com?part=1 > > On 2026/07/31 18:21, sashiko-bot@kernel.org wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > - [High] Early return bypasses ASPM disablement for non-PCIe and CXL host bridges without _OSC, causing an undocumented functional change and potential instability > > -- > > > > --- Patch [1]: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC --- > > commit e68d29f653a1c34f8b7b08baaa548e2ff2d3663f > > Author: KONDO KAZUMA( $B6aF#!!OB?? (B) <kazuma-kondo@nec.com> > > > > ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC > > > > This patch skips _OSC negotiation for non-PCIe host bridges that do not define > > an _OSC method before attempting to evaluate it. It aims to prevent misleading > > messages in the logs on some Intel servers where the OS appears to negotiate > > with the platform despite the lack of an _OSC method. > > > > Link: https://lore.kernel.org/linux-acpi/d0be949d-0e21-472e-a44b-cedb1dd8695d@nec.com/ > > > >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > >> index 4c06c3ffd0cbb..3a24d018436b4 100644 > >> --- a/drivers/acpi/pci_root.c > >> +++ b/drivers/acpi/pci_root.c > >> @@ -574,6 +574,12 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) > >> return; > >> } > >> > >> + /* _OSC is optional for PCI host bridges */ > >> + if (!is_pcie(root) && !acpi_has_method(handle, "_OSC")) { > >> + dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for non-PCIe host bridge)\n"); > >> + return; > >> + } > >> + > > > > [Severity: High] > > Does this early return unintentionally skip disabling ASPM for these host > > bridges? > > > > The commit message states this change produces no functional issues and only > > removes misleading messages. However, before this patch, if the _OSC method > > was missing, the function would proceed, fail the negotiation, and fall > > through to the failure path where *no_aspm = 1 is unconditionally set. > > > > By returning early here, it looks like *no_aspm = 1 is completely bypassed > > for legacy host bridges. > > > > Additionally, is_pcie() only checks for ACPI_BRIDGE_TYPE_PCIE. Could this > > cause CXL host bridges (ACPI_BRIDGE_TYPE_CXL) to evaluate to false here? > > If a CXL bridge lacks an _OSC method, it would take this early return and skip > > ASPM disablement entirely, which could leave ASPM enabled inappropriately > > without firmware negotiation. > > I think Sashiko's review is reasonable. According to [1], unless the > platform explicitly grants the OS control over ASPM via _OSC, the OS > should not change the ASPM state configured by the platform. Though ASPM only is a thing for PCIe, isn't it? > Also, as described in [2], if CXL 1.1 hosts do not publish a CXL _OSC, > the OS should fall back to the PCIe _OSC (just as acpi_pci_query_osc() > does). > > So, negotiate_os_control() should skip _OSC negotiation only when a > legacy PCI host bridge does not have _OSC. For example: > > /* _OSC is optional for PCI host bridges */ The comment above is somewhat redundant IMO. > if (!is_pcie(root) && !is_cxl(root) && > !acpi_has_method(handle, "_OSC")) { > dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for legacy PCI host bridge)\n"); Please make the comment say "Skipping a non-PCIe root without _OSC" or similar. > > /* > * Disable ASPM unless the OS has been granted control over it > * via _OSC. > */ > *no_aspm = 1; So I agree that setting *no_aspm to 1 in this case is a good idea, but the comment above is slightly misleading. I would just make it say "No ASPM for non-PCI host bridges" or equivalent. > return; > } > > If I have misunderstood anything, please let me know. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC 2026-08-03 11:47 ` Rafael J. Wysocki (Intel) @ 2026-08-05 11:28 ` KONDO KAZUMA(近藤 和真) 0 siblings, 0 replies; 4+ messages in thread From: KONDO KAZUMA(近藤 和真) @ 2026-08-05 11:28 UTC (permalink / raw) To: Rafael J. Wysocki (Intel); +Cc: bhelgaas, linux-acpi, linux-pci, linux-kernel Hello, On 2026/08/03 20:47, Rafael J. Wysocki (Intel) wrote: > On Mon, Aug 3, 2026 at 12:05 PM KONDO KAZUMA(近藤 和真) > <kazuma-kondo@nec.com> wrote: >> >> Hello, >> >> I received review comments from Sashiko: >> https://sashiko.dev/#/patchset/20260731090820.493064-1-kazuma-kondo@nec.com?part=1 >> >> On 2026/07/31 18:21, sashiko-bot@kernel.org wrote: >>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: >>> - [High] Early return bypasses ASPM disablement for non-PCIe and CXL host bridges without _OSC, causing an undocumented functional change and potential instability >>> -- >>> >>> --- Patch [1]: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC --- >>> commit e68d29f653a1c34f8b7b08baaa548e2ff2d3663f >>> Author: KONDO KAZUMA( $B6aF#!!OB?? (B) <kazuma-kondo@nec.com> >>> >>> ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC >>> >>> This patch skips _OSC negotiation for non-PCIe host bridges that do not define >>> an _OSC method before attempting to evaluate it. It aims to prevent misleading >>> messages in the logs on some Intel servers where the OS appears to negotiate >>> with the platform despite the lack of an _OSC method. >>> >>> Link: https://lore.kernel.org/linux-acpi/d0be949d-0e21-472e-a44b-cedb1dd8695d@nec.com/ >>> >>>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >>>> index 4c06c3ffd0cbb..3a24d018436b4 100644 >>>> --- a/drivers/acpi/pci_root.c >>>> +++ b/drivers/acpi/pci_root.c >>>> @@ -574,6 +574,12 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) >>>> return; >>>> } >>>> >>>> + /* _OSC is optional for PCI host bridges */ >>>> + if (!is_pcie(root) && !acpi_has_method(handle, "_OSC")) { >>>> + dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for non-PCIe host bridge)\n"); >>>> + return; >>>> + } >>>> + >>> >>> [Severity: High] >>> Does this early return unintentionally skip disabling ASPM for these host >>> bridges? >>> >>> The commit message states this change produces no functional issues and only >>> removes misleading messages. However, before this patch, if the _OSC method >>> was missing, the function would proceed, fail the negotiation, and fall >>> through to the failure path where *no_aspm = 1 is unconditionally set. >>> >>> By returning early here, it looks like *no_aspm = 1 is completely bypassed >>> for legacy host bridges. >>> >>> Additionally, is_pcie() only checks for ACPI_BRIDGE_TYPE_PCIE. Could this >>> cause CXL host bridges (ACPI_BRIDGE_TYPE_CXL) to evaluate to false here? >>> If a CXL bridge lacks an _OSC method, it would take this early return and skip >>> ASPM disablement entirely, which could leave ASPM enabled inappropriately >>> without firmware negotiation. >> >> I think Sashiko's review is reasonable. According to [1], unless the >> platform explicitly grants the OS control over ASPM via _OSC, the OS >> should not change the ASPM state configured by the platform. > > Though ASPM only is a thing for PCIe, isn't it? You're right. I was trying to understand the rationale for disabling ASPM when _OSC negotiation is skipped, but it appears that I have been looking at this the wrong way. > >> Also, as described in [2], if CXL 1.1 hosts do not publish a CXL _OSC, >> the OS should fall back to the PCIe _OSC (just as acpi_pci_query_osc() >> does). >> >> So, negotiate_os_control() should skip _OSC negotiation only when a >> legacy PCI host bridge does not have _OSC. For example: >> >> /* _OSC is optional for PCI host bridges */ > > The comment above is somewhat redundant IMO. > >> if (!is_pcie(root) && !is_cxl(root) && >> !acpi_has_method(handle, "_OSC")) { >> dev_dbg(&device->dev, "_OSC: negotiation skipped (method not present for legacy PCI host bridge)\n"); > > Please make the comment say "Skipping a non-PCIe root without _OSC" or similar. > >> >> /* >> * Disable ASPM unless the OS has been granted control over it >> * via _OSC. >> */ >> *no_aspm = 1; > > So I agree that setting *no_aspm to 1 in this case is a good idea, but > the comment above is slightly misleading. Thank you for your advice! I'll fix the debug message and the comment. Thanks, Kazuma > > I would just make it say "No ASPM for non-PCI host bridges" or equivalent. > >> return; >> } >> >> If I have misunderstood anything, please let me know. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-05 11:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-31 9:09 [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC KONDO KAZUMA(近藤 和真)
[not found] ` <20260731092147.B7C531F00A3F@smtp.kernel.org>
2026-08-03 10:05 ` KONDO KAZUMA(近藤 和真)
2026-08-03 11:47 ` Rafael J. Wysocki (Intel)
2026-08-05 11:28 ` KONDO KAZUMA(近藤 和真)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®