linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/4] s390: only build for new CPUs with clang
@ 2019-04-15  8:35 Arnd Bergmann
  2019-04-15  8:35 ` [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Arnd Bergmann @ 2019-04-15  8:35 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Masahiro Yamada, Vasily Gorbik,
	Philipp Rudo, Tony Krowiak, Ursula Braun, Rob Herring,
	linux-kernel

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.

Part of the early boot code is normally compiled with -march=z900
for maximum compatibility. This also has to get changed with
clang to the oldest supported ISA, which is -march=z10 here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/Kconfig       |  6 ++++++
 arch/s390/boot/Makefile | 18 ++++++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)

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..c6a6b6dd3d52 100644
--- a/arch/s390/boot/Makefile
+++ b/arch/s390/boot/Makefile
@@ -12,18 +12,24 @@ KBUILD_AFLAGS := $(KBUILD_AFLAGS_DECOMPRESSOR)
 KBUILD_CFLAGS := $(KBUILD_CFLAGS_DECOMPRESSOR)
 
 #
-# Use -march=z900 for als.c to be able to print an error
+# Use minimum architecture for als.c to be able to print an error
 # message if the kernel is started on a machine which is too old
 #
-ifneq ($(CC_FLAGS_MARCH),-march=z900)
+ifndef CONFIG_CC_IS_CLANG
+CC_FLAGS_MARCH_MINIMUM := -march=z900
+else
+CC_FLAGS_MARCH_MINIMUM := -march=z10
+endif
+
+ifneq ($(CC_FLAGS_MARCH),$(CC_FLAGS_MARCH_MINIMUM))
 AFLAGS_REMOVE_head.o		+= $(CC_FLAGS_MARCH)
-AFLAGS_head.o			+= -march=z900
+AFLAGS_head.o			+= $(CC_FLAGS_MARCH_MINIMUM)
 AFLAGS_REMOVE_mem.o		+= $(CC_FLAGS_MARCH)
-AFLAGS_mem.o			+= -march=z900
+AFLAGS_mem.o			+= $(CC_FLAGS_MARCH_MINIMUM)
 CFLAGS_REMOVE_als.o		+= $(CC_FLAGS_MARCH)
-CFLAGS_als.o			+= -march=z900
+CFLAGS_als.o			+= $(CC_FLAGS_MARCH_MINIMUM)
 CFLAGS_REMOVE_sclp_early_core.o	+= $(CC_FLAGS_MARCH)
-CFLAGS_sclp_early_core.o	+= -march=z900
+CFLAGS_sclp_early_core.o	+= $(CC_FLAGS_MARCH_MINIMUM)
 endif
 
 CFLAGS_sclp_early_core.o += -I$(srctree)/drivers/s390/char
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
  2019-04-15  8:35 [PATCH v2 1/4] s390: only build for new CPUs with clang Arnd Bergmann
@ 2019-04-15  8:35 ` Arnd Bergmann
  2019-04-15 16:12   ` Nathan Chancellor
  2019-04-15  8:35 ` [PATCH v2 3/4] s390: drop CONFIG_VIRT_TO_BUS Arnd Bergmann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2019-04-15  8:35 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Vasily Gorbik, Masahiro Yamada,
	Luc Van Oostenryck, Philipp Rudo, 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           | 4 ++--
 arch/s390/purgatory/Makefile | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index 9c079a506325..9a228786e34f 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -17,9 +17,9 @@ 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
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] 12+ messages in thread

* [PATCH v2 3/4] s390: drop CONFIG_VIRT_TO_BUS
  2019-04-15  8:35 [PATCH v2 1/4] s390: only build for new CPUs with clang Arnd Bergmann
  2019-04-15  8:35 ` [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
@ 2019-04-15  8:35 ` Arnd Bergmann
  2019-05-03 14:26   ` Heiko Carstens
  2019-04-15  8:35 ` [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code Arnd Bergmann
  2019-05-03 14:25 ` [PATCH v2 1/4] s390: only build for new CPUs with clang Heiko Carstens
  3 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2019-04-15  8:35 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Masahiro Yamada, Vasily Gorbik,
	Philipp Rudo, Tony Krowiak, Ursula Braun, linux-kernel

VIRT_TO_BUS is only used for legacy device PCI and ISA drivers using
virt_to_bus() instead of the streaming DMA mapping API, and the
remaining drivers generally don't work on 64-bit architectures.

Two of these drivers also cause a build warning on s390, so instead
of trying to fix that, let's just disable the option as we do on
most architectures now.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 1a2eec61196d..25e0e7375166 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -188,7 +188,6 @@ config S390
 	select TTY
 	select VIRT_CPU_ACCOUNTING
 	select ARCH_HAS_SCALED_CPUTIME
-	select VIRT_TO_BUS
 	select HAVE_NMI
 
 
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code
  2019-04-15  8:35 [PATCH v2 1/4] s390: only build for new CPUs with clang Arnd Bergmann
  2019-04-15  8:35 ` [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
  2019-04-15  8:35 ` [PATCH v2 3/4] s390: drop CONFIG_VIRT_TO_BUS Arnd Bergmann
@ 2019-04-15  8:35 ` Arnd Bergmann
  2019-04-22 17:51   ` Nick Desaulniers
  2019-05-03 14:27   ` Heiko Carstens
  2019-05-03 14:25 ` [PATCH v2 1/4] s390: only build for new CPUs with clang Heiko Carstens
  3 siblings, 2 replies; 12+ messages in thread
From: Arnd Bergmann @ 2019-04-15  8:35 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Vasily Gorbik, Christian Borntraeger,
	Janosch Frank, Collin Walling, linux-kernel

The arch/s390/boot directory is built with its own set of compiler
options that does not include -Wno-pointer-sign like the rest of
the kernel does, this causes a lot of harmess but correct warnings
when building with clang.

For the atomics, we can add type casts to avoid the warnings, for
everything else the easiest way is to slightly adapt the types
to be more consistent.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/boot/ipl_parm.c       |  2 +-
 arch/s390/include/asm/bitops.h  | 12 ++++++------
 arch/s390/include/asm/ebcdic.h  |  2 +-
 arch/s390/include/asm/lowcore.h |  2 +-
 drivers/s390/char/sclp.h        |  4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/s390/boot/ipl_parm.c b/arch/s390/boot/ipl_parm.c
index 36beb56de021..3f2f0a9f1dad 100644
--- a/arch/s390/boot/ipl_parm.c
+++ b/arch/s390/boot/ipl_parm.c
@@ -51,7 +51,7 @@ void store_ipl_parmblock(void)
 		early_ipl_block_valid = 1;
 }
 
-static size_t scpdata_length(const char *buf, size_t count)
+static size_t scpdata_length(const u8 *buf, size_t count)
 {
 	while (count) {
 		if (buf[count - 1] != '\0' && buf[count - 1] != ' ')
diff --git a/arch/s390/include/asm/bitops.h b/arch/s390/include/asm/bitops.h
index d1f8a4d94cca..9900d655014c 100644
--- a/arch/s390/include/asm/bitops.h
+++ b/arch/s390/include/asm/bitops.h
@@ -73,7 +73,7 @@ static inline void set_bit(unsigned long nr, volatile unsigned long *ptr)
 	}
 #endif
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	__atomic64_or(mask, addr);
+	__atomic64_or(mask, (long *)addr);
 }
 
 static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)
@@ -94,7 +94,7 @@ static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)
 	}
 #endif
 	mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-	__atomic64_and(mask, addr);
+	__atomic64_and(mask, (long *)addr);
 }
 
 static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)
@@ -115,7 +115,7 @@ static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)
 	}
 #endif
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	__atomic64_xor(mask, addr);
+	__atomic64_xor(mask, (long *)addr);
 }
 
 static inline int
@@ -125,7 +125,7 @@ test_and_set_bit(unsigned long nr, volatile unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	old = __atomic64_or_barrier(mask, addr);
+	old = __atomic64_or_barrier(mask, (long *)addr);
 	return (old & mask) != 0;
 }
 
@@ -136,7 +136,7 @@ test_and_clear_bit(unsigned long nr, volatile unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
-	old = __atomic64_and_barrier(mask, addr);
+	old = __atomic64_and_barrier(mask, (long *)addr);
 	return (old & ~mask) != 0;
 }
 
@@ -147,7 +147,7 @@ test_and_change_bit(unsigned long nr, volatile unsigned long *ptr)
 	unsigned long old, mask;
 
 	mask = 1UL << (nr & (BITS_PER_LONG - 1));
-	old = __atomic64_xor_barrier(mask, addr);
+	old = __atomic64_xor_barrier(mask, (long *)addr);
 	return (old & mask) != 0;
 }
 
diff --git a/arch/s390/include/asm/ebcdic.h b/arch/s390/include/asm/ebcdic.h
index 29441beb92e6..d3ce6adacb10 100644
--- a/arch/s390/include/asm/ebcdic.h
+++ b/arch/s390/include/asm/ebcdic.h
@@ -20,7 +20,7 @@ extern __u8 _ebc_tolower[256]; /* EBCDIC -> lowercase */
 extern __u8 _ebc_toupper[256]; /* EBCDIC -> uppercase */
 
 static inline void
-codepage_convert(const __u8 *codepage, volatile __u8 * addr, unsigned long nr)
+codepage_convert(const __u8 *codepage, volatile char* addr, unsigned long nr)
 {
 	if (nr-- <= 0)
 		return;
diff --git a/arch/s390/include/asm/lowcore.h b/arch/s390/include/asm/lowcore.h
index cc0947e08b6f..f3a637afd485 100644
--- a/arch/s390/include/asm/lowcore.h
+++ b/arch/s390/include/asm/lowcore.h
@@ -128,7 +128,7 @@ struct lowcore {
 	/* SMP info area */
 	__u32	cpu_nr;				/* 0x0398 */
 	__u32	softirq_pending;		/* 0x039c */
-	__u32	preempt_count;			/* 0x03a0 */
+	__s32	preempt_count;			/* 0x03a0 */
 	__u32	spinlock_lockval;		/* 0x03a4 */
 	__u32	spinlock_index;			/* 0x03a8 */
 	__u32	fpu_flags;			/* 0x03ac */
diff --git a/drivers/s390/char/sclp.h b/drivers/s390/char/sclp.h
index 367e9d384d85..5918479ffa0e 100644
--- a/drivers/s390/char/sclp.h
+++ b/drivers/s390/char/sclp.h
@@ -365,14 +365,14 @@ sclp_ascebc(unsigned char ch)
 
 /* translate string from EBCDIC to ASCII */
 static inline void
-sclp_ebcasc_str(unsigned char *str, int nr)
+sclp_ebcasc_str(char *str, int nr)
 {
 	(MACHINE_IS_VM) ? EBCASC(str, nr) : EBCASC_500(str, nr);
 }
 
 /* translate string from ASCII to EBCDIC */
 static inline void
-sclp_ascebc_str(unsigned char *str, int nr)
+sclp_ascebc_str(char *str, int nr)
 {
 	(MACHINE_IS_VM) ? ASCEBC(str, nr) : ASCEBC_500(str, nr);
 }
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
  2019-04-15  8:35 ` [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
@ 2019-04-15 16:12   ` Nathan Chancellor
  2019-05-03 14:26     ` Heiko Carstens
  0 siblings, 1 reply; 12+ messages in thread
From: Nathan Chancellor @ 2019-04-15 16:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, linux-s390, Vasily Gorbik, Masahiro Yamada,
	Luc Van Oostenryck, Philipp Rudo, linux-kernel

On Mon, Apr 15, 2019 at 10:35:52AM +0200, Arnd Bergmann 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>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  arch/s390/Makefile           | 4 ++--
>  arch/s390/purgatory/Makefile | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> index 9c079a506325..9a228786e34f 100644
> --- a/arch/s390/Makefile
> +++ b/arch/s390/Makefile
> @@ -17,9 +17,9 @@ 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
> 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	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code
  2019-04-15  8:35 ` [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code Arnd Bergmann
@ 2019-04-22 17:51   ` Nick Desaulniers
  2019-04-23  8:05     ` Arnd Bergmann
  2019-05-03 14:27   ` Heiko Carstens
  1 sibling, 1 reply; 12+ messages in thread
From: Nick Desaulniers @ 2019-04-22 17:51 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nathan Chancellor, linux-s390, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Collin Walling, LKML

On Mon, Apr 15, 2019 at 1:37 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The arch/s390/boot directory is built with its own set of compiler
> options that does not include -Wno-pointer-sign like the rest of
> the kernel does, this causes a lot of harmess but correct warnings

s/harmess/harmless/

> when building with clang.
>
> For the atomics, we can add type casts to avoid the warnings, for
> everything else the easiest way is to slightly adapt the types
> to be more consistent.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/boot/ipl_parm.c       |  2 +-
>  arch/s390/include/asm/bitops.h  | 12 ++++++------
>  arch/s390/include/asm/ebcdic.h  |  2 +-
>  arch/s390/include/asm/lowcore.h |  2 +-
>  drivers/s390/char/sclp.h        |  4 ++--
>  5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/s390/boot/ipl_parm.c b/arch/s390/boot/ipl_parm.c
> index 36beb56de021..3f2f0a9f1dad 100644
> --- a/arch/s390/boot/ipl_parm.c
> +++ b/arch/s390/boot/ipl_parm.c
> @@ -51,7 +51,7 @@ void store_ipl_parmblock(void)
>                 early_ipl_block_valid = 1;
>  }
>
> -static size_t scpdata_length(const char *buf, size_t count)
> +static size_t scpdata_length(const u8 *buf, size_t count)
>  {
>         while (count) {
>                 if (buf[count - 1] != '\0' && buf[count - 1] != ' ')
> diff --git a/arch/s390/include/asm/bitops.h b/arch/s390/include/asm/bitops.h
> index d1f8a4d94cca..9900d655014c 100644
> --- a/arch/s390/include/asm/bitops.h
> +++ b/arch/s390/include/asm/bitops.h
> @@ -73,7 +73,7 @@ static inline void set_bit(unsigned long nr, volatile unsigned long *ptr)
>         }
>  #endif
>         mask = 1UL << (nr & (BITS_PER_LONG - 1));
> -       __atomic64_or(mask, addr);
> +       __atomic64_or(mask, (long *)addr);
>  }
>
>  static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)
> @@ -94,7 +94,7 @@ static inline void clear_bit(unsigned long nr, volatile unsigned long *ptr)
>         }
>  #endif
>         mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
> -       __atomic64_and(mask, addr);
> +       __atomic64_and(mask, (long *)addr);
>  }
>
>  static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)
> @@ -115,7 +115,7 @@ static inline void change_bit(unsigned long nr, volatile unsigned long *ptr)
>         }
>  #endif
>         mask = 1UL << (nr & (BITS_PER_LONG - 1));
> -       __atomic64_xor(mask, addr);
> +       __atomic64_xor(mask, (long *)addr);
>  }
>
>  static inline int
> @@ -125,7 +125,7 @@ test_and_set_bit(unsigned long nr, volatile unsigned long *ptr)
>         unsigned long old, mask;
>
>         mask = 1UL << (nr & (BITS_PER_LONG - 1));
> -       old = __atomic64_or_barrier(mask, addr);
> +       old = __atomic64_or_barrier(mask, (long *)addr);
>         return (old & mask) != 0;
>  }
>
> @@ -136,7 +136,7 @@ test_and_clear_bit(unsigned long nr, volatile unsigned long *ptr)
>         unsigned long old, mask;
>
>         mask = ~(1UL << (nr & (BITS_PER_LONG - 1)));
> -       old = __atomic64_and_barrier(mask, addr);
> +       old = __atomic64_and_barrier(mask, (long *)addr);
>         return (old & ~mask) != 0;
>  }
>
> @@ -147,7 +147,7 @@ test_and_change_bit(unsigned long nr, volatile unsigned long *ptr)
>         unsigned long old, mask;
>
>         mask = 1UL << (nr & (BITS_PER_LONG - 1));
> -       old = __atomic64_xor_barrier(mask, addr);
> +       old = __atomic64_xor_barrier(mask, (long *)addr);

Thanks for the follow up.  This hunk and above all LGTM.

>         return (old & mask) != 0;
>  }
>
> diff --git a/arch/s390/include/asm/ebcdic.h b/arch/s390/include/asm/ebcdic.h
> index 29441beb92e6..d3ce6adacb10 100644
> --- a/arch/s390/include/asm/ebcdic.h
> +++ b/arch/s390/include/asm/ebcdic.h
> @@ -20,7 +20,7 @@ extern __u8 _ebc_tolower[256]; /* EBCDIC -> lowercase */
>  extern __u8 _ebc_toupper[256]; /* EBCDIC -> uppercase */
>
>  static inline void
> -codepage_convert(const __u8 *codepage, volatile __u8 * addr, unsigned long nr)
> +codepage_convert(const __u8 *codepage, volatile char* addr, unsigned long nr)
>  {
>         if (nr-- <= 0)
>                 return;

There are many call sites of ASCEBC which is defined in terms of this
function. Do they all use `char*`?  grep shows an explicit cast to
`unsigned char*` in drivers/s390/char/tape_std.c for example.

> diff --git a/arch/s390/include/asm/lowcore.h b/arch/s390/include/asm/lowcore.h
> index cc0947e08b6f..f3a637afd485 100644
> --- a/arch/s390/include/asm/lowcore.h
> +++ b/arch/s390/include/asm/lowcore.h
> @@ -128,7 +128,7 @@ struct lowcore {
>         /* SMP info area */
>         __u32   cpu_nr;                         /* 0x0398 */
>         __u32   softirq_pending;                /* 0x039c */
> -       __u32   preempt_count;                  /* 0x03a0 */
> +       __s32   preempt_count;                  /* 0x03a0 */

This change is less obvious. Do you still have the warning for that
this hunk fixes?

>         __u32   spinlock_lockval;               /* 0x03a4 */
>         __u32   spinlock_index;                 /* 0x03a8 */
>         __u32   fpu_flags;                      /* 0x03ac */
> diff --git a/drivers/s390/char/sclp.h b/drivers/s390/char/sclp.h
> index 367e9d384d85..5918479ffa0e 100644
> --- a/drivers/s390/char/sclp.h
> +++ b/drivers/s390/char/sclp.h
> @@ -365,14 +365,14 @@ sclp_ascebc(unsigned char ch)
>
>  /* translate string from EBCDIC to ASCII */
>  static inline void
> -sclp_ebcasc_str(unsigned char *str, int nr)
> +sclp_ebcasc_str(char *str, int nr)
>  {
>         (MACHINE_IS_VM) ? EBCASC(str, nr) : EBCASC_500(str, nr);
>  }
>
>  /* translate string from ASCII to EBCDIC */
>  static inline void
> -sclp_ascebc_str(unsigned char *str, int nr)
> +sclp_ascebc_str(char *str, int nr)
>  {
>         (MACHINE_IS_VM) ? ASCEBC(str, nr) : ASCEBC_500(str, nr);
>  }
> --
> 2.20.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code
  2019-04-22 17:51   ` Nick Desaulniers
@ 2019-04-23  8:05     ` Arnd Bergmann
  2019-04-23 18:21       ` Nick Desaulniers
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2019-04-23  8:05 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nathan Chancellor, linux-s390, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Collin Walling, LKML

On Mon, Apr 22, 2019 at 7:52 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:

> > @@ -20,7 +20,7 @@ extern __u8 _ebc_tolower[256]; /* EBCDIC -> lowercase */
> >  extern __u8 _ebc_toupper[256]; /* EBCDIC -> uppercase */
> >
> >  static inline void
> > -codepage_convert(const __u8 *codepage, volatile __u8 * addr, unsigned long nr)
> > +codepage_convert(const __u8 *codepage, volatile char* addr, unsigned long nr)
> >  {
> >         if (nr-- <= 0)
> >                 return;
>
> There are many call sites of ASCEBC which is defined in terms of this
> function. Do they all use `char*`?  grep shows an explicit cast to
> `unsigned char*` in drivers/s390/char/tape_std.c for example.

Generally speaking, the kernel is full of Wpointer-sign warnings, that's why
this warning is disabled in the top-level Makefile by default. My patch fixes
the ones in the s390 boot code that is not built with those default flags, but
I made no attempt to fix the rest of the kernel.

Fun fact: on most architectures, 'char' is signed, but on s390 and 32-bit
arm it is unsigned. The compiler treats 'char', 'unsigned char' and
'signed char'
as three distinct types here for that reason.

> > diff --git a/arch/s390/include/asm/lowcore.h b/arch/s390/include/asm/lowcore.h
> > index cc0947e08b6f..f3a637afd485 100644
> > --- a/arch/s390/include/asm/lowcore.h
> > +++ b/arch/s390/include/asm/lowcore.h
> > @@ -128,7 +128,7 @@ struct lowcore {
> >         /* SMP info area */
> >         __u32   cpu_nr;                         /* 0x0398 */
> >         __u32   softirq_pending;                /* 0x039c */
> > -       __u32   preempt_count;                  /* 0x03a0 */
> > +       __s32   preempt_count;                  /* 0x03a0 */
>
> This change is less obvious. Do you still have the warning for that
> this hunk fixes?

I can't find it now, but there are multiple users that would cause
this, like:

static inline void preempt_count_set(int pc)
{
        int old, new;

        do {
                old = READ_ONCE(S390_lowcore.preempt_count);
                new = (old & PREEMPT_NEED_RESCHED) |
                        (pc & ~PREEMPT_NEED_RESCHED);
        } while (__atomic_cmpxchg(&S390_lowcore.preempt_count,
                                  old, new) != old);
}


      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code
  2019-04-23  8:05     ` Arnd Bergmann
@ 2019-04-23 18:21       ` Nick Desaulniers
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-04-23 18:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nathan Chancellor, linux-s390, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Collin Walling, LKML

On Tue, Apr 23, 2019 at 1:06 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Apr 22, 2019 at 7:52 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
>
> > > @@ -20,7 +20,7 @@ extern __u8 _ebc_tolower[256]; /* EBCDIC -> lowercase */
> > >  extern __u8 _ebc_toupper[256]; /* EBCDIC -> uppercase */
> > >
> > >  static inline void
> > > -codepage_convert(const __u8 *codepage, volatile __u8 * addr, unsigned long nr)
> > > +codepage_convert(const __u8 *codepage, volatile char* addr, unsigned long nr)
> > >  {
> > >         if (nr-- <= 0)
> > >                 return;
> >
> > There are many call sites of ASCEBC which is defined in terms of this
> > function. Do they all use `char*`?  grep shows an explicit cast to
> > `unsigned char*` in drivers/s390/char/tape_std.c for example.
>
> Generally speaking, the kernel is full of Wpointer-sign warnings, that's why
> this warning is disabled in the top-level Makefile by default. My patch fixes
> the ones in the s390 boot code that is not built with those default flags, but
> I made no attempt to fix the rest of the kernel.

Right, sorry, I forgot about that.  This patch looks good to me.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Fun fact: on most architectures, 'char' is signed, but on s390 and 32-bit
> arm it is unsigned. The compiler treats 'char', 'unsigned char' and
> 'signed char'
> as three distinct types here for that reason.

I think I recall reading about that in:
https://www.amazon.com/ARM-System-Developers-Guide-Architecture/dp/1558608745
(I'll try to dig it up and post what the explanation was).

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/4] s390: only build for new CPUs with clang
  2019-04-15  8:35 [PATCH v2 1/4] s390: only build for new CPUs with clang Arnd Bergmann
                   ` (2 preceding siblings ...)
  2019-04-15  8:35 ` [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code Arnd Bergmann
@ 2019-05-03 14:25 ` Heiko Carstens
  3 siblings, 0 replies; 12+ messages in thread
From: Heiko Carstens @ 2019-05-03 14:25 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Masahiro Yamada, Vasily Gorbik,
	Philipp Rudo, Tony Krowiak, Ursula Braun, Rob Herring,
	linux-kernel

On Mon, Apr 15, 2019 at 10:35:51AM +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.
> 
> Part of the early boot code is normally compiled with -march=z900
> for maximum compatibility. This also has to get changed with
> clang to the oldest supported ISA, which is -march=z10 here.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/Kconfig       |  6 ++++++
>  arch/s390/boot/Makefile | 18 ++++++++++++------
>  2 files changed, 18 insertions(+), 6 deletions(-)

Applied, thanks.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed
  2019-04-15 16:12   ` Nathan Chancellor
@ 2019-05-03 14:26     ` Heiko Carstens
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Carstens @ 2019-05-03 14:26 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Arnd Bergmann, Martin Schwidefsky, clang-built-linux,
	Nick Desaulniers, linux-s390, Vasily Gorbik, Masahiro Yamada,
	Luc Van Oostenryck, Philipp Rudo, linux-kernel

On Mon, Apr 15, 2019 at 09:12:06AM -0700, Nathan Chancellor wrote:
> On Mon, Apr 15, 2019 at 10:35:52AM +0200, Arnd Bergmann 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>
> 
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> > ---
> >  arch/s390/Makefile           | 4 ++--
> >  arch/s390/purgatory/Makefile | 1 +
> >  2 files changed, 3 insertions(+), 2 deletions(-)

Applied, thanks.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] s390: drop CONFIG_VIRT_TO_BUS
  2019-04-15  8:35 ` [PATCH v2 3/4] s390: drop CONFIG_VIRT_TO_BUS Arnd Bergmann
@ 2019-05-03 14:26   ` Heiko Carstens
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Carstens @ 2019-05-03 14:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Masahiro Yamada, Vasily Gorbik,
	Philipp Rudo, Tony Krowiak, Ursula Braun, linux-kernel

On Mon, Apr 15, 2019 at 10:35:53AM +0200, Arnd Bergmann wrote:
> VIRT_TO_BUS is only used for legacy device PCI and ISA drivers using
> virt_to_bus() instead of the streaming DMA mapping API, and the
> remaining drivers generally don't work on 64-bit architectures.
> 
> Two of these drivers also cause a build warning on s390, so instead
> of trying to fix that, let's just disable the option as we do on
> most architectures now.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/Kconfig | 1 -
>  1 file changed, 1 deletion(-)

Applied, thanks.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code
  2019-04-15  8:35 ` [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code Arnd Bergmann
  2019-04-22 17:51   ` Nick Desaulniers
@ 2019-05-03 14:27   ` Heiko Carstens
  1 sibling, 0 replies; 12+ messages in thread
From: Heiko Carstens @ 2019-05-03 14:27 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Collin Walling,
	linux-kernel

On Mon, Apr 15, 2019 at 10:35:54AM +0200, Arnd Bergmann wrote:
> The arch/s390/boot directory is built with its own set of compiler
> options that does not include -Wno-pointer-sign like the rest of
> the kernel does, this causes a lot of harmess but correct warnings
> when building with clang.
> 
> For the atomics, we can add type casts to avoid the warnings, for
> everything else the easiest way is to slightly adapt the types
> to be more consistent.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/boot/ipl_parm.c       |  2 +-
>  arch/s390/include/asm/bitops.h  | 12 ++++++------
>  arch/s390/include/asm/ebcdic.h  |  2 +-
>  arch/s390/include/asm/lowcore.h |  2 +-
>  drivers/s390/char/sclp.h        |  4 ++--
>  5 files changed, 11 insertions(+), 11 deletions(-)

Applied, with typo fix and a simple coding style issue.
Thanks!


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-05-03 15:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15  8:35 [PATCH v2 1/4] s390: only build for new CPUs with clang Arnd Bergmann
2019-04-15  8:35 ` [PATCH v2 2/4] s390: boot, purgatory: pass $(CLANG_FLAGS) where needed Arnd Bergmann
2019-04-15 16:12   ` Nathan Chancellor
2019-05-03 14:26     ` Heiko Carstens
2019-04-15  8:35 ` [PATCH v2 3/4] s390: drop CONFIG_VIRT_TO_BUS Arnd Bergmann
2019-05-03 14:26   ` Heiko Carstens
2019-04-15  8:35 ` [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code Arnd Bergmann
2019-04-22 17:51   ` Nick Desaulniers
2019-04-23  8:05     ` Arnd Bergmann
2019-04-23 18:21       ` Nick Desaulniers
2019-05-03 14:27   ` Heiko Carstens
2019-05-03 14:25 ` [PATCH v2 1/4] s390: only build for new CPUs with clang Heiko Carstens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).