* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-22 14:14 ` Ard Biesheuvel
@ 2017-10-23 3:28 ` jeffy
-1 siblings, 0 replies; 22+ messages in thread
From: jeffy @ 2017-10-23 3:28 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-I+IVW8TIWO2tmTQ+vhA3Yw
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
mbrugger-IBi9RG/b67k, matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
Hi Ard,
this patch fixes the zImage file size not aligned issue i meet, thanks:)
Tested-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 10/22/2017 10:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-23 3:28 ` jeffy
0 siblings, 0 replies; 22+ messages in thread
From: jeffy @ 2017-10-23 3:28 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
this patch fixes the zImage file size not aligned issue i meet, thanks:)
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
On 10/22/2017 10:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-22 14:14 ` Ard Biesheuvel
@ 2017-10-23 21:08 ` Matthias Brugger
-1 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-23 21:08 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-I+IVW8TIWO2tmTQ+vhA3Yw
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io, Arnd Bergmann
Hi Ard,
On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
Tested-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This fixes my boot regression.
Thanks a lot!
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-23 21:08 ` Matthias Brugger
0 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-23 21:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
Tested-by: Matthias Brugger <matthias.bgg@gmail.com>
This fixes my boot regression.
Thanks a lot!
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-22 14:14 ` Ard Biesheuvel
@ 2017-10-24 11:05 ` Russell King - ARM Linux
-1 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2017-10-24 11:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
mbrugger-IBi9RG/b67k, matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
On Sun, Oct 22, 2017 at 03:14:57PM +0100, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
I guess as this is shared between ARM and ARM64, ARM64 folk should ack
this before I merge it - can I have an ack from that side please?
Thanks.
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
> --
> 2.11.0
>
--
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] 22+ messages in thread* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-24 11:05 ` Russell King - ARM Linux
0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2017-10-24 11:05 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Oct 22, 2017 at 03:14:57PM +0100, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
I guess as this is shared between ARM and ARM64, ARM64 folk should ack
this before I merge it - can I have an ack from that side please?
Thanks.
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
> --
> 2.11.0
>
--
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] 22+ messages in thread[parent not found: <20171024110519.GE20805-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>]
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-24 11:05 ` Russell King - ARM Linux
@ 2017-10-24 11:09 ` Ard Biesheuvel
-1 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2017-10-24 11:09 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chris Zhong,
Jeffy Chen, Gregory CLEMENT, Matthias Brugger, Matt Fleming
On 24 October 2017 at 12:05, Russell King - ARM Linux
<linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> On Sun, Oct 22, 2017 at 03:14:57PM +0100, Ard Biesheuvel wrote:
>> ARM shares its EFI stub implementation with arm64, which has some
>> special handling in the virtual remapping code to
>> a) make sure that we can map everything even if the OS executes
>> with 64k page size, and
>> b) make sure that adjacent regions with the same attributes are not
>> reordered or moved apart in memory.
>>
>> The latter is a workaround for a 'feature' that was shortly recommended
>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>> it broke many OS installers, including non-Linux ones, and it was never
>> widely implemented for ARM systems. Before implementing b), the arm64
>> code simply rounded up all regions to 64 KB granularity, but given that
>> that results in moving adjacent regions apart, it had to be refined when
>> b) was implemented.
>>
>> The adjacency check requires a sort() pass, due to the fact that the
>> UEFI spec does not mandate any ordering, and the inclusion of the
>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>> triggers the creation of ksymtab/kcrctab sections.
>>
>> So let's simply do away with the adjacency check for ARM, and simply put
>> all UEFI runtime regions together if they have the same memory attributes.
>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>> and allows us to remove the sort() call entirely.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> I guess as this is shared between ARM and ARM64, ARM64 folk should ack
> this before I merge it - can I have an ack from that side please?
>
Note that the patch does not touch arch/arm64, nor does it affect
arm64, given that the change is a no-op if CONFIG_ARM64=y. That said,
I welcome any acks from the arm64 maintainers, but I don't think they
are actually required here.
>> ---
>> drivers/firmware/efi/libstub/Makefile | 6 +++---
>> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
>> index dedf9bde44db..f3e8431565ea 100644
>> --- a/drivers/firmware/efi/libstub/Makefile
>> +++ b/drivers/firmware/efi/libstub/Makefile
>> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
>> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>>
>> # include the stub's generic dependencies from lib/ when building for ARM/arm64
>> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
>> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
>> +arm-deps-$(CONFIG_ARM64) += sort.c
>>
>> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>> $(call if_changed_rule,cc_o_c)
>>
>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
>> - $(patsubst %.c,lib-%.o,$(arm-deps))
>> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>>
>> lib-$(CONFIG_ARM) += arm32-stub.o
>> lib-$(CONFIG_ARM64) += arm64-stub.o
>> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
>> # explicitly by the decompressor linker script.
>> #
>> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
>> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
>> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
>> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
>> index 1cb2d1c070c3..3061e4057483 100644
>> --- a/drivers/firmware/efi/libstub/arm-stub.c
>> +++ b/drivers/firmware/efi/libstub/arm-stub.c
>> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
>> * The easiest way to find adjacent regions is to sort the memory map
>> * before traversing it.
>> */
>> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
>> + if (IS_ENABLED(CONFIG_ARM64))
>> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
>> + NULL);
>>
>> for (l = 0; l < map_size; l += desc_size, prev = in) {
>> u64 paddr, size;
>> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
>> * a 4k page size kernel to kexec a 64k page size kernel and
>> * vice versa.
>> */
>> - if (!regions_are_adjacent(prev, in) ||
>> + if ((IS_ENABLED(CONFIG_ARM64) &&
>> + !regions_are_adjacent(prev, in)) ||
>> !regions_have_compatible_memory_type_attrs(prev, in)) {
>>
>> paddr = round_down(in->phys_addr, SZ_64K);
>> --
>> 2.11.0
>>
>
> --
> 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] 22+ messages in thread* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-24 11:09 ` Ard Biesheuvel
0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2017-10-24 11:09 UTC (permalink / raw)
To: linux-arm-kernel
On 24 October 2017 at 12:05, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Sun, Oct 22, 2017 at 03:14:57PM +0100, Ard Biesheuvel wrote:
>> ARM shares its EFI stub implementation with arm64, which has some
>> special handling in the virtual remapping code to
>> a) make sure that we can map everything even if the OS executes
>> with 64k page size, and
>> b) make sure that adjacent regions with the same attributes are not
>> reordered or moved apart in memory.
>>
>> The latter is a workaround for a 'feature' that was shortly recommended
>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>> it broke many OS installers, including non-Linux ones, and it was never
>> widely implemented for ARM systems. Before implementing b), the arm64
>> code simply rounded up all regions to 64 KB granularity, but given that
>> that results in moving adjacent regions apart, it had to be refined when
>> b) was implemented.
>>
>> The adjacency check requires a sort() pass, due to the fact that the
>> UEFI spec does not mandate any ordering, and the inclusion of the
>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>> triggers the creation of ksymtab/kcrctab sections.
>>
>> So let's simply do away with the adjacency check for ARM, and simply put
>> all UEFI runtime regions together if they have the same memory attributes.
>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>> and allows us to remove the sort() call entirely.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> I guess as this is shared between ARM and ARM64, ARM64 folk should ack
> this before I merge it - can I have an ack from that side please?
>
Note that the patch does not touch arch/arm64, nor does it affect
arm64, given that the change is a no-op if CONFIG_ARM64=y. That said,
I welcome any acks from the arm64 maintainers, but I don't think they
are actually required here.
>> ---
>> drivers/firmware/efi/libstub/Makefile | 6 +++---
>> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
>> index dedf9bde44db..f3e8431565ea 100644
>> --- a/drivers/firmware/efi/libstub/Makefile
>> +++ b/drivers/firmware/efi/libstub/Makefile
>> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
>> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>>
>> # include the stub's generic dependencies from lib/ when building for ARM/arm64
>> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
>> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
>> +arm-deps-$(CONFIG_ARM64) += sort.c
>>
>> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>> $(call if_changed_rule,cc_o_c)
>>
>> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
>> - $(patsubst %.c,lib-%.o,$(arm-deps))
>> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>>
>> lib-$(CONFIG_ARM) += arm32-stub.o
>> lib-$(CONFIG_ARM64) += arm64-stub.o
>> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
>> # explicitly by the decompressor linker script.
>> #
>> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
>> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
>> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
>> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
>> index 1cb2d1c070c3..3061e4057483 100644
>> --- a/drivers/firmware/efi/libstub/arm-stub.c
>> +++ b/drivers/firmware/efi/libstub/arm-stub.c
>> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
>> * The easiest way to find adjacent regions is to sort the memory map
>> * before traversing it.
>> */
>> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
>> + if (IS_ENABLED(CONFIG_ARM64))
>> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
>> + NULL);
>>
>> for (l = 0; l < map_size; l += desc_size, prev = in) {
>> u64 paddr, size;
>> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
>> * a 4k page size kernel to kexec a 64k page size kernel and
>> * vice versa.
>> */
>> - if (!regions_are_adjacent(prev, in) ||
>> + if ((IS_ENABLED(CONFIG_ARM64) &&
>> + !regions_are_adjacent(prev, in)) ||
>> !regions_have_compatible_memory_type_attrs(prev, in)) {
>>
>> paddr = round_down(in->phys_addr, SZ_64K);
>> --
>> 2.11.0
>>
>
> --
> 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] 22+ messages in thread[parent not found: <CAKv+Gu8gPHj1jV1ZyW8u1qSb5F=L6HDrmCcVueo0szy+2USsXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-24 11:09 ` Ard Biesheuvel
@ 2017-10-24 11:21 ` Will Deacon
-1 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2017-10-24 11:21 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Russell King - ARM Linux, linux-efi-u79uwXL29TY76Z2rM5mHXA,
Matt Fleming, Jeffy Chen, Gregory CLEMENT, Matthias Brugger,
Chris Zhong, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Tue, Oct 24, 2017 at 12:09:59PM +0100, Ard Biesheuvel wrote:
> On 24 October 2017 at 12:05, Russell King - ARM Linux
> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> > On Sun, Oct 22, 2017 at 03:14:57PM +0100, Ard Biesheuvel wrote:
> >> ARM shares its EFI stub implementation with arm64, which has some
> >> special handling in the virtual remapping code to
> >> a) make sure that we can map everything even if the OS executes
> >> with 64k page size, and
> >> b) make sure that adjacent regions with the same attributes are not
> >> reordered or moved apart in memory.
> >>
> >> The latter is a workaround for a 'feature' that was shortly recommended
> >> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> >> it broke many OS installers, including non-Linux ones, and it was never
> >> widely implemented for ARM systems. Before implementing b), the arm64
> >> code simply rounded up all regions to 64 KB granularity, but given that
> >> that results in moving adjacent regions apart, it had to be refined when
> >> b) was implemented.
> >>
> >> The adjacency check requires a sort() pass, due to the fact that the
> >> UEFI spec does not mandate any ordering, and the inclusion of the
> >> lib/sort.c code into the ARM EFI stub is causing some trouble with
> >> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> >> triggers the creation of ksymtab/kcrctab sections.
> >>
> >> So let's simply do away with the adjacency check for ARM, and simply put
> >> all UEFI runtime regions together if they have the same memory attributes.
> >> This is guaranteed to work, given that ARM only supports 4 KB pages,
> >> and allows us to remove the sort() call entirely.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >
> > I guess as this is shared between ARM and ARM64, ARM64 folk should ack
> > this before I merge it - can I have an ack from that side please?
> >
>
> Note that the patch does not touch arch/arm64, nor does it affect
> arm64, given that the change is a no-op if CONFIG_ARM64=y. That said,
> I welcome any acks from the arm64 maintainers, but I don't think they
> are actually required here.
Looks fine to me:
Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Thanks,
Will
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-24 11:21 ` Will Deacon
0 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2017-10-24 11:21 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 24, 2017 at 12:09:59PM +0100, Ard Biesheuvel wrote:
> On 24 October 2017 at 12:05, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Sun, Oct 22, 2017 at 03:14:57PM +0100, Ard Biesheuvel wrote:
> >> ARM shares its EFI stub implementation with arm64, which has some
> >> special handling in the virtual remapping code to
> >> a) make sure that we can map everything even if the OS executes
> >> with 64k page size, and
> >> b) make sure that adjacent regions with the same attributes are not
> >> reordered or moved apart in memory.
> >>
> >> The latter is a workaround for a 'feature' that was shortly recommended
> >> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> >> it broke many OS installers, including non-Linux ones, and it was never
> >> widely implemented for ARM systems. Before implementing b), the arm64
> >> code simply rounded up all regions to 64 KB granularity, but given that
> >> that results in moving adjacent regions apart, it had to be refined when
> >> b) was implemented.
> >>
> >> The adjacency check requires a sort() pass, due to the fact that the
> >> UEFI spec does not mandate any ordering, and the inclusion of the
> >> lib/sort.c code into the ARM EFI stub is causing some trouble with
> >> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> >> triggers the creation of ksymtab/kcrctab sections.
> >>
> >> So let's simply do away with the adjacency check for ARM, and simply put
> >> all UEFI runtime regions together if they have the same memory attributes.
> >> This is guaranteed to work, given that ARM only supports 4 KB pages,
> >> and allows us to remove the sort() call entirely.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > I guess as this is shared between ARM and ARM64, ARM64 folk should ack
> > this before I merge it - can I have an ack from that side please?
> >
>
> Note that the patch does not touch arch/arm64, nor does it affect
> arm64, given that the change is a no-op if CONFIG_ARM64=y. That said,
> I welcome any acks from the arm64 maintainers, but I don't think they
> are actually required here.
Looks fine to me:
Acked-by: Will Deacon <will.deacon@arm.com>
Thanks,
Will
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-22 14:14 ` Ard Biesheuvel
@ 2017-10-27 14:41 ` Gregory CLEMENT
-1 siblings, 0 replies; 22+ messages in thread
From: Gregory CLEMENT @ 2017-10-27 14:41 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
mbrugger-IBi9RG/b67k, matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
Hi Ard,
On dim., oct. 22 2017, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
I finally managed to test this patch and it fixed my booting issue. It
was just this single patch onto v4.14-rc6.
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Thanks,
Gregory
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
> --
> 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] 22+ messages in thread* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-27 14:41 ` Gregory CLEMENT
0 siblings, 0 replies; 22+ messages in thread
From: Gregory CLEMENT @ 2017-10-27 14:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On dim., oct. 22 2017, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
I finally managed to test this patch and it fixed my booting issue. It
was just this single patch onto v4.14-rc6.
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Thanks,
Gregory
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
> --
> 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] 22+ messages in thread
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-22 14:14 ` Ard Biesheuvel
@ 2017-10-27 15:12 ` Matthias Brugger
-1 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-27 15:12 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-I+IVW8TIWO2tmTQ+vhA3Yw
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
Hi Ard,
On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
I thought I already provided my tag, but just in case:
This fixes the boot problems I had on the bananapi-r2.
Tested-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-27 15:12 ` Matthias Brugger
0 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-27 15:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ard,
On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> ARM shares its EFI stub implementation with arm64, which has some
> special handling in the virtual remapping code to
> a) make sure that we can map everything even if the OS executes
> with 64k page size, and
> b) make sure that adjacent regions with the same attributes are not
> reordered or moved apart in memory.
>
> The latter is a workaround for a 'feature' that was shortly recommended
> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> it broke many OS installers, including non-Linux ones, and it was never
> widely implemented for ARM systems. Before implementing b), the arm64
> code simply rounded up all regions to 64 KB granularity, but given that
> that results in moving adjacent regions apart, it had to be refined when
> b) was implemented.
>
> The adjacency check requires a sort() pass, due to the fact that the
> UEFI spec does not mandate any ordering, and the inclusion of the
> lib/sort.c code into the ARM EFI stub is causing some trouble with
> the decompressor build due to the fact that its EXPORT_SYMBOL() call
> triggers the creation of ksymtab/kcrctab sections.
>
> So let's simply do away with the adjacency check for ARM, and simply put
> all UEFI runtime regions together if they have the same memory attributes.
> This is guaranteed to work, given that ARM only supports 4 KB pages,
> and allows us to remove the sort() call entirely.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/libstub/Makefile | 6 +++---
> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
I thought I already provided my tag, but just in case:
This fixes the boot problems I had on the bananapi-r2.
Tested-by: Matthias Brugger <matthias.bgg@gmail.com>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index dedf9bde44db..f3e8431565ea 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -33,13 +33,14 @@ lib-y := efi-stub-helper.o gop.o secureboot.o
> lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
> +arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +arm-deps-$(CONFIG_ARM64) += sort.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps))
> + $(patsubst %.c,lib-%.o,$(arm-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -90,5 +91,4 @@ quiet_cmd_stubcopy = STUBCPY $@
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> -STUBCOPY_RM-$(CONFIG_ARM) += -R ___ksymtab+sort -R ___kcrctab+sort
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1cb2d1c070c3..3061e4057483 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -349,7 +349,9 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * The easiest way to find adjacent regions is to sort the memory map
> * before traversing it.
> */
> - sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc, NULL);
> + if (IS_ENABLED(CONFIG_ARM64))
> + sort(memory_map, map_size / desc_size, desc_size, cmp_mem_desc,
> + NULL);
>
> for (l = 0; l < map_size; l += desc_size, prev = in) {
> u64 paddr, size;
> @@ -366,7 +368,8 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
> * a 4k page size kernel to kexec a 64k page size kernel and
> * vice versa.
> */
> - if (!regions_are_adjacent(prev, in) ||
> + if ((IS_ENABLED(CONFIG_ARM64) &&
> + !regions_are_adjacent(prev, in)) ||
> !regions_have_compatible_memory_type_attrs(prev, in)) {
>
> paddr = round_down(in->phys_addr, SZ_64K);
>
^ permalink raw reply [flat|nested] 22+ messages in thread[parent not found: <fe35c87c-8fd0-b33f-3a6f-2bd759726667-IBi9RG/b67k@public.gmane.org>]
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-27 15:12 ` Matthias Brugger
@ 2017-10-27 15:28 ` Russell King - ARM Linux
-1 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2017-10-27 15:28 UTC (permalink / raw)
To: Matthias Brugger
Cc: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
> Hi Ard,
>
> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> > ARM shares its EFI stub implementation with arm64, which has some
> > special handling in the virtual remapping code to
> > a) make sure that we can map everything even if the OS executes
> > with 64k page size, and
> > b) make sure that adjacent regions with the same attributes are not
> > reordered or moved apart in memory.
> >
> > The latter is a workaround for a 'feature' that was shortly recommended
> > by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> > it broke many OS installers, including non-Linux ones, and it was never
> > widely implemented for ARM systems. Before implementing b), the arm64
> > code simply rounded up all regions to 64 KB granularity, but given that
> > that results in moving adjacent regions apart, it had to be refined when
> > b) was implemented.
> >
> > The adjacency check requires a sort() pass, due to the fact that the
> > UEFI spec does not mandate any ordering, and the inclusion of the
> > lib/sort.c code into the ARM EFI stub is causing some trouble with
> > the decompressor build due to the fact that its EXPORT_SYMBOL() call
> > triggers the creation of ksymtab/kcrctab sections.
> >
> > So let's simply do away with the adjacency check for ARM, and simply put
> > all UEFI runtime regions together if they have the same memory attributes.
> > This is guaranteed to work, given that ARM only supports 4 KB pages,
> > and allows us to remove the sort() call entirely.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > drivers/firmware/efi/libstub/Makefile | 6 +++---
> > drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> > 2 files changed, 8 insertions(+), 5 deletions(-)
> >
>
> I thought I already provided my tag, but just in case:
> This fixes the boot problems I had on the bananapi-r2.
>
> Tested-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Please test my "fixes" branch - that has all three patches merged.
We don't have linux-next running at the moment, so folk need to be
manually fetching the appropriate git trees for testing.
--
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] 22+ messages in thread
* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-27 15:28 ` Russell King - ARM Linux
0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2017-10-27 15:28 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
> Hi Ard,
>
> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
> > ARM shares its EFI stub implementation with arm64, which has some
> > special handling in the virtual remapping code to
> > a) make sure that we can map everything even if the OS executes
> > with 64k page size, and
> > b) make sure that adjacent regions with the same attributes are not
> > reordered or moved apart in memory.
> >
> > The latter is a workaround for a 'feature' that was shortly recommended
> > by UEFI spec v2.5, but deprecated shortly after, due to the fact that
> > it broke many OS installers, including non-Linux ones, and it was never
> > widely implemented for ARM systems. Before implementing b), the arm64
> > code simply rounded up all regions to 64 KB granularity, but given that
> > that results in moving adjacent regions apart, it had to be refined when
> > b) was implemented.
> >
> > The adjacency check requires a sort() pass, due to the fact that the
> > UEFI spec does not mandate any ordering, and the inclusion of the
> > lib/sort.c code into the ARM EFI stub is causing some trouble with
> > the decompressor build due to the fact that its EXPORT_SYMBOL() call
> > triggers the creation of ksymtab/kcrctab sections.
> >
> > So let's simply do away with the adjacency check for ARM, and simply put
> > all UEFI runtime regions together if they have the same memory attributes.
> > This is guaranteed to work, given that ARM only supports 4 KB pages,
> > and allows us to remove the sort() call entirely.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > drivers/firmware/efi/libstub/Makefile | 6 +++---
> > drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
> > 2 files changed, 8 insertions(+), 5 deletions(-)
> >
>
> I thought I already provided my tag, but just in case:
> This fixes the boot problems I had on the bananapi-r2.
>
> Tested-by: Matthias Brugger <matthias.bgg@gmail.com>
Please test my "fixes" branch - that has all three patches merged.
We don't have linux-next running at the moment, so folk need to be
manually fetching the appropriate git trees for testing.
--
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] 22+ messages in thread
[parent not found: <20171027152850.GK20805-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>]
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-27 15:28 ` Russell King - ARM Linux
@ 2017-10-27 16:29 ` Matthias Brugger
-1 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-27 16:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
On 10/27/2017 05:28 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
>>> ARM shares its EFI stub implementation with arm64, which has some
>>> special handling in the virtual remapping code to
>>> a) make sure that we can map everything even if the OS executes
>>> with 64k page size, and
>>> b) make sure that adjacent regions with the same attributes are not
>>> reordered or moved apart in memory.
>>>
>>> The latter is a workaround for a 'feature' that was shortly recommended
>>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>>> it broke many OS installers, including non-Linux ones, and it was never
>>> widely implemented for ARM systems. Before implementing b), the arm64
>>> code simply rounded up all regions to 64 KB granularity, but given that
>>> that results in moving adjacent regions apart, it had to be refined when
>>> b) was implemented.
>>>
>>> The adjacency check requires a sort() pass, due to the fact that the
>>> UEFI spec does not mandate any ordering, and the inclusion of the
>>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>>> triggers the creation of ksymtab/kcrctab sections.
>>>
>>> So let's simply do away with the adjacency check for ARM, and simply put
>>> all UEFI runtime regions together if they have the same memory attributes.
>>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>>> and allows us to remove the sort() call entirely.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>> drivers/firmware/efi/libstub/Makefile | 6 +++---
>>> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
>>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>> I thought I already provided my tag, but just in case:
>> This fixes the boot problems I had on the bananapi-r2.
>>
>> Tested-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Please test my "fixes" branch - that has all three patches merged.
>
> We don't have linux-next running at the moment, so folk need to be
> manually fetching the appropriate git trees for testing.
>
I gave your branch a shot, feel free to add my:
Tested-by: Matthias Brugger <mbrugger-IBi9RG/b67k@public.gmane.org>
Especially for commit
4c2b35b71ea3 ARM: verify size of zImage
Regards,
Matthias
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-27 16:29 ` Matthias Brugger
0 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-27 16:29 UTC (permalink / raw)
To: linux-arm-kernel
On 10/27/2017 05:28 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
>>> ARM shares its EFI stub implementation with arm64, which has some
>>> special handling in the virtual remapping code to
>>> a) make sure that we can map everything even if the OS executes
>>> with 64k page size, and
>>> b) make sure that adjacent regions with the same attributes are not
>>> reordered or moved apart in memory.
>>>
>>> The latter is a workaround for a 'feature' that was shortly recommended
>>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>>> it broke many OS installers, including non-Linux ones, and it was never
>>> widely implemented for ARM systems. Before implementing b), the arm64
>>> code simply rounded up all regions to 64 KB granularity, but given that
>>> that results in moving adjacent regions apart, it had to be refined when
>>> b) was implemented.
>>>
>>> The adjacency check requires a sort() pass, due to the fact that the
>>> UEFI spec does not mandate any ordering, and the inclusion of the
>>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>>> triggers the creation of ksymtab/kcrctab sections.
>>>
>>> So let's simply do away with the adjacency check for ARM, and simply put
>>> all UEFI runtime regions together if they have the same memory attributes.
>>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>>> and allows us to remove the sort() call entirely.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> drivers/firmware/efi/libstub/Makefile | 6 +++---
>>> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
>>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>> I thought I already provided my tag, but just in case:
>> This fixes the boot problems I had on the bananapi-r2.
>>
>> Tested-by: Matthias Brugger <matthias.bgg@gmail.com>
>
> Please test my "fixes" branch - that has all three patches merged.
>
> We don't have linux-next running at the moment, so folk need to be
> manually fetching the appropriate git trees for testing.
>
I gave your branch a shot, feel free to add my:
Tested-by: Matthias Brugger <mbrugger@suse.com>
Especially for commit
4c2b35b71ea3 ARM: verify size of zImage
Regards,
Matthias
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
2017-10-27 15:28 ` Russell King - ARM Linux
@ 2017-10-27 16:30 ` Matthias Brugger
-1 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-27 16:30 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
zyw-TNX95d0MmH7DzftRWevZcw, jeffy.chen-TNX95d0MmH7DzftRWevZcw,
gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
On 10/27/2017 05:28 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
>>> ARM shares its EFI stub implementation with arm64, which has some
>>> special handling in the virtual remapping code to
>>> a) make sure that we can map everything even if the OS executes
>>> with 64k page size, and
>>> b) make sure that adjacent regions with the same attributes are not
>>> reordered or moved apart in memory.
>>>
>>> The latter is a workaround for a 'feature' that was shortly recommended
>>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>>> it broke many OS installers, including non-Linux ones, and it was never
>>> widely implemented for ARM systems. Before implementing b), the arm64
>>> code simply rounded up all regions to 64 KB granularity, but given that
>>> that results in moving adjacent regions apart, it had to be refined when
>>> b) was implemented.
>>>
>>> The adjacency check requires a sort() pass, due to the fact that the
>>> UEFI spec does not mandate any ordering, and the inclusion of the
>>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>>> triggers the creation of ksymtab/kcrctab sections.
>>>
>>> So let's simply do away with the adjacency check for ARM, and simply put
>>> all UEFI runtime regions together if they have the same memory attributes.
>>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>>> and allows us to remove the sort() call entirely.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>> drivers/firmware/efi/libstub/Makefile | 6 +++---
>>> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
>>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>> I thought I already provided my tag, but just in case:
>> This fixes the boot problems I had on the bananapi-r2.
>>
>> Tested-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Please test my "fixes" branch - that has all three patches merged.
>
> We don't have linux-next running at the moment, so folk need to be
> manually fetching the appropriate git trees for testing.
>
I gave your branch a shot, feel free to add my:
Tested-by: Matthias Brugger <mbrugger-IBi9RG/b67k@public.gmane.org>
Especially for commit
4c2b35b71ea3 ARM: verify size of zImage
Regards,
Matthias
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map
@ 2017-10-27 16:30 ` Matthias Brugger
0 siblings, 0 replies; 22+ messages in thread
From: Matthias Brugger @ 2017-10-27 16:30 UTC (permalink / raw)
To: linux-arm-kernel
On 10/27/2017 05:28 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 27, 2017 at 05:12:58PM +0200, Matthias Brugger wrote:
>> Hi Ard,
>>
>> On 10/22/2017 04:14 PM, Ard Biesheuvel wrote:
>>> ARM shares its EFI stub implementation with arm64, which has some
>>> special handling in the virtual remapping code to
>>> a) make sure that we can map everything even if the OS executes
>>> with 64k page size, and
>>> b) make sure that adjacent regions with the same attributes are not
>>> reordered or moved apart in memory.
>>>
>>> The latter is a workaround for a 'feature' that was shortly recommended
>>> by UEFI spec v2.5, but deprecated shortly after, due to the fact that
>>> it broke many OS installers, including non-Linux ones, and it was never
>>> widely implemented for ARM systems. Before implementing b), the arm64
>>> code simply rounded up all regions to 64 KB granularity, but given that
>>> that results in moving adjacent regions apart, it had to be refined when
>>> b) was implemented.
>>>
>>> The adjacency check requires a sort() pass, due to the fact that the
>>> UEFI spec does not mandate any ordering, and the inclusion of the
>>> lib/sort.c code into the ARM EFI stub is causing some trouble with
>>> the decompressor build due to the fact that its EXPORT_SYMBOL() call
>>> triggers the creation of ksymtab/kcrctab sections.
>>>
>>> So let's simply do away with the adjacency check for ARM, and simply put
>>> all UEFI runtime regions together if they have the same memory attributes.
>>> This is guaranteed to work, given that ARM only supports 4 KB pages,
>>> and allows us to remove the sort() call entirely.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> drivers/firmware/efi/libstub/Makefile | 6 +++---
>>> drivers/firmware/efi/libstub/arm-stub.c | 7 +++++--
>>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>
>> I thought I already provided my tag, but just in case:
>> This fixes the boot problems I had on the bananapi-r2.
>>
>> Tested-by: Matthias Brugger <matthias.bgg@gmail.com>
>
> Please test my "fixes" branch - that has all three patches merged.
>
> We don't have linux-next running at the moment, so folk need to be
> manually fetching the appropriate git trees for testing.
>
I gave your branch a shot, feel free to add my:
Tested-by: Matthias Brugger <mbrugger@suse.com>
Especially for commit
4c2b35b71ea3 ARM: verify size of zImage
Regards,
Matthias
^ permalink raw reply [flat|nested] 22+ messages in thread