Re: [PATCH] ACPI: PCI: Avoid misleading _OSC messages for non-PCIe host bridges without _OSC

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) <[email protected]>

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/[email protected]/

> 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.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1