diff mbox

mtd: only use __xipram annotation when XIP_KERNEL is set

Message ID 1453736525-1959191-2-git-send-email-arnd@arndb.de
State Superseded
Headers show

Commit Message

Arnd Bergmann Jan. 25, 2016, 3:41 p.m. UTC
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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/asm-generic/vmlinux.lds.h |  1 +
 include/linux/mtd/xip.h           | 10 ++++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

Comments

Brian Norris March 5, 2016, 12:02 a.m. UTC | #1
Hi Arnd,

I know you're travelling, but...

On Mon, Jan 25, 2016 at 04:41:50PM +0100, Arnd Bergmann 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

Can you provide a sample .config that DOES build correctly with
XIP_KERNEL enabled + this patch? My first attempt yields some other
failures I don't care to fixup right now...

Anyway, I don't doubt you have a good fix here, so I can probably take
it. Any review from others would be welcome though.

> 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.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Brian
Brian Norris March 5, 2016, 12:10 a.m. UTC | #2
+ others

On Fri, Mar 04, 2016 at 04:02:25PM -0800, Brian Norris wrote:
> Hi Arnd,
> 
> I know you're travelling, but...
> 
> On Mon, Jan 25, 2016 at 04:41:50PM +0100, Arnd Bergmann 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
> 
> Can you provide a sample .config that DOES build correctly with
> XIP_KERNEL enabled + this patch? My first attempt yields some other
> failures I don't care to fixup right now...

Particularly, mach-pxa MTD_XIP looks like it's broken, at least since
here:

  5d284e353eb1 ARM: pxa: avoid accessing interrupt registers directly

  CC      drivers/mtd/chips/cfi_cmdset_0002.o
drivers/mtd/chips/cfi_cmdset_0002.c: In function ‘xip_udelay’:
drivers/mtd/chips/cfi_cmdset_0002.c:962:35: warning: initialization makes integer from pointer without a cast [enabled by default]
drivers/mtd/chips/cfi_cmdset_0002.c:967:8: error: ‘ICIP’ undeclared (first use in this function)
drivers/mtd/chips/cfi_cmdset_0002.c:967:8: note: each undeclared identifier is reported only once for each function it appears in
drivers/mtd/chips/cfi_cmdset_0002.c:967:15: error: ‘ICMR’ undeclared (first use in this function)
drivers/mtd/chips/cfi_cmdset_0002.c:981:123: error: invalid operands to binary / (have ‘void *’ and ‘int’)
drivers/mtd/chips/cfi_cmdset_0002.c:982:14: warning: assignment makes integer from pointer without a cast [enabled by default]
drivers/mtd/chips/cfi_cmdset_0002.c:984:124: error: invalid operands to binary / (have ‘void *’ and ‘int’)
drivers/mtd/chips/cfi_cmdset_0002.c:1034:10: warning: assignment makes integer from pointer without a cast [enabled by default]
drivers/mtd/chips/cfi_cmdset_0002.c:1045:118: error: invalid operands to binary / (have ‘void *’ and ‘int’)

Looks like arch/arm/mach-pxa/include/mach/mtd-xip.h can't find ICIP or ICMR...

> Anyway, I don't doubt you have a good fix here, so I can probably take
> it. Any review from others would be welcome though.
> 
> > 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.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Brian
diff mbox

Patch

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index c4bd0e2c173c..73a2c72e4252 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -199,6 +199,7 @@ 
 
 /* .data section */
 #define DATA_DATA							\
+	*(.xiptext)							\
 	*(.data)							\
 	*(.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__ */