Re: [REGRESSION] ? system is stuck in clocksource, >60s delay at boot time without tsc=unstable

Re: [REGRESSION] ? system is stuck in clocksource, >60s delay at boot time without tsc=unstable

[Thomas Gleixner]

Fab!

On Sun, Feb 23 2025 at 18:01, Fab Stz wrote:
> Le 15/01/2025 à 17:59, Thomas Gleixner a écrit :
>> AFAICT, that iMac 9.1 is Core 2 Duo based and that generation of
>> processors definitely had issues with the TSC in deeper idle states.
>
> Indeed, cpuinfo reports:
>
> model name      : Intel(R) Core(TM)2 Duo CPU     E8135  @ 2.66GHz

Ok.

>>> BTW, I tried the "processor.max_cstate=1" you mentioned but it didn't
>>> change anything on the delay and/or warning.
>> 
>> That's weird, but we have no idea what kind of magic the BIOS implements
>> there for power management behind the kernels back. I assume that it
>> does because this generation of CPUs uses the ACPI processor idle driver
>> and that disables TSC when it detects that the system supports
>> C-states > 1.
>
> Output of these commands can be found in attached file cpuidle.txt

> + cat /sys/devices/system/cpu/cpuidle/current_driver
> intel_idle

So according to that the intel_idle driver is in use, which does not
have the magic TSC workarounds like the acpi processor driver has, but
it seems to be loaded preferred.

Sigh. Why is the intel_idle driver so agressive in taking over despite
the fact that it does not handle the old CPUs, which are known to
require the TSC workaround? It handles the APIC stops in C2, but not the
TSC oddity while the original ACPI processor_idle driver does the right
thing for more than two decades....

> Can the kernel be patched so that the proper config is used 
> automatically (ie. without the user having to set any parameter)? I'm 
> not sure my question actually makes sense.

Yes, we can. Untested patch below. It just brings the intel idle driver
on par with the original ACPI processor idle driver to deal with that
problem.

Thanks,

        tglx
---
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 118fe1d37c22..0fdb1d1316c4 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -56,6 +56,7 @@
 #include <asm/intel-family.h>
 #include <asm/mwait.h>
 #include <asm/spec-ctrl.h>
+#include <asm/tsc.h>
 #include <asm/fpu/api.h>
 
 #define INTEL_IDLE_VERSION "0.5.1"
@@ -1799,6 +1800,9 @@ static void __init intel_idle_init_cstates_acpi(struct cpuidle_driver *drv)
 		if (intel_idle_state_needs_timer_stop(state))
 			state->flags |= CPUIDLE_FLAG_TIMER_STOP;
 
+		if (cx->type > ACPI_STATE_C1 && !boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+			mark_tsc_unstable("TSC halts in idle");
+
 		state->enter = intel_idle;
 		state->enter_s2idle = intel_idle_s2idle;
 	}

Re: [REGRESSION] ? system is stuck in clocksource, >60s delay at boot time without tsc=unstable

[Fab Stz]

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

Hello Thomas,

Thank you for the patch! I built the 6.1 kernel with it applied and it 
apparently works as expected (no delay). Please find logs below & 
attached dmesg log. Maybe the interesting line is:

Feb 25 08:53:51 debian kernel: tsc: Marking TSC unstable due to TSC 
halts in idle

Comparison shows that it is "Marking TSC unstable due to boot parameter" 
with a non patched kernel + tsc=unstable.

+ cat /sys/devices/system/cpu/cpuidle/available_governors
ladder menu
+ cat /sys/devices/system/cpu/cpuidle/current_driver
intel_idle
+ cat /sys/devices/system/cpu/cpuidle/current_governor
menu
+ cat /sys/devices/system/cpu/cpuidle/current_governor_ro
menu
+ ls /sys/devices/system/cpu/cpu0/cpuidle/
state0  state1  state2  state3
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state0/name
POLL
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state0/disable
0
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state1/name
C1_ACPI
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state1/disable
0
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state2/name
C2_ACPI
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state2/disable
0
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state3/name
C3_ACPI
+ cat /sys/devices/system/cpu/cpu0/cpuidle/state3/disable
0


Will the patch also enter the longterm releases like 6.1?

Regards
Fab


Le 24/02/2025 à 09:13, Thomas Gleixner a écrit :
>>>> BTW, I tried the "processor.max_cstate=1" you mentioned but it didn't
>>>> change anything on the delay and/or warning.
>>>
>>> That's weird, but we have no idea what kind of magic the BIOS implements
>>> there for power management behind the kernels back. I assume that it
>>> does because this generation of CPUs uses the ACPI processor idle driver
>>> and that disables TSC when it detects that the system supports
>>> C-states > 1.
>>
>> Output of these commands can be found in attached file cpuidle.txt
> 
>> + cat /sys/devices/system/cpu/cpuidle/current_driver
>> intel_idle
> 
> So according to that the intel_idle driver is in use, which does not
> have the magic TSC workarounds like the acpi processor driver has, but
> it seems to be loaded preferred.
> 
> Sigh. Why is the intel_idle driver so agressive in taking over despite
> the fact that it does not handle the old CPUs, which are known to
> require the TSC workaround? It handles the APIC stops in C2, but not the
> TSC oddity while the original ACPI processor_idle driver does the right
> thing for more than two decades....
> 
>> Can the kernel be patched so that the proper config is used
>> automatically (ie. without the user having to set any parameter)? I'm
>> not sure my question actually makes sense.
> 
> Yes, we can. Untested patch below. It just brings the intel idle driver
> on par with the original ACPI processor idle driver to deal with that
> problem.
> 
> Thanks,
> 
>          tglx
> ---
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 118fe1d37c22..0fdb1d1316c4 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -56,6 +56,7 @@
>   #include <asm/intel-family.h>
>   #include <asm/mwait.h>
>   #include <asm/spec-ctrl.h>
> +#include <asm/tsc.h>
>   #include <asm/fpu/api.h>
>   
>   #define INTEL_IDLE_VERSION "0.5.1"
> @@ -1799,6 +1800,9 @@ static void __init intel_idle_init_cstates_acpi(struct cpuidle_driver *drv)
>   		if (intel_idle_state_needs_timer_stop(state))
>   			state->flags |= CPUIDLE_FLAG_TIMER_STOP;
>   
> +		if (cx->type > ACPI_STATE_C1 && !boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
> +			mark_tsc_unstable("TSC halts in idle");
> +
>   		state->enter = intel_idle;
>   		state->enter_s2idle = intel_idle_s2idle;
>   	}


[-- Attachment #2: patched.log --]
[-- Type: text/x-log, Size: 2020 bytes --]

Feb 25 08:53:50 debian kernel: Command line: root=UUID=462f57b4-136e-4c18-8c55-e4bc59cfb7aa ro  zswap.enabled=1 mem_sleep_default=deep single initrd=\boot\initrd.img-6.1.0-0.a.test-amd64                                     
Feb 25 08:53:50 debian kernel: tsc: Fast TSC calibration using PIT
Feb 25 08:53:50 debian kernel: tsc: Detected 2653.363 MHz processor
Feb 25 08:53:50 debian kernel: clocksource: refined-jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645519600211568 ns
Feb 25 08:53:50 debian kernel: Kernel command line: root=UUID=462f57b4-136e-4c18-8c55-e4bc59cfb7aa ro  zswap.enabled=1 mem_sleep_default=deep single initrd=\boot\initrd.img-6.1.0-0.a.test-amd64                                     
Feb 25 08:53:50 debian kernel: Unknown kernel command line parameters "single", will be passed to user space.
Feb 25 08:53:50 debian kernel: clocksource: hpet: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 76450417870 ns
Feb 25 08:53:50 debian kernel: clocksource: tsc-early: mask: 0xffffffffffffffff max_cycles: 0x263f25ec687, max_idle_ns: 440795217651 ns
Feb 25 08:53:50 debian kernel: clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
Feb 25 08:53:51 debian kernel: clocksource: Switched to clocksource tsc-early
Feb 25 08:53:51 debian kernel: clocksource: acpi_pm: mask: 0xffffff max_cycles: 0xffffff, max_idle_ns: 2085701024 ns
Feb 25 08:53:51 debian kernel: tsc: Refined TSC clocksource calibration: 2653.335 MHz
Feb 25 08:53:51 debian kernel: clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x263f0bda6a8, max_idle_ns: 440795254345 ns
Feb 25 08:53:51 debian kernel: clocksource: Switched to clocksource tsc
Feb 25 08:53:51 debian kernel: tsc: Marking TSC unstable due to TSC halts in idle
Feb 25 08:53:51 debian kernel: clocksource: Checking clocksource tsc synchronization from CPU 1 to CPUs 0.
Feb 25 08:53:51 debian kernel: clocksource: Switched to clocksource hpet
                               on the kernel command line

Re: [REGRESSION] ? system is stuck in clocksource, >60s delay at boot time without tsc=unstable

[Thomas Gleixner]

On Tue, Feb 25 2025 at 09:11, Fab Stz wrote:
> Thank you for the patch! I built the 6.1 kernel with it applied and it 
> apparently works as expected (no delay). Please find logs below & 
> attached dmesg log. Maybe the interesting line is:
>
> Feb 25 08:53:51 debian kernel: tsc: Marking TSC unstable due to TSC 
> halts in idle

Correct. That's the missing piece.

> Will the patch also enter the longterm releases like 6.1?

Let me write a proper change log with a Fixes tag and cc stable, so it
will be automatically picked up.

Thanks,

        tglx

[PATCH] intel_idle: Handle older CPUs, which stop the TSC in deeper C states, correctly

[Thomas Gleixner]

The Intel idle driver is preferred over the ACPI processor idle driver,
but fails to implement the work around for Core2 generation CPUs, where
the TSC stops in C2 and deeper C-states. This causes stalls and boot
delays, when the clocksource watchdog does not catch the unstable TSC
before the CPU goes deep idle for the first time.

The ACPI driver marks the TSC unstable when it detects that the CPU
supports C2 or deeper and the CPU does not have a non-stop TSC.

Add the equivivalent work around to the Intel idle driver to cure that.

Fixes: 18734958e9bf ("intel_idle: Use ACPI _CST for processor models without C-state tables")
Reported-by: Fab Stz <fabstz-it@yahoo.fr>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Fab Stz <fabstz-it@yahoo.fr>
Cc: stable@vger.kernel.org
Closes: https://lore.kernel.org/all/10cf96aa-1276-4bd4-8966-c890377030c3@yahoo.fr
---
 drivers/idle/intel_idle.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -56,6 +56,7 @@
 #include <asm/intel-family.h>
 #include <asm/mwait.h>
 #include <asm/spec-ctrl.h>
+#include <asm/tsc.h>
 #include <asm/fpu/api.h>
 
 #define INTEL_IDLE_VERSION "0.5.1"
@@ -1799,6 +1800,9 @@ static void __init intel_idle_init_cstat
 		if (intel_idle_state_needs_timer_stop(state))
 			state->flags |= CPUIDLE_FLAG_TIMER_STOP;
 
+		if (cx->type > ACPI_STATE_C1 && !boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+			mark_tsc_unstable("TSC halts in idle");
+
 		state->enter = intel_idle;
 		state->enter_s2idle = intel_idle_s2idle;
 	}

Re: [PATCH] intel_idle: Handle older CPUs, which stop the TSC in deeper C states, correctly

[Rafael J. Wysocki]

On Tue, Feb 25, 2025 at 11:37 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The Intel idle driver is preferred over the ACPI processor idle driver,
> but fails to implement the work around for Core2 generation CPUs, where
> the TSC stops in C2 and deeper C-states. This causes stalls and boot
> delays, when the clocksource watchdog does not catch the unstable TSC
> before the CPU goes deep idle for the first time.
>
> The ACPI driver marks the TSC unstable when it detects that the CPU
> supports C2 or deeper and the CPU does not have a non-stop TSC.
>
> Add the equivivalent work around to the Intel idle driver to cure that.
>
> Fixes: 18734958e9bf ("intel_idle: Use ACPI _CST for processor models without C-state tables")
> Reported-by: Fab Stz <fabstz-it@yahoo.fr>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Fab Stz <fabstz-it@yahoo.fr>
> Cc: stable@vger.kernel.org
> Closes: https://lore.kernel.org/all/10cf96aa-1276-4bd4-8966-c890377030c3@yahoo.fr
> ---
>  drivers/idle/intel_idle.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -56,6 +56,7 @@
>  #include <asm/intel-family.h>
>  #include <asm/mwait.h>
>  #include <asm/spec-ctrl.h>
> +#include <asm/tsc.h>
>  #include <asm/fpu/api.h>
>
>  #define INTEL_IDLE_VERSION "0.5.1"
> @@ -1799,6 +1800,9 @@ static void __init intel_idle_init_cstat
>                 if (intel_idle_state_needs_timer_stop(state))
>                         state->flags |= CPUIDLE_FLAG_TIMER_STOP;
>
> +               if (cx->type > ACPI_STATE_C1 && !boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
> +                       mark_tsc_unstable("TSC halts in idle");
> +
>                 state->enter = intel_idle;
>                 state->enter_s2idle = intel_idle_s2idle;
>         }

Applied as a fix for 6.14-rc5, thank you!