Re: perf getting all-zeroes IP on STM32MP1

[Florian Bezdeka <florian.bezdeka@siemens.com>]

On Wed, 2026-09-09 at 10:21 +0200, Philippe Gerum wrote:
> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
> 
> > On Tue, 2026-09-08 at 10:16 +0200, Philippe Gerum wrote:
> > > 
> > > > 
> > > > We could ask the clock_event_device (real device) for it's IRQ and mark
> > > > it  as __IRQF_TIMER during proxy registration.
> > > > 
> > > > The following seems to work, but might need some more work.
> > > > 
> > > > The on_each_cpu() part is likely a overkill due to percpu IRQs, but the
> > > > real_dev part of mark_timer_tick_irq() is local CPU specific as well.
> > > > 
> > > > diff --git a/kernel/time/tick-proxy.c b/kernel/time/tick-proxy.c
> > > > index 6ef04fcf5dacd..55526cd99e849 100644
> > > > --- a/kernel/time/tick-proxy.c
> > > > +++ b/kernel/time/tick-proxy.c
> > > > @@ -296,6 +296,22 @@ static int enable_oob_timer(void *arg) /* hard_irqs_disabled() */
> > > >   	return 0;
> > > >   }
> > > >   
> > > > +static void mark_timer_tick_irq(void *arg)
> > > > +{
> > > > +	struct clock_event_device *real_dev;
> > > > +	struct irq_desc *desc;
> > > > +	int irq;
> > > > +
> > > > +	real_dev = raw_cpu_ptr(&tick_cpu_device)->evtdev;
> > > > +	irq = real_dev->irq;
> > > > +	desc = irq_to_desc(irq);
> > > > +
> > > > +	if (!desc || !desc->action)
> > > > +		return;
> > > > +
> > > > +	desc->action->flags |= __IRQF_TIMER;
> > > > +}
> > > > +
> > > >   struct proxy_install_arg {
> > > >   	void (*setup_proxy)(struct clock_proxy_device *dev);
> > > >   	int result;
> > > > @@ -400,6 +416,8 @@ int tick_install_proxy(void (*setup_proxy)(struct clock_proxy_device *dev),
> > > >   		return arg.result;
> > > >   	}
> > > >   
> > > > +	on_each_cpu(mark_timer_tick_irq, NULL, true);
> > > > +
> > > >   	/*
> > > >   	 * Start ticking from the out-of-band interrupt stage upon
> > > >   	 * receipt of out-of-band timer events.
> > > 
> > > Almost there, but we still need to provide the registers used in
> > > profiling when no proxy is registered, in which case we cannot depend on
> > > proxy registration for this, but on clock event device registration
> > > instead.
> > 
> > diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c
> > index 85ec0cbf5fb1e..c4eda1243b915 100644
> > --- a/kernel/irq/pipeline.c
> > +++ b/kernel/irq/pipeline.c
> > @@ -1064,7 +1064,7 @@ 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_DEFERRED_TIMER))
> >   		return;
> >   	/*
> >   	 * Given our deferred dispatching model for regular IRQs, we
> > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> > index 90fae659e4ea6..9296164c1d198 100644
> > --- a/kernel/time/tick-common.c
> > +++ b/kernel/time/tick-common.c
> > @@ -326,6 +326,16 @@ bool tick_check_replacement(struct clock_event_device *curdev,
> >   	return tick_check_preferred(curdev, newdev);
> >   }
> >   
> > +static void tick_mark_deferred_timer_irq(struct clock_event_device *dev)
> > +{
> > +	struct irq_desc *desc = irq_to_desc(dev->irq);
> > +
> > +	if (!desc || !desc->action)
> > +		return;
> > +
> > +	desc->action->flags |= IRQF_DEFERRED_TIMER;
> > +}
> > +
> >   /*
> >    * Check, if the new registered device should be used. Called with
> >    * clockevents_lock held and interrupts disabled.
> > @@ -367,6 +377,8 @@ void tick_check_new_device(struct clock_event_device *newdev)
> >   	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
> >   	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
> >   		tick_oneshot_notify();
> > +	if (newdev->features & CLOCK_EVT_FEAT_PIPELINE)
> > +		tick_mark_deferred_timer_irq(newdev);
> >   	return;
> >   
> >   out_bc:
> 
> I would attach this flag to the interrupt descriptor instead because
> this is actually a property of the interrupt line, not of its
> handler(s). Also, we need to consider device shutdown: as the current
> tick source may be replaced dynamically, turning off this bit for proper
> accounting when a clock device goes down would be safer.
> 
> e.g.:
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index b13e4e90ab18f..5f7a2c78b3ca7 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -81,6 +81,7 @@ enum irqchip_irq_state;
>   *                                when pipelining is enabled (CONFIG_IRQ_PIPELINE),
>   *                                regardless of the (virtualized) interrupt state
>   *                                maintained by local_irq_save/disable().
> + * IRQ_TICK                     - Interrupt is a timer tick source.
>   */
>  enum {
>  	IRQ_TYPE_NONE		= 0x00000000,
> @@ -109,14 +110,15 @@ enum {
>  	IRQ_HIDDEN		= (1 << 20),
>  	IRQ_NO_DEBUG		= (1 << 21),
>  	IRQ_OOB			= (1 << 22),
> -	IRQ_RESERVED		= (1 << 23),
> +	IRQ_TICK		= (1 << 23),
> +	IRQ_RESERVED		= (1 << 24),
>  };
>  
>  #define IRQF_MODIFY_MASK	\
>  	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
>  	 IRQ_NOAUTOEN | IRQ_LEVEL | IRQ_NO_BALANCING | \
>  	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \
> -	 IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY | IRQ_HIDDEN | IRQ_OOB)
> +	 IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY | IRQ_HIDDEN | IRQ_OOB | IRQ_TICK)
>  
>  #define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
>  
> @@ -1258,6 +1260,7 @@ static inline struct irq_chip_type *irq_data_get_chip_type(struct irq_data *d)
>  
>  #ifdef CONFIG_IRQ_PIPELINE
>  int irq_switch_oob(unsigned int irq, bool on);
> +void irq_switch_tick(unsigned int irq, bool on);
>  void irq_clear_deferral(struct irq_desc *desc);
>  void irq_clear_forward(struct irq_desc *desc);
>  #else
> @@ -1266,6 +1269,10 @@ static inline int irq_switch_oob(unsigned int irq, bool on)
>  	return 0;
>  }
>  
> +static inline void irq_switch_tick(unsigned int irq, bool on)
> +{
> +}
> +
>  static inline void irq_clear_deferral(struct irq_desc *desc) { }
>  static inline void irq_clear_forward(struct irq_desc *desc) { }
>  #endif	/* !CONFIG_IRQ_PIPELINE */
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 260ffd288bd81..c4e2d6a5d20d5 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -274,6 +274,11 @@ static inline int irq_is_oob(unsigned int irq)
>  	return irq_check_status_bit(irq, IRQ_OOB);
>  }
>  
> +static inline int irq_is_tick(unsigned int irq)
> +{
> +	return irq_check_status_bit(irq, IRQ_TICK);
> +}
> +

I have a patch pending that would remove irq_is_oob(), as there is no
user. The same for irq_is_tick(), no? Is that used somewhere?


The rest makes sense, I will give it a try.