Re: [PATCH v7 5/8] perf cs-etm: Flush thread stacks after decoder reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Temporal and spatial violation causing global thread stack destruction on local queue replenishment.
- [High] Guest machine thread stacks are bypassed during flush when context IDs are disabled (`CS_ETM_PIDFMT_NONE`).
--

commit 44f2b7bdbbeef1f4cf7936476255084d1e513f39
Author: Leo Yan <leo.yan@arm.com>
perf cs-etm: Flush thread stacks after decoder reset

This commit introduces a mechanism to flush all host and guest thread
stacks when the CoreSight decoder is reset due to trace discontinuities.
This is intended to prevent stale call/return histories from corrupting
synthesized callchains.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 8798bf0471faf..7069b4990e610 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
[ ... ]
> @@ -1997,6 +1997,37 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
>  
>  	return 0;
>  }
> +
> +static int cs_etm__flush_stack_cb(struct thread *thread,
> +				  void *data __maybe_unused)
> +{
> +	thread_stack__flush(thread);
> +	return 0;
> +}
> +
> +static void cs_etm__flush_machine_stack(struct cs_etm_queue *etmq, pid_t pid)
> +{
> +	struct machine *machine;
> +
> +	machine = machines__find(&etmq->etm->session->machines, pid);
> +	if (machine)
> +		machine__for_each_thread(machine, cs_etm__flush_stack_cb, NULL);
> +}
> +
> +static void cs_etm__flush_all_stack(struct cs_etm_queue *etmq)
> +{
> +	enum cs_etm_pid_fmt pid_fmt = cs_etm__get_pid_fmt(etmq);
> +
> +	if (!etmq->etm->synth_opts.last_branch)
> +		return;
> +
> +	cs_etm__flush_machine_stack(etmq, HOST_KERNEL_ID);
> +
> +	/* Clear the guest stack if virtualization is supported */
> +	if (pid_fmt == CS_ETM_PIDFMT_CTXTID2)
> +		cs_etm__flush_machine_stack(etmq, DEFAULT_GUEST_KERNEL_ID);

[Severity: High]
Does this logic bypass populated guest machine thread stacks when tracing
with CS_ETM_PIDFMT_NONE on hardware where the kernel runs at EL1?

Looking at cs_etm__get_machine(), traces with exception level ocsd_EL1 are
assigned to the guest machine even when pid_fmt is CS_ETM_PIDFMT_NONE.

By restricting the guest machine flush strictly to CS_ETM_PIDFMT_CTXTID2
here, it seems these populated guest thread stacks are skipped during the
trace discontinuity flush, which could allow stale branch histories to
persist.

> +}
> +
[ ... ]
> @@ -2019,6 +2050,12 @@ static int cs_etm__get_data_block(struct cs_etm_queue *etmq)
>  		ret = cs_etm_decoder__reset(etmq->decoder);
>  		if (ret)
>  			return ret;
> +
> +		/*
> +		 * Since the decoder is reset, this causes a global trace
> +		 * discontinuity. Flush all thread stacks.
> +		 */
> +		cs_etm__flush_all_stack(etmq);

[Severity: High]
Could this global flush cause a temporal and spatial violation by destroying
thread states for events that haven't been processed yet?

When a local auxtrace queue runs out of data and triggers a read-ahead via
cs_etm__get_data_block(), it unconditionally calls cs_etm__flush_all_stack().
Because CoreSight auxtrace operates using a min-heap to chronologically
interleave events across multiple independent CPU queues, events from other
queues are still chronologically sorted in the min-heap waiting to be
processed.

Iterating over machine__for_each_thread() and flushing all thread stacks
globally during the read-ahead phase of a single queue appears to
prematurely destroy the thread state for events that happened before the
discontinuity on the current queue, as well as for threads tracked by
other CPU queues.

Is it safe to perform this synchronous global flush ahead of the
chronological min-heap processing?

>  	}
>  
>  	return etmq->buf_len;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-b4-arm_cs_callchain_support_v1-v7-0-1ba770c862ae@arm.com?part=5