[PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Richard Earnshaw]

On 25/11/2020 15:34, Siddhesh Poyarekar wrote:
> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>>
>> Add various defines and stubs for enabling MTE on AArch64 sysv-like
>> systems such as Linux.  The HWCAP feature bit is copied over in the
>> same way as other feature bits.  Similarly we add a new wrapper header
>> for mman.h to define the PROT_MTE flag that can be used with mmap and
>> related functions.
>>
>> We add a new field to struct cpu_features that can be used, for
>> example, to check whether or not certain ifunc'd routines should be
>> bound to MTE-safe versions.
>>
>> Finally, if we detect that MTE should be enabled (ie via the glibc
>> tunable); we enable MTE during startup as required.
>> ---
>>   sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |  1 +
>>   sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |  7 +++++
>>   .../unix/sysv/linux/aarch64/cpu-features.c    | 28 +++++++++++++++++++
>>   .../unix/sysv/linux/aarch64/cpu-features.h    |  1 +
>>   4 files changed, 37 insertions(+)
>>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> index af90d8a626..389852f1d9 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> @@ -73,3 +73,4 @@
>>  #define HWCAP2_DGH        (1 << 15)
>>  #define HWCAP2_RNG        (1 << 16)
>>  #define HWCAP2_BTI        (1 << 17)
>> +#define HWCAP2_MTE        (1 << 18)
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> index ecae046344..3658b958b5 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> @@ -1,4 +1,5 @@
>>  /* Definitions for POSIX memory map interface.  Linux/AArch64 version.
>> +
> 
> Unnecessary newline.  I spotted a couple in 3/8 as well.
> 
>>     Copyright (C) 2020 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>  
>> @@ -25,6 +26,12 @@
>>  
>>  #define PROT_BTI    0x10
>>  
>> +/* The following definitions basically come from the kernel headers.
>> +   But the kernel header is not namespace clean.  */
>> +
>> +/* Other flags.  */
>> +#define PROT_MTE    0x20        /* Normal Tagged mapping.  */
>> +
>>  #include <bits/mman-map-flags-generic.h>
>>  
>>  /* Include generic Linux declarations.  */
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> index b9ab827aca..aa4a82c6e8 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> @@ -19,6 +19,7 @@
>>  #include <cpu-features.h>
>>  #include <sys/auxv.h>
>>  #include <elf/dl-hwcaps.h>
>> +#include <sys/prctl.h>
>>  
>>  #define DCZID_DZP_MASK (1 << 4)
>>  #define DCZID_BS_MASK (0xf)
>> @@ -86,4 +87,31 @@ init_cpu_features (struct cpu_features *cpu_features)
>>  
>>    /* Check if BTI is supported.  */
>>    cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
>> +
>> +  /* Setup memory tagging support if the HW and kernel support it,
>> and if
>> +     the user has requested it.  */
>> +  cpu_features->mte_state = 0;
>> +
>> +#ifdef _LIBC_MTAG
>> +# if HAVE_TUNABLES
>> +  int mte_state = TUNABLE_GET (glibc, memtag, enable, unsigned, 0);
>> +  cpu_features->mte_state = (GLRO (dl_hwcap2) & HWCAP2_MTE) ?
>> mte_state : 0;
>> +  /* If we lack the MTE feature, disable the tunable, since it will
>> +     otherwise cause instructions that won't run on this CPU to be
>> used.  */
>> +  TUNABLE_SET (glibc, memtag, enable, unsigned,
>> cpu_features->mte_state);
>> +# endif
>> +
>> +  /* For now, disallow tag 0, so that we can clearly see when tagged
>> +     addresses are being allocated.  */
>> +  if (cpu_features->mte_state & 2)
>> +    __prctl (PR_SET_TAGGED_ADDR_CTRL,
>> +         (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC
>> +          | (0xfffe << PR_MTE_TAG_SHIFT)),
> 
> Couldn't this magic number also become a macro too?

Well, I could rewrite (0xfffe << PR_MTE_TAG_SHIFT) as

  (PR_MTE_TAG_MASK & ~(1UL << PR_MTE_TAG_SHIFT))

but I'm not entirely convinced that's a lot more understandable: it's
just a bit-mask of tags that can be enabled.

> 
>> +         0, 0, 0);
>> +  else if (cpu_features->mte_state)
>> +    __prctl (PR_SET_TAGGED_ADDR_CTRL,
>> +         (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_ASYNC
>> +          | (0xfffe << PR_MTE_TAG_SHIFT)),
> 
> Likewise.
> 
>> +         0, 0, 0);
>> +#endif
>>  }
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> index 00a4d0c8e7..838d5c9aba 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> @@ -70,6 +70,7 @@ struct cpu_features
>>    uint64_t midr_el1;
>>    unsigned zva_size;
>>    bool bti;
>> +  unsigned mte_state;
> 
> This could be just a byte unless you foresee expanding the MTE flags
> beyond the 8 bits you've specified in the tunables.

I don't know how stable this features structure has to be.  If it has to
remain fixed in layout forever (append-only?), then I think I'd rather
keep it as an unsigned to allow for more uses of the tunable as the need
arises.  If it's acceptable to just change the layout aribitrarily
between glibc releases, then using a byte for now would be fine.

> 
>>  };
>>  
>>  #endif /* _CPU_FEATURES_AARCH64_H  */
> 
> 

R.