[PATCH] efi_loader: Don't carve out memory reservations too early

[Heinrich Schuchardt]

On 2/16/24 3:17 PM, Mark Kettenis wrote:
>> Date: Fri, 16 Feb 2024 00:38:25 +0100
>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>
>> Am 16. Februar 2024 00:25:34 MEZ schrieb Mark Kettenis <kettenis at 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 at 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)) {
>>
>