[PATCH v2] tee: optee: sync cache on pre-reloc OP-TEE invocation

[PATCH v2] tee: optee: sync cache on pre-reloc OP-TEE invocation

[Etienne Carriere]

This change ensures both U-Boot and OP-TEE see the same content
from shared memory when OP-TEE is invoked prior U-Boot relocation.

This change is required since U-Boot may execute with data cahce off
while OP-TEE always enables cache on memory shared with U-Boot.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Fix invocation argument buffer size in optee/core.c, using
  OPTEE_MSG_GET_ARG_SIZE().
- add missing #include <asm/cache.h> in tee-uclass.c
---
 drivers/tee/optee/core.c | 21 ++++++++++++++++++++-
 drivers/tee/tee-uclass.c | 19 ++++++++++++++++++-
 include/tee.h            |  6 ++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 73dbb22ba0..dad46aa388 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -1,9 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright (c) 2018 Linaro Limited
+ * Copyright (c) 2018-2020 Linaro Limited
  */
 
 #include <common.h>
+#include <cpu_func.h>
 #include <dm.h>
 #include <dm/device_compat.h>
 #include <log.h>
@@ -295,6 +296,16 @@ static u32 call_err_to_res(u32 call_err)
 	}
 }
 
+static void flush_shm_dcache(struct udevice *dev, struct optee_msg_arg *arg)
+{
+	size_t sz = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
+
+	flush_dcache_range(rounddown((ulong)arg, CONFIG_SYS_CACHELINE_SIZE),
+			   roundup((ulong)arg + sz, CONFIG_SYS_CACHELINE_SIZE));
+
+	tee_flush_all_shm_dcache(dev);
+}
+
 static u32 do_call_with_arg(struct udevice *dev, struct optee_msg_arg *arg)
 {
 	struct optee_pdata *pdata = dev_get_plat(dev);
@@ -305,9 +316,17 @@ static u32 do_call_with_arg(struct udevice *dev, struct optee_msg_arg *arg)
 	while (true) {
 		struct arm_smccc_res res;
 
+		/* If cache are off from U-Boot, sync the cache shared with OP-TEE */
+		if (!dcache_status())
+			flush_shm_dcache(dev, arg);
+
 		pdata->invoke_fn(param.a0, param.a1, param.a2, param.a3,
 				 param.a4, param.a5, param.a6, param.a7, &res);
 
+		/* If cache are off from U-Boot, sync the cache shared with OP-TEE */
+		if (!dcache_status())
+			flush_shm_dcache(dev, arg);
+
 		free(page_list);
 		page_list = NULL;
 
diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c
index 2cc6b6c407..24321ba1d2 100644
--- a/drivers/tee/tee-uclass.c
+++ b/drivers/tee/tee-uclass.c
@@ -1,13 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright (c) 2018 Linaro Limited
+ * Copyright (c) 2018-2020 Linaro Limited
  */
 
 #include <common.h>
+#include <cpu_func.h>
 #include <dm.h>
 #include <log.h>
 #include <malloc.h>
 #include <tee.h>
+#include <asm/cache.h>
 #include <dm/device-internal.h>
 #include <dm/uclass-internal.h>
 
@@ -233,3 +235,18 @@ void tee_optee_ta_uuid_to_octets(u8 d[TEE_UUID_LEN],
 	d[7] = s->time_hi_and_version;
 	memcpy(d + 8, s->clock_seq_and_node, sizeof(s->clock_seq_and_node));
 }
+
+void tee_flush_all_shm_dcache(struct udevice *dev)
+{
+	struct tee_uclass_priv *priv = dev_get_uclass_priv(dev);
+	struct tee_shm *s;
+
+	list_for_each_entry(s, &priv->list_shm, link) {
+		ulong start = rounddown((ulong)s->addr,
+					CONFIG_SYS_CACHELINE_SIZE);
+		ulong end = roundup((ulong)s->addr + s->size,
+				    CONFIG_SYS_CACHELINE_SIZE);
+
+		flush_dcache_range(start, end);
+	}
+}
diff --git a/include/tee.h b/include/tee.h
index 99367b258e..2ef29bfc8f 100644
--- a/include/tee.h
+++ b/include/tee.h
@@ -377,4 +377,10 @@ void tee_optee_ta_uuid_from_octets(struct tee_optee_ta_uuid *d,
 void tee_optee_ta_uuid_to_octets(u8 d[TEE_UUID_LEN],
 				 const struct tee_optee_ta_uuid *s);
 
+/**
+ * tee_flush_all_shm_dcache() - Flush data cache for all shared memories
+ * @dev:	The TEE device
+ */
+void tee_flush_all_shm_dcache(struct udevice *dev);
+
 #endif /* __TEE_H */
-- 
2.17.1

[PATCH v2] tee: optee: sync cache on pre-reloc OP-TEE invocation

[Jens Wiklander]

On Wed, May 19, 2021 at 4:28 PM Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change ensures both U-Boot and OP-TEE see the same content
> from shared memory when OP-TEE is invoked prior U-Boot relocation.
>
> This change is required since U-Boot may execute with data cahce off

cache

> while OP-TEE always enables cache on memory shared with U-Boot.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v1:
> - Fix invocation argument buffer size in optee/core.c, using
>   OPTEE_MSG_GET_ARG_SIZE().
> - add missing #include <asm/cache.h> in tee-uclass.c
> ---
>  drivers/tee/optee/core.c | 21 ++++++++++++++++++++-
>  drivers/tee/tee-uclass.c | 19 ++++++++++++++++++-
>  include/tee.h            |  6 ++++++
>  3 files changed, 44 insertions(+), 2 deletions(-)

With the spell error in the commit message fixed:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Cheers,
Jens

Re: [PATCH v2] tee: optee: sync cache on pre-reloc OP-TEE invocation

[Ilias Apalodimas]

On Wed, May 19, 2021 at 04:27:41PM +0200, Etienne Carriere wrote:
> This change ensures both U-Boot and OP-TEE see the same content
> from shared memory when OP-TEE is invoked prior U-Boot relocation.
> 
> This change is required since U-Boot may execute with data cahce off
> while OP-TEE always enables cache on memory shared with U-Boot.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v1:
> - Fix invocation argument buffer size in optee/core.c, using
>   OPTEE_MSG_GET_ARG_SIZE().
> - add missing #include <asm/cache.h> in tee-uclass.c
> ---
>  drivers/tee/optee/core.c | 21 ++++++++++++++++++++-
>  drivers/tee/tee-uclass.c | 19 ++++++++++++++++++-
>  include/tee.h            |  6 ++++++
>  3 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 73dbb22ba0..dad46aa388 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -1,9 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Copyright (c) 2018 Linaro Limited
> + * Copyright (c) 2018-2020 Linaro Limited
>   */
>  
>  #include <common.h>
> +#include <cpu_func.h>
>  #include <dm.h>
>  #include <dm/device_compat.h>
>  #include <log.h>
> @@ -295,6 +296,16 @@ static u32 call_err_to_res(u32 call_err)
>  	}
>  }
>  
> +static void flush_shm_dcache(struct udevice *dev, struct optee_msg_arg *arg)
> +{
> +	size_t sz = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
> +
> +	flush_dcache_range(rounddown((ulong)arg, CONFIG_SYS_CACHELINE_SIZE),
> +			   roundup((ulong)arg + sz, CONFIG_SYS_CACHELINE_SIZE));
> +
> +	tee_flush_all_shm_dcache(dev);
> +}
> +
>  static u32 do_call_with_arg(struct udevice *dev, struct optee_msg_arg *arg)
>  {
>  	struct optee_pdata *pdata = dev_get_plat(dev);
> @@ -305,9 +316,17 @@ static u32 do_call_with_arg(struct udevice *dev, struct optee_msg_arg *arg)
>  	while (true) {
>  		struct arm_smccc_res res;
>  
> +		/* If cache are off from U-Boot, sync the cache shared with OP-TEE */
> +		if (!dcache_status())
> +			flush_shm_dcache(dev, arg);
> +
>  		pdata->invoke_fn(param.a0, param.a1, param.a2, param.a3,
>  				 param.a4, param.a5, param.a6, param.a7, &res);
>  
> +		/* If cache are off from U-Boot, sync the cache shared with OP-TEE */
> +		if (!dcache_status())
> +			flush_shm_dcache(dev, arg);
> +
>  		free(page_list);
>  		page_list = NULL;
>  
> diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c
> index 2cc6b6c407..24321ba1d2 100644
> --- a/drivers/tee/tee-uclass.c
> +++ b/drivers/tee/tee-uclass.c
> @@ -1,13 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Copyright (c) 2018 Linaro Limited
> + * Copyright (c) 2018-2020 Linaro Limited
>   */
>  
>  #include <common.h>
> +#include <cpu_func.h>
>  #include <dm.h>
>  #include <log.h>
>  #include <malloc.h>
>  #include <tee.h>
> +#include <asm/cache.h>
>  #include <dm/device-internal.h>
>  #include <dm/uclass-internal.h>
>  
> @@ -233,3 +235,18 @@ void tee_optee_ta_uuid_to_octets(u8 d[TEE_UUID_LEN],
>  	d[7] = s->time_hi_and_version;
>  	memcpy(d + 8, s->clock_seq_and_node, sizeof(s->clock_seq_and_node));
>  }
> +
> +void tee_flush_all_shm_dcache(struct udevice *dev)
> +{
> +	struct tee_uclass_priv *priv = dev_get_uclass_priv(dev);
> +	struct tee_shm *s;
> +
> +	list_for_each_entry(s, &priv->list_shm, link) {
> +		ulong start = rounddown((ulong)s->addr,
> +					CONFIG_SYS_CACHELINE_SIZE);
> +		ulong end = roundup((ulong)s->addr + s->size,
> +				    CONFIG_SYS_CACHELINE_SIZE);
> +
> +		flush_dcache_range(start, end);
> +	}
> +}
> diff --git a/include/tee.h b/include/tee.h
> index 99367b258e..2ef29bfc8f 100644
> --- a/include/tee.h
> +++ b/include/tee.h
> @@ -377,4 +377,10 @@ void tee_optee_ta_uuid_from_octets(struct tee_optee_ta_uuid *d,
>  void tee_optee_ta_uuid_to_octets(u8 d[TEE_UUID_LEN],
>  				 const struct tee_optee_ta_uuid *s);
>  
> +/**
> + * tee_flush_all_shm_dcache() - Flush data cache for all shared memories
> + * @dev:	The TEE device
> + */
> +void tee_flush_all_shm_dcache(struct udevice *dev);
> +
>  #endif /* __TEE_H */
> -- 
> 2.17.1
> 

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v2] tee: optee: sync cache on pre-reloc OP-TEE invocation

[Tom Rini]

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

On Wed, May 19, 2021 at 04:27:41PM +0200, Etienne Carriere wrote:

> This change ensures both U-Boot and OP-TEE see the same content
> from shared memory when OP-TEE is invoked prior U-Boot relocation.
> 
> This change is required since U-Boot may execute with data cache off
> while OP-TEE always enables cache on memory shared with U-Boot.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
> Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Applied to u-boot/master, thanks!

-- 
Tom

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