mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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

* 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®