[PATCH 01/12] s390: remove -fno-strength-reduce flag

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Arnd Bergmann]

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 <ar...@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

[Nathan Chancellor]

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 <natecha...@gmail.com>

[Nathan Chancellor]

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 <ar...@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 <natecha...@gmail.com>

[Nathan Chancellor]

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 <ar...@arndb.de>

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

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

[Nathan Chancellor]

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 <ar...@arndb.de>

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

[Nathan Chancellor]

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

[Arnd Bergmann]

On Tue, Apr 9, 2019 at 12:03 AM Nathan Chancellor
<natecha...@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 <ar...@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 <natecha...@gmail.com>

Thanks,

Arnd

[Arnd Bergmann]

On Tue, Apr 9, 2019 at 12:16 AM Nathan Chancellor
<natecha...@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 <ar...@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

[Harald Freudenberger]

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

[Arnd Bergmann]

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

Thanks,

Arnd

[Sebastian Ott]

Thanks for the patch! Applied.
Sebastian

[Nathan Chancellor]

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

[Nick Desaulniers]

I think reusing CLANG_FLAGS is a better approach.

--
Thanks,
~Nick Desaulniers

[Martin Schwidefsky]

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 <schwi...@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 <ar...@arndb.de>
Signed-off-by: Martin Schwidefsky <schwi...@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.

[Martin Schwidefsky]

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

[Martin Schwidefsky]

On Mon, 8 Apr 2019 23:26:15 +0200
Arnd Bergmann <ar...@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 <ar...@arndb.de>

[Martin Schwidefsky]

On Mon, 8 Apr 2019 23:26:17 +0200
Arnd Bergmann <ar...@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 <ar...@arndb.de>

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

[Martin Schwidefsky]

Do we agreement on func_coed=0 for this one ?

[Martin Schwidefsky]

On Mon, 8 Apr 2019 23:26:19 +0200
Arnd Bergmann <ar...@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 <ar...@arndb.de>

Added to our internal tree for Julian to pick up.

[Martin Schwidefsky]

On Mon, 8 Apr 2019 23:26:21 +0200
Arnd Bergmann <ar...@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 <ar...@arndb.de>

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

[Martin Schwidefsky]

On Mon, 8 Apr 2019 23:26:22 +0200
Arnd Bergmann <ar...@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 <ar...@arndb.de>

[Martin Schwidefsky]

On Mon, 8 Apr 2019 23:26:24 +0200
Arnd Bergmann <ar...@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 <ar...@arndb.de>

[Martin Schwidefsky]

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

[Steven Rostedt]

On Wed, 10 Apr 2019 18:03:57 +0200
Martin Schwidefsky <schwi...@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

[Nick Desaulniers]

On Mon, Apr 8, 2019 at 2:27 PM Arnd Bergmann <ar...@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 <ar...@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

[Nick Desaulniers]

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)

> --
> 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-li...@googlegroups.com.
> To post to this group, send email to clang-bu...@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

[Arnd Bergmann]

On Wed, Apr 10, 2019 at 3:55 PM Martin Schwidefsky
<schwi...@de.ibm.com> wrote:
>
> On Mon, 8 Apr 2019 23:26:25 +0200
> Arnd Bergmann <ar...@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 <schwi...@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 <ar...@arndb.de>
> Signed-off-by: Martin Schwidefsky <schwi...@de.ibm.com>

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

Arnd

[Arnd Bergmann]

Yes, I think that was the consensus.

Arnd

[Arnd Bergmann]

On Wed, Apr 10, 2019 at 6:26 PM 'Nick Desaulniers' via Clang Built
Linux <clang-bu...@googlegroups.com> wrote:
>
> On Mon, Apr 8, 2019 at 2:27 PM Arnd Bergmann <ar...@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

[Arnd Bergmann]

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

[Martin Schwidefsky]

Ok, done.

[Martin Schwidefsky]

Ok, committed with func_code=0.