[PATCH 0/3] Checking HWCAP bits against compiler flags

[PATCH 0/3] Checking HWCAP bits against compiler flags

[Florian Weimer]

This series adds checks after HWCAP parsing that all bits required for
compiler flags are present.  It is brittle by design because invalid
instructions could already have been used by the dynamic loader at this
point, but for our downstream use case, it is an improvement over a
plain SIGILL.  (If this is deemed too hackish, I'll probably revert our
downstream changes.)

Tested on various variants on z13, z15, POWER8 and POWER9, and also
built with build-many-glibcs.py.

Thanks,
Florian

Florian Weimer (3):
  elf: Add hook for checking HWCAP bits after auxiliary vector parsing
  powerpc64le: Check HWCAP bits against compiler build flags
  s390x: Check HWCAP bits against compiler flags

 elf/dl-sysdep.c                               |  3 ++
 sysdeps/generic/dl-hwcap-check.h              | 28 +++++++++++
 sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h | 49 +++++++++++++++++++
 sysdeps/s390/s390-64/dl-hwcap-check.h         | 40 +++++++++++++++
 4 files changed, 120 insertions(+)
 create mode 100644 sysdeps/generic/dl-hwcap-check.h
 create mode 100644 sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h
 create mode 100644 sysdeps/s390/s390-64/dl-hwcap-check.h

-- 
2.30.2

[PATCH 1/3] elf: Add hook for checking HWCAP bits after auxiliary vector parsing

[Florian Weimer]

---
 elf/dl-sysdep.c                  |  3 +++
 sysdeps/generic/dl-hwcap-check.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 sysdeps/generic/dl-hwcap-check.h

diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index bd5066fe3b..d47bef1340 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -46,6 +46,7 @@
 
 #include <dl-tunables.h>
 #include <dl-auxv.h>
+#include <dl-hwcap-check.h>
 
 extern char **_environ attribute_hidden;
 extern char _end[] attribute_hidden;
@@ -190,6 +191,8 @@ _dl_sysdep_start (void **start_argptr,
       DL_PLATFORM_AUXV
       }
 
+  dl_hwcap_check ();
+
 #ifndef HAVE_AUX_SECURE
   if (seen != -1)
     {
diff --git a/sysdeps/generic/dl-hwcap-check.h b/sysdeps/generic/dl-hwcap-check.h
new file mode 100644
index 0000000000..8db7a86256
--- /dev/null
+++ b/sysdeps/generic/dl-hwcap-check.h
@@ -0,0 +1,28 @@
+/* Check for hardware capabilities after HWCAP parsing.  Generic version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_HWCAP_CHECK_H
+#define _DL_HWCAP_CHECK_H
+
+static inline void
+dl_hwcap_check (void)
+{
+  /* The generic implementation does not perform any checks.  */
+}
+
+#endif /* _DL_HWCAP_CHECK_H */
-- 
2.30.2

[PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Florian Weimer]

When built with GCC 11.1 and -mcpu=power9, ld.so prints this error
message when running on POWER8:

Fatal glibc error: CPU lacks ISA 3.00 support (POWER9 or later required)

This approach does not work for the POWER10 because the bootstrap
relocation already uses PCREL instructions, so the detection code does
not actually run.
---
 sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h

diff --git a/sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h b/sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h
new file mode 100644
index 0000000000..6c7949c6d2
--- /dev/null
+++ b/sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h
@@ -0,0 +1,49 @@
+/* Check for hardware capabilities after HWCAP parsing.  powerpc64le version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_HWCAP_CHECK_H
+#define _DL_HWCAP_CHECK_H
+
+#include <ldsodefs.h>
+
+static inline void
+dl_hwcap_check (void)
+{
+#ifdef _ARCH_PWR9
+  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00) == 0)
+    _dl_fatal_printf ("\
+Fatal glibc error: CPU lacks ISA 3.00 support (POWER9 or later required)\n");
+#endif
+#ifdef __FLOAT128_HARDWARE__
+  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_IEEE128) == 0)
+    _dl_fatal_printf ("\
+Fatal glibc error: CPU lacks float128 support (POWER 9 or later required)\n");
+#endif
+#if defined _ARCH_PWR10 || defined __PCREL__
+  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1) == 0)
+    _dl_fatal_printf ("\
+Fatal glibc error: CPU lacks ISA 3.10 support (POWER10 or later required)\n");
+#endif
+#ifdef __MMA__
+  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_MMA) == 0)
+    _dl_fatal_printf ("\
+Fatal glibc error: CPU lacks MMA support (POWER10 or later required)\n");
+#endif
+}
+
+#endif /* _DL_HWCAP_CHECK_H */
-- 
2.30.2

[PATCH 3/3] s390x: Check HWCAP bits against compiler flags

[Florian Weimer]

When compiled with GCC 11.1 and -march=z14 -O3 build flags, running
ld.so (or any dynamically linked program) prints:

Fatal glibc error: CPU lacks VXE support (z14 or later required)

Co-Authored-By: Stefan Liebler <stli@linux.ibm.com>
---
 sysdeps/s390/s390-64/dl-hwcap-check.h | 40 +++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 sysdeps/s390/s390-64/dl-hwcap-check.h

diff --git a/sysdeps/s390/s390-64/dl-hwcap-check.h b/sysdeps/s390/s390-64/dl-hwcap-check.h
new file mode 100644
index 0000000000..87e18be6bd
--- /dev/null
+++ b/sysdeps/s390/s390-64/dl-hwcap-check.h
@@ -0,0 +1,40 @@
+/* Check for hardware capabilities after HWCAP parsing.  S390 version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_HWCAP_CHECK_H
+#define _DL_HWCAP_CHECK_H
+
+#include <ldsodefs.h>
+
+static inline void
+dl_hwcap_check (void)
+{
+#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
+#endif /* __ARCH__ */
+}
+
+#endif /* _DL_HWCAP_CHECK_H */
-- 
2.30.2

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Lucas A. M. Magalhaes]

Hi Florian. I've tested this patch on POWER[8..10] with -mcpu=power9 and
-mcpu=power10 built with GCC 10.3.1.

When built with -mcpu=power10 I got a SIGILL on POWER8 and POWER10.
For testing I'm just running ./libc.so.  I try to inspect it with
gdb. With -mcpu=power9 I can see it passing through dl_hwcap_check
but not with -mcpu=power10.

I maybe doing something wrong though.

---
Lucas A. M. Magalhães

Quoting Florian Weimer via Libc-alpha (2021-05-06 09:30:15)
> When built with GCC 11.1 and -mcpu=power9, ld.so prints this error
> message when running on POWER8:
> 
> Fatal glibc error: CPU lacks ISA 3.00 support (POWER9 or later required)
> 
> This approach does not work for the POWER10 because the bootstrap
> relocation already uses PCREL instructions, so the detection code does
> not actually run.
> ---
>  sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h
> 
> diff --git a/sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h b/sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h
> new file mode 100644
> index 0000000000..6c7949c6d2
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/le/dl-hwcap-check.h
> @@ -0,0 +1,49 @@
> +/* Check for hardware capabilities after HWCAP parsing.  powerpc64le version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_HWCAP_CHECK_H
> +#define _DL_HWCAP_CHECK_H
> +
> +#include <ldsodefs.h>
> +
> +static inline void
> +dl_hwcap_check (void)
> +{
> +#ifdef _ARCH_PWR9
> +  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_00) == 0)
> +    _dl_fatal_printf ("\
> +Fatal glibc error: CPU lacks ISA 3.00 support (POWER9 or later required)\n");
> +#endif
> +#ifdef __FLOAT128_HARDWARE__
> +  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_IEEE128) == 0)
> +    _dl_fatal_printf ("\
> +Fatal glibc error: CPU lacks float128 support (POWER 9 or later required)\n");
> +#endif
> +#if defined _ARCH_PWR10 || defined __PCREL__
> +  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_ARCH_3_1) == 0)
> +    _dl_fatal_printf ("\
> +Fatal glibc error: CPU lacks ISA 3.10 support (POWER10 or later required)\n");
> +#endif
> +#ifdef __MMA__
> +  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_MMA) == 0)
> +    _dl_fatal_printf ("\
> +Fatal glibc error: CPU lacks MMA support (POWER10 or later required)\n");
> +#endif
> +}
> +
> +#endif /* _DL_HWCAP_CHECK_H */
> -- 
> 2.30.2
> 
>

Re: [PATCH 3/3] s390x: Check HWCAP bits against compiler flags

[Stefan Liebler]

On 06/05/2021 14:30, Florian Weimer via Libc-alpha wrote:
> When compiled with GCC 11.1 and -march=z14 -O3 build flags, running
> ld.so (or any dynamically linked program) prints:
> 
> Fatal glibc error: CPU lacks VXE support (z14 or later required)
> 
> Co-Authored-By: Stefan Liebler <stli@linux.ibm.com>
> ---
>  sysdeps/s390/s390-64/dl-hwcap-check.h | 40 +++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 sysdeps/s390/s390-64/dl-hwcap-check.h
> 
> diff --git a/sysdeps/s390/s390-64/dl-hwcap-check.h b/sysdeps/s390/s390-64/dl-hwcap-check.h
> new file mode 100644
> index 0000000000..87e18be6bd
> --- /dev/null
> +++ b/sysdeps/s390/s390-64/dl-hwcap-check.h
> @@ -0,0 +1,40 @@
> +/* Check for hardware capabilities after HWCAP parsing.  S390 version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_HWCAP_CHECK_H
> +#define _DL_HWCAP_CHECK_H
> +
> +#include <ldsodefs.h>
> +
> +static inline void
> +dl_hwcap_check (void)
> +{
> +#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
> +#endif /* __ARCH__ */
> +}
> +
> +#endif /* _DL_HWCAP_CHECK_H */
> 

Hi Florian,

while running some tests, I got the messages as expected:
on z13:
- "-march=z13" => no message
- "-march=z14" => "z14 or later required"
- "-march=z15" => "z15 or later required"

on z14:
- "-march=z13" => no message
- "-march=z14" => no message
- "-march=z15" => "z15 or later required"

on z15:
- "-march=z13" => no message
- "-march=z14" => no message
- "-march=z15" => no message

Reviewed-by: Stefan Liebler <stli@linux.ibm.com>

Thanks,
Stefan

Re: [PATCH 1/3] elf: Add hook for checking HWCAP bits after auxiliary vector parsing

[Stefan Liebler]

On 06/05/2021 14:30, Florian Weimer via Libc-alpha wrote:
> ---
>  elf/dl-sysdep.c                  |  3 +++
>  sysdeps/generic/dl-hwcap-check.h | 28 ++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 sysdeps/generic/dl-hwcap-check.h
> 
> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
> index bd5066fe3b..d47bef1340 100644
> --- a/elf/dl-sysdep.c
> +++ b/elf/dl-sysdep.c
> @@ -46,6 +46,7 @@
> 
>  #include <dl-tunables.h>
>  #include <dl-auxv.h>
> +#include <dl-hwcap-check.h>
> 
>  extern char **_environ attribute_hidden;
>  extern char _end[] attribute_hidden;
> @@ -190,6 +191,8 @@ _dl_sysdep_start (void **start_argptr,
>        DL_PLATFORM_AUXV
>        }
> 
> +  dl_hwcap_check ();
> +
OK.
(GLRO(dl_hwcap) is setup with content of AT_HWCAP in the loop above)
>  #ifndef HAVE_AUX_SECURE
>    if (seen != -1)
>      {
> diff --git a/sysdeps/generic/dl-hwcap-check.h b/sysdeps/generic/dl-hwcap-check.h
> new file mode 100644
> index 0000000000..8db7a86256
> --- /dev/null
> +++ b/sysdeps/generic/dl-hwcap-check.h
> @@ -0,0 +1,28 @@
> +/* Check for hardware capabilities after HWCAP parsing.  Generic version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_HWCAP_CHECK_H
> +#define _DL_HWCAP_CHECK_H
> +
> +static inline void
> +dl_hwcap_check (void)
> +{
> +  /* The generic implementation does not perform any checks.  */
> +}
> +
> +#endif /* _DL_HWCAP_CHECK_H */
> 
OK

Reviewed-by: Stefan Liebler <stli@linux.ibm.com>

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Florian Weimer]

* Lucas A. M. Magalhaes:

> Hi Florian. I've tested this patch on POWER[8..10] with -mcpu=power9 and
> -mcpu=power10 built with GCC 10.3.1.
>
> When built with -mcpu=power10 I got a SIGILL on POWER8 and POWER10.

Do you mean POWER8 and POWER9?

> For testing I'm just running ./libc.so.  I try to inspect it with
> gdb. With -mcpu=power9 I can see it passing through dl_hwcap_check
> but not with -mcpu=power10.
>
> I maybe doing something wrong though.

I tried to allude in the commit message that this is expected.  Maybe I
should drop the POWER10 check?

I do not think this is fixable in the long term because I expect that
POWER10 builds won't need the bootstrap relocation in the dynamic loader
thanks to its PCREL instructions.  (POWER10 should be
PI_STATIC_AND_HIDDEN.)  We could disable POWER10 code for early parts of
the dynamic loader, but that will only work until we start to rely on
PI_STATIC_AND_HIDDEN, which has other benefits.

Thanks,
Florian

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Lucas A. M. Magalhaes]

Quoting Florian Weimer (2021-05-12 05:27:33)
> * Lucas A. M. Magalhaes:
> 
> > Hi Florian. I've tested this patch on POWER[8..10] with -mcpu=power9 and
> > -mcpu=power10 built with GCC 10.3.1.
> >
> > When built with -mcpu=power10 I got a SIGILL on POWER8 and POWER10.
> 
> Do you mean POWER8 and POWER9?
> 
Yes. Sorry about that.

> > For testing I'm just running ./libc.so.  I try to inspect it with
> > gdb. With -mcpu=power9 I can see it passing through dl_hwcap_check
> > but not with -mcpu=power10.
> >
> > I maybe doing something wrong though.
> 
> I tried to allude in the commit message that this is expected.  Maybe I
> should drop the POWER10 check?
> 
Your commit message is fine, I misunderstood it. IMHO it's better to
drop POWER10 check and add a comment explaining why it's not there.

> I do not think this is fixable in the long term because I expect that
> POWER10 builds won't need the bootstrap relocation in the dynamic loader
> thanks to its PCREL instructions.  (POWER10 should be
> PI_STATIC_AND_HIDDEN.)  We could disable POWER10 code for early parts of
> the dynamic loader, but that will only work until we start to rely on
> PI_STATIC_AND_HIDDEN, which has other benefits.
I'm not familiar with this bootstrap relocation but for what I understood
there is early code using PCREL instructions so it SIGILL before running
this code, right?

I don't oppose this change but still don't get why do we need this code?
What's the benefit of the message instead of just SIGILL?

---
Lucas A. M. Magalhães

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Tulio Magno Quites Machado Filho]

Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:

> I tried to allude in the commit message that this is expected.  Maybe I
> should drop the POWER10 check?

I don't think this is necessary.
Even if you removed the P10 check, I don't think the P9 check is guaranteed to
work because when _ARCH_PWR9 is set, the compiler is allowed to use P9
instructions in this function before the test is executed.

I think this patch is trying to improve the error message to the user.

With that said, it doesn't hurt to copy your explanation as a source code
comment, i.e.:

/* This approach does not work for the POWER10 because the bootstrap
   relocation already uses PCREL instructions, so the detection code does
   not actually run.  */

>> +#ifdef __MMA__
>> +  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_MMA) == 0)
>> +    _dl_fatal_printf ("\
>> +Fatal glibc error: CPU lacks MMA support (POWER10 or later required)\n");
>> +#endif
>> +}

I'm not sure it's a good idea to require MMA.
__MMA__ is enabled with -mcpu=power10.  However, I'm not aware of any compilers
able to auto-mma code.  In other words, we have to use it explicitly right now.
Which means we shouldn't have MMA instructions in glibc at the moment.

-- 
Tulio Magno

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Florian Weimer]

* Tulio Magno Quites Machado Filho:

> Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:
>
>> I tried to allude in the commit message that this is expected.  Maybe I
>> should drop the POWER10 check?
>
> I don't think this is necessary.
> Even if you removed the P10 check, I don't think the P9 check is guaranteed to
> work because when _ARCH_PWR9 is set, the compiler is allowed to use P9
> instructions in this function before the test is executed.

Correct, it just emprically works for me.

> I think this patch is trying to improve the error message to the user.

And make sure that there is an error.  We don't want things to crash
later, or after system upgrades using a different compiler version with
more optimizations.

> With that said, it doesn't hurt to copy your explanation as a source code
> comment, i.e.:
>
> /* This approach does not work for the POWER10 because the bootstrap
>    relocation already uses PCREL instructions, so the detection code does
>    not actually run.  */

My point was not so much that the bootstrap relocation uses PCREL
instructions, but we eventually *want* to use it such instructions and
get rid of the bootstrap relocation (like we do for other
PI_STATIC_AND_HIDDEN targets).

>>> +#ifdef __MMA__
>>> +  if ((GLRO (dl_hwcap2) & PPC_FEATURE2_MMA) == 0)
>>> +    _dl_fatal_printf ("\
>>> +Fatal glibc error: CPU lacks MMA support (POWER10 or later required)\n");
>>> +#endif
>>> +}
>
> I'm not sure it's a good idea to require MMA.  __MMA__ is enabled with
> -mcpu=power10.  However, I'm not aware of any compilers able to
> auto-mma code.  In other words, we have to use it explicitly right
> now.  Which means we shouldn't have MMA instructions in glibc at the
> moment.

I think we should require it if it is part of -mcpu=power10.  This check
is not just for glibc code, but for entire distributions using
consistent build flags across everything.  If they don't want MMA for
some reason, they will have to build wuth -mno-mmma.

Thanks,
Florian

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Tulio Magno Quites Machado Filho]

Florian Weimer <fweimer@redhat.com> writes:

> I think we should require it if it is part of -mcpu=power10.  This check
> is not just for glibc code, but for entire distributions using
> consistent build flags across everything.  If they don't want MMA for
> some reason, they will have to build wuth -mno-mmma.

OK. Good point.
I agree.

Thanks!

-- 
Tulio Magno

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Florian Weimer]

* Tulio Magno Quites Machado Filho:

> Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:
>
>> I tried to allude in the commit message that this is expected.  Maybe I
>> should drop the POWER10 check?
>
> I don't think this is necessary.
> Even if you removed the P10 check, I don't think the P9 check is guaranteed to
> work because when _ARCH_PWR9 is set, the compiler is allowed to use P9
> instructions in this function before the test is executed.
>
> I think this patch is trying to improve the error message to the user.
>
> With that said, it doesn't hurt to copy your explanation as a source code
> comment, i.e.:
>
> /* This approach does not work for the POWER10 because the bootstrap
>    relocation already uses PCREL instructions, so the detection code does
>    not actually run.  */

What about this instead?

  /* This check is not actually reached when building for POWER10 and
     running on POWER9 because there are faulting PCREL instructions
     before this point.  */

Thanks,
Florian

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Tulio Magno Quites Machado Filho]

Florian Weimer <fweimer@redhat.com> writes:

> What about this instead?
>
>   /* This check is not actually reached when building for POWER10 and
>      running on POWER9 because there are faulting PCREL instructions
>      before this point.  */

LGTM

-- 
Tulio Magno

Re: [PATCH 2/3] powerpc64le: Check HWCAP bits against compiler build flags

[Florian Weimer]

* Tulio Magno Quites Machado Filho:

> Florian Weimer <fweimer@redhat.com> writes:
>
>> What about this instead?
>>
>>   /* This check is not actually reached when building for POWER10 and
>>      running on POWER9 because there are faulting PCREL instructions
>>      before this point.  */
>
> LGTM

Thanks, then I'll commit the full series tomorrow.

Florian