| Message ID | 20241011212126.747741-3-sjg@chromium.org |
|---|---|
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Adjust initial EFI memory-allocation to be in the U-Boot region | expand |
Am 11. Oktober 2024 23:21:23 MESZ schrieb Simon Glass <sjg@chromium.org>: >The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is >still needed and 24 boards (lx2160ardb_tfa_stmm, >lx2162aqds_tfa_SECURE_BOOT and the like) use it. > >This feature uses EFI page allocation to create a 64MB buffer 'in space' >without any knowledge of where boards intend to load their images. This >may result in image corruption or other problems. > >For example, if the feature is enabled on qemu_arm64 it puts the EFI >bounce buffer at 1045MB, with the kernel at 1028MB and the ramdisk at >1088MB. The kernel is probably smaller than 27MB but the buffer does >overlap the ramdisk. > >The solution is probably to use BOUNCE_BUFFER instead, with the EFI >version being dropped. For now, show the address of the EFI bounce >buffer so people have a better chance to debug any problem. > >Signed-off-by: Simon Glass <sjg@chromium.org> >--- >In response to questions on how to show this problem: > >First, add CONFIG_EFI_LOADER_BOUNCE_BUFFER=y to >configs/qemu_arm64_defconfig and build > >Then, to run: >qemu-system-aarch64 -machine virt -nographic -cpu cortex-a57 \ > -bios u-boot.bin > >U-Boot 2024.10-00809-gae29e5f76e4a-dirty (Oct 11 2024 - 11:48:46 -0600) > >DRAM: 128 MiB >Core: 51 devices, 14 uclasses, devicetree: board >Flash: 64 MiB >Loading Environment from Flash... *** Warning - bad CRC, using default environment > >In: serial,usbkbd >Out: serial,vidconsole >Err: serial,vidconsole >No USB controllers found >Net: eth0: virtio-net#32 > >starting USB... >No USB controllers found >Hit any key to stop autoboot: 0 >=> bootefi hello >Enabling coop memory >No EFI system partition >No EFI system partition >Failed to persist EFI variables >Missing TPMv2 device for EFI_TCG_PROTOCOL >Missing RNG device for EFI_RNG_PROTOCOL >Warning: EFI bounce buffer 000000004157f000-000000004557f000 >Not loaded from disk >Booting /MemoryMapped(0x0,0x47763730,0x31b0) >EFI: Call: efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) >EFI: 0 returned by efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) >Hello, world! >Running on UEFI 2.10 >Firmware vendor: Das U-Boot >Firmware revision: 20241000 >Have device tree >Load options: <none> >File path: <none> >Boot device: /MemoryMapped(0x0,0x47763730,0x31b0) >=> print kernel_addr_r >kernel_addr_r=0x40400000 >=> print ramdisk_addr_r >ramdisk_addr_r=0x44000000 >=> > >The EFI bounce buffer overlaps with the ramdisk. > >Changes in v4: >- Reword the commit message since this feature is needed > > lib/efi_loader/efi_bootbin.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > >diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c >index a87006b3c0e..eefdd660cee 100644 >--- a/lib/efi_loader/efi_bootbin.c >+++ b/lib/efi_loader/efi_bootbin.c >@@ -209,6 +209,15 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) > return -1; > } > >+#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER >+ /* >+ * Add a warning about this buffer, since it may conflict with other >+ * things >+ */ >+ log_debug("Warning: EFI bounce buffer %p-%p\n", efi_bounce_buffer, >+ efi_bounce_buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE); >+#endif Adding a debug message here is ok. But writing "warning" in a debug message which is just printing an address for a configuration selected item looks weird to me. Best regards Heinrich >+ > ret = efi_install_fdt(fdt); > if (ret != EFI_SUCCESS) > return ret;
Hi Heinrich, On Fri, 11 Oct 2024 at 16:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > Am 11. Oktober 2024 23:21:23 MESZ schrieb Simon Glass <sjg@chromium.org>: > >The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is > >still needed and 24 boards (lx2160ardb_tfa_stmm, > >lx2162aqds_tfa_SECURE_BOOT and the like) use it. > > > >This feature uses EFI page allocation to create a 64MB buffer 'in space' > >without any knowledge of where boards intend to load their images. This > >may result in image corruption or other problems. > > > >For example, if the feature is enabled on qemu_arm64 it puts the EFI > >bounce buffer at 1045MB, with the kernel at 1028MB and the ramdisk at > >1088MB. The kernel is probably smaller than 27MB but the buffer does > >overlap the ramdisk. > > > >The solution is probably to use BOUNCE_BUFFER instead, with the EFI > >version being dropped. For now, show the address of the EFI bounce > >buffer so people have a better chance to debug any problem. > > > >Signed-off-by: Simon Glass <sjg@chromium.org> > >--- > >In response to questions on how to show this problem: > > > >First, add CONFIG_EFI_LOADER_BOUNCE_BUFFER=y to > >configs/qemu_arm64_defconfig and build > > > >Then, to run: > >qemu-system-aarch64 -machine virt -nographic -cpu cortex-a57 \ > > -bios u-boot.bin > > > >U-Boot 2024.10-00809-gae29e5f76e4a-dirty (Oct 11 2024 - 11:48:46 -0600) > > > >DRAM: 128 MiB > >Core: 51 devices, 14 uclasses, devicetree: board > >Flash: 64 MiB > >Loading Environment from Flash... *** Warning - bad CRC, using default environment > > > >In: serial,usbkbd > >Out: serial,vidconsole > >Err: serial,vidconsole > >No USB controllers found > >Net: eth0: virtio-net#32 > > > >starting USB... > >No USB controllers found > >Hit any key to stop autoboot: 0 > >=> bootefi hello > >Enabling coop memory > >No EFI system partition > >No EFI system partition > >Failed to persist EFI variables > >Missing TPMv2 device for EFI_TCG_PROTOCOL > >Missing RNG device for EFI_RNG_PROTOCOL > >Warning: EFI bounce buffer 000000004157f000-000000004557f000 > >Not loaded from disk > >Booting /MemoryMapped(0x0,0x47763730,0x31b0) > >EFI: Call: efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) > >EFI: 0 returned by efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) > >Hello, world! > >Running on UEFI 2.10 > >Firmware vendor: Das U-Boot > >Firmware revision: 20241000 > >Have device tree > >Load options: <none> > >File path: <none> > >Boot device: /MemoryMapped(0x0,0x47763730,0x31b0) > >=> print kernel_addr_r > >kernel_addr_r=0x40400000 > >=> print ramdisk_addr_r > >ramdisk_addr_r=0x44000000 > >=> > > > >The EFI bounce buffer overlaps with the ramdisk. > > > >Changes in v4: > >- Reword the commit message since this feature is needed > > > > lib/efi_loader/efi_bootbin.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > >diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c > >index a87006b3c0e..eefdd660cee 100644 > >--- a/lib/efi_loader/efi_bootbin.c > >+++ b/lib/efi_loader/efi_bootbin.c > >@@ -209,6 +209,15 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) > > return -1; > > } > > > >+#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER > >+ /* > >+ * Add a warning about this buffer, since it may conflict with other > >+ * things > >+ */ > >+ log_debug("Warning: EFI bounce buffer %p-%p\n", efi_bounce_buffer, > >+ efi_bounce_buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE); > >+#endif > > Adding a debug message here is ok. > > But writing "warning" in a debug message which is just printing an address for a configuration selected item looks weird to me. I added that because you said it needed to say it was a warning. I'll remove it and try again. Regards, Simon
On Mon, Oct 14, 2024 at 02:20:09PM -0600, Simon Glass wrote: > Hi Heinrich, > > On Fri, 11 Oct 2024 at 16:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > Am 11. Oktober 2024 23:21:23 MESZ schrieb Simon Glass <sjg@chromium.org>: > > >The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is > > >still needed and 24 boards (lx2160ardb_tfa_stmm, > > >lx2162aqds_tfa_SECURE_BOOT and the like) use it. > > > > > >This feature uses EFI page allocation to create a 64MB buffer 'in space' > > >without any knowledge of where boards intend to load their images. This > > >may result in image corruption or other problems. > > > > > >For example, if the feature is enabled on qemu_arm64 it puts the EFI > > >bounce buffer at 1045MB, with the kernel at 1028MB and the ramdisk at > > >1088MB. The kernel is probably smaller than 27MB but the buffer does > > >overlap the ramdisk. > > > > > >The solution is probably to use BOUNCE_BUFFER instead, with the EFI > > >version being dropped. For now, show the address of the EFI bounce > > >buffer so people have a better chance to debug any problem. > > > > > >Signed-off-by: Simon Glass <sjg@chromium.org> > > >--- > > >In response to questions on how to show this problem: > > > > > >First, add CONFIG_EFI_LOADER_BOUNCE_BUFFER=y to > > >configs/qemu_arm64_defconfig and build > > > > > >Then, to run: > > >qemu-system-aarch64 -machine virt -nographic -cpu cortex-a57 \ > > > -bios u-boot.bin > > > > > >U-Boot 2024.10-00809-gae29e5f76e4a-dirty (Oct 11 2024 - 11:48:46 -0600) > > > > > >DRAM: 128 MiB > > >Core: 51 devices, 14 uclasses, devicetree: board > > >Flash: 64 MiB > > >Loading Environment from Flash... *** Warning - bad CRC, using default environment > > > > > >In: serial,usbkbd > > >Out: serial,vidconsole > > >Err: serial,vidconsole > > >No USB controllers found > > >Net: eth0: virtio-net#32 > > > > > >starting USB... > > >No USB controllers found > > >Hit any key to stop autoboot: 0 > > >=> bootefi hello > > >Enabling coop memory > > >No EFI system partition > > >No EFI system partition > > >Failed to persist EFI variables > > >Missing TPMv2 device for EFI_TCG_PROTOCOL > > >Missing RNG device for EFI_RNG_PROTOCOL > > >Warning: EFI bounce buffer 000000004157f000-000000004557f000 > > >Not loaded from disk > > >Booting /MemoryMapped(0x0,0x47763730,0x31b0) > > >EFI: Call: efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) > > >EFI: 0 returned by efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) > > >Hello, world! > > >Running on UEFI 2.10 > > >Firmware vendor: Das U-Boot > > >Firmware revision: 20241000 > > >Have device tree > > >Load options: <none> > > >File path: <none> > > >Boot device: /MemoryMapped(0x0,0x47763730,0x31b0) > > >=> print kernel_addr_r > > >kernel_addr_r=0x40400000 > > >=> print ramdisk_addr_r > > >ramdisk_addr_r=0x44000000 > > >=> > > > > > >The EFI bounce buffer overlaps with the ramdisk. > > > > > >Changes in v4: > > >- Reword the commit message since this feature is needed > > > > > > lib/efi_loader/efi_bootbin.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > >diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c > > >index a87006b3c0e..eefdd660cee 100644 > > >--- a/lib/efi_loader/efi_bootbin.c > > >+++ b/lib/efi_loader/efi_bootbin.c > > >@@ -209,6 +209,15 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) > > > return -1; > > > } > > > > > >+#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER > > >+ /* > > >+ * Add a warning about this buffer, since it may conflict with other > > >+ * things > > >+ */ > > >+ log_debug("Warning: EFI bounce buffer %p-%p\n", efi_bounce_buffer, > > >+ efi_bounce_buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE); > > >+#endif > > > > Adding a debug message here is ok. > > > > But writing "warning" in a debug message which is just printing an address for a configuration selected item looks weird to me. > > I added that because you said it needed to say it was a warning. I'll > remove it and try again. Since we talked about it on IRC, the most useful option here would be instead to switch this to the regular CONFIG_BOUNCE_BUFFER code so that it can go away entirely. The second most useful option is yes, we can put a debug message for when someone enables "make a hole in memory for me" when they do not need that hole made in memory.
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index a87006b3c0e..eefdd660cee 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -209,6 +209,15 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) return -1; } +#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER + /* + * Add a warning about this buffer, since it may conflict with other + * things + */ + log_debug("Warning: EFI bounce buffer %p-%p\n", efi_bounce_buffer, + efi_bounce_buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE); +#endif + ret = efi_install_fdt(fdt); if (ret != EFI_SUCCESS) return ret;
The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is still needed and 24 boards (lx2160ardb_tfa_stmm, lx2162aqds_tfa_SECURE_BOOT and the like) use it. This feature uses EFI page allocation to create a 64MB buffer 'in space' without any knowledge of where boards intend to load their images. This may result in image corruption or other problems. For example, if the feature is enabled on qemu_arm64 it puts the EFI bounce buffer at 1045MB, with the kernel at 1028MB and the ramdisk at 1088MB. The kernel is probably smaller than 27MB but the buffer does overlap the ramdisk. The solution is probably to use BOUNCE_BUFFER instead, with the EFI version being dropped. For now, show the address of the EFI bounce buffer so people have a better chance to debug any problem. Signed-off-by: Simon Glass <sjg@chromium.org> --- In response to questions on how to show this problem: First, add CONFIG_EFI_LOADER_BOUNCE_BUFFER=y to configs/qemu_arm64_defconfig and build Then, to run: qemu-system-aarch64 -machine virt -nographic -cpu cortex-a57 \ -bios u-boot.bin U-Boot 2024.10-00809-gae29e5f76e4a-dirty (Oct 11 2024 - 11:48:46 -0600) DRAM: 128 MiB Core: 51 devices, 14 uclasses, devicetree: board Flash: 64 MiB Loading Environment from Flash... *** Warning - bad CRC, using default environment In: serial,usbkbd Out: serial,vidconsole Err: serial,vidconsole No USB controllers found Net: eth0: virtio-net#32 starting USB... No USB controllers found Hit any key to stop autoboot: 0 => bootefi hello Enabling coop memory No EFI system partition No EFI system partition Failed to persist EFI variables Missing TPMv2 device for EFI_TCG_PROTOCOL Missing RNG device for EFI_RNG_PROTOCOL Warning: EFI bounce buffer 000000004157f000-000000004557f000 Not loaded from disk Booting /MemoryMapped(0x0,0x47763730,0x31b0) EFI: Call: efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) EFI: 0 returned by efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) Hello, world! Running on UEFI 2.10 Firmware vendor: Das U-Boot Firmware revision: 20241000 Have device tree Load options: <none> File path: <none> Boot device: /MemoryMapped(0x0,0x47763730,0x31b0) => print kernel_addr_r kernel_addr_r=0x40400000 => print ramdisk_addr_r ramdisk_addr_r=0x44000000 => The EFI bounce buffer overlaps with the ramdisk. Changes in v4: - Reword the commit message since this feature is needed lib/efi_loader/efi_bootbin.c | 9 +++++++++ 1 file changed, 9 insertions(+)