Hi Heinrich,
[...]
Atish can you give it a spin and let me know if this fixes the issue for you? The objdump seems to be correct now, but I am not familiar with RISC-V. No regressions on Arm with TEE or memory backed variables. include/efi_variable.h | 12 ++++++++++++ lib/efi_loader/efi_var_mem.c | 12 +++++++++++- lib/efi_loader/efi_variable_tee.c | 2 +- 3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h index 4704a3c16e65..b2317eb7bf1c 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -306,4 +306,16 @@ efi_status_t __efi_runtime EFIAPI efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, u16 *variable_name, efi_guid_t *guid);
+/**
- efi_var_buf_update() - Update the value of efi_var_buf in efi_var_mem.c
Dear Ilias,
thank you for addressing this problem. The code looks fine to me. Just some ideas concerning comment lines:
Well, I broke it to begin with so ...
The value of efi_var_buf is the address it is pointing to. So i would prefer:
efi_var_buf_update() - udpate memory buffer for variables
- @var_buf: Source buffer
%s/Source/source/
Ok
- efi_var_buf is special since we use it on Runtime Services. We need
- to keep it static in efi_var_mem.c and avoid having it pulled into
- .GOT. Since it has to be static this function must be used to update
You already place a comment about .GOT where the declaration is. Here describing how the function is used would be of interest. E.g.
"This function copies to the memory buffer for UEFI variables. Call this function in ExitBootServices() if memory backed variables are only used at runtime to fill the buffer."
I'll replace it. Guess I was too concerned pointing out "Don't touch efi_var_buf"
- it
- */
+void efi_var_buf_update(struct efi_var_file *var_buf);
#endif diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index d155f25f60e6..fcf0043b5d3b 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -10,7 +10,12 @@ #include <efi_variable.h> #include <u-boot/crc.h>
-struct efi_var_file __efi_runtime_data *efi_var_buf; +/*
- keep efi_var_buf as static , moving it out might move it to .got
- which is not mapped in virtual address for Linux. Whenever
- we try to invoke get_variable service, it will panic.
Not everybody will know the abbreviation .got. How about:
"The variables efi_var_file and efi_var_entry must be static to avoid that they are referenced via the global offset table (section .got). The GOT is neither mapped as EfiRuntimeServicesData nor do we support its relocation during SetVirtualAddressMap()."
Sure
Otherwise
I'll wait for Atish to verify it fixes RISC-V, because it makes no difference whatsoever in arm and send a v2.
Thanks /Ilias
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
[...]