Re: [PATCH 10/25] coresight: trbe: Pad snapshot buffers

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

On 15/09/2026 16:48, Leo Yan wrote:
> Snapshot buffers use the head as the write pointer without checking its
> alignment or the space remaining before the limit. After a stop or a
> move to another CPU, the head can be misaligned for the current TRBE.
> It can also leave too little space for the Fill mode out-of-range
> workaround.
> 
> Pad the head to the current CPU's alignment. If the remaining buffer
> cannot hold the minimum trace region, pad to the end of buffer. Update
> the head for setting next write pointer.
> 
> Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
> Assisted-by: Codex:gpt-6
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 36 ++++++++++++++++++----------
>   1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index c7cbca45f2debd4047b93283ea9fe5dd9e1f2ebf..a31f2ebc327cd681344a62de28c0b3840505a9cb 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -374,18 +374,6 @@ static void trbe_pad_buf(struct perf_output_handle *handle, int len)
>   		perf_aux_output_skip(handle, len);
>   }
>   
> -static unsigned long trbe_snapshot_offset(struct perf_output_handle *handle)
> -{
> -	struct trbe_buf *buf = etm_perf_sink_config(handle);
> -
> -	/*
> -	 * The ETE trace has alignment synchronization packets allowing
> -	 * the decoder to reset in case of an overflow or corruption.
> -	 * So we can use the entire buffer for the snapshot mode.
> -	 */
> -	return buf->nr_pages * PAGE_SIZE;
> -}
> -
>   static u64 trbe_min_trace_buf_size(struct perf_output_handle *handle)
>   {
>   	u64 size = TRBE_TRACE_MIN_BUF_SIZE;
> @@ -405,6 +393,30 @@ static u64 trbe_min_trace_buf_size(struct perf_output_handle *handle)
>   	return size;
>   }
>   
> +static unsigned long trbe_snapshot_offset(struct perf_output_handle *handle)
> +{
> +	struct trbe_buf *buf = etm_perf_sink_config(handle);
> +	struct trbe_cpudata *cpudata = buf->cpudata;
> +	u64 buf_size = (u64)buf->nr_pages << PAGE_SHIFT;
> +	u64 head = PERF_IDX2OFF(handle->head, buf);
> +	u64 next = round_up(head, cpudata->trbe_align);
> +
> +	/*
> +	 * A task event may migrate to a CPU with a different alignment or
> +	 * errata. Make sure it has enough space, pad up to the alignment
> +	 * required by the current TRBE.
> +	 */
> +	if (buf_size - next < trbe_min_trace_buf_size(handle))
> +		next = buf_size;
> +
> +	if (next != head) {
> +		__trbe_pad_buf(buf, head, next - head);
> +		handle->head += next - head;
> +	}
> +
> +	return buf_size;
> +}
> +
>   /*
>    * TRBE Limit Calculation
>    *
> 

Wouldn't it make sense to have compute_trbe_buffer_limit() do the 
padding now that it happens for both normal and snapshot mode?

Either way:

Reviewed-by: James Clark <james.clark@linaro.org>