Re: [PATCH 1/4] tee: optee: Add support for enumerating services that only need RPMB

[Jan Kiszka <jan.kiszka@siemens.com>]

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<jan.kiszka@siemens.com>
>>
>> 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<jan.kiszka@siemens.com>
>> ---
>>   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?

Thanks for the feedback,
Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center