s390x: Diagnose missing VXE at run time if required at build time

  It turns out that if glibc is built with -march=z14, it is still
possible to run up to dl_platform_init on z13 CPUs.  This means we can
add a diagnostic to glibc, notifying the user that the CPU is too old.

The check is based on the way GCC sets the __LONG_DOUBLE_VX__
preprocessor macro:

  s390_def_or_undef_macro (
      pfile,
      [] (const struct cl_target_option *opts) { return TARGET_VXE_P (opts); },
      old_opts, opts, "__LONG_DOUBLE_VX__", "__LONG_DOUBLE_VX__");
  

On 06/04/2021 17:55, Florian Weimer wrote:
> It turns out that if glibc is built with -march=z14, it is still
> possible to run up to dl_platform_init on z13 CPUs.  This means we can
> add a diagnostic to glibc, notifying the user that the CPU is too old.
> 
> The check is based on the way GCC sets the __LONG_DOUBLE_VX__
> preprocessor macro:
> 
>   s390_def_or_undef_macro (
>       pfile,
>       [] (const struct cl_target_option *opts) { return TARGET_VXE_P (opts); },
>       old_opts, opts, "__LONG_DOUBLE_VX__", "__LONG_DOUBLE_VX__");
> 
> 
> diff --git a/sysdeps/s390/s390-64/dl-machine.h b/sysdeps/s390/s390-64/dl-machine.h
> index 543361c83637c071..deaf37951206fb7c 100644
> --- a/sysdeps/s390/s390-64/dl-machine.h
> +++ b/sysdeps/s390/s390-64/dl-machine.h
> @@ -235,6 +235,12 @@ _dl_start_user:\n\
>  static inline void __attribute__ ((unused))
>  dl_platform_init (void)
>  {
> +#ifdef __LONG_DOUBLE_VX__
> +  if (!(GLRO(dl_hwcap) & HWCAP_S390_VXE))
> +    _dl_fatal_printf ("\
> +Fatal glibc error: CPU lacks VXE support (z14 or later required)\n");
> +#endif
> +
>    if (GLRO(dl_platform) != NULL && *GLRO(dl_platform) == '\0')
>      /* Avoid an empty string which would disturb us.  */
>      GLRO(dl_platform) = NULL;
> 

Hi Florian,

I've just had a quick look to dl_platform_init for other architectures
to check if there are similar checks. But I haven't found those there.
Are there similar checks for other architectures at a different place?

Is there a special reason for this check beyond giving the user an error
message instead of crashing with SIGILL?

The user will get the error message if build with -march=z14 and running
on z13. If this binary is running on zEC12, it just crashes with SIGILL
as there is a z13 vector instructions before dl_platform_init. Currently
it works on z13, but who knows if this does not change if build with a
different gcc version?

The __LONG_DOUBLE_VX__ macro is quite new and the first gcc release will
be gcc 11. But there is also the __ARCH__ macro. If defined by the used
gcc, it is set to the architecture level determined by -march=xyz
(z15=13; z14=12; z13=11; ...).
See gcc commit "S/390: Rename __S390_ARCH_LEVEL__ to __ARCH__."
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=4727e06bb7c047a10aa502c829b7e4b519d8082b
If I remember correctly, this was introduced with gcc 7 or 8.

Would it make sense to add such a check also if build with -march=z15?:
#if defined __ARCH__
# if __ARCH__ >= 13
  if (!(GLRO(dl_hwcap) & HWCAP_S390_VXRS_EXT2))
    _dl_fatal_printf ("\
Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)\n");
# elif __ARCH__ >= 12
  if (!(GLRO(dl_hwcap) & HWCAP_S390_VXE))
    _dl_fatal_printf ("\
Fatal glibc error: CPU lacks VXE support (z14 or later required)\n");
#endif

There are also configure checks which checks if arch-level specific
instructions are available at build-time (but not for all levels):
z15: HAVE_S390_MIN_ARCH13_ZARCH_ASM_SUPPORT
z13: HAVE_S390_MIN_Z13_ZARCH_ASM_SUPPORT
z196: HAVE_S390_MIN_Z196_ZARCH_ASM_SUPPORT
z10: HAVE_S390_MIN_Z10_ZARCH_ASM_SUPPORT

Thanks,
Stefan
  

* Stefan Liebler:

> I've just had a quick look to dl_platform_init for other architectures
> to check if there are similar checks. But I haven't found those there.
> Are there similar checks for other architectures at a different place?

There is a check in sysdeps/x86/dl-prop.h, but that's different because
it looks at ELF data, not build flags.

> Is there a special reason for this check beyond giving the user an error
> message instead of crashing with SIGILL?

No, there isn't.  It's merely about the diagnostic.

> The user will get the error message if build with -march=z14 and running
> on z13. If this binary is running on zEC12, it just crashes with SIGILL
> as there is a z13 vector instructions before dl_platform_init. Currently
> it works on z13, but who knows if this does not change if build with a
> different gcc version?

To be honest, this change is mostly driven by downstream requirements.
Our zEC12 hardware is powered off, but the z13 hardware is not, so there
is some space for confusion.

I'm going to post a similar patch for POWER9 if I can get it to work.
There we actually hit the issue you mention: some of them min/max
instructions are actually used early during startup.  I don't know yet
if that's before or after the point I'm patching here for s390x.

> The __LONG_DOUBLE_VX__ macro is quite new and the first gcc release will
> be gcc 11. But there is also the __ARCH__ macro. If defined by the used
> gcc, it is set to the architecture level determined by -march=xyz
> (z15=13; z14=12; z13=11; ...).
> See gcc commit "S/390: Rename __S390_ARCH_LEVEL__ to __ARCH__."
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=4727e06bb7c047a10aa502c829b7e4b519d8082b
> If I remember correctly, this was introduced with gcc 7 or 8.
>
> Would it make sense to add such a check also if build with -march=z15?:
> #if defined __ARCH__
> # if __ARCH__ >= 13
>   if (!(GLRO(dl_hwcap) & HWCAP_S390_VXRS_EXT2))
>     _dl_fatal_printf ("\
> Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)\n");
> # elif __ARCH__ >= 12
>   if (!(GLRO(dl_hwcap) & HWCAP_S390_VXE))
>     _dl_fatal_printf ("\
> Fatal glibc error: CPU lacks VXE support (z14 or later required)\n");
> #endif

This is fine with me too.

Thanks,
Florian
  

* Florian Weimer:

> * Stefan Liebler:
>
>> I've just had a quick look to dl_platform_init for other architectures
>> to check if there are similar checks. But I haven't found those there.
>> Are there similar checks for other architectures at a different place?
>
> There is a check in sysdeps/x86/dl-prop.h, but that's different because
> it looks at ELF data, not build flags.

I should have mentioned that the placement isn't arbitrary: This is a
target hook that runs right after dl_hwcap has been extracted from the
auxiliary vector.

Thanks,
Florian
  

On 08/04/2021 17:30, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> I've just had a quick look to dl_platform_init for other architectures
>> to check if there are similar checks. But I haven't found those there.
>> Are there similar checks for other architectures at a different place?
> 
> There is a check in sysdeps/x86/dl-prop.h, but that's different because
> it looks at ELF data, not build flags.
> 
>> Is there a special reason for this check beyond giving the user an error
>> message instead of crashing with SIGILL?
> 
> No, there isn't.  It's merely about the diagnostic.
> 
>> The user will get the error message if build with -march=z14 and running
>> on z13. If this binary is running on zEC12, it just crashes with SIGILL
>> as there is a z13 vector instructions before dl_platform_init. Currently
>> it works on z13, but who knows if this does not change if build with a
>> different gcc version?
> 
> To be honest, this change is mostly driven by downstream requirements.
> Our zEC12 hardware is powered off, but the z13 hardware is not, so there
> is some space for confusion.
> 
> I'm going to post a similar patch for POWER9 if I can get it to work.
> There we actually hit the issue you mention: some of them min/max
> instructions are actually used early during startup.  I don't know yet
> if that's before or after the point I'm patching here for s390x.
> 
>> The __LONG_DOUBLE_VX__ macro is quite new and the first gcc release will
>> be gcc 11. But there is also the __ARCH__ macro. If defined by the used
>> gcc, it is set to the architecture level determined by -march=xyz
>> (z15=13; z14=12; z13=11; ...).
>> See gcc commit "S/390: Rename __S390_ARCH_LEVEL__ to __ARCH__."
>> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=4727e06bb7c047a10aa502c829b7e4b519d8082b
>> If I remember correctly, this was introduced with gcc 7 or 8.
>>
>> Would it make sense to add such a check also if build with -march=z15?:
>> #if defined __ARCH__
>> # if __ARCH__ >= 13
>>   if (!(GLRO(dl_hwcap) & HWCAP_S390_VXRS_EXT2))
>>     _dl_fatal_printf ("\
>> Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)\n");
>> # elif __ARCH__ >= 12
>>   if (!(GLRO(dl_hwcap) & HWCAP_S390_VXE))
>>     _dl_fatal_printf ("\
>> Fatal glibc error: CPU lacks VXE support (z14 or later required)\n");
>> #endif
> 
> This is fine with me too.
Okay. Then I would prefer the __ARCH__ variant.

Thanks,
Stefan