linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/12] s390: remove -fno-strength-reduce flag
@ 2019-04-08 21:26 Arnd Bergmann
  2019-04-08 21:26 ` [PATCH 02/12] s390: don't build vdso32 with clang Arnd Bergmann
                   ` (11 more replies)
  0 siblings, 12 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 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, linux-kernel

This was added as a workaround for really old compilers, and it prevents
building with clang now. I can see no reason for keeping it, as it has
already been removed for most architectures in the pre-git era, so
let's remove it everywhere, rather than only for clang.

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

diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index 86c76b149cc2..0087d11e3eaf 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -114,7 +114,7 @@ endif
 cfi := $(call as-instr,.cfi_startproc\n.cfi_val_offset 15$(comma)-160\n.cfi_endproc,-DCONFIG_AS_CFI_VAL_OFFSET=1)
 
 KBUILD_CFLAGS	+= -mbackchain -msoft-float $(cflags-y)
-KBUILD_CFLAGS	+= -pipe -fno-strength-reduce -Wno-sign-compare
+KBUILD_CFLAGS	+= -pipe -Wno-sign-compare
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables $(cfi)
 KBUILD_AFLAGS	+= $(aflags-y) $(cfi)
 export KBUILD_AFLAGS_DECOMPRESSOR
-- 
2.20.0


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

* [PATCH 02/12] s390: don't build vdso32 with clang
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-10 15:57   ` Martin Schwidefsky
  2019-04-10 16:26   ` Nick Desaulniers
  2019-04-08 21:26 ` [PATCH 03/12] s390: purgatory: pass --target option to clang Arnd Bergmann
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Masahiro Yamada, Vasily Gorbik,
	Ursula Braun, Mike Rapoport, linux-kernel

clang does not support 31 bit object files on s390, so skip
the 32-bit vdso here, and only build it when using gcc to compile
the kernel.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/Kconfig         |  3 +++
 arch/s390/kernel/Makefile |  2 +-
 arch/s390/kernel/vdso.c   | 10 +++++-----
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b6e3d0653002..8cd860cba4d1 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -388,6 +388,9 @@ config COMPAT
 	  (and some other stuff like libraries and such) is needed for
 	  executing 31 bit applications.  It is safe to say "Y".
 
+config COMPAT_VDSO
+	def_bool COMPAT && !CC_IS_CLANG
+
 config SYSVIPC_COMPAT
 	def_bool y if COMPAT && SYSVIPC
 
diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
index 8a62c7f72e1b..1222db6d4ee9 100644
--- a/arch/s390/kernel/Makefile
+++ b/arch/s390/kernel/Makefile
@@ -86,7 +86,7 @@ obj-$(CONFIG_TRACEPOINTS)	+= trace.o
 
 # vdso
 obj-y				+= vdso64/
-obj-$(CONFIG_COMPAT)		+= vdso32/
+obj-$(CONFIG_COMPAT_VDSO)	+= vdso32/
 
 chkbss := head64.o early_nobss.o
 include $(srctree)/arch/s390/scripts/Makefile.chkbss
diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
index e7920a68a12e..243d8b1185bf 100644
--- a/arch/s390/kernel/vdso.c
+++ b/arch/s390/kernel/vdso.c
@@ -29,7 +29,7 @@
 #include <asm/vdso.h>
 #include <asm/facility.h>
 
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_VDSO
 extern char vdso32_start, vdso32_end;
 static void *vdso32_kbase = &vdso32_start;
 static unsigned int vdso32_pages;
@@ -55,7 +55,7 @@ static vm_fault_t vdso_fault(const struct vm_special_mapping *sm,
 
 	vdso_pagelist = vdso64_pagelist;
 	vdso_pages = vdso64_pages;
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_VDSO
 	if (vma->vm_mm->context.compat_mm) {
 		vdso_pagelist = vdso32_pagelist;
 		vdso_pages = vdso32_pages;
@@ -76,7 +76,7 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
 	unsigned long vdso_pages;
 
 	vdso_pages = vdso64_pages;
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_VDSO
 	if (vma->vm_mm->context.compat_mm)
 		vdso_pages = vdso32_pages;
 #endif
@@ -223,7 +223,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 		return 0;
 
 	vdso_pages = vdso64_pages;
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_VDSO
 	mm->context.compat_mm = is_compat_task();
 	if (mm->context.compat_mm)
 		vdso_pages = vdso32_pages;
@@ -280,7 +280,7 @@ static int __init vdso_init(void)
 	int i;
 
 	vdso_init_data(vdso_data);
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_VDSO
 	/* Calculate the size of the 32 bit vDSO */
 	vdso32_pages = ((&vdso32_end - &vdso32_start
 			 + PAGE_SIZE - 1) >> PAGE_SHIFT) + 1;
-- 
2.20.0


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

* [PATCH 03/12] s390: purgatory: pass --target option to clang
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
  2019-04-08 21:26 ` [PATCH 02/12] s390: don't build vdso32 with clang Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-08 22:03   ` Nathan Chancellor
  2019-04-08 21:26 ` [PATCH 04/12] s390: qeth: address type mismatch warning Arnd Bergmann
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Philipp Rudo, Hendrik Brueckner,
	linux-kernel

The purgatory Makefile does 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.

Pass --target=s390x-linux to all compilers that understand
this option.

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

diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
index ce6a3f75065b..3a14b968cec3 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 += $(call cc-option,--target=s390x-linux)
 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] 43+ messages in thread

* [PATCH 04/12] s390: qeth: address type mismatch warning
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
  2019-04-08 21:26 ` [PATCH 02/12] s390: don't build vdso32 with clang Arnd Bergmann
  2019-04-08 21:26 ` [PATCH 03/12] s390: purgatory: pass --target option to clang Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-08 22:16   ` Nathan Chancellor
  2019-04-10 15:58   ` Martin Schwidefsky
  2019-04-08 21:26 ` [PATCH 05/12] s390: zcrypt: initialize variables before_use Arnd Bergmann
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Julian Wiedmann, Ursula Braun
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, David S. Miller, Kittipon Meesompop,
	linux-kernel

clang produces a harmless warning for each use for the qeth_adp_supported
macro:

drivers/s390/net/qeth_l2_main.c:559:31: warning: implicit conversion from enumeration type 'enum qeth_ipa_setadp_cmd' to
      different enumeration type 'enum qeth_ipa_funcs' [-Wenum-conversion]
        if (qeth_adp_supported(card, IPA_SETADP_SET_PROMISC_MODE))
            ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/net/qeth_core.h:179:41: note: expanded from macro 'qeth_adp_supported'
        qeth_is_ipa_supported(&c->options.adp, f)
        ~~~~~~~~~~~~~~~~~~~~~                  ^

Add a version of this macro that uses the correct types, and
remove the unused qeth_adp_enabled() macro that has the same
problem.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/s390/net/qeth_core.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index c851cf6e01c4..d603dfea97ab 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -163,6 +163,12 @@ struct qeth_vnicc_info {
 	bool rx_bcast_enabled;
 };
 
+static inline int qeth_is_adp_supported(struct qeth_ipa_info *ipa,
+		enum qeth_ipa_setadp_cmd func)
+{
+	return (ipa->supported_funcs & func);
+}
+
 static inline int qeth_is_ipa_supported(struct qeth_ipa_info *ipa,
 		enum qeth_ipa_funcs func)
 {
@@ -176,9 +182,7 @@ static inline int qeth_is_ipa_enabled(struct qeth_ipa_info *ipa,
 }
 
 #define qeth_adp_supported(c, f) \
-	qeth_is_ipa_supported(&c->options.adp, f)
-#define qeth_adp_enabled(c, f) \
-	qeth_is_ipa_enabled(&c->options.adp, f)
+	qeth_is_adp_supported(&c->options.adp, f)
 #define qeth_is_supported(c, f) \
 	qeth_is_ipa_supported(&c->options.ipa4, f)
 #define qeth_is_enabled(c, f) \
-- 
2.20.0


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

* [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (2 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 04/12] s390: qeth: address type mismatch warning Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-08 22:31   ` Nathan Chancellor
  2019-04-09  9:54   ` Harald Freudenberger
  2019-04-08 21:26 ` [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code Arnd Bergmann
                   ` (7 subsequent siblings)
  11 siblings, 2 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Harald Freudenberger
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Ingo Franzki, linux-kernel

The 'func_code' variable gets printed in debug statements without
a prior initialization in multiple functions, as reported when building
with clang:

drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true
      [-Wsometimes-uninitialized]
        if (mex->outputdatalength < mex->inputdatalength) {
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here
        trace_s390_zcrypt_rep(mex, func_code, rc,
                                   ^~~~~~~~~
drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false
        if (mex->outputdatalength < mex->inputdatalength) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning
        unsigned int func_code;
                              ^

Add initializations to all affected code paths to shut up the warning
and make the warning output consistent.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/s390/crypto/zcrypt_api.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
index eb93c2d27d0a..23472063d9a8 100644
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,
 	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
 
 	if (mex->outputdatalength < mex->inputdatalength) {
+		func_code = -1;
 		rc = -EINVAL;
 		goto out;
 	}
@@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,
 	trace_s390_zcrypt_req(crt, TP_ICARSACRT);
 
 	if (crt->outputdatalength < crt->inputdatalength) {
+		func_code = -1;
 		rc = -EINVAL;
 		goto out;
 	}
@@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
 
 		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);
 		if (!targets) {
+			func_code = -1;
 			rc = -ENOMEM;
 			goto out;
 		}
@@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
 		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;
 		if (copy_from_user(targets, uptr,
 				   target_num * sizeof(*targets))) {
+			func_code = -1;
 			rc = -EFAULT;
 			goto out_free;
 		}
-- 
2.20.0


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

* [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (3 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 05/12] s390: zcrypt: initialize variables before_use Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-08 22:33   ` Nathan Chancellor
  2019-04-10 16:00   ` Martin Schwidefsky
  2019-04-08 21:26 ` [PATCH 07/12] s390: cio: fix cio_irb declaration Arnd Bergmann
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Julian Wiedmann, Ursula Braun
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, linux-kernel

clang points out that the return code from this function is
undefined for one of the error paths:

../drivers/s390/net/ctcm_main.c:1595:7: warning: variable 'result' is used uninitialized whenever 'if' condition is true
      [-Wsometimes-uninitialized]
                if (priv->channel[direction] == NULL) {
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/s390/net/ctcm_main.c:1638:9: note: uninitialized use occurs here
        return result;
               ^~~~~~
../drivers/s390/net/ctcm_main.c:1595:3: note: remove the 'if' if its condition is always false
                if (priv->channel[direction] == NULL) {
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/s390/net/ctcm_main.c:1539:12: note: initialize the variable 'result' to silence this warning
        int result;
                  ^

Make it return -ENODEV here, as in the related failure cases.
gcc has a known bug in underreporting some of these warnings
when it has already eliminated the assignment of the return code
based on some earlier optimization step.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/s390/net/ctcm_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index 7617d21cb296..f63c5c871d3d 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -1595,6 +1595,7 @@ static int ctcm_new_device(struct ccwgroup_device *cgdev)
 		if (priv->channel[direction] == NULL) {
 			if (direction == CTCM_WRITE)
 				channel_free(priv->channel[CTCM_READ]);
+			result = -ENODEV;
 			goto out_dev;
 		}
 		priv->channel[direction]->netdev = dev;
-- 
2.20.0


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

* [PATCH 07/12] s390: cio: fix cio_irb declaration
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (4 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-08 22:35   ` Nathan Chancellor
  2019-04-09 13:13   ` Sebastian Ott
  2019-04-08 21:26 ` [PATCH 08/12] s390: syscall_wrapper: avoid clang warning Arnd Bergmann
                   ` (5 subsequent siblings)
  11 siblings, 2 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Sebastian Ott, Peter Oberparleiter
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, linux-kernel

clang points out that the declaration of cio_irb does not match the
definition exactly, it is missing the alignment attribute:

../drivers/s390/cio/cio.c:50:1: warning: section does not match previous declaration [-Wsection]
DEFINE_PER_CPU_ALIGNED(struct irb, cio_irb);
^
../include/linux/percpu-defs.h:150:2: note: expanded from macro 'DEFINE_PER_CPU_ALIGNED'
        DEFINE_PER_CPU_SECTION(type, name, PER_CPU_ALIGNED_SECTION)     \
        ^
../include/linux/percpu-defs.h:93:9: note: expanded from macro 'DEFINE_PER_CPU_SECTION'
        extern __PCPU_ATTRS(sec) __typeof__(type) name;                 \
               ^
../include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
        __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
                                ^
../drivers/s390/cio/cio.h:118:1: note: previous attribute is here
DECLARE_PER_CPU(struct irb, cio_irb);
^
../include/linux/percpu-defs.h:111:2: note: expanded from macro 'DECLARE_PER_CPU'
        DECLARE_PER_CPU_SECTION(type, name, "")
        ^
../include/linux/percpu-defs.h:87:9: note: expanded from macro 'DECLARE_PER_CPU_SECTION'
        extern __PCPU_ATTRS(sec) __typeof__(type) name
               ^
../include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
        __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
                                ^
Use DECLARE_PER_CPU_ALIGNED() here, to make the two match.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/s390/cio/cio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
index 9811fd8a0c73..92eabbb5f18d 100644
--- a/drivers/s390/cio/cio.h
+++ b/drivers/s390/cio/cio.h
@@ -115,7 +115,7 @@ struct subchannel {
 	struct schib_config config;
 } __attribute__ ((aligned(8)));
 
-DECLARE_PER_CPU(struct irb, cio_irb);
+DECLARE_PER_CPU_ALIGNED(struct irb, cio_irb);
 
 #define to_subchannel(n) container_of(n, struct subchannel, dev)
 
-- 
2.20.0


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

* [PATCH 08/12] s390: syscall_wrapper: avoid clang warning
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (5 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 07/12] s390: cio: fix cio_irb declaration Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-10 16:00   ` Martin Schwidefsky
  2019-04-08 21:26 ` [PATCH 09/12] s390: make __load_psw_mask work with clang Arnd Bergmann
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Arnd Bergmann
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, linux-kernel

Building system calls with clang results in a warning
about an alias from a global function to a static one:

../fs/namei.c:3847:1: warning: unused function '__se_sys_mkdirat' [-Wunused-function]
SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
^
../include/linux/syscalls.h:219:36: note: expanded from macro 'SYSCALL_DEFINE3'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                   ^
../include/linux/syscalls.h:228:2: note: expanded from macro 'SYSCALL_DEFINEx'
        __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
        ^
../arch/s390/include/asm/syscall_wrapper.h:126:18: note: expanded from macro '__SYSCALL_DEFINEx'
        asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))          \
                        ^
<scratch space>:31:1: note: expanded from here
__se_sys_mkdirat
^

The only reference to the static __se_sys_mkdirat() here is the alias, but
this only gets evaluated later. Making this function global as well avoids
the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I considered opening a bug report for llvm here, but I suspect the
behavior is intentional, so I'm just fixing it here. Let me know if
you disagree.
---
 arch/s390/include/asm/syscall_wrapper.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/include/asm/syscall_wrapper.h b/arch/s390/include/asm/syscall_wrapper.h
index 5596c5c625d2..3c3d6fe8e2f0 100644
--- a/arch/s390/include/asm/syscall_wrapper.h
+++ b/arch/s390/include/asm/syscall_wrapper.h
@@ -119,8 +119,8 @@
 		      "Type aliasing is used to sanitize syscall arguments");\
 	asmlinkage long __s390x_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))	\
 		__attribute__((alias(__stringify(__se_sys##name))));		\
-	ALLOW_ERROR_INJECTION(__s390x_sys##name, ERRNO);				\
-	static long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));		\
+	ALLOW_ERROR_INJECTION(__s390x_sys##name, ERRNO);			\
+	long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));			\
 	static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
 	__S390_SYS_STUBx(x, name, __VA_ARGS__)					\
 	asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))		\
-- 
2.20.0


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

* [PATCH 09/12] s390: make __load_psw_mask work with clang
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (6 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 08/12] s390: syscall_wrapper: avoid clang warning Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-10 16:01   ` Martin Schwidefsky
  2019-04-08 21:26 ` [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang Arnd Bergmann
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Vasily Gorbik, Steven Rostedt (VMware),
	linux-kernel

clang fails to use the %O and %R inline assembly modifiers
the same way as gcc, leading to build failures with every use
of __load_psw_mask():

/tmp/nmi-4a9f80.s: Assembler messages:
/tmp/nmi-4a9f80.s:571: Error: junk at end of line: `+8(160(%r11))'
/tmp/nmi-4a9f80.s:626: Error: junk at end of line: `+8(160(%r11))'

Replace these with a more conventional way of passing the addresses
that should work with both clang and gcc.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/include/asm/processor.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
index 81038ab357ce..700c650ffd4f 100644
--- a/arch/s390/include/asm/processor.h
+++ b/arch/s390/include/asm/processor.h
@@ -339,10 +339,10 @@ static __no_kasan_or_inline void __load_psw_mask(unsigned long mask)
 
 	asm volatile(
 		"	larl	%0,1f\n"
-		"	stg	%0,%O1+8(%R1)\n"
-		"	lpswe	%1\n"
+		"	stg	%0,%1\n"
+		"	lpswe	%2\n"
 		"1:"
-		: "=&d" (addr), "=Q" (psw) : "Q" (psw) : "memory", "cc");
+		: "=&d" (addr), "=Q" (psw.addr) : "Q" (psw) : "memory", "cc");
 }
 
 /*
-- 
2.20.0


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

* [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (7 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 09/12] s390: make __load_psw_mask work with clang Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-10 16:03   ` Martin Schwidefsky
  2019-04-08 21:26 ` [PATCH 11/12] s390: make chkbss work with clang Arnd Bergmann
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens, Steven Rostedt, Ingo Molnar
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Vasily Gorbik, linux-kernel

llvm on s390 has problems with __builtin_return_address(n), with n>0,
this results in a somewhat cryptic error message:

fatal error: error in backend: Unsupported stack frame traversal count

To work around it, use the direct return address directly. This
is probably not ideal here, but gets things to compile and should
only lead to inferior reporting, not to misbehavior of the generated
code.

Link: https://bugs.llvm.org/show_bug.cgi?id=41424
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/include/asm/ftrace.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 5a3c95b11952..7923c63946fb 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -13,7 +13,12 @@
 
 #ifndef __ASSEMBLY__
 
+#ifdef CONFIG_CC_IS_CLANG
+/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
+#define ftrace_return_address(n) __builtin_return_address(0)
+#else
 #define ftrace_return_address(n) __builtin_return_address(n)
+#endif
 
 void _mcount(void);
 void ftrace_caller(void);
-- 
2.20.0


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

* [PATCH 11/12] s390: make chkbss work with clang
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (8 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-10 16:02   ` Martin Schwidefsky
  2019-04-08 21:26 ` [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly Arnd Bergmann
  2019-04-10 15:57 ` [PATCH 01/12] s390: remove -fno-strength-reduce flag Martin Schwidefsky
  11 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Vasily Gorbik, linux-kernel

llvm skips an empty .bss section entirely, which makes
the check fail with an unexpected error:

/tmp/binutils-multi-test/bin/s390x-linux-gnu-objdump: section '.bss' mentioned in a -j option, but not found in any input file
error: arch/s390/boot/compressed/decompressor.o .bss section is not empty
../arch/s390/scripts/Makefile.chkbss:20: recipe for target 'arch/s390/boot/compressed/decompressor.o.chkbss' failed

Change the check so we first see if a .bss section exists
before trying to read its size.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/scripts/Makefile.chkbss | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/s390/scripts/Makefile.chkbss b/arch/s390/scripts/Makefile.chkbss
index cd7e8f4419f5..884a9caff5fb 100644
--- a/arch/s390/scripts/Makefile.chkbss
+++ b/arch/s390/scripts/Makefile.chkbss
@@ -11,7 +11,8 @@ chkbss: $(addprefix $(obj)/, $(chkbss-files))
 
 quiet_cmd_chkbss = CHKBSS  $<
       cmd_chkbss = \
-	if ! $(OBJDUMP) -j .bss -w -h $< | awk 'END { if ($$3) exit 1 }'; then \
+	if $(OBJDUMP) -h $< | grep -q "\.bss" && \
+	   ! $(OBJDUMP) -j .bss -w -h $< | awk 'END { if ($$3) exit 1 }'; then \
 		echo "error: $< .bss section is not empty" >&2; exit 1; \
 	fi; \
 	touch $@;
-- 
2.20.0


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

* [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (9 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 11/12] s390: make chkbss work with clang Arnd Bergmann
@ 2019-04-08 21:26 ` Arnd Bergmann
  2019-04-10 13:55   ` Martin Schwidefsky
  2019-04-10 15:57 ` [PATCH 01/12] s390: remove -fno-strength-reduce flag Martin Schwidefsky
  11 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-08 21:26 UTC (permalink / raw)
  To: Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Arnd Bergmann, Vasily Gorbik, linux-kernel

clang does not understand the contraint "0" in the CALL_ON_STACK()
macro:

../arch/s390/mm/maccess.c:117:10: error: invalid input constraint '0' in asm
                return CALL_ON_STACK(_memcpy_real, S390_lowcore.nodat_stack,
                       ^
../arch/s390/include/asm/processor.h:292:20: note: expanded from macro 'CALL_ON_STACK'
                  [_fn] "X" (fn) CALL_FMT_##nr : CALL_CLOBBER_##nr);    \
                                 ^
<scratch space>:207:1: note: expanded from here
CALL_FMT_3
^
../arch/s390/include/asm/processor.h:267:20: note: expanded from macro 'CALL_FMT_3'
 #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
                   ^
../arch/s390/include/asm/processor.h:266:20: note: expanded from macro 'CALL_FMT_2'
 #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
                   ^
../arch/s390/include/asm/processor.h:265:32: note: expanded from macro 'CALL_FMT_1'
 #define CALL_FMT_1 CALL_FMT_0, "0" (r2)
                               ^

I don't know what the correct fix here would be, changing it to "d" made
it build, since clang does understand this one.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
index 700c650ffd4f..84c59c99668a 100644
--- a/arch/s390/include/asm/processor.h
+++ b/arch/s390/include/asm/processor.h
@@ -262,7 +262,7 @@ static __no_kasan_or_inline unsigned short stap(void)
 	register unsigned long r4 asm("6") = (unsigned long)(arg5)
 
 #define CALL_FMT_0
-#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
+#define CALL_FMT_1 CALL_FMT_0, "d" (r2)
 #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
 #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
 #define CALL_FMT_4 CALL_FMT_3, "d" (r5)
-- 
2.20.0


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

* Re: [PATCH 03/12] s390: purgatory: pass --target option to clang
  2019-04-08 21:26 ` [PATCH 03/12] s390: purgatory: pass --target option to clang Arnd Bergmann
@ 2019-04-08 22:03   ` Nathan Chancellor
  2019-04-09  6:43     ` Arnd Bergmann
  0 siblings, 1 reply; 43+ messages in thread
From: Nathan Chancellor @ 2019-04-08 22:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, linux-s390, Philipp Rudo, Hendrik Brueckner,
	linux-kernel

On Mon, Apr 08, 2019 at 11:26:16PM +0200, Arnd Bergmann wrote:
> The purgatory Makefile does 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.
> 
> Pass --target=s390x-linux to all compilers that understand
> this option.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/purgatory/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
> index ce6a3f75065b..3a14b968cec3 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 += $(call cc-option,--target=s390x-linux)
>  KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
>  KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
>  
> -- 
> 2.20.0
> 

Would

ifdef CONFIG_CC_IS_CLANG
KBUILD_CFLAGS += --target=s390x-linux
endif

be a little clearer (and save a cc-option call)?

Otherwise, makes sense.

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

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

* Re: [PATCH 04/12] s390: qeth: address type mismatch warning
  2019-04-08 21:26 ` [PATCH 04/12] s390: qeth: address type mismatch warning Arnd Bergmann
@ 2019-04-08 22:16   ` Nathan Chancellor
  2019-04-09  6:44     ` Arnd Bergmann
  2019-04-10 15:58   ` Martin Schwidefsky
  1 sibling, 1 reply; 43+ messages in thread
From: Nathan Chancellor @ 2019-04-08 22:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, Julian Wiedmann,
	Ursula Braun, clang-built-linux, Nick Desaulniers, linux-s390,
	David S. Miller, Kittipon Meesompop, linux-kernel

On Mon, Apr 08, 2019 at 11:26:17PM +0200, Arnd Bergmann wrote:
> clang produces a harmless warning for each use for the qeth_adp_supported
> macro:
> 
> drivers/s390/net/qeth_l2_main.c:559:31: warning: implicit conversion from enumeration type 'enum qeth_ipa_setadp_cmd' to
>       different enumeration type 'enum qeth_ipa_funcs' [-Wenum-conversion]
>         if (qeth_adp_supported(card, IPA_SETADP_SET_PROMISC_MODE))
>             ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/net/qeth_core.h:179:41: note: expanded from macro 'qeth_adp_supported'
>         qeth_is_ipa_supported(&c->options.adp, f)
>         ~~~~~~~~~~~~~~~~~~~~~                  ^
> 
> Add a version of this macro that uses the correct types, and
> remove the unused qeth_adp_enabled() macro that has the same
> problem.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I wonder if it is better to just change the func parameter to type long.
I guess it's better to keep the type safety to make sure values aren't
unintentionally mixed but the body of the functions is the same so does
the type actually matter?

Regardless, this change does fix the warning so:

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

> ---
>  drivers/s390/net/qeth_core.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
> index c851cf6e01c4..d603dfea97ab 100644
> --- a/drivers/s390/net/qeth_core.h
> +++ b/drivers/s390/net/qeth_core.h
> @@ -163,6 +163,12 @@ struct qeth_vnicc_info {
>  	bool rx_bcast_enabled;
>  };
>  
> +static inline int qeth_is_adp_supported(struct qeth_ipa_info *ipa,
> +		enum qeth_ipa_setadp_cmd func)
> +{
> +	return (ipa->supported_funcs & func);
> +}
> +
>  static inline int qeth_is_ipa_supported(struct qeth_ipa_info *ipa,
>  		enum qeth_ipa_funcs func)
>  {
> @@ -176,9 +182,7 @@ static inline int qeth_is_ipa_enabled(struct qeth_ipa_info *ipa,
>  }
>  
>  #define qeth_adp_supported(c, f) \
> -	qeth_is_ipa_supported(&c->options.adp, f)
> -#define qeth_adp_enabled(c, f) \
> -	qeth_is_ipa_enabled(&c->options.adp, f)
> +	qeth_is_adp_supported(&c->options.adp, f)
>  #define qeth_is_supported(c, f) \
>  	qeth_is_ipa_supported(&c->options.ipa4, f)
>  #define qeth_is_enabled(c, f) \
> -- 
> 2.20.0
> 

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

* Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-08 21:26 ` [PATCH 05/12] s390: zcrypt: initialize variables before_use Arnd Bergmann
@ 2019-04-08 22:31   ` Nathan Chancellor
  2019-04-09  9:54   ` Harald Freudenberger
  1 sibling, 0 replies; 43+ messages in thread
From: Nathan Chancellor @ 2019-04-08 22:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, Harald Freudenberger,
	clang-built-linux, Nick Desaulniers, linux-s390, Ingo Franzki,
	linux-kernel

On Mon, Apr 08, 2019 at 11:26:18PM +0200, Arnd Bergmann wrote:
> The 'func_code' variable gets printed in debug statements without
> a prior initialization in multiple functions, as reported when building
> with clang:
> 
> drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true
>       [-Wsometimes-uninitialized]
>         if (mex->outputdatalength < mex->inputdatalength) {
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here
>         trace_s390_zcrypt_rep(mex, func_code, rc,
>                                    ^~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false
>         if (mex->outputdatalength < mex->inputdatalength) {
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning
>         unsigned int func_code;
>                               ^
> 
> Add initializations to all affected code paths to shut up the warning
> and make the warning output consistent.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I'll never get used to seeing negative numbers assigned to unsigned
integers...

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

> ---
>  drivers/s390/crypto/zcrypt_api.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index eb93c2d27d0a..23472063d9a8 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,
>  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
>  
>  	if (mex->outputdatalength < mex->inputdatalength) {
> +		func_code = -1;
>  		rc = -EINVAL;
>  		goto out;
>  	}
> @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,
>  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);
>  
>  	if (crt->outputdatalength < crt->inputdatalength) {
> +		func_code = -1;
>  		rc = -EINVAL;
>  		goto out;
>  	}
> @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
>  
>  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);
>  		if (!targets) {
> +			func_code = -1;
>  			rc = -ENOMEM;
>  			goto out;
>  		}
> @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
>  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;
>  		if (copy_from_user(targets, uptr,
>  				   target_num * sizeof(*targets))) {
> +			func_code = -1;
>  			rc = -EFAULT;
>  			goto out_free;
>  		}
> -- 
> 2.20.0
> 

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

* Re: [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code
  2019-04-08 21:26 ` [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code Arnd Bergmann
@ 2019-04-08 22:33   ` Nathan Chancellor
  2019-04-10 16:00   ` Martin Schwidefsky
  1 sibling, 0 replies; 43+ messages in thread
From: Nathan Chancellor @ 2019-04-08 22:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, Julian Wiedmann,
	Ursula Braun, clang-built-linux, Nick Desaulniers, linux-s390,
	linux-kernel

On Mon, Apr 08, 2019 at 11:26:19PM +0200, Arnd Bergmann wrote:
> clang points out that the return code from this function is
> undefined for one of the error paths:
> 
> ../drivers/s390/net/ctcm_main.c:1595:7: warning: variable 'result' is used uninitialized whenever 'if' condition is true
>       [-Wsometimes-uninitialized]
>                 if (priv->channel[direction] == NULL) {
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/s390/net/ctcm_main.c:1638:9: note: uninitialized use occurs here
>         return result;
>                ^~~~~~
> ../drivers/s390/net/ctcm_main.c:1595:3: note: remove the 'if' if its condition is always false
>                 if (priv->channel[direction] == NULL) {
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/s390/net/ctcm_main.c:1539:12: note: initialize the variable 'result' to silence this warning
>         int result;
>                   ^
> 
> Make it return -ENODEV here, as in the related failure cases.
> gcc has a known bug in underreporting some of these warnings
> when it has already eliminated the assignment of the return code
> based on some earlier optimization step.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

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

> ---
>  drivers/s390/net/ctcm_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
> index 7617d21cb296..f63c5c871d3d 100644
> --- a/drivers/s390/net/ctcm_main.c
> +++ b/drivers/s390/net/ctcm_main.c
> @@ -1595,6 +1595,7 @@ static int ctcm_new_device(struct ccwgroup_device *cgdev)
>  		if (priv->channel[direction] == NULL) {
>  			if (direction == CTCM_WRITE)
>  				channel_free(priv->channel[CTCM_READ]);
> +			result = -ENODEV;
>  			goto out_dev;
>  		}
>  		priv->channel[direction]->netdev = dev;
> -- 
> 2.20.0
> 

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

* Re: [PATCH 07/12] s390: cio: fix cio_irb declaration
  2019-04-08 21:26 ` [PATCH 07/12] s390: cio: fix cio_irb declaration Arnd Bergmann
@ 2019-04-08 22:35   ` Nathan Chancellor
  2019-04-09 13:13   ` Sebastian Ott
  1 sibling, 0 replies; 43+ messages in thread
From: Nathan Chancellor @ 2019-04-08 22:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, Sebastian Ott,
	Peter Oberparleiter, clang-built-linux, Nick Desaulniers,
	linux-s390, linux-kernel

On Mon, Apr 08, 2019 at 11:26:20PM +0200, Arnd Bergmann wrote:
> clang points out that the declaration of cio_irb does not match the
> definition exactly, it is missing the alignment attribute:
> 
> ../drivers/s390/cio/cio.c:50:1: warning: section does not match previous declaration [-Wsection]
> DEFINE_PER_CPU_ALIGNED(struct irb, cio_irb);
> ^
> ../include/linux/percpu-defs.h:150:2: note: expanded from macro 'DEFINE_PER_CPU_ALIGNED'
>         DEFINE_PER_CPU_SECTION(type, name, PER_CPU_ALIGNED_SECTION)     \
>         ^
> ../include/linux/percpu-defs.h:93:9: note: expanded from macro 'DEFINE_PER_CPU_SECTION'
>         extern __PCPU_ATTRS(sec) __typeof__(type) name;                 \
>                ^
> ../include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> ../drivers/s390/cio/cio.h:118:1: note: previous attribute is here
> DECLARE_PER_CPU(struct irb, cio_irb);
> ^
> ../include/linux/percpu-defs.h:111:2: note: expanded from macro 'DECLARE_PER_CPU'
>         DECLARE_PER_CPU_SECTION(type, name, "")
>         ^
> ../include/linux/percpu-defs.h:87:9: note: expanded from macro 'DECLARE_PER_CPU_SECTION'
>         extern __PCPU_ATTRS(sec) __typeof__(type) name
>                ^
> ../include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> Use DECLARE_PER_CPU_ALIGNED() here, to make the two match.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

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

> ---
>  drivers/s390/cio/cio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
> index 9811fd8a0c73..92eabbb5f18d 100644
> --- a/drivers/s390/cio/cio.h
> +++ b/drivers/s390/cio/cio.h
> @@ -115,7 +115,7 @@ struct subchannel {
>  	struct schib_config config;
>  } __attribute__ ((aligned(8)));
>  
> -DECLARE_PER_CPU(struct irb, cio_irb);
> +DECLARE_PER_CPU_ALIGNED(struct irb, cio_irb);
>  
>  #define to_subchannel(n) container_of(n, struct subchannel, dev)
>  
> -- 
> 2.20.0
> 

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

* Re: [PATCH 03/12] s390: purgatory: pass --target option to clang
  2019-04-08 22:03   ` Nathan Chancellor
@ 2019-04-09  6:43     ` Arnd Bergmann
  2019-04-09 16:30       ` Nathan Chancellor
  0 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-09  6:43 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, linux-s390, Philipp Rudo, Hendrik Brueckner,
	Linux Kernel Mailing List

On Tue, Apr 9, 2019 at 12:03 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Mon, Apr 08, 2019 at 11:26:16PM +0200, Arnd Bergmann wrote:
> > The purgatory Makefile does 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.
> >
> > Pass --target=s390x-linux to all compilers that understand
> > this option.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/s390/purgatory/Makefile | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
> > index ce6a3f75065b..3a14b968cec3 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 += $(call cc-option,--target=s390x-linux)
> >  KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
> >  KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
> >
> Would
>
> ifdef CONFIG_CC_IS_CLANG
> KBUILD_CFLAGS += --target=s390x-linux
> endif
>
> be a little clearer (and save a cc-option call)?

Fine with me as well. Actually I noticed later that we need the same
thing for arch/s390/boot/Makefile and arch/s390/boot/compressed/Makefile
in some form, so maybe we should drop this one for now and find
a solution that works for all three of them. The boot stuff is the one
patch I did not send, since I did not think I had a good solution there,
just one that happened to make it build.

> Otherwise, makes sense.
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

 Thanks,

       Arnd

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

* Re: [PATCH 04/12] s390: qeth: address type mismatch warning
  2019-04-08 22:16   ` Nathan Chancellor
@ 2019-04-09  6:44     ` Arnd Bergmann
  0 siblings, 0 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-09  6:44 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Martin Schwidefsky, Heiko Carstens, Julian Wiedmann,
	Ursula Braun, clang-built-linux, Nick Desaulniers, linux-s390,
	David S. Miller, Kittipon Meesompop, Linux Kernel Mailing List

On Tue, Apr 9, 2019 at 12:16 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Mon, Apr 08, 2019 at 11:26:17PM +0200, Arnd Bergmann wrote:
> > clang produces a harmless warning for each use for the qeth_adp_supported
> > macro:
> >
> > drivers/s390/net/qeth_l2_main.c:559:31: warning: implicit conversion from enumeration type 'enum qeth_ipa_setadp_cmd' to
> >       different enumeration type 'enum qeth_ipa_funcs' [-Wenum-conversion]
> >         if (qeth_adp_supported(card, IPA_SETADP_SET_PROMISC_MODE))
> >             ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/s390/net/qeth_core.h:179:41: note: expanded from macro 'qeth_adp_supported'
> >         qeth_is_ipa_supported(&c->options.adp, f)
> >         ~~~~~~~~~~~~~~~~~~~~~                  ^
> >
> > Add a version of this macro that uses the correct types, and
> > remove the unused qeth_adp_enabled() macro that has the same
> > problem.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> I wonder if it is better to just change the func parameter to type long.
> I guess it's better to keep the type safety to make sure values aren't
> unintentionally mixed but the body of the functions is the same so does
> the type actually matter?

I think using the right enum type makes most sense here, as this seems
to be something that is easy to confuse.

      Arnd

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

* Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-08 21:26 ` [PATCH 05/12] s390: zcrypt: initialize variables before_use Arnd Bergmann
  2019-04-08 22:31   ` Nathan Chancellor
@ 2019-04-09  9:54   ` Harald Freudenberger
  2019-04-09 10:13     ` Arnd Bergmann
  2019-04-10 15:59     ` Martin Schwidefsky
  1 sibling, 2 replies; 43+ messages in thread
From: Harald Freudenberger @ 2019-04-09  9:54 UTC (permalink / raw)
  To: Arnd Bergmann, Martin Schwidefsky, Heiko Carstens
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Ingo Franzki, linux-kernel

On 08.04.19 23:26, Arnd Bergmann wrote:
> The 'func_code' variable gets printed in debug statements without
> a prior initialization in multiple functions, as reported when building
> with clang:
>
> drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true
>       [-Wsometimes-uninitialized]
>         if (mex->outputdatalength < mex->inputdatalength) {
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here
>         trace_s390_zcrypt_rep(mex, func_code, rc,
>                                    ^~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false
>         if (mex->outputdatalength < mex->inputdatalength) {
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning
>         unsigned int func_code;
>                               ^
>
> Add initializations to all affected code paths to shut up the warning
> and make the warning output consistent.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/s390/crypto/zcrypt_api.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index eb93c2d27d0a..23472063d9a8 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,
>  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
>  
>  	if (mex->outputdatalength < mex->inputdatalength) {
> +		func_code = -1;
>  		rc = -EINVAL;
>  		goto out;
>  	}
> @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,
>  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);
>  
>  	if (crt->outputdatalength < crt->inputdatalength) {
> +		func_code = -1;
>  		rc = -EINVAL;
>  		goto out;
>  	}
> @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
>  
>  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);
>  		if (!targets) {
> +			func_code = -1;
>  			rc = -ENOMEM;
>  			goto out;
>  		}
> @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
>  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;
>  		if (copy_from_user(targets, uptr,
>  				   target_num * sizeof(*targets))) {
> +			func_code = -1;
>  			rc = -EFAULT;
>  			goto out_free;
>  		}
Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
variable initialized with 0 instead of -1.
If you agree with this, I'll rewrite the patch and apply it to our
internal git and it will appear at kernel org with the next s390 code merge then.
regards Harald Freudenberger


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

* Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-09  9:54   ` Harald Freudenberger
@ 2019-04-09 10:13     ` Arnd Bergmann
  2019-04-10 15:59     ` Martin Schwidefsky
  1 sibling, 0 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-09 10:13 UTC (permalink / raw)
  To: Harald Freudenberger
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Ingo Franzki,
	Linux Kernel Mailing List

On Tue, Apr 9, 2019 at 11:54 AM Harald Freudenberger
<freude@linux.ibm.com> wrote:
> On 08.04.19 23:26, Arnd Bergmann wrote:

> Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
> variable initialized with 0 instead of -1.
> If you agree with this, I'll rewrite the patch and apply it to our
> internal git and it will appear at kernel org with the next s390 code merge then.

That's fine with me, I don't care about the specific value that gets
assigned here.

Thanks,

      Arnd

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

* Re: [PATCH 07/12] s390: cio: fix cio_irb declaration
  2019-04-08 21:26 ` [PATCH 07/12] s390: cio: fix cio_irb declaration Arnd Bergmann
  2019-04-08 22:35   ` Nathan Chancellor
@ 2019-04-09 13:13   ` Sebastian Ott
  1 sibling, 0 replies; 43+ messages in thread
From: Sebastian Ott @ 2019-04-09 13:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, Peter Oberparleiter,
	clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, linux-kernel

On Mon, 8 Apr 2019, Arnd Bergmann wrote:
> clang points out that the declaration of cio_irb does not match the
> definition exactly, it is missing the alignment attribute:
> 
> ../drivers/s390/cio/cio.c:50:1: warning: section does not match previous declaration [-Wsection]
> DEFINE_PER_CPU_ALIGNED(struct irb, cio_irb);
> ^
> ../include/linux/percpu-defs.h:150:2: note: expanded from macro 'DEFINE_PER_CPU_ALIGNED'
>         DEFINE_PER_CPU_SECTION(type, name, PER_CPU_ALIGNED_SECTION)     \
>         ^
> ../include/linux/percpu-defs.h:93:9: note: expanded from macro 'DEFINE_PER_CPU_SECTION'
>         extern __PCPU_ATTRS(sec) __typeof__(type) name;                 \
>                ^
> ../include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> ../drivers/s390/cio/cio.h:118:1: note: previous attribute is here
> DECLARE_PER_CPU(struct irb, cio_irb);
> ^
> ../include/linux/percpu-defs.h:111:2: note: expanded from macro 'DECLARE_PER_CPU'
>         DECLARE_PER_CPU_SECTION(type, name, "")
>         ^
> ../include/linux/percpu-defs.h:87:9: note: expanded from macro 'DECLARE_PER_CPU_SECTION'
>         extern __PCPU_ATTRS(sec) __typeof__(type) name
>                ^
> ../include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> Use DECLARE_PER_CPU_ALIGNED() here, to make the two match.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for the patch! Applied.
Sebastian


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

* Re: [PATCH 03/12] s390: purgatory: pass --target option to clang
  2019-04-09  6:43     ` Arnd Bergmann
@ 2019-04-09 16:30       ` Nathan Chancellor
  2019-04-09 18:11         ` Nick Desaulniers
  0 siblings, 1 reply; 43+ messages in thread
From: Nathan Chancellor @ 2019-04-09 16:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, linux-s390, Philipp Rudo, Hendrik Brueckner,
	Linux Kernel Mailing List

On Tue, Apr 09, 2019 at 08:43:09AM +0200, Arnd Bergmann wrote:
> On Tue, Apr 9, 2019 at 12:03 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Mon, Apr 08, 2019 at 11:26:16PM +0200, Arnd Bergmann wrote:
> > > The purgatory Makefile does 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.
> > >
> > > Pass --target=s390x-linux to all compilers that understand
> > > this option.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  arch/s390/purgatory/Makefile | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
> > > index ce6a3f75065b..3a14b968cec3 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 += $(call cc-option,--target=s390x-linux)
> > >  KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
> > >  KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
> > >
> > Would
> >
> > ifdef CONFIG_CC_IS_CLANG
> > KBUILD_CFLAGS += --target=s390x-linux
> > endif
> >
> > be a little clearer (and save a cc-option call)?
> 
> Fine with me as well. Actually I noticed later that we need the same
> thing for arch/s390/boot/Makefile and arch/s390/boot/compressed/Makefile
> in some form, so maybe we should drop this one for now and find
> a solution that works for all three of them. The boot stuff is the one
> patch I did not send, since I did not think I had a good solution there,
> just one that happened to make it build.
> 

It did just occur to me that we added the --target and --gcc-toolchain
flags to a clang specific variable, CLANG_FLAGS, that has already been
used by powerpc in commit 813af51f5d30 ("powerpc/boot: Set target when
cross-compiling for clang"). I assume that could be used here?

Nathan

> > Otherwise, makes sense.
> >
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> 
>  Thanks,
> 
>        Arnd

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

* Re: [PATCH 03/12] s390: purgatory: pass --target option to clang
  2019-04-09 16:30       ` Nathan Chancellor
@ 2019-04-09 18:11         ` Nick Desaulniers
  0 siblings, 0 replies; 43+ messages in thread
From: Nick Desaulniers @ 2019-04-09 18:11 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Arnd Bergmann, Martin Schwidefsky, Heiko Carstens,
	clang-built-linux, linux-s390, Philipp Rudo, Hendrik Brueckner,
	Linux Kernel Mailing List

On Tue, Apr 9, 2019 at 9:30 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
> It did just occur to me that we added the --target and --gcc-toolchain
> flags to a clang specific variable, CLANG_FLAGS, that has already been
> used by powerpc in commit 813af51f5d30 ("powerpc/boot: Set target when
> cross-compiling for clang"). I assume that could be used here?

I think reusing CLANG_FLAGS is a better approach.

-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly
  2019-04-08 21:26 ` [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly Arnd Bergmann
@ 2019-04-10 13:55   ` Martin Schwidefsky
  2019-04-10 16:35     ` Nick Desaulniers
  2019-04-10 18:55     ` Arnd Bergmann
  0 siblings, 2 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 13:55 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Vasily Gorbik, linux-kernel

On Mon,  8 Apr 2019 23:26:25 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> clang does not understand the contraint "0" in the CALL_ON_STACK()
> macro:
> 
> ../arch/s390/mm/maccess.c:117:10: error: invalid input constraint '0' in asm
>                 return CALL_ON_STACK(_memcpy_real, S390_lowcore.nodat_stack,
>                        ^
> ../arch/s390/include/asm/processor.h:292:20: note: expanded from macro 'CALL_ON_STACK'
>                   [_fn] "X" (fn) CALL_FMT_##nr : CALL_CLOBBER_##nr);    \
>                                  ^
> <scratch space>:207:1: note: expanded from here
> CALL_FMT_3
> ^
> ../arch/s390/include/asm/processor.h:267:20: note: expanded from macro 'CALL_FMT_3'
>  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
>                    ^
> ../arch/s390/include/asm/processor.h:266:20: note: expanded from macro 'CALL_FMT_2'
>  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
>                    ^
> ../arch/s390/include/asm/processor.h:265:32: note: expanded from macro 'CALL_FMT_1'
>  #define CALL_FMT_1 CALL_FMT_0, "0" (r2)
>                                ^
> 
> I don't know what the correct fix here would be, changing it to "d" made
> it build, since clang does understand this one.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/include/asm/processor.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> index 700c650ffd4f..84c59c99668a 100644
> --- a/arch/s390/include/asm/processor.h
> +++ b/arch/s390/include/asm/processor.h
> @@ -262,7 +262,7 @@ static __no_kasan_or_inline unsigned short stap(void)
>  	register unsigned long r4 asm("6") = (unsigned long)(arg5)
> 
>  #define CALL_FMT_0
> -#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> +#define CALL_FMT_1 CALL_FMT_0, "d" (r2)
>  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
>  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
>  #define CALL_FMT_4 CALL_FMT_3, "d" (r5)

This is (slightly) wrong. %r2 is used as the input register for the first argument
and the result value for the call. With your patch you force the compiler to load
the first argument in two registers. One solution would be to CALL_FMT1 as

	#define CALL_FMT1 CALL_FMT_0

It still is not optimal though as for CALL_FMT_0 the "+&d" (r2) indicates an
input but CALL_ARGS_0 does not initialize r2.

I am thinking about the following patch to cover all cases:
--
From 91a4abbec91a9f26f84f7386f2c0f96de669b0eb Mon Sep 17 00:00:00 2001
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
Date: Wed, 10 Apr 2019 15:48:43 +0200
Subject: [PATCH] s390: fine-tune stack switch helper

The CALL_ON_STACK helper currently does not work with clang and for
calls without arguments it does not initialize r2 although the contraint
is "+&d". Rework the CALL_FMT_x and the CALL_ON_STACK macros to work
with clang and produce optimal code in all cases.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 arch/s390/include/asm/processor.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
index 81038ab357ce..0ee022247580 100644
--- a/arch/s390/include/asm/processor.h
+++ b/arch/s390/include/asm/processor.h
@@ -261,12 +261,12 @@ static __no_kasan_or_inline unsigned short stap(void)
 	CALL_ARGS_4(arg1, arg2, arg3, arg4);				\
 	register unsigned long r4 asm("6") = (unsigned long)(arg5)
 
-#define CALL_FMT_0
-#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
-#define CALL_FMT_2 CALL_FMT_1, "d" (r3)
-#define CALL_FMT_3 CALL_FMT_2, "d" (r4)
-#define CALL_FMT_4 CALL_FMT_3, "d" (r5)
-#define CALL_FMT_5 CALL_FMT_4, "d" (r6)
+#define CALL_FMT_0 "=&d" (r2) :
+#define CALL_FMT_1 "+&d" (r2) :
+#define CALL_FMT_2 CALL_FMT_1 "d" (r3),
+#define CALL_FMT_3 CALL_FMT_2 "d" (r4),
+#define CALL_FMT_4 CALL_FMT_3 "d" (r5),
+#define CALL_FMT_5 CALL_FMT_4 "d" (r6),
 
 #define CALL_CLOBBER_5 "0", "1", "14", "cc", "memory"
 #define CALL_CLOBBER_4 CALL_CLOBBER_5
@@ -286,10 +286,10 @@ static __no_kasan_or_inline unsigned short stap(void)
 		"	stg	%[_prev],%[_bc](15)\n"			\
 		"	brasl	14,%[_fn]\n"				\
 		"	la	15,0(%[_prev])\n"			\
-		: "+&d" (r2), [_prev] "=&a" (prev)			\
-		: [_stack] "a" (stack),					\
+		: [_prev] "=&a" (prev), CALL_FMT_##nr			\
+		  [_stack] "a" (stack),					\
 		  [_bc] "i" (offsetof(struct stack_frame, back_chain)),	\
-		  [_fn] "X" (fn) CALL_FMT_##nr : CALL_CLOBBER_##nr);	\
+		  [_fn] "X" (fn) : CALL_CLOBBER_##nr);			\
 	r2;								\
 })
 
-- 
2.16.4
-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 01/12] s390: remove -fno-strength-reduce flag
  2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
                   ` (10 preceding siblings ...)
  2019-04-08 21:26 ` [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly Arnd Bergmann
@ 2019-04-10 15:57 ` Martin Schwidefsky
  11 siblings, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 15:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Vasily Gorbik, Masahiro Yamada,
	Luc Van Oostenryck, linux-kernel

On Mon,  8 Apr 2019 23:26:14 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> This was added as a workaround for really old compilers, and it prevents
> building with clang now. I can see no reason for keeping it, as it has
> already been removed for most architectures in the pre-git era, so
> let's remove it everywhere, rather than only for clang.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/Makefile b/arch/s390/Makefile
> index 86c76b149cc2..0087d11e3eaf 100644
> --- a/arch/s390/Makefile
> +++ b/arch/s390/Makefile
> @@ -114,7 +114,7 @@ endif
>  cfi := $(call as-instr,.cfi_startproc\n.cfi_val_offset 15$(comma)-160\n.cfi_endproc,-DCONFIG_AS_CFI_VAL_OFFSET=1)
> 
>  KBUILD_CFLAGS	+= -mbackchain -msoft-float $(cflags-y)
> -KBUILD_CFLAGS	+= -pipe -fno-strength-reduce -Wno-sign-compare
> +KBUILD_CFLAGS	+= -pipe -Wno-sign-compare
>  KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables $(cfi)
>  KBUILD_AFLAGS	+= $(aflags-y) $(cfi)
>  export KBUILD_AFLAGS_DECOMPRESSOR

Added to s390/linux:features for the next merge window. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 02/12] s390: don't build vdso32 with clang
  2019-04-08 21:26 ` [PATCH 02/12] s390: don't build vdso32 with clang Arnd Bergmann
@ 2019-04-10 15:57   ` Martin Schwidefsky
  2019-04-10 16:26   ` Nick Desaulniers
  1 sibling, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 15:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Masahiro Yamada, Vasily Gorbik,
	Ursula Braun, Mike Rapoport, linux-kernel

On Mon,  8 Apr 2019 23:26:15 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> clang does not support 31 bit object files on s390, so skip
> the 32-bit vdso here, and only build it when using gcc to compile
> the kernel.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Added to s390/linux:features for the next merge window. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 04/12] s390: qeth: address type mismatch warning
  2019-04-08 21:26 ` [PATCH 04/12] s390: qeth: address type mismatch warning Arnd Bergmann
  2019-04-08 22:16   ` Nathan Chancellor
@ 2019-04-10 15:58   ` Martin Schwidefsky
  1 sibling, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 15:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, Julian Wiedmann, Ursula Braun, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, David S. Miller,
	Kittipon Meesompop, linux-kernel

On Mon,  8 Apr 2019 23:26:17 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> clang produces a harmless warning for each use for the qeth_adp_supported
> macro:
> 
> drivers/s390/net/qeth_l2_main.c:559:31: warning: implicit conversion from enumeration type 'enum qeth_ipa_setadp_cmd' to
>       different enumeration type 'enum qeth_ipa_funcs' [-Wenum-conversion]
>         if (qeth_adp_supported(card, IPA_SETADP_SET_PROMISC_MODE))
>             ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/net/qeth_core.h:179:41: note: expanded from macro 'qeth_adp_supported'
>         qeth_is_ipa_supported(&c->options.adp, f)
>         ~~~~~~~~~~~~~~~~~~~~~                  ^
> 
> Add a version of this macro that uses the correct types, and
> remove the unused qeth_adp_enabled() macro that has the same
> problem.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I have added this to our internal tree for Julian to pick up.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-09  9:54   ` Harald Freudenberger
  2019-04-09 10:13     ` Arnd Bergmann
@ 2019-04-10 15:59     ` Martin Schwidefsky
  2019-04-10 18:57       ` Arnd Bergmann
  1 sibling, 1 reply; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 15:59 UTC (permalink / raw)
  To: Harald Freudenberger
  Cc: Arnd Bergmann, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Ingo Franzki,
	linux-kernel

On Tue, 9 Apr 2019 11:54:30 +0200
Harald Freudenberger <freude@linux.ibm.com> wrote:

> On 08.04.19 23:26, Arnd Bergmann wrote:
> > The 'func_code' variable gets printed in debug statements without
> > a prior initialization in multiple functions, as reported when building
> > with clang:
> >
> > drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true
> >       [-Wsometimes-uninitialized]
> >         if (mex->outputdatalength < mex->inputdatalength) {
> >             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here
> >         trace_s390_zcrypt_rep(mex, func_code, rc,
> >                                    ^~~~~~~~~
> > drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false
> >         if (mex->outputdatalength < mex->inputdatalength) {
> >         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning
> >         unsigned int func_code;
> >                               ^
> >
> > Add initializations to all affected code paths to shut up the warning
> > and make the warning output consistent.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/s390/crypto/zcrypt_api.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> > index eb93c2d27d0a..23472063d9a8 100644
> > --- a/drivers/s390/crypto/zcrypt_api.c
> > +++ b/drivers/s390/crypto/zcrypt_api.c
> > @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,
> >  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
> >  
> >  	if (mex->outputdatalength < mex->inputdatalength) {
> > +		func_code = -1;
> >  		rc = -EINVAL;
> >  		goto out;
> >  	}
> > @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,
> >  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);
> >  
> >  	if (crt->outputdatalength < crt->inputdatalength) {
> > +		func_code = -1;
> >  		rc = -EINVAL;
> >  		goto out;
> >  	}
> > @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
> >  
> >  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);
> >  		if (!targets) {
> > +			func_code = -1;
> >  			rc = -ENOMEM;
> >  			goto out;
> >  		}
> > @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
> >  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;
> >  		if (copy_from_user(targets, uptr,
> >  				   target_num * sizeof(*targets))) {
> > +			func_code = -1;
> >  			rc = -EFAULT;
> >  			goto out_free;
> >  		}  
> Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
> variable initialized with 0 instead of -1.
> If you agree with this, I'll rewrite the patch and apply it to our
> internal git and it will appear at kernel org with the next s390 code merge then.

Do we agreement on func_coed=0 for this one ?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code
  2019-04-08 21:26 ` [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code Arnd Bergmann
  2019-04-08 22:33   ` Nathan Chancellor
@ 2019-04-10 16:00   ` Martin Schwidefsky
  1 sibling, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 16:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, Julian Wiedmann, Ursula Braun, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, linux-kernel

On Mon,  8 Apr 2019 23:26:19 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> clang points out that the return code from this function is
> undefined for one of the error paths:
> 
> ../drivers/s390/net/ctcm_main.c:1595:7: warning: variable 'result' is used uninitialized whenever 'if' condition is true
>       [-Wsometimes-uninitialized]
>                 if (priv->channel[direction] == NULL) {
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/s390/net/ctcm_main.c:1638:9: note: uninitialized use occurs here
>         return result;
>                ^~~~~~
> ../drivers/s390/net/ctcm_main.c:1595:3: note: remove the 'if' if its condition is always false
>                 if (priv->channel[direction] == NULL) {
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/s390/net/ctcm_main.c:1539:12: note: initialize the variable 'result' to silence this warning
>         int result;
>                   ^
> 
> Make it return -ENODEV here, as in the related failure cases.
> gcc has a known bug in underreporting some of these warnings
> when it has already eliminated the assignment of the return code
> based on some earlier optimization step.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Added to our internal tree for Julian to pick up.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 08/12] s390: syscall_wrapper: avoid clang warning
  2019-04-08 21:26 ` [PATCH 08/12] s390: syscall_wrapper: avoid clang warning Arnd Bergmann
@ 2019-04-10 16:00   ` Martin Schwidefsky
  0 siblings, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 16:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, linux-kernel

On Mon,  8 Apr 2019 23:26:21 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> Building system calls with clang results in a warning
> about an alias from a global function to a static one:
> 
> ../fs/namei.c:3847:1: warning: unused function '__se_sys_mkdirat' [-Wunused-function]
> SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
> ^
> ../include/linux/syscalls.h:219:36: note: expanded from macro 'SYSCALL_DEFINE3'
>  #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>                                    ^
> ../include/linux/syscalls.h:228:2: note: expanded from macro 'SYSCALL_DEFINEx'
>         __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>         ^
> ../arch/s390/include/asm/syscall_wrapper.h:126:18: note: expanded from macro '__SYSCALL_DEFINEx'
>         asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))          \
>                         ^
> <scratch space>:31:1: note: expanded from here
> __se_sys_mkdirat
> ^
> 
> The only reference to the static __se_sys_mkdirat() here is the alias, but
> this only gets evaluated later. Making this function global as well avoids
> the warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Added to s390/linux:features for the next merge window. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 09/12] s390: make __load_psw_mask work with clang
  2019-04-08 21:26 ` [PATCH 09/12] s390: make __load_psw_mask work with clang Arnd Bergmann
@ 2019-04-10 16:01   ` Martin Schwidefsky
  0 siblings, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 16:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Vasily Gorbik,
	Steven Rostedt (VMware),
	linux-kernel

On Mon,  8 Apr 2019 23:26:22 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> clang fails to use the %O and %R inline assembly modifiers
> the same way as gcc, leading to build failures with every use
> of __load_psw_mask():
> 
> /tmp/nmi-4a9f80.s: Assembler messages:
> /tmp/nmi-4a9f80.s:571: Error: junk at end of line: `+8(160(%r11))'
> /tmp/nmi-4a9f80.s:626: Error: junk at end of line: `+8(160(%r11))'
> 
> Replace these with a more conventional way of passing the addresses
> that should work with both clang and gcc.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Added to s390/linux:features for the next merge window. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 11/12] s390: make chkbss work with clang
  2019-04-08 21:26 ` [PATCH 11/12] s390: make chkbss work with clang Arnd Bergmann
@ 2019-04-10 16:02   ` Martin Schwidefsky
  0 siblings, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 16:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Vasily Gorbik, linux-kernel

On Mon,  8 Apr 2019 23:26:24 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> llvm skips an empty .bss section entirely, which makes
> the check fail with an unexpected error:
> 
> /tmp/binutils-multi-test/bin/s390x-linux-gnu-objdump: section '.bss' mentioned in a -j option, but not found in any input file
> error: arch/s390/boot/compressed/decompressor.o .bss section is not empty
> ../arch/s390/scripts/Makefile.chkbss:20: recipe for target 'arch/s390/boot/compressed/decompressor.o.chkbss' failed
> 
> Change the check so we first see if a .bss section exists
> before trying to read its size.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Added to s390/linux:features for the next merge window. Thanks.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang
  2019-04-08 21:26 ` [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang Arnd Bergmann
@ 2019-04-10 16:03   ` Martin Schwidefsky
  2019-04-10 16:14     ` Steven Rostedt
  0 siblings, 1 reply; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-10 16:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Heiko Carstens, Steven Rostedt, Ingo Molnar, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Vasily Gorbik,
	linux-kernel

On Mon,  8 Apr 2019 23:26:23 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> llvm on s390 has problems with __builtin_return_address(n), with n>0,
> this results in a somewhat cryptic error message:
> 
> fatal error: error in backend: Unsupported stack frame traversal count
> 
> To work around it, use the direct return address directly. This
> is probably not ideal here, but gets things to compile and should
> only lead to inferior reporting, not to misbehavior of the generated
> code.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=41424
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/include/asm/ftrace.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
> index 5a3c95b11952..7923c63946fb 100644
> --- a/arch/s390/include/asm/ftrace.h
> +++ b/arch/s390/include/asm/ftrace.h
> @@ -13,7 +13,12 @@
> 
>  #ifndef __ASSEMBLY__
> 
> +#ifdef CONFIG_CC_IS_CLANG
> +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
> +#define ftrace_return_address(n) __builtin_return_address(0)
> +#else
>  #define ftrace_return_address(n) __builtin_return_address(n)
> +#endif
> 
>  void _mcount(void);
>  void ftrace_caller(void);

I can say I like this one. If the compiler can not do __builtin_return_address(n)
it feels wrong to just use __builtin_return_address(0).

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang
  2019-04-10 16:03   ` Martin Schwidefsky
@ 2019-04-10 16:14     ` Steven Rostedt
  2019-04-10 19:07       ` Arnd Bergmann
  0 siblings, 1 reply; 43+ messages in thread
From: Steven Rostedt @ 2019-04-10 16:14 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Arnd Bergmann, Heiko Carstens, Ingo Molnar, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Vasily Gorbik,
	linux-kernel

On Wed, 10 Apr 2019 18:03:57 +0200
Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> > --- a/arch/s390/include/asm/ftrace.h
> > +++ b/arch/s390/include/asm/ftrace.h
> > @@ -13,7 +13,12 @@
> > 
> >  #ifndef __ASSEMBLY__
> > 
> > +#ifdef CONFIG_CC_IS_CLANG
> > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
> > +#define ftrace_return_address(n) __builtin_return_address(0)
> > +#else
> >  #define ftrace_return_address(n) __builtin_return_address(n)
> > +#endif
> > 
> >  void _mcount(void);
> >  void ftrace_caller(void);  
> 
> I can say I like this one. If the compiler can not do __builtin_return_address(n)
> it feels wrong to just use __builtin_return_address(0).

I agree. The proper return value is 0UL, see include/linux/ftrace.h

/* Archs may use other ways for ADDR1 and beyond */
#ifndef ftrace_return_address
# ifdef CONFIG_FRAME_POINTER
#  define ftrace_return_address(n) __builtin_return_address(n)
# else
#  define ftrace_return_address(n) 0UL
# endif
#endif

This is why we treat zero differently:

#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))


-- Steve

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

* Re: [PATCH 02/12] s390: don't build vdso32 with clang
  2019-04-08 21:26 ` [PATCH 02/12] s390: don't build vdso32 with clang Arnd Bergmann
  2019-04-10 15:57   ` Martin Schwidefsky
@ 2019-04-10 16:26   ` Nick Desaulniers
  2019-04-10 19:00     ` Arnd Bergmann
  1 sibling, 1 reply; 43+ messages in thread
From: Nick Desaulniers @ 2019-04-10 16:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nathan Chancellor, linux-s390, Masahiro Yamada, Vasily Gorbik,
	Ursula Braun, Mike Rapoport, LKML

On Mon, Apr 8, 2019 at 2:27 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> clang does not support 31 bit object files on s390, so skip
> the 32-bit vdso here, and only build it when using gcc to compile
> the kernel.

What's the build failure?  Would you mind filing a bug against LLVM's
issue tracker for it, please?

>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/Kconfig         |  3 +++
>  arch/s390/kernel/Makefile |  2 +-
>  arch/s390/kernel/vdso.c   | 10 +++++-----
>  3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b6e3d0653002..8cd860cba4d1 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -388,6 +388,9 @@ config COMPAT
>           (and some other stuff like libraries and such) is needed for
>           executing 31 bit applications.  It is safe to say "Y".
>
> +config COMPAT_VDSO
> +       def_bool COMPAT && !CC_IS_CLANG
> +
>  config SYSVIPC_COMPAT
>         def_bool y if COMPAT && SYSVIPC
>
> diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
> index 8a62c7f72e1b..1222db6d4ee9 100644
> --- a/arch/s390/kernel/Makefile
> +++ b/arch/s390/kernel/Makefile
> @@ -86,7 +86,7 @@ obj-$(CONFIG_TRACEPOINTS)     += trace.o
>
>  # vdso
>  obj-y                          += vdso64/
> -obj-$(CONFIG_COMPAT)           += vdso32/
> +obj-$(CONFIG_COMPAT_VDSO)      += vdso32/
>
>  chkbss := head64.o early_nobss.o
>  include $(srctree)/arch/s390/scripts/Makefile.chkbss
> diff --git a/arch/s390/kernel/vdso.c b/arch/s390/kernel/vdso.c
> index e7920a68a12e..243d8b1185bf 100644
> --- a/arch/s390/kernel/vdso.c
> +++ b/arch/s390/kernel/vdso.c
> @@ -29,7 +29,7 @@
>  #include <asm/vdso.h>
>  #include <asm/facility.h>
>
> -#ifdef CONFIG_COMPAT
> +#ifdef CONFIG_COMPAT_VDSO
>  extern char vdso32_start, vdso32_end;
>  static void *vdso32_kbase = &vdso32_start;
>  static unsigned int vdso32_pages;
> @@ -55,7 +55,7 @@ static vm_fault_t vdso_fault(const struct vm_special_mapping *sm,
>
>         vdso_pagelist = vdso64_pagelist;
>         vdso_pages = vdso64_pages;
> -#ifdef CONFIG_COMPAT
> +#ifdef CONFIG_COMPAT_VDSO
>         if (vma->vm_mm->context.compat_mm) {
>                 vdso_pagelist = vdso32_pagelist;
>                 vdso_pages = vdso32_pages;
> @@ -76,7 +76,7 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
>         unsigned long vdso_pages;
>
>         vdso_pages = vdso64_pages;
> -#ifdef CONFIG_COMPAT
> +#ifdef CONFIG_COMPAT_VDSO
>         if (vma->vm_mm->context.compat_mm)
>                 vdso_pages = vdso32_pages;
>  #endif
> @@ -223,7 +223,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
>                 return 0;
>
>         vdso_pages = vdso64_pages;
> -#ifdef CONFIG_COMPAT
> +#ifdef CONFIG_COMPAT_VDSO
>         mm->context.compat_mm = is_compat_task();
>         if (mm->context.compat_mm)
>                 vdso_pages = vdso32_pages;
> @@ -280,7 +280,7 @@ static int __init vdso_init(void)
>         int i;
>
>         vdso_init_data(vdso_data);
> -#ifdef CONFIG_COMPAT
> +#ifdef CONFIG_COMPAT_VDSO
>         /* Calculate the size of the 32 bit vDSO */
>         vdso32_pages = ((&vdso32_end - &vdso32_start
>                          + PAGE_SIZE - 1) >> PAGE_SHIFT) + 1;
> --
> 2.20.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly
  2019-04-10 13:55   ` Martin Schwidefsky
@ 2019-04-10 16:35     ` Nick Desaulniers
  2019-04-10 18:55     ` Arnd Bergmann
  1 sibling, 0 replies; 43+ messages in thread
From: Nick Desaulniers @ 2019-04-10 16:35 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Arnd Bergmann, Heiko Carstens, clang-built-linux,
	Nathan Chancellor, linux-s390, Vasily Gorbik, LKML

On Wed, Apr 10, 2019 at 6:55 AM Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
>
> On Mon,  8 Apr 2019 23:26:25 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
> > clang does not understand the contraint "0" in the CALL_ON_STACK()
> > macro:
> >
> > ../arch/s390/mm/maccess.c:117:10: error: invalid input constraint '0' in asm
> >                 return CALL_ON_STACK(_memcpy_real, S390_lowcore.nodat_stack,
> >                        ^
> > ../arch/s390/include/asm/processor.h:292:20: note: expanded from macro 'CALL_ON_STACK'
> >                   [_fn] "X" (fn) CALL_FMT_##nr : CALL_CLOBBER_##nr);    \
> >                                  ^
> > <scratch space>:207:1: note: expanded from here
> > CALL_FMT_3
> > ^
> > ../arch/s390/include/asm/processor.h:267:20: note: expanded from macro 'CALL_FMT_3'
> >  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
> >                    ^
> > ../arch/s390/include/asm/processor.h:266:20: note: expanded from macro 'CALL_FMT_2'
> >  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
> >                    ^
> > ../arch/s390/include/asm/processor.h:265:32: note: expanded from macro 'CALL_FMT_1'
> >  #define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> >                                ^
> >
> > I don't know what the correct fix here would be, changing it to "d" made
> > it build, since clang does understand this one.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/s390/include/asm/processor.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> > index 700c650ffd4f..84c59c99668a 100644
> > --- a/arch/s390/include/asm/processor.h
> > +++ b/arch/s390/include/asm/processor.h
> > @@ -262,7 +262,7 @@ static __no_kasan_or_inline unsigned short stap(void)
> >       register unsigned long r4 asm("6") = (unsigned long)(arg5)
> >
> >  #define CALL_FMT_0
> > -#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> > +#define CALL_FMT_1 CALL_FMT_0, "d" (r2)
> >  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
> >  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
> >  #define CALL_FMT_4 CALL_FMT_3, "d" (r5)
>
> This is (slightly) wrong. %r2 is used as the input register for the first argument
> and the result value for the call. With your patch you force the compiler to load
> the first argument in two registers. One solution would be to CALL_FMT1 as
>
>         #define CALL_FMT1 CALL_FMT_0
>
> It still is not optimal though as for CALL_FMT_0 the "+&d" (r2) indicates an
> input but CALL_ARGS_0 does not initialize r2.
>
> I am thinking about the following patch to cover all cases:
> --
> From 91a4abbec91a9f26f84f7386f2c0f96de669b0eb Mon Sep 17 00:00:00 2001
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Date: Wed, 10 Apr 2019 15:48:43 +0200
> Subject: [PATCH] s390: fine-tune stack switch helper

Thanks for the patch. Just some minor typos in the commit.

>
> The CALL_ON_STACK helper currently does not work with clang and for
> calls without arguments it does not initialize r2 although the contraint

- calls without arguments it ...
+ calls without arguments.  It ...

- although the contraint
+ although the constraint

`:set spell` in vim (I wonder if there's a way to set that when
writing commit messages automatically)

> is "+&d". Rework the CALL_FMT_x and the CALL_ON_STACK macros to work
> with clang and produce optimal code in all cases.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
>  arch/s390/include/asm/processor.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> index 81038ab357ce..0ee022247580 100644
> --- a/arch/s390/include/asm/processor.h
> +++ b/arch/s390/include/asm/processor.h
> @@ -261,12 +261,12 @@ static __no_kasan_or_inline unsigned short stap(void)
>         CALL_ARGS_4(arg1, arg2, arg3, arg4);                            \
>         register unsigned long r4 asm("6") = (unsigned long)(arg5)
>
> -#define CALL_FMT_0
> -#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> -#define CALL_FMT_2 CALL_FMT_1, "d" (r3)
> -#define CALL_FMT_3 CALL_FMT_2, "d" (r4)
> -#define CALL_FMT_4 CALL_FMT_3, "d" (r5)
> -#define CALL_FMT_5 CALL_FMT_4, "d" (r6)
> +#define CALL_FMT_0 "=&d" (r2) :
> +#define CALL_FMT_1 "+&d" (r2) :
> +#define CALL_FMT_2 CALL_FMT_1 "d" (r3),
> +#define CALL_FMT_3 CALL_FMT_2 "d" (r4),
> +#define CALL_FMT_4 CALL_FMT_3 "d" (r5),
> +#define CALL_FMT_5 CALL_FMT_4 "d" (r6),
>
>  #define CALL_CLOBBER_5 "0", "1", "14", "cc", "memory"
>  #define CALL_CLOBBER_4 CALL_CLOBBER_5
> @@ -286,10 +286,10 @@ static __no_kasan_or_inline unsigned short stap(void)
>                 "       stg     %[_prev],%[_bc](15)\n"                  \
>                 "       brasl   14,%[_fn]\n"                            \
>                 "       la      15,0(%[_prev])\n"                       \
> -               : "+&d" (r2), [_prev] "=&a" (prev)                      \
> -               : [_stack] "a" (stack),                                 \
> +               : [_prev] "=&a" (prev), CALL_FMT_##nr                   \
> +                 [_stack] "a" (stack),                                 \
>                   [_bc] "i" (offsetof(struct stack_frame, back_chain)), \
> -                 [_fn] "X" (fn) CALL_FMT_##nr : CALL_CLOBBER_##nr);    \
> +                 [_fn] "X" (fn) : CALL_CLOBBER_##nr);                  \
>         r2;                                                             \
>  })
>
> --
> 2.16.4
> --
> blue skies,
>    Martin.
>
> "Reality continues to ruin my life." - Calvin.
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To post to this group, send email to clang-built-linux@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190410155513.1609f1a1%40mschwideX1.
> For more options, visit https://groups.google.com/d/optout.



-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly
  2019-04-10 13:55   ` Martin Schwidefsky
  2019-04-10 16:35     ` Nick Desaulniers
@ 2019-04-10 18:55     ` Arnd Bergmann
  1 sibling, 0 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-10 18:55 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Heiko Carstens, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, linux-s390, Vasily Gorbik,
	Linux Kernel Mailing List

On Wed, Apr 10, 2019 at 3:55 PM Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
>
> On Mon,  8 Apr 2019 23:26:25 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
> > diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> > index 700c650ffd4f..84c59c99668a 100644
> > --- a/arch/s390/include/asm/processor.h
> > +++ b/arch/s390/include/asm/processor.h
> > @@ -262,7 +262,7 @@ static __no_kasan_or_inline unsigned short stap(void)
> >       register unsigned long r4 asm("6") = (unsigned long)(arg5)
> >
> >  #define CALL_FMT_0
> > -#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> > +#define CALL_FMT_1 CALL_FMT_0, "d" (r2)
> >  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
> >  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
> >  #define CALL_FMT_4 CALL_FMT_3, "d" (r5)
>
> This is (slightly) wrong. %r2 is used as the input register for the first argument
> and the result value for the call. With your patch you force the compiler to load
> the first argument in two registers. One solution would be to CALL_FMT1 as
>
>         #define CALL_FMT1 CALL_FMT_0
>
> It still is not optimal though as for CALL_FMT_0 the "+&d" (r2) indicates an
> input but CALL_ARGS_0 does not initialize r2.

Ok, thanks for taking a closer look!

> I am thinking about the following patch to cover all cases:
> --
> From 91a4abbec91a9f26f84f7386f2c0f96de669b0eb Mon Sep 17 00:00:00 2001
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Date: Wed, 10 Apr 2019 15:48:43 +0200
> Subject: [PATCH] s390: fine-tune stack switch helper
>
> The CALL_ON_STACK helper currently does not work with clang and for
> calls without arguments it does not initialize r2 although the contraint
> is "+&d". Rework the CALL_FMT_x and the CALL_ON_STACK macros to work
> with clang and produce optimal code in all cases.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

I did another build test to confirm that your patch works fine with clang
as well, looks good to me.

      Arnd

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

* Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-10 15:59     ` Martin Schwidefsky
@ 2019-04-10 18:57       ` Arnd Bergmann
  2019-04-11  7:40         ` Martin Schwidefsky
  0 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-10 18:57 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Harald Freudenberger, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Ingo Franzki,
	Linux Kernel Mailing List

On Wed, Apr 10, 2019 at 5:59 PM Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> On Tue, 9 Apr 2019 11:54:30 +0200 Harald Freudenberger <freude@linux.ibm.com> wrote:
> > On 08.04.19 23:26, Arnd Bergmann wrote:
> > >             }
> > Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
> > variable initialized with 0 instead of -1.
> > If you agree with this, I'll rewrite the patch and apply it to our
> > internal git and it will appear at kernel org with the next s390 code merge then.
>
> Do we agreement on func_coed=0 for this one ?

Yes, I think that was the consensus.

       Arnd

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

* Re: [PATCH 02/12] s390: don't build vdso32 with clang
  2019-04-10 16:26   ` Nick Desaulniers
@ 2019-04-10 19:00     ` Arnd Bergmann
  0 siblings, 0 replies; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-10 19:00 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Martin Schwidefsky, Heiko Carstens, clang-built-linux,
	Nathan Chancellor, linux-s390, Masahiro Yamada, Vasily Gorbik,
	Ursula Braun, Mike Rapoport, LKML

On Wed, Apr 10, 2019 at 6:26 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Mon, Apr 8, 2019 at 2:27 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > clang does not support 31 bit object files on s390, so skip
> > the 32-bit vdso here, and only build it when using gcc to compile
> > the kernel.
>
> What's the build failure?  Would you mind filing a bug against LLVM's
> issue tracker for it, please?

As far as I can tell, llvm does only supports 64-bit output for s390, so this
is not a bug but rather a missing feature that seems highly unlikely to
ever get added.

32-bit (31-bit) mode on s390 is only used for very old existing binaries,
and the vdso support is optional there.

      Arnd

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

* Re: [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang
  2019-04-10 16:14     ` Steven Rostedt
@ 2019-04-10 19:07       ` Arnd Bergmann
  2019-04-11  7:32         ` Martin Schwidefsky
  0 siblings, 1 reply; 43+ messages in thread
From: Arnd Bergmann @ 2019-04-10 19:07 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Martin Schwidefsky, Heiko Carstens, Ingo Molnar,
	clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	linux-s390, Vasily Gorbik, Linux Kernel Mailing List

On Wed, Apr 10, 2019 at 6:14 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>
> > > --- a/arch/s390/include/asm/ftrace.h
> > > +++ b/arch/s390/include/asm/ftrace.h
> > > @@ -13,7 +13,12 @@
> > >
> > >  #ifndef __ASSEMBLY__
> > >
> > > +#ifdef CONFIG_CC_IS_CLANG
> > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
> > > +#define ftrace_return_address(n) __builtin_return_address(0)
> > > +#else
> > >  #define ftrace_return_address(n) __builtin_return_address(n)
> > > +#endif
> > >
> > >  void _mcount(void);
> > >  void ftrace_caller(void);
> >
> > I can say I like this one. If the compiler can not do __builtin_return_address(n)
> > it feels wrong to just use __builtin_return_address(0).
>
> I agree. The proper return value is 0UL, see include/linux/ftrace.h
>
> /* Archs may use other ways for ADDR1 and beyond */
> #ifndef ftrace_return_address
> # ifdef CONFIG_FRAME_POINTER
> #  define ftrace_return_address(n) __builtin_return_address(n)
> # else
> #  define ftrace_return_address(n) 0UL
> # endif
> #endif
>
> This is why we treat zero differently:
>
> #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
> #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
> #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
> #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
> #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
> #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
> #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))

Right, got it.

Martin, do you want me to send a replacement patch, or can you
commit the patch with

#ifdef CONFIG_CC_IS_CLANG
/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
#define ftrace_return_address(n) 0UL
#else
#define ftrace_return_address(n) __builtin_return_address(n)
#endif

instead?

    Arnd

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

* Re: [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang
  2019-04-10 19:07       ` Arnd Bergmann
@ 2019-04-11  7:32         ` Martin Schwidefsky
  0 siblings, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-11  7:32 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Steven Rostedt, Heiko Carstens, Ingo Molnar, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Vasily Gorbik,
	Linux Kernel Mailing List

On Wed, 10 Apr 2019 21:07:56 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wed, Apr 10, 2019 at 6:14 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> >  
> > > > --- a/arch/s390/include/asm/ftrace.h
> > > > +++ b/arch/s390/include/asm/ftrace.h
> > > > @@ -13,7 +13,12 @@
> > > >
> > > >  #ifndef __ASSEMBLY__
> > > >
> > > > +#ifdef CONFIG_CC_IS_CLANG
> > > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */
> > > > +#define ftrace_return_address(n) __builtin_return_address(0)
> > > > +#else
> > > >  #define ftrace_return_address(n) __builtin_return_address(n)
> > > > +#endif
> > > >
> > > >  void _mcount(void);
> > > >  void ftrace_caller(void);  
> > >
> > > I can say I like this one. If the compiler can not do __builtin_return_address(n)
> > > it feels wrong to just use __builtin_return_address(0).  
> >
> > I agree. The proper return value is 0UL, see include/linux/ftrace.h
> >
> > /* Archs may use other ways for ADDR1 and beyond */
> > #ifndef ftrace_return_address
> > # ifdef CONFIG_FRAME_POINTER
> > #  define ftrace_return_address(n) __builtin_return_address(n)
> > # else
> > #  define ftrace_return_address(n) 0UL
> > # endif
> > #endif
> >
> > This is why we treat zero differently:
> >
> > #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
> > #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
> > #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
> > #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
> > #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
> > #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
> > #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))  
> 
> Right, got it.
> 
> Martin, do you want me to send a replacement patch, or can you
> commit the patch with
> 
> #ifdef CONFIG_CC_IS_CLANG
> /* https://bugs.llvm.org/show_bug.cgi?id=41424 */
> #define ftrace_return_address(n) 0UL
> #else
> #define ftrace_return_address(n) __builtin_return_address(n)
> #endif
> 
> instead?

Ok, done.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

* Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use
  2019-04-10 18:57       ` Arnd Bergmann
@ 2019-04-11  7:40         ` Martin Schwidefsky
  0 siblings, 0 replies; 43+ messages in thread
From: Martin Schwidefsky @ 2019-04-11  7:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Harald Freudenberger, Heiko Carstens, clang-built-linux,
	Nick Desaulniers, Nathan Chancellor, linux-s390, Ingo Franzki,
	Linux Kernel Mailing List

On Wed, 10 Apr 2019 20:57:21 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wed, Apr 10, 2019 at 5:59 PM Martin Schwidefsky
> <schwidefsky@de.ibm.com> wrote:
> > On Tue, 9 Apr 2019 11:54:30 +0200 Harald Freudenberger <freude@linux.ibm.com> wrote:  
> > > On 08.04.19 23:26, Arnd Bergmann wrote:  
> > > >             }  
> > > Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
> > > variable initialized with 0 instead of -1.
> > > If you agree with this, I'll rewrite the patch and apply it to our
> > > internal git and it will appear at kernel org with the next s390 code merge then.  
> >
> > Do we agreement on func_coed=0 for this one ?  
> 
> Yes, I think that was the consensus.
> 
>        Arnd
> 

Ok, committed with func_code=0.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


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

end of thread, other threads:[~2019-04-11  7:43 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 21:26 [PATCH 01/12] s390: remove -fno-strength-reduce flag Arnd Bergmann
2019-04-08 21:26 ` [PATCH 02/12] s390: don't build vdso32 with clang Arnd Bergmann
2019-04-10 15:57   ` Martin Schwidefsky
2019-04-10 16:26   ` Nick Desaulniers
2019-04-10 19:00     ` Arnd Bergmann
2019-04-08 21:26 ` [PATCH 03/12] s390: purgatory: pass --target option to clang Arnd Bergmann
2019-04-08 22:03   ` Nathan Chancellor
2019-04-09  6:43     ` Arnd Bergmann
2019-04-09 16:30       ` Nathan Chancellor
2019-04-09 18:11         ` Nick Desaulniers
2019-04-08 21:26 ` [PATCH 04/12] s390: qeth: address type mismatch warning Arnd Bergmann
2019-04-08 22:16   ` Nathan Chancellor
2019-04-09  6:44     ` Arnd Bergmann
2019-04-10 15:58   ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 05/12] s390: zcrypt: initialize variables before_use Arnd Bergmann
2019-04-08 22:31   ` Nathan Chancellor
2019-04-09  9:54   ` Harald Freudenberger
2019-04-09 10:13     ` Arnd Bergmann
2019-04-10 15:59     ` Martin Schwidefsky
2019-04-10 18:57       ` Arnd Bergmann
2019-04-11  7:40         ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 06/12] s390: ctcm: fix ctcm_new_device error return code Arnd Bergmann
2019-04-08 22:33   ` Nathan Chancellor
2019-04-10 16:00   ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 07/12] s390: cio: fix cio_irb declaration Arnd Bergmann
2019-04-08 22:35   ` Nathan Chancellor
2019-04-09 13:13   ` Sebastian Ott
2019-04-08 21:26 ` [PATCH 08/12] s390: syscall_wrapper: avoid clang warning Arnd Bergmann
2019-04-10 16:00   ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 09/12] s390: make __load_psw_mask work with clang Arnd Bergmann
2019-04-10 16:01   ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 10/12] s390: avoid __builtin_return_address(n) on clang Arnd Bergmann
2019-04-10 16:03   ` Martin Schwidefsky
2019-04-10 16:14     ` Steven Rostedt
2019-04-10 19:07       ` Arnd Bergmann
2019-04-11  7:32         ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 11/12] s390: make chkbss work with clang Arnd Bergmann
2019-04-10 16:02   ` Martin Schwidefsky
2019-04-08 21:26 ` [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly Arnd Bergmann
2019-04-10 13:55   ` Martin Schwidefsky
2019-04-10 16:35     ` Nick Desaulniers
2019-04-10 18:55     ` Arnd Bergmann
2019-04-10 15:57 ` [PATCH 01/12] s390: remove -fno-strength-reduce flag Martin Schwidefsky

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).