Re: [PATCH 17/25] perf arm_spe: Expose length for SVE and SME operations

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

On 29/09/2025 5:37 pm, Leo Yan wrote:
> Record length for SVE and SME operations into records.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   tools/perf/util/arm-spe-decoder/arm-spe-decoder.c | 3 +++
>   tools/perf/util/arm-spe-decoder/arm-spe-decoder.h | 1 +
>   2 files changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
> index 9e02b2bdd1177193996d071dd88f969e25b1ad86..82e3053131db62603553bd092388879ce9a9bcd4 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
> @@ -220,6 +220,7 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder)
>   						decoder->record.op |= ARM_SPE_OP_ATOMIC;
>   				} else if (SPE_OP_PKT_LDST_SUBCLASS_SVE_SME_REG(payload)) {
>   					decoder->record.op |= ARM_SPE_OP_SVE;
> +					decoder->record.length = SPE_OP_PKG_SVE_EVL(payload);
>   					if (payload & SPE_OP_PKT_SVE_PRED)
>   						decoder->record.op |= ARM_SPE_OP_PRED;
>   					if (payload & SPE_OP_PKT_SVE_SG)
> @@ -239,12 +240,14 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder)
>   				decoder->record.op |= ARM_SPE_OP_OTHER;
>   				if (SPE_OP_PKT_OTHER_SUBCLASS_SVE(payload)) {
>   					decoder->record.op |= ARM_SPE_OP_SVE | ARM_SPE_OP_DP;
> +					decoder->record.length = SPE_OP_PKG_SVE_EVL(payload);
>   					if (payload & SPE_OP_PKT_OTHER_FP)
>   						decoder->record.op |= ARM_SPE_OP_FP;
>   					if (payload & SPE_OP_PKT_SVE_PRED)
>   						decoder->record.op |= ARM_SPE_OP_PRED;
>   				} else if (SPE_OP_PKT_OTHER_SUBCLASS_SME(payload)) {
>   					decoder->record.op |= ARM_SPE_OP_SME;
> +					decoder->record.length = SPE_OP_PKG_SME_ETS(payload);

These are all assigned but never read. I'm not sure if that's a mistake 
or they're just there for completeness. I don't think there's much point 
in going for completeness unless it actually makes it all the way to the 
user though. It would be better to leave it out and finish it another time.

>   					if (payload & SPE_OP_PKT_OTHER_FP)
>   						decoder->record.op |= ARM_SPE_OP_FP;
>   				} else if (SPE_OP_PKT_OTHER_SUBCLASS_OTHER(payload)) {
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
> index d4574268cf793efe154f252b3e2af9c721bada97..b9288cd774bea2534d9991213c942dcbaf2a7232 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
> @@ -122,6 +122,7 @@ struct arm_spe_record {
>   	u64 phys_addr;
>   	u64 context_id;
>   	u16 source;
> +	u32 length;
>   };
>   
>   struct arm_spe_insn;
>