Re: [PATCH] clocksource/drivers/arm_arch_timer: Workaround bcm2712 broken EL2 virtual timer

On 22/07/2026 21:22, Marc Zyngier wrote:
> On Wed, 22 Jul 2026 15:14:39 +0100,
> Jon Hunter <[email protected]> wrote:
>>
>>
>> On 10/07/2026 09:09, Marc Zyngier wrote:
>>> It appears that the bcm2712 SoC found in the relatively popular
>>> RPi5 has a broken EL2 virtual timer.
>>>
>>> We do not know the reason why the timer isn't working (the timer
>>> is ticking, but the interrupt never fires), and the SoC vendor
>>> doesn't communicate on the reason why this isn't working, leaving
>>> users and maintainers in the dark.
>>>
>>> Paper over the issue by detecting the broken HW, falling back to
>>> the physical timer instead, and let the user know about it.
>>> Also taint the kernel as the machine is definitely not compliant
>>> with the spec, and we don't know what else is wrong with it.
>>>
>>> Reported-by: John <[email protected]>
>>> Reported-by: Daniel Drake <[email protected]>
>>> Reported-by: Marek Szyprowski <[email protected]>
>>> Signed-off-by: Marc Zyngier <[email protected]>
>>> Cc: Florian Fainelli <[email protected]>
>>> Cc: Daniel Lezcano <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: Mark Rutland <[email protected]>
>>> ---
>>>    drivers/clocksource/arm_arch_timer.c | 24 +++++++++++++++++++++++-
>>>    1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>>> index 4adf756423de9..7b4a98df6962b 100644
>>> --- a/drivers/clocksource/arm_arch_timer.c
>>> +++ b/drivers/clocksource/arm_arch_timer.c
>>> @@ -1090,6 +1090,27 @@ static int __init arch_timer_common_init(void)
>>>    	return arch_timer_arch_init();
>>>    }
>>>    +static bool __init has_broken_el2_vtimer(void)
>>> +{
>>> +	/*
>>> +	 * SoCs described here have been found to be broken, though no
>>> +	 * explanation has been volunteered by the vendor. Let the user know
>>> +	 * we're papering over the vendor's lack of communication.
>>> +	 */
>>> +	static const char * const broken_el2_vtimer[] __initconst = {
>>> +		"brcm,bcm2712",
>>> +		NULL
>>> +	};
>>> +
>>> +	if (of_machine_compatible_match(broken_el2_vtimer)) {
>>> +		add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
>>> +		pr_warn_once(HW_ERR "Known broken EL2 virtual timer, ignoring it\n");
>>
>> After this change you will now get two warnings; the above and the
>> below. Is this what you want?
> 
> Absolutely.
> 
>>
>>> +		return true;
>>> +	}
>>> +
>>> +	return false;
>>> +}
>>> +
>>>    /**
>>>     * arch_timer_select_ppi() - Select suitable PPI for the current system.
>>>     *
>>> @@ -1115,7 +1136,8 @@ static int __init arch_timer_common_init(void)
>>>    static enum arch_timer_ppi_nr __init arch_timer_select_ppi(void)
>>>    {
>>>    	if (is_kernel_in_hyp_mode()) {
>>> -		if (arch_timer_ppi[ARCH_TIMER_HYP_VIRT_PPI])
>>> +		if (arch_timer_ppi[ARCH_TIMER_HYP_VIRT_PPI] &&
>>> +		    !has_broken_el2_vtimer())
>>>    			return ARCH_TIMER_HYP_VIRT_PPI;
>>>      		pr_warn_once(FW_BUG "VHE-capable CPU without EL2
>>> virtual timer interrupt\n");
>>
>>
>> I have posted something similar for Tegra [0], but because this is not
>> expected to work, I wanted to avoid the warnings here. We test for
> 
> "not expected to work"? In which parallel universe is that a thing?

FWIU, at least for Tegra194, we have a CPU and GIC pairing where the CPU 
supports this but the GIC does not.

>> kernel warnings and ideally we would not warn if is known not to
>> work. We could always display an info level print if it is needed.
> 
> No. These warnings are required because the HW is broken, and violates
> the basics of the architecture, which the kernel relies on. That's
> important information that needs to be captured, and that's why the
> kernel also gets tainted.
> 
> This applies to any implementation that hasn't been bothered to follow
> the spec. Don't worry, you're in good company.

Well Tegra194 does not appear to have, but Tegra234 does (but we have a 
firmware issue which should be easy to fix but the current released 
firmware as this issue). I have also checked Tegra264 and that should be 
following the spec too.

Jon

-- 
nvpublic