Re: [PATCH 2/2] tee: optee: discover services dependent on tee-supplicant

Hi,

On 12/7/22 16:30, Etienne Carriere wrote:

Makes OP-TEE to enumerate also services depending on tee-supplicant support in U-Boot. This change allows OP-TEE services like fTPM TA to be discovered and get a TPM device registered in U-Boot.

Signed-off-by: Etienne Carriere etienne.carriere@linaro.org

drivers/tee/optee/core.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 604fd1414f..b21031d7d8 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -102,13 +102,14 @@ static int bind_service_list(struct udevice *dev, struct tee_shm *service_list, return 0; }

-static int __enum_services(struct udevice *dev, struct tee_shm *shm, size_t *shm_size, u32 tee_sess) +static int __enum_services(struct udevice *dev, struct tee_shm *shm, size_t *shm_size, u32 tee_sess,

• 	   unsigned int pta_cmd)
  

  { struct tee_invoke_arg arg = { }; struct tee_param param = { }; int ret = 0;

• arg.func = PTA_CMD_GET_DEVICES;

• arg.func = pta_cmd; arg.session = tee_sess;

  /* Fill invoke cmd params */

@@ -118,7 +119,7 @@ static int __enum_services(struct udevice *dev, struct tee_shm *shm, size_t *shm

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);
  

• dev_err(dev, "Enumeration command 0x%x failed: 0x%x\n", pta_cmd, arg.ret);
  

  return -EINVAL; }

@@ -127,12 +128,13 @@ static int __enum_services(struct udevice *dev, struct tee_shm *shm, size_t *shm return 0; }

-static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t *count, u32 tee_sess) +static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t *count, u32 tee_sess,

• 	 unsigned int pta_cmd)
  

  { size_t shm_size = 0; int ret;

• ret = __enum_services(dev, NULL, &shm_size, tee_sess);

• ret = __enum_services(dev, NULL, &shm_size, tee_sess, pta_cmd); if (ret) return ret;

@@ -142,7 +144,7 @@ static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t *coun return ret; }

• ret = __enum_services(dev, *shm, &shm_size, tee_sess);

• ret = __enum_services(dev, *shm, &shm_size, tee_sess, pta_cmd); if (!ret) *count = shm_size / sizeof(struct tee_optee_ta_uuid);

@@ -174,20 +176,32 @@ static int bind_service_drivers(struct udevice *dev) struct tee_shm *service_list = NULL; size_t service_count; u32 tee_sess;

• int ret;

• int ret, ret2;

  ret = open_enum_session(dev, &tee_sess); if (ret) return ret;

• ret = enum_services(dev, &service_list, &service_count, tee_sess);

• ret = enum_services(dev, &service_list, &service_count, tee_sess,

• 	    PTA_CMD_GET_DEVICES);
  

  if (!ret) ret = bind_service_list(dev, service_list, service_count);

  tee_shm_free(service_list);

can you move part of duplicate code in enum_services() function ? => variables service_list and service_count => call to bind_service_list() => call to tee_shm_free(service_list);

then the call is more simple :

static int bind_service_drivers(struct udevice *dev) u32 tee_sess; int ret, ret2;

ret = open_enum_session(dev, &tee_sess); if (ret) return ret;

ret = enum_services(dev, tee_sess, PTA_CMD_GET_DEVICES); ret2 = enum_services(dev, tee_sess, PTA_CMD_GET_DEVICES_SUPP);

tee_close_session(dev, tee_sess);

if (ret) return ret;

return ret2;

}

• ret2 = enum_services(dev, &service_list, &service_count, tee_sess,

• 	     PTA_CMD_GET_DEVICES_SUPP);
  

• if (!ret2)

• ret2 = bind_service_list(dev, service_list, service_count);
  

• tee_shm_free(service_list);
• tee_close_session(dev, tee_sess);

• return ret;

• if (ret)

• return ret;
  

• return ret2; }

  /**

with or without

Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com

Thanks Patrick