[PATCH v2] armv8: implement workaround for broken CNTFRQ_EL0 value

[PATCH v2] armv8: implement workaround for broken CNTFRQ_EL0 value

[Kaustabh Chakraborty]

In devices where the U-Boot is used as a secondary bootloader, we rely
on the device's primary bootloader to implement CNTFRQ_EL0. However,
this reliance may lead to a non-functional timer in broken firmware.

For instance, some versions of Samsung's S-Boot don't implement it. It's
also not possible to set it in the U-Boot, because it's booted in a lower
exception level. CNTFRQ_EL0 is reported to be 0.

Use gd->arch.timer_rate_hz to override the queried value if set. This
setting needs to be done in the board file, preferrably in timer_init().

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
Changes in v2:
- Switched to using gd->arch.timer_rate_hz instead of a new config
  option (Ahmad Fatoum)
- Link to v1: https://lore.kernel.org/r/20251014-armv8-broken-cntfrq-v1-1-b63f9c69ffcb@disroot.org
---
 arch/arm/cpu/armv8/generic_timer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/cpu/armv8/generic_timer.c b/arch/arm/cpu/armv8/generic_timer.c
index 1de7ec596fc7cbbc3e78a241f163bc0a4fcad6b6..1bc72ed26ca31af8e8e72008ed7885cbde23b076 100644
--- a/arch/arm/cpu/armv8/generic_timer.c
+++ b/arch/arm/cpu/armv8/generic_timer.c
@@ -19,6 +19,10 @@ DECLARE_GLOBAL_DATA_PTR;
 unsigned long notrace get_tbclk(void)
 {
 	unsigned long cntfrq;
+
+	if (gd->arch.timer_rate_hz)
+		return gd->arch.timer_rate_hz;
+
 	asm volatile("mrs %0, cntfrq_el0" : "=r" (cntfrq));
 	return cntfrq;
 }

---
base-commit: 582a04763aa80738c1c8ac60c47d1a5159a42833
change-id: 20251014-armv8-broken-cntfrq-f350f68fbf6e

Best regards,
-- 
Kaustabh Chakraborty <kauschluss@disroot.org>

Re: [PATCH v2] armv8: implement workaround for broken CNTFRQ_EL0 value

[Tom Rini]

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

On Fri, Oct 17, 2025 at 08:49:47PM +0530, Kaustabh Chakraborty wrote:

> In devices where the U-Boot is used as a secondary bootloader, we rely
> on the device's primary bootloader to implement CNTFRQ_EL0. However,
> this reliance may lead to a non-functional timer in broken firmware.
> 
> For instance, some versions of Samsung's S-Boot don't implement it. It's
> also not possible to set it in the U-Boot, because it's booted in a lower
> exception level. CNTFRQ_EL0 is reported to be 0.
> 
> Use gd->arch.timer_rate_hz to override the queried value if set. This
> setting needs to be done in the board file, preferrably in timer_init().
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> Changes in v2:
> - Switched to using gd->arch.timer_rate_hz instead of a new config
>   option (Ahmad Fatoum)
> - Link to v1: https://lore.kernel.org/r/20251014-armv8-broken-cntfrq-v1-1-b63f9c69ffcb@disroot.org
> ---
>  arch/arm/cpu/armv8/generic_timer.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/generic_timer.c b/arch/arm/cpu/armv8/generic_timer.c
> index 1de7ec596fc7cbbc3e78a241f163bc0a4fcad6b6..1bc72ed26ca31af8e8e72008ed7885cbde23b076 100644
> --- a/arch/arm/cpu/armv8/generic_timer.c
> +++ b/arch/arm/cpu/armv8/generic_timer.c
> @@ -19,6 +19,10 @@ DECLARE_GLOBAL_DATA_PTR;
>  unsigned long notrace get_tbclk(void)
>  {
>  	unsigned long cntfrq;
> +
> +	if (gd->arch.timer_rate_hz)
> +		return gd->arch.timer_rate_hz;
> +
>  	asm volatile("mrs %0, cntfrq_el0" : "=r" (cntfrq));
>  	return cntfrq;
>  }

This would impact coreprimevelte (as it's the only ARM64 platform to set
gd->arch.timer_rate_hz) but as it's also a Samsung platform it's likely
in the same broken situation your platforms are.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] armv8: implement workaround for broken CNTFRQ_EL0 value

[Kaustabh Chakraborty]

On 2025-10-17 20:40, Tom Rini wrote:
> On Fri, Oct 17, 2025 at 08:49:47PM +0530, Kaustabh Chakraborty wrote:
> 
>> In devices where the U-Boot is used as a secondary bootloader, we rely
>> on the device's primary bootloader to implement CNTFRQ_EL0. However,
>> this reliance may lead to a non-functional timer in broken firmware.
>> 
>> For instance, some versions of Samsung's S-Boot don't implement it. 
>> It's
>> also not possible to set it in the U-Boot, because it's booted in a 
>> lower
>> exception level. CNTFRQ_EL0 is reported to be 0.
>> 
>> Use gd->arch.timer_rate_hz to override the queried value if set. This
>> setting needs to be done in the board file, preferrably in 
>> timer_init().
>> 
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> ---
>> Changes in v2:
>> - Switched to using gd->arch.timer_rate_hz instead of a new config
>>   option (Ahmad Fatoum)
>> - Link to v1: 
>> https://lore.kernel.org/r/20251014-armv8-broken-cntfrq-v1-1-b63f9c69ffcb@disroot.org
>> ---
>>  arch/arm/cpu/armv8/generic_timer.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/arch/arm/cpu/armv8/generic_timer.c 
>> b/arch/arm/cpu/armv8/generic_timer.c
>> index 
>> 1de7ec596fc7cbbc3e78a241f163bc0a4fcad6b6..1bc72ed26ca31af8e8e72008ed7885cbde23b076 
>> 100644
>> --- a/arch/arm/cpu/armv8/generic_timer.c
>> +++ b/arch/arm/cpu/armv8/generic_timer.c
>> @@ -19,6 +19,10 @@ DECLARE_GLOBAL_DATA_PTR;
>>  unsigned long notrace get_tbclk(void)
>>  {
>>  	unsigned long cntfrq;
>> +
>> +	if (gd->arch.timer_rate_hz)
>> +		return gd->arch.timer_rate_hz;
>> +
>>  	asm volatile("mrs %0, cntfrq_el0" : "=r" (cntfrq));
>>  	return cntfrq;
>>  }
> 
> This would impact coreprimevelte (as it's the only ARM64 platform to 
> set
> gd->arch.timer_rate_hz) but as it's also a Samsung platform it's likely
> in the same broken situation your platforms are.

I guess coreprimevelte is a a bit too old to have that issue. Either 
way,
it would be fine as the frequency is set to 26MHz [1], which is the 
timer
frequency of that device [2].

[1] 
https://elixir.bootlin.com/u-boot/v2025.10/source/arch/arm/mach-mmp/board.c#L31
[2] 
https://github.com/LegoLivesMatter/android_kernel_samsung_coreprimevelte/blob/android-5.1/arch/arm64/boot/dts/pxa1908.dtsi#L91

> 
> --
> Tom

Re: [PATCH v2] armv8: implement workaround for broken CNTFRQ_EL0 value

[Tom Rini]

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

On Fri, Oct 17, 2025 at 08:49:47PM +0530, Kaustabh Chakraborty wrote:

> In devices where the U-Boot is used as a secondary bootloader, we rely
> on the device's primary bootloader to implement CNTFRQ_EL0. However,
> this reliance may lead to a non-functional timer in broken firmware.
> 
> For instance, some versions of Samsung's S-Boot don't implement it. It's
> also not possible to set it in the U-Boot, because it's booted in a lower
> exception level. CNTFRQ_EL0 is reported to be 0.
> 
> Use gd->arch.timer_rate_hz to override the queried value if set. This
> setting needs to be done in the board file, preferrably in timer_init().
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> Changes in v2:
> - Switched to using gd->arch.timer_rate_hz instead of a new config
>   option (Ahmad Fatoum)
> - Link to v1: https://lore.kernel.org/r/20251014-armv8-broken-cntfrq-v1-1-b63f9c69ffcb@disroot.org
> ---
>  arch/arm/cpu/armv8/generic_timer.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/generic_timer.c b/arch/arm/cpu/armv8/generic_timer.c
> index 1de7ec596fc7cbbc3e78a241f163bc0a4fcad6b6..1bc72ed26ca31af8e8e72008ed7885cbde23b076 100644
> --- a/arch/arm/cpu/armv8/generic_timer.c
> +++ b/arch/arm/cpu/armv8/generic_timer.c
> @@ -19,6 +19,10 @@ DECLARE_GLOBAL_DATA_PTR;
>  unsigned long notrace get_tbclk(void)
>  {
>  	unsigned long cntfrq;
> +
> +	if (gd->arch.timer_rate_hz)
> +		return gd->arch.timer_rate_hz;
> +
>  	asm volatile("mrs %0, cntfrq_el0" : "=r" (cntfrq));
>  	return cntfrq;
>  }

If we do a size comparison on say imx8mm_evk_fspi, we see:
               u-boot: add: 1/0, grow: 5/0 bytes: 68/0 (68)
                 function                                   old     new   delta
                 get_tbclk                                    -      32     +32
                 timer_get_us                                44      56     +12
                 static.get_timer_us                         44      56     +12
                 udelay                                     116     120      +4
                 static.efi_stall                            88      92      +4
                 cli_readline_into_buffer                  2452    2456      +4
               spl-u-boot-spl: add: 1/0, grow: 3/0 bytes: 52/0 (52)
                 function                                   old     new   delta
                 get_tbclk                                    -      32     +32
                 static.get_timer_us                         44      56     +12
                 timer_get_us                                44      48      +4
                 __udelay                                    64      68      +4

So I think the CONFIG option from before was part of what we want too.
I think this happens late enough that we can do:
#ifdef CONFIG_WHATEVER
	return gd->arch.timer_rate_hz;
#else
	asm(...);
	return cntfrq;
#endif

Yes? Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]