Re: [PATCH v3 3/6] perf arm_spe: Store MIDR in arm_spe_pkt

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Unconditional calls to `arm_spe__get_midr()` with an unknown CPU (-1) cause massive un-ratelimited `pr_err` console spam on heterogeneous systems when decoding per-thread traces.
--

commit 1f340d9a2571e040cbd5dd8740efe18dff0e27aa
Author: James Clark <james.clark@linaro.org>

perf arm_spe: Store MIDR in arm_spe_pkt

This patch stores a copy of the MIDR in the arm_spe_pkt structure to
affect the printing of arm_spe_pkts in raw dump mode, and additionally
updates the decoder to contain the MIDR.

> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index f9188fdb8e786..ed4515fef2de3 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c

[ ... ]

> @@ -302,8 +305,10 @@ static void arm_spe_set_pid_tid_cpu(struct arm_spe *spe,
>  
>  	if (speq->thread) {
>  		speq->pid = thread__pid(speq->thread);
> -		if (queue->cpu == -1)
> +		if (queue->cpu == -1) {
>  			speq->cpu = thread__cpu(speq->thread);
> +			arm_spe__get_midr(spe, speq->cpu, &speq->decoder->midr);

If the thread's CPU is unknown, can thread__cpu() return -1 here?

If so, could this cause arm_spe__get_midr() to fail and print un-ratelimited
errors (such as "Failed to get MIDR for CPU -1") on heterogeneous systems?
Because arm_spe_set_pid_tid_cpu() is called for trace chunks in the queue
processing loop, would this flood the console with errors?

> +		}
>  	}
>  }
>  
> @@ -1248,6 +1253,7 @@ static int arm_spe__setup_queue(struct arm_spe *spe,
>  
>  	if (queue->cpu != -1)
>  		speq->cpu = queue->cpu;
> +	arm_spe__get_midr(spe, queue->cpu, &speq->decoder->midr);

In per-thread recording mode, queue->cpu can be -1. Does calling
arm_spe__get_midr() unconditionally with -1 result in the same console spam
mentioned above?

>  
>  	if (!speq->on_heap) {
>  		int ret;

[ ... ]

> @@ -1490,8 +1496,11 @@ static int arm_spe_process_auxtrace_event(struct perf_session *session,
>  		/* Dump here now we have copied a piped trace out of the pipe */
>  		if (dump_trace) {
>  			if (auxtrace_buffer__get_data(buffer, fd)) {
> +				u64 midr = 0;
> +
> +				arm_spe__get_midr(spe, buffer->cpu.cpu, &midr);

Similarly, can buffer->cpu.cpu be -1 here, leading to the same console issue
when processing raw buffers?

>  				arm_spe_dump_event(spe, buffer->data,
> -						buffer->size);
> +						   buffer->size, midr);
>  				auxtrace_buffer__put_data(buffer);
>  			}
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260414-james-spe-impdef-decode-v3-0-63baf9c893b1@linaro.org?part=3