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

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 O=
S to illegally configure ASPM registers on the root bridge without acquirin=
g _OSC control from the firmware.
--

--- Patch [1]: [PATCH] acpi: pci_root: Add quirks table for _OSC support ---
commit ae1792c3abcd3cde32bf0136c38058905b107a50
Author: Derek J. Clark <[email protected]>

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 |=3D OSC_PCI_EDR_SUPPORT;
> =20
> +	support &=3D 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) !=3D 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 =3D 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) =3D=3D ctrl &&
	    (root->osc_ext_control_set & cxl_ctrl) =3D=3D 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 =3D 1:

drivers/acpi/pci_root.c:negotiate_os_control() {
	status =3D acpi_pci_osc_control_set(handle, &control, support,
					  &cxl_control, cxl_support);
	if (ACPI_SUCCESS(status)) {
		...
	} else {
		*no_aspm =3D 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;
>  }
>

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803203459.1068=
[email protected]?part=3D1