Re: [PATCH] Support Intel FRED LKGS

[Jan Beulich <jbeulich@suse.com>]

On 18.05.2023 05:41, Zhang, Jun wrote:
> This patch aims to add Intel FRED and LKGS instructions.
> 
> The information is based on newly released
> Intel Flexible Return and Event Delivery(FRED).
> 
> The document comes following:
> https://cdrdv2.intel.com/v1/dl/getContent/678938

You say "newly released", but this link points to a document from May
2022.

> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -494,6 +494,7 @@ if [gas_32_check] then {
>      run_dump_test "raoint"
>      run_dump_test "raoint-intel"
>      run_list_test "amx-complex-inval"
> +    run_dump_test "fred"

According to my reading of the spec the new insns aren't legal outside
of 64-bit mode. That's not even "long mode", seeing the spec saying
"FRED transitions also prevents entry to compatibility mode in ring 0."

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-lkgs.s
> @@ -0,0 +1,21 @@
> +# Check 64bit LKGS instructions
> +
> +	.allow_index_reg
> +	.text
> +_start:
> +	lkgs	%r12	 #LKGS
> +	lkgs    %r12w    #LKGS
> +	lkgsw   %r12w    #LKGS
> +	lkgs	0x10000000(%rbp, %r14, 8)	 #LKGS
> +	lkgs	(%r9)	 #LKGS
> +	lkgs	254(%rcx)	 #LKGS Disp32(fe000000)
> +	lkgs	-256(%rdx)	 #LKGS Disp32(00ffffff)
> +
> +.intel_syntax noprefix
> +	lkgs	r12	 #LKGS
> +	lkgs    r12w     #LKGS
> +        lkgsw   r12w     #LKGS

Nit: Indentation (want to use tab here just like everywhere else).

> @@ -2755,7 +2758,7 @@ static const struct dis386 reg_table[][8] = {
>      { "ltr",	{ Ew }, 0 },
>      { "verr",	{ Ew }, 0 },
>      { "verw",	{ Ew }, 0 },
> -    { Bad_Opcode },
> +    { X86_64_TABLE (X86_64_0F00_REG_6) },
>      { Bad_Opcode },
>    },
>    /* REG_0F01 */
> @@ -2996,6 +2999,14 @@ static const struct dis386 prefix_table[][4] = {
>      { NULL, { { NULL, 0 } }, PREFIX_IGNORED }
>    },
>  
> +  /* PREFIX_0F00_REG_6_X86_64 */
> +  {
> +    { Bad_Opcode },
> +    { Bad_Opcode },
> +    { Bad_Opcode },
> +    { "lkgs",  { Ew }, 0 },
> +  },

While consistent with LTR et al (visible above), I question the use
of Ew here (and there). See below for the assembler side of things,
which this would better be consistent with.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3351,3 +3351,16 @@ aor, 0xf20f38fc, RAO_INT, Modrm|IgnoreSize|CheckOperandSize|NoSuf, { Reg32|Reg64
>  axor, 0xf30f38fc, RAO_INT, Modrm|IgnoreSize|CheckOperandSize|NoSuf, { Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex }
>  
>  // RAO-INT instructions end.
> +
> +// FRED instructions.
> +
> +erets, 0xf20f01ca, FRED, NoSuf, {}
> +eretu, 0xf30f01ca, FRED, NoSuf, {}

As per above, I think this lacks |x64 (and the disassembler parts would
then need adjusting accordingly).

> +//FRED instructions end.

Nit: Missing blank.

> +// LKGS instructions.
> +
> +lkgs, 0xf20f00/6, LKGS|x64, Modrm|IgnoreSize|No_bSuf|No_lSuf|No_sSuf|No_qSuf|NoRex64, { Reg16|Reg32|Reg64|Word|Unspecified|BaseIndex }

Word is redundant with Reg16, and once omitted you'll notice how what you
have doesn't achieve the intended effect. Please take other similar insns
as reference. Some were split not all that long ago, to properly express
both their register forms (allowing all three widths) and their memory
ones (allowing only 16-bit operands). The set of permitted suffixes also
varies between both.

Jan