> Date: Sat, 17 Feb 2024 12:02:56 +0100 > From: Heinrich Schuchardt <xypron.g...@gmx.de>
Hi Heinrich, > On 2/16/24 3:17 PM, Mark Kettenis wrote: > >> Date: Fri, 16 Feb 2024 00:38:25 +0100 > >> From: Heinrich Schuchardt <xypron.g...@gmx.de> > >> > >> Am 16. Februar 2024 00:25:34 MEZ schrieb Mark Kettenis > >> <kette...@openbsd.org>: > >>> Moving the efi_carve_out_dt_rsv() call in commit 1be415b21b2d > >>> ("efi_loader: create memory reservations in ACPI case") > >>> broke boards that create additional memory reservations in > >>> ft_board_setup() since it is now called before those additional > >>> memory reservations are made. This is the case for the rk3588 > >>> boards and breaks booting OpenBSD on those boards. > >>> > >>> Move the call back to its original location and add a call in > >>> the code path used for ACPI. > >>> > >>> Fixes: 1be415b21b2d ("efi_loader: create memory reservations in ACPI > >>> case") > >>> Signed-off-by: Mark Kettenis <kette...@openbsd.org> > >>> --- > >>> lib/efi_loader/efi_helper.c | 11 +++++++---- > >>> 1 file changed, 7 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c > >>> index 5dd9cc876e..58761fae78 100644 > >>> --- a/lib/efi_loader/efi_helper.c > >>> +++ b/lib/efi_loader/efi_helper.c > >>> @@ -456,11 +456,11 @@ efi_status_t efi_install_fdt(void *fdt) > >>> return EFI_LOAD_ERROR; > >>> } > >>> > >>> - /* Create memory reservations as indicated by the device tree */ > >>> - efi_carve_out_dt_rsv(fdt); > >>> - > >>> - if (CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) > >>> + if (CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) { > >>> + /* Create memory reservations as indicated by the device tree */ > >> > >> Imagine booting the rk3588 board with ACPI. > > > > I'd rather not, thank you ;) > > > >> Wouldn't we miss creating the ft_board_setup() reservations before > >> efi_carve_out_dt_rsv(fdt)? > > > > Yes. And arguably the these memory reservations should be made way > > earlier, at the the time that efi_memory_init() runs. I think we're > > just lucky that efi_allocate_pages() doesn't hand us memory from these > > areas in copy_fdt(). > > > > Better ideas? > > > > image_setup_libfdt(&img, fdt, NULL) must be called before > efi_carve_out_dt_rsv(). > > Could you, please, add this call in this path, too. Otherwise the patch > looks correct. Yes I can. Just sent out v2. Thanks, Mark > > > >>> + efi_carve_out_dt_rsv(fdt); > >>> return EFI_SUCCESS; > >>> + } > >>> > >>> /* Prepare device tree for payload */ > >>> ret = copy_fdt(&fdt); > >>> @@ -474,6 +474,9 @@ efi_status_t efi_install_fdt(void *fdt) > >>> return EFI_LOAD_ERROR; > >>> } > >>> > >>> + /* Create memory reservations as indicated by the device tree */ > >>> + efi_carve_out_dt_rsv(fdt); > >>> + > >>> efi_try_purge_kaslr_seed(fdt); > >>> > >>> if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { > >> > > > >