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

"Kathpalia, Tanmay" <[email protected]>
Newsgroups gmane.comp.boot-loaders.u-boot
Message-ID <495e9da4-cbcd-40ad-87aa-b8a156dc9022__16824.2098692403$1787495535$gmane$org@altera.com>
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.

> +{
> +	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.

> +
> +	tee_shm_free(service_list);
> +
> +	tee_close_session(dev, tee_sess);
> +}
> +#endif
> +
>   /**
>    * reg_pair_to_ptr() - Make a pointer of 2 32-bit values
>    * @reg0:	High bits of the pointer
> @@ -852,6 +882,8 @@ static int optee_probe(struct udevice *dev)
>   		ret = bind_service_drivers(dev);
>   		if (ret)
>   			dev_warn(dev, "optee service enumeration failed: %d\n", ret);
> +		else
> +			optee_bind_service_drivers_rpmb(dev);
>   	} else if (IS_ENABLED(CONFIG_RNG_OPTEE)) {
>   		/*
>   		 * Discovery of TAs on the TEE bus is not supported in U-Boot:
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 1f07a27ee4b..b5a58df5b96 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -79,4 +79,6 @@ static inline void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
>   
>   void *optee_alloc_and_init_page_list(void *buf, ulong len, u64 *phys_buf_ptr);
>   
> +void optee_bind_service_drivers_rpmb(struct udevice *dev);
> +

Please wrap this like the other RPMB helpers in this header (#ifdef
CONFIG_SUPPORT_EMMC_RPMB plus an empty inline stub), rather than a bare 
prototype.

>   #endif /* __OPTEE_PRIVATE_H */
> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c
> index bacced6af6c..f4a66641755 100644
> --- a/drivers/tee/optee/rpmb.c
> +++ b/drivers/tee/optee/rpmb.c
> @@ -191,3 +191,14 @@ void optee_suppl_rpmb_release(struct udevice *dev)
>   {
>   	release_mmc(dev_get_priv(dev));
>   }
> +
> +void optee_rpmb_available(void)
> +{
> +	struct udevice *dev;
> +	struct uclass *uc;
> +
> +	uclass_id_foreach_dev(UCLASS_TEE, dev, uc) {
> +		if (strcmp(dev->driver->name, "optee") == 0)
> +			optee_bind_service_drivers_rpmb(dev);
> +	}
> +}
> diff --git a/include/tee/optee.h b/include/tee/optee.h
> index d1194493780..c12fda3cc73 100644
> --- a/include/tee/optee.h
> +++ b/include/tee/optee.h
> @@ -74,4 +74,6 @@ static inline bool is_optee_smc_api(void)
>   }
>   #endif
>   
> +void optee_rpmb_available(void);
> +

When OPTEE=y and SUPPORT_EMMC_RPMB=n, where the call is made to
optee_rpmb_avaliable but the symbol only lives in rpmb.o. Either stub when
RPMB is off, or you can gate the MMC call on SUPPORT_EMMC_RPMB.

>   #endif /* _OPTEE_H */
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.