Re: perf getting all-zeroes IP on STM32MP1

[Philippe Gerum <rpm@xenomai.org>]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> On Fri, 2026-09-04 at 17:17 +0200, Philippe Gerum wrote:
>> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>> 
>> > Hi Andrew,
>> > 
>> > [CC + Philippe]
>> > 
>> > On Fri, 2026-09-04 at 15:26 +0200, Andrew MacPherson wrote:
>> > > Hello,
>> > > 
>> > > We're working on an STM32MP1-based system running Xenomai and found
>> > > that perf top shows every symbol as "unknown [00000000]", i.e.
>> > > profiling is collecting samples, but they all point at address zero.
>> > > 
>> > > I'm not a kernel developer but went through a few debug kernel builds
>> > > with an agent which eventually led to the attached patch. This change
>> > > does resolve the issue with perf, however I'm not sure if it's the
>> > > correct solution.
>> > > 
>> > > The reasoning is that arm_arch_timer.c's percpu IRQ registration never
>> > > sets IRQF_TIMER, so Dovetail's copy_timer_regs() never populates
>> > > tick_regs, leaving get_irq_regs() to always return an all-zero
>> > > pt_regs, which in turn breaks perf's sample IP.
>> > 
>> > Yep, that is wrong. The patch you provided looks OK to me. I'm just
>> > wondering if that should be addressed in Linux as well / first.
>> > 
>> 
>> In fact, Dovetail is somewhat abusing IRQF_TIMER in that the only bit in
>> that mask we should care about is __IRQF_TIMER, which is only used for
>> recovering from misrouted IRQs these days, specifically excluding some
>> descriptors from the polling loop which tries to find a proper
>> handler. Problem is that we also carry the no-suspend semantics attached
>> to this mask when using it, which is not what we mean.
>> 
>> I think that the mainline code simply acknowledges the fact that per-CPU
>> interrupt handlers should never be polled for solving a misrouted IRQ
>> issue by definition, so there is no point in tagging those lines with
>> __IRQ_TIMER in the first place, which applies to the architected timer
>> interrupt as well. IIRC, that was the point of the recent set of
>> upstream changes to request_percpu_irq(), dropping those flags. Such
>> change prompted us to put back a secondary interface accepting flags so
>> that we can keep on passing IRQF_TIMER.
>> 
>> > @Philippe: Any additional thoughts? Should we take it already?
>> > 
>> 
>> I believe that we should not live much longer with this hack, we should
>> provide a dedicated IRQF_* flag introduced by Dovetail instead that
>> would specifically say "this line generates deferred tick events" or
>> something along these lines.
>> 
>> 
>
> Would the qualify as starting point? We would have to identify all the
> dovetail specific IRQF_TIMER usages, but that should be doable:
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 61a1e33cb2ca3..8258fbaf54a2d 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -76,6 +76,8 @@
>   *            handler any time interrupts are enabled in the CPU,
>   *            regardless of the (virtualized) interrupt state
>   *            maintained by local_irq_save/disable().
> + * IRQF_TIMER_DEFERRED - Dovetail: The interrupt line generates deferred
> + *                       tick events
>   */
>  #define IRQF_SHARED		0x00000080
>  #define IRQF_PROBE_SHARED	0x00000100
> @@ -93,6 +95,7 @@
>  #define IRQF_NO_DEBUG		0x00100000
>  #define IRQF_COND_ONESHOT	0x00200000
>  #define IRQF_OOB		0x00400000
> +#define IRQF_TIMER_DEFERRED	0x00800000
>  
>  #define IRQF_TIMER		(__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
>  
> diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c
> index de401973c6230..31fd2fc70f74d 100644
> --- a/kernel/irq/pipeline.c
> +++ b/kernel/irq/pipeline.c
> @@ -1065,7 +1065,8 @@ void copy_timer_regs(struct irq_desc *desc, struct pt_regs *regs)
>  {
>  	struct irq_pipeline_data *p;
>  
> -	if (desc->action == NULL || !(desc->action->flags & __IRQF_TIMER))
> +	if (desc->action == NULL ||
> +	    !(desc->action->flags & IRQF_TIMER_DEFERRED))
>  		return;
>  	/*
>  	 * Given our deferred dispatching model for regular IRQs, we

This is indeed what I meant.

However, on second thought, there may be another way based on Dovetail's
tick proxy infrastructure. Given that we are only interested in saving a
portion of the active register file when receiving a clock tick that
might be deferred, we could leverage the routine every oob-capable tick
interrupt handler must call in order to fire the associated clock event
handler: i.e.  clockevents_handle_event().

IOW, instead of marking the interrupt line as a provider of timer ticks,
could we just tell the single piece of code dispatching those ticks to
save the few regs we'd need later on if the event is going to be
deferred?

This idea needs more thought to implement it right, but this would save
us from having to amend every call site which mentions IRQF_TIMER for
that purpose.

-- 
Philippe.