[PATCH]: Support ZHAOXIN GMI and PAUSEOPT instructions

[Jan Beulich]

On 28.06.2024 05:10, MayShao-oc wrote:
> From: mayshao-oc <mayshao-oc@zhaoxin.com>
> 
> Hi all:
>       This patch adds:
> 	- ZHAOXIN GMI instructions
> 	- ZHAOXIN PAUSEOPT instruction
> 	- some Padlock instructions
> 
> 	Test ok, ok for trunk?

Before one can even review this, can you please point us at the underlying
documentation? Some basic comments nevertheless, below.

Also, just to mention it, I had almost skipped looking at this patch, for
it having nothing x86-ish in the subject.

> --- a/gas/testsuite/gas/i386/padlock.s
> +++ b/gas/testsuite/gas/i386/padlock.s
> @@ -8,6 +8,8 @@ foo:
>  	rep xcryptecb
>  	xcryptcbc
>  	rep xcryptcbc
> +	xcryptctr
> +	rep xcryptctr
>  	xcryptcfb
>  	rep xcryptcfb
>  	xcryptofb

This surely wants splitting out, for being a correction to the existing
test.

> @@ -1238,6 +1239,15 @@ enum
>    PREFIX_EVEX_MAP6_57,
>    PREFIX_EVEX_MAP6_D6,
>    PREFIX_EVEX_MAP6_D7,
> +
> +  PREFIX_0FA6C0,
> +  PREFIX_0FA6C8,
> +  PREFIX_0FA6D0,
> +  PREFIX_0FA6D8,
> +  PREFIX_0FA6E0,
> +  PREFIX_0FA6E8,
> +  PREFIX_0FA6F0,
> +  PREFIX_0FA6F8,
>  };

These want to move up, to maintain a reasonably sorted set.

> @@ -1312,7 +1322,8 @@ enum
>  enum
>  {
>    THREE_BYTE_0F38 = 0,
> -  THREE_BYTE_0F3A
> +  THREE_BYTE_0F3A,
> +  THREE_BYTE_0FA6
>  };

This looks suspicious to me; I don't think these insns count as "3-byte"
ones. But as per above, without seeing the spec it's not entirely certain.

> @@ -4139,6 +4146,62 @@ static const struct dis386 prefix_table[][4] = {
>    },
>  
>  #include "i386-dis-evex-prefix.h"
> +  /* PREFIX_0FA6C0 */

Please add above this #include (according to the sorting of the enum).

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -2188,6 +2188,11 @@ xcrypt-ofb, 0xf30fa7e8, PadLock, NoSuf|RepPrefixOk, {}
>  montmul, 0xf30fa6c0, PadLock, NoSuf|RepPrefixOk, {}
>  xsha1, 0xf30fa6c8, PadLock, NoSuf|RepPrefixOk, {}
>  xsha256, 0xf30fa6d0, PadLock, NoSuf|RepPrefixOk, {}
> +xsha384, 0xf30fa6d8, PadLock, NoSuf|RepPrefixOk, {}
> +xsha512, 0xf30fa6e0, PadLock, NoSuf|RepPrefixOk, {}
> +montmul2, 0xf30fa6f0, PadLock, NoSuf|RepPrefixOk, {}
> +xmodexp, 0xf30fa6f8, PadLock, NoSuf|RepPrefixOk, {}
> +xrng2, 0xf30fa7f8, PadLock, NoSuf|RepPrefixOk, {}

I don't think these can validly be added to an existing feature, unless
you can prove that they've been part of PadLock forever, and we were
merely lacking support for them. Seeing how this set and ...

> @@ -2198,6 +2203,14 @@ xcryptofb, 0xf30fa7e8, PadLock, NoSuf|RepPrefixOk, {}
>  // Alias for xstore-rng.
>  xstore, 0xfa7c0, PadLock, NoSuf|RepPrefixOk, {}
>  
> +// ZHAOXIN GMI
> +sm2, 0xf20fa6c0, GMI, NoSuf, {}
> +sm3, 0xf30fa6e8, GMI, NoSuf, {}
> +sm4, 0xf30fa7f0, GMI, NoSuf, {}
> +
> +// ZHAOXIN PAUSEOPT
> +pauseopt, 0xf20fa6d0, PAUSEOPT, NoSuf, {}

... these appear to be pretty independent of one another, please also
consider splitting the patch.

Jan