* perf getting all-zeroes IP on STM32MP1 @ 2026-09-04 13:26 Andrew MacPherson 2026-09-04 14:08 ` Florian Bezdeka 0 siblings, 1 reply; 19+ messages in thread From: Andrew MacPherson @ 2026-09-04 13:26 UTC (permalink / raw) To: xenomai [-- Attachment #1: Type: text/plain, Size: 980 bytes --] 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. We're on kernel 6.6.48, i.e. an older revision of the (now-deprecated) v6.6.y-evl-rebase branch, so entirely possible that this is fixed in a later version, though the code appears to be the same. Just wondering if anyone has any more insight around this and thanks for any help! Cheers, Andrew [-- Attachment #2: arm_arch_timer_flag.patch --] [-- Type: application/octet-stream, Size: 1539 bytes --] diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 0000000000000..0000000000000 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -1235,25 +1235,25 @@ ppi = arch_timer_ppi[arch_timer_uses_ppi]; switch (arch_timer_uses_ppi) { case ARCH_TIMER_VIRT_PPI: - err = request_percpu_irq(ppi, arch_timer_handler_virt, - "arch_timer", arch_timer_evt); + err = __request_percpu_irq(ppi, arch_timer_handler_virt, + IRQF_TIMER, "arch_timer", arch_timer_evt); break; case ARCH_TIMER_PHYS_SECURE_PPI: case ARCH_TIMER_PHYS_NONSECURE_PPI: - err = request_percpu_irq(ppi, arch_timer_handler_phys, - "arch_timer", arch_timer_evt); + err = __request_percpu_irq(ppi, arch_timer_handler_phys, + IRQF_TIMER, "arch_timer", arch_timer_evt); if (!err && arch_timer_has_nonsecure_ppi()) { ppi = arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]; - err = request_percpu_irq(ppi, arch_timer_handler_phys, - "arch_timer", arch_timer_evt); + err = __request_percpu_irq(ppi, arch_timer_handler_phys, + IRQF_TIMER, "arch_timer", arch_timer_evt); if (err) free_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_SECURE_PPI], arch_timer_evt); } break; case ARCH_TIMER_HYP_PPI: - err = request_percpu_irq(ppi, arch_timer_handler_phys, - "arch_timer", arch_timer_evt); + err = __request_percpu_irq(ppi, arch_timer_handler_phys, + IRQF_TIMER, "arch_timer", arch_timer_evt); break; default: BUG(); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-04 13:26 perf getting all-zeroes IP on STM32MP1 Andrew MacPherson @ 2026-09-04 14:08 ` Florian Bezdeka 2026-09-04 15:17 ` Philippe Gerum 2026-09-07 8:57 ` Andrew MacPherson 0 siblings, 2 replies; 19+ messages in thread From: Florian Bezdeka @ 2026-09-04 14:08 UTC (permalink / raw) To: Andrew MacPherson, xenomai; +Cc: Philippe Gerum 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. @Philippe: Any additional thoughts? Should we take it already? @Andrew: Could you please provide a formal patch with proper signed-off and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. branch? That should help to speed things up. Thanks! Florian -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-04 14:08 ` Florian Bezdeka @ 2026-09-04 15:17 ` Philippe Gerum 2026-09-04 15:55 ` Florian Bezdeka 2026-09-07 8:57 ` Andrew MacPherson 1 sibling, 1 reply; 19+ messages in thread From: Philippe Gerum @ 2026-09-04 15:17 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai 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. > > @Andrew: Could you please provide a formal patch with proper signed-off > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. > branch? That should help to speed things up. Thanks! > > Florian -- Philippe. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-04 15:17 ` Philippe Gerum @ 2026-09-04 15:55 ` Florian Bezdeka 2026-09-05 9:16 ` Philippe Gerum 0 siblings, 1 reply; 19+ messages in thread From: Florian Bezdeka @ 2026-09-04 15:55 UTC (permalink / raw) To: Philippe Gerum; +Cc: Andrew MacPherson, xenomai 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-04 15:55 ` Florian Bezdeka @ 2026-09-05 9:16 ` Philippe Gerum 0 siblings, 0 replies; 19+ messages in thread From: Philippe Gerum @ 2026-09-05 9:16 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-04 14:08 ` Florian Bezdeka 2026-09-04 15:17 ` Philippe Gerum @ 2026-09-07 8:57 ` Andrew MacPherson 2026-09-07 11:34 ` Florian Bezdeka 1 sibling, 1 reply; 19+ messages in thread From: Andrew MacPherson @ 2026-09-07 8:57 UTC (permalink / raw) To: Florian Bezdeka; +Cc: xenomai, Philippe Gerum On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka <florian.bezdeka@siemens.com> wrote: > > 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. > > @Philippe: Any additional thoughts? Should we take it already? > > > @Andrew: Could you please provide a formal patch with proper signed-off > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. > branch? That should help to speed things up. Thanks! > > Florian > > -- > Siemens AG, Foundational Technologies > Linux Expert Center > > Hi Florian, I've submitted a patch now against dovetail 7.2, let me know if you need anything else and thanks for the help! Cheers, Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 8:57 ` Andrew MacPherson @ 2026-09-07 11:34 ` Florian Bezdeka 2026-09-07 12:03 ` Gerte Hoogewerf ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Florian Bezdeka @ 2026-09-07 11:34 UTC (permalink / raw) To: Andrew MacPherson; +Cc: xenomai, Philippe Gerum, Gerte Hoogewerf On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: > On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka > <florian.bezdeka@siemens.com> wrote: > > > > 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. > > > > @Philippe: Any additional thoughts? Should we take it already? > > > > > > @Andrew: Could you please provide a formal patch with proper signed-off > > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. > > branch? That should help to speed things up. Thanks! > > > > Florian > > > > -- > > Siemens AG, Foundational Technologies > > Linux Expert Center > > > > > > Hi Florian, > > I've submitted a patch now against dovetail 7.2, let me know if you > need anything else and thanks for the help! > > Thanks! We might consider merging that while working on a better solution - as suggested by Philippe. The following is build + boot tested on arm, arm64 and x86 using qemu. Would be nice to get some early feedback. It should fix the problems reported by Andrew and Gerte. diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index a46872cf1384b..553243174368d 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -16,6 +16,9 @@ # include <linux/ktime.h> # include <linux/notifier.h> # include <linux/irqstage.h> +# include <linux/preempt.h> +# include <asm-generic/irq_regs.h> +# include <asm/irq_pipeline.h> struct clock_event_device; struct module; @@ -259,9 +262,30 @@ struct clock_proxy_device { void tick_notify_proxy(void); +static inline void copy_timer_regs(void) +{ + struct pt_regs *regs = get_irq_regs(); + struct irq_pipeline_data *p; + + if (!in_pipeline()) + return; + + /* + * Given our deferred dispatching model for regular IRQs, we + * record the preempted context registers only for the latest + * timer interrupt, so that the regular tick handler charges + * CPU times properly. It is assumed that no other interrupt + * handler cares for such information. + */ + p = raw_cpu_ptr(&irq_pipeline); + arch_save_timer_regs(&p->tick_regs, regs); +} + static inline void clockevents_handle_event(struct clock_event_device *ced) { + copy_timer_regs(); + /* * If called from the in-band stage, or for delivering a * high-precision timer event to the out-of-band stage, call diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c index 85ec0cbf5fb1e..d160d99e32c5f 100644 --- a/kernel/irq/pipeline.c +++ b/kernel/irq/pipeline.c @@ -1059,24 +1059,6 @@ bool handle_oob_irq(struct irq_desc *desc) return true; } -static inline -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)) - return; - /* - * Given our deferred dispatching model for regular IRQs, we - * record the preempted context registers only for the latest - * timer interrupt, so that the regular tick handler charges - * CPU times properly. It is assumed that no other interrupt - * handler cares for such information. - */ - p = raw_cpu_ptr(&irq_pipeline); - arch_save_timer_regs(&p->tick_regs, regs); -} - static __always_inline struct irq_stage_data *switch_stage_on_irq(void) { @@ -1123,7 +1105,6 @@ void restore_stage_on_irq(struct irq_stage_data *prevd) */ int generic_pipeline_irq_desc(struct irq_desc *desc) { - struct pt_regs *regs = get_irq_regs(); int irq; if (!desc) @@ -1137,7 +1118,6 @@ int generic_pipeline_irq_desc(struct irq_desc *desc) } trace_irq_pipeline_entry(irq); - copy_timer_regs(desc, regs); generic_handle_irq_desc(desc); trace_irq_pipeline_exit(irq); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 11:34 ` Florian Bezdeka @ 2026-09-07 12:03 ` Gerte Hoogewerf 2026-09-07 12:41 ` Philippe Gerum 2026-09-07 12:51 ` Andrew MacPherson 2 siblings, 0 replies; 19+ messages in thread From: Gerte Hoogewerf @ 2026-09-07 12:03 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Philippe Gerum Hi Florian/Andrew, On Mon, Sep 7, 2026 at 1:34 PM Florian Bezdeka <florian.bezdeka@siemens.com> wrote: > [..] It should fix the problems reported by Andrew and Gerte. Yes, confirmed on my end. Awesome! Thanks, -- This email and any attachment(s) it may contain is confidential and is intended solely for the use of the individual(s) to whom it is addressed. If you are not the intended recipient of this email, you must not take action based on the contents, nor distribute, nor expose any part of the content(s) to entities or person(s) beyond the original distribution list. Please contact the sender and delete the email if you have received it in error. Thank you. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 11:34 ` Florian Bezdeka 2026-09-07 12:03 ` Gerte Hoogewerf @ 2026-09-07 12:41 ` Philippe Gerum 2026-09-07 14:39 ` Florian Bezdeka 2026-09-08 8:11 ` Philippe Gerum 2026-09-07 12:51 ` Andrew MacPherson 2 siblings, 2 replies; 19+ messages in thread From: Philippe Gerum @ 2026-09-07 12:41 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf Florian Bezdeka <florian.bezdeka@siemens.com> writes: > On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: >> On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka >> <florian.bezdeka@siemens.com> wrote: >> > >> > 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. >> > >> > @Philippe: Any additional thoughts? Should we take it already? >> > >> > >> > @Andrew: Could you please provide a formal patch with proper signed-off >> > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. >> > branch? That should help to speed things up. Thanks! >> > >> > Florian >> > >> > -- >> > Siemens AG, Foundational Technologies >> > Linux Expert Center >> > >> > >> >> Hi Florian, >> >> I've submitted a patch now against dovetail 7.2, let me know if you >> need anything else and thanks for the help! >> >> > > Thanks! We might consider merging that while working on a better > solution - as suggested by Philippe. > Unfortunately, thinking a bit more/better, we'd still have an issue with what I suggested. i.e. There are three contexts we need to care about in this case: 1. when a timer tick can be immediately delivered to its handler (i.e. hw irqs on) from a line tagged with IRQF_OOB. 2. when a timer tick can be immediately delivered (i.e. in-band stage is installed) from a line set for in-band delivery (i.e. not tagged with IRQF_OOB). In this case, the interrupt log is synchronized before leaving handle_irq_pipelined_finish(). 3. when a timer tick /should/ but cannot be delivered to the in-band stage because the latter is stalled, i.e. need for deferral via the interrupt log. In the first two cases, postponing the copy logic to clockevents_handle_event() would be ok, because the interrupt frame of the timer event would still be active, therefore using get_irq_regs() to find the regs to copy would be correct. In case #3, we have a deferral, therefore the interrupt frame is certainly gone when the in-band stage is unstalled. Since other interrupts could happen in between, we are toast. IOW, close, but no cigar. Back to the drawing board. -- Philippe. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 12:41 ` Philippe Gerum @ 2026-09-07 14:39 ` Florian Bezdeka 2026-09-07 15:00 ` Philippe Gerum 2026-09-08 8:11 ` Philippe Gerum 1 sibling, 1 reply; 19+ messages in thread From: Florian Bezdeka @ 2026-09-07 14:39 UTC (permalink / raw) To: Philippe Gerum; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf On Mon, 2026-09-07 at 14:41 +0200, Philippe Gerum wrote: > Florian Bezdeka <florian.bezdeka@siemens.com> writes: > > > On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: > > > On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka > > > <florian.bezdeka@siemens.com> wrote: > > > > > > > > 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. > > > > > > > > @Philippe: Any additional thoughts? Should we take it already? > > > > > > > > > > > > @Andrew: Could you please provide a formal patch with proper signed-off > > > > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. > > > > branch? That should help to speed things up. Thanks! > > > > > > > > Florian > > > > > > > > -- > > > > Siemens AG, Foundational Technologies > > > > Linux Expert Center > > > > > > > > > > > > > > Hi Florian, > > > > > > I've submitted a patch now against dovetail 7.2, let me know if you > > > need anything else and thanks for the help! > > > > > > > > > > Thanks! We might consider merging that while working on a better > > solution - as suggested by Philippe. > > > > Unfortunately, thinking a bit more/better, we'd still have an issue with > what I suggested. i.e. There are three contexts we need to care about in > this case: > > 1. when a timer tick can be immediately delivered to its handler > (i.e. hw irqs on) from a line tagged with IRQF_OOB. > > 2. when a timer tick can be immediately delivered (i.e. in-band stage is > installed) from a line set for in-band delivery (i.e. not tagged with > IRQF_OOB). In this case, the interrupt log is synchronized before > leaving handle_irq_pipelined_finish(). > > 3. when a timer tick /should/ but cannot be delivered to the in-band stage > because the latter is stalled, i.e. need for deferral via the > interrupt log. > > In the first two cases, postponing the copy logic to > clockevents_handle_event() would be ok, because the interrupt frame of > the timer event would still be active, therefore using get_irq_regs() to > find the regs to copy would be correct. Agree. > > In case #3, we have a deferral, therefore the interrupt frame is > certainly gone when the in-band stage is unstalled. Since other > interrupts could happen in between, we are toast. > Hm, when setting up the proxy device, we mark the affected IRQs as OOB IRQs. Doesn't that mean that we end up in clockevents_handle_event() for those IRQs as well? That seems to be the case, if my debugging here is right. We want to copy the registers of the last OOB timer tick, no? That looks doable, but I might miss something or just did not run into the "inband stalled" case. Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 14:39 ` Florian Bezdeka @ 2026-09-07 15:00 ` Philippe Gerum 2026-09-08 8:02 ` Florian Bezdeka 0 siblings, 1 reply; 19+ messages in thread From: Philippe Gerum @ 2026-09-07 15:00 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf Florian Bezdeka <florian.bezdeka@siemens.com> writes: > On Mon, 2026-09-07 at 14:41 +0200, Philippe Gerum wrote: >> Florian Bezdeka <florian.bezdeka@siemens.com> writes: >> >> > On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: >> > > On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka >> > > <florian.bezdeka@siemens.com> wrote: >> > > > >> > > > 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. >> > > > >> > > > @Philippe: Any additional thoughts? Should we take it already? >> > > > >> > > > >> > > > @Andrew: Could you please provide a formal patch with proper signed-off >> > > > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. >> > > > branch? That should help to speed things up. Thanks! >> > > > >> > > > Florian >> > > > >> > > > -- >> > > > Siemens AG, Foundational Technologies >> > > > Linux Expert Center >> > > > >> > > > >> > > >> > > Hi Florian, >> > > >> > > I've submitted a patch now against dovetail 7.2, let me know if you >> > > need anything else and thanks for the help! >> > > >> > > >> > >> > Thanks! We might consider merging that while working on a better >> > solution - as suggested by Philippe. >> > >> >> Unfortunately, thinking a bit more/better, we'd still have an issue with >> what I suggested. i.e. There are three contexts we need to care about in >> this case: >> >> 1. when a timer tick can be immediately delivered to its handler >> (i.e. hw irqs on) from a line tagged with IRQF_OOB. >> >> 2. when a timer tick can be immediately delivered (i.e. in-band stage is >> installed) from a line set for in-band delivery (i.e. not tagged with >> IRQF_OOB). In this case, the interrupt log is synchronized before >> leaving handle_irq_pipelined_finish(). >> >> 3. when a timer tick /should/ but cannot be delivered to the in-band stage >> because the latter is stalled, i.e. need for deferral via the >> interrupt log. >> >> In the first two cases, postponing the copy logic to >> clockevents_handle_event() would be ok, because the interrupt frame of >> the timer event would still be active, therefore using get_irq_regs() to >> find the regs to copy would be correct. > > Agree. > >> >> In case #3, we have a deferral, therefore the interrupt frame is >> certainly gone when the in-band stage is unstalled. Since other >> interrupts could happen in between, we are toast. >> > > Hm, when setting up the proxy device, we mark the affected IRQs as OOB > IRQs. Doesn't that mean that we end up in clockevents_handle_event() for > those IRQs as well? That seems to be the case, if my debugging here is > right. > > We want to copy the registers of the last OOB timer tick, no? That looks > doable, but I might miss something or just did not run into the "inband > stalled" case. We want to copy those registers every time the profiling code may consume them, including when the tick is delivered to the in-band stage only. The proxy tick infrastructure allows for enabling only a subset of the CPU range for oob traffic, other CPUs would keep on receiving timer events from the in-band stage. In order to extend the test case, I would tweak oob_cpus and look at the perf results for a task affine to a non-oob processor. -- Philippe. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 15:00 ` Philippe Gerum @ 2026-09-08 8:02 ` Florian Bezdeka 2026-09-08 8:16 ` Philippe Gerum 0 siblings, 1 reply; 19+ messages in thread From: Florian Bezdeka @ 2026-09-08 8:02 UTC (permalink / raw) To: Philippe Gerum; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf On Mon, 2026-09-07 at 17:00 +0200, Philippe Gerum wrote: > Florian Bezdeka <florian.bezdeka@siemens.com> writes: > > > On Mon, 2026-09-07 at 14:41 +0200, Philippe Gerum wrote: > > > Florian Bezdeka <florian.bezdeka@siemens.com> writes: > > > > > > > On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: > > > > > On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka > > > > > <florian.bezdeka@siemens.com> wrote: > > > > > > > > > > > > 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. > > > > > > > > > > > > @Philippe: Any additional thoughts? Should we take it already? > > > > > > > > > > > > > > > > > > @Andrew: Could you please provide a formal patch with proper signed-off > > > > > > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. > > > > > > branch? That should help to speed things up. Thanks! > > > > > > > > > > > > Florian > > > > > > > > > > > > -- > > > > > > Siemens AG, Foundational Technologies > > > > > > Linux Expert Center > > > > > > > > > > > > > > > > > > > > > > Hi Florian, > > > > > > > > > > I've submitted a patch now against dovetail 7.2, let me know if you > > > > > need anything else and thanks for the help! > > > > > > > > > > > > > > > > > > Thanks! We might consider merging that while working on a better > > > > solution - as suggested by Philippe. > > > > > > > > > > Unfortunately, thinking a bit more/better, we'd still have an issue with > > > what I suggested. i.e. There are three contexts we need to care about in > > > this case: > > > > > > 1. when a timer tick can be immediately delivered to its handler > > > (i.e. hw irqs on) from a line tagged with IRQF_OOB. > > > > > > 2. when a timer tick can be immediately delivered (i.e. in-band stage is > > > installed) from a line set for in-band delivery (i.e. not tagged with > > > IRQF_OOB). In this case, the interrupt log is synchronized before > > > leaving handle_irq_pipelined_finish(). > > > > > > 3. when a timer tick /should/ but cannot be delivered to the in-band stage > > > because the latter is stalled, i.e. need for deferral via the > > > interrupt log. > > > > > > In the first two cases, postponing the copy logic to > > > clockevents_handle_event() would be ok, because the interrupt frame of > > > the timer event would still be active, therefore using get_irq_regs() to > > > find the regs to copy would be correct. > > > > Agree. > > > > > > > > In case #3, we have a deferral, therefore the interrupt frame is > > > certainly gone when the in-band stage is unstalled. Since other > > > interrupts could happen in between, we are toast. > > > > > > > Hm, when setting up the proxy device, we mark the affected IRQs as OOB > > IRQs. Doesn't that mean that we end up in clockevents_handle_event() for > > those IRQs as well? That seems to be the case, if my debugging here is > > right. > > > > We want to copy the registers of the last OOB timer tick, no? That looks > > doable, but I might miss something or just did not run into the "inband > > stalled" case. > > We want to copy those registers every time the profiling code may > consume them, including when the tick is delivered to the in-band stage > only. The proxy tick infrastructure allows for enabling only a subset of > the CPU range for oob traffic, other CPUs would keep on receiving timer > events from the in-band stage. > > In order to extend the test case, I would tweak oob_cpus and look at the > perf results for a task affine to a non-oob processor. 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. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-08 8:02 ` Florian Bezdeka @ 2026-09-08 8:16 ` Philippe Gerum 2026-09-08 8:46 ` Florian Bezdeka 0 siblings, 1 reply; 19+ messages in thread From: Philippe Gerum @ 2026-09-08 8:16 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf Florian Bezdeka <florian.bezdeka@siemens.com> writes: > On Mon, 2026-09-07 at 17:00 +0200, Philippe Gerum wrote: >> Florian Bezdeka <florian.bezdeka@siemens.com> writes: >> >> > On Mon, 2026-09-07 at 14:41 +0200, Philippe Gerum wrote: >> > > Florian Bezdeka <florian.bezdeka@siemens.com> writes: >> > > >> > > > On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: >> > > > > On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka >> > > > > <florian.bezdeka@siemens.com> wrote: >> > > > > > >> > > > > > 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. >> > > > > > >> > > > > > @Philippe: Any additional thoughts? Should we take it already? >> > > > > > >> > > > > > >> > > > > > @Andrew: Could you please provide a formal patch with proper signed-off >> > > > > > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. >> > > > > > branch? That should help to speed things up. Thanks! >> > > > > > >> > > > > > Florian >> > > > > > >> > > > > > -- >> > > > > > Siemens AG, Foundational Technologies >> > > > > > Linux Expert Center >> > > > > > >> > > > > > >> > > > > >> > > > > Hi Florian, >> > > > > >> > > > > I've submitted a patch now against dovetail 7.2, let me know if you >> > > > > need anything else and thanks for the help! >> > > > > >> > > > > >> > > > >> > > > Thanks! We might consider merging that while working on a better >> > > > solution - as suggested by Philippe. >> > > > >> > > >> > > Unfortunately, thinking a bit more/better, we'd still have an issue with >> > > what I suggested. i.e. There are three contexts we need to care about in >> > > this case: >> > > >> > > 1. when a timer tick can be immediately delivered to its handler >> > > (i.e. hw irqs on) from a line tagged with IRQF_OOB. >> > > >> > > 2. when a timer tick can be immediately delivered (i.e. in-band stage is >> > > installed) from a line set for in-band delivery (i.e. not tagged with >> > > IRQF_OOB). In this case, the interrupt log is synchronized before >> > > leaving handle_irq_pipelined_finish(). >> > > >> > > 3. when a timer tick /should/ but cannot be delivered to the in-band stage >> > > because the latter is stalled, i.e. need for deferral via the >> > > interrupt log. >> > > >> > > In the first two cases, postponing the copy logic to >> > > clockevents_handle_event() would be ok, because the interrupt frame of >> > > the timer event would still be active, therefore using get_irq_regs() to >> > > find the regs to copy would be correct. >> > >> > Agree. >> > >> > > >> > > In case #3, we have a deferral, therefore the interrupt frame is >> > > certainly gone when the in-band stage is unstalled. Since other >> > > interrupts could happen in between, we are toast. >> > > >> > >> > Hm, when setting up the proxy device, we mark the affected IRQs as OOB >> > IRQs. Doesn't that mean that we end up in clockevents_handle_event() for >> > those IRQs as well? That seems to be the case, if my debugging here is >> > right. >> > >> > We want to copy the registers of the last OOB timer tick, no? That looks >> > doable, but I might miss something or just did not run into the "inband >> > stalled" case. >> >> We want to copy those registers every time the profiling code may >> consume them, including when the tick is delivered to the in-band stage >> only. The proxy tick infrastructure allows for enabling only a subset of >> the CPU range for oob traffic, other CPUs would keep on receiving timer >> events from the in-band stage. >> >> In order to extend the test case, I would tweak oob_cpus and look at the >> perf results for a task affine to a non-oob processor. > > 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. -- Philippe. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-08 8:16 ` Philippe Gerum @ 2026-09-08 8:46 ` Florian Bezdeka 2026-09-09 8:21 ` Philippe Gerum 0 siblings, 1 reply; 19+ messages in thread From: Florian Bezdeka @ 2026-09-08 8:46 UTC (permalink / raw) To: Philippe Gerum; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf 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: ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-08 8:46 ` Florian Bezdeka @ 2026-09-09 8:21 ` Philippe Gerum 2026-09-09 8:29 ` Florian Bezdeka 0 siblings, 1 reply; 19+ messages in thread From: Philippe Gerum @ 2026-09-09 8:21 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf 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); +} + void __irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class, struct lock_class_key *request_class); static inline void diff --git a/kernel/irq/debug.h b/kernel/irq/debug.h index 4eafd04a62962..9813c8ec66a44 100644 --- a/kernel/irq/debug.h +++ b/kernel/irq/debug.h @@ -34,6 +34,7 @@ static inline void print_irq_desc(unsigned int irq, struct irq_desc *desc) ___P(IRQ_NOTHREAD); ___P(IRQ_NOAUTOEN); ___P(IRQ_OOB); + ___P(IRQ_TICK); ___PS(IRQS_AUTODETECT); ___PS(IRQS_REPLAY); diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 1c1920f5fbc08..a10d1081bea8c 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -949,6 +949,17 @@ int irq_switch_oob(unsigned int irq, bool on) } EXPORT_SYMBOL_GPL(irq_switch_oob); +void irq_switch_tick(unsigned int irq, bool on) +{ + scoped_irqdesc_get_and_lock(irq, 0) { + if (on) + irq_settings_set_tick(scoped_irqdesc); + else + irq_settings_clr_tick(scoped_irqdesc); + } +} +EXPORT_SYMBOL_GPL(irq_switch_tick); + #endif /* CONFIG_IRQ_PIPELINE */ /* diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c index 85ec0cbf5fb1e..ce4404b7b2c4d 100644 --- a/kernel/irq/pipeline.c +++ b/kernel/irq/pipeline.c @@ -1062,10 +1062,6 @@ bool handle_oob_irq(struct irq_desc *desc) static inline 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)) - return; /* * Given our deferred dispatching model for regular IRQs, we * record the preempted context registers only for the latest @@ -1073,8 +1069,10 @@ void copy_timer_regs(struct irq_desc *desc, struct pt_regs *regs) * CPU times properly. It is assumed that no other interrupt * handler cares for such information. */ - p = raw_cpu_ptr(&irq_pipeline); - arch_save_timer_regs(&p->tick_regs, regs); + if (irq_settings_is_tick(desc)) { + struct irq_pipeline_data *p = raw_cpu_ptr(&irq_pipeline); + arch_save_timer_regs(&p->tick_regs, regs); + } } static __always_inline diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h index 27a37d992f237..c4b48d22d271a 100644 --- a/kernel/irq/settings.h +++ b/kernel/irq/settings.h @@ -19,6 +19,7 @@ enum { _IRQ_HIDDEN = IRQ_HIDDEN, _IRQ_NO_DEBUG = IRQ_NO_DEBUG, _IRQ_OOB = IRQ_OOB, + _IRQ_TICK = IRQ_TICK, _IRQ_PROC_VALID = IRQ_RESERVED, _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK, }; @@ -37,6 +38,7 @@ enum { #define IRQ_HIDDEN GOT_YOU_MORON #define IRQ_NO_DEBUG GOT_YOU_MORON #define IRQ_OOB GOT_YOU_MORON +#define IRQ_TICK GOT_YOU_MORON #define IRQ_RESERVED GOT_YOU_MORON #undef IRQF_MODIFY_MASK #define IRQF_MODIFY_MASK GOT_YOU_MORON @@ -210,3 +212,18 @@ static inline void irq_settings_set_oob(struct irq_desc *desc) { desc->status_use_accessors |= _IRQ_OOB; } + +static inline bool irq_settings_is_tick(struct irq_desc *desc) +{ + return desc->status_use_accessors & _IRQ_TICK; +} + +static inline void irq_settings_clr_tick(struct irq_desc *desc) +{ + desc->status_use_accessors &= ~_IRQ_TICK; +} + +static inline void irq_settings_set_tick(struct irq_desc *desc) +{ + desc->status_use_accessors |= _IRQ_TICK; +} diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index 0ed4122d40986..d8d2dd43baaa0 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -12,6 +12,7 @@ #include <linux/init.h> #include <linux/module.h> #include <linux/smp.h> +#include <linux/irq.h> #include <linux/device.h> #include "tick-internal.h" @@ -177,6 +178,8 @@ void clockevents_shutdown(struct clock_event_device *dev) clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN); dev->next_event = KTIME_MAX; dev->next_event_forced = 0; + if (dev->features & CLOCK_EVT_FEAT_PIPELINE) + irq_switch_tick(dev->irq, false); } /** diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index 90fae659e4ea6..d15bd478cef89 100644 --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -13,6 +13,7 @@ #include <linux/hrtimer.h> #include <linux/interrupt.h> #include <linux/nmi.h> +#include <linux/irq.h> #include <linux/percpu.h> #include <linux/profile.h> #include <linux/sched.h> @@ -244,6 +245,9 @@ static void tick_setup_device(struct tick_device *td, if (!cpumask_equal(newdev->cpumask, cpumask)) irq_set_affinity(newdev->irq, cpumask); + if (newdev->features & CLOCK_EVT_FEAT_PIPELINE) + irq_switch_tick(newdev->irq, true); + /* * When global broadcasting is active, check if the current * device is registered as a placeholder for broadcast mode. -- Philippe. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-09 8:21 ` Philippe Gerum @ 2026-09-09 8:29 ` Florian Bezdeka 2026-09-09 8:49 ` Philippe Gerum 0 siblings, 1 reply; 19+ messages in thread From: Florian Bezdeka @ 2026-09-09 8:29 UTC (permalink / raw) To: Philippe Gerum; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-09 8:29 ` Florian Bezdeka @ 2026-09-09 8:49 ` Philippe Gerum 0 siblings, 0 replies; 19+ messages in thread From: Philippe Gerum @ 2026-09-09 8:49 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf Florian Bezdeka <florian.bezdeka@siemens.com> writes: > 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? > No, I added it only for symmetry with _oob, but actually those flags are tested using the descriptor-based helper instead. So, we could drop the irq_is_{oob, tick}() helpers indeed. > > The rest makes sense, I will give it a try. -- Philippe. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 12:41 ` Philippe Gerum 2026-09-07 14:39 ` Florian Bezdeka @ 2026-09-08 8:11 ` Philippe Gerum 1 sibling, 0 replies; 19+ messages in thread From: Philippe Gerum @ 2026-09-08 8:11 UTC (permalink / raw) To: Florian Bezdeka; +Cc: Andrew MacPherson, xenomai, Gerte Hoogewerf Philippe Gerum <rpm@xenomai.org> writes: > Florian Bezdeka <florian.bezdeka@siemens.com> writes: > >> On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: >>> On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka >>> <florian.bezdeka@siemens.com> wrote: >>> > >>> > 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. >>> > >>> > @Philippe: Any additional thoughts? Should we take it already? >>> > >>> > >>> > @Andrew: Could you please provide a formal patch with proper signed-off >>> > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. >>> > branch? That should help to speed things up. Thanks! >>> > >>> > Florian >>> > >>> > -- >>> > Siemens AG, Foundational Technologies >>> > Linux Expert Center >>> > >>> > >>> >>> Hi Florian, >>> >>> I've submitted a patch now against dovetail 7.2, let me know if you >>> need anything else and thanks for the help! >>> >>> >> >> Thanks! We might consider merging that while working on a better >> solution - as suggested by Philippe. >> > > Unfortunately, thinking a bit more/better, we'd still have an issue with > what I suggested. i.e. There are three contexts we need to care about in > this case: > > 1. when a timer tick can be immediately delivered to its handler > (i.e. hw irqs on) from a line tagged with IRQF_OOB. > > 2. when a timer tick can be immediately delivered (i.e. in-band stage is > installed) from a line set for in-band delivery (i.e. not tagged with > IRQF_OOB). In this case, the interrupt log is synchronized before > leaving handle_irq_pipelined_finish(). > > 3. when a timer tick /should/ but cannot be delivered to the in-band stage > because the latter is stalled, i.e. need for deferral via the > interrupt log. > > In the first two cases, postponing the copy logic to > clockevents_handle_event() would be ok, because the interrupt frame of > the timer event would still be active, therefore using get_irq_regs() to > find the regs to copy would be correct. > > In case #3, we have a deferral, therefore the interrupt frame is > certainly gone when the in-band stage is unstalled. Since other > interrupts could happen in between, we are toast. > > IOW, close, but no cigar. Back to the drawing board. Ok, here is another proposal, which would also reuse the tick-proxy infrastructure. We know that any clockevent device driver which supports pipelining must declare the irq feeding it. No ifs or buts, the infrastructure already requires it: /* * ... * @name: ptr to clock event name * @rating: variable to rate clock event devices * @irq: IRQ number (only for non CPU local devices, or pipelined timers) * @bound_on: Bound on CPU * @cpumask: cpumask to indicate for which CPUs this device works * ... */ struct clock_event_device { ... int irq; ... }; With that in mind, and assuming that any profiling event source has to be controlled by a clockchip abstraction, we could turn on some internal __IRQF_* flag of our own into that irq's descriptor when registering a clock event device which advertises CLOCK_EVT_FEAT_PIPELINE (clockevents_config_and_register() and friends). We would then check such flag from generic_pipeline_irq_desc() to figure out whether we should save the CPU registers for profiling. That way, we would not have to mention it in the flags argument passed to request_percpu_irq_affinity_flags(), so no sweeping change ahead. Moreover, this would automagically cover all the cases where IRQF_TIMER is missing from the irq registration call, while keeping the flag we've just added strictly internal to the irq pipeline innards. -- Philippe. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: perf getting all-zeroes IP on STM32MP1 2026-09-07 11:34 ` Florian Bezdeka 2026-09-07 12:03 ` Gerte Hoogewerf 2026-09-07 12:41 ` Philippe Gerum @ 2026-09-07 12:51 ` Andrew MacPherson 2 siblings, 0 replies; 19+ messages in thread From: Andrew MacPherson @ 2026-09-07 12:51 UTC (permalink / raw) To: Florian Bezdeka; +Cc: xenomai, Philippe Gerum, Gerte Hoogewerf On Mon, 7 Sept 2026 at 13:34, Florian Bezdeka <florian.bezdeka@siemens.com> wrote: > > On Mon, 2026-09-07 at 10:57 +0200, Andrew MacPherson wrote: > > On Fri, 4 Sept 2026 at 16:08, Florian Bezdeka > > <florian.bezdeka@siemens.com> wrote: > > > > > > 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. > > > > > > @Philippe: Any additional thoughts? Should we take it already? > > > > > > > > > @Andrew: Could you please provide a formal patch with proper signed-off > > > and LLM notice (assuming agent means AI ;-)) targeting the dovetail 7.2. > > > branch? That should help to speed things up. Thanks! > > > > > > Florian > > > > > > -- > > > Siemens AG, Foundational Technologies > > > Linux Expert Center > > > > > > > > > > Hi Florian, > > > > I've submitted a patch now against dovetail 7.2, let me know if you > > need anything else and thanks for the help! > > > > > > Thanks! We might consider merging that while working on a better > solution - as suggested by Philippe. > > The following is build + boot tested on arm, arm64 and x86 using qemu. > Would be nice to get some early feedback. > > It should fix the problems reported by Andrew and Gerte. > > diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h > index a46872cf1384b..553243174368d 100644 > --- a/include/linux/clockchips.h > +++ b/include/linux/clockchips.h > @@ -16,6 +16,9 @@ > # include <linux/ktime.h> > # include <linux/notifier.h> > # include <linux/irqstage.h> > +# include <linux/preempt.h> > +# include <asm-generic/irq_regs.h> > +# include <asm/irq_pipeline.h> > > struct clock_event_device; > struct module; > @@ -259,9 +262,30 @@ struct clock_proxy_device { > > void tick_notify_proxy(void); > > +static inline void copy_timer_regs(void) > +{ > + struct pt_regs *regs = get_irq_regs(); > + struct irq_pipeline_data *p; > + > + if (!in_pipeline()) > + return; > + > + /* > + * Given our deferred dispatching model for regular IRQs, we > + * record the preempted context registers only for the latest > + * timer interrupt, so that the regular tick handler charges > + * CPU times properly. It is assumed that no other interrupt > + * handler cares for such information. > + */ > + p = raw_cpu_ptr(&irq_pipeline); > + arch_save_timer_regs(&p->tick_regs, regs); > +} > + > static inline > void clockevents_handle_event(struct clock_event_device *ced) > { > + copy_timer_regs(); > + > /* > * If called from the in-band stage, or for delivering a > * high-precision timer event to the out-of-band stage, call > diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c > index 85ec0cbf5fb1e..d160d99e32c5f 100644 > --- a/kernel/irq/pipeline.c > +++ b/kernel/irq/pipeline.c > @@ -1059,24 +1059,6 @@ bool handle_oob_irq(struct irq_desc *desc) > return true; > } > > -static inline > -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)) > - return; > - /* > - * Given our deferred dispatching model for regular IRQs, we > - * record the preempted context registers only for the latest > - * timer interrupt, so that the regular tick handler charges > - * CPU times properly. It is assumed that no other interrupt > - * handler cares for such information. > - */ > - p = raw_cpu_ptr(&irq_pipeline); > - arch_save_timer_regs(&p->tick_regs, regs); > -} > - > static __always_inline > struct irq_stage_data *switch_stage_on_irq(void) > { > @@ -1123,7 +1105,6 @@ void restore_stage_on_irq(struct irq_stage_data *prevd) > */ > int generic_pipeline_irq_desc(struct irq_desc *desc) > { > - struct pt_regs *regs = get_irq_regs(); > int irq; > > if (!desc) > @@ -1137,7 +1118,6 @@ int generic_pipeline_irq_desc(struct irq_desc *desc) > } > > trace_irq_pipeline_entry(irq); > - copy_timer_regs(desc, regs); > generic_handle_irq_desc(desc); > trace_irq_pipeline_exit(irq); > Just adding that this patch also appears to fix our issues with perf, tested as a backport against our 6.6.48 kernel. Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-09-09 8:50 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-04 13:26 perf getting all-zeroes IP on STM32MP1 Andrew MacPherson 2026-09-04 14:08 ` Florian Bezdeka 2026-09-04 15:17 ` Philippe Gerum 2026-09-04 15:55 ` Florian Bezdeka 2026-09-05 9:16 ` Philippe Gerum 2026-09-07 8:57 ` Andrew MacPherson 2026-09-07 11:34 ` Florian Bezdeka 2026-09-07 12:03 ` Gerte Hoogewerf 2026-09-07 12:41 ` Philippe Gerum 2026-09-07 14:39 ` Florian Bezdeka 2026-09-07 15:00 ` Philippe Gerum 2026-09-08 8:02 ` Florian Bezdeka 2026-09-08 8:16 ` Philippe Gerum 2026-09-08 8:46 ` Florian Bezdeka 2026-09-09 8:21 ` Philippe Gerum 2026-09-09 8:29 ` Florian Bezdeka 2026-09-09 8:49 ` Philippe Gerum 2026-09-08 8:11 ` Philippe Gerum 2026-09-07 12:51 ` Andrew MacPherson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.