CoreSight

coresight@lists.linaro.org

5 participants
2848 discussions

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

1 month

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

1 month

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>

1 month, 1 week

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

1 month, 1 week

Re: [PATCH 7/9] perf test cs-etm: Test branch history on existing samples

by James Clark

On 03/08/2026 10:06, 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. > > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > .../tests/shell/coresight/add_last_branch.sh | 175 ++++++++++++++++++ > 1 file changed, 175 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..4654069ad651f > --- /dev/null > +++ b/tools/perf/tests/shell/coresight/add_last_branch.sh > @@ -0,0 +1,175 @@ > +#!/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 Is this so you can use -C 0? It's not completely obvious what that has to do with the test. Can you not drop the -C option or use --per-thread mode with a simpler non-forking workload? I don't mind keeping it for some variety in the tests, but it should be documented. > + > +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() > +{ > + if perf record -T -o "$tmpdir/data" -C 0 \ > + -e cs_etm/aux-action=start-paused,timestamp/u \ Timestamp needs a value on newer kernels or Perf returns an error. But do you need to provide the option at all? It's on by default for per-CPU mode. > + -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 -w context_switch_loop 100000 \ The other Coresight tests use --workload-ctl to record less data and save some decode time. I think this test might benefit from it too. > + >/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 > + > + 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 > + return 1 > + } > + done > +} > + > +check_callchains() > +{ > + local output="$tmpdir/script-callchain" > + > + perf script -i "$tmpdir/data" -F comm,event,ip >"$output" 2>/dev/null > + > + awk ' > + /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" >&2 > + return 1 > + } Can you add some example output in the test saying what these awks are looking for. It failed for me but I wasn't sure why. I've attached my script-callchain file if that helps. > +} > + > +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 > + 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

1 month, 1 week

Re: [PATCH 0/9] perf cs-etm: Add branch history to existing samples

by James Clark

On 03/08/2026 10:01, Amir Ayupov wrote: > This series implements --itrace=L for Arm CoreSight ETM: decoded branch Hi Amir, How did you send this? The cover letter seems to be on a different thread to the patches. > history is attached to the PMU samples already present in the recording, > rather than to synthesised instruction samples. > > The motivating use case is context-sensitive PGO, which wants a callchain > and a branch stack describing the same point in time. Recording a cycles > event with call-graph=fp and aux-action=pause supplies the callchain, and > the ETM trace leading up to that sample supplies the branch stack, without > having to trace a long-running process continuously. > > Intel PT has had this since commit f0a0251cee80 ("perf intel-pt: Add > support for synthesizing branch stacks for regular events"), so this > deliberately follows intel-pt: the same --itrace=L option and the same > thread_stack__br_sample_late() call. > > Patches 1 to 4 are independent fixes and infrastructure the feature needs: > > 1 makes an inconsistent HEADER_GROUP_DESC non-fatal. AUX recordings > using aux-action pause/resume produce a group descriptor the strict > reader rejects, which makes an otherwise readable perf.data > unreadable, so without this the recipe in patch 9 cannot be decoded > at all. Useful on its own. > 2 reports hw_idx as -1 rather than 0 in reconstructed branch stacks, > since they have no hardware index. > 3 bounds a wrapped memcpy in thread_stack__br_sample(). Latent today, > reachable once a caller keeps a ring larger than the requested output > depth. > 4 adds a dlfilter that drops samples with an empty branch stack. > > Patch 5 is a no-functional-change refactor splitting > cs_etm__process_timestamped_queues() into its three parts. Heap seeding > moves to cs_etm__update_queues(), gated on queues.new_data and mirroring > intel_pt_update_queues(); the end-of-session flush moves to > cs_etm__flush_timestamped_queues(); and the decode loop is left on its own > so patch 6 can drive it once per sample. Neither seeding nor flushing can > be repeated, which is why they have to come out first. The moved code is > unchanged, so both loops appear as context in the diff. > > Patch 6 is the feature and patch 7 adds a shell test. > > Patch 8 is where review attention is most useful. --itrace=L attaches > whatever the thread stack holds when a sample is processed. With a duty > cycled trace most samples fire while the trace is off; they have nothing > newly decoded, but the thread stack still holds the previous window, so > they were being given branches that ran an arbitrary amount of time > earlier. On a 12 s capture with pause period 100003 and resume period > 8350251, of 335291 samples that received branch history only 3371 were > backed by trace decoded for that sample. > > A trace window belongs to exactly one sample, and with AUX pause and > resume the sample is what stops the trace, so the pairing is one to one by What happens when aux-pause isn't used and there isn't a 1:1 pairing? There is a lot of description of that which implies that it doesn't work if there isn't. But as far as I can tell it works just as well by using the timestamps? > construction. Patch 8 therefore takes the branch history when attaching it > instead of copying it, and a later sample with nothing newly decoded finds > an empty branch stack, which the dlfilter removes. > thread_stack__br_sample() is unchanged, so lowercase --itrace=l keeps the > overlapping branch stacks it produces today. > > Patch 9 documents the workflow. > > Because the sample is what stops the trace, the history attached to it > lines up well with the callchain: on a brstack capture the leaf of the > callchain matched the function containing the newest branch stack entry's > target for 93.5% of attached samples. The residual comes from the decode > loop stopping on interpolated timestamps, so a few branches that ran just > after the sample can still be included. Trimming those with the sample ip > raises it to 96.8%, but that matters far more for free-running ETM > strobing than for pause and resume, so I have left it out of this series > and will send it separately. > > Patch 8 could be squashed into patch 6, since patch 6 on its own produces > mostly stale history. I kept them apart so the decode mechanism and the > attachment policy can be reviewed separately, but I am happy to fold them. > > Testing > ------- > > Built with: > > make -C tools/perf NO_LIBELF=1 NO_LIBTRACEEVENT=1 CORESIGHT=1 > > Every patch builds individually. checkpatch reports only "does MAINTAINERS > need updating?" for the two new files and "quoted string split across > lines" for the dlfilter description string, which matches how > dlfilter-show-cycles.c already writes it. > > Tested on Arm Neoverse V2 with CoreSight ETM: > > - perf test "CoreSight branch history on existing samples": Ok, 3 for 3 > - captures from 5 MiB to 2.5 GiB decoded with --itrace=L64, no decode > errors > - the other CoreSight tests are unchanged by this series; four of them > fail identically at the base commit on this machine Can you report or investigate these failures please. None of the tests should be failing on TRBE hardware, at least on the latest perf-tools-next branch. You can try applying "[PATCH 00/14] perf cs-etm: Per-thread mode fixes and snapshot wrap support" to be sure, but I don't think that fixes any current failures. > > Amir Ayupov (9): > perf header: Tolerate inconsistent HEADER_GROUP_DESC > perf thread-stack: Report branch stack hw_idx as not available > perf thread-stack: Bound wrapped branch stack copy > 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 > perf cs-etm: Consume branch history when attaching it to a sample > 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 | 175 +++++++++++++ > tools/perf/util/cs-etm.c | 242 ++++++++++++++++-- > tools/perf/util/header.c | 42 ++- > tools/perf/util/thread-stack.c | 21 +- > tools/perf/util/thread-stack.h | 1 + > 8 files changed, 544 insertions(+), 26 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: da85966dfd23a3b03e00ee3bce6ad301f0a2b229

1 month, 1 week

Re: [PATCH 7/9] perf test cs-etm: Test branch history on existing samples

by James Clark

1 month, 1 week

Re: [PATCH] coresight: configfs: restrict address parameter value to root

by Leo Yan

On Wed, Aug 12, 2026 at 05:13:14PM +0800, Junrui Luo via B4 Relay wrote: > Mark parameters that can hold a kernel address, and give those a > config_item_type whose 'value' attribute is 0600. Parameters holding > plain numbers, such as the strobing 'window' and 'period' counts, keep > the existing mode. Thanks for reporting the issue. The patch seems overly complex to me. I'd suggest: --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c @@ -281,9 +281,16 @@ static ssize_t cscfg_param_value_show(struct config_item *item, char *page) { struct cscfg_fs_param *param_item = container_of(to_config_group(item), struct cscfg_fs_param, group); - u64 value = param_item->feat_desc->params_desc[param_item->param_idx].value; - - return scnprintf(page, PAGE_SIZE, "0x%llx\n", value); + struct cscfg_parameter_desc *param_desc = + param_item->feat_desc->params_desc + param_item->param_idx; + const char *name = param_desc->name; + u64 value = param_desc->value; + + /* The kernel address should print with the "%pK" specifier */ + if (!strncmp(name, "address")) + return scnprintf(page, PAGE_SIZE, "0x%pK\n", value); + else + return scnprintf(page, PAGE_SIZE, "0x%llx\n", value); } We can add a flag (e.g., is_addr) in cscfg_parameter_desc to indicate a parameter presents an address. Since currently only pstop's "address" parameter has this issue, adding a general flag can be deferred until it is actually needed. Thanks, Leo

1 month, 1 week

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

by James Clark

On 03/08/2026 10: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. > This fix makes sense in case someone doesn't overwrite it, but I think "matches what cs-etm already puts in the branch stacks" isn't quite right. Based on the hw_id docs, cs-etm should be setting 0 shouldn't it? * For the architectures whose raw branch records are * already stored in age order, the hw_idx should be 0. -1 "unknown" isn't accurate as we do know what the order is because we generate them in order. If anyone is reading the field 0 is much more useful than -1. Can we change cs_etm__synth_branch_sample() to 0 and update this commit message to not say that -1 is right for cs-etm? > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > 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;

1 month, 1 week

[PATCH v2] coresight: trbe: Hide enable_sink sysfs file

by James Clark

TRBE doesn't support sysfs mode, but the enable_sink file can still be successfully written to enable the device, and only attempting to enable the source would later fail. Avoid misleading users by adding a flag that devices can use to hide either the enable_sink or enable_source files, and set it for TRBE. Don't set it for ETE as it's possible that ETE could appear on the legacy bus and work with sysfs, and writing to enable_source already reports EINVAL if the device doesn't support sysfs mode. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v2: - Re-use CORESIGHT_DESC flags in csdev instead of converting to bool. - Hide all remaining attrs after label instead of only source and sink attrs as there aren't any others anyway (Leo). - Link to v1: https://lore.kernel.org/r/20260507-james-cs-hide-trbe-enable-v1-1-b4e40439f… --- drivers/hwtracing/coresight/coresight-core.c | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 15 +++++++++------ drivers/hwtracing/coresight/coresight-trbe.c | 7 +++++++ include/linux/coresight.h | 4 ++++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index f7b1308a759c..a0c098980096 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1543,6 +1543,7 @@ coresight_init_device(struct coresight_desc *desc) csdev->ops = desc->ops; csdev->access = desc->access; csdev->orphan = true; + csdev->flags = desc->flags; if (desc->flags & CORESIGHT_DESC_CPU_BOUND) { csdev->cpu = desc->cpu; diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index 4b010f8bc4c0..75000fe64fda 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -346,16 +346,19 @@ static ssize_t label_show(struct device *dev, } static DEVICE_ATTR_RO(label); -static umode_t label_is_visible(struct kobject *kobj, - struct attribute *attr, int n) +static umode_t coresight_attr_is_visible(struct kobject *kobj, + struct attribute *attr, int n) { struct device *dev = kobj_to_dev(kobj); + struct coresight_device *csdev = to_coresight_device(dev); if (attr == &dev_attr_label.attr) { if (fwnode_property_present(dev_fwnode(dev), "label")) return attr->mode; else return 0; + } else if (csdev->flags & CORESIGHT_DESC_NO_SYSFS_MODE) { + return 0; } return attr->mode; @@ -369,7 +372,7 @@ static struct attribute *coresight_sink_attrs[] = { static struct attribute_group coresight_sink_group = { .attrs = coresight_sink_attrs, - .is_visible = label_is_visible, + .is_visible = coresight_attr_is_visible, }; __ATTRIBUTE_GROUPS(coresight_sink); @@ -381,7 +384,7 @@ static struct attribute *coresight_source_attrs[] = { static struct attribute_group coresight_source_group = { .attrs = coresight_source_attrs, - .is_visible = label_is_visible, + .is_visible = coresight_attr_is_visible, }; __ATTRIBUTE_GROUPS(coresight_source); @@ -392,7 +395,7 @@ static struct attribute *coresight_link_attrs[] = { static struct attribute_group coresight_link_group = { .attrs = coresight_link_attrs, - .is_visible = label_is_visible, + .is_visible = coresight_attr_is_visible, }; __ATTRIBUTE_GROUPS(coresight_link); @@ -403,7 +406,7 @@ static struct attribute *coresight_helper_attrs[] = { static struct attribute_group coresight_helper_group = { .attrs = coresight_helper_attrs, - .is_visible = label_is_visible, + .is_visible = coresight_attr_is_visible, }; __ATTRIBUTE_GROUPS(coresight_helper); diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index c7cbca45f2de..eca8e13a9744 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1348,6 +1348,13 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp desc.dev = dev; desc.cpu = cpu; desc.flags = CORESIGHT_DESC_CPU_BOUND; + /* + * ETE isn't connected to TRBE with a link like other Coresight devices + * and the TRBE driver has been written to always assume Perf mode, so + * Prevent sysfs from being used. + */ + desc.flags |= CORESIGHT_DESC_NO_SYSFS_MODE; + trbe_csdev = coresight_register(&desc); if (IS_ERR(trbe_csdev)) goto cpu_clear; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index add0579cad88..6cd1d01b843b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -142,6 +142,8 @@ struct csdev_access { }) #define CORESIGHT_DESC_CPU_BOUND BIT(0) +/* Device can't be activated from sysfs, only via Perf. */ +#define CORESIGHT_DESC_NO_SYSFS_MODE BIT(1) /** * struct coresight_desc - description of a component required from drivers @@ -310,6 +312,8 @@ struct coresight_device { struct list_head config_csdev_list; raw_spinlock_t cscfg_csdev_lock; void *active_cscfg_ctxt; + /* CORESIGHT_DESC_[x] flags */ + u32 flags; }; /* --- base-commit: c8eb4a8dd259fb2b215da32764fa8c6c76519896 change-id: 20260506-james-cs-hide-trbe-enable-3c8d784e72d8 Best regards, -- James Clark <james.clark(a)linaro.org>

1 month, 1 week

← Newer
1
2
3
4
5
6
7
...
285
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight