2021-12-17 14:41:35

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 0/6] fix warning and errors on arm built with clang

Hi,

Can this patchset be applied to linux-4.19.y. I've tried to build an arm
kernel for these defconfigs:

mini2440_defconfig, mxs_defconfig, imx_v4_v5_defconfig,
integrator_defconfig, lpc32xx_defconfig, s3c2410_defconfig,
nhk8815_defconfig, imx_v6_v7_defconfig, at91_dt_defconfig,
shmobile_defconfig, omap1_defconfig, multi_v5_defconfig,
orion5x_defconfig, footbridge_defconfig, davinci_all_defconfig

Without this patchset these configs faild to build.
Also I fixed a few warnings.

There are still a few more warnings to fix.
But this is a start.

I built the kernel with tuxmake and this is the command:
tuxmake --runtime podman --target-arch arm --toolchain clang-nightly --kconfig tinyconfig LLVM=1 LLVM_IAS=0

Similar results with clang-13.


Patch "net: lan78xx: Avoid unnecessary self assignment" fixes:

drivers/net/usb/lan78xx.c:949:11: warning: explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Wself-assign]
offset = offset;
~~~~~~ ^ ~~~~~~
1 warning generated.


Patch "ARM: 8805/2: remove unneeded naked function usage" fixes:

arch/arm/mm/copypage-v4wb.c:47:9: error: parameter references not allowed in naked functions
: "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
^
/builds/linux/arch/arm/mm/copypage-v4wb.c:25:13: note: attribute is here
static void __naked
^
/builds/linux/include/linux/compiler_types.h:249:34: note: expanded from macro '__naked'
#define __naked __attribute__((naked)) notrace
^
1 error generated.


Patch "mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO" fixes:

drivers/net/wireless/marvell/mwifiex/cmdevt.c:219:22: warning: '(' and '{' tokens introducing statement expression appear in different macro expansion contexts [-Wcompound-token-split-by-macro]
host_cmd->seq_num = cpu_to_le16(HostCmd_SET_SEQ_NO_BSS_INFO
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builds/linux/include/linux/byteorder/generic.h:90:21: note: expanded from macro 'cpu_to_le16'
#define cpu_to_le16 __cpu_to_le16
^


Patch "Input: touchscreen - avoid bitwise vs logical OR warning" fixes:

drivers/input/touchscreen/of_touchscreen.c:80:17: warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical]
data_present = touchscreen_get_prop_u32(dev, "touchscreen-size-x",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Patch "ARM: 8788/1: ftrace: remove old mcount support" fixes:

arch/arm/kernel/entry-ftrace.S:56:2: error: Ftrace requires CONFIG_FRAME_POINTER=y with GCC older than 4.4.0.
#error Ftrace requires CONFIG_FRAME_POINTER=y with GCC older than 4.4.0.
^
1 error generated.


Patch "ARM: 8800/1: use choice for kernel unwinders" fixes the build
error:

clang: error: unknown argument: '-mapcs'
clang: error: unknown argument: '-mno-sched-prolog'


Cheers,
Anders

Nathan Chancellor (3):
net: lan78xx: Avoid unnecessary self assignment
mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO
Input: touchscreen - avoid bitwise vs logical OR warning

Nicolas Pitre (1):
ARM: 8805/2: remove unneeded naked function usage

Stefan Agner (2):
ARM: 8800/1: use choice for kernel unwinders
ARM: 8788/1: ftrace: remove old mcount support

arch/arm/Kconfig.debug | 45 +++++----
arch/arm/include/asm/ftrace.h | 3 -
arch/arm/kernel/armksyms.c | 3 -
arch/arm/kernel/entry-ftrace.S | 75 +-------------
arch/arm/kernel/ftrace.c | 51 ----------
arch/arm/mm/copypage-fa.c | 35 ++++---
arch/arm/mm/copypage-feroceon.c | 98 +++++++++----------
arch/arm/mm/copypage-v4mc.c | 19 ++--
arch/arm/mm/copypage-v4wb.c | 41 ++++----
arch/arm/mm/copypage-v4wt.c | 37 ++++---
arch/arm/mm/copypage-xsc3.c | 71 ++++++--------
arch/arm/mm/copypage-xscale.c | 71 +++++++-------
drivers/input/touchscreen/of_touchscreen.c | 18 ++--
drivers/net/usb/lan78xx.c | 6 +-
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 4 +-
drivers/net/wireless/marvell/mwifiex/fw.h | 8 +-
lib/Kconfig.debug | 6 +-
17 files changed, 228 insertions(+), 363 deletions(-)

--
2.34.1



2021-12-17 14:41:37

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 1/6] net: lan78xx: Avoid unnecessary self assignment

From: Nathan Chancellor <[email protected]>

commit 94e7c844990f0db92418586b107be135b4963b66 upstream.

Clang warns when a variable is assigned to itself.

drivers/net/usb/lan78xx.c:940:11: warning: explicitly assigning value of
variable of type 'u32' (aka 'unsigned int') to itself [-Wself-assign]
offset = offset;
~~~~~~ ^ ~~~~~~
1 warning generated.

Reorder the if statement to acheive the same result and avoid a self
assignment warning.

Link: https://github.com/ClangBuiltLinux/linux/issues/129
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
drivers/net/usb/lan78xx.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index b328207c0455..f438be83d259 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -945,11 +945,9 @@ static int lan78xx_read_otp(struct lan78xx_net *dev, u32 offset,
ret = lan78xx_read_raw_otp(dev, 0, 1, &sig);

if (ret == 0) {
- if (sig == OTP_INDICATOR_1)
- offset = offset;
- else if (sig == OTP_INDICATOR_2)
+ if (sig == OTP_INDICATOR_2)
offset += 0x100;
- else
+ else if (sig != OTP_INDICATOR_1)
ret = -EINVAL;
if (!ret)
ret = lan78xx_read_raw_otp(dev, offset, length, data);
--
2.34.1


2021-12-17 14:41:38

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 3/6] mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO

From: Nathan Chancellor <[email protected]>

commit 6a953dc4dbd1c7057fb765a24f37a5e953c85fb0 upstream.

A new warning in clang points out when macro expansion might result in a
GNU C statement expression. There is an instance of this in the mwifiex
driver:

drivers/net/wireless/marvell/mwifiex/cmdevt.c:217:34: warning: '}' and
')' tokens terminating statement expression appear in different macro
expansion contexts [-Wcompound-token-split-by-macro]
host_cmd->seq_num = cpu_to_le16(HostCmd_SET_SEQ_NO_BSS_INFO
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/marvell/mwifiex/fw.h:519:46: note: expanded from
macro 'HostCmd_SET_SEQ_NO_BSS_INFO'
(((type) & 0x000f) << 12); }
^

This does not appear to be a real issue. Removing the braces and
replacing them with parentheses will fix the warning and not change the
meaning of the code.

Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
Link: https://github.com/ClangBuiltLinux/linux/issues/1146
Reported-by: Andy Lavr <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 4 ++--
drivers/net/wireless/marvell/mwifiex/fw.h | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 60db2b969e20..b7ced103b814 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -324,9 +324,9 @@ static int mwifiex_dnld_sleep_confirm_cmd(struct mwifiex_adapter *adapter)

adapter->seq_num++;
sleep_cfm_buf->seq_num =
- cpu_to_le16((HostCmd_SET_SEQ_NO_BSS_INFO
+ cpu_to_le16(HostCmd_SET_SEQ_NO_BSS_INFO
(adapter->seq_num, priv->bss_num,
- priv->bss_type)));
+ priv->bss_type));

mwifiex_dbg(adapter, CMD,
"cmd: DNLD_CMD: %#x, act %#x, len %d, seqno %#x\n",
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 8b9d0809daf6..076ea1c4b921 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -512,10 +512,10 @@ enum mwifiex_channel_flags {

#define RF_ANTENNA_AUTO 0xFFFF

-#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) { \
- (((seq) & 0x00ff) | \
- (((num) & 0x000f) << 8)) | \
- (((type) & 0x000f) << 12); }
+#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) \
+ ((((seq) & 0x00ff) | \
+ (((num) & 0x000f) << 8)) | \
+ (((type) & 0x000f) << 12))

#define HostCmd_GET_SEQ_NO(seq) \
((seq) & HostCmd_SEQ_NUM_MASK)
--
2.34.1


2021-12-17 14:41:42

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 5/6] ARM: 8788/1: ftrace: remove old mcount support

From: Stefan Agner <[email protected]>

commit d3c61619568c88d48eccd5e74b4f84faa1440652 upstream.

Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6")
raised the minimum GCC version to 4.6. Old mcount is only required for
GCC versions older than 4.4.0. Hence old mcount support can be dropped
too.

Signed-off-by: Stefan Agner <[email protected]>
Signed-off-by: Russell King <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
arch/arm/Kconfig.debug | 5 ---
arch/arm/include/asm/ftrace.h | 3 --
arch/arm/kernel/armksyms.c | 3 --
arch/arm/kernel/entry-ftrace.S | 75 ++--------------------------------
arch/arm/kernel/ftrace.c | 51 -----------------------
5 files changed, 4 insertions(+), 133 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 01c760929c9e..9d363399cb35 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -82,11 +82,6 @@ config ARM_UNWIND
config FRAME_POINTER
bool

-config OLD_MCOUNT
- bool
- depends on FUNCTION_TRACER && FRAME_POINTER
- default y
-
config DEBUG_USER
bool "Verbose user fault messages"
help
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index faeb6b1c0089..15bd9af13497 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -16,9 +16,6 @@ extern void __gnu_mcount_nc(void);

#ifdef CONFIG_DYNAMIC_FTRACE
struct dyn_arch_ftrace {
-#ifdef CONFIG_OLD_MCOUNT
- bool old_mcount;
-#endif
#ifdef CONFIG_ARM_MODULE_PLTS
struct module *mod;
#endif
diff --git a/arch/arm/kernel/armksyms.c b/arch/arm/kernel/armksyms.c
index 783fbb4de5f9..8fa2dc21d332 100644
--- a/arch/arm/kernel/armksyms.c
+++ b/arch/arm/kernel/armksyms.c
@@ -167,9 +167,6 @@ EXPORT_SYMBOL(_find_next_bit_be);
#endif

#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_OLD_MCOUNT
-EXPORT_SYMBOL(mcount);
-#endif
EXPORT_SYMBOL(__gnu_mcount_nc);
#endif

diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index efcd9f25a14b..0be69e551a64 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -15,23 +15,8 @@
* start of every function. In mcount, apart from the function's address (in
* lr), we need to get hold of the function's caller's address.
*
- * Older GCCs (pre-4.4) inserted a call to a routine called mcount like this:
- *
- * bl mcount
- *
- * These versions have the limitation that in order for the mcount routine to
- * be able to determine the function's caller's address, an APCS-style frame
- * pointer (which is set up with something like the code below) is required.
- *
- * mov ip, sp
- * push {fp, ip, lr, pc}
- * sub fp, ip, #4
- *
- * With EABI, these frame pointers are not available unless -mapcs-frame is
- * specified, and if building as Thumb-2, not even then.
- *
- * Newer GCCs (4.4+) solve this problem by introducing a new version of mcount,
- * with call sites like:
+ * Newer GCCs (4.4+) solve this problem by using a version of mcount with call
+ * sites like:
*
* push {lr}
* bl __gnu_mcount_nc
@@ -46,17 +31,10 @@
* allows it to be clobbered in subroutines and doesn't use it to hold
* parameters.)
*
- * When using dynamic ftrace, we patch out the mcount call by a "mov r0, r0"
- * for the mcount case, and a "pop {lr}" for the __gnu_mcount_nc case (see
- * arch/arm/kernel/ftrace.c).
+ * When using dynamic ftrace, we patch out the mcount call by a "pop {lr}"
+ * instead of the __gnu_mcount_nc call (see arch/arm/kernel/ftrace.c).
*/

-#ifndef CONFIG_OLD_MCOUNT
-#if (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4))
-#error Ftrace requires CONFIG_FRAME_POINTER=y with GCC older than 4.4.0.
-#endif
-#endif
-
.macro mcount_adjust_addr rd, rn
bic \rd, \rn, #1 @ clear the Thumb bit if present
sub \rd, \rd, #MCOUNT_INSN_SIZE
@@ -209,51 +187,6 @@ ftrace_graph_call\suffix:
mcount_exit
.endm

-#ifdef CONFIG_OLD_MCOUNT
-/*
- * mcount
- */
-
-.macro mcount_enter
- stmdb sp!, {r0-r3, lr}
-.endm
-
-.macro mcount_get_lr reg
- ldr \reg, [fp, #-4]
-.endm
-
-.macro mcount_exit
- ldr lr, [fp, #-4]
- ldmia sp!, {r0-r3, pc}
-.endm
-
-ENTRY(mcount)
-#ifdef CONFIG_DYNAMIC_FTRACE
- stmdb sp!, {lr}
- ldr lr, [fp, #-4]
- ldmia sp!, {pc}
-#else
- __mcount _old
-#endif
-ENDPROC(mcount)
-
-#ifdef CONFIG_DYNAMIC_FTRACE
-ENTRY(ftrace_caller_old)
- __ftrace_caller _old
-ENDPROC(ftrace_caller_old)
-#endif
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-ENTRY(ftrace_graph_caller_old)
- __ftrace_graph_caller
-ENDPROC(ftrace_graph_caller_old)
-#endif
-
-.purgem mcount_enter
-.purgem mcount_get_lr
-.purgem mcount_exit
-#endif
-
/*
* __gnu_mcount_nc
*/
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 51839250e49a..12b6da56f88d 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -47,30 +47,6 @@ void arch_ftrace_update_code(int command)
stop_machine(__ftrace_modify_code, &command, NULL);
}

-#ifdef CONFIG_OLD_MCOUNT
-#define OLD_MCOUNT_ADDR ((unsigned long) mcount)
-#define OLD_FTRACE_ADDR ((unsigned long) ftrace_caller_old)
-
-#define OLD_NOP 0xe1a00000 /* mov r0, r0 */
-
-static unsigned long ftrace_nop_replace(struct dyn_ftrace *rec)
-{
- return rec->arch.old_mcount ? OLD_NOP : NOP;
-}
-
-static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
-{
- if (!rec->arch.old_mcount)
- return addr;
-
- if (addr == MCOUNT_ADDR)
- addr = OLD_MCOUNT_ADDR;
- else if (addr == FTRACE_ADDR)
- addr = OLD_FTRACE_ADDR;
-
- return addr;
-}
-#else
static unsigned long ftrace_nop_replace(struct dyn_ftrace *rec)
{
return NOP;
@@ -80,7 +56,6 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
{
return addr;
}
-#endif

int ftrace_arch_code_modify_prepare(void)
{
@@ -151,15 +126,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
}
#endif

-#ifdef CONFIG_OLD_MCOUNT
- if (!ret) {
- pc = (unsigned long)&ftrace_call_old;
- new = ftrace_call_replace(pc, (unsigned long)func, true);
-
- ret = ftrace_modify_code(pc, 0, new, false);
- }
-#endif
-
return ret;
}

@@ -233,16 +199,6 @@ int ftrace_make_nop(struct module *mod,
new = ftrace_nop_replace(rec);
ret = ftrace_modify_code(ip, old, new, true);

-#ifdef CONFIG_OLD_MCOUNT
- if (ret == -EINVAL && addr == MCOUNT_ADDR) {
- rec->arch.old_mcount = true;
-
- old = ftrace_call_replace(ip, adjust_address(rec, addr), true);
- new = ftrace_nop_replace(rec);
- ret = ftrace_modify_code(ip, old, new, true);
- }
-#endif
-
return ret;
}

@@ -305,13 +261,6 @@ static int ftrace_modify_graph_caller(bool enable)
#endif


-#ifdef CONFIG_OLD_MCOUNT
- if (!ret)
- ret = __ftrace_modify_caller(&ftrace_graph_call_old,
- ftrace_graph_caller_old,
- enable);
-#endif
-
return ret;
}

--
2.34.1


2021-12-17 14:41:47

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 4/6] ARM: 8800/1: use choice for kernel unwinders

From: Stefan Agner <[email protected]>

commit f9b58e8c7d031b0daa5c9a9ee27f5a4028ba53ac upstream.

While in theory multiple unwinders could be compiled in, it does
not make sense in practise. Use a choice to make the unwinder
selection mutually exclusive and mandatory.

Already before this commit it has not been possible to deselect
FRAME_POINTER. Remove the obsolete comment.

Furthermore, to produce a meaningful backtrace with FRAME_POINTER
enabled the kernel needs a specific function prologue:
mov ip, sp
stmfd sp!, {fp, ip, lr, pc}
sub fp, ip, #4

To get to the required prologue gcc uses apcs and no-sched-prolog.
This compiler options are not available on clang, and clang is not
able to generate the required prologue. Make the FRAME_POINTER
config symbol depending on !clang.

Suggested-by: Arnd Bergmann <[email protected]>
Signed-off-by: Stefan Agner <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Russell King <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
arch/arm/Kconfig.debug | 44 +++++++++++++++++++++++++++---------------
lib/Kconfig.debug | 6 +++---
2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index bee0ba1d1cfb..01c760929c9e 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -45,30 +45,42 @@ config DEBUG_WX

If in doubt, say "Y".

-# RMK wants arm kernels compiled with frame pointers or stack unwinding.
-# If you know what you are doing and are willing to live without stack
-# traces, you can get a slightly smaller kernel by setting this option to
-# n, but then RMK will have to kill you ;).
-config FRAME_POINTER
- bool
- depends on !THUMB2_KERNEL
- default y if !ARM_UNWIND || FUNCTION_GRAPH_TRACER
+choice
+ prompt "Choose kernel unwinder"
+ default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER
+ default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER
+ help
+ This determines which method will be used for unwinding kernel stack
+ traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack,
+ livepatch, lockdep, and more.
+
+config UNWINDER_FRAME_POINTER
+ bool "Frame pointer unwinder"
+ depends on !THUMB2_KERNEL && !CC_IS_CLANG
+ select ARCH_WANT_FRAME_POINTERS
+ select FRAME_POINTER
help
- If you say N here, the resulting kernel will be slightly smaller and
- faster. However, if neither FRAME_POINTER nor ARM_UNWIND are enabled,
- when a problem occurs with the kernel, the information that is
- reported is severely limited.
+ This option enables the frame pointer unwinder for unwinding
+ kernel stack traces.

-config ARM_UNWIND
- bool "Enable stack unwinding support (EXPERIMENTAL)"
+config UNWINDER_ARM
+ bool "ARM EABI stack unwinder"
depends on AEABI
- default y
+ select ARM_UNWIND
help
This option enables stack unwinding support in the kernel
using the information automatically generated by the
compiler. The resulting kernel image is slightly bigger but
the performance is not affected. Currently, this feature
- only works with EABI compilers. If unsure say Y.
+ only works with EABI compilers.
+
+endchoice
+
+config ARM_UNWIND
+ bool
+
+config FRAME_POINTER
+ bool

config OLD_MCOUNT
bool
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 6970759f296c..621859a453f8 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1178,7 +1178,7 @@ config LOCKDEP
bool
depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
select STACKTRACE
- select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !X86
+ select FRAME_POINTER if !MIPS && !PPC && !ARM && !S390 && !MICROBLAZE && !ARC && !X86
select KALLSYMS
select KALLSYMS_ALL

@@ -1589,7 +1589,7 @@ config FAULT_INJECTION_STACKTRACE_FILTER
depends on FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT
depends on !X86_64
select STACKTRACE
- select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM_UNWIND && !ARC && !X86
+ select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86
help
Provide stacktrace filter for fault-injection capabilities

@@ -1598,7 +1598,7 @@ config LATENCYTOP
depends on DEBUG_KERNEL
depends on STACKTRACE_SUPPORT
depends on PROC_FS
- select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM_UNWIND && !ARC && !X86
+ select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86
select KALLSYMS
select KALLSYMS_ALL
select STACKTRACE
--
2.34.1


2021-12-17 14:41:48

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 2/6] ARM: 8805/2: remove unneeded naked function usage

From: Nicolas Pitre <[email protected]>

commit b99afae1390140f5b0039e6b37a7380de31ae874 upstream.

The naked attribute is known to confuse some old gcc versions when
function arguments aren't explicitly listed as inline assembly operands
despite the gcc documentation. That resulted in commit 9a40ac86152c
("ARM: 6164/1: Add kto and kfrom to input operands list.").

Yet that commit has problems of its own by having assembly operand
constraints completely wrong. If the generated code has been OK since
then, it is due to luck rather than correctness. So this patch also
provides proper assembly operand constraints, and removes two instances
of redundant register usages in the implementation while at it.

Inspection of the generated code with this patch doesn't show any
obvious quality degradation either, so not relying on __naked at all
will make the code less fragile, and avoid some issues with clang.

The only remaining __naked instances (excluding the kprobes test cases)
are exynos_pm_power_up_setup(), tc2_pm_power_up_setup() and

cci_enable_port_for_self(. But in the first two cases, only the function
address is used by the compiler with no chance of inlining it by
mistake, and the third case is called from assembly code only. And the
fact that no stack is available when the corresponding code is executed
does warrant the __naked usage in those cases.

Signed-off-by: Nicolas Pitre <[email protected]>
Reviewed-by: Stefan Agner <[email protected]>
Tested-by: Stefan Agner <[email protected]>
Signed-off-by: Russell King <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
arch/arm/mm/copypage-fa.c | 35 ++++++------
arch/arm/mm/copypage-feroceon.c | 98 ++++++++++++++++-----------------
arch/arm/mm/copypage-v4mc.c | 19 +++----
arch/arm/mm/copypage-v4wb.c | 41 +++++++-------
arch/arm/mm/copypage-v4wt.c | 37 ++++++-------
arch/arm/mm/copypage-xsc3.c | 71 +++++++++++-------------
arch/arm/mm/copypage-xscale.c | 71 ++++++++++++------------
7 files changed, 178 insertions(+), 194 deletions(-)

diff --git a/arch/arm/mm/copypage-fa.c b/arch/arm/mm/copypage-fa.c
index d130a5ece5d5..bf24690ec83a 100644
--- a/arch/arm/mm/copypage-fa.c
+++ b/arch/arm/mm/copypage-fa.c
@@ -17,26 +17,25 @@
/*
* Faraday optimised copy_user_page
*/
-static void __naked
-fa_copy_user_page(void *kto, const void *kfrom)
+static void fa_copy_user_page(void *kto, const void *kfrom)
{
- asm("\
- stmfd sp!, {r4, lr} @ 2\n\
- mov r2, %0 @ 1\n\
-1: ldmia r1!, {r3, r4, ip, lr} @ 4\n\
- stmia r0, {r3, r4, ip, lr} @ 4\n\
- mcr p15, 0, r0, c7, c14, 1 @ 1 clean and invalidate D line\n\
- add r0, r0, #16 @ 1\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
- stmia r0, {r3, r4, ip, lr} @ 4\n\
- mcr p15, 0, r0, c7, c14, 1 @ 1 clean and invalidate D line\n\
- add r0, r0, #16 @ 1\n\
- subs r2, r2, #1 @ 1\n\
+ int tmp;
+
+ asm volatile ("\
+1: ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+ stmia %0, {r3, r4, ip, lr} @ 4\n\
+ mcr p15, 0, %0, c7, c14, 1 @ 1 clean and invalidate D line\n\
+ add %0, %0, #16 @ 1\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+ stmia %0, {r3, r4, ip, lr} @ 4\n\
+ mcr p15, 0, %0, c7, c14, 1 @ 1 clean and invalidate D line\n\
+ add %0, %0, #16 @ 1\n\
+ subs %2, %2, #1 @ 1\n\
bne 1b @ 1\n\
- mcr p15, 0, r2, c7, c10, 4 @ 1 drain WB\n\
- ldmfd sp!, {r4, pc} @ 3"
- :
- : "I" (PAGE_SIZE / 32));
+ mcr p15, 0, %2, c7, c10, 4 @ 1 drain WB"
+ : "+&r" (kto), "+&r" (kfrom), "=&r" (tmp)
+ : "2" (PAGE_SIZE / 32)
+ : "r3", "r4", "ip", "lr");
}

void fa_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-feroceon.c b/arch/arm/mm/copypage-feroceon.c
index 49ee0c1a7209..cc819732d9b8 100644
--- a/arch/arm/mm/copypage-feroceon.c
+++ b/arch/arm/mm/copypage-feroceon.c
@@ -13,58 +13,56 @@
#include <linux/init.h>
#include <linux/highmem.h>

-static void __naked
-feroceon_copy_user_page(void *kto, const void *kfrom)
+static void feroceon_copy_user_page(void *kto, const void *kfrom)
{
- asm("\
- stmfd sp!, {r4-r9, lr} \n\
- mov ip, %2 \n\
-1: mov lr, r1 \n\
- ldmia r1!, {r2 - r9} \n\
- pld [lr, #32] \n\
- pld [lr, #64] \n\
- pld [lr, #96] \n\
- pld [lr, #128] \n\
- pld [lr, #160] \n\
- pld [lr, #192] \n\
- pld [lr, #224] \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- ldmia r1!, {r2 - r9} \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
- stmia r0, {r2 - r9} \n\
- subs ip, ip, #(32 * 8) \n\
- mcr p15, 0, r0, c7, c14, 1 @ clean and invalidate D line\n\
- add r0, r0, #32 \n\
+ int tmp;
+
+ asm volatile ("\
+1: ldmia %1!, {r2 - r7, ip, lr} \n\
+ pld [%1, #0] \n\
+ pld [%1, #32] \n\
+ pld [%1, #64] \n\
+ pld [%1, #96] \n\
+ pld [%1, #128] \n\
+ pld [%1, #160] \n\
+ pld [%1, #192] \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ ldmia %1!, {r2 - r7, ip, lr} \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
+ stmia %0, {r2 - r7, ip, lr} \n\
+ subs %2, %2, #(32 * 8) \n\
+ mcr p15, 0, %0, c7, c14, 1 @ clean and invalidate D line\n\
+ add %0, %0, #32 \n\
bne 1b \n\
- mcr p15, 0, ip, c7, c10, 4 @ drain WB\n\
- ldmfd sp!, {r4-r9, pc}"
- :
- : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE));
+ mcr p15, 0, %2, c7, c10, 4 @ drain WB"
+ : "+&r" (kto), "+&r" (kfrom), "=&r" (tmp)
+ : "2" (PAGE_SIZE)
+ : "r2", "r3", "r4", "r5", "r6", "r7", "ip", "lr");
}

void feroceon_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-v4mc.c b/arch/arm/mm/copypage-v4mc.c
index 0224416cba3c..b03202cddddb 100644
--- a/arch/arm/mm/copypage-v4mc.c
+++ b/arch/arm/mm/copypage-v4mc.c
@@ -40,12 +40,11 @@ static DEFINE_RAW_SPINLOCK(minicache_lock);
* instruction. If your processor does not supply this, you have to write your
* own copy_user_highpage that does the right thing.
*/
-static void __naked
-mc_copy_user_page(void *from, void *to)
+static void mc_copy_user_page(void *from, void *to)
{
- asm volatile(
- "stmfd sp!, {r4, lr} @ 2\n\
- mov r4, %2 @ 1\n\
+ int tmp;
+
+ asm volatile ("\
ldmia %0!, {r2, r3, ip, lr} @ 4\n\
1: mcr p15, 0, %1, c7, c6, 1 @ 1 invalidate D line\n\
stmia %1!, {r2, r3, ip, lr} @ 4\n\
@@ -55,13 +54,13 @@ mc_copy_user_page(void *from, void *to)
mcr p15, 0, %1, c7, c6, 1 @ 1 invalidate D line\n\
stmia %1!, {r2, r3, ip, lr} @ 4\n\
ldmia %0!, {r2, r3, ip, lr} @ 4\n\
- subs r4, r4, #1 @ 1\n\
+ subs %2, %2, #1 @ 1\n\
stmia %1!, {r2, r3, ip, lr} @ 4\n\
ldmneia %0!, {r2, r3, ip, lr} @ 4\n\
- bne 1b @ 1\n\
- ldmfd sp!, {r4, pc} @ 3"
- :
- : "r" (from), "r" (to), "I" (PAGE_SIZE / 64));
+ bne 1b @ "
+ : "+&r" (from), "+&r" (to), "=&r" (tmp)
+ : "2" (PAGE_SIZE / 64)
+ : "r2", "r3", "ip", "lr");
}

void v4_mc_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-v4wb.c b/arch/arm/mm/copypage-v4wb.c
index 067d0fdd630c..cd3e165afeed 100644
--- a/arch/arm/mm/copypage-v4wb.c
+++ b/arch/arm/mm/copypage-v4wb.c
@@ -22,29 +22,28 @@
* instruction. If your processor does not supply this, you have to write your
* own copy_user_highpage that does the right thing.
*/
-static void __naked
-v4wb_copy_user_page(void *kto, const void *kfrom)
+static void v4wb_copy_user_page(void *kto, const void *kfrom)
{
- asm("\
- stmfd sp!, {r4, lr} @ 2\n\
- mov r2, %2 @ 1\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
-1: mcr p15, 0, r0, c7, c6, 1 @ 1 invalidate D line\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4+1\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
- mcr p15, 0, r0, c7, c6, 1 @ 1 invalidate D line\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
- subs r2, r2, #1 @ 1\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmneia r1!, {r3, r4, ip, lr} @ 4\n\
+ int tmp;
+
+ asm volatile ("\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+1: mcr p15, 0, %0, c7, c6, 1 @ 1 invalidate D line\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4+1\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+ mcr p15, 0, %0, c7, c6, 1 @ 1 invalidate D line\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+ subs %2, %2, #1 @ 1\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmneia %1!, {r3, r4, ip, lr} @ 4\n\
bne 1b @ 1\n\
- mcr p15, 0, r1, c7, c10, 4 @ 1 drain WB\n\
- ldmfd sp!, {r4, pc} @ 3"
- :
- : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
+ mcr p15, 0, %1, c7, c10, 4 @ 1 drain WB"
+ : "+&r" (kto), "+&r" (kfrom), "=&r" (tmp)
+ : "2" (PAGE_SIZE / 64)
+ : "r3", "r4", "ip", "lr");
}

void v4wb_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-v4wt.c b/arch/arm/mm/copypage-v4wt.c
index b85c5da2e510..8614572e1296 100644
--- a/arch/arm/mm/copypage-v4wt.c
+++ b/arch/arm/mm/copypage-v4wt.c
@@ -20,27 +20,26 @@
* dirty data in the cache. However, we do have to ensure that
* subsequent reads are up to date.
*/
-static void __naked
-v4wt_copy_user_page(void *kto, const void *kfrom)
+static void v4wt_copy_user_page(void *kto, const void *kfrom)
{
- asm("\
- stmfd sp!, {r4, lr} @ 2\n\
- mov r2, %2 @ 1\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
-1: stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4+1\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmia r1!, {r3, r4, ip, lr} @ 4\n\
- subs r2, r2, #1 @ 1\n\
- stmia r0!, {r3, r4, ip, lr} @ 4\n\
- ldmneia r1!, {r3, r4, ip, lr} @ 4\n\
+ int tmp;
+
+ asm volatile ("\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+1: stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4+1\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmia %1!, {r3, r4, ip, lr} @ 4\n\
+ subs %2, %2, #1 @ 1\n\
+ stmia %0!, {r3, r4, ip, lr} @ 4\n\
+ ldmneia %1!, {r3, r4, ip, lr} @ 4\n\
bne 1b @ 1\n\
- mcr p15, 0, r2, c7, c7, 0 @ flush ID cache\n\
- ldmfd sp!, {r4, pc} @ 3"
- :
- : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64));
+ mcr p15, 0, %2, c7, c7, 0 @ flush ID cache"
+ : "+&r" (kto), "+&r" (kfrom), "=&r" (tmp)
+ : "2" (PAGE_SIZE / 64)
+ : "r3", "r4", "ip", "lr");
}

void v4wt_copy_user_highpage(struct page *to, struct page *from,
diff --git a/arch/arm/mm/copypage-xsc3.c b/arch/arm/mm/copypage-xsc3.c
index 03a2042aced5..55cbc3a89d85 100644
--- a/arch/arm/mm/copypage-xsc3.c
+++ b/arch/arm/mm/copypage-xsc3.c
@@ -21,53 +21,46 @@

/*
* XSC3 optimised copy_user_highpage
- * r0 = destination
- * r1 = source
*
* The source page may have some clean entries in the cache already, but we
* can safely ignore them - break_cow() will flush them out of the cache
* if we eventually end up using our copied page.
*
*/
-static void __naked
-xsc3_mc_copy_user_page(void *kto, const void *kfrom)
+static void xsc3_mc_copy_user_page(void *kto, const void *kfrom)
{
- asm("\
- stmfd sp!, {r4, r5, lr} \n\
- mov lr, %2 \n\
- \n\
- pld [r1, #0] \n\
- pld [r1, #32] \n\
-1: pld [r1, #64] \n\
- pld [r1, #96] \n\
+ int tmp;
+
+ asm volatile ("\
+ pld [%1, #0] \n\
+ pld [%1, #32] \n\
+1: pld [%1, #64] \n\
+ pld [%1, #96] \n\
\n\
-2: ldrd r2, [r1], #8 \n\
- mov ip, r0 \n\
- ldrd r4, [r1], #8 \n\
- mcr p15, 0, ip, c7, c6, 1 @ invalidate\n\
- strd r2, [r0], #8 \n\
- ldrd r2, [r1], #8 \n\
- strd r4, [r0], #8 \n\
- ldrd r4, [r1], #8 \n\
- strd r2, [r0], #8 \n\
- strd r4, [r0], #8 \n\
- ldrd r2, [r1], #8 \n\
- mov ip, r0 \n\
- ldrd r4, [r1], #8 \n\
- mcr p15, 0, ip, c7, c6, 1 @ invalidate\n\
- strd r2, [r0], #8 \n\
- ldrd r2, [r1], #8 \n\
- subs lr, lr, #1 \n\
- strd r4, [r0], #8 \n\
- ldrd r4, [r1], #8 \n\
- strd r2, [r0], #8 \n\
- strd r4, [r0], #8 \n\
+2: ldrd r2, [%1], #8 \n\
+ ldrd r4, [%1], #8 \n\
+ mcr p15, 0, %0, c7, c6, 1 @ invalidate\n\
+ strd r2, [%0], #8 \n\
+ ldrd r2, [%1], #8 \n\
+ strd r4, [%0], #8 \n\
+ ldrd r4, [%1], #8 \n\
+ strd r2, [%0], #8 \n\
+ strd r4, [%0], #8 \n\
+ ldrd r2, [%1], #8 \n\
+ ldrd r4, [%1], #8 \n\
+ mcr p15, 0, %0, c7, c6, 1 @ invalidate\n\
+ strd r2, [%0], #8 \n\
+ ldrd r2, [%1], #8 \n\
+ subs %2, %2, #1 \n\
+ strd r4, [%0], #8 \n\
+ ldrd r4, [%1], #8 \n\
+ strd r2, [%0], #8 \n\
+ strd r4, [%0], #8 \n\
bgt 1b \n\
- beq 2b \n\
- \n\
- ldmfd sp!, {r4, r5, pc}"
- :
- : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 64 - 1));
+ beq 2b "
+ : "+&r" (kto), "+&r" (kfrom), "=&r" (tmp)
+ : "2" (PAGE_SIZE / 64 - 1)
+ : "r2", "r3", "r4", "r5");
}

void xsc3_mc_copy_user_highpage(struct page *to, struct page *from,
@@ -85,8 +78,6 @@ void xsc3_mc_copy_user_highpage(struct page *to, struct page *from,

/*
* XScale optimised clear_user_page
- * r0 = destination
- * r1 = virtual user address of ultimate destination page
*/
void xsc3_mc_clear_user_highpage(struct page *page, unsigned long vaddr)
{
diff --git a/arch/arm/mm/copypage-xscale.c b/arch/arm/mm/copypage-xscale.c
index 97972379f4d6..b0ae8c7acb48 100644
--- a/arch/arm/mm/copypage-xscale.c
+++ b/arch/arm/mm/copypage-xscale.c
@@ -36,52 +36,51 @@ static DEFINE_RAW_SPINLOCK(minicache_lock);
* Dcache aliasing issue. The writes will be forwarded to the write buffer,
* and merged as appropriate.
*/
-static void __naked
-mc_copy_user_page(void *from, void *to)
+static void mc_copy_user_page(void *from, void *to)
{
+ int tmp;
+
/*
* Strangely enough, best performance is achieved
* when prefetching destination as well. (NP)
*/
- asm volatile(
- "stmfd sp!, {r4, r5, lr} \n\
- mov lr, %2 \n\
- pld [r0, #0] \n\
- pld [r0, #32] \n\
- pld [r1, #0] \n\
- pld [r1, #32] \n\
-1: pld [r0, #64] \n\
- pld [r0, #96] \n\
- pld [r1, #64] \n\
- pld [r1, #96] \n\
-2: ldrd r2, [r0], #8 \n\
- ldrd r4, [r0], #8 \n\
- mov ip, r1 \n\
- strd r2, [r1], #8 \n\
- ldrd r2, [r0], #8 \n\
- strd r4, [r1], #8 \n\
- ldrd r4, [r0], #8 \n\
- strd r2, [r1], #8 \n\
- strd r4, [r1], #8 \n\
+ asm volatile ("\
+ pld [%0, #0] \n\
+ pld [%0, #32] \n\
+ pld [%1, #0] \n\
+ pld [%1, #32] \n\
+1: pld [%0, #64] \n\
+ pld [%0, #96] \n\
+ pld [%1, #64] \n\
+ pld [%1, #96] \n\
+2: ldrd r2, [%0], #8 \n\
+ ldrd r4, [%0], #8 \n\
+ mov ip, %1 \n\
+ strd r2, [%1], #8 \n\
+ ldrd r2, [%0], #8 \n\
+ strd r4, [%1], #8 \n\
+ ldrd r4, [%0], #8 \n\
+ strd r2, [%1], #8 \n\
+ strd r4, [%1], #8 \n\
mcr p15, 0, ip, c7, c10, 1 @ clean D line\n\
- ldrd r2, [r0], #8 \n\
+ ldrd r2, [%0], #8 \n\
mcr p15, 0, ip, c7, c6, 1 @ invalidate D line\n\
- ldrd r4, [r0], #8 \n\
- mov ip, r1 \n\
- strd r2, [r1], #8 \n\
- ldrd r2, [r0], #8 \n\
- strd r4, [r1], #8 \n\
- ldrd r4, [r0], #8 \n\
- strd r2, [r1], #8 \n\
- strd r4, [r1], #8 \n\
+ ldrd r4, [%0], #8 \n\
+ mov ip, %1 \n\
+ strd r2, [%1], #8 \n\
+ ldrd r2, [%0], #8 \n\
+ strd r4, [%1], #8 \n\
+ ldrd r4, [%0], #8 \n\
+ strd r2, [%1], #8 \n\
+ strd r4, [%1], #8 \n\
mcr p15, 0, ip, c7, c10, 1 @ clean D line\n\
- subs lr, lr, #1 \n\
+ subs %2, %2, #1 \n\
mcr p15, 0, ip, c7, c6, 1 @ invalidate D line\n\
bgt 1b \n\
- beq 2b \n\
- ldmfd sp!, {r4, r5, pc} "
- :
- : "r" (from), "r" (to), "I" (PAGE_SIZE / 64 - 1));
+ beq 2b "
+ : "+&r" (from), "+&r" (to), "=&r" (tmp)
+ : "2" (PAGE_SIZE / 64 - 1)
+ : "r2", "r3", "r4", "r5", "ip");
}

void xscale_mc_copy_user_highpage(struct page *to, struct page *from,
--
2.34.1


2021-12-17 14:41:56

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 4.19 6/6] Input: touchscreen - avoid bitwise vs logical OR warning

From: Nathan Chancellor <[email protected]>

commit a02dcde595f7cbd240ccd64de96034ad91cffc40 upstream.

A new warning in clang points out a few places in this driver where a
bitwise OR is being used with boolean types:

drivers/input/touchscreen.c:81:17: warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical]
data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This use of a bitwise OR is intentional, as bitwise operations do not
short circuit, which allows all the calls to touchscreen_get_prop_u32()
to happen so that the last parameter is initialized while coalescing the
results of the calls to make a decision after they are all evaluated.

To make this clearer to the compiler, use the '|=' operator to assign
the result of each touchscreen_get_prop_u32() call to data_present,
which keeps the meaning of the code the same but makes it obvious that
every one of these calls is expected to happen.

Signed-off-by: Nathan Chancellor <[email protected]>
Reported-by: Nick Desaulniers <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
drivers/input/touchscreen/of_touchscreen.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index 9642f103b726..b9baad7d34a7 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -80,8 +80,8 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
data_present = touchscreen_get_prop_u32(dev, "touchscreen-size-x",
input_abs_get_max(input,
axis) + 1,
- &maximum) |
- touchscreen_get_prop_u32(dev, "touchscreen-fuzz-x",
+ &maximum);
+ data_present |= touchscreen_get_prop_u32(dev, "touchscreen-fuzz-x",
input_abs_get_fuzz(input, axis),
&fuzz);
if (data_present)
@@ -91,8 +91,8 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
data_present = touchscreen_get_prop_u32(dev, "touchscreen-size-y",
input_abs_get_max(input,
axis) + 1,
- &maximum) |
- touchscreen_get_prop_u32(dev, "touchscreen-fuzz-y",
+ &maximum);
+ data_present |= touchscreen_get_prop_u32(dev, "touchscreen-fuzz-y",
input_abs_get_fuzz(input, axis),
&fuzz);
if (data_present)
@@ -102,11 +102,11 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
data_present = touchscreen_get_prop_u32(dev,
"touchscreen-max-pressure",
input_abs_get_max(input, axis),
- &maximum) |
- touchscreen_get_prop_u32(dev,
- "touchscreen-fuzz-pressure",
- input_abs_get_fuzz(input, axis),
- &fuzz);
+ &maximum);
+ data_present |= touchscreen_get_prop_u32(dev,
+ "touchscreen-fuzz-pressure",
+ input_abs_get_fuzz(input, axis),
+ &fuzz);
if (data_present)
touchscreen_set_params(input, axis, maximum, fuzz);

--
2.34.1


2021-12-20 10:56:44

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 4.19 3/6] mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO

On Fri, Dec 17, 2021 at 03:41:16PM +0100, Anders Roxell wrote:
> From: Nathan Chancellor <[email protected]>
>
> commit 6a953dc4dbd1c7057fb765a24f37a5e953c85fb0 upstream.
>
> A new warning in clang points out when macro expansion might result in a
> GNU C statement expression. There is an instance of this in the mwifiex
> driver:
>
> drivers/net/wireless/marvell/mwifiex/cmdevt.c:217:34: warning: '}' and
> ')' tokens terminating statement expression appear in different macro
> expansion contexts [-Wcompound-token-split-by-macro]
> host_cmd->seq_num = cpu_to_le16(HostCmd_SET_SEQ_NO_BSS_INFO
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/marvell/mwifiex/fw.h:519:46: note: expanded from
> macro 'HostCmd_SET_SEQ_NO_BSS_INFO'
> (((type) & 0x000f) << 12); }
> ^
>
> This does not appear to be a real issue. Removing the braces and
> replacing them with parentheses will fix the warning and not change the
> meaning of the code.
>
> Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1146
> Reported-by: Andy Lavr <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>
> ---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 4 ++--
> drivers/net/wireless/marvell/mwifiex/fw.h | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)

Also needed in 5.4.y, right?

2021-12-20 10:57:50

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 4.19 5/6] ARM: 8788/1: ftrace: remove old mcount support

On Fri, Dec 17, 2021 at 03:41:18PM +0100, Anders Roxell wrote:
> From: Stefan Agner <[email protected]>
>
> commit d3c61619568c88d48eccd5e74b4f84faa1440652 upstream.
>
> Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6")
> raised the minimum GCC version to 4.6. Old mcount is only required for
> GCC versions older than 4.4.0. Hence old mcount support can be dropped
> too.
>
> Signed-off-by: Stefan Agner <[email protected]>
> Signed-off-by: Russell King <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>

Why is this needed for clang builds in 4.19?


2021-12-20 10:59:41

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 4.19 6/6] Input: touchscreen - avoid bitwise vs logical OR warning

On Fri, Dec 17, 2021 at 03:41:19PM +0100, Anders Roxell wrote:
> From: Nathan Chancellor <[email protected]>
>
> commit a02dcde595f7cbd240ccd64de96034ad91cffc40 upstream.
>
> A new warning in clang points out a few places in this driver where a
> bitwise OR is being used with boolean types:
>
> drivers/input/touchscreen.c:81:17: warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical]
> data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This use of a bitwise OR is intentional, as bitwise operations do not
> short circuit, which allows all the calls to touchscreen_get_prop_u32()
> to happen so that the last parameter is initialized while coalescing the
> results of the calls to make a decision after they are all evaluated.
>
> To make this clearer to the compiler, use the '|=' operator to assign
> the result of each touchscreen_get_prop_u32() call to data_present,
> which keeps the meaning of the code the same but makes it obvious that
> every one of these calls is expected to happen.
>
> Signed-off-by: Nathan Chancellor <[email protected]>
> Reported-by: Nick Desaulniers <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Dmitry Torokhov <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>
> ---
> drivers/input/touchscreen/of_touchscreen.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)

Also needed in 5.10.y and 5.4.y.

Please be more careful next time.

2021-12-20 12:13:38

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 4.19 3/6] mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO

On Fri, 2021-12-17 at 15:41 +0100, Anders Roxell wrote:
> From: Nathan Chancellor <[email protected]>
>
> commit 6a953dc4dbd1c7057fb765a24f37a5e953c85fb0 upstream.
>
> A new warning in clang points out when macro expansion might result in a
> GNU C statement expression. There is an instance of this in the mwifiex
> driver:
>
> drivers/net/wireless/marvell/mwifiex/cmdevt.c:217:34: warning: '}' and
> ')' tokens terminating statement expression appear in different macro
> expansion contexts [-Wcompound-token-split-by-macro]
> host_cmd->seq_num = cpu_to_le16(HostCmd_SET_SEQ_NO_BSS_INFO
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
[]
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
[]
> @@ -512,10 +512,10 @@ enum mwifiex_channel_flags {
>
> #define RF_ANTENNA_AUTO 0xFFFF
>
> -#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) { \
> - (((seq) & 0x00ff) | \
> - (((num) & 0x000f) << 8)) | \
> - (((type) & 0x000f) << 12); }
> +#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) \
> + ((((seq) & 0x00ff) | \
> + (((num) & 0x000f) << 8)) | \
> + (((type) & 0x000f) << 12))

Perhaps this would be better as a static inline

static inline u16 HostCmd_SET_SEQ_NO_BSS_INFO(u16 seq, u8 num, u8 type)
{
return (type & 0x000f) << 12 | (num & 0x000f) << 8 | (seq & 0x00ff);
}



2021-12-20 14:00:47

by David Laight

[permalink] [raw]
Subject: RE: [PATCH 4.19 3/6] mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO

From: Joe Perches
> Sent: 20 December 2021 12:13
>
> On Fri, 2021-12-17 at 15:41 +0100, Anders Roxell wrote:
> > From: Nathan Chancellor <[email protected]>
> >
> > commit 6a953dc4dbd1c7057fb765a24f37a5e953c85fb0 upstream.
> >
> > A new warning in clang points out when macro expansion might result in a
> > GNU C statement expression. There is an instance of this in the mwifiex
> > driver:
> >
> > drivers/net/wireless/marvell/mwifiex/cmdevt.c:217:34: warning: '}' and
> > ')' tokens terminating statement expression appear in different macro
> > expansion contexts [-Wcompound-token-split-by-macro]
> > host_cmd->seq_num = cpu_to_le16(HostCmd_SET_SEQ_NO_BSS_INFO
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> []
> > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> []
> > @@ -512,10 +512,10 @@ enum mwifiex_channel_flags {
> >
> > #define RF_ANTENNA_AUTO 0xFFFF
> >
> > -#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) { \
> > - (((seq) & 0x00ff) | \
> > - (((num) & 0x000f) << 8)) | \
> > - (((type) & 0x000f) << 12); }
> > +#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) \
> > + ((((seq) & 0x00ff) | \
> > + (((num) & 0x000f) << 8)) | \
> > + (((type) & 0x000f) << 12))
>
> Perhaps this would be better as a static inline
>
> static inline u16 HostCmd_SET_SEQ_NO_BSS_INFO(u16 seq, u8 num, u8 type)
> {
> return (type & 0x000f) << 12 | (num & 0x000f) << 8 | (seq & 0x00ff);
> }

Just writing in on one line helps readability!
It is also used exactly twice, both with a cpu_to_le16().
I wonder how well the compiler handles that on BE?
The #define is more likely to be handled better.

I've only made a cursory glance at the code, but I get splitting
host_cmd->seq_num into two u8 fields would give better code!

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


2021-12-21 12:24:54

by Anders Roxell

[permalink] [raw]
Subject: Re: [PATCH 4.19 6/6] Input: touchscreen - avoid bitwise vs logical OR warning

On Mon, 20 Dec 2021 at 11:59, Greg KH <[email protected]> wrote:
>
> On Fri, Dec 17, 2021 at 03:41:19PM +0100, Anders Roxell wrote:
> > From: Nathan Chancellor <[email protected]>
> >
> > commit a02dcde595f7cbd240ccd64de96034ad91cffc40 upstream.
> >
> > A new warning in clang points out a few places in this driver where a
> > bitwise OR is being used with boolean types:
> >
> > drivers/input/touchscreen.c:81:17: warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical]
> > data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This use of a bitwise OR is intentional, as bitwise operations do not
> > short circuit, which allows all the calls to touchscreen_get_prop_u32()
> > to happen so that the last parameter is initialized while coalescing the
> > results of the calls to make a decision after they are all evaluated.
> >
> > To make this clearer to the compiler, use the '|=' operator to assign
> > the result of each touchscreen_get_prop_u32() call to data_present,
> > which keeps the meaning of the code the same but makes it obvious that
> > every one of these calls is expected to happen.
> >
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > Reported-by: Nick Desaulniers <[email protected]>
> > Reviewed-by: Nick Desaulniers <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Dmitry Torokhov <[email protected]>
> > Signed-off-by: Anders Roxell <[email protected]>
> > ---
> > drivers/input/touchscreen/of_touchscreen.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
>
> Also needed in 5.10.y and 5.4.y.
>
> Please be more careful next time.

Yes I will, I'm sorry.

Cheers,
Anders

2022-01-11 21:05:14

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 4.19 5/6] ARM: 8788/1: ftrace: remove old mcount support

On Fri, Dec 17, 2021 at 6:41 AM Anders Roxell <[email protected]> wrote:
>
> From: Stefan Agner <[email protected]>
>
> commit d3c61619568c88d48eccd5e74b4f84faa1440652 upstream.
>
> Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6")
> raised the minimum GCC version to 4.6. Old mcount is only required for
> GCC versions older than 4.4.0. Hence old mcount support can be dropped
> too.

cafa0010cd51 first landed in v4.19-rc1, so that lgtm.
--
Thanks,
~Nick Desaulniers

2022-01-11 21:05:18

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH 4.19 5/6] ARM: 8788/1: ftrace: remove old mcount support

On 2021-12-20 11:57, Greg KH wrote:
> On Fri, Dec 17, 2021 at 03:41:18PM +0100, Anders Roxell wrote:
>> From: Stefan Agner <[email protected]>
>>
>> commit d3c61619568c88d48eccd5e74b4f84faa1440652 upstream.
>>
>> Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6")
>> raised the minimum GCC version to 4.6. Old mcount is only required for
>> GCC versions older than 4.4.0. Hence old mcount support can be dropped
>> too.
>>
>> Signed-off-by: Stefan Agner <[email protected]>
>> Signed-off-by: Russell King <[email protected]>
>> Signed-off-by: Anders Roxell <[email protected]>
>
> Why is this needed for clang builds in 4.19?

As far as I remember, Clang tripped over this part:

-#ifndef CONFIG_OLD_MCOUNT
-#if (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4))
-#error Ftrace requires CONFIG_FRAME_POINTER=y with GCC older than
4.4.0.
-#endif
-#endif

Since mcount support wasn't required upstream at that point, instead of
fixing it for Clang I just removed it.

--
Stefan