On 8/27/26 10:58, Ilias Apalodimas wrote: > I am trying to compile this and test it against the working non -FFA > case, However, I am getting a compilation error with the defconfig > below (which is basically the qemu defconfig + COONFIG_OPTEE and > CONFIG_EFI_MM_COMM_TEE) > > CONFIG_ARM=y > CONFIG_KVM_VIRT_INS=y > CONFIG_ARCH_QEMU=y > CONFIG_SYS_MALLOC_LEN=0x1000000 > CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x40200000 > CONFIG_ENV_SIZE=0x40000 > CONFIG_ENV_SECT_SIZE=0x40000 > CONFIG_DEFAULT_DEVICE_TREE="qemu-arm64" > CONFIG_OF_LIBFDT_OVERLAY=y > CONFIG_SYS_LOAD_ADDR=0x40200000 > CONFIG_DEBUG_UART_BASE=0x9000000 > CONFIG_DEBUG_UART_CLOCK=0 > CONFIG_ARMV8_CRYPTO=y > CONFIG_ENV_ADDR=0x4000000 > CONFIG_PCI=y > CONFIG_DEBUG_UART=y > CONFIG_RELOC_ADDR_TOP=y > CONFIG_EFI_MM_COMM_TEE=y > CONFIG_EFI_HTTP_BOOT=y > CONFIG_FIT=y > CONFIG_FIT_SIGNATURE=y > CONFIG_FIT_VERBOSE=y > CONFIG_FIT_BEST_MATCH=y > CONFIG_BOOTSTD_FULL=y > CONFIG_LEGACY_IMAGE_FORMAT=y > CONFIG_USE_PREBOOT=y > # CONFIG_DISPLAY_CPUINFO is not set > # CONFIG_DISPLAY_BOARDINFO is not set > # CONFIG_BOARD_INIT is not set > CONFIG_PCI_INIT_R=y > CONFIG_BLOBLIST=y > CONFIG_CMD_SMBIOS=y > CONFIG_CMD_BOOTZ=y > CONFIG_CMD_BOOTEFI_SELFTEST=y > CONFIG_CMD_NVEDIT_EFI=y > CONFIG_CMD_DFU=y > CONFIG_CMD_MTD=y > CONFIG_CMD_PCI=y > CONFIG_CMD_NFS=y > CONFIG_CMD_SNTP=y > CONFIG_CMD_TFTPSRV=y > CONFIG_WGET_HTTPS=y > CONFIG_WGET_CACERT=y > CONFIG_CMD_EFIDEBUG=y > CONFIG_CMD_TPM=y > CONFIG_CMD_MTDPARTS=y > CONFIG_CMD_SPAWN=y > CONFIG_ENV_IS_IN_FLASH=y > CONFIG_NET_LWIP=y > CONFIG_AHCI=y > CONFIG_SCSI_AHCI=y > CONFIG_AHCI_PCI=y > CONFIG_DFU_MTD=y > CONFIG_DFU_RAM=y > # CONFIG_MMC is not set > CONFIG_MTD=y > CONFIG_DM_MTD=y > CONFIG_MTD_NOR_FLASH=y > CONFIG_FLASH_SHOW_PROGRESS=0 > CONFIG_CFI_FLASH=y > CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS=y > CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y > CONFIG_FLASH_CFI_MTD=y > CONFIG_SYS_FLASH_CFI=y > CONFIG_SYS_MAX_FLASH_SECT=256 > CONFIG_SYS_MAX_FLASH_BANKS=2 > CONFIG_SYS_MAX_FLASH_BANKS_DETECT=y > CONFIG_E1000=y > CONFIG_NVME_PCI=y > CONFIG_PCIE_ECAM_GENERIC=y > CONFIG_SCSI=y > CONFIG_DEBUG_UART_PL011=y > CONFIG_DEBUG_UART_SHIFT=2 > CONFIG_SYSINFO=y > CONFIG_SYSINFO_SMBIOS=y > CONFIG_SYSRESET=y > CONFIG_SYSRESET_CMD_POWEROFF=y > CONFIG_SYSRESET_PSCI=y > CONFIG_TEE=y > CONFIG_OPTEE=y > CONFIG_TPM2_MMIO=y > CONFIG_USB_EHCI_HCD=y > CONFIG_USB_EHCI_PCI=y > CONFIG_MBEDTLS_LIB=y > CONFIG_TPM=y > CONFIG_TPM_PCR_ALLOCATE=y > CONFIG_GENERATE_SMBIOS_TABLE_VERBOSE=y > CONFIG_UTHREAD=y > CONFIG_UNIT_TEST=y > > lib/efi_loader/efi_variable_tee.c: In function ‘get_comm_buf’: > lib/efi_loader/efi_variable_tee.c:562:45: error: > ‘CONFIG_FFA_SHARED_MM_BUF_SIZE’ undeclared (first use in this > function); did you mean ‘CONFIG_EFI_VAR_BUF_SIZE’? > 562 | if (comm_buf_size > > CONFIG_FFA_SHARED_MM_BUF_SIZE) > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | CONFIG_EFI_VAR_BUF_SIZE > lib/efi_loader/efi_variable_tee.c:562:45: note: each undeclared > identifier is reported only once for each function it appears in > In file included from include/stdio.h:5, > from include/log.h:12, > from include/linux/printk.h:4, > from include/linux/kernel.h:5, > from include/linux/libfdt_env.h:13, > from include/linux/libfdt.h:6, > from include/fdtdec.h:17, > from include/dm/ofnode.h:11, > from include/dm/device.h:13, > from include/dm.h:13, > from lib/efi_loader/efi_variable_tee.c:20: > lib/efi_loader/efi_variable_tee.c: In function ‘efi_init_variables’: > lib/efi_loader/efi_variable_tee.c:1488:30: error: > ‘CONFIG_FFA_SHARED_MM_BUF_ADDR’ undeclared (first use in this > function) > 1488 | BUILD_BUG_ON(CONFIG_FFA_SHARED_MM_BUF_ADDR & > EFI_PAGE_MASK); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > include/linux/compiler.h:326:23: note: in definition of macro > ‘__compiletime_assert’ > 326 | if (!(condition)) > \ > | ^~~~~~~~~ > include/linux/compiler.h:346:9: note: in expansion of macro > ‘_compiletime_assert’ > 346 | _compiletime_assert(condition, msg, > __compiletime_assert_, __COUNTER__) > | ^~~~~~~~~~~~~~~~~~~ > include/linux/build_bug.h:38:37: note: in expansion of macro > ‘compiletime_assert’ > 38 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) > | ^~~~~~~~~~~~~~~~~~ > include/linux/build_bug.h:49:9: note: in expansion of macro ‘BUILD_BUG_ON_MSG’ > 49 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " > #condition) > | ^~~~~~~~~~~~~~~~ > lib/efi_loader/efi_variable_tee.c:1488:17: note: in expansion of macro > ‘BUILD_BUG_ON’ > 1488 | BUILD_BUG_ON(CONFIG_FFA_SHARED_MM_BUF_ADDR & > EFI_PAGE_MASK); > | ^~~~~~~~~~~~ > lib/efi_loader/efi_variable_tee.c:1497:42: error: > ‘CONFIG_FFA_SHARED_MM_BUF_SIZE’ undeclared (first use in this > function); did you mean ‘CONFIG_EFI_VAR_BUF_SIZE’? > 1497 | > CONFIG_FFA_SHARED_MM_BUF_SIZE, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | CONFIG_EFI_VAR_BUF_SIZE > make[2]: *** [scripts/Makefile.build:272: > lib/efi_loader/efi_variable_tee.o] Error 1 > > Can you reproduce this? > > Thanks > /Ilias >
Thanks Ilias, Yes, I can reproduce this. The issue is that CONFIG_FFA_SHARED_MM_BUF_ADDR/SIZE/OFFSET depend on CONFIG_ARM_FFA_TRANSPORT, but the OP-TEE-only MM_COMM_TEE build enables CONFIG_EFI_MM_COMM_TEE without enabling CONFIG_ARM_FFA_TRANSPORT. So the FF-A shared-buffer symbols are not defined in that configuration, but efi_variable_tee.c still references them inside IS_ENABLED() runtime checks. That is wrong because the compiler still parses those references. I can fix this by wrapping the FF-A shared-buffer code with CONFIG_IS_ENABLED(ARM_FFA_TRANSPORT) preprocessor guards, so the existing OP-TEE-only path builds and runs unchanged, while the FF-A path still uses the shared buffer when ARM_FFA_TRANSPORT/ARM_FFA_RT_MODE are enabled. Will this change acceptable to you? Regards Harsimran Singh Tungal > On Wed, 29 Jul 2026 at 11:52, Harsimran Singh Tungal > <[email protected]> wrote: >> >> Add the TEE-backed runtime SetVariable() implementation for the FF-A/MM >> variable backend. The runtime path uses EFI runtime-safe memory helpers >> and the shared MM communication path so SetVariable() can reach the MM >> secure partition after ExitBootServices(). >> >> Add runtime-safe variable-property helpers for the SetVariable() path >> and use the original data_size argument when restoring the read-only >> property maxsize, instead of relying on the MM response buffer. >> >> Reviewed-by: Simon Glass <[email protected]> >> Signed-off-by: Harsimran Singh Tungal <[email protected]> >> --- >> lib/efi_loader/efi_variable_tee.c | 147 ++++++++++++++++++++++++++++-- >> 1 file changed, 140 insertions(+), 7 deletions(-) >> >> diff --git a/lib/efi_loader/efi_variable_tee.c >> b/lib/efi_loader/efi_variable_tee.c >> index 5c7104662e9..d04b8dc1376 100644 >> --- a/lib/efi_loader/efi_variable_tee.c >> +++ b/lib/efi_loader/efi_variable_tee.c >> @@ -717,6 +717,38 @@ out: >> return ret; >> } >> >> +static efi_status_t __efi_runtime set_property_int_runtime(const u16 >> *variable_name, >> + 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, >> @@ -762,6 +794,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) >> +{ >> + 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, >> @@ -995,7 +1070,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); >> } >> >> @@ -1072,7 +1147,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 >> @@ -1082,11 +1157,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; >> } >> >> /** >> @@ -1123,7 +1256,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; >> - 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 */ >> -- >> 2.34.1 >> 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.
