Re: [PATCH 2/5] perf: arm_spe: Add new event packet bits

From: Leo Yan <leo.yan@linaro.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Tan Xiaojun <tanxiaojun@huawei.com>,
	James Clark <james.clark@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] perf: arm_spe: Add new event packet bits
Date: Sun, 27 Sep 2020 11:03:29 +0800	[thread overview]
Message-ID: <20200927030329.GB9677@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <20200922101225.183554-3-andre.przywara@arm.com>

On Tue, Sep 22, 2020 at 11:12:22AM +0100, Andre Przywara wrote:
> The ARMv8.3-SPE extension adds some new bits to the event packet
> fields.
> 
> Handle bits 11 (alignment), 17 and 18 (SVE predication) when decoding
> the SPE buffer content.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../util/arm-spe-decoder/arm-spe-pkt-decoder.c  | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> index b94001b756c7..e633bb5b8e65 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> @@ -346,6 +346,23 @@ int arm_spe_pkt_desc(const struct arm_spe_pkt *packet, char *buf,
>  				buf += ret;
>  				blen -= ret;
>  			}
> +			if (payload & BIT(11)) {
> +				ret = snprintf(buf, buf_len, " ALIGNMENT");
> +				buf += ret;
> +				blen -= ret;
> +			}
> +		}
> +		if (idx > 2) {
> +			if (payload & BIT(17)) {
> +				ret = snprintf(buf, buf_len, " SVE-PARTIAL-PRED");
> +				buf += ret;
> +				blen -= ret;
> +			}
> +			if (payload & BIT(18)) {
> +				ret = snprintf(buf, buf_len, " SVE-EMPTY-PRED");
> +				buf += ret;
> +				blen -= ret;
> +			}

From patch 02 to patch 05, some changes have been included in the
patch set "perf arm-spe: Refactor decoding & dumping flow".  I
refactored the Arm SPE decoder so uses macros to replace the hard code
numbers for packet formats.  So I'd like your changes could rebase on
this refactor patch set, thus can reuse the predefined macros for
decoding.

For this patch, it has been included in the patch [2].  You could see
your implementation is difference for handling "ALIGNMENT", it misses
to check "idx > 2".  It would be very helpful if you could review
patch [2].

Thanks,
Leo

[1] https://lore.kernel.org/patchwork/cover/1288406/
[2] https://lore.kernel.org/patchwork/patch/1288413/

>  		}
>  		if (ret < 0)
>  			return ret;
> -- 
> 2.17.1
> 