MIPS provides multiple definitions for the following functions:
fw_init_cmdline
__delay
__udelay
__ndelay
memmove
__rmemcpy
memcpy
__copy_user
The generic ones are defined in lib-y objects, which are overridden by
the Octeon ones when CONFIG_CAVIUM_OCTEON_SOC is enabled.
The use of EXPORT_SYMBOL in static libraries potentially causes a
problem for the llvm linker [1]. So, I want to forcibly link lib-y
objects to vmlinux when CONFIG_MODULES=y.
As a groundwork, we must fix multiple definitions that have been
hidden by lib-y.
In this case, the generic implementations in arch/mips/lib/ are
weaker than the ones in arch/mips/cavium-octen/, so annotating __weak
is a straight-forward solution.
I also removed EXPORT_SYMBOL from the Octeon files to avoid the
'exported twice' warnings from modpost.
[1]: https://github.com/ClangBuiltLinux/linux/issues/515
Reported-by: kbuild test robot <[email protected]>
Suggested-by: Nick Desaulniers <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
arch/mips/cavium-octeon/csrc-octeon.c | 4 ----
arch/mips/cavium-octeon/octeon-memcpy.S | 3 ---
arch/mips/fw/lib/cmdline.c | 2 +-
arch/mips/lib/delay.c | 6 +++---
arch/mips/lib/memcpy.S | 5 +++++
5 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/arch/mips/cavium-octeon/csrc-octeon.c b/arch/mips/cavium-octeon/csrc-octeon.c
index 124817609ce0..fdc28fb5eda4 100644
--- a/arch/mips/cavium-octeon/csrc-octeon.c
+++ b/arch/mips/cavium-octeon/csrc-octeon.c
@@ -153,7 +153,6 @@ void __udelay(unsigned long us)
while (end > cur)
cur = read_c0_cvmcount();
}
-EXPORT_SYMBOL(__udelay);
void __ndelay(unsigned long ns)
{
@@ -167,7 +166,6 @@ void __ndelay(unsigned long ns)
while (end > cur)
cur = read_c0_cvmcount();
}
-EXPORT_SYMBOL(__ndelay);
void __delay(unsigned long loops)
{
@@ -179,8 +177,6 @@ void __delay(unsigned long loops)
while (end > cur)
cur = read_c0_cvmcount();
}
-EXPORT_SYMBOL(__delay);
-
/**
* octeon_io_clk_delay - wait for a given number of io clock cycles to pass.
diff --git a/arch/mips/cavium-octeon/octeon-memcpy.S b/arch/mips/cavium-octeon/octeon-memcpy.S
index 0a7c9834b81c..3eb8d1a72d7f 100644
--- a/arch/mips/cavium-octeon/octeon-memcpy.S
+++ b/arch/mips/cavium-octeon/octeon-memcpy.S
@@ -147,11 +147,9 @@
*/
.align 5
LEAF(memcpy) /* a0=dst a1=src a2=len */
-EXPORT_SYMBOL(memcpy)
move v0, dst /* return value */
__memcpy:
FEXPORT(__copy_user)
-EXPORT_SYMBOL(__copy_user)
/*
* Note: dst & src may be unaligned, len may be 0
* Temps
@@ -438,7 +436,6 @@ s_exc:
.align 5
LEAF(memmove)
-EXPORT_SYMBOL(memmove)
ADD t0, a0, a2
ADD t1, a1, a2
sltu t0, a1, t0 # dst + len <= src -> memcpy
diff --git a/arch/mips/fw/lib/cmdline.c b/arch/mips/fw/lib/cmdline.c
index 6ecda64ad184..e1f9a0c23005 100644
--- a/arch/mips/fw/lib/cmdline.c
+++ b/arch/mips/fw/lib/cmdline.c
@@ -16,7 +16,7 @@ int fw_argc;
int *_fw_argv;
int *_fw_envp;
-void __init fw_init_cmdline(void)
+void __init __weak fw_init_cmdline(void)
{
int i;
diff --git a/arch/mips/lib/delay.c b/arch/mips/lib/delay.c
index 68c495ed71e3..ba0ae7da5ced 100644
--- a/arch/mips/lib/delay.c
+++ b/arch/mips/lib/delay.c
@@ -24,7 +24,7 @@
#define GCC_DADDI_IMM_ASM() "r"
#endif
-void __delay(unsigned long loops)
+void __weak __delay(unsigned long loops)
{
__asm__ __volatile__ (
" .set noreorder \n"
@@ -48,7 +48,7 @@ EXPORT_SYMBOL(__delay);
* a constant)
*/
-void __udelay(unsigned long us)
+void __weak __udelay(unsigned long us)
{
unsigned int lpj = raw_current_cpu_data.udelay_val;
@@ -56,7 +56,7 @@ void __udelay(unsigned long us)
}
EXPORT_SYMBOL(__udelay);
-void __ndelay(unsigned long ns)
+void __weak __ndelay(unsigned long ns)
{
unsigned int lpj = raw_current_cpu_data.udelay_val;
diff --git a/arch/mips/lib/memcpy.S b/arch/mips/lib/memcpy.S
index f7994d936505..f2f58326b927 100644
--- a/arch/mips/lib/memcpy.S
+++ b/arch/mips/lib/memcpy.S
@@ -598,6 +598,9 @@ SEXC(1)
nop
.endm
+ .weak memmove
+ .weak __rmemcpy
+
.align 5
LEAF(memmove)
EXPORT_SYMBOL(memmove)
@@ -655,6 +658,8 @@ LEAF(__rmemcpy) /* a0=dst a1=src a2=len */
* the number of uncopied bytes.
* memcpy sets v0 to dst.
*/
+ .weak memcpy
+ .weak __copy_user
.align 5
LEAF(memcpy) /* a0=dst a1=src a2=len */
EXPORT_SYMBOL(memcpy)
--
2.17.1
On Tue, Mar 24, 2020 at 9:40 AM Masahiro Yamada <[email protected]> wrote:
>
> MIPS provides multiple definitions for the following functions:
>
> fw_init_cmdline
> __delay
> __udelay
> __ndelay
> memmove
> __rmemcpy
> memcpy
> __copy_user
>
> The generic ones are defined in lib-y objects, which are overridden by
> the Octeon ones when CONFIG_CAVIUM_OCTEON_SOC is enabled.
>
> The use of EXPORT_SYMBOL in static libraries potentially causes a
> problem for the llvm linker [1]. So, I want to forcibly link lib-y
> objects to vmlinux when CONFIG_MODULES=y.
>
> As a groundwork, we must fix multiple definitions that have been
> hidden by lib-y.
>
> In this case, the generic implementations in arch/mips/lib/ are
> weaker than the ones in arch/mips/cavium-octen/, so annotating __weak
> is a straight-forward solution.
>
> I also removed EXPORT_SYMBOL from the Octeon files to avoid the
> 'exported twice' warnings from modpost.
>
> [1]: https://github.com/ClangBuiltLinux/linux/issues/515
>
> Reported-by: kbuild test robot <[email protected]>
> Suggested-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/mips/cavium-octeon/csrc-octeon.c | 4 ----
> arch/mips/cavium-octeon/octeon-memcpy.S | 3 ---
> arch/mips/fw/lib/cmdline.c | 2 +-
> arch/mips/lib/delay.c | 6 +++---
> arch/mips/lib/memcpy.S | 5 +++++
> 5 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/csrc-octeon.c b/arch/mips/cavium-octeon/csrc-octeon.c
> index 124817609ce0..fdc28fb5eda4 100644
> --- a/arch/mips/cavium-octeon/csrc-octeon.c
> +++ b/arch/mips/cavium-octeon/csrc-octeon.c
> @@ -153,7 +153,6 @@ void __udelay(unsigned long us)
> while (end > cur)
> cur = read_c0_cvmcount();
> }
> -EXPORT_SYMBOL(__udelay);
>
> void __ndelay(unsigned long ns)
> {
> @@ -167,7 +166,6 @@ void __ndelay(unsigned long ns)
> while (end > cur)
> cur = read_c0_cvmcount();
> }
> -EXPORT_SYMBOL(__ndelay);
>
> void __delay(unsigned long loops)
> {
> @@ -179,8 +177,6 @@ void __delay(unsigned long loops)
> while (end > cur)
> cur = read_c0_cvmcount();
> }
> -EXPORT_SYMBOL(__delay);
> -
>
> /**
> * octeon_io_clk_delay - wait for a given number of io clock cycles to pass.
> diff --git a/arch/mips/cavium-octeon/octeon-memcpy.S b/arch/mips/cavium-octeon/octeon-memcpy.S
> index 0a7c9834b81c..3eb8d1a72d7f 100644
> --- a/arch/mips/cavium-octeon/octeon-memcpy.S
> +++ b/arch/mips/cavium-octeon/octeon-memcpy.S
> @@ -147,11 +147,9 @@
> */
> .align 5
> LEAF(memcpy) /* a0=dst a1=src a2=len */
> -EXPORT_SYMBOL(memcpy)
> move v0, dst /* return value */
> __memcpy:
> FEXPORT(__copy_user)
> -EXPORT_SYMBOL(__copy_user)
> /*
> * Note: dst & src may be unaligned, len may be 0
> * Temps
> @@ -438,7 +436,6 @@ s_exc:
>
> .align 5
> LEAF(memmove)
> -EXPORT_SYMBOL(memmove)
> ADD t0, a0, a2
> ADD t1, a1, a2
> sltu t0, a1, t0 # dst + len <= src -> memcpy
> diff --git a/arch/mips/fw/lib/cmdline.c b/arch/mips/fw/lib/cmdline.c
> index 6ecda64ad184..e1f9a0c23005 100644
> --- a/arch/mips/fw/lib/cmdline.c
> +++ b/arch/mips/fw/lib/cmdline.c
> @@ -16,7 +16,7 @@ int fw_argc;
> int *_fw_argv;
> int *_fw_envp;
>
> -void __init fw_init_cmdline(void)
> +void __init __weak fw_init_cmdline(void)
> {
> int i;
>
> diff --git a/arch/mips/lib/delay.c b/arch/mips/lib/delay.c
> index 68c495ed71e3..ba0ae7da5ced 100644
> --- a/arch/mips/lib/delay.c
> +++ b/arch/mips/lib/delay.c
> @@ -24,7 +24,7 @@
> #define GCC_DADDI_IMM_ASM() "r"
> #endif
>
> -void __delay(unsigned long loops)
> +void __weak __delay(unsigned long loops)
> {
> __asm__ __volatile__ (
> " .set noreorder \n"
> @@ -48,7 +48,7 @@ EXPORT_SYMBOL(__delay);
> * a constant)
> */
>
> -void __udelay(unsigned long us)
> +void __weak __udelay(unsigned long us)
> {
> unsigned int lpj = raw_current_cpu_data.udelay_val;
>
> @@ -56,7 +56,7 @@ void __udelay(unsigned long us)
> }
> EXPORT_SYMBOL(__udelay);
>
> -void __ndelay(unsigned long ns)
> +void __weak __ndelay(unsigned long ns)
> {
> unsigned int lpj = raw_current_cpu_data.udelay_val;
>
> diff --git a/arch/mips/lib/memcpy.S b/arch/mips/lib/memcpy.S
> index f7994d936505..f2f58326b927 100644
> --- a/arch/mips/lib/memcpy.S
> +++ b/arch/mips/lib/memcpy.S
> @@ -598,6 +598,9 @@ SEXC(1)
> nop
> .endm
>
> + .weak memmove
> + .weak __rmemcpy
> +
> .align 5
> LEAF(memmove)
> EXPORT_SYMBOL(memmove)
> @@ -655,6 +658,8 @@ LEAF(__rmemcpy) /* a0=dst a1=src a2=len */
> * the number of uncopied bytes.
> * memcpy sets v0 to dst.
> */
> + .weak memcpy
> + .weak __copy_user
I think it would be better to use SYM_FUNC_START_WEAK from
include/linux/linkage.h rather than LEAF, but it looks like LEAF uses
a different value for ALIGN, and sets up call frame information CFI.
So in that case, no complaints.
Reviewed-by: Nick Desaulniers <[email protected]>
Thanks for the patch!
> .align 5
> LEAF(memcpy) /* a0=dst a1=src a2=len */
> EXPORT_SYMBOL(memcpy)
> --
> 2.17.1
>
> --
--
Thanks,
~Nick Desaulniers
On Wed, 25 Mar 2020, Masahiro Yamada wrote:
> MIPS provides multiple definitions for the following functions:
>
> fw_init_cmdline
> __delay
> __udelay
> __ndelay
> memmove
> __rmemcpy
> memcpy
> __copy_user
>
> The generic ones are defined in lib-y objects, which are overridden by
> the Octeon ones when CONFIG_CAVIUM_OCTEON_SOC is enabled.
>
> The use of EXPORT_SYMBOL in static libraries potentially causes a
> problem for the llvm linker [1]. So, I want to forcibly link lib-y
> objects to vmlinux when CONFIG_MODULES=y.
>
> As a groundwork, we must fix multiple definitions that have been
> hidden by lib-y.
IIUC that causes known dead code to be included in the kernel image.
Wouldn't it be possible to actually omit replaced functions from output by
keying the build of the sources providing generic code with appropriate
CONFIG_* settings (such as CONFIG_GENERIC_DELAY, CONFIG_GENERIC_MEMCPY,
etc. or suchlike)?
Maciej
Hi Maciej,
On Wed, Mar 25, 2020 at 11:46 AM Maciej W. Rozycki <[email protected]> wrote:
>
> On Wed, 25 Mar 2020, Masahiro Yamada wrote:
>
> > MIPS provides multiple definitions for the following functions:
> >
> > fw_init_cmdline
> > __delay
> > __udelay
> > __ndelay
> > memmove
> > __rmemcpy
> > memcpy
> > __copy_user
> >
> > The generic ones are defined in lib-y objects, which are overridden by
> > the Octeon ones when CONFIG_CAVIUM_OCTEON_SOC is enabled.
> >
> > The use of EXPORT_SYMBOL in static libraries potentially causes a
> > problem for the llvm linker [1]. So, I want to forcibly link lib-y
> > objects to vmlinux when CONFIG_MODULES=y.
> >
> > As a groundwork, we must fix multiple definitions that have been
> > hidden by lib-y.
>
> IIUC that causes known dead code to be included in the kernel image.
> Wouldn't it be possible to actually omit replaced functions from output by
> keying the build of the sources providing generic code with appropriate
> CONFIG_* settings (such as CONFIG_GENERIC_DELAY, CONFIG_GENERIC_MEMCPY,
> etc. or suchlike)?
>
> Maciej
You are right.
__weak cannot trim the dead code.
I can work on the CONFIG_ approach,
but I'd rather to use inverted
CONFIG_HAVE_PLAT_* because it is easier
to make CAVIUM_OCTEON_SOC select them.
--
Best Regards
Masahiro Yamada