* [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
2019-04-10 20:12 [PATCH 1/2] s390: only build for new CPUs with clang Arnd Bergmann
@ 2019-04-10 20:12 ` Arnd Bergmann
2019-04-10 22:14 ` Nick Desaulniers
2019-04-10 22:20 ` [PATCH 1/2] s390: only build for new CPUs with clang Nick Desaulniers
2019-04-11 6:23 ` Heiko Carstens
2 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2019-04-10 20:12 UTC (permalink / raw)
To: Martin Schwidefsky, Heiko Carstens
Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
linux-s390, Arnd Bergmann, Vasily Gorbik, Masahiro Yamada,
Philipp Rudo, Hendrik Brueckner, linux-kernel
The purgatory and boot Makefiles do not inherit the original cflags,
so clang falls back to the default target architecture when building it,
typically this would be x86 when cross-compiling.
Add $(CLANG_FLAGS) everywhere so we pass the correct --target=s390x-linux
option when cross-compiling.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/s390/Makefile | 5 +++--
arch/s390/purgatory/Makefile | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index 9c079a506325..443990791099 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -17,12 +17,13 @@ KBUILD_CFLAGS_MODULE += -fPIC
KBUILD_AFLAGS += -m64
KBUILD_CFLAGS += -m64
aflags_dwarf := -Wa,-gdwarf-2
-KBUILD_AFLAGS_DECOMPRESSOR := -m64 -D__ASSEMBLY__
+KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
-KBUILD_CFLAGS_DECOMPRESSOR := -m64 -O2
+KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2
KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float
KBUILD_CFLAGS_DECOMPRESSOR += -fno-asynchronous-unwind-tables
+KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-disable-warning,pointer-sign)
KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-option,-ffreestanding)
KBUILD_CFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),-g)
KBUILD_CFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO_DWARF4), $(call cc-option, -gdwarf-4,))
diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
index ce6a3f75065b..ecd0b3847fef 100644
--- a/arch/s390/purgatory/Makefile
+++ b/arch/s390/purgatory/Makefile
@@ -22,6 +22,7 @@ KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes
KBUILD_CFLAGS += -Wno-pointer-sign -Wno-sign-compare
KBUILD_CFLAGS += -fno-zero-initialized-in-bss -fno-builtin -ffreestanding
KBUILD_CFLAGS += -c -MD -Os -m64 -msoft-float -fno-common
+KBUILD_CFLAGS += $(CLANG_FLAGS)
KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
--
2.20.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
2019-04-10 20:12 ` [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
@ 2019-04-10 22:14 ` Nick Desaulniers
2019-04-11 8:52 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-04-10 22:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
Nathan Chancellor, linux-s390, Vasily Gorbik, Masahiro Yamada,
Philipp Rudo, Hendrik Brueckner, LKML
On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The purgatory and boot Makefiles do not inherit the original cflags,
> so clang falls back to the default target architecture when building it,
> typically this would be x86 when cross-compiling.
>
> Add $(CLANG_FLAGS) everywhere so we pass the correct --target=s390x-linux
> option when cross-compiling.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/s390/Makefile | 5 +++--
> arch/s390/purgatory/Makefile | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> index 9c079a506325..443990791099 100644
> --- a/arch/s390/Makefile
> +++ b/arch/s390/Makefile
> @@ -17,12 +17,13 @@ KBUILD_CFLAGS_MODULE += -fPIC
> KBUILD_AFLAGS += -m64
> KBUILD_CFLAGS += -m64
> aflags_dwarf := -Wa,-gdwarf-2
> -KBUILD_AFLAGS_DECOMPRESSOR := -m64 -D__ASSEMBLY__
> +KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
> KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
> -KBUILD_CFLAGS_DECOMPRESSOR := -m64 -O2
> +KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2
> KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
> KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float
> KBUILD_CFLAGS_DECOMPRESSOR += -fno-asynchronous-unwind-tables
Thanks for the respin with Nathan's suggestion.
> +KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-disable-warning,pointer-sign)
What's up with this ^ ? Seems like the top level sets it (without
cc-disable-warning :( ), but then KBUILD_CFLAGS_DECOMPRESSOR discards
it. Does Clang actually flag code in this arch (that GCC doesn't)?
> KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-option,-ffreestanding)
> KBUILD_CFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),-g)
> KBUILD_CFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO_DWARF4), $(call cc-option, -gdwarf-4,))
> diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
> index ce6a3f75065b..ecd0b3847fef 100644
> --- a/arch/s390/purgatory/Makefile
> +++ b/arch/s390/purgatory/Makefile
> @@ -22,6 +22,7 @@ KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes
> KBUILD_CFLAGS += -Wno-pointer-sign -Wno-sign-compare
> KBUILD_CFLAGS += -fno-zero-initialized-in-bss -fno-builtin -ffreestanding
> KBUILD_CFLAGS += -c -MD -Os -m64 -msoft-float -fno-common
> +KBUILD_CFLAGS += $(CLANG_FLAGS)
> KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
> KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
>
> --
> 2.20.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
2019-04-10 22:14 ` Nick Desaulniers
@ 2019-04-11 8:52 ` Arnd Bergmann
2019-04-11 18:08 ` Nick Desaulniers
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2019-04-11 8:52 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
Nathan Chancellor, linux-s390, Vasily Gorbik, Masahiro Yamada,
Philipp Rudo, Hendrik Brueckner, LKML
On Thu, Apr 11, 2019 at 12:14 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
> On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > The purgatory and boot Makefiles do not inherit the original cflags,
> > so clang falls back to the default target architecture when building it,
> > typically this would be x86 when cross-compiling.
> >
> > Add $(CLANG_FLAGS) everywhere so we pass the correct --target=s390x-linux
> > option when cross-compiling.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > arch/s390/Makefile | 5 +++--
> > arch/s390/purgatory/Makefile | 1 +
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> > index 9c079a506325..443990791099 100644
> > --- a/arch/s390/Makefile
> > +++ b/arch/s390/Makefile
> > @@ -17,12 +17,13 @@ KBUILD_CFLAGS_MODULE += -fPIC
> > KBUILD_AFLAGS += -m64
> > KBUILD_CFLAGS += -m64
> > aflags_dwarf := -Wa,-gdwarf-2
> > -KBUILD_AFLAGS_DECOMPRESSOR := -m64 -D__ASSEMBLY__
> > +KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
> > KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
> > -KBUILD_CFLAGS_DECOMPRESSOR := -m64 -O2
> > +KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2
> > KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
> > KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float
> > KBUILD_CFLAGS_DECOMPRESSOR += -fno-asynchronous-unwind-tables
>
> Thanks for the respin with Nathan's suggestion.
>
> > +KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-disable-warning,pointer-sign)
>
> What's up with this ^ ? Seems like the top level sets it (without
> cc-disable-warning :( ), but then KBUILD_CFLAGS_DECOMPRESSOR discards
> it. Does Clang actually flag code in this arch (that GCC doesn't)?
Oops, that should have been a separate patch.
I think what happens is that clang warns more aggressively about pointer sign
bugs than gcc in some cases, and some of those cases happen in s390
header files that are included by both the kernel and the decompressor.
The full warning log without this change is rather long, see
https://pastebin.com/KG9xaTNB
I also tried patching the code to avoid the warnings, but I'm not entirely
happy with that result either, see
https://pastebin.com/pSMz5eZA
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
2019-04-11 8:52 ` Arnd Bergmann
@ 2019-04-11 18:08 ` Nick Desaulniers
2019-04-15 6:32 ` Martin Schwidefsky
0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-04-11 18:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
Nathan Chancellor, linux-s390, Vasily Gorbik, Masahiro Yamada,
Philipp Rudo, Hendrik Brueckner, LKML
On Thu, Apr 11, 2019 at 1:52 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Apr 11, 2019 at 12:14 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> > On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > The purgatory and boot Makefiles do not inherit the original cflags,
> > > so clang falls back to the default target architecture when building it,
> > > typically this would be x86 when cross-compiling.
> > >
> > > Add $(CLANG_FLAGS) everywhere so we pass the correct --target=s390x-linux
> > > option when cross-compiling.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > arch/s390/Makefile | 5 +++--
> > > arch/s390/purgatory/Makefile | 1 +
> > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> > > index 9c079a506325..443990791099 100644
> > > --- a/arch/s390/Makefile
> > > +++ b/arch/s390/Makefile
> > > @@ -17,12 +17,13 @@ KBUILD_CFLAGS_MODULE += -fPIC
> > > KBUILD_AFLAGS += -m64
> > > KBUILD_CFLAGS += -m64
> > > aflags_dwarf := -Wa,-gdwarf-2
> > > -KBUILD_AFLAGS_DECOMPRESSOR := -m64 -D__ASSEMBLY__
> > > +KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
> > > KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
> > > -KBUILD_CFLAGS_DECOMPRESSOR := -m64 -O2
> > > +KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2
> > > KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
> > > KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float
> > > KBUILD_CFLAGS_DECOMPRESSOR += -fno-asynchronous-unwind-tables
> >
> > Thanks for the respin with Nathan's suggestion.
> >
> > > +KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-disable-warning,pointer-sign)
> >
> > What's up with this ^ ? Seems like the top level sets it (without
> > cc-disable-warning :( ), but then KBUILD_CFLAGS_DECOMPRESSOR discards
> > it. Does Clang actually flag code in this arch (that GCC doesn't)?
>
> Oops, that should have been a separate patch.
>
> I think what happens is that clang warns more aggressively about pointer sign
> bugs than gcc in some cases, and some of those cases happen in s390
> header files that are included by both the kernel and the decompressor.
>
> The full warning log without this change is rather long, see
> https://pastebin.com/KG9xaTNB
From this link, it looks like the definitions of:
__atomic64_or
__atomic64_and
__atomic64_xor
and their *_barrier variants are problematic. I think converting
those to use unsigned long is the way to go. Shouldn't you be doing
bitwise ops on unsigned types anyways?
The warnings with __atomic64_add are tougher to read/understand since
at that point the log lines look like they start to mix together.
>
> I also tried patching the code to avoid the warnings, but I'm not entirely
> happy with that result either, see
> https://pastebin.com/pSMz5eZA
That's no terrible, IMO, particularly with the change I suggest above.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
2019-04-11 18:08 ` Nick Desaulniers
@ 2019-04-15 6:32 ` Martin Schwidefsky
0 siblings, 0 replies; 11+ messages in thread
From: Martin Schwidefsky @ 2019-04-15 6:32 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Arnd Bergmann, Heiko Carstens, clang-built-linux,
Nathan Chancellor, linux-s390, Vasily Gorbik, Masahiro Yamada,
Philipp Rudo, Hendrik Brueckner, LKML
On Thu, 11 Apr 2019 11:08:31 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Thu, Apr 11, 2019 at 1:52 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Thu, Apr 11, 2019 at 12:14 AM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> > > On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > The purgatory and boot Makefiles do not inherit the original cflags,
> > > > so clang falls back to the default target architecture when building it,
> > > > typically this would be x86 when cross-compiling.
> > > >
> > > > Add $(CLANG_FLAGS) everywhere so we pass the correct --target=s390x-linux
> > > > option when cross-compiling.
> > > >
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > > arch/s390/Makefile | 5 +++--
> > > > arch/s390/purgatory/Makefile | 1 +
> > > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> > > > index 9c079a506325..443990791099 100644
> > > > --- a/arch/s390/Makefile
> > > > +++ b/arch/s390/Makefile
> > > > @@ -17,12 +17,13 @@ KBUILD_CFLAGS_MODULE += -fPIC
> > > > KBUILD_AFLAGS += -m64
> > > > KBUILD_CFLAGS += -m64
> > > > aflags_dwarf := -Wa,-gdwarf-2
> > > > -KBUILD_AFLAGS_DECOMPRESSOR := -m64 -D__ASSEMBLY__
> > > > +KBUILD_AFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -D__ASSEMBLY__
> > > > KBUILD_AFLAGS_DECOMPRESSOR += $(if $(CONFIG_DEBUG_INFO),$(aflags_dwarf))
> > > > -KBUILD_CFLAGS_DECOMPRESSOR := -m64 -O2
> > > > +KBUILD_CFLAGS_DECOMPRESSOR := $(CLANG_FLAGS) -m64 -O2
> > > > KBUILD_CFLAGS_DECOMPRESSOR += -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY
> > > > KBUILD_CFLAGS_DECOMPRESSOR += -fno-delete-null-pointer-checks -msoft-float
> > > > KBUILD_CFLAGS_DECOMPRESSOR += -fno-asynchronous-unwind-tables
> > >
> > > Thanks for the respin with Nathan's suggestion.
> > >
> > > > +KBUILD_CFLAGS_DECOMPRESSOR += $(call cc-disable-warning,pointer-sign)
> > >
> > > What's up with this ^ ? Seems like the top level sets it (without
> > > cc-disable-warning :( ), but then KBUILD_CFLAGS_DECOMPRESSOR discards
> > > it. Does Clang actually flag code in this arch (that GCC doesn't)?
> >
> > Oops, that should have been a separate patch.
> >
> > I think what happens is that clang warns more aggressively about pointer sign
> > bugs than gcc in some cases, and some of those cases happen in s390
> > header files that are included by both the kernel and the decompressor.
> >
> > The full warning log without this change is rather long, see
> > https://pastebin.com/KG9xaTNB
>
> From this link, it looks like the definitions of:
> __atomic64_or
> __atomic64_and
> __atomic64_xor
> and their *_barrier variants are problematic. I think converting
> those to use unsigned long is the way to go. Shouldn't you be doing
> bitwise ops on unsigned types anyways?
These functions follow the type of atomic64_t which is a "long" wrapped
in a structure. We do not want to change that to unsigned long, are we?
Then having some of the functions operate on "long" and others on
"unsigned long" seem odd.
> The warnings with __atomic64_add are tougher to read/understand since
> at that point the log lines look like they start to mix together.
>
> >
> > I also tried patching the code to avoid the warnings, but I'm not entirely
> > happy with that result either, see
> > https://pastebin.com/pSMz5eZA
>
> That's no terrible, IMO, particularly with the change I suggest above.
That is not too bad, the only change I do not like is the s/u8/char/ in
struct ipl_block_fcp.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] s390: only build for new CPUs with clang
2019-04-10 20:12 [PATCH 1/2] s390: only build for new CPUs with clang Arnd Bergmann
2019-04-10 20:12 ` [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
@ 2019-04-10 22:20 ` Nick Desaulniers
2019-04-11 10:18 ` Arnd Bergmann
2019-04-11 6:23 ` Heiko Carstens
2 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-04-10 22:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Martin Schwidefsky, Heiko Carstens, Vasily Gorbik,
clang-built-linux, Nathan Chancellor, linux-s390, Masahiro Yamada,
Philipp Rudo, Tony Krowiak, LKML
On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> llvm does does not understand -march=z9-109 and older target
Please file bugs for these in LLVM's issue tracker. It might be
possible to enable these additional architecture variants if they're
similar to existing ones and simply unrecognized. IIRC, we had this
issue with armv7 variants.
> specifiers, so disable the respective Kconfig settings and
> the logic to make the boot code work on old systems when
> building with clang.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/s390/Kconfig | 6 ++++++
> arch/s390/boot/Makefile | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 8cd860cba4d1..1a2eec61196d 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -240,6 +240,7 @@ choice
>
> config MARCH_Z900
> bool "IBM zSeries model z800 and z900"
> + depends on !CC_IS_CLANG
> select HAVE_MARCH_Z900_FEATURES
> help
> Select this to enable optimizations for model z800/z900 (2064 and
> @@ -248,6 +249,7 @@ config MARCH_Z900
>
> config MARCH_Z990
> bool "IBM zSeries model z890 and z990"
> + depends on !CC_IS_CLANG
> select HAVE_MARCH_Z990_FEATURES
> help
> Select this to enable optimizations for model z890/z990 (2084 and
> @@ -256,6 +258,7 @@ config MARCH_Z990
>
> config MARCH_Z9_109
> bool "IBM System z9"
> + depends on !CC_IS_CLANG
> select HAVE_MARCH_Z9_109_FEATURES
> help
> Select this to enable optimizations for IBM System z9 (2094 and
> @@ -347,12 +350,15 @@ config TUNE_DEFAULT
>
> config TUNE_Z900
> bool "IBM zSeries model z800 and z900"
> + depends on !CC_IS_CLANG
>
> config TUNE_Z990
> bool "IBM zSeries model z890 and z990"
> + depends on !CC_IS_CLANG
>
> config TUNE_Z9_109
> bool "IBM System z9"
> + depends on !CC_IS_CLANG
>
> config TUNE_Z10
> bool "IBM System z10"
> diff --git a/arch/s390/boot/Makefile b/arch/s390/boot/Makefile
> index c844eaf24ed7..953a74d04990 100644
> --- a/arch/s390/boot/Makefile
> +++ b/arch/s390/boot/Makefile
> @@ -11,6 +11,7 @@ KASAN_SANITIZE := n
> KBUILD_AFLAGS := $(KBUILD_AFLAGS_DECOMPRESSOR)
> KBUILD_CFLAGS := $(KBUILD_CFLAGS_DECOMPRESSOR)
>
> +ifndef CONFIG_CC_IS_CLANG
> #
> # Use -march=z900 for als.c to be able to print an error
> # message if the kernel is started on a machine which is too old
> @@ -25,6 +26,7 @@ CFLAGS_als.o += -march=z900
> CFLAGS_REMOVE_sclp_early_core.o += $(CC_FLAGS_MARCH)
> CFLAGS_sclp_early_core.o += -march=z900
> endif
> +endif
>
> CFLAGS_sclp_early_core.o += -I$(srctree)/drivers/s390/char
>
> --
> 2.20.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] s390: only build for new CPUs with clang
2019-04-10 22:20 ` [PATCH 1/2] s390: only build for new CPUs with clang Nick Desaulniers
@ 2019-04-11 10:18 ` Arnd Bergmann
2019-04-11 17:46 ` Nick Desaulniers
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2019-04-11 10:18 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Martin Schwidefsky, Heiko Carstens, Vasily Gorbik,
clang-built-linux, Nathan Chancellor, linux-s390, Masahiro Yamada,
Philipp Rudo, Tony Krowiak, LKML
On Thu, Apr 11, 2019 at 12:20 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > llvm does does not understand -march=z9-109 and older target
>
> Please file bugs for these in LLVM's issue tracker. It might be
> possible to enable these additional architecture variants if they're
> similar to existing ones and simply unrecognized. IIRC, we had this
> issue with armv7 variants.
I really don't see that as necessary here. While generally speaking,
it would be nice to support the widest possible range of hardware,
and it would not be hard to add this support, it seems highly unlikely
that anyone would actually want to use that in case of s390.
The last enterprise distros to support z9 were SLES11 (end of life as
of last month) and RHEL6 (ending its 10 year life next year). Anyone
who is still on those releases will probably not install a brand new
compiler, and anyone who is on newer releases won't run on prehistoric
hardware.
Debian still runs on very old hardware in theory, but even there you'd
have a hard time finding someone to test the output of the compiler
on an old machine.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/2] s390: only build for new CPUs with clang
2019-04-11 10:18 ` Arnd Bergmann
@ 2019-04-11 17:46 ` Nick Desaulniers
0 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2019-04-11 17:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Martin Schwidefsky, Heiko Carstens, Vasily Gorbik,
clang-built-linux, Nathan Chancellor, linux-s390, Masahiro Yamada,
Philipp Rudo, Tony Krowiak, LKML
On Thu, Apr 11, 2019 at 3:18 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Apr 11, 2019 at 12:20 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > On Wed, Apr 10, 2019 at 1:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > llvm does does not understand -march=z9-109 and older target
> >
> > Please file bugs for these in LLVM's issue tracker. It might be
> > possible to enable these additional architecture variants if they're
> > similar to existing ones and simply unrecognized. IIRC, we had this
> > issue with armv7 variants.
>
> I really don't see that as necessary here. While generally speaking,
> it would be nice to support the widest possible range of hardware,
> and it would not be hard to add this support, it seems highly unlikely
> that anyone would actually want to use that in case of s390.
>
> The last enterprise distros to support z9 were SLES11 (end of life as
> of last month) and RHEL6 (ending its 10 year life next year). Anyone
> who is still on those releases will probably not install a brand new
> compiler, and anyone who is on newer releases won't run on prehistoric
> hardware.
Are those machine variants worth dropping kernel support for then?
>
> Debian still runs on very old hardware in theory, but even there you'd
> have a hard time finding someone to test the output of the compiler
> on an old machine.
>
> Arnd
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] s390: only build for new CPUs with clang
2019-04-10 20:12 [PATCH 1/2] s390: only build for new CPUs with clang Arnd Bergmann
2019-04-10 20:12 ` [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
2019-04-10 22:20 ` [PATCH 1/2] s390: only build for new CPUs with clang Nick Desaulniers
@ 2019-04-11 6:23 ` Heiko Carstens
2019-04-11 10:19 ` Arnd Bergmann
2 siblings, 1 reply; 11+ messages in thread
From: Heiko Carstens @ 2019-04-11 6:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Martin Schwidefsky, Vasily Gorbik, clang-built-linux,
Nick Desaulniers, Nathan Chancellor, linux-s390, Masahiro Yamada,
Philipp Rudo, Tony Krowiak, linux-kernel
On Wed, Apr 10, 2019 at 10:12:40PM +0200, Arnd Bergmann wrote:
> llvm does does not understand -march=z9-109 and older target
> specifiers, so disable the respective Kconfig settings and
> the logic to make the boot code work on old systems when
> building with clang.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/s390/Kconfig | 6 ++++++
> arch/s390/boot/Makefile | 2 ++
> 2 files changed, 8 insertions(+)
...
> diff --git a/arch/s390/boot/Makefile b/arch/s390/boot/Makefile
> index c844eaf24ed7..953a74d04990 100644
> --- a/arch/s390/boot/Makefile
> +++ b/arch/s390/boot/Makefile
> @@ -11,6 +11,7 @@ KASAN_SANITIZE := n
> KBUILD_AFLAGS := $(KBUILD_AFLAGS_DECOMPRESSOR)
> KBUILD_CFLAGS := $(KBUILD_CFLAGS_DECOMPRESSOR)
>
> +ifndef CONFIG_CC_IS_CLANG
> #
> # Use -march=z900 for als.c to be able to print an error
> # message if the kernel is started on a machine which is too old
> @@ -25,6 +26,7 @@ CFLAGS_als.o += -march=z900
> CFLAGS_REMOVE_sclp_early_core.o += $(CC_FLAGS_MARCH)
> CFLAGS_sclp_early_core.o += -march=z900
> endif
> +endif
This contradicts the whole purpose of als.c - printing an error
message to the console if the kernel is compiled for a newer
architecture than it is running on (and therefore uses instructions
unknown to the current system).
If this can't be fixed/changed in clang, then it should be at least
changed to the lowest possible architecture.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] s390: only build for new CPUs with clang
2019-04-11 6:23 ` Heiko Carstens
@ 2019-04-11 10:19 ` Arnd Bergmann
0 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2019-04-11 10:19 UTC (permalink / raw)
To: Heiko Carstens
Cc: Martin Schwidefsky, Vasily Gorbik, clang-built-linux,
Nick Desaulniers, Nathan Chancellor, linux-s390, Masahiro Yamada,
Philipp Rudo, Tony Krowiak, Linux Kernel Mailing List
On Thu, Apr 11, 2019 at 8:24 AM Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
>
> On Wed, Apr 10, 2019 at 10:12:40PM +0200, Arnd Bergmann wrote:
> > llvm does does not understand -march=z9-109 and older target
> > specifiers, so disable the respective Kconfig settings and
> > the logic to make the boot code work on old systems when
> > building with clang.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > arch/s390/Kconfig | 6 ++++++
> > arch/s390/boot/Makefile | 2 ++
> > 2 files changed, 8 insertions(+)
> ...
> > diff --git a/arch/s390/boot/Makefile b/arch/s390/boot/Makefile
> > index c844eaf24ed7..953a74d04990 100644
> > --- a/arch/s390/boot/Makefile
> > +++ b/arch/s390/boot/Makefile
> > @@ -11,6 +11,7 @@ KASAN_SANITIZE := n
> > KBUILD_AFLAGS := $(KBUILD_AFLAGS_DECOMPRESSOR)
> > KBUILD_CFLAGS := $(KBUILD_CFLAGS_DECOMPRESSOR)
> >
> > +ifndef CONFIG_CC_IS_CLANG
> > #
> > # Use -march=z900 for als.c to be able to print an error
> > # message if the kernel is started on a machine which is too old
> > @@ -25,6 +26,7 @@ CFLAGS_als.o += -march=z900
> > CFLAGS_REMOVE_sclp_early_core.o += $(CC_FLAGS_MARCH)
> > CFLAGS_sclp_early_core.o += -march=z900
> > endif
> > +endif
>
> This contradicts the whole purpose of als.c - printing an error
> message to the console if the kernel is compiled for a newer
> architecture than it is running on (and therefore uses instructions
> unknown to the current system).
> If this can't be fixed/changed in clang, then it should be at least
> changed to the lowest possible architecture.
Ok, I'll do that. I originally did something like it, but then went back
to the simpler workaround to just make it compile.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread