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

[James Clark <james.clark@linaro.org>]

On 21/08/2026 11:01, sashiko-bot@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@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.

>>   	}
>>   
>>   	/*
>