Re: [PATCH] ACPI: processor_idle: Skip dummy wait for processors based on the Zen microarchitecture

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On 9/22/22 09:54, K Prateek Nayak wrote:
> 
> On 9/22/2022 10:14 PM, Dave Hansen wrote:
>> On 9/20/22 23:36, K Prateek Nayak wrote:
>>> Cc: stable@vger.kernel.org
>>> Cc: regressions@lists.linux.dev
>> *Is* this a regression?
> On second thought, it is not a regression.
> Will remove the tag on v2.

What were you planning for v2?

Rafael suggested something like the attached patch.  It's not nearly as
fragile as the Zen check you proposed earlier.

Any testing or corrections on the commentary would be appreciated.

[-- Attachment #2: 0001-ACPI-processor-idle-Limit-Dummy-wait-workaround-to-o.patch --]
[-- Type: text/x-patch, Size: 2918 bytes --]

From 54e4668122d447ee80ec465f244b19d968c4a7c6 Mon Sep 17 00:00:00 2001
From: Dave Hansen <dave.hansen@intel.com>
Date: Thu, 22 Sep 2022 09:22:26 -0700
Subject: [PATCH] ACPI: processor idle: Limit "Dummy wait" workaround to old
 Intel systems

Old, circa 2002 chipsets have a bug: they don't go idle when they are
supposed to.  So, a workaround was added to slow the CPU down and
ensure that the CPU waits a bit for the chipset to actually go idle.
This workaround is ancient and has been in place in some form since
the original kernel ACPI implementation.

But, this workaround is very painful on modern systems.  The "inb()"
can take thousands of cycles (see Link: for some more detailed
archaeology).

First and foremost, modern systems should not be using this code.
Typical Intel systems have not used it in over a decade because it
is horribly inferior to MWAIT-based idle.

Despite this, people do seem to be tripping over this workaround on
AMD CPUs today.

Limit the "dummy wait" workaround to Intel systems.  Since this
code is only used on ancient Intel systems, this fix should render it
harmless everywhere else, including the modern AMD ones.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reported-by: K Prateek Nayak <kprateek.nayak@amd.com>
Link: https://lore.kernel.org/all/20220921063638.2489-1-kprateek.nayak@amd.com/
---
 drivers/acpi/processor_idle.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 16a1663d02d4..9f40917c49ef 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -531,10 +531,27 @@ static void wait_for_freeze(void)
 	/* No delay is needed if we are in guest */
 	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
 		return;
+	/*
+	 * Modern (>=Nehalem) Intel systems use ACPI via intel_idle,
+	 * not this code.  Assume that any Intel systems using this
+	 * are ancient and may need the dummy wait.  This also assumes
+	 * that the motivating chipset issue was Intel-only.
+	 */
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+		return;
 #endif
-	/* Dummy wait op - must do something useless after P_LVL2 read
-	   because chipsets cannot guarantee that STPCLK# signal
-	   gets asserted in time to freeze execution properly. */
+	/*
+	 * Dummy wait op - must do something useless after P_LVL2 read
+	 * because chipsets cannot guarantee that STPCLK# signal gets
+	 * asserted in time to freeze execution properly
+	 *
+	 * This workaround has been in place since the original ACPI
+	 * implementation was merged, circa 2002.
+	 *
+	 * If a profile is pointing to this instruction, please first
+	 * consider moving your system to a more modern idle
+	 * mechanism.
+	 */
 	inl(acpi_gbl_FADT.xpm_timer_block.address);
 }
 
-- 
2.25.1