| Message ID | 20180829185330.30493-1-swarren@wwwdotorg.org |
|---|---|
| State | Superseded |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | [U-Boot,1/2] ARM: tegra: reserve unmapped RAM so EFI doesn't use it | expand |
On 08/29/2018 08:53 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> > --- > 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..869d5d99d2fd 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 / 4096, Please, do not hard code the page size here. You can use >> EFI_PAGE_SHIFT. The constant is defined in efi.h. > + EFI_RESERVED_MEMORY_TYPE, false); The EFI spec says: "Regions that are backed by physical hardware, but are not supposed to be accessed by the OS, must be returned as EfiReservedMemoryType." I assume this is not what you want. To only avoid that the U-Boot EFI implementation uses the memory make it EFI_BOOT_SERVICES_DATA. Best regards Heinrich > + } > +#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 22:52, Heinrich Schuchardt wrote: > On 08/29/2018 08:53 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> >> --- >> 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..869d5d99d2fd 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 / 4096, > > Please, do not hard code the page size here. You can use >>> EFI_PAGE_SHIFT. The constant is defined in efi.h. > >> + EFI_RESERVED_MEMORY_TYPE, false); > > The EFI spec says: > > "Regions that are backed by physical hardware, but are not supposed to > be accessed by the OS, must be returned as EfiReservedMemoryType." > > I assume this is not what you want. To only avoid that the U-Boot EFI > implementation uses the memory make it EFI_BOOT_SERVICES_DATA. Basically it boils down to what the memory is reserved for. For DRAM that the OS should use, use EFI_BOOT_SERVICES_DATA. That way no U-Boot allocations will use the memory, but Linux for example will. If the memory however should not be used by an OS either (like a reserved ATF region), then mark it as reserved. Alex
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 421a71b3014d..869d5d99d2fd 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 / 4096, + EFI_RESERVED_MEMORY_TYPE, false); + } +#endif + #if defined(CONFIG_TEGRA_SUPPORT_NON_SECURE) if (tegra_cpu_is_non_secure()) { printf("CPU is in NS mode\n");