[PATCH] efi_loader: Don't carve out memory reservations too early
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 kettenis@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 */ + 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)) {
Am 16. Februar 2024 00:25:34 MEZ schrieb Mark Kettenis kettenis@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 kettenis@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. Wouldn't we miss creating the ft_board_setup() reservations before efi_carve_out_dt_rsv(fdt)?
Best regards
Heinrich
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)) {
Date: Fri, 16 Feb 2024 00:38:25 +0100 From: Heinrich Schuchardt xypron.glpk@gmx.de
Am 16. Februar 2024 00:25:34 MEZ schrieb Mark Kettenis kettenis@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 kettenis@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?
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)) {
On 2/16/24 3:17 PM, Mark Kettenis wrote:
Date: Fri, 16 Feb 2024 00:38:25 +0100 From: Heinrich Schuchardt xypron.glpk@gmx.de
Am 16. Februar 2024 00:25:34 MEZ schrieb Mark Kettenis kettenis@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 kettenis@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.
Best regards
Heinrich
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)) {
Date: Sat, 17 Feb 2024 12:02:56 +0100 From: Heinrich Schuchardt xypron.glpk@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.glpk@gmx.de
Am 16. Februar 2024 00:25:34 MEZ schrieb Mark Kettenis kettenis@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 kettenis@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)) {
participants (3)
-
Heinrich Schuchardt -
Mark Kettenis -
Mark Kettenis