This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH, BINUTILS, AARCH64, 7/9] Add BTI instruction

From: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
To: Sudakshina Das <sudi dot das at arm dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
Cc: nd <nd at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>, "nickc at redhat dot com" <nickc at redhat dot com>
Date: Tue, 9 Oct 2018 15:43:44 +0100
Subject: Re: [PATCH, BINUTILS, AARCH64, 7/9] Add BTI instruction
References: <5bcecf6a-7f33-75e0-8a94-f72c90864790@arm.com> <032c4e84-fcd0-1e5b-9e8f-92ab72dcf8fe@arm.com> <559f1f6b-91d7-6a6f-72c3-819aa119f0c0@arm.com> <f2bd10c5-0fe2-34c8-898d-682db51799a2@arm.com>

On 09/10/18 14:22, Sudakshina Das wrote:
> Hi Richard
> 
> On 08/10/18 12:59, Sudakshina Das wrote:
>> Hi Richard
>>
>> On 05/10/18 16:43, Richard Earnshaw (lists) wrote:
>>> On 19/09/18 15:29, Sudakshina Das wrote:
>>>> Hi
>>>>
>>>> This patch is part of the patch series to add support for ARMv8.5-A
>>>> extensions.
>>>> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/docs/ddi0596/a/a64-base-instructions-alphabetic-order/bti-branch-target-identification)
>>>>
>>>>
>>>> The Branch Target Identification instructions (BTI) are allocated to
>>>> existing HINT space, using HINT numbers 32, 34, 36, 38, such that
>>>> bits[7:6] of the instruction identify the compatibility of the BTI
>>>> instruction to different branches.
>>>>          BTI {<targets>}
>>>> where <targets> one of the following, specifying which type of
>>>> indirection is allowed:
>>>>         j : Can be a target of any BR Xn isntruction.
>>>>         c : Can be a target of any BLR Xn and BR {X16|X17}.
>>>>         jc: Can be a target of any free branch.
>>>>
>>>> A BTI instruction without any <targets> is the strictest of all and
>>>> can not be a target of nay free branch.
>>>>
>>>> Testing done: Builds and reg tests all pass on aarch64-none-linux-gnu.
>>>> and aarch64-none-elf. Updated and added new tests to check.
>>>>
>>>> Ok for trunk?
>>>>
>>>> Thanks
>>>> Sudi
>>>> PS. I do not have commit access so if OK can someone apply for me?
>>>> Also the patch was too big to attach due to the regenerated files.
>>>>
>>>>
>>>> *** include/ChangeLog ***
>>>>
>>>> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
>>>>
>>>>     * opcode/aarch64.h (AARCH64_FEATURE_BTI): New.
>>>>     (AARCH64_ARCH_V8_5): Add AARCH64_FEATURE_BTI by default.
>>>>     (aarch64_opnd): Add AARCH64_OPND_BTI_TARGET.
>>>>
>>>> *** opcodes/ChangeLog ***
>>>>
>>>> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
>>>>
>>>>     * aarch64-opc.c (aarch64_hint_options): New entries for
>>>>     c, j, jc and default for BTI.
>>>>     (aarch64_print_operand): Add case for AARCH64_OPND_BTI_TARGET
>>>>     and create an exception for "default" option.
>>>>     * aarch64-tbl.h (aarch64_feature_bti, BTI, BTI_INSN): New.
>>>>     (aarch64_opcode_table): Add entry for BTI.
>>>>     (AARCH64_OPERANDS): Add new description for BTI targets.
>>>>     * aarch64-asm-2.c: Regenerate.
>>>>     * aarch64-dis-2.c: Regenerate.
>>>>     * aarch64-opc-2.c: Regenerate.
>>>>
>>>> *** gas/ChangeLog ***
>>>>
>>>> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
>>>>
>>>>     * config/tc-aarch64.c (parse_bti_operand): New.
>>>>     (process_omitted_operand): Add case for AARCH64_OPND_BTI_TARGET.
>>>>     (parse_operands): Likewise.
>>>>     * testsuite/gas/aarch64/system.d: Update for BTI.
>>>>     * testsuite/gas/aarch64/bti.s: New.
>>>>     * testsuite/gas/aarch64/bti.d: New.
>>>>     * testsuite/gas/aarch64/illegal-bti.d: New.
>>>>     * testsuite/gas/aarch64/illegal-bti.l: New.
>>>>
>>>>
>>>>
>>>
>>> gas/config/tc-aarch64.c:
>>>
>>> +    case 0x22:
>>> +    case 0x24:
>>> +    case 0x26: break;
>>> +    default:
>>>
>>> Put the 'break on a separate line when you have multiple cases.  Blank
>>> line before the default.
>>>
>>> Also, if the user writes
>>>
>>>     bti default
>>>     bti fred
>>>
>>> they will get a different error message for each line.  That's
>>> inconsistent.
>>>
>>> opcodes/aarch64-opc.c:
>>>
>>>       case AARCH64_OPND_BARRIER_PSB:
>>> -      snprintf (buf, size, "%s", opnd->hint_option->name);
>>> +    case AARCH64_OPND_BTI_TARGET:
>>> +      /* BTI with no target.  */
>>> +      if (opnd->hint_option->value != 0x20)
>>> +    snprintf (buf, size, "%s", opnd->hint_option->name);
>>>         break;
>>>
>>> I don't particularly like the specific exception here.  Can we find a
>>> better way to do this?  Perhaps a flag bit associated with the tupple to
>>> indicate that the name field should not be printed.
>>>
>>
>> Please find a new version attached. Since the #imm for HINT is only a
>> 7-bit value. I have edited the use of the "value" in the
>> aarch64_name_value_pair to encode both #imm and a Flag indicating
>> that the name does not need printing.
>>
> 
> With new macros for the bti operands:
> 
> *** include/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
> 
>     * opcode/aarch64.h (AARCH64_FEATURE_BTI): New.
>     (AARCH64_ARCH_V8_5): Add AARCH64_FEATURE_BTI by default.
>     (aarch64_opnd): Add AARCH64_OPND_BTI_TARGET.
>     (HINT_OPD_CSYNC, HINT_OPD_C, HINT_OPD_J): New macros to
>     define HINT #imm values.
>     (HINT_OPD_JC, HINT_OPD_NULL): Likewise.
> 
> *** opcodes/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
> 
>     * aarch64-opc.h (HINT_OPD_NOPRINT, HINT_ENCODE): New.
>      (HINT_FLAG, HINT_VALUE): New macros to encode NO_PRINT flag
>     with the hint immediate.
>     * aarch64-opc.c (aarch64_hint_options): New entries for
>     c, j, jc and default (with HINT_OPD_F_NOPRINT flag) for BTI.
>     (aarch64_print_operand): Add case for AARCH64_OPND_BTI_TARGET
>     while checking for HINT_OPD_F_NOPRINT flag.
>     * aarch64-dis.c (aarch64_ext_hint): Use new HINT_VALUE to
>     extract value.
>     * aarch64-tbl.h (aarch64_feature_bti, BTI, BTI_INSN): New.
>     (aarch64_opcode_table): Add entry for BTI.
>     (AARCH64_OPERANDS): Add new description for BTI targets.
>     * aarch64-asm-2.c: Regenerate.
>     * aarch64-dis-2.c: Regenerate.
>     * aarch64-opc-2.c: Regenerate.
> 
> *** gas/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
> 
>     * config/tc-aarch64.c (parse_bti_operand): New.
>     (process_omitted_operand): Add case for AARCH64_OPND_BTI_TARGET.
>     (parse_operands): Likewise.
>     * testsuite/gas/aarch64/system.d: Update for BTI.
>     * testsuite/gas/aarch64/bti.s: New.
>     * testsuite/gas/aarch64/bti.d: New.
>     * testsuite/gas/aarch64/illegal-bti.d: New.
>     * testsuite/gas/aarch64/illegal-bti.l: New.

Committed.

R.

> 
> Thanks
> Sudi
> 
>> *** include/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
>>
>>      * opcode/aarch64.h (AARCH64_FEATURE_BTI): New.
>>      (AARCH64_ARCH_V8_5): Add AARCH64_FEATURE_BTI by default.
>>      (aarch64_opnd): Add AARCH64_OPND_BTI_TARGET.
>>
>> *** opcodes/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
>>
>>      * aarch64-opc.h (HINT_OPD_NOPRINT, HINT_ENCODE): New.
>>       (HINT_FLAG, HINT_VALUE): New macros to encode NO_PRINT flag
>>      with the hint immediate.
>>      * aarch64-opc.c (aarch64_hint_options): New entries for
>>      c, j, jc and default (with HINT_NOPRINT flag) for BTI.
>>      (aarch64_print_operand): Add case for AARCH64_OPND_BTI_TARGET
>>      while checking for HINT_OPD_NOPRINT flag.
>>      * aarch64-dis.c (aarch64_ext_hint): Use new HINT_VALUE to
>>      extract value.
>>      * aarch64-tbl.h (aarch64_feature_bti, BTI, BTI_INSN): New.
>>      (aarch64_opcode_table): Add entry for BTI.
>>      (AARCH64_OPERANDS): Add new description for BTI targets.
>>      * aarch64-asm-2.c: Regenerate.
>>      * aarch64-dis-2.c: Regenerate.
>>      * aarch64-opc-2.c: Regenerate.
>>
>> *** gas/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
>>
>>      * config/tc-aarch64.c (parse_bti_operand): New.
>>      (process_omitted_operand): Add case for AARCH64_OPND_BTI_TARGET.
>>      (parse_operands): Likewise.
>>      * testsuite/gas/aarch64/system.d: Update for BTI.
>>      * testsuite/gas/aarch64/bti.s: New.
>>      * testsuite/gas/aarch64/bti.d: New.
>>      * testsuite/gas/aarch64/illegal-bti.d: New.
>>      * testsuite/gas/aarch64/illegal-bti.l: New.
>>
>>
>> Thanks
>> Sudi
>>
>

References:
- Re: [PATCH, BINUTILS, AARCH64, 7/9] Add BTI instruction
  - From: Richard Earnshaw (lists)
- Re: [PATCH, BINUTILS, AARCH64, 7/9] Add BTI instruction
  - From: Sudakshina Das
- Re: [PATCH, BINUTILS, AARCH64, 7/9] Add BTI instruction
  - From: Sudakshina Das

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]