On 2026-05-15 12:26 -0600, Simon Glass wrote: > Hi Harsimran, > > On 2026-05-14T12:49:13, Harsimran Singh Tungal > <[email protected]> wrote: > > efi_loader: enable EFI runtime SetVariable()/GetVariable() using FF-A > > transport > > > > Route EFI runtime variable APIs through FF-A MM communication > > > > Route EFI runtime variable services through the FF-A/MM backend in > > lib/efi_loader/efi_variable_tee.c. After ExitBootServices(), > > GetVariable(), SetVariable(), GetNextVariableName(), and > > QueryVariableInfo() use the runtime entry points and continue to reach > > the MM secure partition. > > > > Keep the existing boot-time helpers unchanged and add runtime service > > wrappers for variable access and property handling. Reuse the > > runtime-safe setup_mm_hdr() and common mm_communicate() path, which > > selects the FF-A transport appropriate for the current phase, and use > > the EFI runtime-safe memory helpers in the runtime-only code. > > > > Signed-off-by: Harsimran Singh Tungal <[email protected]> > > > > lib/efi_loader/efi_variable_tee.c | 320 > > +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 315 insertions(+), 5 deletions(-) > > Reviewed-by: Simon Glass <[email protected]> > > > diff --git a/lib/efi_loader/efi_variable_tee.c > > b/lib/efi_loader/efi_variable_tee.c > > @@ -830,6 +905,95 @@ out: > > +efi_status_t __efi_runtime efi_get_variable_runtime(u16 *variable_name, > > + const efi_guid_t *vendor, > > + u32 *attributes, > > + efi_uintn_t *data_size, > > + void *data) > > EFIAPI is missing so this will break on x86_64 where there is a > different ABI. Same for efi_get_next_variable_name_runtime() below - > please match efi_set_variable_runtime() / > efi_query_variable_info_runtime() which already carry EFIAPI. > > > diff --git a/lib/efi_loader/efi_variable_tee.c > > b/lib/efi_loader/efi_variable_tee.c > > @@ -743,6 +775,49 @@ out: > > + ret = mm_communicate(comm_buf, payload_size); > > + /* > > + * Currently only R/O property is supported in StMM. > > + * Variables that are not set to R/O will not set the property in StMM > > + * and the call will return EFI_NOT_FOUND. We are setting the > > + * properties to 0x0 so checking against that is enough for the > > + * EFI_NOT_FOUND case. > > + */ > > + if (ret == EFI_NOT_FOUND) > > + ret = EFI_SUCCESS; > > + if (ret != EFI_SUCCESS) > > + return ret; > > + efi_memcpy_runtime(var_property, &smm_property->property, > > sizeof(*var_property)); > > When EFI_NOT_FOUND is mapped to EFI_SUCCESS we still fall through and > memcpy smm_property->property over the zeroed var_property - at > runtime smm_property aliases the FF-A shared buffer, so whatever the > SP left in those bytes ends up in var_property and undoes the memset > above. Either move the memcpy under an explicit found branch, or skip > it on the NOT_FOUND-mapped-to-SUCCESS path. The boot-time copy doesn't > matter because the SP fills the response into the local buffer; here > the shared buffer is the response. > > > diff --git a/lib/efi_loader/efi_variable_tee.c > > b/lib/efi_loader/efi_variable_tee.c > > @@ -1067,7 +1319,65 @@ efi_set_variable_runtime(u16 *variable_name, const > > efi_guid_t *guid, > > + if (ro && !(var_property.property & > > VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)) { > > + var_property.revision = VAR_CHECK_VARIABLE_PROPERTY_REVISION; > > + var_property.property |= > > VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY; > > + var_property.attributes = attributes; > > + var_property.minsize = 1; > > + var_property.maxsize = var_acc->data_size; > > + ret = set_property_int_runtime(variable_name, name_size, > > guid, &var_property); > > + } > > Just to check - at runtime var_acc points into the FF-A shared buffer, > and the preceding mm_communicate() has already let the SP write its > response there, so var_acc->data_size is whatever the SP reported (and > may be 0 on failure). Is that what we want as maxsize, or should this > be the original data_size argument? Please confirm the SP guarantees > data_size on the way out for SET_VARIABLE before relying on it. > > Regards, > Simon >
Hi Simon, Thanks for pointing this out. It make sense to allow the boot-time and runtime TEE SetVariable() paths to use the original data_size argument for var_property.maxsize, instead of relying on var_acc->data_size after the MM exchange. I will update this in v3. Regards, Harsimran Singh Tungal
