Re: [PATCH v2 01/17] MdePkg/ProcessorBind AARCH64: Add asm macro to emit GNU BTI note

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On Mon, Mar 27, 2023 at 13:00:56 +0200, Ard Biesheuvel wrote:
> Implement a CPP macro that can be called from .S files to emit the .note
> section carrying the annotation that informs the linker that the object
> file is compatible with BTI control flow integrity checks.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  MdePkg/Include/AArch64/ProcessorBind.h | 31 ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> index abe2571245c665f3..11814f1ffaef698a 100644
> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> @@ -186,6 +186,37 @@ typedef INT64 INTN;
>  #define GCC_ASM_IMPORT(func__)  \
>           .extern  _CONCATENATE (__USER_LABEL_PREFIX__, func__)
>  
> +#if defined(__ARM_FEATURE_BTI_DEFAULT) && __ARM_FEATURE_BTI_DEFAULT == 1
> +#define AARCH64_BTI(__type)                                        \
> +    .ifnc         __type,                                         ;\
> +    bti           __type                                          ;\
> +    .endif                                                        ;\

This didn't jump out at me until looking at the consumer side.
This overlays two different sets of functionality depending on whether
an option is given to the macro or not, which feels semantically
suboptimal to me (i.e. it makes my head hurt).

Could we split this into two macros - one that inserts the instruction
and one that inserts the note, and expand the latter in the former?

/
    Leif

> +    .ifndef       .Lgnu_bti_notesize                              ;\
> +    .pushsection  .note.gnu.property, "a"                         ;\
> +    .set          NT_GNU_PROPERTY_TYPE_0, 0x5                     ;\
> +    .set          GNU_PROPERTY_AARCH64_FEATURE_1_AND, 0xc0000000  ;\
> +    .set          GNU_PROPERTY_AARCH64_FEATURE_1_BTI, 0x1         ;\
> +    .align        3                                               ;\
> +    .long         .Lnamesize                                      ;\
> +    .long         .Lgnu_bti_notesize                              ;\
> +    .long         NT_GNU_PROPERTY_TYPE_0                          ;\
> +0:  .asciz        "GNU"                                           ;\
> +    .set          .Lnamesize, . - 0b                              ;\
> +    .align        3                                               ;\
> +1:  .long         GNU_PROPERTY_AARCH64_FEATURE_1_AND              ;\
> +    .long         .Lvalsize                                       ;\
> +2:  .long         GNU_PROPERTY_AARCH64_FEATURE_1_BTI              ;\
> +    .set          .Lvalsize, . - 2b                               ;\
> +    .align        3                                               ;\
> +    .set          .Lgnu_bti_notesize, . - 1b                      ;\
> +    .popsection                                                   ;\
> +    .endif
> +#endif
> +
> +#endif
> +
> +#ifndef AARCH64_BTI
> +#define AARCH64_BTI(__type)
>  #endif
>  
>  /**
> -- 
> 2.39.2
>