u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
From: "Kathpalia, Tanmay" <tanmay.kathpalia@altera.com>
To: Jan Kiszka <jan.kiszka@siemens.com>,
	U-Boot Mailing List <u-boot@lists.u-boot-project.org>
Cc: Jens Wiklander <jens.wiklander@oss.qualcomm.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Sai Sree Kartheek Adivi <s-adivi@ti.com>,
	Alexander Heinisch <alexander.heinisch@siemens.com>
Subject: Re: [PATCH 1/4] tee: optee: Add support for enumerating services that only need RPMB
Date: Sun, 23 Aug 2026 20:01:42 +0530	[thread overview]
Message-ID: <495e9da4-cbcd-40ad-87aa-b8a156dc9022@altera.com> (raw)
In-Reply-To: <bfd9927ca141783a9f73f82a10eca75699222e1d.1787304541.git.jan.kiszka@siemens.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<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.

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

  reply	other threads:[~2026-08-23 14:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  9:28 [PATCH 0/4] optee: Add support for enumerating RPMB-dependent services Jan Kiszka
2026-08-21  9:28 ` [PATCH 1/4] tee: optee: Add support for enumerating services that only need RPMB Jan Kiszka
2026-08-23 14:31   ` Kathpalia, Tanmay [this message]
2026-08-23 18:32     ` Jan Kiszka
2026-08-23 19:09       ` Kathpalia, Tanmay
2026-08-23 19:37         ` Jan Kiszka
2026-08-21  9:28 ` [PATCH 2/4] tee: optee: Do not warn about TEE_ERROR_STORAGE_NOT_AVAILABLE Jan Kiszka
2026-08-23 14:39   ` Kathpalia, Tanmay
2026-08-21  9:29 ` [PATCH 3/4] mmc: Call back to optee after a successful mmc_init Jan Kiszka
2026-08-23 14:48   ` Kathpalia, Tanmay
2026-08-21  9:29 ` [PATCH 4/4] mmc: Make "MMC Device not found" a debug output Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=495e9da4-cbcd-40ad-87aa-b8a156dc9022@altera.com \
    --to=tanmay.kathpalia@altera.com \
    --cc=alexander.heinisch@siemens.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jan.kiszka@siemens.com \
    --cc=jens.wiklander@oss.qualcomm.com \
    --cc=s-adivi@ti.com \
    --cc=u-boot@lists.u-boot-project.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).