Re: [PATCH v2 10/14] perf cs-etm: Synthesize exception entries separately from branches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: miscalculation of ETMv3 SVC/HYP source addresses
--

commit 29b742481ea9116881bdfbed14f1f45c6d6ded76
Author: Leo Yan <leo.yan@arm.com>

perf cs-etm: Synthesize exception entries separately from branches

This commit modifies the handling of ARM CoreSight ETM trace exception
entries, synthesizing them separately from branches. It derives exception
source PCs from the preferred return address rather than attributing the
exception to the last completed instruction.

> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> index 48e8c355a3e49..83a851bdd87bf 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c

[ ... ]

> @@ -524,8 +525,34 @@ cs_etm_decoder__buffer_exception(struct cs_etm_queue *etmq,
>  	packet->exception_number = elem->exception_number;
>  	if (elem->context.el_valid)
>  		packet->el = elem->context.exception_level;
> -	if (elem->excep_ret_addr)
> -		packet->end_addr = elem->en_addr;
> +
> +	/* RESET has no defined preferred return address. */
> +	if (!elem->excep_ret_addr ||
> +	    packet->exception_number == CS_ETMV4_EXC_RESET)
> +		return ret;
> +
> +	packet->end_addr = elem->en_addr;
> +
> +	/*
> +	 * T32 calls can be two or four bytes, requiring an instruction read
> +	 * before end_addr to determine their size. Keep end_addr for the
> +	 * frontend's SVC check and leave start_addr unknown for its fallback.
> +	 */
> +	if (packet->isa != CS_ETM_ISA_A32 && packet->isa != CS_ETM_ISA_A64)
> +		return ret;
> +
> +	/*
> +	 * For A32 and A64, exception calls execute a four-byte SVC, HVC or
> +	 * SMC and save the following PC. Other exceptions use the interrupted
> +	 * or faulting PC, including traps on those instructions.
> +	 *
> +	 * With a shared branch target, no instruction has completed
> +	 * at the target, so last_instr_size remains zero.
> +	 */
> +	if (elem->exception_number == CS_ETMV4_EXC_CALL)

[Severity: Medium]
Does cs_etm_decoder__buffer_exception() miscalculate the instruction size for
ETMv3 SVC and HYP call exceptions here?

By only checking against CS_ETMV4_EXC_CALL, it appears to miss the
corresponding ETMv3 call exception values (CS_ETMV3_EXC_SVC=10,
CS_ETMV3_EXC_HYP=3). When tracing ETMv3 with an SVC or HYP exception that
provides an excep_ret_addr, this would result in a 0-byte instruction size
and a misattributed source address in the trace output.

> +		last_instr_size = 4;
> +	packet->start_addr = packet->end_addr - last_instr_size;
> +	packet->last_instr_size = last_instr_size;
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-perf_cs_etm_fix_non_taken-v2-0-6ab8c07a5455@arm.com?part=10