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 */