[PATCH V5 9/9] Support APX JMPABS for disassembler

[Jan Beulich]

On 08.01.2024 03:32, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Friday, January 5, 2024 8:09 PM
>> To: Cui, Lili <lili.cui@intel.com>
>> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; Hu, Lin1 <lin1.hu@intel.com>;
>> binutils@sourceware.org
>> Subject: Re: [PATCH V5 9/9] Support APX JMPABS for disassembler
>>
>> On 28.12.2023 02:27, Cui, Lili wrote:
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs-inval.s
>>> @@ -0,0 +1,15 @@
>>> +# Check bytecode of APX_F jmpabs instructions with illegal encode.
>>> +
>>> +	.text
>>> +# With 66 prefix
>>> +	.byte 0x66,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>> +# With 67 prefix
>>> +	.byte 0x67,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>> +# With F2 prefix
>>> +	.byte 0xf2,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>> +# With F3 prefix
>>> +	.byte 0xf3,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>> +# With LOCK prefix
>>> +	.byte 0xf0,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>> +# REX2.M0 = 0 REX2.W = 1
>>> +	.byte 0xd5,0x08,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>
>> Considering that I specifically asked that this use .insn, and that I further took
>> the time to make a patch to make .insn work with {rex2}, I find it rather poor
>> that here and ...
>>
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs.s
>>> @@ -0,0 +1,5 @@
>>> +# Check 64bit APX_F JMPABS instructions
>>> +
>>> +	.text
>>> + _start:
>>> +	.byte 0xd5,0x00,0xa1,0x02,0x00,0x00,0x00,0x00,0x00,0x00,0x00
>>
>> ... here it is still .byte that is being used.
>>
> 
> I'm not always keeping my eye on what patches push in Binutils.

That's not a general requirement of course, but when it specifically is
work done for you, I would have expected it to be recognized and then
leveraged.

> We can upstream a new fix patch like this. 
>         .text
>  # With 66 prefix
> -       .byte 0x66,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +       .insn {rex2} data16 0xa1, $1{:u64}
>  # With 67 prefix
> -       .byte 0x67,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +       .insn {rex2} addr32 0xa1, $1{:u64}
>  # With F2 prefix
> -       .byte 0xf2,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +       .insn {rex2} repne 0xa1, $1{:u64}
>  # With F3 prefix
> -       .byte 0xf3,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +       .insn {rex2} rep 0xa1, $1{:u64}
>  # With LOCK prefix
> -       .byte 0xf0,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +       .insn {rex2} lock 0xa1, $1{:u64}
>  # REX2.M0 = 0 REX2.W = 1
> -       .byte 0xd5,0x08,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +       .insn {rex2} 0x08,0xa1, $1{:u64}
> +#.byte 0xd5,0x08,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> 
> But the last test " REX2.M0 = 0 REX2.W = 1" is invalid, do you have some advise?

Well, no, as long as {rex2} cannot specify any of the payload bits, and when
there are no operands controlling the individual bit (due to there not being
any register/memory operands), it can't be easily expressed using .insn.
Further work would be required to permit that, but for the time being in
_such_ cases it is (of course) okay to use .byte.

Jan