[PATCH 1/2] drivers: tee: optee: discover OP-TEE services

Patrick DELAUNAY patrick.delaunay at foss.st.com
Thu Jun 2 13:59:48 CEST 2022


Hi,

minbor remarks on "#ifdef" usage.

On 6/1/22 10:27, 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.wiklander at linaro.org>
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere at 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
>   	help
>   	  Enables support for controlling (enabling, provisioning) the
>   	  Secure Channel Protocol 03 operation in the OP-TEE SCP03 TA.
> +
> +config OPTEE_SERVICE_DISCOVERY
> +	bool "OP-TEE service discovery"
> +	default y
> +	help
> +	  This implements automated driver binding of OP-TEE service drivers by
> +	  requesting OP-TEE firmware to enumerate its hosted services.
> +
>   endmenu
>   
>   endif
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index a89d62aaf0..86a32f2a81 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -14,6 +14,7 @@
>   #include <linux/arm-smccc.h>
>   #include <linux/err.h>
>   #include <linux/io.h>
> +#include <tee/optee_service.h>
>   
>   #include "optee_smc.h"
>   #include "optee_msg.h"
> @@ -22,6 +23,25 @@
>   #define PAGELIST_ENTRIES_PER_PAGE \
>   	((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(u64)) - 1)
>   
> +/*
> + * PTA_DEVICE_ENUM interface exposed by OP-TEE to discover enumerated services
> + */
> +#define PTA_DEVICE_ENUM		{ 0x7011a688, 0xddde, 0x4053, \
> +				  { 0xa5, 0xa9, 0x7b, 0x3c, 0x4d, 0xdf, 0x13, 0xb8 } }
> +/*
> + * PTA_CMD_GET_DEVICES - List services without supplicant dependencies
> + *
> + * [out]    memref[0]: List of the UUIDs of service enumerated by OP-TEE
> + */
> +#define PTA_CMD_GET_DEVICES		0x0
> +
> +/*
> + * PTA_CMD_GET_DEVICES_SUPP - List services depending on tee supplicant
> + *
> + * [out]    memref[0]: List of the UUIDs of service enumerated by OP-TEE
> + */
> +#define PTA_CMD_GET_DEVICES_SUPP	0x1
> +
>   typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
>   			       unsigned long, unsigned long, unsigned long,
>   			       unsigned long, unsigned long,
> @@ -42,6 +62,152 @@ struct rpc_param {
>   	u32	a7;
>   };
>   
> +#ifdef CONFIG_OPTEE_SERVICE_DISCOVERY


avoid #ifdef CONFIG whne it is not mandatory

unused function will be dropped by linker

> +static struct optee_service *find_service_driver(const struct tee_optee_ta_uuid *uuid)
> +{
> +	struct optee_service *service;
> +	u8 loc_uuid[TEE_UUID_LEN];
> +	size_t service_cnt, idx;
> +
> +	service_cnt = ll_entry_count(struct optee_service, optee_service);
> +	service = ll_entry_start(struct optee_service, optee_service);
> +
> +	for (idx = 0; idx < service_cnt; idx++, service++) {
> +		tee_optee_ta_uuid_to_octets(loc_uuid, &service->uuid);
> +		if (!memcmp(uuid, loc_uuid, sizeof(uuid)))
> +			return service;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int bind_service_list(struct udevice *dev, struct tee_shm *service_list, size_t count)
> +{
> +	const struct tee_optee_ta_uuid *service_uuid = (const void *)service_list->addr;
> +	struct optee_service *service;
> +	size_t idx;
> +	int ret;
> +
> +	for (idx = 0; idx < count; idx++) {
> +		service = find_service_driver(service_uuid + idx);
> +		if (!service)
> +			continue;
> +
> +		ret = device_bind_driver(dev, service->driver_name, service->driver_name, NULL);
> +		if (ret) {
> +			dev_warn(dev, "%s was not bound: %d, ignored\n", service->driver_name, ret);
> +			continue;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int __enum_services(struct udevice *dev, struct tee_shm *shm, u32 *shm_size, u32 tee_sess)
> +{
> +	struct tee_invoke_arg arg = { };
> +	struct tee_param param = { };
> +	int ret = 0;
> +
> +	arg.func = PTA_CMD_GET_DEVICES;
> +	arg.session = tee_sess;
> +
> +	/* Fill invoke cmd params */
> +	param.attr = TEE_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
> +	param.u.memref.shm = shm;
> +	param.u.memref.size = *shm_size;
> +
> +	ret = tee_invoke_func(dev, &arg, 1, &param);
> +	if (ret || (arg.ret && arg.ret != TEE_ERROR_SHORT_BUFFER)) {
> +		dev_err(dev, "PTA_CMD_GET_DEVICES invoke function err: 0x%x\n", arg.ret);
> +		return -EINVAL;
> +	}
> +
> +	*shm_size = param.u.memref.size;
> +
> +	return 0;
> +}
> +
> +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);
> +	else
> +		*count = shm_size / sizeof(struct tee_optee_ta_uuid);
> +
> +	return ret;
> +}
> +
> +static int open_session(struct udevice *dev, u32 *tee_sess)
> +{
> +	const struct tee_optee_ta_uuid pta_uuid = PTA_DEVICE_ENUM;
> +	struct tee_open_session_arg arg = { };
> +	int ret;
> +
> +	tee_optee_ta_uuid_to_octets(arg.uuid, &pta_uuid);
> +
> +	ret = tee_open_session(dev, &arg, 0, NULL);
> +	if (ret || arg.ret) {
> +		if (!ret)
> +			ret = -EIO;
> +		return ret;
> +	}
> +
> +	*tee_sess = arg.session;
> +
> +	return 0;
> +}
> +
> +static void close_session(struct udevice *dev, u32 tee_sess)
> +{
> +	tee_close_session(dev, tee_sess);
> +}
> +
> +static int bind_service_drivers(struct udevice *dev)
> +{
> +	struct tee_shm *service_list = NULL;
> +	size_t service_count;
> +	u32 tee_sess;
> +	int ret;
> +
> +	ret = open_session(dev, &tee_sess);
> +	if (ret)
> +		return ret;
> +
> +	ret = enum_services(dev, &service_list, &service_count, tee_sess);
> +	if (ret)
> +		goto close_session;
> +
> +	ret = bind_service_list(dev, service_list, service_count);
> +
> +	tee_shm_free(service_list);
> +
> +close_session:
> +	close_session(dev, tee_sess);
> +
> +	return ret;
> +}
> +#else
> +static int bind_service_drivers(struct udevice *dev)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_OPTEE_SERVICE_DISCOVERY */


always define the function with:

+static int bind_service_drivers(struct udevice *dev)
+{
+	struct tee_shm *service_list = NULL;
+	size_t service_count;
+	u32 tee_sess;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY))
+		dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
+
+	ret = open_session(dev, &tee_sess);
+	if (ret)
+		return ret;
+
+	ret = enum_services(dev, &service_list, &service_count, tee_sess);
+	if (ret)
+		goto close_session;
+
+	ret = bind_service_list(dev, service_list, service_count);
+
+	tee_shm_free(service_list);
+
+close_session:
+	close_session(dev, tee_sess);
+
+	return ret;
+}



+static int optee_bind(struct udevice *dev)
+{
+	if (IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY))
+		dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
+
+	return 0;
+}


> +
>   /**
>    * reg_pair_to_ptr() - Make a pointer of 2 32-bit values
>    * @reg0:	High bits of the pointer
> @@ -638,11 +804,19 @@ static int optee_of_to_plat(struct udevice *dev)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_OPTEE_SERVICE_DISCOVERY
> +static int optee_bind(struct udevice *dev)
> +{
> +	dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
> +
> +	return 0;
> +}
> +#endif

always define the function with:

+static int optee_bind(struct udevice *dev)
+{
+	if (IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY))
+		dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
+
+	return 0;
+}


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


cna be handle wihout any #ifdef with:


+ if (IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY)) {
+	ret = bind_service_drivers(dev);
+	if (ret)
+		return ret;
+ } else {
+ 	/*
+ 	 * When 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);
+ 		if (ret)
+ 			return ret;
+ 	}
+ }

>   	/*
>   	 * 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);
>   		if (ret)
>   			return ret;
>   	}
> +#endif
>   
>   	return 0;
>   }
> @@ -692,6 +874,9 @@ U_BOOT_DRIVER(optee) = {
>   	.of_match = optee_match,
>   	.of_to_plat = optee_of_to_plat,
>   	.probe = optee_probe,
> +#ifdef CONFIG_OPTEE_SERVICE_DISCOVERY
> +	.bind = optee_bind,
> +#endif
>   	.ops = &optee_ops,
>   	.plat_auto	= sizeof(struct optee_pdata),
>   	.priv_auto	= sizeof(struct optee_private),
> diff --git a/include/tee/optee_service.h b/include/tee/optee_service.h
> new file mode 100644
> index 0000000000..31732979da
> --- /dev/null
> +++ b/include/tee/optee_service.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * (C) Copyright 2022 Linaro Limited
> + */
> +
> +#ifndef _OPTEE_SERVICE_H
> +#define _OPTEE_SERVICE_H
> +
> +/*
> + * struct optee_service - Discoverable OP-TEE service
> + *
> + * @driver_name - Name of the related driver
> + * @uuid - UUID of the OP-TEE service related to the driver
> + *
> + * Use macro OPTEE_SERVICE_DRIVER() to register a driver related to an
> + * OP-TEE service discovered when driver asks OP-TEE services enumaration.
> + */
> +struct optee_service {
> +	const char *driver_name;
> +	const struct tee_optee_ta_uuid uuid;
> +};
> +
> +#define OPTEE_SERVICE_DRIVER(__name) \
> +	ll_entry_declare(struct optee_service, __name, optee_service)
> +
> +#define OPTEE_SERVICE_DRIVER_GET(__name) \
> +	ll_entry_get(struct optee_service, __name, optee_service)
> +
> +#endif /* _OPTEE_SERVICE_H */


Regards

Patrick



More information about the U-Boot mailing list