Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

Having looked at various kallsyms failures today, this is my conclusion:

1. Large ARM kernels are potentially harmful for stable kallsyms data.

When we have a kernel approaching 64MB, the linker can insert veneers
which are "ldr pc, pc + 4; .word target" code fragments.  These are
inserted after the text sections.

The problem is that the kallsyms data lives between the main .text
section, and the init .text section.  The first link without kallsyms
data results in some veneers created - these are inserted into the
symbolic table as "__%s_veneer" where %s is the name of the target
symbol.  These are inserted into the kallsyms data, which causes it
to grow in size, and on the following link, the init .text section is
pushed further away from the main .text section, resulting in more
veneers generated, and more veneer symbols.  That changes the kallsyms
data again - making it increase in size, and so the situation continues
until there is no further expansion in the number of veneers.

Omitting the veneer symbols from kallsyms fixes this, and the patch is
quite simple and easy.  Arnd already has a copy of my hack which does
this, which resolves a few of his failing configurations, though it's
probably easier to use grep in the kallsyms pipeline in link-vmlinux.sh
to remove these symbols - grep -v '_veneer$' (see 2a too.)

2. XIP - This one is truely horrible, and there's two issues here:

2a. commits f6537f2f0eba4eba3354e48dbe3047db6d8b6254 and
    7122c3e9154b5d9a7422f68f02d8acf050fad2b0

These two commits added filtering using CONFIG_PAGE_OFFSET to kallsyms.
This may have appeared like a good idea at the time, but it really isn't.
With XIP kernels, the main .text section is placed in the module area,
which is below PAGE_OFFSET (and so CONFIG_PAGE_OFFSET).  The result of
this change is that all the symbols for the main .text section are always
omitted from an XIP kernel.

Hence, as long as we have XIP kernel support, the above two commits are
positively harmful, and I think they should be reverted, and the original
problem re-investigated.  Maybe the troublesome symbols should be omitted
via a grep regexp in the kallsyms pipeline instead?

2b. Large XIP kernels are broken.

You only have to see what happens to the sections:

Idx Name          Size      VMA       LMA       File off  Algn
  2 .rodata       00400f94  7fd5c000  7fd5c000  00ce4000  2**8
  3 __bug_table   0000a854  8015cf98  8015cf98  010e4f98  2**0
...
 21 .data         00347e50  80008000  801fb490  01190000  2**8

Look at the VMA - the .data section overlaps the .rodata section.  This
kernel can never be run, and the linker doesn't detect this failure.

Thankfully, kallsyms fails on this, because the addition of kallsyms data
causes this change in symbolic information because of this overlap:

-80048bdc T reiserfs_xattr_register_handlers
-80048bf0 t init_ext3_fs
-80048d14 t ftrace_define_fields_ext3_load_inode
+80048250 R kallsyms_num_syms
+80048260 R kallsyms_names
 80048d6c d pm800_volt_range_ops
-80048dcc t ftrace_define_fields_ext3_get_blocks_exit
 80048dcc d pm800_volt_table_ops
 80048e2c d act8865_pmic_driver

 801b8bd4 d ______f.33729
+801b8bdc T reiserfs_xattr_register_handlers
 801b8be8 d ______f.33734
+801b8bf0 t init_ext3_fs
 801b8bfc d ______f.33743
...
 801b8d14 d ______f.33860
+801b8d14 t ftrace_define_fields_ext3_load_inode
 801b8d28 d ______f.33872

As the symbols are now "shuffled", the string compression which kallsyms
uses changes, which changes the size of its output data - which then
changes the placement of these symbols (kallsyms_addresses is at
0x7ffe2c70.)

I don't see that this is solvable - XIP kernels just can't be big things.
Even if we decide to disable XIP support for randconfigs (by seeding it
with CONFIG_XIP=n) that doesn't really solve the problem: it would still
be possible to configure an overly large kernel and have it apparently
link successfully, but because of the above overlap between .rodata and
.data, it would have no hope in hell of running.

Thoughts?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

This is the hack I'm using to avoid the veneers appearing in the kallsyms
data.  This solves my randconfig failure I saw last night - but obviously
is not bullet proof as it'll find _veneer anywhere in the symbol name.

 scripts/kallsyms.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index c6d33bd15b04..eea599c701f5 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -158,6 +158,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	/* exclude debugging symbols */
 	else if (stype == 'N')
 		return -1;
+	else if (strstr(sym, "_veneer"))
+		return -1;
 
 	/* include the type field in the symbol name, so that it gets
 	 * compressed together */

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

Here's a potential patch for issue 2b - to cause the link to fail if
the resulting kernel would be broken by trying to run it.

 arch/arm/kernel/vmlinux.lds.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index b31aa73e8076..9351f7fbdfb1 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -351,3 +351,13 @@ ASSERT((__arch_info_end - __arch_info_begin), "no machine record defined")
  * The above comment applies as well.
  */
 ASSERT(((__hyp_idmap_text_end - __hyp_idmap_text_start) <= PAGE_SIZE), "HYP init code too big")
+
+#ifdef CONFIG_XIP_KERNEL
+/*
+ * __data_loc is not only the LMA of the data section, but also the VMA of
+ * the end of the .rodata section.  This must not overlap the VMA of the
+ * data section.  Since the .text section starts in module space, and that
+ * is always below the .data section, this should be sufficient.
+ */
+ASSERT((_data >= __data_loc), "Text section oversize")
+#endif


-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Nicolas Pitre]

On Fri, 30 Jan 2015, Russell King - ARM Linux wrote:

> Here's a potential patch for issue 2b - to cause the link to fail if
> the resulting kernel would be broken by trying to run it.
> 
>  arch/arm/kernel/vmlinux.lds.S | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index b31aa73e8076..9351f7fbdfb1 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -351,3 +351,13 @@ ASSERT((__arch_info_end - __arch_info_begin), "no machine record defined")
>   * The above comment applies as well.
>   */
>  ASSERT(((__hyp_idmap_text_end - __hyp_idmap_text_start) <= PAGE_SIZE), "HYP init code too big")
> +
> +#ifdef CONFIG_XIP_KERNEL
> +/*
> + * __data_loc is not only the LMA of the data section, but also the VMA of
> + * the end of the .rodata section.  This must not overlap the VMA of the
> + * data section.  Since the .text section starts in module space, and that
> + * is always below the .data section, this should be sufficient.
> + */
> +ASSERT((_data >= __data_loc), "Text section oversize")
> +#endif

I agree with this patch.

Acked-by: Nicolas Pitre <nico@linaro.org>

This might not prevent a config leading to this from happening, but at 
least it makes the issue much clearer.  XIP kernel was created for 
systems where the total amount of RAM is often smaller than the imposed
size limit here.


Nicolas

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

On Fri, Jan 30, 2015 at 12:29:30PM -0500, Nicolas Pitre wrote:
> On Fri, 30 Jan 2015, Russell King - ARM Linux wrote:
> > +#ifdef CONFIG_XIP_KERNEL
> > +/*
> > + * __data_loc is not only the LMA of the data section, but also the VMA of
> > + * the end of the .rodata section.  This must not overlap the VMA of the
> > + * data section.  Since the .text section starts in module space, and that
> > + * is always below the .data section, this should be sufficient.
> > + */
> > +ASSERT((_data >= __data_loc), "Text section oversize")
> > +#endif
> 
> I agree with this patch.
> 
> Acked-by: Nicolas Pitre <nico@linaro.org>
> 
> This might not prevent a config leading to this from happening, but at 
> least it makes the issue much clearer.  XIP kernel was created for 
> systems where the total amount of RAM is often smaller than the imposed
> size limit here.

Yes, I expect more of Arnd's randconfigs to fail with this patch applied.

I did also notice that we still have swapper_pg_dir at _data - 0x4000
for XIP kernels - so the above check is slightly too lenient.  A better
threshold for __data_loc might be MODULES_END, since we can't allow the
XIP part to overlap into RAM.

#ifdef CONFIG_HIGHMEM
#define MODULES_END             (PAGE_OFFSET - PMD_SIZE)
#else
#define MODULES_END             (PAGE_OFFSET)
#endif


-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

On Sat, Jan 31, 2015 at 12:22:53AM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 30, 2015 at 12:29:30PM -0500, Nicolas Pitre wrote:
> > On Fri, 30 Jan 2015, Russell King - ARM Linux wrote:
> > > +#ifdef CONFIG_XIP_KERNEL
> > > +/*
> > > + * __data_loc is not only the LMA of the data section, but also the VMA of
> > > + * the end of the .rodata section.  This must not overlap the VMA of the
> > > + * data section.  Since the .text section starts in module space, and that
> > > + * is always below the .data section, this should be sufficient.
> > > + */
> > > +ASSERT((_data >= __data_loc), "Text section oversize")
> > > +#endif
> > 
> > I agree with this patch.
> > 
> > Acked-by: Nicolas Pitre <nico@linaro.org>
> > 
> > This might not prevent a config leading to this from happening, but at 
> > least it makes the issue much clearer.  XIP kernel was created for 
> > systems where the total amount of RAM is often smaller than the imposed
> > size limit here.
> 
> Yes, I expect more of Arnd's randconfigs to fail with this patch applied.
> 
> I did also notice that we still have swapper_pg_dir at _data - 0x4000
> for XIP kernels - so the above check is slightly too lenient.  A better
> threshold for __data_loc might be MODULES_END, since we can't allow the
> XIP part to overlap into RAM.
> 
> #ifdef CONFIG_HIGHMEM
> #define MODULES_END             (PAGE_OFFSET - PMD_SIZE)
> #else
> #define MODULES_END             (PAGE_OFFSET)
> #endif

It looks like we have cases where this falsely triggers.  Consider EFM32:

CONFIG_DRAM_BASE=0x88000000
CONFIG_DRAM_SIZE=0x00400000
CONFIG_FLASH_MEM_BASE=0x8c000000
CONFIG_FLASH_SIZE=0x01000000

This means that we quite legally end up with the .data section below the
.text section, which makes:

ASSERT((_data >= __data_loc), "Text section oversize") 

falsely trigger.

The linker has the capacity to specify regions of ROM and RAM in the
linker file, I wonder if we should be using those for XIP.  Merely
adding the MEMORY table to the linker file is not good enough - we
also need to explicitly tell the linker which memory regions to place
the output sections, otherwise the linker ends up making assumptions.

What that means is... asm-generic/vmlinux.lds.h breaks for us.

Any ideas?  I think using the MEMORY table would be the best approach,
because that should allow us to properly verify that the resulting
binary should fit in the memory regions.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Nicolas Pitre]

On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:

> It looks like we have cases where this falsely triggers.  Consider EFM32:
> 
> CONFIG_DRAM_BASE=0x88000000
> CONFIG_DRAM_SIZE=0x00400000
> CONFIG_FLASH_MEM_BASE=0x8c000000
> CONFIG_FLASH_SIZE=0x01000000
> 
> This means that we quite legally end up with the .data section below the
> .text section, which makes:
> 
> ASSERT((_data >= __data_loc), "Text section oversize") 
> 
> falsely trigger.
> 
> The linker has the capacity to specify regions of ROM and RAM in the
> linker file, I wonder if we should be using those for XIP.  Merely
> adding the MEMORY table to the linker file is not good enough - we
> also need to explicitly tell the linker which memory regions to place
> the output sections, otherwise the linker ends up making assumptions.
> 
> What that means is... asm-generic/vmlinux.lds.h breaks for us.
> 
> Any ideas?  I think using the MEMORY table would be the best approach,
> because that should allow us to properly verify that the resulting
> binary should fit in the memory regions.

Maybe simply having an assert() on the size of the .text section could 
be all that is needed.  We already know the maximum size in advance.


Nicolas

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

On Tue, Feb 03, 2015 at 08:59:15PM -0500, Nicolas Pitre wrote:
> On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:
> 
> > It looks like we have cases where this falsely triggers.  Consider EFM32:
> > 
> > CONFIG_DRAM_BASE=0x88000000
> > CONFIG_DRAM_SIZE=0x00400000
> > CONFIG_FLASH_MEM_BASE=0x8c000000
> > CONFIG_FLASH_SIZE=0x01000000
> > 
> > This means that we quite legally end up with the .data section below the
> > .text section, which makes:
> > 
> > ASSERT((_data >= __data_loc), "Text section oversize") 
> > 
> > falsely trigger.
> > 
> > The linker has the capacity to specify regions of ROM and RAM in the
> > linker file, I wonder if we should be using those for XIP.  Merely
> > adding the MEMORY table to the linker file is not good enough - we
> > also need to explicitly tell the linker which memory regions to place
> > the output sections, otherwise the linker ends up making assumptions.
> > 
> > What that means is... asm-generic/vmlinux.lds.h breaks for us.
> > 
> > Any ideas?  I think using the MEMORY table would be the best approach,
> > because that should allow us to properly verify that the resulting
> > binary should fit in the memory regions.
> 
> Maybe simply having an assert() on the size of the .text section could 
> be all that is needed.  We already know the maximum size in advance.

That doesn't work, it's not just the .text section but also .rodata,
__bug_table, __ksymtab, __ksymtab_gpl, __kcrctab, __kcrctab_gpl,
__ksymtab_strings, __param, __modver, __ex_table, .notes, .vectors,
.stubs, .init.text, maybe .exit.text, .init.arch.info, .init.tagtable,
.init.smpalt, .init.pv_table, and apparently .init.data (which is
surely wrong?)  The following is from Arnd's failing configuration:

 18 .init.tagtable 00000040  80073a9c  80073a9c  0100ba9c  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 19 .init.data    000010e8  80073adc  80073adc  0100badc  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 20 .data         003552c4  80008000  80074bc4  01010000  2**8
                  CONTENTS, ALLOC, LOAD, DATA

Hmm, if .init.data is contained in the flash section (which it seemingly
is), it seems that XIP support is currently broken - that section is
definitely a read/write section.  No one has seemingly noticed that it's
broken and it's been broken for a long time, so maybe the simple answer
then is just to rip XIP support out?

How does EFM32 work?  Does it work?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Nicolas Pitre]

On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:

> On Tue, Feb 03, 2015 at 08:59:15PM -0500, Nicolas Pitre wrote:
> > On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:
> > 
> > > It looks like we have cases where this falsely triggers.  Consider EFM32:
> > > 
> > > CONFIG_DRAM_BASE=0x88000000
> > > CONFIG_DRAM_SIZE=0x00400000
> > > CONFIG_FLASH_MEM_BASE=0x8c000000
> > > CONFIG_FLASH_SIZE=0x01000000
> > > 
> > > This means that we quite legally end up with the .data section below the
> > > .text section, which makes:
> > > 
> > > ASSERT((_data >= __data_loc), "Text section oversize") 
> > > 
> > > falsely trigger.
> > > 
> > > The linker has the capacity to specify regions of ROM and RAM in the
> > > linker file, I wonder if we should be using those for XIP.  Merely
> > > adding the MEMORY table to the linker file is not good enough - we
> > > also need to explicitly tell the linker which memory regions to place
> > > the output sections, otherwise the linker ends up making assumptions.
> > > 
> > > What that means is... asm-generic/vmlinux.lds.h breaks for us.
> > > 
> > > Any ideas?  I think using the MEMORY table would be the best approach,
> > > because that should allow us to properly verify that the resulting
> > > binary should fit in the memory regions.
> > 
> > Maybe simply having an assert() on the size of the .text section could 
> > be all that is needed.  We already know the maximum size in advance.
> 
> That doesn't work, it's not just the .text section but also .rodata,
> __bug_table, __ksymtab, __ksymtab_gpl, __kcrctab, __kcrctab_gpl,
> __ksymtab_strings, __param, __modver, __ex_table, .notes, .vectors,
> .stubs, .init.text, maybe .exit.text, .init.arch.info, .init.tagtable,
> .init.smpalt, .init.pv_table, 

My suggestion would be to put a symbol at the end of it all to size the 
lot.

> and apparently .init.data (which is
> surely wrong?)  The following is from Arnd's failing configuration:
> 
>  18 .init.tagtable 00000040  80073a9c  80073a9c  0100ba9c  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  19 .init.data    000010e8  80073adc  80073adc  0100badc  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>  20 .data         003552c4  80008000  80074bc4  01010000  2**8
>                   CONTENTS, ALLOC, LOAD, DATA
> 
> Hmm, if .init.data is contained in the flash section (which it seemingly
> is), it seems that XIP support is currently broken - that section is
> definitely a read/write section.  No one has seemingly noticed that it's
> broken and it's been broken for a long time, so maybe the simple answer
> then is just to rip XIP support out?

That's what I proposed a couple years ago but some people were 
apparently still using it.

Now with all the buzz around IOT it is well possible that XIP will gain 
more traction again.


Nicolas

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Arnd Bergmann]

On Wednesday 04 February 2015 09:44:14 Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 08:59:15PM -0500, Nicolas Pitre wrote:
> > On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:
> > 
> > > It looks like we have cases where this falsely triggers.  Consider EFM32:
> > > 
> > > CONFIG_DRAM_BASE=0x88000000
> > > CONFIG_DRAM_SIZE=0x00400000
> > > CONFIG_FLASH_MEM_BASE=0x8c000000
> > > CONFIG_FLASH_SIZE=0x01000000
> > > 
> > > This means that we quite legally end up with the .data section below the
> > > .text section, which makes:
> > > 
> > > ASSERT((_data >= __data_loc), "Text section oversize") 
> > > 
> > > falsely trigger.
> > > 
> > > The linker has the capacity to specify regions of ROM and RAM in the
> > > linker file, I wonder if we should be using those for XIP.  Merely
> > > adding the MEMORY table to the linker file is not good enough - we
> > > also need to explicitly tell the linker which memory regions to place
> > > the output sections, otherwise the linker ends up making assumptions.
> > > 
> > > What that means is... asm-generic/vmlinux.lds.h breaks for us.
> > > 
> > > Any ideas?  I think using the MEMORY table would be the best approach,
> > > because that should allow us to properly verify that the resulting
> > > binary should fit in the memory regions.
> > 
> > Maybe simply having an assert() on the size of the .text section could 
> > be all that is needed.  We already know the maximum size in advance.
> 
> That doesn't work, it's not just the .text section but also .rodata,
> __bug_table, __ksymtab, __ksymtab_gpl, __kcrctab, __kcrctab_gpl,
> __ksymtab_strings, __param, __modver, __ex_table, .notes, .vectors,
> .stubs, .init.text, maybe .exit.text, .init.arch.info, .init.tagtable,
> .init.smpalt, .init.pv_table, and apparently .init.data (which is
> surely wrong?)  The following is from Arnd's failing configuration:
> 
>  18 .init.tagtable 00000040  80073a9c  80073a9c  0100ba9c  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  19 .init.data    000010e8  80073adc  80073adc  0100badc  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>  20 .data         003552c4  80008000  80074bc4  01010000  2**8
>                   CONTENTS, ALLOC, LOAD, DATA
> 
> Hmm, if .init.data is contained in the flash section (which it seemingly
> is), it seems that XIP support is currently broken - that section is
> definitely a read/write section.  No one has seemingly noticed that it's
> broken and it's been broken for a long time, so maybe the simple answer
> then is just to rip XIP support out?
> 
> How does EFM32 work?  Does it work?

I believe that Uwe has a patch series on top of mainline that he still
requires for using the platform.

The part I'm sure about is that it does not work without XIP_KERNEL,
given the tight memory constraints (2MB RAM?).

	Arnd

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Uwe Kleine-König]

Hello Russell,

On Wed, Feb 04, 2015 at 09:44:14AM +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 08:59:15PM -0500, Nicolas Pitre wrote:
> > On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:
> > 
> > > It looks like we have cases where this falsely triggers.  Consider EFM32:
> > > 
> > > CONFIG_DRAM_BASE=0x88000000
> > > CONFIG_DRAM_SIZE=0x00400000
> > > CONFIG_FLASH_MEM_BASE=0x8c000000
> > > CONFIG_FLASH_SIZE=0x01000000
> > > 
> > > This means that we quite legally end up with the .data section below the
> > > .text section, which makes:
> > > 
> > > ASSERT((_data >= __data_loc), "Text section oversize") 
> > > 
> > > falsely trigger.
> > > 
> > > The linker has the capacity to specify regions of ROM and RAM in the
> > > linker file, I wonder if we should be using those for XIP.  Merely
> > > adding the MEMORY table to the linker file is not good enough - we
> > > also need to explicitly tell the linker which memory regions to place
> > > the output sections, otherwise the linker ends up making assumptions.
> > > 
> > > What that means is... asm-generic/vmlinux.lds.h breaks for us.
> > > 
> > > Any ideas?  I think using the MEMORY table would be the best approach,
> > > because that should allow us to properly verify that the resulting
> > > binary should fit in the memory regions.
> > 
> > Maybe simply having an assert() on the size of the .text section could 
> > be all that is needed.  We already know the maximum size in advance.
> 
> That doesn't work, it's not just the .text section but also .rodata,
> __bug_table, __ksymtab, __ksymtab_gpl, __kcrctab, __kcrctab_gpl,
> __ksymtab_strings, __param, __modver, __ex_table, .notes, .vectors,
> .stubs, .init.text, maybe .exit.text, .init.arch.info, .init.tagtable,
> .init.smpalt, .init.pv_table, and apparently .init.data (which is
> surely wrong?)  The following is from Arnd's failing configuration:
> 
>  18 .init.tagtable 00000040  80073a9c  80073a9c  0100ba9c  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  19 .init.data    000010e8  80073adc  80073adc  0100badc  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>  20 .data         003552c4  80008000  80074bc4  01010000  2**8
>                   CONTENTS, ALLOC, LOAD, DATA
> 
> Hmm, if .init.data is contained in the flash section (which it seemingly
> is), it seems that XIP support is currently broken - that section is
> definitely a read/write section.  No one has seemingly noticed that it's
> broken and it's been broken for a long time, so maybe the simple answer
> then is just to rip XIP support out?
> 
> How does EFM32 work?  Does it work?
An unmodified 3.19.0-rc6 + efm32_defconfig boots just fine with XIP (and
has to little RAM for holding the kernel image in it's 4 MiB RAM).

And also modifying initdata seems to work. I tested with:

--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -895,6 +895,8 @@ void __init hyp_mode_check(void)
 #endif
 }
 
+volatile int test __initdata;
+
 void __init setup_arch(char **cmdline_p)
 {
        const struct machine_desc *mdesc;
@@ -929,8 +931,14 @@ void __init setup_arch(char **cmdline_p)
        paging_init(mdesc);
        request_standard_resources(mdesc);
 
-       if (mdesc->restart)
+       if (mdesc->restart) {
                arm_pm_restart = mdesc->restart;
+               test = 3;
+       } else {
+               test = 5;
+       }
+
+       pr_info("%s: test = %d (%p)\n", __func__, test, &test);
 
        unflatten_device_tree();
 
The resulting assembler gives the impression that the assignment was not
optimized by the compiler:

8c17c310:       4fa5            ldr     r7, [pc, #660]  ; (8c17c5a8 <setup_arch+0x2d6>)
...
8c17c5a8:       8800a8b4        .word   0x8800a8b4
...
8c17c670:       6e2b            ldr     r3, [r5, #96]   ; 0x60
8c17c672:       b11b            cbz     r3, 8c17c67c <setup_arch+0x3aa>
8c17c674:       4a13            ldr     r2, [pc, #76]   ; (8c17c6c4 <setup_arch+0x3f2>)
8c17c676:       6013            str     r3, [r2, #0]
8c17c678:       2303            movs    r3, #3
8c17c67a:       e000            b.n     8c17c67e <setup_arch+0x3ac>
8c17c67c:       2305            movs    r3, #5
8c17c67e:       f8c7 3410       str.w   r3, [r7, #1040] ; 0x410
8c17c682:       f8d7 2410       ldr.w   r2, [r7, #1040] ; 0x410
8c17c686:       4b10            ldr     r3, [pc, #64]   ; (8c17c6c8 <setup_arch+0x3f6>)
8c17c688:       4910            ldr     r1, [pc, #64]   ; (8c17c6cc <setup_arch+0x3fa>)
8c17c68a:       4811            ldr     r0, [pc, #68]   ; (8c17c6d0 <setup_arch+0x3fe>)
8c17c68c:       f79a fec8       bl      8c117420 <printk>
...
8c17c6c8:       8800acc4        .word   0x8800acc4

and the result is

	[    0.000000] setup_arch: test = 3 (8800acc4)

Still I have:

$ objdump -h vmlinux
 14 .init.data    0000051c  8c18605c  8c18605c  0018e05c  2**2
                  CONTENTS, ALLOC, LOAD, DATA

with 0x8cxxxxxx being flash and 0x88xxxxxx being RAM.

I don't understand why test doesn't end in .init.data. Where is the obvious
error? Initializing test to 1 didn't change the output either. Neither does
making test static.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Uwe Kleine-König]

Hello Russell,

On Thu, Feb 05, 2015 at 09:43:23AM +0100, Uwe Kleine-K?nig wrote:
> An unmodified 3.19.0-rc6 + efm32_defconfig boots just fine with XIP (and
> has to little RAM for holding the kernel image in it's 4 MiB RAM).
And btw next fails to build because of your patch
51f36035999ca403d08304176bdfa288a549b50e. Maybe you can limit it to MMU
configs only to prevent false positives?

51f36035999ca403d08304176bdfa288a549b50e^ builds and boots just fine on
my efm32 board.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

On Fri, Feb 06, 2015 at 03:20:33PM +0100, Uwe Kleine-K?nig wrote:
> Hello Russell,
> 
> On Thu, Feb 05, 2015 at 09:43:23AM +0100, Uwe Kleine-K?nig wrote:
> > An unmodified 3.19.0-rc6 + efm32_defconfig boots just fine with XIP (and
> > has to little RAM for holding the kernel image in it's 4 MiB RAM).
> And btw next fails to build because of your patch
> 51f36035999ca403d08304176bdfa288a549b50e. Maybe you can limit it to MMU
> configs only to prevent false positives?

I'm intending to drop that patch; I think it's far too difficult to try
and modify vmlinux.lds to be able to detect these kinds of overflows,
which is a real shame.

I have been experimenting with MEMORY {} over the last few days, but
getting that right is proving to be rather problematical - had all the
LMA adjustments for the kernel already used it, that might have been a
way forward (and it certainly does give us protection against any
section getting too large) but as we don't, it's just too big a patch
with too high a chance of breaking stuff.

Part of the problem is that the linker script is already soo damn
obfuscated because of all the NoMMU vs XIP vs MMU variants that we
support through #ifdef hell - and don't even suggest that we can pull
other preprocessor tricks to sort that - it's the fact that stuff is
conditional that's the problem - that makes it much harder to verify
any change, and ensure that the resulting layout is _actually_ correct
and isn't going to break in some corner case.

Yet again, our complexity gets the better of us, and makes stuff
effectively unmaintainable.

So I guess we're just going to have to put up with a kallsyms failure
indicating that something isn't right.

This is what I ended up with so far:

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 9351f7fbdfb1..85a26ed9bf36 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -54,6 +54,43 @@ jiffies = jiffies_64;
 jiffies = jiffies_64 + 4;
 #endif
 
+#ifndef CONFIG_MMU
+MEMORY
+{
+  FLASH (RXA) : ORIGIN = CONFIG_FLASH_MEM_BASE, LENGTH = CONFIG_FLASH_SIZE
+  RAM (!RX) : ORIGIN = CONFIG_DRAM_BASE, LENGTH = CONFIG_DRAM_SIZE
+}
+#define REGION_RO_VMA >FLASH
+#define REGION_RW_VMA >RAM
+#elif defined(CONFIG_XIP_KERNEL)
+MEMORY
+{
+  XIP_VMA_RO   (RX) : ORIGIN = XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) LENGTH = 16M
+  XIP_VMA_RW   (W)  : ORIGIN = (PAGE_OFFSET + TEXT_OFFSET) LENGTH = 16M
+  XIP_VMA_VEC  (RX) : ORIGIN = 0 LENGTH = 4K
+  XIP_VMA_STUB (RX) : ORIGIN = 4K LENGTH = 4K
+
+  XIP_LMA_RO   (RX) : ORIGIN = XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) LENGTH = 32M
+}
+#define REGION_VEC_VMA  >XIP_VMA_VEC
+#define REGION_STUB_VMA >XIP_VMA_STUB
+#define REGION_RO_VMA   >XIP_VMA_RO
+#define REGION_RO_LMA AT>XIP_LMA_RO
+#define REGION_RW_VMA   >XIP_VMA_RW
+#else
+MEMORY
+{
+  VMA (ra) : ORIGIN = (PAGE_OFFSET + TEXT_OFFSET) LENGTH = 1024M
+  VMA_VEC : ORIGIN = 0 LENGTH = 4K
+  VMA_STUB : ORIGIN = 4K LENGTH = 4K
+}
+#define REGION_VEC_VMA  >VMA_VEC
+#define REGION_STUB_VMA >VMA_STUB
+#define REGION_RO_VMA   >VMA
+#define REGION_RO_LMA   AT>VMA
+#define REGION_RW_VMA   >VMA
+#endif
+
 SECTIONS
 {
 	/*
@@ -92,7 +129,7 @@ SECTIONS
 	.head.text : {
 		_text = .;
 		HEAD_TEXT
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 
 #ifdef CONFIG_ARM_KERNMEM_PERMS
 	. = ALIGN(1<<SECTION_SHIFT);
@@ -118,21 +155,25 @@ SECTIONS
 		. = ALIGN(4);
 		*(.got)			/* Global offset table		*/
 			ARM_CPU_KEEP(PROC_INFO)
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 
 #ifdef CONFIG_DEBUG_RODATA
+	/*
+	 * If we want to ensure that data sections can't be executed,
+	 * we need a section boundary@the end of the .text section.
+	 */
 	. = ALIGN(1<<SECTION_SHIFT);
 #endif
 	RO_DATA(PAGE_SIZE)
 
 	. = ALIGN(4);
-	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
+	__ex_table : {
 		__start___ex_table = .;
 #ifdef CONFIG_MMU
 		*(__ex_table)
 #endif
 		__stop___ex_table = .;
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 
 #ifdef CONFIG_ARM_UNWIND
 	/*
@@ -143,15 +184,19 @@ SECTIONS
 		__start_unwind_idx = .;
 		*(.ARM.exidx*)
 		__stop_unwind_idx = .;
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 	.ARM.unwind_tab : {
 		__start_unwind_tab = .;
 		*(.ARM.extab*)
 		__stop_unwind_tab = .;
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 #endif
 
-	NOTES
+	.notes : {
+		__start_notes = .;
+		*(.note.*)
+		__stop_notes = .;
+	} REGION_RO_VMA REGION_RO_LMA
 
 	_etext = .;			/* End of text and rodata section */
 
@@ -168,66 +213,67 @@ SECTIONS
 	 * only thing that matters is their relative offsets
 	 */
 	__vectors_start = .;
-	.vectors 0 : AT(__vectors_start) {
+	.vectors : {
 		*(.vectors)
-	}
+	} REGION_VEC_VMA REGION_RO_LMA
 	. = __vectors_start + SIZEOF(.vectors);
 	__vectors_end = .;
 
 	__stubs_start = .;
-	.stubs 0x1000 : AT(__stubs_start) {
+	.stubs : {
 		*(.stubs)
-	}
+	} REGION_STUB_VMA REGION_RO_LMA
 	. = __stubs_start + SIZEOF(.stubs);
 	__stubs_end = .;
 
 	INIT_TEXT_SECTION(8)
 	.exit.text : {
 		ARM_EXIT_KEEP(EXIT_TEXT)
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 	.init.proc.info : {
 		ARM_CPU_DISCARD(PROC_INFO)
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 	.init.arch.info : {
 		__arch_info_begin = .;
 		*(.arch.info.init)
 		__arch_info_end = .;
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 	.init.tagtable : {
 		__tagtable_begin = .;
 		*(.taglist.init)
 		__tagtable_end = .;
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 #ifdef CONFIG_SMP_ON_UP
 	.init.smpalt : {
 		__smpalt_begin = .;
 		*(.alt.smp.init)
 		__smpalt_end = .;
-	}
+	} REGION_RO_VMA REGION_RO_LMA
 #endif
 	.init.pv_table : {
 		__pv_table_begin = .;
 		*(.pv_table)
 		__pv_table_end = .;
-	}
-	.init.data : {
-#ifndef CONFIG_XIP_KERNEL
-		INIT_DATA
-#endif
+	} REGION_RO_VMA REGION_RO_LMA
+	.init.const.data : {
 		INIT_SETUP(16)
 		INIT_CALLS
 		CON_INITCALL
 		SECURITY_INITCALL
 		INIT_RAM_FS
-	}
+	} REGION_RO_VMA REGION_RO_LMA
+
+#ifdef CONFIG_SMP
+	PERCPU_SECTION(L1_CACHE_BYTES) REGION_RO_VMA
+#endif
+
 #ifndef CONFIG_XIP_KERNEL
+	.init.data : {
+		INIT_DATA
+	} REGION_RW_VMA
 	.exit.data : {
 		ARM_EXIT_KEEP(EXIT_DATA)
-	}
-#endif
-
-#ifdef CONFIG_SMP
-	PERCPU_SECTION(L1_CACHE_BYTES)
+	} REGION_RW_VMA
 #endif
 
 #ifdef CONFIG_XIP_KERNEL
@@ -273,7 +319,7 @@ SECTIONS
 		CONSTRUCTORS
 
 		_edata = .;
-	}
+	} REGION_RW_VMA
 	_edata_loc = __data_loc + SIZEOF(.data);
 
 #ifdef CONFIG_HAVE_TCM
@@ -359,5 +405,5 @@ ASSERT(((__hyp_idmap_text_end - __hyp_idmap_text_start) <= PAGE_SIZE), "HYP init
  * data section.  Since the .text section starts in module space, and that
  * is always below the .data section, this should be sufficient.
  */
-ASSERT((_data >= __data_loc), "Text section oversize")
+/*ASSERT((_data >= __data_loc), "Text section oversize") */
 #endif
 

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Uwe Kleine-König]

Hello Russell,

On Fri, Feb 06, 2015 at 04:14:28PM +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 06, 2015 at 03:20:33PM +0100, Uwe Kleine-K?nig wrote:
> > On Thu, Feb 05, 2015 at 09:43:23AM +0100, Uwe Kleine-K?nig wrote:
> > > An unmodified 3.19.0-rc6 + efm32_defconfig boots just fine with XIP (and
> > > has to little RAM for holding the kernel image in it's 4 MiB RAM).
> > And btw next fails to build because of your patch
> > 51f36035999ca403d08304176bdfa288a549b50e. Maybe you can limit it to MMU
> > configs only to prevent false positives?
> 
> I'm intending to drop that patch; I think it's far too difficult to try
> and modify vmlinux.lds to be able to detect these kinds of overflows,
> which is a real shame.
I guess you didn't change your mind and just didn't come around yet to
drop this patch?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Russell King - ARM Linux]

On Tue, Feb 10, 2015 at 09:17:12AM +0100, Uwe Kleine-K?nig wrote:
> Hello Russell,
> 
> On Fri, Feb 06, 2015 at 04:14:28PM +0000, Russell King - ARM Linux wrote:
> > On Fri, Feb 06, 2015 at 03:20:33PM +0100, Uwe Kleine-K?nig wrote:
> > > On Thu, Feb 05, 2015 at 09:43:23AM +0100, Uwe Kleine-K?nig wrote:
> > > > An unmodified 3.19.0-rc6 + efm32_defconfig boots just fine with XIP (and
> > > > has to little RAM for holding the kernel image in it's 4 MiB RAM).
> > > And btw next fails to build because of your patch
> > > 51f36035999ca403d08304176bdfa288a549b50e. Maybe you can limit it to MMU
> > > configs only to prevent false positives?
> > 
> > I'm intending to drop that patch; I think it's far too difficult to try
> > and modify vmlinux.lds to be able to detect these kinds of overflows,
> > which is a real shame.
> I guess you didn't change your mind and just didn't come around yet to
> drop this patch?

Yea, I forgot to remove it... It's gone now, but whether it gets picked
up by linux-next or not depends on a certain company fixing their core
internal UK network.  Somehow, I suspect it may take a couple of days
before it's visible in linux-next.

(sfr, don't panic if you can't pull my tree tonight.)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Stephen Rothwell]

Hi Russell,

On Tue, 10 Feb 2015 19:13:13 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>
> (sfr, don't panic if you can't pull my tree tonight.)

Thanks for the heads up.  I have managed to fetch the tree this morning
(but it failed yesterday as you predicted).

-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150212/759790de/attachment.sig>

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Stefan Agner]

On 2015-02-04 10:44, Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 08:59:15PM -0500, Nicolas Pitre wrote:
>> On Wed, 4 Feb 2015, Russell King - ARM Linux wrote:
>>
>> > It looks like we have cases where this falsely triggers.  Consider EFM32:
>> >
>> > CONFIG_DRAM_BASE=0x88000000
>> > CONFIG_DRAM_SIZE=0x00400000
>> > CONFIG_FLASH_MEM_BASE=0x8c000000
>> > CONFIG_FLASH_SIZE=0x01000000
>> >
>> > This means that we quite legally end up with the .data section below the
>> > .text section, which makes:
>> >
>> > ASSERT((_data >= __data_loc), "Text section oversize")
>> >
>> > falsely trigger.
>> >
>> > The linker has the capacity to specify regions of ROM and RAM in the
>> > linker file, I wonder if we should be using those for XIP.  Merely
>> > adding the MEMORY table to the linker file is not good enough - we
>> > also need to explicitly tell the linker which memory regions to place
>> > the output sections, otherwise the linker ends up making assumptions.
>> >
>> > What that means is... asm-generic/vmlinux.lds.h breaks for us.
>> >
>> > Any ideas?  I think using the MEMORY table would be the best approach,
>> > because that should allow us to properly verify that the resulting
>> > binary should fit in the memory regions.
>>
>> Maybe simply having an assert() on the size of the .text section could
>> be all that is needed.  We already know the maximum size in advance.
> 
> That doesn't work, it's not just the .text section but also .rodata,
> __bug_table, __ksymtab, __ksymtab_gpl, __kcrctab, __kcrctab_gpl,
> __ksymtab_strings, __param, __modver, __ex_table, .notes, .vectors,
> .stubs, .init.text, maybe .exit.text, .init.arch.info, .init.tagtable,
> .init.smpalt, .init.pv_table, and apparently .init.data (which is
> surely wrong?)  The following is from Arnd's failing configuration:
> 
>  18 .init.tagtable 00000040  80073a9c  80073a9c  0100ba9c  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  19 .init.data    000010e8  80073adc  80073adc  0100badc  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>  20 .data         003552c4  80008000  80074bc4  01010000  2**8
>                   CONTENTS, ALLOC, LOAD, DATA
> 
> Hmm, if .init.data is contained in the flash section (which it seemingly
> is), it seems that XIP support is currently broken - that section is
> definitely a read/write section.  No one has seemingly noticed that it's
> broken and it's been broken for a long time, so maybe the simple answer
> then is just to rip XIP support out?

Hi Russell,

I noticed that kallsyms resolution fails with XIP kernel in my patchset
to support Vybrids Cortex-M4, see
http://article.gmane.org/gmane.linux.drivers.devicetree/101196

Fwiw, although that platform has enough memory to use a normal kernel
too, the Cortex-M4 architecture has a dedicated code bus. I used the XIP
kernel to make use of the code bus for the kernel code at least.
However, there might be better ways to archive this.

--
Stefan  

Continuing kallsyms failures - large kernels, XIP kernels, and large XIP kernels

[Uwe Kleine-König]

On Sat, Jan 31, 2015 at 12:22:53AM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 30, 2015 at 12:29:30PM -0500, Nicolas Pitre wrote:
> > On Fri, 30 Jan 2015, Russell King - ARM Linux wrote:
> > > +#ifdef CONFIG_XIP_KERNEL
> > > +/*
> > > + * __data_loc is not only the LMA of the data section, but also the VMA of
> > > + * the end of the .rodata section.  This must not overlap the VMA of the
> > > + * data section.  Since the .text section starts in module space, and that
> > > + * is always below the .data section, this should be sufficient.
> > > + */
> > > +ASSERT((_data >= __data_loc), "Text section oversize")
> > > +#endif
> > 
> > I agree with this patch.
> > 
> > Acked-by: Nicolas Pitre <nico@linaro.org>
> > 
> > This might not prevent a config leading to this from happening, but at 
> > least it makes the issue much clearer.  XIP kernel was created for 
> > systems where the total amount of RAM is often smaller than the imposed
> > size limit here.
> 
> Yes, I expect more of Arnd's randconfigs to fail with this patch applied.
> 
> I did also notice that we still have swapper_pg_dir at _data - 0x4000
> for XIP kernels - so the above check is slightly too lenient.  A better
> threshold for __data_loc might be MODULES_END, since we can't allow the
> XIP part to overlap into RAM.
> 
> #ifdef CONFIG_HIGHMEM
> #define MODULES_END             (PAGE_OFFSET - PMD_SIZE)
> #else
> #define MODULES_END             (PAGE_OFFSET)
> #endif
BTW, on no-MMU MODULES_END is defined as

	#define END_MEM                 (UL(CONFIG_DRAM_BASE) + CONFIG_DRAM_SIZE)
	#define MODULES_END             (END_MEM)

I'd like to get rid of CONFIG_DRAM_SIZE because it's stupid to have that
fixed in .config. MODULES_END is hardly used on no-MMU apart from
printing the (IIRC wrong) memory layout during boot.

If we define END_MEM to 0xffffffff on no-MMU this should effectively nop
the assertion which at least prevents false positives?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |