[U-Boot] [PATCH V2 1/2] ARM: tegra: reserve unmapped RAM so EFI doesn't use it
From: Stephen Warren swarren@nvidia.com
Tegra U-Boot ensures that board_get_usable_ram_top() never returns a value over 4GB, since some peripherals can't access such addresses. However, on systems with more than 2GB of RAM, RAM bank 1 does describe this extra RAM, so that Linux (or whatever OS) can use it, subject to DMA limitations. Since board_get_usable_ram_top() points at the top of RAM bank 0, the memory locations describes by RAM bank 1 are not mapped by U-Boot's MMU configuration, and so cannot be used for anything.
For some completely inexplicable reason, U-Boot's EFI support ignores the value returned by board_get_usable_ram_top(), and EFI memory allocation routines will return values above U-Boot's RAM top. This causes U-Boot to crash when it accesses that RAM, since it isn't mapped by the MMU. One use-case where this happens is TFTP download of a file on Jetson TX1 (p2371-2180).
This change explicitly tells the EFI code that this extra RAM should not be used, thus avoiding the crash.
A previous attempt to make EFI honor board_get_usable_ram_top() was rejected. So, this patch will need to be replicated for any board that implements board_get_usable_ram_top().
Fixes: aa909462d018 ("efi_loader: efi_allocate_pages is too restrictive") Signed-off-by: Stephen Warren swarren@nvidia.com --- v2: - Don't hard-code EFI page size. - Register RAM as a boot services data rather than reserved. --- arch/arm/mach-tegra/board2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 421a71b3014d..f893966140a1 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -6,6 +6,7 @@
#include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <ns16550.h> #include <usb.h> @@ -210,6 +211,19 @@ int board_early_init_f(void)
int board_late_init(void) { +#ifdef CONFIG_EFI_LOADER + if (gd->bd->bi_dram[1].start) { + /* + * Only bank 0 is below board_get_usable_ram_top(), so all of + * bank 1 is not mapped by the U-Boot MMU configuration, and so + * we must prevent EFI from using it. + */ + efi_add_memory_map(gd->bd->bi_dram[1].start, + gd->bd->bi_dram[1].size / EFI_PAGE_SIZE, + EFI_BOOT_SERVICES_DATA, false); + } +#endif + #if defined(CONFIG_TEGRA_SUPPORT_NON_SECURE) if (tegra_cpu_is_non_secure()) { printf("CPU is in NS mode\n");
From: Stephen Warren swarren@nvidia.com
This reverts commit ccfc78b820e5e431c5bd73b072e7536a972e1710.
Now that the underlying issue is fixed, we can revert the revert and hence restore the original EFI code.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v2: No change. --- lib/efi_loader/efi_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index e2b40aa85b5a..e902d5a280bb 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -304,7 +304,7 @@ efi_status_t efi_allocate_pages(int type, int memory_type, switch (type) { case EFI_ALLOCATE_ANY_PAGES: /* Any page */ - addr = efi_find_free_memory(len, gd->start_addr_sp); + addr = efi_find_free_memory(len, -1ULL); if (!addr) { r = EFI_NOT_FOUND; break;
On 08/29/2018 11:34 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Tegra U-Boot ensures that board_get_usable_ram_top() never returns a value over 4GB, since some peripherals can't access such addresses. However, on systems with more than 2GB of RAM, RAM bank 1 does describe this extra RAM, so that Linux (or whatever OS) can use it, subject to DMA limitations. Since board_get_usable_ram_top() points at the top of RAM bank 0, the memory locations describes by RAM bank 1 are not mapped by U-Boot's MMU configuration, and so cannot be used for anything.
For some completely inexplicable reason, U-Boot's EFI support ignores the value returned by board_get_usable_ram_top(), and EFI memory allocation routines will return values above U-Boot's RAM top. This causes U-Boot to crash when it accesses that RAM, since it isn't mapped by the MMU. One use-case where this happens is TFTP download of a file on Jetson TX1 (p2371-2180).
This change explicitly tells the EFI code that this extra RAM should not be used, thus avoiding the crash.
A previous attempt to make EFI honor board_get_usable_ram_top() was rejected. So, this patch will need to be replicated for any board that implements board_get_usable_ram_top().
Fixes: aa909462d018 ("efi_loader: efi_allocate_pages is too restrictive") Signed-off-by: Stephen Warren swarren@nvidia.com
v2:
- Don't hard-code EFI page size.
- Register RAM as a boot services data rather than reserved.
arch/arm/mach-tegra/board2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 421a71b3014d..f893966140a1 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -6,6 +6,7 @@
#include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <ns16550.h> #include <usb.h> @@ -210,6 +211,19 @@ int board_early_init_f(void)
int board_late_init(void) { +#ifdef CONFIG_EFI_LOADER
- if (gd->bd->bi_dram[1].start) {
/** Only bank 0 is below board_get_usable_ram_top(), so all of* bank 1 is not mapped by the U-Boot MMU configuration, and so* we must prevent EFI from using it.*/efi_add_memory_map(gd->bd->bi_dram[1].start,gd->bd->bi_dram[1].size / EFI_PAGE_SIZE,
Are you sure all boards do the division without library function? This is why I prefer >> EFI_PAGE_SHIFT.
Compiling SPL fails for harmony_defconfig.
LD spl/common/spl/built-in.o arch/arm/mach-tegra/board2.c: In function ‘board_late_init’: arch/arm/mach-tegra/board2.c:221:3: warning: implicit declaration of function ‘efi_add_memory_map’; did you mean ‘fdt_add_mem_rsv’? [-Wimplicit-function-declaration] efi_add_memory_map(gd->bd->bi_dram[1].start, ^~~~~~~~~~~~~~~~~~ fdt_add_mem_rsv
Regards
Heinrich
EFI_BOOT_SERVICES_DATA, false);- }
+#endif
#if defined(CONFIG_TEGRA_SUPPORT_NON_SECURE) if (tegra_cpu_is_non_secure()) { printf("CPU is in NS mode\n");
On 29.08.18 23:52, Heinrich Schuchardt wrote:
On 08/29/2018 11:34 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Tegra U-Boot ensures that board_get_usable_ram_top() never returns a value over 4GB, since some peripherals can't access such addresses. However, on systems with more than 2GB of RAM, RAM bank 1 does describe this extra RAM, so that Linux (or whatever OS) can use it, subject to DMA limitations. Since board_get_usable_ram_top() points at the top of RAM bank 0, the memory locations describes by RAM bank 1 are not mapped by U-Boot's MMU configuration, and so cannot be used for anything.
For some completely inexplicable reason, U-Boot's EFI support ignores the value returned by board_get_usable_ram_top(), and EFI memory allocation routines will return values above U-Boot's RAM top. This causes U-Boot to crash when it accesses that RAM, since it isn't mapped by the MMU. One use-case where this happens is TFTP download of a file on Jetson TX1 (p2371-2180).
This change explicitly tells the EFI code that this extra RAM should not be used, thus avoiding the crash.
A previous attempt to make EFI honor board_get_usable_ram_top() was rejected. So, this patch will need to be replicated for any board that implements board_get_usable_ram_top().
Fixes: aa909462d018 ("efi_loader: efi_allocate_pages is too restrictive") Signed-off-by: Stephen Warren swarren@nvidia.com
v2:
- Don't hard-code EFI page size.
- Register RAM as a boot services data rather than reserved.
arch/arm/mach-tegra/board2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 421a71b3014d..f893966140a1 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -6,6 +6,7 @@
#include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <ns16550.h> #include <usb.h> @@ -210,6 +211,19 @@ int board_early_init_f(void)
int board_late_init(void) { +#ifdef CONFIG_EFI_LOADER
- if (gd->bd->bi_dram[1].start) {
/** Only bank 0 is below board_get_usable_ram_top(), so all of* bank 1 is not mapped by the U-Boot MMU configuration, and so* we must prevent EFI from using it.*/efi_add_memory_map(gd->bd->bi_dram[1].start,gd->bd->bi_dram[1].size / EFI_PAGE_SIZE,Are you sure all boards do the division without library function? This is why I prefer >> EFI_PAGE_SHIFT.
Compiling SPL fails for harmony_defconfig.
LD spl/common/spl/built-in.o arch/arm/mach-tegra/board2.c: In function ‘board_late_init’: arch/arm/mach-tegra/board2.c:221:3: warning: implicit declaration of function ‘efi_add_memory_map’; did you mean ‘fdt_add_mem_rsv’? [-Wimplicit-function-declaration] efi_add_memory_map(gd->bd->bi_dram[1].start, ^~~~~~~~~~~~~~~~~~ fdt_add_mem_rsv
I think that means you want #if CONFIG_IS_ENABLED(EFI_LOADER) because the file is also compiled for SPL.
Alex
On 08/30/2018 12:43 AM, Alexander Graf wrote:
On 29.08.18 23:52, Heinrich Schuchardt wrote:
On 08/29/2018 11:34 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Tegra U-Boot ensures that board_get_usable_ram_top() never returns a value over 4GB, since some peripherals can't access such addresses. However, on systems with more than 2GB of RAM, RAM bank 1 does describe this extra RAM, so that Linux (or whatever OS) can use it, subject to DMA limitations. Since board_get_usable_ram_top() points at the top of RAM bank 0, the memory locations describes by RAM bank 1 are not mapped by U-Boot's MMU configuration, and so cannot be used for anything.
For some completely inexplicable reason, U-Boot's EFI support ignores the value returned by board_get_usable_ram_top(), and EFI memory allocation routines will return values above U-Boot's RAM top. This causes U-Boot to crash when it accesses that RAM, since it isn't mapped by the MMU. One use-case where this happens is TFTP download of a file on Jetson TX1 (p2371-2180).
This change explicitly tells the EFI code that this extra RAM should not be used, thus avoiding the crash.
A previous attempt to make EFI honor board_get_usable_ram_top() was rejected. So, this patch will need to be replicated for any board that implements board_get_usable_ram_top().
Fixes: aa909462d018 ("efi_loader: efi_allocate_pages is too restrictive") Signed-off-by: Stephen Warren swarren@nvidia.com
v2:
- Don't hard-code EFI page size.
- Register RAM as a boot services data rather than reserved.
arch/arm/mach-tegra/board2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 421a71b3014d..f893966140a1 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -6,6 +6,7 @@
#include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <ns16550.h> #include <usb.h> @@ -210,6 +211,19 @@ int board_early_init_f(void)
int board_late_init(void) { +#ifdef CONFIG_EFI_LOADER
- if (gd->bd->bi_dram[1].start) {
/** Only bank 0 is below board_get_usable_ram_top(), so all of* bank 1 is not mapped by the U-Boot MMU configuration, and so* we must prevent EFI from using it.*/efi_add_memory_map(gd->bd->bi_dram[1].start,gd->bd->bi_dram[1].size / EFI_PAGE_SIZE,Are you sure all boards do the division without library function? This is why I prefer >> EFI_PAGE_SHIFT.
Compiling SPL fails for harmony_defconfig.
LD spl/common/spl/built-in.o arch/arm/mach-tegra/board2.c: In function ‘board_late_init’: arch/arm/mach-tegra/board2.c:221:3: warning: implicit declaration of function ‘efi_add_memory_map’; did you mean ‘fdt_add_mem_rsv’? [-Wimplicit-function-declaration] efi_add_memory_map(gd->bd->bi_dram[1].start, ^~~~~~~~~~~~~~~~~~ fdt_add_mem_rsv
Oh, I figured that warning were errors and only checked that Jenkins passed, not for warnings in the logs.
I think that means you want #if CONFIG_IS_ENABLED(EFI_LOADER) because the file is also compiled for SPL.
That does work, but include/efi_loader.h uses the following, so I think it makes sense to use the exact same ifdef in the source code:
#if defined(CONFIG_EFI_LOADER) && !defined(CONFIG_SPL_BUILD)
Or, should I update the header to use CONFIG_IS_ENABLED too (and hope that won't break any other boards).
On 30.08.18 17:45, Stephen Warren wrote:
On 08/30/2018 12:43 AM, Alexander Graf wrote:
On 29.08.18 23:52, Heinrich Schuchardt wrote:
On 08/29/2018 11:34 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Tegra U-Boot ensures that board_get_usable_ram_top() never returns a value over 4GB, since some peripherals can't access such addresses. However, on systems with more than 2GB of RAM, RAM bank 1 does describe this extra RAM, so that Linux (or whatever OS) can use it, subject to DMA limitations. Since board_get_usable_ram_top() points at the top of RAM bank 0, the memory locations describes by RAM bank 1 are not mapped by U-Boot's MMU configuration, and so cannot be used for anything.
For some completely inexplicable reason, U-Boot's EFI support ignores the value returned by board_get_usable_ram_top(), and EFI memory allocation routines will return values above U-Boot's RAM top. This causes U-Boot to crash when it accesses that RAM, since it isn't mapped by the MMU. One use-case where this happens is TFTP download of a file on Jetson TX1 (p2371-2180).
This change explicitly tells the EFI code that this extra RAM should not be used, thus avoiding the crash.
A previous attempt to make EFI honor board_get_usable_ram_top() was rejected. So, this patch will need to be replicated for any board that implements board_get_usable_ram_top().
Fixes: aa909462d018 ("efi_loader: efi_allocate_pages is too restrictive") Signed-off-by: Stephen Warren swarren@nvidia.com
v2:
- Don't hard-code EFI page size.
- Register RAM as a boot services data rather than reserved.
arch/arm/mach-tegra/board2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 421a71b3014d..f893966140a1 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -6,6 +6,7 @@ #include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <ns16550.h> #include <usb.h> @@ -210,6 +211,19 @@ int board_early_init_f(void) int board_late_init(void) { +#ifdef CONFIG_EFI_LOADER + if (gd->bd->bi_dram[1].start) { + /* + * Only bank 0 is below board_get_usable_ram_top(), so all of + * bank 1 is not mapped by the U-Boot MMU configuration, and so + * we must prevent EFI from using it. + */ + efi_add_memory_map(gd->bd->bi_dram[1].start, + gd->bd->bi_dram[1].size / EFI_PAGE_SIZE,
Are you sure all boards do the division without library function? This is why I prefer >> EFI_PAGE_SHIFT.
Compiling SPL fails for harmony_defconfig.
LD spl/common/spl/built-in.o arch/arm/mach-tegra/board2.c: In function ‘board_late_init’: arch/arm/mach-tegra/board2.c:221:3: warning: implicit declaration of function ‘efi_add_memory_map’; did you mean ‘fdt_add_mem_rsv’? [-Wimplicit-function-declaration] efi_add_memory_map(gd->bd->bi_dram[1].start, ^~~~~~~~~~~~~~~~~~ fdt_add_mem_rsv
Oh, I figured that warning were errors and only checked that Jenkins passed, not for warnings in the logs.
I think that means you want #if CONFIG_IS_ENABLED(EFI_LOADER) because the file is also compiled for SPL.
That does work, but include/efi_loader.h uses the following, so I think it makes sense to use the exact same ifdef in the source code:
#if defined(CONFIG_EFI_LOADER) && !defined(CONFIG_SPL_BUILD)
Or, should I update the header to use CONFIG_IS_ENABLED too (and hope that won't break any other boards).
Yes, please. I just didn't know about CONFIG_IS_ENABLED() back when I wrote those lines.
Alex
participants (3)
-
Alexander Graf -
Heinrich Schuchardt -
Stephen Warren