linux-kernel - Re: [PATCH 2/2] x86/insn: perf tools: Add AVX-512 support to the instruction decoder

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: <20160720105033.43d4553550031e5dd33d5f9d@kernel.org>
Date:	Wed, 20 Jul 2016 10:50:33 +0900
From:	Masami Hiramatsu <mhiramat@...nel.org>
To:	Adrian Hunter <adrian.hunter@...el.com>
Cc:	linux-kernel@...r.kernel.org,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Jiri Olsa <jolsa@...hat.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andy Lutomirski <luto@...capital.net>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH 2/2] x86/insn: perf tools: Add AVX-512 support to the
 instruction decoder

On Tue, 19 Jul 2016 15:46:48 +0300
Adrian Hunter <adrian.hunter@...el.com> wrote:

> Add support for Intel's AVX-512 instructions to the instruction decoder.
> 
> AVX-512 instructions are documented in Intel Architecture Instruction Set
> Extensions Programming Reference (February 2016).
> 
> AVX-512 instructions are identifed by a EVEX prefix which, for the purpose
> of instruction decoding, can be treated as though it were a 4-byte VEX
> prefix.
> 
> Existing instructions which can now accept an EVEX prefix need not be
> further annotated in the op code map (x86-opcode-map.txt). In the case of
> new instructions, the op code map is updated accordingly.
> 
> Also add associated Mask Instructions that are used to manipulate mask
> registers used in AVX-512 instructions.
> 
> Add a representative set of instructions to the perf tools new instructons
> test.

Hmm, could you split this patch into 2 or 3 parts for review?
One is for core-kernel, the others are for perf tools (feature
adding and tests).

[..]
> diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
> index 28082de46f0d..92b89fa5f414 100644
> --- a/arch/x86/lib/x86-opcode-map.txt
> +++ b/arch/x86/lib/x86-opcode-map.txt
> @@ -13,12 +13,17 @@
>  # opcode: escape # escaped-name
>  # EndTable
>  #
> +# mnemonics that begin with lowercase 'v' accept a VEX or EVEX prefix
> +# mnemonics that begin with lowercase 'k' accept a VEX prefix
> +#

Ah, nice :)

>  #<group maps>
>  # GrpTable: GrpXXX
>  # reg:  mnemonic [operand1[,operand2...]] [(extra1)[,(extra2)...] [| 2nd-mnemonic ...]
>  # EndTable
>  #
>  # AVX Superscripts
> +#  (ev): this opcode requires EVEX prefix.
> +#  (evo): this opcode accepts EVEX prefix.

Hmm, what the 'o' stands for? I thought it means "EVEX ONLY", but the comment is
opposite. For example, see 'o64', which means "64bit only".
And anyway, as you said above, if all existing instructions can accept EVEX prefix,
why would we need it?

> -5b: vcvtdq2ps Vps,Wdq | vcvtps2dq Vdq,Wps (66) | vcvttps2dq Vdq,Wps (F3)
> +5b: vcvtdq2ps Vps,Wdq | vcvtqq2ps Vps,Wqq (evo) | vcvtps2dq Vdq,Wps (66) | vcvttps2dq Vdq,Wps (F3)

Ah, I see, so that is for the instructions which change the mnemonic since
byte-width is changed...

OK, so please update the above description of the superscript, like as
"this opcode is changed by EVEX prefix (EVEX opcode)" etc.

Thank you,


-- 
Masami Hiramatsu <mhiramat@...nel.org>