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

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

[Etienne Carriere]

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

[PATCH 2/2] tee: optee: don't enumerate services if there ain't any

[Etienne Carriere]

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

Re: [PATCH 2/2] tee: optee: don't enumerate services if there ain't any

[Patrice CHOTARD]

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

Re: [PATCH 2/2] tee: optee: don't enumerate services if there ain't any

[Jens Wiklander]

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
>

Re: [PATCH 2/2] tee: optee: don't enumerate services if there ain't any

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Patrice CHOTARD]

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

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

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

[Jens Wiklander]

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

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]