Hi Shantur Please don't send a v2 unless the v1 discussion has settled. It just makes life harder. I'll ignore v2 for now and respond here.
[...] > > > > > > + if (ret) > > > + goto error; > > > + > > > + ret = spi_flash_erase_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET, > > > EFI_VAR_BUF_SIZE); > > > + debug("%s - Erased SPI Flash offset %lx\n", __func__, > > > CONFIG_EFI_VARIABLE_SF_OFFSET); > > > + if (ret) > > > + goto error; > > > + > > > + ret = spi_flash_write_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET, > > > len, buf); > > > + debug("%s - Wrote buffer to SPI Flash : %ld\n", __func__, ret); > > > + > > > + if (ret) > > > + goto error; > > > + > > > + ret = EFI_SUCCESS; > > > +error: > > > + if (ret) > > > + log_err("Failed to persist EFI variables in SF\n"); > > > + free(buf); > > > + return ret; > > > +} > > > + > > > +efi_status_t efi_var_from_sf(void) > > > +{ > > > + struct efi_var_file *buf; > > > + efi_status_t ret; > > > + struct udevice *sfdev; > > > + > > > + buf = calloc(1, EFI_VAR_BUF_SIZE); > > > + if (!buf) { > > > + log_err("Out of memory\n"); > > > + return EFI_OUT_OF_RESOURCES; > > > + } > > > > You don't want normal memory for the variables. Instead, you want EFI > > memory which is marked for EFI runtime services data. U-Boot already > > has code to support Get/GetNextvariableRT, you just need to use > > efi_var_mem_init(). > > If you need hints have a look at lib/efi_loader/efi_variable_tee.c > > which implements variables stored in an RPMB, but still uses the > > memory backend for runtime services. > > > > I believe EFI Runtime stuff is handled by efi_variable.c functions. Yes, they are, but there is a subtle difference here. efi_variable.c and efi_variable_tee.c have their own version of runtime service implementation. The functions that can be shared across the two are in efi_var_common.c. What your patch does is shoehorn an SPI backend storage to the existing implementation for storing variables to a file. Depending on what we decide for runtime calls that can end up being messy. efi_variable.c -> deals with variables on a file efi_variable_tee.c -> deals with variables hidden behind the secure world I think it would be cleaner to add efi_variable_sf.c and move the common functions you need from efi_variable.c -> efi_var_common.c Heinrich, do you have a preference for that? Thanks /Ilias > This code is run while we are still in U-boot to initialise the memory backed. > As of now, we don't have the SetVariableRT so I guess this won't be a problem. > I took the code from efi_var_file.c and just replaced file storage > with SPI Flash. > > I will have to look deeper once I get to implement SetVariableRT which will > be a separate patch series once I get this in. > > I have submitted V2 of the patch, can you kindly review them. > > Kind regards, > Shantur