Re: [PATCH] acpi: pci_root: Add quirks table for _OSC support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch bypasses the global ASPM disablement path, causing the OS to illegally configure ASPM registers on the root bridge without acquiring _OSC control from the firmware.
--

--- Patch [1]: [PATCH] acpi: pci_root: Add quirks table for _OSC support ---
commit ae1792c3abcd3cde32bf0136c38058905b107a50
Author: Derek J. Clark <derekjohn.clark@gmail.com>

acpi: pci_root: Add quirks table for _OSC support

This patch introduces a quirk table to dynamically strip specific _OSC
support bits (like ASPM) from certain root complexes. It aims to fix a
hard lock on s2idle resume for the MSI Claw A8 by preventing the OS
from requesting _OSC control when an SD card is present.

> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 84030804a7633..22e97d4064c61 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c

[ ... ]

> @@ -451,6 +513,8 @@ static u32 calculate_support(void)
>  	if (IS_ENABLED(CONFIG_PCIE_EDR))
>  		support |= OSC_PCI_EDR_SUPPORT;
>  
> +	support &= pci_osc_support_quirk_mask(root);
> +
[Severity: High]
Does stripping the ASPM support bit here bypass the global ASPM disablement
path?

When this quirk strips OSC_PCI_ASPM_SUPPORT, os_control_query_checks() will
fail because the mask lacks ACPI_PCIE_REQ_SUPPORT:

drivers/acpi/pci_root.c:os_control_query_checks() {
	if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
		decode_osc_support(root, "not requesting OS control; OS requires",
				   ACPI_PCIE_REQ_SUPPORT);
		return false;
	}
	...
}

This causes the requested control to default to 0. When
acpi_pci_osc_control_set() is called with ctrl = 0, it hits an early exit and
returns AE_OK:

drivers/acpi/pci_root.c:acpi_pci_osc_control_set() {
	/* No need to request _OSC if the control was already granted. */
	if ((root->osc_control_set & ctrl) == ctrl &&
	    (root->osc_ext_control_set & cxl_ctrl) == cxl_ctrl)
		return AE_OK;
	...
}

Because AE_OK is returned, negotiate_os_control() treats this as a success
and bypasses the failure path that sets *no_aspm = 1:

drivers/acpi/pci_root.c:negotiate_os_control() {
	status = acpi_pci_osc_control_set(handle, &control, support,
					  &cxl_control, cxl_support);
	if (ACPI_SUCCESS(status)) {
		...
	} else {
		*no_aspm = 1;
		...
	}
}

With the global aspm_disabled flag remaining false, won't the ASPM core
proceed to actively configure ASPM on the root bridge in
pcie_aspm_init_link_state() without actually acquiring _OSC control from the
firmware?

>  	return support;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803203459.10680-1-derekjohn.clark@gmail.com?part=1