On Mon, 5 Oct 2026 at 16:06, Harsimran Singh Tungal <[email protected]> wrote: > > > > On 18/09/2026 19:20, Ilias Apalodimas wrote: > > Hi Harsimran > > > > > > [...] > > > >> + efi_uintn_t > >> name_size, > >> + const efi_guid_t > >> *vendor, > >> + struct > >> var_check_property *var_property) > >> +{ > >> + struct smm_variable_var_check_property *smm_property; > >> + efi_uintn_t payload_size; > >> + u8 *comm_buf = NULL; > >> + efi_status_t ret; > >> + > >> + payload_size = sizeof(*smm_property) + name_size; > >> + if (payload_size > max_payload_size) { > >> + ret = EFI_INVALID_PARAMETER; > >> + return ret; > >> + } > >> + comm_buf = setup_mm_hdr((void **)&smm_property, payload_size, > >> + > >> SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_SET, > >> + &ret); > >> + if (!comm_buf) > >> + return ret; > >> + > >> + efi_memcpy_runtime(&smm_property->guid, vendor, sizeof(*vendor)); > >> + smm_property->name_size = name_size; > >> + efi_memcpy_runtime(&smm_property->property, var_property, > >> + sizeof(smm_property->property)); > >> + efi_memcpy_runtime(smm_property->name, variable_name, name_size); > >> + > >> + ret = mm_communicate(comm_buf, payload_size); > >> + > >> + return ret; > >> +} > >> + > >> static efi_status_t get_property_int(const u16 *variable_name, > >> efi_uintn_t name_size, > >> const efi_guid_t *vendor, > >> @@ -768,6 +800,49 @@ out: > >> return ret; > >> } > >> > >> +static efi_status_t __efi_runtime get_property_int_runtime(const u16 > >> *variable_name, > >> + efi_uintn_t > >> name_size, > >> + const efi_guid_t > >> *vendor, > >> + struct > >> var_check_property *var_property) > >> +{ > > > > The changes between this and the non-runtime version are limited to a few > > memcpy/memset calls. > > It's pretty easy to only keep the runtime version and get rid of all the > > duplication. Same > > goes for the set_property_int() > > > >> + struct smm_variable_var_check_property *smm_property; > >> + efi_uintn_t payload_size; > >> + u8 *comm_buf = NULL; > >> + efi_status_t ret; > >> + > >> + efi_memset_runtime(var_property, 0, sizeof(*var_property)); > >> + payload_size = sizeof(*smm_property) + name_size; > >> + if (payload_size > max_payload_size) { > >> + ret = EFI_INVALID_PARAMETER; > >> + return ret; > >> + } > >> + comm_buf = setup_mm_hdr((void **)&smm_property, payload_size, > >> + > >> SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET, > >> + &ret); > >> + if (!comm_buf) > >> + return ret; > >> + > >> + efi_memcpy_runtime(&smm_property->guid, vendor, > >> sizeof(smm_property->guid)); > >> + smm_property->name_size = name_size; > >> + efi_memcpy_runtime(smm_property->name, variable_name, name_size); > >> + > >> + 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) > >> + return EFI_SUCCESS; > >> + if (ret != EFI_SUCCESS) > >> + return ret; > >> + efi_memcpy_runtime(var_property, &smm_property->property, > >> sizeof(*var_property)); > >> + > >> + return EFI_SUCCESS; > >> +} > >> + > >> efi_status_t efi_get_variable_int(const u16 *variable_name, > >> const efi_guid_t *vendor, > >> u32 *attributes, efi_uintn_t *data_size, > >> @@ -1001,7 +1076,7 @@ efi_status_t efi_set_variable_int(const u16 > >> *variable_name, > >> var_property.property |= > >> VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY; > >> var_property.attributes = attributes; > >> var_property.minsize = 1; > >> - var_property.maxsize = var_acc->data_size; > >> + var_property.maxsize = data_size; > >> ret = set_property_int(variable_name, name_size, vendor, > >> &var_property); > >> } > >> > >> @@ -1078,7 +1153,7 @@ efi_query_variable_info_runtime(u32 attributes, u64 > >> *max_variable_storage_size, > >> } > >> > >> /** > >> - * efi_set_variable_runtime() - runtime implementation of SetVariable() > >> + * efi_set_variable_int_runtime() - runtime implementation of > >> SetVariable() > >> * > >> * @variable_name: name of the variable > >> * @guid: vendor GUID > >> @@ -1088,11 +1163,69 @@ efi_query_variable_info_runtime(u32 attributes, > >> u64 *max_variable_storage_size, > >> * Return: status code > >> */ > >> static efi_status_t __efi_runtime EFIAPI > >> -efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *guid, > >> - u32 attributes, efi_uintn_t data_size, > >> - const void *data) > >> +efi_set_variable_int_runtime(u16 *variable_name, const efi_guid_t *guid, > >> + u32 attributes, efi_uintn_t data_size, > >> + const void *data) > >> { > >> - return EFI_UNSUPPORTED; > >> + efi_status_t ret, mm_communicate_ret = EFI_SUCCESS; > >> + struct var_check_property var_property; > >> + struct smm_variable_access *var_acc; > >> + efi_uintn_t payload_size; > >> + efi_uintn_t name_size; > >> + u8 *comm_buf = NULL; > >> + bool ro; > >> + > >> + if (!variable_name || variable_name[0] == 0 || !guid) > >> + return EFI_INVALID_PARAMETER; > >> + > >> + if (data_size > 0 && !data) > >> + return EFI_INVALID_PARAMETER; > >> + > >> + /* Check payload size */ > >> + name_size = u16_strsize(variable_name); > >> + payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size; > >> + if (payload_size > max_payload_size) > >> + return EFI_INVALID_PARAMETER; > >> + > >> + ro = !!(attributes & EFI_VARIABLE_READ_ONLY); > >> + attributes &= EFI_VARIABLE_MASK; > >> + > >> + ret = get_property_int_runtime(variable_name, name_size, guid, > >> + &var_property); > >> + if (ret != EFI_SUCCESS) > >> + return ret; > >> + > >> + if (var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY) > >> + return EFI_WRITE_PROTECTED; > >> + > >> + comm_buf = setup_mm_hdr((void **)&var_acc, payload_size, > >> + SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret); > >> + if (!comm_buf) > >> + return ret; > >> + > >> + /* Fill in contents */ > >> + efi_memcpy_runtime(&var_acc->guid, guid, sizeof(*guid)); > >> + var_acc->data_size = data_size; > >> + var_acc->name_size = name_size; > >> + var_acc->attr = attributes; > >> + efi_memcpy_runtime(var_acc->name, variable_name, name_size); > >> + efi_memcpy_runtime((u8 *)var_acc->name + name_size, data, data_size); > >> + > >> + /* Communicate */ > >> + ret = mm_communicate(comm_buf, payload_size); > >> + if (ret != EFI_SUCCESS) > >> + mm_communicate_ret = ret; > >> + > >> + 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 = data_size; > >> + ret = set_property_int_runtime(variable_name, name_size, > >> guid, &var_property); > >> + } > >> + > >> + return (mm_communicate_ret == EFI_SUCCESS) ? ret : mm_communicate_ret; > >> } > >> > >> /** > >> @@ -1129,7 +1262,7 @@ void efi_variables_boot_exit_notify(void) > >> efi_runtime_services.get_variable = efi_get_variable_runtime; > >> efi_runtime_services.get_next_variable_name = > >> efi_get_next_variable_name_runtime; > > > > > > All the get_variable*() calls used to point to the function from > > efi_var_common.c. > > By changing this you are breaking the runtime case for OP-TEE since now we > > will get > > EFI_OUT_OF_RESOURCES. > > There's no easy way to fix this at runtime. For OP-TEE the kernel might > > replace this call > > with an internal OP-TEE call if it's part of the firmware and announces it > > can provide > > EFI runtime services. However, the default uuse case, which we don't > > support SetVariable > > at runtime only needs to keep working. > > > > I'll post a script next week so you can replicate and test this in QEMU > > with OP-TEE etc. > > > > Cheers > > /Ilias > > > Hi Ilias, > Can you please share the script, so that I can replicate and test this > in QEMU with OP-TEE?
https://github.com/apalos/efi_optee_variables make run-fip should give you a QEMU command line. You just have to add an OS image to test. Cheers /Ilias > > Thanks > Harsimran Singh Tungal > >> - efi_runtime_services.set_variable = efi_set_variable_runtime; > >> + efi_runtime_services.set_variable = efi_set_variable_int_runtime; > >> efi_update_table_header_crc32(&efi_runtime_services.hdr); > >> > >> /* Record that ExitBootServices() has been called */ > > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you.
