On 05/10/2026 14:15, Ilias Apalodimas wrote:
> 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
>>
Hi Ilias,
Thank you for sharing the repository and instructions.
I tried the OP-TEE/non-FF-A QEMU setup from your repository and it
exposed a regression in my current series.
With CONFIG_EFI_MM_COMM_TEE=y and CONFIG_ARM_FFA_RT_MODE=n, Linux hit
an EFI runtime paging fault when booted through U-Boot EFI. The reason
was that TEE ExitBootServices() path only installed runtime variable
callbacks inside the CONFIG_ARM_FFA_RT_MODE path. That kept the FF-A
case working, but it accidentally stopped the non-FF-A OP-TEE case from
switching to the existing cached-variable runtime callbacks.
I tested a fix by keeping the FF-A/MM runtime callbacks only for
CONFIG_ARM_FFA_RT_MODE=y, and using the common cached-variable runtime
callbacks when CONFIG_ARM_FFA_RT_MODE=n. To make that clean, I plan to
add a small separate commit which moves the common runtime
QueryVariableInfo() and SetVariable() helpers, plus their shared
SetVariable() validation helper, from efi_variable.c into
efi_var_common.c. Then, efi_variable_tee.c can select either the common
cached-variable runtime handlers for OP-TEE/non-FF-A, or the new MM/FF-A
runtime handlers for FF-A runtime mode.
With that change, the same QEMU setup boots Linux through bootefi and a
small initramfs, /sys/firmware/efi is present, and efivarfs mounts
successfully.
Does this direction look reasonable to you?
Regards,
Harsimran Singh Tungal
>> 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.
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.
