[PATCH] [RFC] ARM: move __bug_table into .data for XIP_KERNEL

[PATCH] [RFC] ARM: move __bug_table into .data for XIP_KERNEL

[Arnd Bergmann]

Matt Hart reports that vf610m4_defconfig kernels grew to 2GB
xipImage size after the __bug_table change.

I tried out a few things and found that moving the bug table
into the .data section avoids this problem. However, the
linker script magic is beyond my capabilities here, so this
is almost certainly not correct.

I've added a few people to Cc that understand this better
than I do, hopefully someone can turn my bogus patch into
a proper one.

Fixes: b5effd3815cc ("debug: Fix __bug_table[] in arch linker scripts")
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Matt Hart <matthew.hart@linaro.org>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Chris Brandt <chris.brandt@renesas.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/kernel/vmlinux-xip.lds.S | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
index a8ceec3e0580..1ebb40ecf411 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -241,12 +241,17 @@ SECTIONS
 		DATA_DATA
 		CONSTRUCTORS
 
+#ifdef CONFIG_GENERIC_BUG
+		__bug_table = ALIGN(8);
+		VMLINUX_SYMBOL(__start___bug_table) = .;
+		KEEP(*(__bug_table))
+		VMLINUX_SYMBOL(__stop___bug_table) = .;
+#endif
+
 		_edata = .;
 	}
 	_edata_loc = __data_loc + SIZEOF(.data);
 
-	BUG_TABLE
-
 #ifdef CONFIG_HAVE_TCM
         /*
 	 * We align everything to a page boundary so we can
-- 
2.9.0

Re: [PATCH] [RFC] ARM: move __bug_table into .data for XIP_KERNEL

[Nicolas Pitre]

On Fri, 28 Jul 2017, Arnd Bergmann wrote:

> Matt Hart reports that vf610m4_defconfig kernels grew to 2GB
> xipImage size after the __bug_table change.
> 
> I tried out a few things and found that moving the bug table
> into the .data section avoids this problem. However, the
> linker script magic is beyond my capabilities here, so this
> is almost certainly not correct.

Well, before your patch the BUG_TABLE location as well as its runtime 
functionality were completely wrong and broken.

> I've added a few people to Cc that understand this better
> than I do, hopefully someone can turn my bogus patch into
> a proper one.

Your patch isn't as bad as you make it, but maybe the following will 
avoid open recoding BUG_TABLE locally:

diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
index 8265b11621..21b4b29c2f 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -237,13 +237,13 @@ SECTIONS
 		 */
 		DATA_DATA
 		CONSTRUCTORS
-
-		_edata = .;
 	}
-	_edata_loc = __data_loc + SIZEOF(.data);
 
 	BUG_TABLE
 
+	_edata = .;
+	_edata_loc = __data_loc + SIZEOF(.data);
+
 #ifdef CONFIG_HAVE_TCM
         /*
 	 * We align everything to a page boundary so we can


Nicolas

Re: [PATCH] [RFC] ARM: move __bug_table into .data for XIP_KERNEL

[Ard Biesheuvel]

> On 28 Jul 2017, at 16:27, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
>> On Fri, 28 Jul 2017, Arnd Bergmann wrote:
>> 
>> Matt Hart reports that vf610m4_defconfig kernels grew to 2GB
>> xipImage size after the __bug_table change.
>> 
>> I tried out a few things and found that moving the bug table
>> into the .data section avoids this problem. However, the
>> linker script magic is beyond my capabilities here, so this
>> is almost certainly not correct.
> 
> Well, before your patch the BUG_TABLE location as well as its runtime 
> functionality were completely wrong and broken.
> 
>> I've added a few people to Cc that understand this better
>> than I do, hopefully someone can turn my bogus patch into
>> a proper one.
> 
> Your patch isn't as bad as you make it, but maybe the following will 
> avoid open recoding BUG_TABLE locally:
> 
> diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
> index 8265b11621..21b4b29c2f 100644
> --- a/arch/arm/kernel/vmlinux-xip.lds.S
> +++ b/arch/arm/kernel/vmlinux-xip.lds.S
> @@ -237,13 +237,13 @@ SECTIONS
>         */
>        DATA_DATA
>        CONSTRUCTORS
> -
> -        _edata = .;
>    }
> -    _edata_loc = __data_loc + SIZEOF(.data);
> 
>    BUG_TABLE
> 

The .data section is deliberately emitted with LMA != VMA so that the code refers to the correct offset in RAM while the initial contents are in ROM and are copied into RAM by the startup code.

This applies to the bug table as well (now that it needs to be writable) so the only correct way to do this is to move it into .data like Arnd's patch does.

I guess we could split the macro so we can emit bug table into an existing section, but in itself, the code is correct, and i don't see a better way of doing it.

> +    _edata = .;
> +    _edata_loc = __data_loc + SIZEOF(.data);
> +
> #ifdef CONFIG_HAVE_TCM
>         /*
>     * We align everything to a page boundary so we can
> 
> 
> Nicolas

Re: [PATCH] [RFC] ARM: move __bug_table into .data for XIP_KERNEL

[Nicolas Pitre]

On Sat, 29 Jul 2017, Ard Biesheuvel wrote:

> 
> 
> > On 28 Jul 2017, at 16:27, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > 
> >> On Fri, 28 Jul 2017, Arnd Bergmann wrote:
> >> 
> >> Matt Hart reports that vf610m4_defconfig kernels grew to 2GB
> >> xipImage size after the __bug_table change.
> >> 
> >> I tried out a few things and found that moving the bug table
> >> into the .data section avoids this problem. However, the
> >> linker script magic is beyond my capabilities here, so this
> >> is almost certainly not correct.
> > 
> > Well, before your patch the BUG_TABLE location as well as its runtime 
> > functionality were completely wrong and broken.
> > 
> >> I've added a few people to Cc that understand this better
> >> than I do, hopefully someone can turn my bogus patch into
> >> a proper one.
> > 
> > Your patch isn't as bad as you make it, but maybe the following will 
> > avoid open recoding BUG_TABLE locally:
> > 
> > diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
> > index 8265b11621..21b4b29c2f 100644
> > --- a/arch/arm/kernel/vmlinux-xip.lds.S
> > +++ b/arch/arm/kernel/vmlinux-xip.lds.S
> > @@ -237,13 +237,13 @@ SECTIONS
> >         */
> >        DATA_DATA
> >        CONSTRUCTORS
> > -
> > -        _edata = .;
> >    }
> > -    _edata_loc = __data_loc + SIZEOF(.data);
> > 
> >    BUG_TABLE
> > 
> 
> The .data section is deliberately emitted with LMA != VMA so that the 
> code refers to the correct offset in RAM while the initial contents 
> are in ROM and are copied into RAM by the startup code.

You're right of course.  And I have no excuse as the relevant part of 
the startup code is actually mine.

> This applies to the bug table as well (now that it needs to be 
> writable) so the only correct way to do this is to move it into .data 
> like Arnd's patch does.

I got distracted by trying to reuse the macro as is and therefore I 
didn't fix anything.

> I guess we could split the macro so we can emit bug table into an 
> existing section, but in itself, the code is correct, and i don't see 
> a better way of doing it.

Agreed.


Nicolas