Re: [PATCH] [RESEND] mtd: only use __xipram annotation when XIP_KERNEL is set

From: Boris Brezillon
Date: Fri Aug 04 2017 - 03:36:01 EST


On Fri, 21 Jul 2017 22:26:25 +0200
Arnd Bergmann <arnd@xxxxxxxx> wrote:

> When XIP_KERNEL is enabled, some functions are defined in the .data
> ELF section because we require them to be in RAM whenever we communicate
> with the flash chip. However this causes problems when FTRACE is
> enabled and gcc emits calls to __gnu_mcount_nc in the function
> prolog:
>
> drivers/built-in.o: In function `cfi_chip_setup':
> :(.data+0x272fc): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o
> drivers/built-in.o: In function `cfi_probe_chip':
> :(.data+0x27de8): relocation truncated to fit: R_ARM_CALL against symbol `__gnu_mcount_nc' defined in .text section in arch/arm/kernel/built-in.o
> /tmp/ccY172rP.s: Assembler messages:
> /tmp/ccY172rP.s:70: Warning: ignoring changed section attributes for .data
> /tmp/ccY172rP.s: Error: 1 warning, treating warnings as errors
> make[5]: *** [drivers/mtd/chips/cfi_probe.o] Error 1
> /tmp/ccK4rjeO.s: Assembler messages:
> /tmp/ccK4rjeO.s:421: Warning: ignoring changed section attributes for .data
> /tmp/ccK4rjeO.s: Error: 1 warning, treating warnings as errors
> make[5]: *** [drivers/mtd/chips/cfi_util.o] Error 1
> /tmp/ccUvhCYR.s: Assembler messages:
> /tmp/ccUvhCYR.s:1895: Warning: ignoring changed section attributes for .data
> /tmp/ccUvhCYR.s: Error: 1 warning, treating warnings as errors
>
> Specifically, this does not work because the .data section is not
> marked executable, which leads LD to not generate trampolines for
> long calls.
>
> This moves the __xipram functions into their own .xiptext section instead.
> The section is still placed next to .data and located in RAM but is marked
> executable, which avoids the build errors.
>
> Also, we only need to place the XIP functions into a separate section
> if both CONFIG_XIP_KERNEL and CONFIG_MTD_XIP are set: When only MTD_XIP
> is used, the whole kernel is still in RAM and we do not need to worry
> about pulling out the rug under it. When only XIP_KERNEL but not MTD_XIP
> is set, the kernel is in some form of ROM, but we never write to it.
>
> Note that MTD_XIP has been broken on ARM since around 2011 or 2012. I
> have sent another patch[2] to fix compilation, which I plan to merge
> through arm-soc unless there are objections. The obvious alternative
> to that would be to completely rip out the MTD_XIP support from the
> kernel, since obviously nobody has been using it in a long while.
>
> Link: [1] https://patchwork.kernel.org/patch/8109771/
> Link: [2] https://patchwork.kernel.org/patch/9855225/
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> include/asm-generic/vmlinux.lds.h | 1 +
> include/linux/mtd/xip.h | 10 ++++++----

For the MTD part,

Acked-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>

> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index da0be9a8d1de..ff507c178327 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -203,6 +203,7 @@
> * pull in .data..stuff which has its own requirements. Same for bss.
> */
> #define DATA_DATA \
> + *(.xiptext) \
> *(.data .data.[0-9a-zA-Z_]*) \
> *(.ref.data) \
> *(.data..shared_aligned) /* percpu related */ \
> diff --git a/include/linux/mtd/xip.h b/include/linux/mtd/xip.h
> index abed4dec5c2f..e373690cce0a 100644
> --- a/include/linux/mtd/xip.h
> +++ b/include/linux/mtd/xip.h
> @@ -30,7 +30,9 @@
> * obviously not be running from flash. The __xipram is therefore marking
> * those functions so they get relocated to ram.
> */
> -#define __xipram noinline __attribute__ ((__section__ (".data")))
> +#ifdef CONFIG_XIP_KERNEL
> +#define __xipram noinline __attribute__ ((__section__ (".xiptext")))
> +#endif
>
> /*
> * Each architecture has to provide the following macros. They must access
> @@ -90,10 +92,10 @@
> #define xip_cpu_idle() do { } while (0)
> #endif
>
> -#else
> +#endif /* CONFIG_MTD_XIP */
>
> +#ifndef __xipram
> #define __xipram
> -
> -#endif /* CONFIG_MTD_XIP */
> +#endif
>
> #endif /* __LINUX_MTD_XIP_H__ */