* [PATCH] ARM: compressed: discard ksym/kcrctab input section
@ 2017-09-08 15:31 Ard Biesheuvel
2017-09-08 15:39 ` Gregory CLEMENT
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2017-09-08 15:31 UTC (permalink / raw)
To: linux-arm-kernel
As it turns out, building the ARM kernel with EFI support pulls in
a couple of sections that we don't really need in the decompressor.
This is due to the fact the the UEFI stub uses sort() to sort the UEFI
memory map, which is an exported symbol pulled in from lib/sort.c.
Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
separate PE/COFF sections"), this resulted in the following layout
for the decompressor ELF binary.
[Nr] Name Type Addr Off Size ES Flg Lk Inf Al
[ 0] NULL 00000000 000000 000000 00 0 0 0
[ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
[ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
[ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
[ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
[ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
[ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
[ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
[ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
[ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
[10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
[11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
Commit e4bae4d0b5f3 made some changes to the linker script to allow the
UEFI firmware to map the decompressor with strict R-X/RW- permissions
before invoking it. Unfortunately, this turns out to break the boot on
some systems, because the linker now also moves the ksymtab/kcrctab
sections around, resulting in .piggydata to appear misaligned.
[Nr] Name Type Addr Off Size ES Flg Lk Inf Al
[ 0] NULL 00000000 000000 000000 00 0 0 0
[ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
[ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
[ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
[ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
[ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
[ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
[ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
[ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
[ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
[10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
[11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
So let's align piggydata explicitly, and discard these sections from the
binary.
Cc: Russell King <linux@armlinux.org.uk>
Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/boot/compressed/piggy.S | 1 +
arch/arm/boot/compressed/vmlinux.lds.S | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm/boot/compressed/piggy.S b/arch/arm/boot/compressed/piggy.S
index f72088495f43..5d52c556dd32 100644
--- a/arch/arm/boot/compressed/piggy.S
+++ b/arch/arm/boot/compressed/piggy.S
@@ -1,5 +1,6 @@
.section .piggydata,#alloc
.globl input_data
+ .align 2
input_data:
.incbin "arch/arm/boot/compressed/piggy_data"
.globl input_data_end
diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index 7a4c59154361..5c5265be4605 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -29,6 +29,7 @@ SECTIONS
* of the text/got segments.
*/
*(.data)
+ *(*ksymtab* *kcrctab*)
}
. = TEXT_START;
--
2.11.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-09-08 15:31 [PATCH] ARM: compressed: discard ksym/kcrctab input section Ard Biesheuvel
@ 2017-09-08 15:39 ` Gregory CLEMENT
2017-09-08 15:41 ` Ard Biesheuvel
2017-10-04 12:16 ` Gregory CLEMENT
2017-10-21 7:56 ` Matthias Brugger
2 siblings, 1 reply; 30+ messages in thread
From: Gregory CLEMENT @ 2017-09-08 15:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> As it turns out, building the ARM kernel with EFI support pulls in
> a couple of sections that we don't really need in the decompressor.
> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
> memory map, which is an exported symbol pulled in from lib/sort.c.
>
> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
> separate PE/COFF sections"), this resulted in the following layout
> for the decompressor ELF binary.
>
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>
> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
> UEFI firmware to map the decompressor with strict R-X/RW- permissions
> before invoking it. Unfortunately, this turns out to break the boot on
> some systems, because the linker now also moves the ksymtab/kcrctab
> sections around, resulting in .piggydata to appear misaligned.
>
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>
> So let's align piggydata explicitly, and discard these sections from the
> binary.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Actually I had not tested a kernel with these two changes in the same
time. But now I've just done it, and it still works, so the patch and my
tested-by remain valid.
Thanks!
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm/boot/compressed/piggy.S | 1 +
> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/compressed/piggy.S b/arch/arm/boot/compressed/piggy.S
> index f72088495f43..5d52c556dd32 100644
> --- a/arch/arm/boot/compressed/piggy.S
> +++ b/arch/arm/boot/compressed/piggy.S
> @@ -1,5 +1,6 @@
> .section .piggydata,#alloc
> .globl input_data
> + .align 2
> input_data:
> .incbin "arch/arm/boot/compressed/piggy_data"
> .globl input_data_end
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> index 7a4c59154361..5c5265be4605 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.S
> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> @@ -29,6 +29,7 @@ SECTIONS
> * of the text/got segments.
> */
> *(.data)
> + *(*ksymtab* *kcrctab*)
> }
>
> . = TEXT_START;
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-09-08 15:39 ` Gregory CLEMENT
@ 2017-09-08 15:41 ` Ard Biesheuvel
0 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2017-09-08 15:41 UTC (permalink / raw)
To: linux-arm-kernel
On 8 September 2017 at 16:39, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Ard,
>
> On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> As it turns out, building the ARM kernel with EFI support pulls in
>> a couple of sections that we don't really need in the decompressor.
>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>
>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>> separate PE/COFF sections"), this resulted in the following layout
>> for the decompressor ELF binary.
>>
>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>
>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>> before invoking it. Unfortunately, this turns out to break the boot on
>> some systems, because the linker now also moves the ksymtab/kcrctab
>> sections around, resulting in .piggydata to appear misaligned.
>>
>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>
>> So let's align piggydata explicitly, and discard these sections from the
>> binary.
>>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Actually I had not tested a kernel with these two changes in the same
> time. But now I've just done it, and it still works, so the patch and my
> tested-by remain valid.
>
> Thanks!
>
Thanks for confirming. I was a bit sneaky, but the added .align can
never make a difference when the section is already aligned. I don't
understand why just the .align is not sufficient, but the patch is
simple enough so I think I am not going to investigate any further.
Thanks
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-09-08 15:31 [PATCH] ARM: compressed: discard ksym/kcrctab input section Ard Biesheuvel
2017-09-08 15:39 ` Gregory CLEMENT
@ 2017-10-04 12:16 ` Gregory CLEMENT
2017-10-04 12:20 ` Ard Biesheuvel
2017-10-21 7:56 ` Matthias Brugger
2 siblings, 1 reply; 30+ messages in thread
From: Gregory CLEMENT @ 2017-10-04 12:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> As it turns out, building the ARM kernel with EFI support pulls in
> a couple of sections that we don't really need in the decompressor.
> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
> memory map, which is an exported symbol pulled in from lib/sort.c.
>
> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
> separate PE/COFF sections"), this resulted in the following layout
> for the decompressor ELF binary.
>
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>
> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
> UEFI firmware to map the decompressor with strict R-X/RW- permissions
> before invoking it. Unfortunately, this turns out to break the boot on
> some systems, because the linker now also moves the ksymtab/kcrctab
> sections around, resulting in .piggydata to appear misaligned.
>
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>
> So let's align piggydata explicitly, and discard these sections from the
> binary.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Any new for this patch?
It is not yet in v4.14-rc whereas "arm/efi: Split zImage code and data
into separate ..." was already merged. So currently I have many boards
which still does not boot in v4.14-rc3.
Gregory
> ---
> arch/arm/boot/compressed/piggy.S | 1 +
> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/compressed/piggy.S b/arch/arm/boot/compressed/piggy.S
> index f72088495f43..5d52c556dd32 100644
> --- a/arch/arm/boot/compressed/piggy.S
> +++ b/arch/arm/boot/compressed/piggy.S
> @@ -1,5 +1,6 @@
> .section .piggydata,#alloc
> .globl input_data
> + .align 2
> input_data:
> .incbin "arch/arm/boot/compressed/piggy_data"
> .globl input_data_end
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> index 7a4c59154361..5c5265be4605 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.S
> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> @@ -29,6 +29,7 @@ SECTIONS
> * of the text/got segments.
> */
> *(.data)
> + *(*ksymtab* *kcrctab*)
> }
>
> . = TEXT_START;
> --
> 2.11.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-04 12:16 ` Gregory CLEMENT
@ 2017-10-04 12:20 ` Ard Biesheuvel
2017-10-04 12:43 ` Russell King - ARM Linux
0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-04 12:20 UTC (permalink / raw)
To: linux-arm-kernel
On 4 October 2017 at 13:16, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Ard,
>
> On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> As it turns out, building the ARM kernel with EFI support pulls in
>> a couple of sections that we don't really need in the decompressor.
>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>
>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>> separate PE/COFF sections"), this resulted in the following layout
>> for the decompressor ELF binary.
>>
>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>
>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>> before invoking it. Unfortunately, this turns out to break the boot on
>> some systems, because the linker now also moves the ksymtab/kcrctab
>> sections around, resulting in .piggydata to appear misaligned.
>>
>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>
>> So let's align piggydata explicitly, and discard these sections from the
>> binary.
>>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Any new for this patch?
>
> It is not yet in v4.14-rc whereas "arm/efi: Split zImage code and data
> into separate ..." was already merged. So currently I have many boards
> which still does not boot in v4.14-rc3.
>
Russell, any objections?
>> ---
>> arch/arm/boot/compressed/piggy.S | 1 +
>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/compressed/piggy.S b/arch/arm/boot/compressed/piggy.S
>> index f72088495f43..5d52c556dd32 100644
>> --- a/arch/arm/boot/compressed/piggy.S
>> +++ b/arch/arm/boot/compressed/piggy.S
>> @@ -1,5 +1,6 @@
>> .section .piggydata,#alloc
>> .globl input_data
>> + .align 2
>> input_data:
>> .incbin "arch/arm/boot/compressed/piggy_data"
>> .globl input_data_end
>> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
>> index 7a4c59154361..5c5265be4605 100644
>> --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> @@ -29,6 +29,7 @@ SECTIONS
>> * of the text/got segments.
>> */
>> *(.data)
>> + *(*ksymtab* *kcrctab*)
>> }
>>
>> . = TEXT_START;
>> --
>> 2.11.0
>>
>
> --
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-04 12:20 ` Ard Biesheuvel
@ 2017-10-04 12:43 ` Russell King - ARM Linux
2017-10-09 12:39 ` Ard Biesheuvel
0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-04 12:43 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 04, 2017 at 01:20:26PM +0100, Ard Biesheuvel wrote:
> On 4 October 2017 at 13:16, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> > Hi Ard,
> >
> > On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> >> As it turns out, building the ARM kernel with EFI support pulls in
> >> a couple of sections that we don't really need in the decompressor.
> >> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
> >> memory map, which is an exported symbol pulled in from lib/sort.c.
> >>
> >> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
> >> separate PE/COFF sections"), this resulted in the following layout
> >> for the decompressor ELF binary.
> >>
> >> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> >> [ 0] NULL 00000000 000000 000000 00 0 0 0
> >> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
> >> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
> >> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
> >> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
> >> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
> >> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
> >> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
> >> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
> >> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
> >> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
> >> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
> >>
> >> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
> >> UEFI firmware to map the decompressor with strict R-X/RW- permissions
> >> before invoking it. Unfortunately, this turns out to break the boot on
> >> some systems, because the linker now also moves the ksymtab/kcrctab
> >> sections around, resulting in .piggydata to appear misaligned.
> >>
> >> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> >> [ 0] NULL 00000000 000000 000000 00 0 0 0
> >> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
> >> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
> >> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
> >> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
> >> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
> >> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
> >> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
> >> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
> >> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
> >> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
> >> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
> >>
> >> So let's align piggydata explicitly, and discard these sections from the
> >> binary.
> >>
> >> Cc: Russell King <linux@armlinux.org.uk>
> >> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
> >> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > Any new for this patch?
> >
> > It is not yet in v4.14-rc whereas "arm/efi: Split zImage code and data
> > into separate ..." was already merged. So currently I have many boards
> > which still does not boot in v4.14-rc3.
> >
>
> Russell, any objections?
It would be nice if there was something in the commit log that described
why we need to align data that is basically a byte stream, and which
decompressor methods it affects. Maybe the decompressors should cope
with a misaligned byte stream - what if (for example) someone supplies
the kernel with a compressed initramfs image that is not word aligned?
We already have people using non-page aligned compressed initramfs
images.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-04 12:43 ` Russell King - ARM Linux
@ 2017-10-09 12:39 ` Ard Biesheuvel
2017-10-12 9:24 ` Gregory CLEMENT
0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-09 12:39 UTC (permalink / raw)
To: linux-arm-kernel
On 4 October 2017 at 13:43, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Oct 04, 2017 at 01:20:26PM +0100, Ard Biesheuvel wrote:
>> On 4 October 2017 at 13:16, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>> > Hi Ard,
>> >
>> > On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >
>> >> As it turns out, building the ARM kernel with EFI support pulls in
>> >> a couple of sections that we don't really need in the decompressor.
>> >> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>> >> memory map, which is an exported symbol pulled in from lib/sort.c.
>> >>
>> >> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>> >> separate PE/COFF sections"), this resulted in the following layout
>> >> for the decompressor ELF binary.
>> >>
>> >> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> >> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> >> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>> >> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>> >> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>> >> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>> >> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>> >> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>> >> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>> >> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>> >> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>> >> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>> >> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>> >>
>> >> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>> >> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>> >> before invoking it. Unfortunately, this turns out to break the boot on
>> >> some systems, because the linker now also moves the ksymtab/kcrctab
>> >> sections around, resulting in .piggydata to appear misaligned.
>> >>
>> >> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> >> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> >> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>> >> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>> >> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>> >> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>> >> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>> >> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>> >> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>> >> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>> >> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>> >> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>> >> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>> >>
>> >> So let's align piggydata explicitly, and discard these sections from the
>> >> binary.
>> >>
>> >> Cc: Russell King <linux@armlinux.org.uk>
>> >> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>> >> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >
>> > Any new for this patch?
>> >
>> > It is not yet in v4.14-rc whereas "arm/efi: Split zImage code and data
>> > into separate ..." was already merged. So currently I have many boards
>> > which still does not boot in v4.14-rc3.
>> >
>>
>> Russell, any objections?
>
> It would be nice if there was something in the commit log that described
> why we need to align data that is basically a byte stream, and which
> decompressor methods it affects. Maybe the decompressors should cope
> with a misaligned byte stream - what if (for example) someone supplies
> the kernel with a compressed initramfs image that is not word aligned?
The decompressor copes with a misaligned byte stream by using
get_unaligned et al. Only, on v7, these are simply converted to word
wide unaligned accesses, which the compiler may merge into ldm/stm if
they occur adjacently. In the kernel proper, this is caught and fixed
up by the alignment fixup code, but in the decompressor you hit the
fault.
> We already have people using non-page aligned compressed initramfs
> images.
>
Yes, but initramfs accesses are fixed up by the alignment fixup code as well.
So I suppose Arnd's patch to switch to the struct type unaligned
accessor would deal with this issue as well.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-09 12:39 ` Ard Biesheuvel
@ 2017-10-12 9:24 ` Gregory CLEMENT
2017-10-12 9:45 ` Russell King - ARM Linux
0 siblings, 1 reply; 30+ messages in thread
From: Gregory CLEMENT @ 2017-10-12 9:24 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On lun., oct. 09 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 4 October 2017 at 13:43, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Wed, Oct 04, 2017 at 01:20:26PM +0100, Ard Biesheuvel wrote:
>>> On 4 October 2017 at 13:16, Gregory CLEMENT
>>> <gregory.clement@free-electrons.com> wrote:
>>> > Hi Ard,
>>> >
>>> > On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> >
>>> >> As it turns out, building the ARM kernel with EFI support pulls in
>>> >> a couple of sections that we don't really need in the decompressor.
>>> >> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>>> >> memory map, which is an exported symbol pulled in from lib/sort.c.
>>> >>
>>> >> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>>> >> separate PE/COFF sections"), this resulted in the following layout
>>> >> for the decompressor ELF binary.
>>> >>
>>> >> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>> >> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>> >> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>>> >> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>>> >> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>>> >> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>>> >> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>>> >> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>>> >> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>>> >> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>>> >> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>>> >> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>>> >> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>> >>
>>> >> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>>> >> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>>> >> before invoking it. Unfortunately, this turns out to break the boot on
>>> >> some systems, because the linker now also moves the ksymtab/kcrctab
>>> >> sections around, resulting in .piggydata to appear misaligned.
>>> >>
>>> >> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>> >> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>> >> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>>> >> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>>> >> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>>> >> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>>> >> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>>> >> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>>> >> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>>> >> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>>> >> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>>> >> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>>> >> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>> >>
>>> >> So let's align piggydata explicitly, and discard these sections from the
>>> >> binary.
>>> >>
>>> >> Cc: Russell King <linux@armlinux.org.uk>
>>> >> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>>> >> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> >
>>> > Any new for this patch?
>>> >
>>> > It is not yet in v4.14-rc whereas "arm/efi: Split zImage code and data
>>> > into separate ..." was already merged. So currently I have many boards
>>> > which still does not boot in v4.14-rc3.
>>> >
>>>
>>> Russell, any objections?
>>
>> It would be nice if there was something in the commit log that described
>> why we need to align data that is basically a byte stream, and which
>> decompressor methods it affects. Maybe the decompressors should cope
>> with a misaligned byte stream - what if (for example) someone supplies
>> the kernel with a compressed initramfs image that is not word aligned?
>
> The decompressor copes with a misaligned byte stream by using
> get_unaligned et al. Only, on v7, these are simply converted to word
> wide unaligned accesses, which the compiler may merge into ldm/stm if
> they occur adjacently. In the kernel proper, this is caught and fixed
> up by the alignment fixup code, but in the decompressor you hit the
> fault.
>
Can we move forward to fix the booting problem ?
What about amending your commit log with this new information and then
submit it to Russell patch system?
Thanks,
Gregory
>> We already have people using non-page aligned compressed initramfs
>> images.
>>
>
> Yes, but initramfs accesses are fixed up by the alignment fixup code as well.
>
> So I suppose Arnd's patch to switch to the struct type unaligned
> accessor would deal with this issue as well.
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-12 9:24 ` Gregory CLEMENT
@ 2017-10-12 9:45 ` Russell King - ARM Linux
2017-10-12 19:03 ` Ard Biesheuvel
0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-12 9:45 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
> Hi Ard,
>
> Can we move forward to fix the booting problem ?
>
> What about amending your commit log with this new information and then
> submit it to Russell patch system?
Well, I think there's a choice that needs to be made between this
approach and Arnd's approach.
I'm not all that thrilled with the need to add explicit alignment to
data that is inherently a byte stream, and that invariably results in
unaligned data words even if you do align the start of it. That
sounds to me very much like a hack rather than a proper solution.
So, right now I'm leaning more towards Arnd's solution than Ard's
from what's been said in this thread.
However, I don't recall Arnd's patch, it's probably buried deep in
my mailbox.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-12 9:45 ` Russell King - ARM Linux
@ 2017-10-12 19:03 ` Ard Biesheuvel
2017-10-20 15:25 ` Gregory CLEMENT
0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-12 19:03 UTC (permalink / raw)
To: linux-arm-kernel
On 12 October 2017 at 10:45, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
>> Hi Ard,
>>
>> Can we move forward to fix the booting problem ?
>>
>> What about amending your commit log with this new information and then
>> submit it to Russell patch system?
>
> Well, I think there's a choice that needs to be made between this
> approach and Arnd's approach.
>
> I'm not all that thrilled with the need to add explicit alignment to
> data that is inherently a byte stream, and that invariably results in
> unaligned data words even if you do align the start of it. That
> sounds to me very much like a hack rather than a proper solution.
> So, right now I'm leaning more towards Arnd's solution than Ard's
> from what's been said in this thread.
>
I agree that the struct type unaligned accessors are the best choice
for ARM in any case, given that it will also prevent hitting the
alignment fixup handler in the kernel unnecessarily.
> However, I don't recall Arnd's patch, it's probably buried deep in
> my mailbox.
>
Well, unless you are considering changing the unaligned accessors from
access_ok.h to le_struct.h as a bugfix, I think we need both patches.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-12 19:03 ` Ard Biesheuvel
@ 2017-10-20 15:25 ` Gregory CLEMENT
2017-10-20 15:28 ` Ard Biesheuvel
0 siblings, 1 reply; 30+ messages in thread
From: Gregory CLEMENT @ 2017-10-20 15:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On jeu., oct. 12 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 12 October 2017 at 10:45, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
>>> Hi Ard,
>>>
>>> Can we move forward to fix the booting problem ?
>>>
>>> What about amending your commit log with this new information and then
>>> submit it to Russell patch system?
>>
>> Well, I think there's a choice that needs to be made between this
>> approach and Arnd's approach.
>>
>> I'm not all that thrilled with the need to add explicit alignment to
>> data that is inherently a byte stream, and that invariably results in
>> unaligned data words even if you do align the start of it. That
>> sounds to me very much like a hack rather than a proper solution.
>> So, right now I'm leaning more towards Arnd's solution than Ard's
>> from what's been said in this thread.
>>
>
> I agree that the struct type unaligned accessors are the best choice
> for ARM in any case, given that it will also prevent hitting the
> alignment fixup handler in the kernel unnecessarily.
>
>> However, I don't recall Arnd's patch, it's probably buried deep in
>> my mailbox.
>>
>
> Well, unless you are considering changing the unaligned accessors from
> access_ok.h to le_struct.h as a bugfix, I think we need both patches.
We will soon reach v4.14-rc6 and the Armada XP and Armada 370 still not
boot. I also didn't see your patch in rmk patch system.
Waiting for you find a agreement an other option is to remove CONFIG_EFI
from multi_v7_defconfig as I don't really see any armv7 base board using
EFI.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 15:25 ` Gregory CLEMENT
@ 2017-10-20 15:28 ` Ard Biesheuvel
2017-10-20 16:11 ` Russell King - ARM Linux
0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-20 15:28 UTC (permalink / raw)
To: linux-arm-kernel
On 20 October 2017 at 16:25, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Ard,
>
> On jeu., oct. 12 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 12 October 2017 at 10:45, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>>> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
>>>> Hi Ard,
>>>>
>>>> Can we move forward to fix the booting problem ?
>>>>
>>>> What about amending your commit log with this new information and then
>>>> submit it to Russell patch system?
>>>
>>> Well, I think there's a choice that needs to be made between this
>>> approach and Arnd's approach.
>>>
>>> I'm not all that thrilled with the need to add explicit alignment to
>>> data that is inherently a byte stream, and that invariably results in
>>> unaligned data words even if you do align the start of it. That
>>> sounds to me very much like a hack rather than a proper solution.
>>> So, right now I'm leaning more towards Arnd's solution than Ard's
>>> from what's been said in this thread.
>>>
>>
>> I agree that the struct type unaligned accessors are the best choice
>> for ARM in any case, given that it will also prevent hitting the
>> alignment fixup handler in the kernel unnecessarily.
>>
>>> However, I don't recall Arnd's patch, it's probably buried deep in
>>> my mailbox.
>>>
>>
>> Well, unless you are considering changing the unaligned accessors from
>> access_ok.h to le_struct.h as a bugfix, I think we need both patches.
>
> We will soon reach v4.14-rc6 and the Armada XP and Armada 370 still not
> boot. I also didn't see your patch in rmk patch system.
>
> Waiting for you find a agreement an other option is to remove CONFIG_EFI
> from multi_v7_defconfig as I don't really see any armv7 base board using
> EFI.
>
It is up to Russell to decide how he wants to proceed. Russell?
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 15:28 ` Ard Biesheuvel
@ 2017-10-20 16:11 ` Russell King - ARM Linux
2017-10-20 16:20 ` Ard Biesheuvel
0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-20 16:11 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 20, 2017 at 04:28:49PM +0100, Ard Biesheuvel wrote:
> On 20 October 2017 at 16:25, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> > Hi Ard,
> >
> > On jeu., oct. 12 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> >> On 12 October 2017 at 10:45, Russell King - ARM Linux
> >> <linux@armlinux.org.uk> wrote:
> >>> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
> >>>> Hi Ard,
> >>>>
> >>>> Can we move forward to fix the booting problem ?
> >>>>
> >>>> What about amending your commit log with this new information and then
> >>>> submit it to Russell patch system?
> >>>
> >>> Well, I think there's a choice that needs to be made between this
> >>> approach and Arnd's approach.
> >>>
> >>> I'm not all that thrilled with the need to add explicit alignment to
> >>> data that is inherently a byte stream, and that invariably results in
> >>> unaligned data words even if you do align the start of it. That
> >>> sounds to me very much like a hack rather than a proper solution.
> >>> So, right now I'm leaning more towards Arnd's solution than Ard's
> >>> from what's been said in this thread.
> >>>
> >>
> >> I agree that the struct type unaligned accessors are the best choice
> >> for ARM in any case, given that it will also prevent hitting the
> >> alignment fixup handler in the kernel unnecessarily.
> >>
> >>> However, I don't recall Arnd's patch, it's probably buried deep in
> >>> my mailbox.
> >>>
> >>
> >> Well, unless you are considering changing the unaligned accessors from
> >> access_ok.h to le_struct.h as a bugfix, I think we need both patches.
> >
> > We will soon reach v4.14-rc6 and the Armada XP and Armada 370 still not
> > boot. I also didn't see your patch in rmk patch system.
> >
> > Waiting for you find a agreement an other option is to remove CONFIG_EFI
> > from multi_v7_defconfig as I don't really see any armv7 base board using
> > EFI.
> >
>
> It is up to Russell to decide how he wants to proceed. Russell?
Well, having failed to attract Arnd's attention, I've spent 20 minutes
searching my mailbox to find it.
It turns out that there was never a proper patch from Arnd - it was a
patch in pastebin. It's not ARM specific either, it's an asm-generic
change, for which Arnd is the maintainer for.
I've just replied to that old thread and I've included Gregory and some
PXA folk who are also having alignment problems in the decompressor. It
could all be due to this same issue.
There's also one additional factor that hasn't been considered, and I'm
scared to point it out, but:
1. Does Arnd's patch fix the PXA problem as well?
2. What is the performance impact of Arnd's fix?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 16:11 ` Russell King - ARM Linux
@ 2017-10-20 16:20 ` Ard Biesheuvel
2017-10-20 16:32 ` Russell King - ARM Linux
0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-20 16:20 UTC (permalink / raw)
To: linux-arm-kernel
On 20 October 2017 at 17:11, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Fri, Oct 20, 2017 at 04:28:49PM +0100, Ard Biesheuvel wrote:
>> On 20 October 2017 at 16:25, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>> > Hi Ard,
>> >
>> > On jeu., oct. 12 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >
>> >> On 12 October 2017 at 10:45, Russell King - ARM Linux
>> >> <linux@armlinux.org.uk> wrote:
>> >>> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
>> >>>> Hi Ard,
>> >>>>
>> >>>> Can we move forward to fix the booting problem ?
>> >>>>
>> >>>> What about amending your commit log with this new information and then
>> >>>> submit it to Russell patch system?
>> >>>
>> >>> Well, I think there's a choice that needs to be made between this
>> >>> approach and Arnd's approach.
>> >>>
>> >>> I'm not all that thrilled with the need to add explicit alignment to
>> >>> data that is inherently a byte stream, and that invariably results in
>> >>> unaligned data words even if you do align the start of it. That
>> >>> sounds to me very much like a hack rather than a proper solution.
>> >>> So, right now I'm leaning more towards Arnd's solution than Ard's
>> >>> from what's been said in this thread.
>> >>>
>> >>
>> >> I agree that the struct type unaligned accessors are the best choice
>> >> for ARM in any case, given that it will also prevent hitting the
>> >> alignment fixup handler in the kernel unnecessarily.
>> >>
>> >>> However, I don't recall Arnd's patch, it's probably buried deep in
>> >>> my mailbox.
>> >>>
>> >>
>> >> Well, unless you are considering changing the unaligned accessors from
>> >> access_ok.h to le_struct.h as a bugfix, I think we need both patches.
>> >
>> > We will soon reach v4.14-rc6 and the Armada XP and Armada 370 still not
>> > boot. I also didn't see your patch in rmk patch system.
>> >
>> > Waiting for you find a agreement an other option is to remove CONFIG_EFI
>> > from multi_v7_defconfig as I don't really see any armv7 base board using
>> > EFI.
>> >
>>
>> It is up to Russell to decide how he wants to proceed. Russell?
>
> Well, having failed to attract Arnd's attention, I've spent 20 minutes
> searching my mailbox to find it.
>
> It turns out that there was never a proper patch from Arnd - it was a
> patch in pastebin. It's not ARM specific either, it's an asm-generic
> change, for which Arnd is the maintainer for.
>
> I've just replied to that old thread and I've included Gregory and some
> PXA folk who are also having alignment problems in the decompressor. It
> could all be due to this same issue.
>
> There's also one additional factor that hasn't been considered, and I'm
> scared to point it out, but:
>
> 1. Does Arnd's patch fix the PXA problem as well?
>
I don't think it will help configs that don't have
HAVE_EFFICIENT_UNALIGNED_ACCESS set, given that they don't use
access_ok.h in the first place.
> 2. What is the performance impact of Arnd's fix?
>
Well, given that we may be relying on the alignment fixup handler to
fix up kernel accesses, the performance impact could actually be
favorable in some cases. But I don't have any numbers, nor do I have
access to a representative sampling of ARM hardware so I can't be of
any help here, unfortunately.
--
Ard.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 16:20 ` Ard Biesheuvel
@ 2017-10-20 16:32 ` Russell King - ARM Linux
2017-10-20 16:36 ` Ard Biesheuvel
0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-20 16:32 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 20, 2017 at 05:20:26PM +0100, Ard Biesheuvel wrote:
> On 20 October 2017 at 17:11, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Fri, Oct 20, 2017 at 04:28:49PM +0100, Ard Biesheuvel wrote:
> >> On 20 October 2017 at 16:25, Gregory CLEMENT
> >> <gregory.clement@free-electrons.com> wrote:
> >> > Hi Ard,
> >> >
> >> > On jeu., oct. 12 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >
> >> >> On 12 October 2017 at 10:45, Russell King - ARM Linux
> >> >> <linux@armlinux.org.uk> wrote:
> >> >>> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
> >> >>>> Hi Ard,
> >> >>>>
> >> >>>> Can we move forward to fix the booting problem ?
> >> >>>>
> >> >>>> What about amending your commit log with this new information and then
> >> >>>> submit it to Russell patch system?
> >> >>>
> >> >>> Well, I think there's a choice that needs to be made between this
> >> >>> approach and Arnd's approach.
> >> >>>
> >> >>> I'm not all that thrilled with the need to add explicit alignment to
> >> >>> data that is inherently a byte stream, and that invariably results in
> >> >>> unaligned data words even if you do align the start of it. That
> >> >>> sounds to me very much like a hack rather than a proper solution.
> >> >>> So, right now I'm leaning more towards Arnd's solution than Ard's
> >> >>> from what's been said in this thread.
> >> >>>
> >> >>
> >> >> I agree that the struct type unaligned accessors are the best choice
> >> >> for ARM in any case, given that it will also prevent hitting the
> >> >> alignment fixup handler in the kernel unnecessarily.
> >> >>
> >> >>> However, I don't recall Arnd's patch, it's probably buried deep in
> >> >>> my mailbox.
> >> >>>
> >> >>
> >> >> Well, unless you are considering changing the unaligned accessors from
> >> >> access_ok.h to le_struct.h as a bugfix, I think we need both patches.
> >> >
> >> > We will soon reach v4.14-rc6 and the Armada XP and Armada 370 still not
> >> > boot. I also didn't see your patch in rmk patch system.
> >> >
> >> > Waiting for you find a agreement an other option is to remove CONFIG_EFI
> >> > from multi_v7_defconfig as I don't really see any armv7 base board using
> >> > EFI.
> >> >
> >>
> >> It is up to Russell to decide how he wants to proceed. Russell?
> >
> > Well, having failed to attract Arnd's attention, I've spent 20 minutes
> > searching my mailbox to find it.
> >
> > It turns out that there was never a proper patch from Arnd - it was a
> > patch in pastebin. It's not ARM specific either, it's an asm-generic
> > change, for which Arnd is the maintainer for.
> >
> > I've just replied to that old thread and I've included Gregory and some
> > PXA folk who are also having alignment problems in the decompressor. It
> > could all be due to this same issue.
> >
> > There's also one additional factor that hasn't been considered, and I'm
> > scared to point it out, but:
> >
> > 1. Does Arnd's patch fix the PXA problem as well?
> >
>
> I don't think it will help configs that don't have
> HAVE_EFFICIENT_UNALIGNED_ACCESS set, given that they don't use
> access_ok.h in the first place.
>
> > 2. What is the performance impact of Arnd's fix?
> >
>
> Well, given that we may be relying on the alignment fixup handler to
> fix up kernel accesses, the performance impact could actually be
> favorable in some cases. But I don't have any numbers, nor do I have
> access to a representative sampling of ARM hardware so I can't be of
> any help here, unfortunately.
Well, everyone can tell whether alignment fixups get used on their
platforms, by looking at /proc/cpu/alignment - this counts any
alignment faults and their types. If it's all zeros, then the
alignment fixup handler isn't being used.
I just checked one of my imx6 platforms (3G/wifi gateway), all the
stats are zero. Another imx6 platform (whose port 80 is open to the
world) has one double-word fault in nsm_get_handle+0x200/0x484.
My ARMv5 gateway here also has all zero stats.
However, all these are built with GCC 4.7.4, and we already know
that newer compilers behave significantly differently. So, I don't
think what I see here is representative, so I can't decide based on
what I see locally.
So, it seems we're rather stuck.
I suppose I could set up a dart board, and throw a dart blind folded
and if it hits a red area, pick one patch, otherwise take the other.
Or toss a coin. Or some other rediculous game.
It would be much better if there was some way to evaluate the impact,
but I see that no one is interested in lifting a finger to help with
that.
Meanwhile, I'm quite sure there'll be an on-going cry for me to make
a decision. A decision based on a whim. Yea, great basis for taking
decisions.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 16:32 ` Russell King - ARM Linux
@ 2017-10-20 16:36 ` Ard Biesheuvel
2017-10-20 16:54 ` Russell King - ARM Linux
0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-20 16:36 UTC (permalink / raw)
To: linux-arm-kernel
On 20 October 2017 at 17:32, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Fri, Oct 20, 2017 at 05:20:26PM +0100, Ard Biesheuvel wrote:
>> On 20 October 2017 at 17:11, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > On Fri, Oct 20, 2017 at 04:28:49PM +0100, Ard Biesheuvel wrote:
>> >> On 20 October 2017 at 16:25, Gregory CLEMENT
>> >> <gregory.clement@free-electrons.com> wrote:
>> >> > Hi Ard,
>> >> >
>> >> > On jeu., oct. 12 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >> >
>> >> >> On 12 October 2017 at 10:45, Russell King - ARM Linux
>> >> >> <linux@armlinux.org.uk> wrote:
>> >> >>> On Thu, Oct 12, 2017 at 11:24:57AM +0200, Gregory CLEMENT wrote:
>> >> >>>> Hi Ard,
>> >> >>>>
>> >> >>>> Can we move forward to fix the booting problem ?
>> >> >>>>
>> >> >>>> What about amending your commit log with this new information and then
>> >> >>>> submit it to Russell patch system?
>> >> >>>
>> >> >>> Well, I think there's a choice that needs to be made between this
>> >> >>> approach and Arnd's approach.
>> >> >>>
>> >> >>> I'm not all that thrilled with the need to add explicit alignment to
>> >> >>> data that is inherently a byte stream, and that invariably results in
>> >> >>> unaligned data words even if you do align the start of it. That
>> >> >>> sounds to me very much like a hack rather than a proper solution.
>> >> >>> So, right now I'm leaning more towards Arnd's solution than Ard's
>> >> >>> from what's been said in this thread.
>> >> >>>
>> >> >>
>> >> >> I agree that the struct type unaligned accessors are the best choice
>> >> >> for ARM in any case, given that it will also prevent hitting the
>> >> >> alignment fixup handler in the kernel unnecessarily.
>> >> >>
>> >> >>> However, I don't recall Arnd's patch, it's probably buried deep in
>> >> >>> my mailbox.
>> >> >>>
>> >> >>
>> >> >> Well, unless you are considering changing the unaligned accessors from
>> >> >> access_ok.h to le_struct.h as a bugfix, I think we need both patches.
>> >> >
>> >> > We will soon reach v4.14-rc6 and the Armada XP and Armada 370 still not
>> >> > boot. I also didn't see your patch in rmk patch system.
>> >> >
>> >> > Waiting for you find a agreement an other option is to remove CONFIG_EFI
>> >> > from multi_v7_defconfig as I don't really see any armv7 base board using
>> >> > EFI.
>> >> >
>> >>
>> >> It is up to Russell to decide how he wants to proceed. Russell?
>> >
>> > Well, having failed to attract Arnd's attention, I've spent 20 minutes
>> > searching my mailbox to find it.
>> >
>> > It turns out that there was never a proper patch from Arnd - it was a
>> > patch in pastebin. It's not ARM specific either, it's an asm-generic
>> > change, for which Arnd is the maintainer for.
>> >
>> > I've just replied to that old thread and I've included Gregory and some
>> > PXA folk who are also having alignment problems in the decompressor. It
>> > could all be due to this same issue.
>> >
>> > There's also one additional factor that hasn't been considered, and I'm
>> > scared to point it out, but:
>> >
>> > 1. Does Arnd's patch fix the PXA problem as well?
>> >
>>
>> I don't think it will help configs that don't have
>> HAVE_EFFICIENT_UNALIGNED_ACCESS set, given that they don't use
>> access_ok.h in the first place.
>>
>> > 2. What is the performance impact of Arnd's fix?
>> >
>>
>> Well, given that we may be relying on the alignment fixup handler to
>> fix up kernel accesses, the performance impact could actually be
>> favorable in some cases. But I don't have any numbers, nor do I have
>> access to a representative sampling of ARM hardware so I can't be of
>> any help here, unfortunately.
>
> Well, everyone can tell whether alignment fixups get used on their
> platforms, by looking at /proc/cpu/alignment - this counts any
> alignment faults and their types. If it's all zeros, then the
> alignment fixup handler isn't being used.
>
> I just checked one of my imx6 platforms (3G/wifi gateway), all the
> stats are zero. Another imx6 platform (whose port 80 is open to the
> world) has one double-word fault in nsm_get_handle+0x200/0x484.
> My ARMv5 gateway here also has all zero stats.
>
> However, all these are built with GCC 4.7.4, and we already know
> that newer compilers behave significantly differently. So, I don't
> think what I see here is representative, so I can't decide based on
> what I see locally.
>
> So, it seems we're rather stuck.
>
> I suppose I could set up a dart board, and throw a dart blind folded
> and if it hits a red area, pick one patch, otherwise take the other.
> Or toss a coin. Or some other rediculous game.
>
Why would you have to choose between these patches? You can simply
apply mine as a bug fix, and postpone the le_struct.h discussion for
the next cycle.
I know that does not fix PXA, but Arnd's patch is unlikely to fix that anyway.
> It would be much better if there was some way to evaluate the impact,
> but I see that no one is interested in lifting a finger to help with
> that.
>
> Meanwhile, I'm quite sure there'll be an on-going cry for me to make
> a decision. A decision based on a whim. Yea, great basis for taking
> decisions.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 16:36 ` Ard Biesheuvel
@ 2017-10-20 16:54 ` Russell King - ARM Linux
2017-10-20 17:12 ` Ard Biesheuvel
0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-20 16:54 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 20, 2017 at 05:36:08PM +0100, Ard Biesheuvel wrote:
> Why would you have to choose between these patches? You can simply
> apply mine as a bug fix, and postpone the le_struct.h discussion for
> the next cycle.
It's not a choice between these two patches. I think we need Arnd's
patch, because we know that fixes Romain Izard's problem. If I apply
just your patch, then Gregory stops complaining, and there'll be
nothing to remind us of this issue - Romain isn't shouting about it.
That's a problem, because it's a real bug that then gets forgotten
about.
Or do we have evidence that your patch fixes Romain Izard's problem
as well? I don't see any such suggestion.
So, let me go back to the dart board... I just don't have the
information to be able to make any decision on this, so stop asking
me for a decision on something I know nothing about. Either provide
me with some information I can base a decision on, or stop asking
for a decision.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-20 16:54 ` Russell King - ARM Linux
@ 2017-10-20 17:12 ` Ard Biesheuvel
0 siblings, 0 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-20 17:12 UTC (permalink / raw)
To: linux-arm-kernel
On 20 October 2017 at 17:54, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Fri, Oct 20, 2017 at 05:36:08PM +0100, Ard Biesheuvel wrote:
>> Why would you have to choose between these patches? You can simply
>> apply mine as a bug fix, and postpone the le_struct.h discussion for
>> the next cycle.
>
> It's not a choice between these two patches. I think we need Arnd's
> patch, because we know that fixes Romain Izard's problem. If I apply
> just your patch, then Gregory stops complaining, and there'll be
> nothing to remind us of this issue - Romain isn't shouting about it.
> That's a problem, because it's a real bug that then gets forgotten
> about.
>
> Or do we have evidence that your patch fixes Romain Izard's problem
> as well? I don't see any such suggestion.
>
OK, so you are reluctant to apply patch A which fixes issue X, because
it will result in you forgetting about patch B that fixes issue Y. I
am sorry, but that does not make any sense to me.
> So, let me go back to the dart board... I just don't have the
> information to be able to make any decision on this, so stop asking
> me for a decision on something I know nothing about. Either provide
> me with some information I can base a decision on, or stop asking
> for a decision.
>
Nobody is asking you to choose. I am asking you to apply this patch:
piggydata used to be word aligned, and there is no harm in restoring
that behavior explicitly.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-09-08 15:31 [PATCH] ARM: compressed: discard ksym/kcrctab input section Ard Biesheuvel
2017-09-08 15:39 ` Gregory CLEMENT
2017-10-04 12:16 ` Gregory CLEMENT
@ 2017-10-21 7:56 ` Matthias Brugger
2017-10-21 8:14 ` Ard Biesheuvel
2 siblings, 1 reply; 30+ messages in thread
From: Matthias Brugger @ 2017-10-21 7:56 UTC (permalink / raw)
To: linux-arm-kernel
On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
> As it turns out, building the ARM kernel with EFI support pulls in
> a couple of sections that we don't really need in the decompressor.
> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
> memory map, which is an exported symbol pulled in from lib/sort.c.
>
> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
> separate PE/COFF sections"), this resulted in the following layout
> for the decompressor ELF binary.
>
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>
> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
> UEFI firmware to map the decompressor with strict R-X/RW- permissions
> before invoking it. Unfortunately, this turns out to break the boot on
> some systems, because the linker now also moves the ksymtab/kcrctab
> sections around, resulting in .piggydata to appear misaligned.
>
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>
> So let's align piggydata explicitly, and discard these sections from the
> binary.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm/boot/compressed/piggy.S | 1 +
> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
> 2 files changed, 2 insertions(+)
>
This fixes the boot regression on bananapi-r2.
Thanks! Feel free to add:
Tested-by: Matthias Brugger <mbrugger@suse.com>
> diff --git a/arch/arm/boot/compressed/piggy.S b/arch/arm/boot/compressed/piggy.S
> index f72088495f43..5d52c556dd32 100644
> --- a/arch/arm/boot/compressed/piggy.S
> +++ b/arch/arm/boot/compressed/piggy.S
> @@ -1,5 +1,6 @@
> .section .piggydata,#alloc
> .globl input_data
> + .align 2
> input_data:
> .incbin "arch/arm/boot/compressed/piggy_data"
> .globl input_data_end
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> index 7a4c59154361..5c5265be4605 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.S
> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> @@ -29,6 +29,7 @@ SECTIONS
> * of the text/got segments.
> */
> *(.data)
> + *(*ksymtab* *kcrctab*)
> }
>
> . = TEXT_START;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-21 7:56 ` Matthias Brugger
@ 2017-10-21 8:14 ` Ard Biesheuvel
2017-10-22 22:00 ` Matthias Brugger
2017-10-23 21:32 ` Matthias Brugger
0 siblings, 2 replies; 30+ messages in thread
From: Ard Biesheuvel @ 2017-10-21 8:14 UTC (permalink / raw)
To: linux-arm-kernel
On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
>> As it turns out, building the ARM kernel with EFI support pulls in
>> a couple of sections that we don't really need in the decompressor.
>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>
>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>> separate PE/COFF sections"), this resulted in the following layout
>> for the decompressor ELF binary.
>>
>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>
>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>> before invoking it. Unfortunately, this turns out to break the boot on
>> some systems, because the linker now also moves the ksymtab/kcrctab
>> sections around, resulting in .piggydata to appear misaligned.
>>
>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>
>> So let's align piggydata explicitly, and discard these sections from the
>> binary.
>>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm/boot/compressed/piggy.S | 1 +
>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>> 2 files changed, 2 insertions(+)
>>
>
> This fixes the boot regression on bananapi-r2.
> Thanks! Feel free to add:
>
> Tested-by: Matthias Brugger <mbrugger@suse.com>
>
Thanks for confirming Matthias. Could you please check whether this
patch from Arnd
https://marc.info/?l=linux-kernel&m=150852980119217&w=2
fixes the issue as well? (after reverting this one)
Thanks,
Ard.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-21 8:14 ` Ard Biesheuvel
@ 2017-10-22 22:00 ` Matthias Brugger
2017-10-23 9:29 ` Russell King - ARM Linux
2017-10-23 21:32 ` Matthias Brugger
1 sibling, 1 reply; 30+ messages in thread
From: Matthias Brugger @ 2017-10-22 22:00 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
> On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
>> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
>>> As it turns out, building the ARM kernel with EFI support pulls in
>>> a couple of sections that we don't really need in the decompressor.
>>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>>
>>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>>> separate PE/COFF sections"), this resulted in the following layout
>>> for the decompressor ELF binary.
>>>
>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>>
>>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>>> before invoking it. Unfortunately, this turns out to break the boot on
>>> some systems, because the linker now also moves the ksymtab/kcrctab
>>> sections around, resulting in .piggydata to appear misaligned.
>>>
>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>>
>>> So let's align piggydata explicitly, and discard these sections from the
>>> binary.
>>>
>>> Cc: Russell King <linux@armlinux.org.uk>
>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> arch/arm/boot/compressed/piggy.S | 1 +
>>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>>> 2 files changed, 2 insertions(+)
>>>
>>
>> This fixes the boot regression on bananapi-r2.
>> Thanks! Feel free to add:
>>
>> Tested-by: Matthias Brugger <mbrugger@suse.com>
>>
>
> Thanks for confirming Matthias. Could you please check whether this
> patch from Arnd
>
> https://marc.info/?l=linux-kernel&m=150852980119217&w=2
>
> fixes the issue as well? (after reverting this one)
>
After only applying this patch on top of v4.14-rc5 I was not able to boot.
Regards,
Matthias
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-22 22:00 ` Matthias Brugger
@ 2017-10-23 9:29 ` Russell King - ARM Linux
2017-10-23 11:48 ` Russell King - ARM Linux
2017-10-23 21:15 ` Matthias Brugger
0 siblings, 2 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-23 9:29 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 23, 2017 at 12:00:46AM +0200, Matthias Brugger wrote:
> Hi Ard,
>
> On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
> > On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
> >> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
> >>> As it turns out, building the ARM kernel with EFI support pulls in
> >>> a couple of sections that we don't really need in the decompressor.
> >>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
> >>> memory map, which is an exported symbol pulled in from lib/sort.c.
> >>>
> >>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
> >>> separate PE/COFF sections"), this resulted in the following layout
> >>> for the decompressor ELF binary.
> >>>
> >>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> >>> [ 0] NULL 00000000 000000 000000 00 0 0 0
> >>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
> >>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
> >>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
> >>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
> >>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
> >>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
> >>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
> >>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
> >>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
> >>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
> >>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
> >>>
> >>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
> >>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
> >>> before invoking it. Unfortunately, this turns out to break the boot on
> >>> some systems, because the linker now also moves the ksymtab/kcrctab
> >>> sections around, resulting in .piggydata to appear misaligned.
> >>>
> >>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> >>> [ 0] NULL 00000000 000000 000000 00 0 0 0
> >>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
> >>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
> >>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
> >>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
> >>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
> >>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
> >>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
> >>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
> >>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
> >>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
> >>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
> >>>
> >>> So let's align piggydata explicitly, and discard these sections from the
> >>> binary.
> >>>
> >>> Cc: Russell King <linux@armlinux.org.uk>
> >>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
> >>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>> ---
> >>> arch/arm/boot/compressed/piggy.S | 1 +
> >>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
> >>> 2 files changed, 2 insertions(+)
> >>>
> >>
> >> This fixes the boot regression on bananapi-r2.
> >> Thanks! Feel free to add:
> >>
> >> Tested-by: Matthias Brugger <mbrugger@suse.com>
> >>
> >
> > Thanks for confirming Matthias. Could you please check whether this
> > patch from Arnd
> >
> > https://marc.info/?l=linux-kernel&m=150852980119217&w=2
> >
> > fixes the issue as well? (after reverting this one)
> >
>
> After only applying this patch on top of v4.14-rc5 I was not able to boot.
Which decompression method are you using?
What does objdump -h arch/arm/boot/vmlinux say?
Thanks.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 9:29 ` Russell King - ARM Linux
@ 2017-10-23 11:48 ` Russell King - ARM Linux
2017-10-23 21:17 ` Matthias Brugger
2017-10-23 21:15 ` Matthias Brugger
1 sibling, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-23 11:48 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 23, 2017 at 10:29:56AM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 23, 2017 at 12:00:46AM +0200, Matthias Brugger wrote:
> > Hi Ard,
> >
> > On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
> > > On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
> > >> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
> > >>> As it turns out, building the ARM kernel with EFI support pulls in
> > >>> a couple of sections that we don't really need in the decompressor.
> > >>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
> > >>> memory map, which is an exported symbol pulled in from lib/sort.c.
> > >>>
> > >>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
> > >>> separate PE/COFF sections"), this resulted in the following layout
> > >>> for the decompressor ELF binary.
> > >>>
> > >>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> > >>> [ 0] NULL 00000000 000000 000000 00 0 0 0
> > >>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
> > >>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
> > >>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
> > >>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
> > >>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
> > >>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
> > >>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
> > >>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
> > >>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
> > >>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
> > >>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
> > >>>
> > >>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
> > >>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
> > >>> before invoking it. Unfortunately, this turns out to break the boot on
> > >>> some systems, because the linker now also moves the ksymtab/kcrctab
> > >>> sections around, resulting in .piggydata to appear misaligned.
> > >>>
> > >>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> > >>> [ 0] NULL 00000000 000000 000000 00 0 0 0
> > >>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
> > >>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
> > >>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
> > >>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
> > >>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
> > >>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
> > >>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
> > >>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
> > >>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
> > >>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
> > >>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
> > >>>
> > >>> So let's align piggydata explicitly, and discard these sections from the
> > >>> binary.
> > >>>
> > >>> Cc: Russell King <linux@armlinux.org.uk>
> > >>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
> > >>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >>> ---
> > >>> arch/arm/boot/compressed/piggy.S | 1 +
> > >>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
> > >>> 2 files changed, 2 insertions(+)
> > >>>
> > >>
> > >> This fixes the boot regression on bananapi-r2.
> > >> Thanks! Feel free to add:
> > >>
> > >> Tested-by: Matthias Brugger <mbrugger@suse.com>
> > >>
> > >
> > > Thanks for confirming Matthias. Could you please check whether this
> > > patch from Arnd
> > >
> > > https://marc.info/?l=linux-kernel&m=150852980119217&w=2
> > >
> > > fixes the issue as well? (after reverting this one)
> > >
> >
> > After only applying this patch on top of v4.14-rc5 I was not able to boot.
>
> Which decompression method are you using?
>
> What does objdump -h arch/arm/boot/vmlinux say?
Oh, and one more question: are you appending a dtb to your zImage (and
if so, why are you doing that, that's supposed to be for obsolete boot
loaders only.)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 9:29 ` Russell King - ARM Linux
2017-10-23 11:48 ` Russell King - ARM Linux
@ 2017-10-23 21:15 ` Matthias Brugger
1 sibling, 0 replies; 30+ messages in thread
From: Matthias Brugger @ 2017-10-23 21:15 UTC (permalink / raw)
To: linux-arm-kernel
On 10/23/2017 11:29 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 23, 2017 at 12:00:46AM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
>>> On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
>>>> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
>>>>> As it turns out, building the ARM kernel with EFI support pulls in
>>>>> a couple of sections that we don't really need in the decompressor.
>>>>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>>>>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>>>>
>>>>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>>>>> separate PE/COFF sections"), this resulted in the following layout
>>>>> for the decompressor ELF binary.
>>>>>
>>>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>>>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>>>>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>>>>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>>>>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>>>>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>>>>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>>>>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>>>>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>>>>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>>>>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>>>>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>>>>
>>>>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>>>>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>>>>> before invoking it. Unfortunately, this turns out to break the boot on
>>>>> some systems, because the linker now also moves the ksymtab/kcrctab
>>>>> sections around, resulting in .piggydata to appear misaligned.
>>>>>
>>>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>>>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>>>>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>>>>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>>>>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>>>>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>>>>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>>>>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>>>>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>>>>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>>>>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>>>>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>>>>
>>>>> So let's align piggydata explicitly, and discard these sections from the
>>>>> binary.
>>>>>
>>>>> Cc: Russell King <linux@armlinux.org.uk>
>>>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>>>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>> arch/arm/boot/compressed/piggy.S | 1 +
>>>>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>>>>> 2 files changed, 2 insertions(+)
>>>>>
>>>>
>>>> This fixes the boot regression on bananapi-r2.
>>>> Thanks! Feel free to add:
>>>>
>>>> Tested-by: Matthias Brugger <mbrugger@suse.com>
>>>>
>>>
>>> Thanks for confirming Matthias. Could you please check whether this
>>> patch from Arnd
>>>
>>> https://marc.info/?l=linux-kernel&m=150852980119217&w=2
>>>
>>> fixes the issue as well? (after reverting this one)
>>>
>>
>> After only applying this patch on top of v4.14-rc5 I was not able to boot.
>
> Which decompression method are you using?
uImage/zImage
>
> What does objdump -h arch/arm/boot/vmlinux say?
>
arch/arm/boot/compressed/vmlinux: file format elf32-littlearm
Sections:
Idx Name Size VMA LMA File off Algn
0 .text 0000b06c 00000000 00000000 00010000 2**12
CONTENTS, ALLOC, LOAD, READONLY, CODE
1 .rodata 000016cc 0000b06c 0000b06c 0001b06c 2**2
CONTENTS, ALLOC, LOAD, READONLY, DATA
2 __ksymtab_strings 00000005 0000c738 0000c738 0001c738 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
3 .piggydata 007ce9c5 0000c73d 0000c73d 0001c73d 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
4 .got.plt 0000000c 007db104 007db104 007eb104 2**2
CONTENTS, ALLOC, LOAD, DATA
5 .got 00000028 007db110 007db110 007eb110 2**2
CONTENTS, ALLOC, LOAD, DATA
6 .pad 00000008 007db138 007db138 007eb138 2**0
CONTENTS, ALLOC, LOAD, DATA
7 .data 00000200 007dc000 007dc000 007ec000 2**12
CONTENTS, ALLOC, LOAD, DATA
8 ___ksymtab+sort 00000008 007dc200 007dc200 007ec200 2**2
CONTENTS, ALLOC, LOAD, DATA
9 .bss 0000001c 007dc208 007dc208 007ec208 2**2
ALLOC
10 .stack 00001000 007dc228 007dc228 007ec208 2**0
ALLOC
11 .comment 0000002d 00000000 00000000 007ec208 2**0
CONTENTS, READONLY
12 .ARM.attributes 0000002d 00000000 00000000 007ec235 2**0
CONTENTS, READONLY
13 .debug_line 00007088 00000000 00000000 007ec262 2**0
CONTENTS, READONLY, DEBUGGING
14 .debug_info 00057677 00000000 00000000 007f32ea 2**0
CONTENTS, READONLY, DEBUGGING
15 .debug_abbrev 00003bc2 00000000 00000000 0084a961 2**0
CONTENTS, READONLY, DEBUGGING
16 .debug_aranges 00000308 00000000 00000000 0084e528 2**3
CONTENTS, READONLY, DEBUGGING
17 .debug_ranges 00000e78 00000000 00000000 0084e830 2**3
CONTENTS, READONLY, DEBUGGING
18 .debug_frame 00002228 00000000 00000000 0084f6a8 2**2
CONTENTS, READONLY, DEBUGGING
19 .debug_loc 00005bff 00000000 00000000 008518d0 2**0
CONTENTS, READONLY, DEBUGGING
20 .debug_str 00008c41 00000000 00000000 008574cf 2**0
CONTENTS, READONLY, DEBUGGING
This objdump is with Arnd Bergmans patch applied on top of v4.14-rc5.
Beware that Ard provided a patch against efi/libstub which independently fixes
the boot regression:
"efi/libstub: arm: omit sorting of the UEFI memory map"
Regards,
Matthias
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 11:48 ` Russell King - ARM Linux
@ 2017-10-23 21:17 ` Matthias Brugger
2017-10-23 22:19 ` Russell King - ARM Linux
0 siblings, 1 reply; 30+ messages in thread
From: Matthias Brugger @ 2017-10-23 21:17 UTC (permalink / raw)
To: linux-arm-kernel
On 10/23/2017 01:48 PM, Russell King - ARM Linux wrote:
> On Mon, Oct 23, 2017 at 10:29:56AM +0100, Russell King - ARM Linux wrote:
>> On Mon, Oct 23, 2017 at 12:00:46AM +0200, Matthias Brugger wrote:
>>> Hi Ard,
>>>
>>> On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
>>>> On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
>>>>> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
>>>>>> As it turns out, building the ARM kernel with EFI support pulls in
>>>>>> a couple of sections that we don't really need in the decompressor.
>>>>>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>>>>>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>>>>>
>>>>>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>>>>>> separate PE/COFF sections"), this resulted in the following layout
>>>>>> for the decompressor ELF binary.
>>>>>>
>>>>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>>>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>>>>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>>>>>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>>>>>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>>>>>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>>>>>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>>>>>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>>>>>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>>>>>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>>>>>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>>>>>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>>>>>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>>>>>
>>>>>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>>>>>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>>>>>> before invoking it. Unfortunately, this turns out to break the boot on
>>>>>> some systems, because the linker now also moves the ksymtab/kcrctab
>>>>>> sections around, resulting in .piggydata to appear misaligned.
>>>>>>
>>>>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>>>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>>>>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>>>>>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>>>>>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>>>>>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>>>>>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>>>>>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>>>>>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>>>>>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>>>>>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>>>>>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>>>>>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>>>>>
>>>>>> So let's align piggydata explicitly, and discard these sections from the
>>>>>> binary.
>>>>>>
>>>>>> Cc: Russell King <linux@armlinux.org.uk>
>>>>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>>>>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>> ---
>>>>>> arch/arm/boot/compressed/piggy.S | 1 +
>>>>>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>>>>>> 2 files changed, 2 insertions(+)
>>>>>>
>>>>>
>>>>> This fixes the boot regression on bananapi-r2.
>>>>> Thanks! Feel free to add:
>>>>>
>>>>> Tested-by: Matthias Brugger <mbrugger@suse.com>
>>>>>
>>>>
>>>> Thanks for confirming Matthias. Could you please check whether this
>>>> patch from Arnd
>>>>
>>>> https://marc.info/?l=linux-kernel&m=150852980119217&w=2
>>>>
>>>> fixes the issue as well? (after reverting this one)
>>>>
>>>
>>> After only applying this patch on top of v4.14-rc5 I was not able to boot.
>>
>> Which decompression method are you using?
>>
>> What does objdump -h arch/arm/boot/vmlinux say?
>
> Oh, and one more question: are you appending a dtb to your zImage (and
> if so, why are you doing that, that's supposed to be for obsolete boot
> loaders only.)
>
Yes, I apply a dtb to my zImage. I do that because the board has no support on
mainline u-boot right now and I do kernel enablement on the board. Support in
u-boot is no priority ATM.
Regards,
Matthias
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-21 8:14 ` Ard Biesheuvel
2017-10-22 22:00 ` Matthias Brugger
@ 2017-10-23 21:32 ` Matthias Brugger
2017-10-23 22:29 ` Russell King - ARM Linux
2017-10-24 8:36 ` Andrea Adami
1 sibling, 2 replies; 30+ messages in thread
From: Matthias Brugger @ 2017-10-23 21:32 UTC (permalink / raw)
To: linux-arm-kernel
On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
> On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
>> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
>>> As it turns out, building the ARM kernel with EFI support pulls in
>>> a couple of sections that we don't really need in the decompressor.
>>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>>
>>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>>> separate PE/COFF sections"), this resulted in the following layout
>>> for the decompressor ELF binary.
>>>
>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>>
>>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>>> before invoking it. Unfortunately, this turns out to break the boot on
>>> some systems, because the linker now also moves the ksymtab/kcrctab
>>> sections around, resulting in .piggydata to appear misaligned.
>>>
>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>>
>>> So let's align piggydata explicitly, and discard these sections from the
>>> binary.
>>>
>>> Cc: Russell King <linux@armlinux.org.uk>
>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> arch/arm/boot/compressed/piggy.S | 1 +
>>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>>> 2 files changed, 2 insertions(+)
>>>
>>
>> This fixes the boot regression on bananapi-r2.
>> Thanks! Feel free to add:
>>
>> Tested-by: Matthias Brugger <mbrugger@suse.com>
>>
>
> Thanks for confirming Matthias. Could you please check whether this
> patch from Arnd
>
> https://marc.info/?l=linux-kernel&m=150852980119217&w=2
>
> fixes the issue as well? (after reverting this one)
>
For the record, gcc7 has a alignment problem which was fixed for PR82445 [1].
I tried to boot with the aforementioned patch from Arnd and the fixed gcc, but
that didn't help.
Regards,
Matthias
[1]
https://github.com/gcc-mirror/gcc/commit/f59996b56aaa1c1d62a16cbb4010775b624cbde0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 21:17 ` Matthias Brugger
@ 2017-10-23 22:19 ` Russell King - ARM Linux
2017-10-24 6:51 ` Matthias Brugger
0 siblings, 1 reply; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-23 22:19 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 23, 2017 at 11:17:35PM +0200, Matthias Brugger wrote:
> Yes, I apply a dtb to my zImage. I do that because the board has no support on
> mainline u-boot right now and I do kernel enablement on the board. Support in
> u-boot is no priority ATM.
Right, so the reason your system isn't booting is because of the
additional section in the zImage, which causes additional bytes
after _edata.
That causes the appending of the dtb to be in the wrong place,
so it doesn't get found.
With Arnd's patch applied, if you do:
dd if=arch/arm/boot/zImage bs=1 count=$(( $(stat -c %s arch/arm/boot/zImage) - 8)) of=zImg
cat zImg $dtb > zImg.boot
and then try booting zImg.boot instead, that should work, and
would prove my point.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 21:32 ` Matthias Brugger
@ 2017-10-23 22:29 ` Russell King - ARM Linux
2017-10-24 8:36 ` Andrea Adami
1 sibling, 0 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2017-10-23 22:29 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 23, 2017 at 11:32:11PM +0200, Matthias Brugger wrote:
> For the record, gcc7 has a alignment problem which was fixed for PR82445 [1].
>
> I tried to boot with the aforementioned patch from Arnd and the fixed
> gcc, but that didn't help.
That's because we have two entirely separate problems here, both leading
to the same "it doesn't boot" outcome.
1. We have additional bytes in the zImage file after _edata, which
means that appending a dtb using the usual "cat" method doesn't
work. We look for the appended dtb at _edata.
2. Misaligned loads in the decompressor causing faults.
While removing "sort" and the ksymtab sections appears to fix the
problem, it does so by removing the troublesome sections. However,
what it's actually highlighting is that we have more fundamental
issues here.
Additional sections that are not mentioned in the linker script
will be passed through by the linker to the output file, and can
result in exactly the same issue. So, a patch that discards the
current sections doesn't really fix the issue, it papers it over.
I also feel that removing the "sort" code from the EFI stub also
papers over the problem. Both of those remove the /current/ cause
of a more fundamental problem without addressing that fundamental
problem.
I'm not saying we shouldn't discard the sections, I'm saying we
need to do more to detect the fundamental problem, rather than
hiding it.
That fundamental problem is that we allow the build to succeed
when the results of the build are obviously incorrect. I've
proposed a patch that causes the copy of vmlinux to zImage to
fail if the zImage size does not match the expected size of the
binary image.
We should also eliminate the reason for (2), which is what Arnds
patch addresses.
So, in total, I think we need three patches:
1. Arnds patch to fix the alignment issues.
2. My patch to detect wrong zImage size.
3. A patch to discard troublesome sections.
Optionally, removal of the sort code from the EFI stub is an
orthogonal issue - the sort code is merely the vehicle by which
the real problems have been found.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 22:19 ` Russell King - ARM Linux
@ 2017-10-24 6:51 ` Matthias Brugger
0 siblings, 0 replies; 30+ messages in thread
From: Matthias Brugger @ 2017-10-24 6:51 UTC (permalink / raw)
To: linux-arm-kernel
On 10/24/2017 12:19 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 23, 2017 at 11:17:35PM +0200, Matthias Brugger wrote:
>> Yes, I apply a dtb to my zImage. I do that because the board has no support on
>> mainline u-boot right now and I do kernel enablement on the board. Support in
>> u-boot is no priority ATM.
>
> Right, so the reason your system isn't booting is because of the
> additional section in the zImage, which causes additional bytes
> after _edata.
>
> That causes the appending of the dtb to be in the wrong place,
> so it doesn't get found.
>
> With Arnd's patch applied, if you do:
>
> dd if=arch/arm/boot/zImage bs=1 count=$(( $(stat -c %s arch/arm/boot/zImage) - 8)) of=zImg
> cat zImg $dtb > zImg.boot
>
> and then try booting zImg.boot instead, that should work, and
> would prove my point.
>
I can confirm that this fixes the issue.
Regards,
Matthias
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] ARM: compressed: discard ksym/kcrctab input section
2017-10-23 21:32 ` Matthias Brugger
2017-10-23 22:29 ` Russell King - ARM Linux
@ 2017-10-24 8:36 ` Andrea Adami
1 sibling, 0 replies; 30+ messages in thread
From: Andrea Adami @ 2017-10-24 8:36 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 23, 2017 at 11:32 PM, Matthias Brugger
<matthias.bgg@gmail.com> wrote:
>
>
> On 10/21/2017 10:14 AM, Ard Biesheuvel wrote:
>> On 21 October 2017 at 08:56, Matthias Brugger <mbrugger@suse.com> wrote:
>>> On 09/08/2017 05:31 PM, Ard Biesheuvel wrote:
>>>> As it turns out, building the ARM kernel with EFI support pulls in
>>>> a couple of sections that we don't really need in the decompressor.
>>>> This is due to the fact the the UEFI stub uses sort() to sort the UEFI
>>>> memory map, which is an exported symbol pulled in from lib/sort.c.
>>>>
>>>> Before commit e4bae4d0b5f3 ("arm/efi: Split zImage code and data into
>>>> separate PE/COFF sections"), this resulted in the following layout
>>>> for the decompressor ELF binary.
>>>>
>>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>>> [ 1] .text PROGBITS 00000000 010000 009b3c 00 AX 0 0 512
>>>> [ 2] .rodata PROGBITS 00009b3c 019b3c 001684 00 A 0 0 4
>>>> [ 3] __ksymtab_strings PROGBITS 0000b1c0 01b1c0 000005 00 A 0 0 1
>>>> [ 4] .data PROGBITS 0000b1c8 01b1c8 000020 00 WA 0 0 8
>>>> [ 5] ___ksymtab+sort PROGBITS 0000b1e8 01b1e8 000008 00 WA 0 0 4
>>>> [ 6] .piggydata PROGBITS 0000b1f0 01b1f0 77ac38 00 A 0 0 1
>>>> [ 7] .got.plt PROGBITS 00785e28 795e28 00000c 04 WA 0 0 4
>>>> [ 8] .got PROGBITS 00785e34 795e34 000028 00 WA 0 0 4
>>>> [ 9] .pad PROGBITS 00785e5c 795e5c 000004 00 WA 0 0 1
>>>> [10] .bss NOBITS 00785e60 795e60 00001c 00 WA 0 0 4
>>>> [11] .stack NOBITS 00785e80 795e60 001000 00 WA 0 0 1
>>>>
>>>> Commit e4bae4d0b5f3 made some changes to the linker script to allow the
>>>> UEFI firmware to map the decompressor with strict R-X/RW- permissions
>>>> before invoking it. Unfortunately, this turns out to break the boot on
>>>> some systems, because the linker now also moves the ksymtab/kcrctab
>>>> sections around, resulting in .piggydata to appear misaligned.
>>>>
>>>> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
>>>> [ 0] NULL 00000000 000000 000000 00 0 0 0
>>>> [ 1] .text PROGBITS 00000000 010000 00a93c 00 AX 0 0 4096
>>>> [ 2] .rodata PROGBITS 0000a93c 01a93c 001684 00 A 0 0 4
>>>> [ 3] __ksymtab_strings PROGBITS 0000bfc0 01bfc0 000005 00 A 0 0 1
>>>> [ 4] .piggydata PROGBITS 0000bfc5 01bfc5 77ac47 00 A 0 0 1
>>>> [ 5] .got.plt PROGBITS 00786c0c 796c0c 00000c 04 WA 0 0 4
>>>> [ 6] .got PROGBITS 00786c18 796c18 000028 00 WA 0 0 4
>>>> [ 7] .pad PROGBITS 00786c40 796c40 000008 00 WA 0 0 1
>>>> [ 8] .data PROGBITS 00787000 797000 000200 00 WA 0 0 4096
>>>> [ 9] ___ksymtab+sort PROGBITS 00787200 797200 000008 00 WA 0 0 4
>>>> [10] .bss NOBITS 00787208 797208 00001c 00 WA 0 0 4
>>>> [11] .stack NOBITS 00787228 797208 001000 00 WA 0 0 1
>>>>
>>>> So let's align piggydata explicitly, and discard these sections from the
>>>> binary.
>>>>
>>>> Cc: Russell King <linux@armlinux.org.uk>
>>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate ...")
>>>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>> arch/arm/boot/compressed/piggy.S | 1 +
>>>> arch/arm/boot/compressed/vmlinux.lds.S | 1 +
>>>> 2 files changed, 2 insertions(+)
>>>>
>>>
>>> This fixes the boot regression on bananapi-r2.
>>> Thanks! Feel free to add:
>>>
>>> Tested-by: Matthias Brugger <mbrugger@suse.com>
>>>
>>
>> Thanks for confirming Matthias. Could you please check whether this
>> patch from Arnd
>>
>> https://marc.info/?l=linux-kernel&m=150852980119217&w=2
>>
>> fixes the issue as well? (after reverting this one)
>>
>
> For the record, gcc7 has a alignment problem which was fixed for PR82445 [1].
>
This fixes the decompressor error on pxa [1].
Thanks for the pointer!
Cheers
Andrea
[1] https://www.spinics.net/lists/arm-kernel/msg594654.html
> I tried to boot with the aforementioned patch from Arnd and the fixed gcc, but
> that didn't help.
>
> Regards,
> Matthias
>
> [1]
> https://github.com/gcc-mirror/gcc/commit/f59996b56aaa1c1d62a16cbb4010775b624cbde0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2017-10-24 8:36 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-08 15:31 [PATCH] ARM: compressed: discard ksym/kcrctab input section Ard Biesheuvel
2017-09-08 15:39 ` Gregory CLEMENT
2017-09-08 15:41 ` Ard Biesheuvel
2017-10-04 12:16 ` Gregory CLEMENT
2017-10-04 12:20 ` Ard Biesheuvel
2017-10-04 12:43 ` Russell King - ARM Linux
2017-10-09 12:39 ` Ard Biesheuvel
2017-10-12 9:24 ` Gregory CLEMENT
2017-10-12 9:45 ` Russell King - ARM Linux
2017-10-12 19:03 ` Ard Biesheuvel
2017-10-20 15:25 ` Gregory CLEMENT
2017-10-20 15:28 ` Ard Biesheuvel
2017-10-20 16:11 ` Russell King - ARM Linux
2017-10-20 16:20 ` Ard Biesheuvel
2017-10-20 16:32 ` Russell King - ARM Linux
2017-10-20 16:36 ` Ard Biesheuvel
2017-10-20 16:54 ` Russell King - ARM Linux
2017-10-20 17:12 ` Ard Biesheuvel
2017-10-21 7:56 ` Matthias Brugger
2017-10-21 8:14 ` Ard Biesheuvel
2017-10-22 22:00 ` Matthias Brugger
2017-10-23 9:29 ` Russell King - ARM Linux
2017-10-23 11:48 ` Russell King - ARM Linux
2017-10-23 21:17 ` Matthias Brugger
2017-10-23 22:19 ` Russell King - ARM Linux
2017-10-24 6:51 ` Matthias Brugger
2017-10-23 21:15 ` Matthias Brugger
2017-10-23 21:32 ` Matthias Brugger
2017-10-23 22:29 ` Russell King - ARM Linux
2017-10-24 8:36 ` Andrea Adami
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.