[PATCH 1/2] s390: only build for new CPUs with clang

[PATCH 1/2] s390: only build for new CPUs with clang

[Arnd Bergmann]

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/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

[PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed

[Arnd Bergmann]

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

Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed

[Nick Desaulniers]

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

Re: [PATCH 1/2] s390: only build for new CPUs with clang

[Nick Desaulniers]

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

Re: [PATCH 1/2] s390: only build for new CPUs with clang

[Heiko Carstens]

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.

Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed

[Arnd Bergmann]

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

Re: [PATCH 1/2] s390: only build for new CPUs with clang

[Arnd Bergmann]

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

Re: [PATCH 1/2] s390: only build for new CPUs with clang

[Arnd Bergmann]

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

Re: [PATCH 1/2] s390: only build for new CPUs with clang

[Nick Desaulniers]

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

Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed

[Nick Desaulniers]

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

Re: [PATCH 2/2] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed

[Martin Schwidefsky]

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.