[PATCH 0/2] Add a workaround for s2idle resume on Lenovo laptops

[PATCH 0/2] Add a workaround for s2idle resume on Lenovo laptops

[Mario Limonciello]

Lenovo Thinkpads have a SMI handler during the D0 transition for NVME
devices specifically during resume from s2idle.  When the IOMMU
translation layer is enabled for NVME devices (which is the default
behavior per the IVRS table), then this SMI handler causes a very long
resume time (10+ seconds).

For the common s2idle circumstance on Linux this SMI handler is
unnecessary and just significantly inflates resume time. To avoid it,
add a new s2idle resume handler to thinkpad_acpi that will prevent it
from running on known problematic systems.  If the SMI handler is fixed on
these systems the DMI data can be modified to exclude them or only match
problematic BIOS versions.

Mario Limonciello (2):
  platform/x86: thinkpad_acpi: Convert btusb DMI list to quirks
  platform/x86: thinkpad_acpi: Add a s2idle resume quirk for a number of
    laptops

 drivers/platform/x86/thinkpad_acpi.c | 134 ++++++++++++++++++++++++++-
 1 file changed, 131 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/2] platform/x86: thinkpad_acpi: Convert btusb DMI list to quirks

[Mario Limonciello]

DMI matching in thinkpad_acpi happens local to a function meaning
quirks can only match that function.

Future changes to thinkpad_acpi may need to quirk other code, so
change this to use a quirk infrastructure.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index c568fae56db2..f49f59130c3c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -309,6 +309,15 @@ struct ibm_init_struct {
 	struct ibm_struct *data;
 };
 
+/* DMI Quirks */
+struct quirk_entry {
+	bool btusb_bug;
+};
+
+struct quirk_entry quirk_btusb_bug = {
+	.btusb_bug = true,
+};
+
 static struct {
 	u32 bluetooth:1;
 	u32 hotkey:1;
@@ -338,6 +347,7 @@ static struct {
 	u32 hotkey_poll_active:1;
 	u32 has_adaptive_kbd:1;
 	u32 kbd_lang:1;
+	struct quirk_entry *quirks;
 } tp_features;
 
 static struct {
@@ -4359,9 +4369,10 @@ static void bluetooth_exit(void)
 	bluetooth_shutdown();
 }
 
-static const struct dmi_system_id bt_fwbug_list[] __initconst = {
+static const struct dmi_system_id fwbug_list[] __initconst = {
 	{
 		.ident = "ThinkPad E485",
+		.driver_data = &quirk_btusb_bug,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_MATCH(DMI_BOARD_NAME, "20KU"),
@@ -4369,6 +4380,7 @@ static const struct dmi_system_id bt_fwbug_list[] __initconst = {
 	},
 	{
 		.ident = "ThinkPad E585",
+		.driver_data = &quirk_btusb_bug,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_MATCH(DMI_BOARD_NAME, "20KV"),
@@ -4376,13 +4388,16 @@ static const struct dmi_system_id bt_fwbug_list[] __initconst = {
 	},
 	{
 		.ident = "ThinkPad A285 - 20MW",
+		.driver_data = &quirk_btusb_bug,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_MATCH(DMI_BOARD_NAME, "20MW"),
 		},
+
 	},
 	{
 		.ident = "ThinkPad A285 - 20MX",
+		.driver_data = &quirk_btusb_bug,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_MATCH(DMI_BOARD_NAME, "20MX"),
@@ -4390,6 +4405,7 @@ static const struct dmi_system_id bt_fwbug_list[] __initconst = {
 	},
 	{
 		.ident = "ThinkPad A485 - 20MU",
+		.driver_data = &quirk_btusb_bug,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_MATCH(DMI_BOARD_NAME, "20MU"),
@@ -4397,6 +4413,7 @@ static const struct dmi_system_id bt_fwbug_list[] __initconst = {
 	},
 	{
 		.ident = "ThinkPad A485 - 20MV",
+		.driver_data = &quirk_btusb_bug,
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_MATCH(DMI_BOARD_NAME, "20MV"),
@@ -4419,7 +4436,8 @@ static int __init have_bt_fwbug(void)
 	 * Some AMD based ThinkPads have a firmware bug that calling
 	 * "GBDC" will cause bluetooth on Intel wireless cards blocked
 	 */
-	if (dmi_check_system(bt_fwbug_list) && pci_dev_present(fwbug_cards_ids)) {
+	if (tp_features.quirks && tp_features.quirks->btusb_bug &&
+	    pci_dev_present(fwbug_cards_ids)) {
 		vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
 			FW_BUG "disable bluetooth subdriver for Intel cards\n");
 		return 1;
@@ -11496,6 +11514,7 @@ static void thinkpad_acpi_module_exit(void)
 
 static int __init thinkpad_acpi_module_init(void)
 {
+	const struct dmi_system_id *dmi_id;
 	int ret, i;
 
 	tpacpi_lifecycle = TPACPI_LIFE_INIT;
@@ -11623,7 +11642,10 @@ static int __init thinkpad_acpi_module_init(void)
 		tp_features.input_device_registered = 1;
 	}
 
-	return 0;
+	dmi_id = dmi_first_match(fwbug_list);
+	if (dmi_id)
+		tp_features.quirks = dmi_id->driver_data;
+	return ret;
 }
 
 MODULE_ALIAS(TPACPI_DRVR_SHORTNAME);
-- 
2.34.1

[PATCH 2/2] platform/x86: thinkpad_acpi: Add a s2idle resume quirk for a number of laptops

[Mario Limonciello]

Lenovo laptops that contain NVME SSDs across a variety of generations have
trouble resuming from suspend to idle when the IOMMU translation layer is
active for the NVME storage device.

This generally manifests as a large resume delay or page faults. These
delays and page faults occur as a result of a Lenovo BIOS specific SMI
that runs during the D3->D0 transition on NVME devices.

This SMI occurs because of a flag that is set during resume by Lenovo
firmware:

```
OperationRegion (PM80, SystemMemory, 0xFED80380, 0x10)
Field (PM80, AnyAcc, NoLock, Preserve)
{
	SI3R,   1
}

Method (_ON, 0, NotSerialized)  // _ON_: Power On
{
	TPST (0x60D0)
	If ((DAS3 == 0x00))
	{
		If (SI3R)
		{
			TPST (0x60E0)
			M020 (NBRI, 0x00, 0x00, 0x04, (NCMD | 0x06))
			M020 (NBRI, 0x00, 0x00, 0x10, NBAR)
			APMC = HDSI /* \HDSI */
			SLPS = 0x01
			SI3R = 0x00
			TPST (0x60E1)
		}
		D0NV = 0x01
	}
}
```

Create a quirk that will run early in the resume process to prevent this
SMI from running. As any of these machines are fixed, they can be peeled
back from this quirk or narrowed down to individual firmware versions.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1910
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1689
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 108 ++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index f49f59130c3c..9161f356fcdd 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -312,12 +312,17 @@ struct ibm_init_struct {
 /* DMI Quirks */
 struct quirk_entry {
 	bool btusb_bug;
+	u32 s2idle_bug_mmio;
 };
 
 struct quirk_entry quirk_btusb_bug = {
 	.btusb_bug = true,
 };
 
+struct quirk_entry quirk_s2idle_bug = {
+	.s2idle_bug_mmio = 0xfed80380,
+};
+
 static struct {
 	u32 bluetooth:1;
 	u32 hotkey:1;
@@ -4419,9 +4424,101 @@ static const struct dmi_system_id fwbug_list[] __initconst = {
 			DMI_MATCH(DMI_BOARD_NAME, "20MV"),
 		},
 	},
+	{
+		.ident = "L14 Gen2 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20X5"),
+		}
+	},
+	{
+		.ident = "T14s Gen2 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20XF"),
+		}
+	},
+	{
+		.ident = "X13 Gen2 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20XH"),
+		}
+	},
+	{
+		.ident = "T14 Gen2 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20XK"),
+		}
+	},
+	{
+		.ident = "T14 Gen1 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20UD"),
+		}
+	},
+	{
+		.ident = "P14s Gen1 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20Y1"),
+		}
+	},
+	{
+		.ident = "P14s Gen2 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "21A0"),
+		}
+	},
 	{}
 };
 
+/*
+ * Lenovo laptops from a variety of generations run a SMI handler during the D3->D0
+ * transition that occurs specifically when exiting suspend to idle which can cause
+ * large delays during resume when the IOMMU translation layer is enabled (the default
+ * behavior) for NVME devices:
+ *
+ * To avoid this firmware problem, skip the SMI handler on these machines before the
+ * D0 transition occurs.
+ */
+static void thinkpad_acpi_amd_s2idle_restore(void)
+{
+	struct resource *res;
+	void __iomem *addr;
+	u8 val;
+
+	res = request_mem_region_muxed(tp_features.quirks->s2idle_bug_mmio, 1,
+					"thinkpad_acpi_pm80");
+	if (!res)
+		return;
+
+	addr = ioremap(tp_features.quirks->s2idle_bug_mmio, 1);
+	if (!addr)
+		goto cleanup_resource;
+
+	val = ioread8(addr);
+	iowrite8(val & ~BIT(0), addr);
+
+	iounmap(addr);
+cleanup_resource:
+	release_resource(res);
+}
+
+static struct acpi_s2idle_dev_ops thinkpad_acpi_s2idle_dev_ops = {
+	.restore = thinkpad_acpi_amd_s2idle_restore,
+};
+
 static const struct pci_device_id fwbug_cards_ids[] __initconst = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x24F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x24FD) },
@@ -11473,6 +11570,8 @@ static void thinkpad_acpi_module_exit(void)
 
 	tpacpi_lifecycle = TPACPI_LIFE_EXITING;
 
+	if (tp_features.quirks && tp_features.quirks->s2idle_bug_mmio)
+		acpi_unregister_lps0_dev(&thinkpad_acpi_s2idle_dev_ops);
 	if (tpacpi_hwmon)
 		hwmon_device_unregister(tpacpi_hwmon);
 	if (tp_features.sensors_pdrv_registered)
@@ -11643,8 +11742,15 @@ static int __init thinkpad_acpi_module_init(void)
 	}
 
 	dmi_id = dmi_first_match(fwbug_list);
-	if (dmi_id)
+	if (dmi_id) {
 		tp_features.quirks = dmi_id->driver_data;
+		if (tp_features.quirks->s2idle_bug_mmio) {
+			if (!acpi_register_lps0_dev(&thinkpad_acpi_s2idle_dev_ops))
+				pr_info("Using s2idle quirk to avoid %s platform firmware bug\n",
+					dmi_id->ident ? dmi_id->ident : "");
+		}
+	}
+
 	return ret;
 }
 
-- 
2.34.1

Re: [External] [PATCH 2/2] platform/x86: thinkpad_acpi: Add a s2idle resume quirk for a number of laptops

[Mark Pearson]

On 4/28/22 23:05, Mario Limonciello wrote:
> Lenovo laptops that contain NVME SSDs across a variety of generations have
> trouble resuming from suspend to idle when the IOMMU translation layer is
> active for the NVME storage device.
> 
> This generally manifests as a large resume delay or page faults. These
> delays and page faults occur as a result of a Lenovo BIOS specific SMI
> that runs during the D3->D0 transition on NVME devices.
> 
> This SMI occurs because of a flag that is set during resume by Lenovo
> firmware:
> 
> ```
> OperationRegion (PM80, SystemMemory, 0xFED80380, 0x10)
> Field (PM80, AnyAcc, NoLock, Preserve)
> {
> 	SI3R,   1
> }
> 
> Method (_ON, 0, NotSerialized)  // _ON_: Power On
> {
> 	TPST (0x60D0)
> 	If ((DAS3 == 0x00))
> 	{
> 		If (SI3R)
> 		{
> 			TPST (0x60E0)
> 			M020 (NBRI, 0x00, 0x00, 0x04, (NCMD | 0x06))
> 			M020 (NBRI, 0x00, 0x00, 0x10, NBAR)
> 			APMC = HDSI /* \HDSI */
> 			SLPS = 0x01
> 			SI3R = 0x00
> 			TPST (0x60E1)
> 		}
> 		D0NV = 0x01
> 	}
> }
> ```
> 
> Create a quirk that will run early in the resume process to prevent this
> SMI from running. As any of these machines are fixed, they can be peeled
> back from this quirk or narrowed down to individual firmware versions.
> 
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1910>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1689>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Thanks Mario,

Tested the series on the T14 G1 AMD and it fixes the long delay when
resuming from S0ix suspend nicely.

Patches look good to me. I am working with the Lenovo FW teams for the
various platforms, but as S0ix support wasn't in the original scope I
don't know yet when (or if in some cases) they'll get FW fixes released.
I believe these changes are benign even once the FW is fixed so think
this is a great idea.

I'll push updates removing platform quirks once FW is released though.

Tested-by: Mark Pearson <markpearson@lenvo.com>

Re: [PATCH 0/2] Add a workaround for s2idle resume on Lenovo laptops

[Hans de Goede]

Hi,

On 4/29/22 05:04, Mario Limonciello wrote:
> Lenovo Thinkpads have a SMI handler during the D0 transition for NVME
> devices specifically during resume from s2idle.  When the IOMMU
> translation layer is enabled for NVME devices (which is the default
> behavior per the IVRS table), then this SMI handler causes a very long
> resume time (10+ seconds).
> 
> For the common s2idle circumstance on Linux this SMI handler is
> unnecessary and just significantly inflates resume time. To avoid it,
> add a new s2idle resume handler to thinkpad_acpi that will prevent it
> from running on known problematic systems.  If the SMI handler is fixed on
> these systems the DMI data can be modified to exclude them or only match
> problematic BIOS versions.
> 
> Mario Limonciello (2):
>   platform/x86: thinkpad_acpi: Convert btusb DMI list to quirks
>   platform/x86: thinkpad_acpi: Add a s2idle resume quirk for a number of
>     laptops
> 
>  drivers/platform/x86/thinkpad_acpi.c | 134 ++++++++++++++++++++++++++-
>  1 file changed, 131 insertions(+), 3 deletions(-)

Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

I'll also cherry-pick these into the pdx86 fixes branch and add them
to my next 5.18 fixes pull-req to Linus.

Regards,

Hans