[PATCH 1/2] tee: optee: don't fail on services enumeration failure

Change optee probe function to only warn when service enumeration sequence fails instead of reporting an optee driver probe failure. Indeed U-Boot can still use OP-TEE even if some OP-TEE services are not discovered.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com --- drivers/tee/optee/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 9a9b697e91..5308dd58ce 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -841,7 +841,7 @@ static int optee_probe(struct udevice *dev) if (IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY)) { ret = bind_service_drivers(dev); if (ret) - return ret; + dev_warn(dev, "optee service enumeration failed: %d\n", ret); } else if (IS_ENABLED(CONFIG_RNG_OPTEE)) { /* * Discovery of TAs on the TEE bus is not supported in U-Boot:

Change optee driver service enumeration to not enumerate (and allocate a zero sized shared memory buffer) when OP-TEE reports that there is no service to enumerate.
This change fixes an existing issue that occurs when the such zero sized shared memory buffer allocated from malloc() has a physical address of offset 0 of a physical 4kB page. In such case, OP-TEE secure world refuses to register the zero-sized shared memory area and makes U-Boot optee service enumeration to fail.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com --- drivers/tee/optee/core.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5308dd58ce..47f845cffe 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -139,6 +139,11 @@ static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t *coun if (ret) return ret;
+ if (!shm_size) { + *count = 0; + return 0; + } + ret = tee_shm_alloc(dev, shm_size, 0, shm); if (ret) { dev_err(dev, "Failed to allocated shared memory: %d\n", ret); @@ -185,14 +190,15 @@ static int bind_service_drivers(struct udevice *dev)
ret = enum_services(dev, &service_list, &service_count, tee_sess, PTA_CMD_GET_DEVICES); - if (!ret) + if (!ret && service_count) ret = bind_service_list(dev, service_list, service_count);
tee_shm_free(service_list); + service_list = NULL;
ret2 = enum_services(dev, &service_list, &service_count, tee_sess, PTA_CMD_GET_DEVICES_SUPP); - if (!ret2) + if (!ret2 && service_count) ret2 = bind_service_list(dev, service_list, service_count);
tee_shm_free(service_list);

On 11/29/23 13:37, Etienne Carriere wrote:
Change optee driver service enumeration to not enumerate (and allocate a zero sized shared memory buffer) when OP-TEE reports that there is no service to enumerate.
This change fixes an existing issue that occurs when the such zero sized shared memory buffer allocated from malloc() has a physical address of offset 0 of a physical 4kB page. In such case, OP-TEE secure world refuses to register the zero-sized shared memory area and makes U-Boot optee service enumeration to fail.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com
drivers/tee/optee/core.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5308dd58ce..47f845cffe 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -139,6 +139,11 @@ static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t *coun if (ret) return ret;
- if (!shm_size) {
*count = 0;
return 0;
- }
- ret = tee_shm_alloc(dev, shm_size, 0, shm); if (ret) { dev_err(dev, "Failed to allocated shared memory: %d\n", ret);
@@ -185,14 +190,15 @@ static int bind_service_drivers(struct udevice *dev)
ret = enum_services(dev, &service_list, &service_count, tee_sess, PTA_CMD_GET_DEVICES);
- if (!ret)
if (!ret && service_count) ret = bind_service_list(dev, service_list, service_count);
tee_shm_free(service_list);
service_list = NULL;
ret2 = enum_services(dev, &service_list, &service_count, tee_sess, PTA_CMD_GET_DEVICES_SUPP);
- if (!ret2)
if (!ret2 && service_count) ret2 = bind_service_list(dev, service_list, service_count);
tee_shm_free(service_list);
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice

On Wed, Nov 29, 2023 at 1:37 PM Etienne Carriere etienne.carriere@foss.st.com wrote:
Change optee driver service enumeration to not enumerate (and allocate a zero sized shared memory buffer) when OP-TEE reports that there is no service to enumerate.
This change fixes an existing issue that occurs when the such zero sized shared memory buffer allocated from malloc() has a physical address of offset 0 of a physical 4kB page. In such case, OP-TEE secure world refuses to register the zero-sized shared memory area and makes U-Boot optee service enumeration to fail.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com
drivers/tee/optee/core.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Thanks, Jens
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5308dd58ce..47f845cffe 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -139,6 +139,11 @@ static int enum_services(struct udevice *dev, struct tee_shm **shm, size_t *coun if (ret) return ret;
if (!shm_size) {
*count = 0;
return 0;
}
ret = tee_shm_alloc(dev, shm_size, 0, shm); if (ret) { dev_err(dev, "Failed to allocated shared memory: %d\n", ret);
@@ -185,14 +190,15 @@ static int bind_service_drivers(struct udevice *dev)
ret = enum_services(dev, &service_list, &service_count, tee_sess, PTA_CMD_GET_DEVICES);
if (!ret)
if (!ret && service_count) ret = bind_service_list(dev, service_list, service_count); tee_shm_free(service_list);
service_list = NULL; ret2 = enum_services(dev, &service_list, &service_count, tee_sess, PTA_CMD_GET_DEVICES_SUPP);
if (!ret2)
if (!ret2 && service_count) ret2 = bind_service_list(dev, service_list, service_count); tee_shm_free(service_list);
-- 2.25.1

On Wed, Nov 29, 2023 at 01:37:53PM +0100, Etienne Carriere wrote:
Change optee driver service enumeration to not enumerate (and allocate a zero sized shared memory buffer) when OP-TEE reports that there is no service to enumerate.
This change fixes an existing issue that occurs when the such zero sized shared memory buffer allocated from malloc() has a physical address of offset 0 of a physical 4kB page. In such case, OP-TEE secure world refuses to register the zero-sized shared memory area and makes U-Boot optee service enumeration to fail.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Applied to u-boot/next, thanks!

On 11/29/23 13:37, Etienne Carriere wrote:
Change optee probe function to only warn when service enumeration sequence fails instead of reporting an optee driver probe failure. Indeed U-Boot can still use OP-TEE even if some OP-TEE services are not discovered.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com
drivers/tee/optee/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 9a9b697e91..5308dd58ce 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -841,7 +841,7 @@ static int optee_probe(struct udevice *dev) if (IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY)) { ret = bind_service_drivers(dev); if (ret)
return ret;
} else if (IS_ENABLED(CONFIG_RNG_OPTEE)) { /*dev_warn(dev, "optee service enumeration failed: %d\n", ret);
- Discovery of TAs on the TEE bus is not supported in U-Boot:
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice

On Wed, Nov 29, 2023 at 1:37 PM Etienne Carriere etienne.carriere@foss.st.com wrote:
Change optee probe function to only warn when service enumeration sequence fails instead of reporting an optee driver probe failure. Indeed U-Boot can still use OP-TEE even if some OP-TEE services are not discovered.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com
drivers/tee/optee/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 9a9b697e91..5308dd58ce 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -841,7 +841,7 @@ static int optee_probe(struct udevice *dev) if (IS_ENABLED(CONFIG_OPTEE_SERVICE_DISCOVERY)) { ret = bind_service_drivers(dev); if (ret)
return ret;
dev_warn(dev, "optee service enumeration failed: %d\n", ret); } else if (IS_ENABLED(CONFIG_RNG_OPTEE)) { /* * Discovery of TAs on the TEE bus is not supported in U-Boot:
-- 2.25.1
Looks good. Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Thanks, Jens

On Wed, Nov 29, 2023 at 01:37:52PM +0100, Etienne Carriere wrote:
Change optee probe function to only warn when service enumeration sequence fails instead of reporting an optee driver probe failure. Indeed U-Boot can still use OP-TEE even if some OP-TEE services are not discovered.
Fixes: 94ccfb78a4d6 ("drivers: tee: optee: discover OP-TEE services") Signed-off-by: Etienne Carriere etienne.carriere@foss.st.com Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Applied to u-boot/next, thanks!
participants (4)
-
Etienne Carriere
-
Jens Wiklander
-
Patrice CHOTARD
-
Tom Rini