[PATCH v1] efi_loader: Handle GD_FLG_SKIP_RELOC

[Ilias Apalodimas]

On Sat Mar 29, 2025 at 2:02 PM EET, Heinrich Schuchardt wrote:
> On 3/26/25 06:46, Varadarajan Narayanan wrote:
>> If the EFI runtime services pointers are relocated even though
>> relocation is skipped, it corrupts some other data resulting in some
>> unexpected behaviour.
>>
>> In this specific case, it overwrote some page table entries resulting in
>> the device memory address range's mappings getting removed. Eventually,
>> after the completion of efi_runtime_relocate(), when a driver tries to
>> access its device's registers it crashes since the mappings are absent.
>
> Hello Varadarajan,
>
> thank you for pointing out this issue.
>
> I found two places where the flag is set:
>
> lib/efi/efi_app.c:198:  board_init_f(GD_FLG_SKIP_RELOC);
> board/synopsys/iot_devkit/iot_devkit.c:117:     gd->flags |=
> GD_FLG_SKIP_RELOC;
>
> iot_devkit_defconfig does not support UEFI.
>
>
> lib/efi/efi_app.c:198:  board_init_f(GD_FLG_SKIP_RELOC);
>
> efi-x86_app64_defconfig does not support UEFI either.
>
>
> For existing boards this seems to be a non-issue.

It's not but IIRC it was discovered because they added the non reloc flag in a Qualcomm board.

>
>>
>> Signed-off-by: Varadarajan Narayanan <quic_varada at quicinc.com>
>> ---
>>   common/board_r.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/board_r.c b/common/board_r.c
>> index 179259b00de..1dd3b96c2de 100644
>> --- a/common/board_r.c
>> +++ b/common/board_r.c
>> @@ -169,7 +169,8 @@ static int initr_reloc_global_data(void)
>>   	 */
>>   	efi_save_gd();
>>
>> -	efi_runtime_relocate(gd->relocaddr, NULL);
>> +	if (!(gd->flags & GD_FLG_SKIP_RELOC))
>> +		efi_runtime_relocate(gd->relocaddr, NULL);
>
> efi_runtime_relocate() is called twice:
>
> * when moving U-Boot to high memory (that is here)
> * when the OS call SetVirtualAddressMap()
>
> Removing the call here looks like the right thing to do when main U-Boot
> is not relocated.
>
> Did you test that starting an operating system via UEFI works in your
> use case?

He did and I also asked him to run a runtime service -- e.g GetVariable.

>
> It would be preferable to have a QEMU based defconfig where we could
> test this scenario and add it to the CI.
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>

As I said this looks correct, but please test the runtime invovation

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

>
>
>>   #endif
>>
>>   	return 0;
>>
>> base-commit: 244e61fbb7f5045e4e187024f7ae80434c952145