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

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

[Jon Hunter]

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 <therealgraysky@proton.me>
> Reported-by: Daniel Drake <dan@reactivated.net>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: Florian Fainelli <florian.fainelli@broadcom.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>   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?

> +		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 
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.

Jon

[0] 
https://lore.kernel.org/linux-tegra/20260722-cordial-zippy-oxpecker-dcdabb@sudeepholla/T/#m0c203a9b743500f0d1e728e80d1385d829844e3f
-- 
nvpublic

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

[Marc Zyngier]

On Wed, 22 Jul 2026 15:14:39 +0100,
Jon Hunter <jonathanh@nvidia.com> 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 <therealgraysky@proton.me>
> > Reported-by: Daniel Drake <dan@reactivated.net>
> > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Cc: Florian Fainelli <florian.fainelli@broadcom.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> >   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?

> 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.

	M.

-- 
Jazz isn't dead. It just smells funny.

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

[Jon Hunter]

On 22/07/2026 21:22, Marc Zyngier wrote:
> On Wed, 22 Jul 2026 15:14:39 +0100,
> Jon Hunter <jonathanh@nvidia.com> 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 <therealgraysky@proton.me>
>>> Reported-by: Daniel Drake <dan@reactivated.net>
>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>> Cc: Florian Fainelli <florian.fainelli@broadcom.com>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> ---
>>>    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

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

[Marc Zyngier]

On Thu, 23 Jul 2026 10:24:00 +0100,
Jon Hunter <jonathanh@nvidia.com> wrote:
> 
> >>
> >> 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.

Oh please, you know better than this.

It isn't the GIC that defines the number of supported PPIs, it is the
*integrator*. I.e. you. The GIC (GIC400 in this example) has full
support for 16 PPIs per CPU.

I have a collection of VHE-capable machines with GIC400 that correctly
implement the EL2 virtual timer PPI (if even Amlogic and AllWinner can
get this right, really *anybody* can). My conclusion is that someone
couldn't be bothered to drag a wire from one end to the other.

In any case, the result is the same: a non-working timer, and a
machine that is out of specification.

> 
> >> 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.

Then provide a way to unambiguously identify the updated firmware, and
the warning will magically disappear once people update their firmware
to a non-broken version.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

[Jon Hunter]

On 23/07/2026 10:58, Marc Zyngier wrote:

...

>>>> 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.
> 
> Oh please, you know better than this.
 >
> It isn't the GIC that defines the number of supported PPIs, it is the
> *integrator*. I.e. you. The GIC (GIC400 in this example) has full
> support for 16 PPIs per CPU.

Hmmm, I am looking at this spec [0] and it says ...

"The GIC-400 implements the interrupt types:
   16 Software Generated Interrupts (SGIs).
   6 external Private Peripheral Interrupts (PPIs) for each processor.
   1 internal PPI for each processor."

The table in [1] shows interrupts for non-sec, sec, virt and hypervisor 
timers, but I don't see a hyp-virt.

> I have a collection of VHE-capable machines with GIC400 that correctly
> implement the EL2 virtual timer PPI (if even Amlogic and AllWinner can
> get this right, really *anybody* can). My conclusion is that someone
> couldn't be bothered to drag a wire from one end to the other.

OK, so most likely I am still missing something here.

[0] 
https://developer.arm.com/documentation/ddi0471/b/introduction/about-the-gic-400?lang=en
[1] 
https://developer.arm.com/documentation/ddi0471/b/functional-description/interrupt-inputs-to-the-gic-400/ppis?lang=en

-- 
nvpublic

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

[Marc Zyngier]

On Thu, 23 Jul 2026 11:22:54 +0100,
Jon Hunter <jonathanh@nvidia.com> wrote:
> 
> 
> On 23/07/2026 10:58, Marc Zyngier wrote:
> 
> ...
> 
> >>>> 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.
> > 
> > Oh please, you know better than this.
> >
> > It isn't the GIC that defines the number of supported PPIs, it is the
> > *integrator*. I.e. you. The GIC (GIC400 in this example) has full
> > support for 16 PPIs per CPU.
> 
> Hmmm, I am looking at this spec [0] and it says ...
> 
> "The GIC-400 implements the interrupt types:
>   16 Software Generated Interrupts (SGIs).
>   6 external Private Peripheral Interrupts (PPIs) for each processor.
>   1 internal PPI for each processor."
> 
> The table in [1] shows interrupts for non-sec, sec, virt and
> hypervisor timers, but I don't see a hyp-virt.

This documentation was never updated past 2012, which predates ARMv8.1
(go complain to ARM). But the IP does support it:

[    0.000000] Machine model: Khadas VIM3L
[...]
[    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.000000] Root IRQ handler: gic_handle_irq
[    0.000000] GIC: Using split EOI/Deactivate mode
[    0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
[    0.000000] arch_timer: cp15 timer running at 24.00MHz (hyp-virt).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x588fe9dc0, max_idle_ns: 440795202592 ns
[    0.000000] sched_clock: 56 bits at 24MHz, resolution 41ns, wraps every 4398046511097ns

GIC400, A55 cores. Produced by one of the worse SoC vendor on this
side of the galaxy.

And the signal you are looking for is the one labelled as "Legacy FIQ
signal" (PPI12, aka INTID 28). The labels are purely indicative of
what you *could* connect to it, not a requirement.

Anyway, this is beside the point. Even if the IP didn't support it,
you shouldn't end-up with a design that explicitly contradict the
architecture, because that's the contract SW relies on.

	M.

-- 
Without deviation from the norm, progress is not possible.