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_var...@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.g...@gmx.de> As I said this looks correct, but please test the runtime invovation Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > >> #endif >> >> return 0; >> >> base-commit: 244e61fbb7f5045e4e187024f7ae80434c952145
