linux-kernel - Re: [PATCH 3/4] perf auxtrace: Add new itrace options for ARMv8.3-SPE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200729065148.GF4343@leoy-ThinkPad-X240s>
Date:   Wed, 29 Jul 2020 14:51:48 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Wei Li <liwei391@...wei.com>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        James Clark <james.clark@....com>,
        Jiri Olsa <jolsa@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Namhyung Kim <namhyung@...nel.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Will Deacon <will@...nel.org>, zhangshaokun@...ilicon.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>, guohanjun@...wei.com
Subject: Re: [PATCH 3/4] perf auxtrace: Add new itrace options for ARMv8.3-SPE

On Fri, Jul 24, 2020 at 05:16:06PM +0800, Wei Li wrote:
> This patch is to add two options to synthesize events which are
> described as below:
> 
>  'u': synthesize unaligned address access events
>  'v': synthesize partial/empty predicated SVE events
> 
> This two options will be used by ARM SPE as their first consumer.
> 
> Signed-off-by: Wei Li <liwei391@...wei.com>
> ---
>  tools/perf/Documentation/itrace.txt | 2 ++
>  tools/perf/util/auxtrace.c          | 8 ++++++++
>  tools/perf/util/auxtrace.h          | 4 ++++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt
> index e817179c5027..25bcf3622709 100644
> --- a/tools/perf/Documentation/itrace.txt
> +++ b/tools/perf/Documentation/itrace.txt
> @@ -13,6 +13,8 @@
>  		m	synthesize last level cache events
>  		t	synthesize TLB events
>  		a	synthesize remote access events
> +		u	synthesize unaligned address access events
> +		v	synthesize partial/empty predicated SVE events
>  		g	synthesize a call chain (use with i or x)
>  		G	synthesize a call chain on existing event records
>  		l	synthesize last branch entries (use with i or x)
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index 25c639ac4ad4..2033eb3708ec 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -1334,6 +1334,8 @@ void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts,
>  	synth_opts->llc = true;
>  	synth_opts->tlb = true;
>  	synth_opts->remote_access = true;
> +	synth_opts->alignment = true;
> +	synth_opts->sve = true;
>  
>  	if (no_sample) {
>  		synth_opts->period_type = PERF_ITRACE_PERIOD_INSTRUCTIONS;
> @@ -1507,6 +1509,12 @@ int itrace_parse_synth_opts(const struct option *opt, const char *str,
>  		case 'a':
>  			synth_opts->remote_access = true;
>  			break;
> +		case 'u':
> +			synth_opts->alignment = true;
> +			break;
> +		case 'v':
> +			synth_opts->sve = true;
> +			break;
>  		case ' ':
>  		case ',':
>  			break;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 142ccf7d34df..972df7b06b0d 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -116,6 +116,8 @@ struct itrace_synth_opts {
>  	bool			llc;
>  	bool			tlb;
>  	bool			remote_access;
> +	bool			alignment;

Patch 03 and 04 are directive, it's good for me.

The naming 'unalignment' would be more clear and avoid confusion.

> +	bool			sve;
>  	unsigned int		callchain_sz;
>  	unsigned int		last_branch_sz;
>  	unsigned long long	period;
> @@ -617,6 +619,8 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
>  "				m:	    		synthesize last level cache events\n" \
>  "				t:	    		synthesize TLB events\n" \
>  "				a:	    		synthesize remote access events\n" \
> +"				u:	    		synthesize unaligned address access events\n" \

The 'unalignment' events are for 'unaligned address access and size of data'.

Otherwise, looks good to me:
Reviewed-by: Leo Yan <leo.yan@...aro.org>

Thanks,
Leo

> +"				v:	    		synthesize partial/empty predicated SVE events\n" \
>  "				g[len]:     		synthesize a call chain (use with i or x)\n" \
>  "				l[len]:     		synthesize last branch entries (use with i or x)\n" \
>  "				sNUMBER:    		skip initial number of events\n"		\
> -- 
> 2.17.1
>