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.g...@gmx.de> [...]
