Re: [PATCH 1/4] tee: optee: Add support for enumerating services that only need RPMB
"Kathpalia, Tanmay" <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <[email protected]> |
On 8/24/2026 12:02 AM, Jan Kiszka wrote: > On 23.08.26 16:31, Kathpalia, Tanmay wrote: >> Hi Jan, >> >> I am not an expert in TEE, but I went through this patch while reviewing >> the MMC side of the series and found a few issues. >> >> On 21-08-2026 14:58, Jan Kiszka wrote: >>> From: Jan Kiszka<[email protected]> >>> >>> Up to OP-TEE 4.4.0, all services that needed a supplicant where returned >>> by PTA_CMD_GET_DEVICES_SUPP. Since then, services that only need a >>> supplicant for the purpose of accessing the RPMB are only enumerated by >>> the new, separate PTA_CMD_GET_DEVICES_RPMB. U-Boot so far lacks support >>> for that, thus no longer finds such services, e.g. fTPM. >>> >>> Perform the separate enumeration during probe but, as that may fail if >>> the MMC is not probed yet, also provide a callback to trigger a retry >>> when another MMC device becomes available. >>> >>> Signed-off-by: Jan Kiszka<[email protected]> >>> --- >>> drivers/tee/optee/core.c | 32 +++++++++++++++++++++++++++++++ >>> drivers/tee/optee/optee_private.h | 2 ++ >>> drivers/tee/optee/rpmb.c | 11 +++++++++++ >>> include/tee/optee.h | 2 ++ >>> 4 files changed, 47 insertions(+) >>> >>> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c >>> index 4d67c948ec1..5600d5a4c7d 100644 >>> --- a/drivers/tee/optee/core.c >>> +++ b/drivers/tee/optee/core.c >>> @@ -41,6 +41,13 @@ >>> */ >>> #define PTA_CMD_GET_DEVICES_SUPP 0x1 >>> +/* >>> + * PTA_CMD_GET_DEVICES_RPMB - List services only depending on RPMB >>> support >>> + * >>> + * [out] memref[0]: List of the UUIDs of service enumerated by OP-TEE >>> + */ >>> +#define PTA_CMD_GET_DEVICES_RPMB 0x2 >>> + >>> typedef void (optee_invoke_fn)(unsigned long, unsigned long, >>> unsigned long, >>> unsigned long, unsigned long, unsigned long, >>> unsigned long, unsigned long, >>> @@ -210,6 +217,29 @@ static int bind_service_drivers(struct udevice *dev) >>> return ret2; >>> } >>> +#ifdef CONFIG_SUPPORT_EMMC_RPMB >>> +void optee_bind_service_drivers_rpmb(struct udevice *dev) >> This function is only compiled when SUPPORT_EMMC_RPMB is set, but >> optee_probe() calls it unconditionally after a successful >> bind_service_drivers(). optee_private.h stubs other functions for the >> !RPMB case, this new prototype has no such stub, so for >> SUPPORT_EMMC_RPMB=n should fail to link. >> > Yeah, thanks for reminding - I forgot again that the linker won't > resolve that. Will also refactor this to move the rpmb part into the > rpmb module. > >>> +{ >>> + struct tee_shm *service_list = NULL; >>> + size_t service_count; >>> + u32 tee_sess; >>> + int ret; >>> + >>> + ret = open_enum_session(dev, &tee_sess); >>> + if (ret) >>> + return; >>> + >>> + ret = enum_services(dev, &service_list, &service_count, tee_sess, >>> + PTA_CMD_GET_DEVICES_RPMB); >>> + if (!ret && service_count) >>> + ret = bind_service_list(dev, service_list, service_count); >> A dev_dbg print would help here. >> > Of what exactly? > Sorry, that was unclear. I meant the early returns, not the successful bind. open_enum_session() and enum_services() can fail and the function just returns. Patch 3 will retry this from mmc_init(), so a dev_dbg() on those failures would make that retry easier to follow. Thanks, Tanmay