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?
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.
