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

On 23.08.26 21:09, Kathpalia, Tanmay wrote:
> 
> 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.
> 

That's what patch 2 is for. Other enumeration errors are printed
unconditionally by __enum_services.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center