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

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

[Derek J. Clark]

The MSI Claw A8 hard-locks on resume from s2idle whenever an SD/MMC
card is present in the RTS525A card reader (10ec:525a, PCI ID). This
issue is not present on the Lenovo Legion Go with the same card reader.
The primary difference is that the Claw A8 withholds LTR and DPC while
granting ASPM control to the OS, leading to a split ownership. The issue
can be mitigated by passing pcie_asmp=off, but quirking on the pci_dev
in pci/quirks has no effect.

Attempted quirks included use of pci_disable_link_state(),
dev->link_state = NULL, manually zeroing aspm_l0s_support/aspm_l1_support
via DECLARE_PCI_FIXUP_FINAL, and pcie_aspm_remove_cap() on
DECLARE_PCI_FIXUP_HEADER. Only by disabling ASPM on the root hub is the
system able to resume successfully. This strongly suggests a race between
the OS resuming the link under its own ASPM assumptions and firmware
independently acting on the same link based on state it never
relinquished.

As an attempt to localize the workaround to as low level as possible
while also being effective, introduce a quirk system to the existing
acpi pci_root calculate_support() mechanism.

Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/acpi/pci_root.c | 68 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 6f78f96332ea..abd436a10438 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -422,7 +422,69 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
 	return AE_OK;
 }
 
-static u32 calculate_support(void)
+/*
+ * Some platforms advertise ASPM support in _OSC but withhold related
+ * control (e.g. LTR, DPC) from the OS on a specific root complex. The
+ * resulting split ownership between OS-managed ASPM and firmware-owned
+ * LTR/DPC can cause resume failures on s2idle. Rather than disabling
+ * ASPM system-wide, strip the offending support bits only on the
+ * affected root bridge so the OS abstains from requesting _OSC control
+ * there, leaving every other root complex on the system unaffected.
+ */
+struct osc_support_quirk {
+	const struct dmi_system_id dmi_match[2];
+	u16 segment;
+	u8 bus;
+	u32 strip_support;
+};
+
+static const struct osc_support_quirk osc_support_quirks[] = {
+	/*
+	 * MSI Claw A8 (MS-1T8K): firmware withholds LTR/DPC control on
+	 * the primary root complex despite advertising ASPM support,
+	 * causing a hard lock on s2idle resume when the onboard RTS525A
+	 * SD card reader is populated.
+	 */
+	{
+		.dmi_match = {
+			{
+				.ident = "MSI Claw A8 BZ2EM",
+				.matches = {
+					DMI_MATCH(DMI_SYS_VENDOR,
+						  "Micro-Star International Co., Ltd."),
+					DMI_MATCH(DMI_BOARD_NAME, "MS-1T8K"),
+				},
+			},
+			{}
+		},
+		.segment = 0,
+		.bus = 0,
+		.strip_support = OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT,
+	},
+};
+
+static u32 pci_osc_support_quirk_mask(struct acpi_pci_root *root)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(osc_support_quirks); i++) {
+		const struct osc_support_quirk *q = &osc_support_quirks[i];
+
+		if (dmi_first_match(q->dmi_match) &&
+		    root->segment == q->segment &&
+		    root->secondary.start == q->bus) {
+			dev_info(&root->device->dev,
+				 "PCI Root Bridge [%04x:%02x] _OSC quirk: stripping support 0x%08x (%s)\n",
+				 root->segment, (unsigned int)root->secondary.start,
+				 q->strip_support, q->dmi_match[0].ident);
+			return ~q->strip_support;
+		}
+	}
+
+	return ~0;
+}
+
+static u32 calculate_support(struct acpi_pci_root *root)
 {
 	u32 support;
 
@@ -441,6 +503,8 @@ static u32 calculate_support(void)
 	if (IS_ENABLED(CONFIG_PCIE_EDR))
 		support |= OSC_PCI_EDR_SUPPORT;
 
+	support &= pci_osc_support_quirk_mask(root);
+
 	return support;
 }
 
@@ -571,7 +635,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
 		return;
 	}
 
-	support = calculate_support();
+	support = calculate_support(root);
 
 	decode_osc_support(root, "OS supports", support);
 
-- 
2.55.0

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

[sashiko-bot]

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