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

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 <[email protected]>

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?

>  	}
>  
>  	/*

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-unformatted-per-thread-fix-v2-0-00c4fd0701b4@linaro.org?part=3