[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Ard Biesheuvel]

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);
-- 
2.11.0

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Ard Biesheuvel]

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);
-- 
2.11.0

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[jeffy]

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);
>

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[jeffy]

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);
>

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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);
> 

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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);
> 

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Russell King - ARM Linux]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Russell King - ARM Linux]

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

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Ard Biesheuvel]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Ard Biesheuvel]

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

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Will Deacon]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Will Deacon]

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

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Gregory CLEMENT]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Gregory CLEMENT]

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

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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);
> 

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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);
> 

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Russell King - ARM Linux]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Russell King - ARM Linux]

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

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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

Re: [PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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

[PATCH] efi/libstub: arm: omit sorting of the UEFI memory map

[Matthias Brugger]

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