Hi Etienne,
On Wed, Jun 01, 2022 at 10:27:51AM +0200, Etienne Carriere wrote:
> This change defines resources for OP-TEE service drivers to register
> themselves for being bound to when OP-TEE firmware reports the related
> service is supported. OP-TEE services are discovered during optee
> driver probe sequence. Discovery of optee services and binding to
> related U-Boot drivers is embedded upon configuration switch
> CONFIG_OPTEE_SERVICE_DISCOVERY.
>
> Cc: Jens Wiklander <jens.wiklan...@linaro.org>
> Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carri...@linaro.org>
> ---
> drivers/tee/optee/Kconfig | 8 ++
> drivers/tee/optee/core.c | 187 +++++++++++++++++++++++++++++++++++-
> include/tee/optee_service.h | 29 ++++++
> 3 files changed, 223 insertions(+), 1 deletion(-)
> create mode 100644 include/tee/optee_service.h
>
> diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
> index d03028070b..9dc65b0501 100644
> --- a/drivers/tee/optee/Kconfig
> +++ b/drivers/tee/optee/Kconfig
> @@ -37,6 +37,14 @@ config OPTEE_TA_SCP03
>
[...]
> +static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t
> *count, u32 tee_sess)
> +{
> + size_t shm_size = 0;
> + int ret;
> +
> + ret = __enum_services(dev, NULL, &shm_size, tee_sess);
> + if (ret)
> + return ret;
> +
> + ret = tee_shm_alloc(dev, shm_size, 0, shm);
> + if (ret) {
> + dev_err(dev, "Failed to allocated shared memory: %d\n", ret);
> + return ret;
> + }
> +
> + ret = __enum_services(dev, *shm, &shm_size, tee_sess);
> + if (ret)
> + tee_shm_free(*shm);
I'd prefer if we handled this a bit differently. Instead of freeing the
buffer here, just release it on bind_service_drivers() always
> + else
> + *count = shm_size / sizeof(struct tee_optee_ta_uuid);
> +
> + return ret;
> +}
> +
> +
> static int optee_probe(struct udevice *dev)
> {
> struct optee_pdata *pdata = dev_get_plat(dev);
> u32 sec_caps;
> - struct udevice *child;
> int ret;
>
> if (!is_optee_api(pdata->invoke_fn)) {
> @@ -668,15 +842,23 @@ static int optee_probe(struct udevice *dev)
> return -ENOENT;
> }
>
> + ret = bind_service_drivers(dev);
> + if (ret)
> + return ret;
> +
> +#ifndef CONFIG_OPTEE_SERVICE_DISCOVERY
> /*
> * in U-Boot, the discovery of TA on the TEE bus is not supported:
> * only bind the drivers associated to the supported OP-TEE TA
> */
> if (IS_ENABLED(CONFIG_RNG_OPTEE)) {
> + struct udevice *child;
> +
> ret = device_bind_driver(dev, "optee-rng", "optee-rng", &child);
The same principle applies for fTPM. Moreover the linux kernel supports
bus scanning, which creates a conflict when the fTPM is added on the .dts
(for u-boot to scan it).
Can we make this a bit more generic, even though only the rng is added on
this patch?
something like
struct devices {
const char *drv_name;
const char *dev_name;
} tee_bus_devices = {
{
"optee-rng",
"optee-rng",
},
}
and add an array of the 'scanable' devices? It would make adding the ftpm
and other devices trivial
> if (ret)
> return ret;
> }
> +#endif
[...]
Thanks!
/Ilias
