Message ID | 87k0pfmmpy.fsf@oldenburg.str.redhat.com |
---|---|
State | Superseded |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4B6C13893672; Tue, 6 Apr 2021 15:55:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4B6C13893672 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1617724534; bh=TJGm9fLDY8r+iXyPAKkLg2xtHxRAF9pmn0+ZnvGtgbQ=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=R3lqQXJnAKTlDyx+er3PoiUdTNjaNdvNWib8Ncm/ht3YJ2+P4yq9ajQ59gofGhmXy EoFj4rNxZrifUv+MIb7r6ElGtfTq+E6Mf85MWSq+Ts1pWRThWPQmo1Gh+ns3pQk5JB w8MerLpJNL2ovTeD02Z7yM+wah6UkwTo8xrIM3Go= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 3F3CD3893661 for <libc-alpha@sourceware.org>; Tue, 6 Apr 2021 15:55:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3F3CD3893661 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-98-Y_HPiLxjN4i1sMVS1l8qkw-1; Tue, 06 Apr 2021 11:55:26 -0400 X-MC-Unique: Y_HPiLxjN4i1sMVS1l8qkw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 86EAB800D53; Tue, 6 Apr 2021 15:55:25 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-12.rdu2.redhat.com [10.10.112.12]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C53855DAA5; Tue, 6 Apr 2021 15:55:24 +0000 (UTC) To: libc-alpha@sourceware.org Subject: s390x: Diagnose missing VXE at run time if required at build time Date: Tue, 06 Apr 2021 17:55:37 +0200 Message-ID: <87k0pfmmpy.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Florian Weimer <fweimer@redhat.com> Cc: Stefan Liebler <stli@linux.ibm.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
s390x: Diagnose missing VXE at run time if required at build time
|
|
Commit Message
Florian Weimer
April 6, 2021, 3:55 p.m. UTC
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__");
Comments
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
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;