CoreSight

coresight@lists.linaro.org

4 participants
2722 discussions

Re: [PATCH v4] coresight: coresight.h: add '*' to kernel-doc lines

by Leo Yan

On Sun, Aug 30, 2026 at 08:09:12PM -0700, Randy Dunlap wrote: > kernel-doc reports 2 (kernel-doc) lines in coresight.h that don't have > a beginning '*' in them, so fix these lines. > > Also convert struct coresight_trace_id_map to kernel-doc format to > remove another warning. > > Warning: include/linux/coresight.h:173 bad line: > connected to @src_port. NULL until the device is created > Warning: include/linux/coresight.h:177 bad line: > needs to be filtered. > Warning: include/linux/coresight.h:236 This comment starts with '/**', > but isn't a kernel-doc comment. > > Fixes: ec9903d6cc34 ("coresight: Add support for trace filtering by source") > Fixes: d49c9cf15f89 ("coresight: Rename connection members to make the direction explicit") > Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> Reviewed-by: Leo Yan <leo.yan(a)arm.com>

2 months, 1 week

[PATCH v9 00/13] fix several inconsistencies with sysfs configuration in etmX

by Yeoreum Yun

The current ETMx configuration via sysfs can lead to the following inconsistencies: - If a configuration is modified via sysfs while a perf session is active, the running configuration may differ between before a sched-out and after a subsequent sched-in. - If a perf session and sysfs session tries to enable concurrently, configuration from configfs could be corrupted (etm4). - There is chance to corrupt drvdata->config if perf session tries to enabled among handling cscfg_csdev_disable_active_config() in etm4_disable_sysfs() (etm4). To resolve these inconsistencies, the configuration should be separated into: - active_config, which is applied configuration for the current session - config, which stores the settings configured via sysfs. and apply configuration from configfs after taking a mode. Also, This patch set includes some small fixes: - missing trace id release in etm4x. - underflow issue for nrseqstate. - wrong check in etm4x_sspcicrn_present(). - missing call of cscfg_csdev_disable_active_config() This patch based on coresight tree's next Patch History ============= from v8 to v9: - add feat_csdev_lock guard interface. - set feat_csdev->drv_spinlock as NULL for etmv4_drvdata. - https://lore.kernel.org/all/20260629090007.1718746-1-yeoreum.yun@arm.com/ from v7 to v8: - accept @Leo Yan' suggestion to handle error. - small minor fixes following @Suzuki' suggestion. - https://lore.kernel.org/all/20260519154812.254884-1-yeoreum.yun@arm.com/ from v6 to v7: - rebase on coresight/next - add ETM_MAX_SEQ_TRANSITIONS define - remove redundant patch relavent cpu-hotplug as coresight-pm patch merged. - https://lore.kernel.org/all/20260422132203.977549-1-yeoreum.yun@arm.com/ from v5 to v6: - fix missing of calling cscfg_csdev_disable_active_config() - add rb & fixes tags. - add ss_status field in etm4x_drvdata to expose STATUS and PENDING bits. - https://lore.kernel.org/all/20260415165528.3369607-1-yeoreum.yun@arm.com/ from v4 to v5: - add rb-tag. - fix underflow issue for nrseqstate. - fix wrong check in etm4_sspcicrn_present(). - remove redundant fields on etmv4_save_state. - rename caps->ss_status to ss_cmp. - fix wrong location of etm4_release_trace_id. - https://lore.kernel.org/all/20260413142003.3549310-1-yeoreum.yun@arm.com/ from v3 to v4: - change etm_drvdata->spinlock type to raw_spin_lock_t - remove redundant call etmX_enable_hw() with starting_cpu() callsback. - fix missing trace id release. - add missing docs. - https://lore.kernel.org/all/20260412175506.412301-1-yeoreum.yun@arm.com/ from v2 to v3: - fix build error for etm3x. - fix checkpatch warning. - https://lore.kernel.org/all/20260410074310.2693385-1-yeoreum.yun@arm.com/ from v1 to v2 - rebased to v7.0-rc7. - introduce etmX_caps structure to save etmX's capabilities. - remove ss_status from etmv4_config. - modify active_config after taking a mode (perf/sysfs). - https://lore.kernel.org/all/20260317181705.2456271-1-yeoreum.yun@arm.com/ Yeoreum Yun (13): coresight: etm4x: fix wrong check of etm4x_sspcicrn_present() coresight: etm4x: fix underflow for usage of (nrseqstate - 1) coresight: etm4x: fix leaked trace id coresight: etm4x: fix inconsistencies with sysfs configuration coresight: etm4x: missing cscfg_csdev_disable_active_config() in perf enable coresight: etm3x: fix inconsistencies with sysfs configuration coresight: etm3x: change drvdata->spinlock type to raw_spin_lock_t coresight: etm3x: remove redundant cpu online check on etm_enable_sysfs() coresight: etm4x: introduce struct etm4_caps coresight: etm4x: exclude ss_status from drvdata->config coresight: etm4x: remove s_ex_level from config coresight: etm4x: remove redundant fields in etmv4_save_state coresight: etm3x: introduce struct etm_caps .../hwtracing/coresight/coresight-config.c | 18 +- .../hwtracing/coresight/coresight-config.h | 26 + drivers/hwtracing/coresight/coresight-etm.h | 48 +- .../coresight/coresight-etm3x-core.c | 96 ++-- .../coresight/coresight-etm3x-sysfs.c | 159 +++--- .../hwtracing/coresight/coresight-etm4x-cfg.c | 16 +- .../coresight/coresight-etm4x-core.c | 454 ++++++++++-------- .../coresight/coresight-etm4x-sysfs.c | 204 ++++---- drivers/hwtracing/coresight/coresight-etm4x.h | 204 ++++---- 9 files changed, 682 insertions(+), 543 deletions(-) base-commit: 98495b5a4d77dd22e106f462b76e1093a55b29a7 -- LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

2 months, 2 weeks

Re: [PATCH v2 11/14] perf auxtrace: cs-etm: Capture wrapped snapshots

by James Clark

On 24/08/2026 13:21, Adrian Hunter wrote: > On 21/08/2026 12:49, James Clark wrote: >> snapshot_has_wrapped() isn't implemented for Coresight, so when a wrap >> occurs we only save from offset 0 to current head, modulo buffer size, >> rather than capturing the whole buffer. Coresight drivers currently use >> a monotonic head pointer even in snapshot mode, which gives a lot more >> information for taking snapshots than a wrapped pointer. It means we >> don't need to search for data at the end of the buffer to guess if a >> wrap has occurred, and we can go back to saving partial buffers after a >> wrap by checking if the next size to take is less than the buffer >> size. >> >> The drivers will always be able to generate a monotonic head because >> TRBE has a bytes written counter, so even if we stop generating an AUX >> record on every overflow, we can check if the counter indicated an >> overwrite and offset head by a whole buffer size if it wrapped. This >> forces userspace to read the whole buffer rather than between last and >> current head. ETR and other sinks use software double buffering, so can >> continue as they are. >> >> Add a much simpler 'monotonic_snapshot_head' path for tracers to use, >> and use it for Coresight. >> >> Signed-off-by: James Clark <james.clark(a)linaro.org> >> --- >> tools/perf/arch/arm/util/cs-etm.c | 1 + >> .../perf/tests/shell/coresight/raw_dump_stress.sh | 5 ----- >> tools/perf/util/auxtrace.c | 22 ++++++++++++++++------ >> tools/perf/util/auxtrace.h | 2 ++ >> 4 files changed, 19 insertions(+), 11 deletions(-) >> >> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c >> index d36b7e41399b..dbf76dd3349e 100644 >> --- a/tools/perf/arch/arm/util/cs-etm.c >> +++ b/tools/perf/arch/arm/util/cs-etm.c >> @@ -918,6 +918,7 @@ struct auxtrace_record *cs_etm_record_init(int *err) >> ptr->itr.info_fill = cs_etm_info_fill; >> ptr->itr.snapshot_start = cs_etm_snapshot_start; >> ptr->itr.snapshot_finish = cs_etm_snapshot_finish; >> + ptr->itr.monotonic_snapshot_head = true; >> ptr->itr.reference = cs_etm_reference; >> ptr->itr.free = cs_etm_recording_free; >> ptr->itr.read_finish = auxtrace_record__read_finish; >> diff --git a/tools/perf/tests/shell/coresight/raw_dump_stress.sh b/tools/perf/tests/shell/coresight/raw_dump_stress.sh >> index bea70d825596..c9459ded4cbe 100755 >> --- a/tools/perf/tests/shell/coresight/raw_dump_stress.sh >> +++ b/tools/perf/tests/shell/coresight/raw_dump_stress.sh >> @@ -48,11 +48,6 @@ read -r size offset last_idx <<< "$(awk ' >> # everything was dumped. Allow 48 bytes difference to cover 3 frames: current >> # frame length, a partial frame and a final empty one, all of which aren't >> # dumped. >> -# >> -# TODO: for a single snapshot, offset should always be zero. However, we >> -# currently output AUX records in snapshot mode when we shouldn't, which >> -# increments the offset. Allow for that until it's fixed so we can test raw >> -# dumping. >> decode_size=$((1 + last_idx - offset)) >> if [ "$decode_size" -gt "$((size - 48))" ] && [ "$decode_size" -le "$((size))" ]; then >> echo "PASS: AUXTRACE buffer length matches dumped packet index" >> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c >> index 8ca872e6d465..e3c770b46e94 100644 >> --- a/tools/perf/util/auxtrace.c >> +++ b/tools/perf/util/auxtrace.c >> @@ -2051,9 +2051,9 @@ static int __auxtrace_mmap__read(struct mmap *map, >> bool snapshot, size_t snapshot_size) >> { >> struct auxtrace_mmap *mm = &map->auxtrace_mmap; >> - u64 head, old = mm->prev, offset, ref; >> + u64 head, old = mm->prev, offset, ref, size; >> unsigned char *data = mm->base; >> - size_t size, head_off, old_off, len1, len2, padding; >> + size_t head_off, old_off, len1, len2, padding; >> union perf_event ev; >> void *data1, *data2; >> int kernel_is_64_bit = perf_env__kernel_is_64_bit(env); >> @@ -2061,10 +2061,20 @@ static int __auxtrace_mmap__read(struct mmap *map, >> >> head = auxtrace_mmap__read_head(mm, kernel_is_64_bit); >> >> - if (snapshot && itr->snapshot_has_wrapped) { >> - err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); >> - if (err) >> - return err; >> + if (snapshot) { >> + if (itr->snapshot_has_wrapped) { >> + err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); >> + if (err) >> + return err; >> + } else if (itr->monotonic_snapshot_head) { >> + size = head - old; >> + /* Force a full buffer read if a wrap has occurred */ >> + if (size > mm->len) { >> + pr_debug3("%s: wrap detected, adjusting old from 0x%"PRIx64" to 0x%"PRIx64"\n", >> + __func__, old, head - mm->len); >> + old = head - mm->len; >> + } >> + } > > I feel like this is asking to keep the ->find_snapshot() callback. > i.e. basically the same thing, but add a helper that BTS and SPE > can call directly, like auxtrace_record__default_find_snapshot(). > For PT, add/use auxtrace_record__snapshot_fixup() that does the > common old/head adjustment. > > Then ->monotonic_snapshot_head is not needed. > Hmm yeah I suppose that makes sense. I can give it a go. >> } >> >> if (old == head) >> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h >> index a9d0c84184c5..971b817d3396 100644 >> --- a/tools/perf/util/auxtrace.h >> +++ b/tools/perf/util/auxtrace.h >> @@ -385,6 +385,7 @@ struct auxtrace_mmap_params { >> * @evlist: selected events list >> * @snapshot_wrapped_len: number of bits in @snapshot_wrapped >> * @snapshot_wrapped: bitmap indicating if each aux buffer has wrapped >> + * @monotonic_snapshot_head: driver doesn't wrap head pointer on overflow >> */ >> struct auxtrace_record { >> int (*recording_options)(struct auxtrace_record *itr, >> @@ -412,6 +413,7 @@ struct auxtrace_record { >> int snapshot_wrapped_len; >> unsigned long *snapshot_wrapped; >> int snapshot_search_bytes; >> + bool monotonic_snapshot_head; >> }; >> >> /** >> >

2 months, 2 weeks

Re: [PATCH v2 03/14] perf cs-etm: Turn on context packet timestamps in per-thread mode

by James Clark

On 21/08/2026 11:01, sashiko-bot(a)kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] Unconditionally enabling timestamps in per-thread mode breaks recording out-of-the-box on hardware without timestamp support (e.g., ETMv3 or ETMv4 without TRCIDR0.TSSIZE). > -- > > --- Patch [3]: [PATCH v2 03/14] perf cs-etm: Turn on context packet timestamps in per-thread mode --- > commit 23dc37429523715cdb26c5647b22b1a9dc0c89ab > Author: James Clark <james.clark(a)linaro.org> > > perf cs-etm: Turn on context packet timestamps in per-thread mode > > This enables context packet timestamps in per-thread mode, replacing the > previous unset default. It helps keep samples in order when supporting > multiple per-thread mode threads. > >> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c >> index acfdb3f53821e..d0486a164a700 100644 >> --- a/tools/perf/arch/arm/util/cs-etm.c >> +++ b/tools/perf/arch/arm/util/cs-etm.c >> @@ -453,13 +453,22 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, > [ ... ] >> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) { >> + /* >> + * Timestamps are required to interleave samples from different >> + * CPUs. >> + */ >> evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1); >> + /* Context IDs are required to associate trace to a process */ >> evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1); >> + } else { >> + /* >> + * Enable context packet timestamps only (no periodic >> + * timestamps). Even in per-thread mode with a single process we >> + * still need to correlate trace to a specific mmap around >> + * execs, which can be done with just context packet timestamps. >> + */ >> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", UINT64_MAX); > > [Severity: Medium] > Will unconditionally setting this timestamp break recording out-of-the-box on > hardware without timestamp support, like ETMv3? > > Looking at cs_etm_validate_timestamp(), if the timestamp config is set but > unsupported, it aborts the trace: > > tools/perf/arch/arm/util/cs-etm.c:cs_etm_validate_timestamp() { > if (!val) > return 0; > > if (cs_etm_get_version(cs_etm_pmu, cpu) == CS_ETMV3) { > pr_err("%s: timestamp not supported in ETMv3...\n"); > return -EINVAL; > } > } > > Does this mean users running perf record --per-thread on such hardware will > now get an error by default unless they explicitly pass timestamp=0? > Yes but the behavior is the same in per-CPU mode so this isn't a new issue. It's easily worked around and nobody has ever complained. >> } >> >> /* >

2 months, 3 weeks

Re: [PATCH v2 3/5] perf cs-etm: Add branch history to existing samples

by James Clark

On 17/08/2026 23:22, Amir Ayupov wrote: > Implement --itrace=L for CoreSight ETM: decode timestamped trace up to > each existing PMU sample and attach the branch history that led to it. > The sample keeps its own ip, callchain and event identity, and a sample > that already carries a branch stack is left alone. > > Samples are correlated with the trace by time, so this requires virtual > ETM timestamps that are correlated to perf time; timeless decoding is > rejected. The decode loop, which the previous patch left on its own in > cs_etm__process_timestamped_queues(), grows a timestamp argument and > stops once the decode frontier reaches it, so on return the > thread stack holds the branches that executed before the sample and none > that executed after. Attaching then reduces to the same > thread_stack__br_sample_late() call intel-pt uses. > > No explicit sample-to-queue matching is needed: > thread_stack__br_sample_late() keys on the thread, and the thread stack > is already emptied whenever the decoder reports a discontinuity. The one > case that was not covered is a queue whose trace runs out: flush the > thread stack there too, otherwise samples recorded after the last trace > would pick up stale history. > > Take the branch history when attaching it rather than leaving it in the > thread stack. With AUX pause and resume, a pause sample ends a completed > trace window and that window belongs to the sample. Execution while AUX > is paused is not traced, so retaining the window would let a later sample > reuse branches from before the untraced gap. Consuming it ensures that a > sample with no newly decoded trace gets an empty branch stack instead. > > As with intel-pt, the internal reconstruction ring is kept deeper than > the requested output depth to cover branches decoded between the sampled > ip and the point at which the sample time was recorded, so --itrace=L<n> > can actually return n entries. Kernel-inclusive trace gets the same > conservative 1024-entry headroom that intel-pt uses. > > Assisted-by: Devmate:GPT-5.6 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > tools/perf/util/cs-etm.c | 185 +++++++++++++++++++++++++++++++-- > tools/perf/util/thread-stack.c | 17 +++ > tools/perf/util/thread-stack.h | 1 + > 3 files changed, 195 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index 4d895f11deb7f..00407a80933e1 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -72,6 +72,11 @@ struct cs_etm_auxtrace { > bool use_callchain; > > int num_cpu; > + /* Output depth requested with --itrace=L<n> */ > + unsigned int br_stack_sz; > + /* Internal reconstruction depth, see cs_etm__br_stack_init() */ > + unsigned int br_stack_sz_plus;> + struct branch_stack *br_stack; > u64 latest_kernel_timestamp; > u32 auxtrace_type; > u32 branches_filter; > @@ -91,6 +96,7 @@ struct cs_etm_traceid_queue { > u64 kernel_start; > union perf_event *event_buf; > unsigned int br_stack_sz; > + unsigned int br_stack_sz_plus; > struct branch_stack *last_branch; > struct ip_callchain *callchain; > struct cs_etm_packet *prev_packet; > @@ -141,7 +147,8 @@ struct cs_etm_queue { > }; > > static int cs_etm__update_queues(struct cs_etm_auxtrace *etm); > -static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm); > +static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm, > + u64 timestamp); > static int cs_etm__flush_timestamped_queues(struct cs_etm_auxtrace *etm); > static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm, > pid_t tid); > @@ -165,6 +172,7 @@ static int cs_etm__metadata_set_trace_id(u8 trace_chan_id, u64 *cpu_metadata); > #define TO_QUEUE_NR(cs_queue_nr) (cs_queue_nr >> 16) > #define TO_TRACE_CHAN_ID(cs_queue_nr) (cs_queue_nr & 0x0000ffff) > #define SINK_UNSET ((u32) -1) > +#define MAX_TIMESTAMP (~0ULL) > > static u32 cs_etm__get_v7_protocol_version(u32 etmidr) > { > @@ -674,7 +682,8 @@ static int cs_etm__init_traceid_queue(struct cs_etm_queue *etmq, > if (!tidq->last_branch) > goto out_free; > > - tidq->br_stack_sz = etm->synth_opts.last_branch_sz; > + tidq->br_stack_sz = etm->br_stack_sz; > + tidq->br_stack_sz_plus = etm->br_stack_sz_plus; > } > > if (etm->synth_opts.callchain) { > @@ -794,7 +803,7 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm, > struct cs_etm_packet *tmp; > > if (etm->synth_opts.branches || etm->synth_opts.last_branch || > - etm->synth_opts.instructions) { > + etm->synth_opts.add_last_branch || etm->synth_opts.instructions) { > /* > * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for > * the next incoming packet. > @@ -963,7 +972,7 @@ static int cs_etm__flush_events(struct perf_session *session, > if (ret) > return ret; > > - ret = cs_etm__process_timestamped_queues(etm); > + ret = cs_etm__process_timestamped_queues(etm, MAX_TIMESTAMP); > if (ret) > return ret; > > @@ -1060,6 +1069,7 @@ static void cs_etm__free(struct perf_session *session) > zfree(&aux->metadata[i]); > > zfree(&aux->metadata); > + zfree(&aux->br_stack); > zfree(&aux); > } > > @@ -1597,7 +1607,8 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq, > u64 from, to; > int size; > > - if (!etm->synth_opts.branches && !etm->synth_opts.instructions) > + if (!etm->synth_opts.branches && !etm->synth_opts.instructions && > + !etm->synth_opts.add_last_branch) > return; > > if (!cs_etm__packet_has_taken_branch(tidq->prev_packet)) > @@ -1614,7 +1625,7 @@ static void cs_etm__add_stack_event(struct cs_etm_queue *etmq, > tidq->prev_packet->flags, from, to, size, > etmq->buffer->buffer_nr + 1, > etmq->etm->use_callchain, > - tidq->br_stack_sz, 0); > + tidq->br_stack_sz_plus, 0); > } else { > thread_stack__set_trace_nr(tidq->frontend_thread, > tidq->prev_packet->cpu, > @@ -2817,7 +2828,8 @@ static int cs_etm__update_queues(struct cs_etm_auxtrace *etm) > return ret; > } > > -static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > +static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm, > + u64 timestamp) > { > int ret = 0; > unsigned int cs_queue_nr, queue_nr; > @@ -2831,6 +2843,9 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > if (!etm->heap.heap_cnt) > break; > > + if (etm->heap.heap_array[0].ordinal >= timestamp) > + break; > + > /* Take the entry at the top of the min heap */ > cs_queue_nr = etm->heap.heap_array[0].queue_nr; > queue_nr = TO_QUEUE_NR(cs_queue_nr); > @@ -2878,8 +2893,25 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > * No more auxtrace_buffers to process in this etmq, simply > * move on to another entry in the auxtrace_heap. > */ > - if (!ret) > + if (!ret) { > + /* > + * The trace for this physical queue is exhausted. Drop > + * branch history for every trace ID it carried so that > + * samples arriving later cannot pick up entries decoded > + * before the gap. > + */ > + if (etm->synth_opts.add_last_branch) { > + struct int_node *inode; > + > + intlist__for_each_entry(inode, etmq->traceid_queues_list) { > + int idx = (int)(intptr_t)inode->priv; > + > + tidq = etmq->traceid_queues[idx]; > + thread_stack__flush(tidq->frontend_thread); > + } > + } > continue; > + } > > ret = cs_etm__decode_data_block(etmq); > if (ret) > @@ -3011,6 +3043,116 @@ static int cs_etm__process_switch_cpu_wide(struct cs_etm_auxtrace *etm, > return 0; > } > > +static bool cs_etm__tracing_kernel(struct cs_etm_auxtrace *etm, > + struct perf_session *session) > +{ > + struct evsel *evsel; > + > + evlist__for_each_entry(session->evlist, evsel) { > + if (evsel->core.attr.type == etm->pmu_type && > + !evsel->core.attr.exclude_kernel) > + return true; > + } > + > + return false; > +} > + > +static int cs_etm__br_stack_init(struct cs_etm_auxtrace *etm, > + struct perf_session *session) > +{ > + struct evsel *evsel; > + > + evlist__for_each_entry(session->evlist, evsel) { > + /* > + * Only timestamped events can be matched against the decoded > + * trace, so do not advertise a branch stack on any other. > + */ > + if (!(evsel->core.attr.sample_type & PERF_SAMPLE_TIME)) > + continue; Do you not also want to check for the coresight virtual timestamp option here for the same reason? Although I do see that checked somewhere else below. > + if (!(evsel->core.attr.sample_type & PERF_SAMPLE_BRANCH_STACK)) > + evsel->synth_sample_type |= PERF_SAMPLE_BRANCH_STACK; > + } > + > + /* > + * Additional branch stack depth to cater for the branches decoded > + * between the sampled ip and the point at which the sample time was > + * recorded. Those are trimmed by thread_stack__br_sample_late(), so How does the trimmer know what branches came after an IP? I could understand trimming between two timestamps, but not between one IP and one timestamp. > + * the extra depth keeps the requested output depth achievable. If > + * kernel space is not traced, only the branch into the kernel needs > + * to be accounted for. > + */ I'm not sure if this description is missing something, but I can't understand why this needs to be done. Or how kernel tracing affects it. > + if (cs_etm__tracing_kernel(etm, session)) > + etm->br_stack_sz_plus += 1024; > + else > + etm->br_stack_sz_plus += 1; > + > + etm->br_stack = zalloc(sizeof(struct branch_stack) + > + etm->br_stack_sz * sizeof(struct branch_entry)); > + if (!etm->br_stack) > + return -ENOMEM; > + > + return 0; > +} > + > +/* > + * Add decoded branch history to an existing sample. The sample keeps its own > + * ip, callchain and event identity; only an absent branch stack is filled in. > + */ > +static int cs_etm__process_sample(struct cs_etm_auxtrace *etm, > + struct perf_session *session, > + struct perf_sample *sample) > +{ > + struct machine *machine = &session->machines.host; > + struct thread *thread; > + int err; > + > + if (!etm->synth_opts.add_last_branch || sample->branch_stack || > + !sample->ip || !sample->time || sample->time == (u64)-1) > + return 0; > + > + /* Adding branch history to existing samples supports the host only */ > + if (sample->cpumode == PERF_RECORD_MISC_GUEST_KERNEL || > + sample->cpumode == PERF_RECORD_MISC_GUEST_USER) > + return 0; > + > + err = cs_etm__update_queues(etm); > + if (err) > + return err; > + > + /* > + * Decode every queue up to this sample's time. Afterwards the thread > + * stack holds the branches that executed before the sample, and > + * nothing that executed after it. > + */ > + err = cs_etm__process_timestamped_queues(etm, sample->time); > + if (err) > + return err; > + > + thread = machine__findnew_thread(machine, sample->pid, sample->tid); > + if (!thread) > + return -ENOMEM; > + > + /* > + * Take the branch history rather than copying it. The trace window > + * belongs to the sample that ends it, so once it has been attached a > + * later sample with nothing newly decoded finds an empty stack rather > + * than being given an earlier window's branches. That is the common > + * case whenever the trace is duty cycled, by AUX pause/resume or by > + * ETM strobing. > + */ > + thread_stack__br_sample_late(thread, sample->cpu, etm->br_stack, > + etm->br_stack_sz, sample->ip, > + machine__kernel_start(machine)); > + thread_stack__br_stack_consume(thread, sample->cpu); > + > + if (etm->br_stack->nr) > + sample->branch_stack = etm->br_stack; How does this work? We have a queue for each CPU, and decoding happens in parallel (kind of), but when a Perf sample arrives we just attach the stack from the global br_stack? Shouldn't we look at the CPU of the sample and use cs_etm__get_queue() to get the right queue and branch stack? Maybe it's not functionally different if br_stack always happens to be set from the queue related to the last Perf sample, but it would be nicer to not have to assume. You might want to check "[PATCH 00/14] perf cs-etm: Per-thread mode fixes and snapshot wrap support" because it changes to a per-CPU queue even for per-thread mode which could help. > + > + thread__put(thread); > + > + return 0; > +} > + > static int cs_etm__process_event(struct perf_session *session, > union perf_event *event, > struct perf_sample *sample, > @@ -3049,6 +3191,9 @@ static int cs_etm__process_event(struct perf_session *session, > case PERF_RECORD_SWITCH_CPU_WIDE: > return cs_etm__process_switch_cpu_wide(etm, event); > > + case PERF_RECORD_SAMPLE: > + return cs_etm__process_sample(etm, session, sample); > + Don't we want to generalise this and process trace up to the timestamp of _any_ event. Can we move the cs_etm__process_timestamped_queues() call into cs_etm__process_event(). Surely we want to always decode up to any event if coresight virtual timestamps are enabled? That way we access the right mmaps too and the decode order doesn't change depending on the branch stack options. > case PERF_RECORD_AUX: > /* > * Record the latest kernel timestamp available in the header > @@ -3752,11 +3897,34 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > > etm->use_thread_stack = etm->synth_opts.thread_stack || > etm->synth_opts.last_branch || > + etm->synth_opts.add_last_branch || > etm->synth_opts.callchain; > > etm->use_callchain = etm->synth_opts.thread_stack || > etm->synth_opts.callchain; > > + if (etm->synth_opts.last_branch || etm->synth_opts.add_last_branch) { > + etm->br_stack_sz = etm->synth_opts.last_branch_sz; > + etm->br_stack_sz_plus = etm->br_stack_sz; > + } > + > + if (etm->synth_opts.add_last_branch) { > + /* > + * Existing samples are matched to decoded trace by time, so > + * the trace must carry timestamps that are correlated to perf > + * time and the queues must be decoded in time order. > + */ > + if (etm->timeless_decoding || !etm->has_virtual_ts) { > + pr_err("CS ETM Trace: --itrace=L requires virtual timestamped trace\n"); > + err = -EINVAL; > + goto err_free_queues; > + } > + > + err = cs_etm__br_stack_init(etm, session); > + if (err) > + goto err_free_queues; > + } > + > err = cs_etm__synth_events(etm, session); > if (err) > goto err_free_queues; > @@ -3812,6 +3980,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > auxtrace_queues__free(&etm->queues); > session->auxtrace = NULL; > err_free_etm: > + zfree(&etm->br_stack); > zfree(&etm); > err_free_metadata: > /* No need to check @metadata[j], free(NULL) is supported */ > diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c > index 1360f44421ef8..2713a2ad70b69 100644 > --- a/tools/perf/util/thread-stack.c > +++ b/tools/perf/util/thread-stack.c > @@ -614,6 +614,23 @@ void thread_stack__sample_late(struct thread *thread, int cpu, > } > } > > +/* > + * Branch history belongs to the sample that ends the trace window, so a > + * decoder that attaches it to an existing sample should take it rather than > + * copy it. A later sample with no newly decoded trace then finds an empty > + * branch stack instead of the previous window's branches. > + */ > +void thread_stack__br_stack_consume(struct thread *thread, int cpu) > +{ > + struct thread_stack *ts = thread__stack(thread, cpu); > + > + if (!ts || !ts->br_stack_rb) > + return; > + > + ts->br_stack_pos = 0; > + ts->br_stack_rb->nr = 0; > +} > + > void thread_stack__br_sample(struct thread *thread, int cpu, > struct branch_stack *dst, unsigned int sz) > { > diff --git a/tools/perf/util/thread-stack.h b/tools/perf/util/thread-stack.h > index b3cd09beb62f0..2aec292bd1bcb 100644 > --- a/tools/perf/util/thread-stack.h > +++ b/tools/perf/util/thread-stack.h > @@ -88,6 +88,7 @@ void thread_stack__sample(struct thread *thread, int cpu, struct ip_callchain *c > void thread_stack__sample_late(struct thread *thread, int cpu, > struct ip_callchain *chain, size_t sz, u64 ip, > u64 kernel_start); > +void thread_stack__br_stack_consume(struct thread *thread, int cpu); > void thread_stack__br_sample(struct thread *thread, int cpu, > struct branch_stack *dst, unsigned int sz); > void thread_stack__br_sample_late(struct thread *thread, int cpu,

2 months, 3 weeks

Re: [PATCH v2 2/5] perf cs-etm: Split up cs_etm__process_timestamped_queues()

by James Clark

On 17/08/2026 23:22, Amir Ayupov wrote: > cs_etm__process_timestamped_queues() currently does three things: it seeds > the auxtrace heap with one entry per queue, it decodes until the heap is > empty, and it then walks every traceID queue to flush whatever is left in > the branch stacks. That is fine while the only caller is > cs_etm__flush_events(), which runs once, but it does not survive the > function being called repeatedly. > > Seeding cannot be repeated because a queue that still holds a heap slot > would be seeded again, adding duplicate entries and growing the heap > without bound. Flushing cannot be repeated either, because ending a block > finalises state that later trace still needs. > > Move both out. Seeding becomes cs_etm__update_queues(), gated on > queues.new_data so it only runs when new AUX data has been queued, with > etmq->on_heap tracking whether a queue currently occupies a heap slot; > this mirrors intel_pt_update_queues() and intel_pt_queue::on_heap. > Flushing becomes cs_etm__flush_timestamped_queues(). What remains is the > decode loop on its own, which a later patch can then drive incrementally. > > No functional change: the sole caller performs the same three steps in the > same order. > > Assisted-by: Devmate:GPT-5.6 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> LGTM but I'd still like to run the test. > --- > tools/perf/util/cs-etm.c | 71 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 61 insertions(+), 10 deletions(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index 114b3cd2da495..4d895f11deb7f 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -136,9 +136,13 @@ struct cs_etm_queue { > */ > struct intlist *own_traceid_list; > u32 sink_id; > + /* Whether this queue currently occupies a slot in etm->heap */ > + bool on_heap; > }; > > +static int cs_etm__update_queues(struct cs_etm_auxtrace *etm); > static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm); > +static int cs_etm__flush_timestamped_queues(struct cs_etm_auxtrace *etm); > static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm, > pid_t tid); > static int cs_etm__get_data_block(struct cs_etm_queue *etmq); > @@ -939,6 +943,8 @@ static int cs_etm__flush_events(struct perf_session *session, > struct cs_etm_auxtrace *etm = container_of(session->auxtrace, > struct cs_etm_auxtrace, > auxtrace); > + int ret; > + > if (dump_trace) > return 0; > > @@ -953,7 +959,15 @@ static int cs_etm__flush_events(struct perf_session *session, > return cs_etm__process_timeless_queues(etm, -1); > } > > - return cs_etm__process_timestamped_queues(etm); > + ret = cs_etm__update_queues(etm); > + if (ret) > + return ret; > + > + ret = cs_etm__process_timestamped_queues(etm); > + if (ret) > + return ret; > + > + return cs_etm__flush_timestamped_queues(etm); > } > > static void cs_etm__free_traceid_queues(struct cs_etm_queue *etmq) > @@ -1330,6 +1344,8 @@ static int cs_etm__queue_first_cs_timestamp(struct cs_etm_auxtrace *etm, > */ > cs_queue_nr = TO_CS_QUEUE_NR(queue_nr, trace_chan_id); > ret = auxtrace_heap__add(&etm->heap, cs_queue_nr, cs_timestamp); > + if (!ret) > + etmq->on_heap = true; > out: > return ret; > } > @@ -2767,23 +2783,30 @@ static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm, > return 0; > } > > -static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > +/* > + * Seed the heap with one entry from each queue that is not already > + * represented in it, so that decoding proceeds in time order across all > + * queues. Only queues that have newly queued data need to be considered. > + */ > +static int cs_etm__update_queues(struct cs_etm_auxtrace *etm) > { > int ret = 0; > - unsigned int cs_queue_nr, queue_nr, i; > - u8 trace_chan_id; > - u64 cs_timestamp; > - struct auxtrace_queue *queue; > + unsigned int i; > struct cs_etm_queue *etmq; > - struct cs_etm_traceid_queue *tidq; > + > + if (!etm->queues.new_data) > + return 0; > + > + etm->queues.new_data = false; > > /* > * Pre-populate the heap with one entry from each queue so that we can > - * start processing in time order across all queues. > + * start processing in time order across all queues. Skip queues that > + * already occupy a heap slot, otherwise they would be added twice. > */ > for (i = 0; i < etm->queues.nr_queues; i++) { > etmq = etm->queues.queue_array[i].priv; > - if (!etmq) > + if (!etmq || etmq->on_heap) > continue; > > ret = cs_etm__queue_first_cs_timestamp(etm, etmq, i); > @@ -2791,6 +2814,19 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > return ret; > } > > + return ret; > +} > + > +static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > +{ > + int ret = 0; > + unsigned int cs_queue_nr, queue_nr; > + u8 trace_chan_id; > + u64 cs_timestamp; > + struct auxtrace_queue *queue; > + struct cs_etm_queue *etmq; > + struct cs_etm_traceid_queue *tidq; > + > while (1) { > if (!etm->heap.heap_cnt) > break; > @@ -2807,6 +2843,7 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > * to process it. > */ > auxtrace_heap__pop(&etm->heap); > + etmq->on_heap = false; > > tidq = cs_etm__etmq_get_traceid_queue(etmq, trace_chan_id); > if (!tidq) { > @@ -2874,7 +2911,21 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > */ > cs_queue_nr = TO_CS_QUEUE_NR(queue_nr, trace_chan_id); > ret = auxtrace_heap__add(&etm->heap, cs_queue_nr, cs_timestamp); > + if (ret) > + goto out; > + etmq->on_heap = true; > } > +out: > + return ret; > +} > + > +/* Flush any branch stack entries left over once all trace is decoded */ > +static int cs_etm__flush_timestamped_queues(struct cs_etm_auxtrace *etm) > +{ > + int ret = 0; > + unsigned int i; > + struct cs_etm_queue *etmq; > + struct cs_etm_traceid_queue *tidq; > > for (i = 0; i < etm->queues.nr_queues; i++) { > struct int_node *inode; > @@ -2893,7 +2944,7 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm) > return ret; > } > } > -out: > + > return ret; > } >

2 months, 3 weeks

Re: [PATCH v2 4/5] perf test cs-etm: Test branch history on existing samples

by James Clark

On 17/08/2026 23:22, Amir Ayupov wrote: > Add a CoreSight shell test for --itrace=L. Record timestamped ETM trace > with explicit -T sample timestamps and AUX pause/resume events, then > check that the pause samples carry both a multi-frame callchain and a > non-empty branch stack for each of the workload's two processes. > > Decode the same recording with L4 and L64 and reject any branch stack > deeper than the requested depth. > > The test skips when cs_etm is absent, when not run as root, or when the > recording turns out to lack virtual timestamps. It exercises the > timestamp-gated path and the requested-depth bound; it does not attempt > to verify that the attached history is correlated to the sample. > > Assisted-by: Devmate:GPT-5.6 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > .../tests/shell/coresight/add_last_branch.sh | 203 ++++++++++++++++++ > 1 file changed, 203 insertions(+) > create mode 100755 tools/perf/tests/shell/coresight/add_last_branch.sh > > diff --git a/tools/perf/tests/shell/coresight/add_last_branch.sh b/tools/perf/tests/shell/coresight/add_last_branch.sh > new file mode 100755 > index 0000000000000..6f09e720abe80 > --- /dev/null > +++ b/tools/perf/tests/shell/coresight/add_last_branch.sh > @@ -0,0 +1,203 @@ > +#!/bin/bash -e > +# SPDX-License-Identifier: GPL-2.0 > +# CoreSight branch history on existing samples (exclusive) > + > +perf list pmu | grep -q 'cs_etm//' || exit 2 > + > +if [ "$(id -u)" != 0 ]; then > + echo "[Skip] No root permission" > + exit 2 > +fi > + > +tmpdir=$(mktemp -d /tmp/perf-cs-add-last-branch.XXXXX) > + > +cleanup() > +{ > + rm -rf "$tmpdir" > + trap - EXIT TERM INT > +} > + > +# shellcheck disable=SC2317 # Called through trap. > +trap_cleanup() > +{ > + cleanup > + exit 1 > +} > +trap trap_cleanup EXIT TERM INT > + > +record_data() > +{ > + local cf="$tmpdir/ctl" > + local af="$tmpdir/ack" > + > + mkfifo "$cf" "$af" > + > + # Pin to one CPU so proc1 and proc2 alternate in one per-CPU trace > + # buffer. Start disabled and use the control FIFO to record only the > + # workload, not perf test setup and teardown. This is more of a what comment than a why. I got that we were doing that, but the reason I left the comment on V1 was because I couldn't see why context switching is related to branch history. Doesn't the test still test the same thing if you record a single process without -C? It's a bit hard to see what behavior the test is exercising. I see it also adds call-graph=fp, but it doesn't look for symbol names. How is testing for any non zero callchain related to coresight unless we also check the coresight branch stack matches it exactly via symbol names? Seems like it's just testing some other part of Perf here. I would expect some references to the named symbols "context_switch_loop_proc2" from the workload, but I don't see them. It also seems to be very interested in timestamps as per the commit message. But how do you know it's not attaching the branch stack from proc1 to proc2 because it gets the timestamps wrong? If it's easier to add a new workload with a deterministic branch history written in asm after a deterministic call chain we can do that. Then the test can test if a fragment of the branch history appears after an end fragment of the callchain. > + if perf record -T -o "$tmpdir/data" -C 0 -D -1 \ > + --control fifo:"$cf","$af" \ > + -e cs_etm/aux-action=start-paused/u \ > + -e cycles/aux-action=resume,period=550019/u \ > + -e cycles/aux-action=pause,period=100003,call-graph=fp/u -- \ > + taskset --cpu-list 0 perf test --record-ctl fifo:"$cf","$af" \ > + -w context_switch_loop 10000 \ > + >/dev/null 2>"$tmpdir/stderr"; then > + return 0 > + fi > + > + echo "Failed to record ETM trace with AUX pause/resume" >&2 > + cat "$tmpdir/stderr" >&2 > + return 1 > +} > + > +decode() > +{ > + local size=$1 > + local output=$2 > + > + if perf script -i "$tmpdir/data" --itrace="L$size" \ > + -F comm,pid,tid,event,ip,brstack >"$output" \ > + 2>"$tmpdir/stderr"; then > + return 0 > + fi > + > + if grep -q "itrace=L requires virtual timestamped trace" \ > + "$tmpdir/stderr"; then > + echo "[Skip] Virtual CoreSight timestamps are not available" > + cleanup > + exit 2 > + fi > + > + cat "$tmpdir/stderr" >&2 > + return 1 > +} > + > +check_process_samples() > +{ > + local output=$1 > + local comm > + > + # Expect each process to have a pause-event sample followed by at least > + # one branch entry in 0xFROM/0xTO/... form. Can we have an exact copy paste of the output instead of the text description? I still can't tell if what I'm seeing is expected based on this. For example I get this, which you could paste verbatim into the test: proc2 armv8_pmuv3_0/cycles,aux-action=pause,period=100003,call-graph=fp/u: ffff800080021440 ffff8000813a6a04 (This is output from V1, I couldn't run V2 because of the invalid group desc issue) > + for comm in proc1 proc2; do > + awk -v comm="$comm" ' > + $1 == comm && /cycles\/aux-action=pause/ { > + in_sample = 1 > + next > + } > + !NF { > + in_sample = 0 > + next > + } > + in_sample && /0x[[:xdigit:]]+\/0x[[:xdigit:]]+\// { > + found = 1 > + } > + END { exit !found } > + ' "$output" || { > + echo "No pause-event branch stack found for $comm" >&2 > + grep -A 4 "^$comm .*cycles/aux-action=pause" "$output" \ > + | head -n 20 >&2 || true > + return 1 > + } > + done > +} > + > +check_callchains() > +{ > + local output="$tmpdir/script-callchain" > + local comm > + > + if ! perf script -i "$tmpdir/data" -F comm,event,ip >"$output" \ > + 2>"$tmpdir/stderr"; then > + echo "Failed to dump pause-event callchains" >&2 > + cat "$tmpdir/stderr" >&2 > + return 1 > + fi > + > + # Expect a pause-event header for each process followed by at least two > + # indented instruction-pointer frames. Ditto > + for comm in proc1 proc2; do > + awk -v comm="$comm" ' > + $1 == comm && /cycles\/aux-action=pause/ { > + in_sample = 1 > + frames = 0 > + next > + } > + !NF { > + if (in_sample && frames >= 2) > + found = 1 > + in_sample = 0 > + next > + } > + in_sample && /^[[:space:]]+[[:xdigit:]]+([[:space:]]|$)/ { > + frames++ > + } > + END { > + if (in_sample && frames >= 2) > + found = 1 > + exit !found > + } > + ' "$output" || { > + echo "No multi-frame pause-event callchain found for $comm" >&2 > + grep -A 8 "^$comm .*cycles/aux-action=pause" "$output" \ > + | head -n 40 >&2 || true > + return 1 > + } > + done > +} > + > +check_branch_stacks() > +{ > + local output=$1 > + local max_entries=$2 > + > + local ret > + > + if awk -v max="$max_entries" ' > + /0x[[:xdigit:]]+\/0x[[:xdigit:]]+\// { > + entries = 0 > + for (i = 1; i <= NF; i++) > + if ($i ~ /^0x[[:xdigit:]]+\/0x[[:xdigit:]]+\//) > + entries++ > + if (entries) > + found = 1 > + if (entries > max) { > + status = 2 > + exit > + } > + } > + END { > + if (status) > + exit status > + if (!found) > + exit 1 > + } > + ' "$output"; then > + return 0 > + else > + ret=$? > + fi > + > + case $ret in > + 1) echo "No ETM branch stacks found" >&2 ;; > + 2) echo "Branch stack exceeds requested L$max_entries depth" >&2 ;; > + esac > + # Expected decoded pause-event lines contain at most L<n> branch entries. > + grep 'cycles/aux-action=pause' "$output" | head -n 5 >&2 || true > + return 1 > +} > + > +record_data > +check_callchains > + > +decode 4 "$tmpdir/script-L4" > +check_process_samples "$tmpdir/script-L4" > +check_branch_stacks "$tmpdir/script-L4" 4 > + > +decode 64 "$tmpdir/script-L64" > +check_process_samples "$tmpdir/script-L64" > +check_branch_stacks "$tmpdir/script-L64" 64 > + > +cleanup > +exit 0

2 months, 3 weeks

Re: [PATCH v2 0/5] perf: Add CoreSight branch history to existing samples

by James Clark

On 17/08/2026 23:22, Amir Ayupov wrote: > CoreSight ETM can synthesize branch samples from an instruction trace, but > context-sensitive PGO needs the branch history leading to an existing PMU > sample together with that sample's event identity and callchain. This series > implements that mode as --itrace=L, following the corresponding Intel PT > behavior. > > The series first separates timestamped queue setup and teardown from the ETM > decode loop so decoding can stop at an existing sample's timestamp. It then > reconstructs branch history in the thread stack and attaches it to eligible > samples without replacing their IP, event, or callchain. The attached history > is consumed after use so a later sample cannot reuse an earlier trace window. > > This enables a context-sensitive PGO workflow where a cycles event supplies a > frame-pointer callchain while duty-cycled ETM supplies the path leading to the > sample. A dlfilter removes samples for which no ETM history was available, and > the documentation describes the complete recording and decoding workflow. > > Changes since v1: > > - Rebased onto perf-tools-next at d17c5b770972. > - Dropped the HEADER_GROUP_DESC reader workaround. The issue is in the writer > and should be fixed separately. Doesn't it still need to be part of this patchset regardless of where the issue is? Running the new test is blocked on this. Also is this a regression? I noticed there was no fixes: tag on V1 for this commit. > - Dropped the branch-stack hw_idx patch after review established that zero is > appropriate for age-ordered CoreSight branch stacks. This doesn't seem to match what happened. The original patch initialized all branch stacks to hw_idx = -1, which I think was a good fix so shouldn't have been dropped. Coresight still sets -1 which I said should be changed to 0, but there doesn't seem to be a change for that in V2. So now we're missing both changes. > - Dropped the local wrapped branch-stack copy fix in favor of upstream commit > ab9c84d1cd59 ("perf thread-stack: Fix heap buffer overflow on branch stack > wrap copy"). > - Added James Clark's Reviewed-by tag to the dlfilter patch. > - Consume branch history after attaching it so samples with no newly decoded > trace cannot reuse a window from before an untraced AUX pause interval. > - Flush all trace-ID frontend thread stacks when their physical ETM queue is > exhausted, preventing stale history from surviving a trace gap. > - Check every matching CoreSight event when deciding whether kernel trace is > enabled. Were these changes due to my comment about the test not working? Might be worth some discussion about what the issue was on the V1 thread so it's easier to follow along with why these changes were made to V2. > - Reworked the shell test to use FIFO recording control, removed the invalid > bare timestamp option, reduced the workload to 10000 iterations, and check > proc1 and proc2 callchains independently with bounded failure diagnostics. > - Renamed decoded test outputs to script-L4 and script-L64. > > Amir Ayupov (5): > perf dlfilter: Add non-empty branch stack filter > perf cs-etm: Split up cs_etm__process_timestamped_queues() > perf cs-etm: Add branch history to existing samples > perf test cs-etm: Test branch history on existing samples > Documentation: coresight: Document context-sensitive PGO workflow > > .../trace/coresight/coresight-perf.rst | 62 +++++ > tools/perf/Makefile.perf | 1 + > .../dlfilters/dlfilter-nonempty-brstack.c | 26 ++ > .../tests/shell/coresight/add_last_branch.sh | 203 ++++++++++++++ > tools/perf/util/cs-etm.c | 252 ++++++++++++++++-- > tools/perf/util/thread-stack.c | 17 ++ > tools/perf/util/thread-stack.h | 1 + > 7 files changed, 546 insertions(+), 16 deletions(-) > create mode 100644 tools/perf/dlfilters/dlfilter-nonempty-brstack.c > create mode 100755 tools/perf/tests/shell/coresight/add_last_branch.sh > > > base-commit: d17c5b770972854a4fe4cf5cc22e17eb21cdc787

2 months, 3 weeks

Re: [PATCH 4/9] perf dlfilter: Add non-empty branch stack filter

by James Clark

On 03/08/2026 10:06, Amir Ayupov wrote: > --itrace=L adds decoded branch history to existing samples, but a sample > that was recorded while the decoder had no trace for that thread keeps an > empty branch stack. Consumers of the resulting perf script output, such > as profile generators for context-sensitive PGO, have no use for those > samples. > > Add an opt-in dlfilter that drops samples whose parsed branch stack is > empty, so users can exclude them without changing default sample > semantics. Build and install it alongside perf's existing dlfilters. > > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > tools/perf/Makefile.perf | 1 + > .../dlfilters/dlfilter-nonempty-brstack.c | 26 +++++++++++++++++++ > 2 files changed, 27 insertions(+) > create mode 100644 tools/perf/dlfilters/dlfilter-nonempty-brstack.c > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf > index 0031112c036e8..aeb8085b0756d 100644 > --- a/tools/perf/Makefile.perf > +++ b/tools/perf/Makefile.perf > @@ -427,6 +427,7 @@ PROGRAMS += $(OUTPUT)$(LIBJVMTI) > endif > > DLFILTERS := dlfilter-test-api-v0.so dlfilter-test-api-v2.so dlfilter-show-cycles.so > +DLFILTERS += dlfilter-nonempty-brstack.so > DLFILTERS := $(patsubst %,$(OUTPUT)dlfilters/%,$(DLFILTERS)) > > # what 'all' will build and 'install' will install, in perfexecdir > diff --git a/tools/perf/dlfilters/dlfilter-nonempty-brstack.c b/tools/perf/dlfilters/dlfilter-nonempty-brstack.c > new file mode 100644 > index 0000000000000..9e66205b841d5 > --- /dev/null > +++ b/tools/perf/dlfilters/dlfilter-nonempty-brstack.c > @@ -0,0 +1,26 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * dlfilter-nonempty-brstack.c: Filter out samples with no branch stack > + * Copyright (c) 2026, Meta Platforms, Inc. > + */ > +#include <stddef.h> > + > +#include <perf/perf_dlfilter.h> > + > +int filter_event(void *data, const struct perf_dlfilter_sample *sample, void *ctx) > +{ > + /* Return 1 to filter out the sample, 0 to keep it */ > + return !sample->brstack_nr; > +} > + > +const char *filter_description(const char **long_description) > +{ > + static char *long_desc = > + "Instruction trace decoders can add branch history to existing " > + "samples, but samples that were recorded while no trace was " > + "being collected get an empty branch stack. Filter those out so " > + "that only samples carrying branch history remain."; > + > + *long_description = long_desc; > + return "Keep only samples with a non-empty branch stack"; > +} Reviewed-by: James Clark <james.clark(a)linaro.org>

3 months

Re: [PATCH 2/9] perf thread-stack: Report branch stack hw_idx as not available

by Arnaldo Carvalho de Melo

On Wed, Aug 12, 2026 at 06:48:24PM +0300, Adrian Hunter wrote: > On 11/08/2026 18:58, Adrian Hunter wrote: > > On 03/08/2026 12:06, Amir Ayupov wrote: > >> thread_stack__br_sample() and thread_stack__br_sample_late() fill a > >> caller-supplied branch_stack that is typically allocated with zalloc(), > >> leaving hw_idx as 0. Zero is a valid hardware index, so consumers that > >> honour PERF_SAMPLE_BRANCH_HW_INDEX see a reconstructed branch stack > >> claiming to start at LBR TOS entry 0. > >> > >> These branch stacks are reconstructed from instruction trace and have no > >> hardware index at all. Set hw_idx to -1ULL, which is the established way > >> to say "not available" and matches what intel-pt and cs-etm already put > >> in the branch stacks they synthesise directly. > >> > >> Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > > > > Fixes tag? > > > > Otherwise: > > > > Reviewed-by: Adrian Hunter <adrian.hunter(a)intel.com> > > On second thoughts, it seems that hw_idx is only used for stitching > LBRs which is anyway disabled by default and only enabled by --stitch-lbr. > > Setting -1ULL will prevent has_stitched_lbr() making a match, but we can > rely on the user to decide that for themselves via --stitch-lbr. This is one of those options that few people use as its so specialized, do you think we could auto-enable it if we notice it is a good idea for some specific machine and request from the user? I.e. user requests callchains, unlimited or with a limit that is more than what we can do without stitching: we auto stich? - Arnaldo > So, in fact, it doesn't look like this change should be needed? > > > > >> --- > >> tools/perf/util/thread-stack.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c > >> index c5ce741b07446..1a3dffa83bde2 100644 > >> --- a/tools/perf/util/thread-stack.c > >> +++ b/tools/perf/util/thread-stack.c > >> @@ -624,6 +624,7 @@ void thread_stack__br_sample(struct thread *thread, int cpu, > >> unsigned int nr; > >> > >> dst->nr = 0; > >> + dst->hw_idx = -1ULL; > >> > >> if (!ts) > >> return; > >> @@ -686,6 +687,7 @@ void thread_stack__br_sample_late(struct thread *thread, int cpu, > >> bool start = false; > >> > >> dst->nr = 0; > >> + dst->hw_idx = -1ULL; > >> > >> if (!ts) > >> return; > >

3 months

← Newer
1
...
9
10
11
12
13
14
15
...
273
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight