[PATCH] optee: allow to work without static shared memory

[PATCH] optee: allow to work without static shared memory

[Volodymyr Babchuk]

From: Volodymyr Babchuk <vlad.babchuk@gmail.com>

On virtualized systems it is possible that OP-TEE will provide
only dynamic shared memory support. So it is fine to boot
without static SHM enabled if dymanic one is supported.

Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
---
 drivers/tee/optee/core.c | 83 ++++++++++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 4a2c420..d80da29 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -420,9 +420,35 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn,
 	return true;
 }
 
+static struct tee_shm_pool *optee_config_dyn_shm(void)
+{
+	struct tee_shm_pool_mgr *priv_mgr;
+	struct tee_shm_pool_mgr *dmabuf_mgr;
+	void *rc;
+
+	rc = optee_shm_pool_alloc_pages();
+	if (IS_ERR(rc))
+		return rc;
+	priv_mgr = rc;
+
+	rc = optee_shm_pool_alloc_pages();
+	if (IS_ERR(rc)) {
+		tee_shm_pool_mgr_destroy(priv_mgr);
+		return rc;
+	}
+	dmabuf_mgr = rc;
+
+	rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
+	if (IS_ERR(rc)) {
+		tee_shm_pool_mgr_destroy(priv_mgr);
+		tee_shm_pool_mgr_destroy(dmabuf_mgr);
+	}
+
+	return rc;
+}
+
 static struct tee_shm_pool *
-optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
-			  u32 sec_caps)
+optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm)
 {
 	union {
 		struct arm_smccc_res smccc;
@@ -437,10 +463,11 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
 	struct tee_shm_pool_mgr *priv_mgr;
 	struct tee_shm_pool_mgr *dmabuf_mgr;
 	void *rc;
+	const int sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
 
 	invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
 	if (res.result.status != OPTEE_SMC_RETURN_OK) {
-		pr_info("shm service not available\n");
+		pr_err("static shm service not available\n");
 		return ERR_PTR(-ENOENT);
 	}
 
@@ -466,28 +493,15 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
 	}
 	vaddr = (unsigned long)va;
 
-	/*
-	 * If OP-TEE can work with unregistered SHM, we will use own pool
-	 * for private shm
-	 */
-	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) {
-		rc = optee_shm_pool_alloc_pages();
-		if (IS_ERR(rc))
-			goto err_memunmap;
-		priv_mgr = rc;
-	} else {
-		const size_t sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
-
-		rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
-						    3 /* 8 bytes aligned */);
-		if (IS_ERR(rc))
-			goto err_memunmap;
-		priv_mgr = rc;
-
-		vaddr += sz;
-		paddr += sz;
-		size -= sz;
-	}
+	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
+					    3 /* 8 bytes aligned */);
+	if (IS_ERR(rc))
+		goto err_memunmap;
+	priv_mgr = rc;
+
+	vaddr += sz;
+	paddr += sz;
+	size -= sz;
 
 	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, size, PAGE_SHIFT);
 	if (IS_ERR(rc))
@@ -553,7 +567,7 @@ static optee_invoke_fn *get_invoke_func(struct device_node *np)
 static struct optee *optee_probe(struct device_node *np)
 {
 	optee_invoke_fn *invoke_fn;
-	struct tee_shm_pool *pool;
+	struct tee_shm_pool *pool = ERR_PTR(-EINVAL);
 	struct optee *optee = NULL;
 	void *memremaped_shm = NULL;
 	struct tee_device *teedev;
@@ -582,13 +596,17 @@ static struct optee *optee_probe(struct device_node *np)
 	}
 
 	/*
-	 * We have no other option for shared memory, if secure world
-	 * doesn't have any reserved memory we can use we can't continue.
+	 * Try to use dynamic shared memory if possible
 	 */
-	if (!(sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
-		return ERR_PTR(-EINVAL);
+	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
+		pool = optee_config_dyn_shm();
+
+	/*
+	 * If dynamic shared memory is not available or failed - try static one
+	 */
+	if (IS_ERR(pool) && (sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
+		pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm);
 
-	pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm, sec_caps);
 	if (IS_ERR(pool))
 		return (void *)pool;
 
@@ -632,9 +650,6 @@ static struct optee *optee_probe(struct device_node *np)
 
 	optee_enable_shm_cache(optee);
 
-	if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
-		pr_info("dynamic shared memory is enabled\n");
-
 	pr_info("initialized driver\n");
 	return optee;
 err:
-- 
2.7.4

Re: [Tee-dev] [PATCH] optee: allow to work without static shared memory

[Rouven Czerwinski]

Volodymyr Babchuk <volodymyr_babchuk@epam.com> writes:

> From: Volodymyr Babchuk <vlad.babchuk@gmail.com>
>
> On virtualized systems it is possible that OP-TEE will provide
> only dynamic shared memory support. So it is fine to boot
> without static SHM enabled if dymanic one is supported.
>
> Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
> ---
>  drivers/tee/optee/core.c | 83 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 49 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 4a2c420..d80da29 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -420,9 +420,35 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn,
>  	return true;
>  }
>  
> +static struct tee_shm_pool *optee_config_dyn_shm(void)
> +{
> +	struct tee_shm_pool_mgr *priv_mgr;
> +	struct tee_shm_pool_mgr *dmabuf_mgr;
> +	void *rc;
> +
> +	rc = optee_shm_pool_alloc_pages();
> +	if (IS_ERR(rc))
> +		return rc;
> +	priv_mgr = rc;
> +
> +	rc = optee_shm_pool_alloc_pages();
> +	if (IS_ERR(rc)) {
> +		tee_shm_pool_mgr_destroy(priv_mgr);
> +		return rc;
> +	}
> +	dmabuf_mgr = rc;
> +
> +	rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
> +	if (IS_ERR(rc)) {
> +		tee_shm_pool_mgr_destroy(priv_mgr);
> +		tee_shm_pool_mgr_destroy(dmabuf_mgr);
> +	}
> +
> +	return rc;
> +}
> +
>  static struct tee_shm_pool *
> -optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
> -			  u32 sec_caps)
> +optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm)
>  {
>  	union {
>  		struct arm_smccc_res smccc;
> @@ -437,10 +463,11 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
>  	struct tee_shm_pool_mgr *priv_mgr;
>  	struct tee_shm_pool_mgr *dmabuf_mgr;
>  	void *rc;
> +	const int sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
>  
>  	invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
>  	if (res.result.status != OPTEE_SMC_RETURN_OK) {
> -		pr_info("shm service not available\n");
> +		pr_err("static shm service not available\n");
>  		return ERR_PTR(-ENOENT);
>  	}
>  
> @@ -466,28 +493,15 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
>  	}
>  	vaddr = (unsigned long)va;
>  
> -	/*
> -	 * If OP-TEE can work with unregistered SHM, we will use own pool
> -	 * for private shm
> -	 */
> -	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) {
> -		rc = optee_shm_pool_alloc_pages();
> -		if (IS_ERR(rc))
> -			goto err_memunmap;
> -		priv_mgr = rc;
> -	} else {
> -		const size_t sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
> -
> -		rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
> -						    3 /* 8 bytes aligned */);
> -		if (IS_ERR(rc))
> -			goto err_memunmap;
> -		priv_mgr = rc;
> -
> -		vaddr += sz;
> -		paddr += sz;
> -		size -= sz;
> -	}
> +	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
> +					    3 /* 8 bytes aligned */);
> +	if (IS_ERR(rc))
> +		goto err_memunmap;
> +	priv_mgr = rc;
> +
> +	vaddr += sz;
> +	paddr += sz;
> +	size -= sz;
>  
>  	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, size, PAGE_SHIFT);
>  	if (IS_ERR(rc))
> @@ -553,7 +567,7 @@ static optee_invoke_fn *get_invoke_func(struct device_node *np)
>  static struct optee *optee_probe(struct device_node *np)
>  {
>  	optee_invoke_fn *invoke_fn;
> -	struct tee_shm_pool *pool;
> +	struct tee_shm_pool *pool = ERR_PTR(-EINVAL);
>  	struct optee *optee = NULL;
>  	void *memremaped_shm = NULL;
>  	struct tee_device *teedev;
> @@ -582,13 +596,17 @@ static struct optee *optee_probe(struct device_node *np)
>  	}
>  
>  	/*
> -	 * We have no other option for shared memory, if secure world
> -	 * doesn't have any reserved memory we can use we can't continue.
> +	 * Try to use dynamic shared memory if possible
>  	 */
> -	if (!(sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
> -		return ERR_PTR(-EINVAL);
> +	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
> +		pool = optee_config_dyn_shm();
> +
> +	/*
> +	 * If dynamic shared memory is not available or failed - try static one
> +	 */
> +	if (IS_ERR(pool) && (sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
> +		pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm);
>  
> -	pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm, sec_caps);
>  	if (IS_ERR(pool))
>  		return (void *)pool;
>  

-- BEGIN --
> @@ -632,9 +650,6 @@ static struct optee *optee_probe(struct device_node *np)
>  
>  	optee_enable_shm_cache(optee);
>  
> -	if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
> -		pr_info("dynamic shared memory is enabled\n");
> -
>  	pr_info("initialized driver\n");
>  	return optee;
>  err:
-- END --

This part does not apply against upstream.

Greetings

Rouven

[PATCH v2] optee: allow to work without static shared memory

[Volodymyr Babchuk]

From: Volodymyr Babchuk <vlad.babchuk@gmail.com>

On virtualized systems it is possible that OP-TEE will provide
only dynamic shared memory support. So it is fine to boot
without static SHM enabled if dymanic one is supported.

Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
---

 Changes from v1:
   Patch is now can be applied to vanilla kernel instead of
   linaro's op-tee branch

 drivers/tee/optee/core.c | 80 +++++++++++++++++++++++++++++-------------------
 1 file changed, 49 insertions(+), 31 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index e1aafe8..efd2e5c 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -419,9 +419,35 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn,
 	return true;
 }
 
+static struct tee_shm_pool *optee_config_dyn_shm(void)
+{
+	struct tee_shm_pool_mgr *priv_mgr;
+	struct tee_shm_pool_mgr *dmabuf_mgr;
+	void *rc;
+
+	rc = optee_shm_pool_alloc_pages();
+	if (IS_ERR(rc))
+		return rc;
+	priv_mgr = rc;
+
+	rc = optee_shm_pool_alloc_pages();
+	if (IS_ERR(rc)) {
+		tee_shm_pool_mgr_destroy(priv_mgr);
+		return rc;
+	}
+	dmabuf_mgr = rc;
+
+	rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
+	if (IS_ERR(rc)) {
+		tee_shm_pool_mgr_destroy(priv_mgr);
+		tee_shm_pool_mgr_destroy(dmabuf_mgr);
+	}
+
+	return rc;
+}
+
 static struct tee_shm_pool *
-optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
-			  u32 sec_caps)
+optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm)
 {
 	union {
 		struct arm_smccc_res smccc;
@@ -436,10 +462,11 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
 	struct tee_shm_pool_mgr *priv_mgr;
 	struct tee_shm_pool_mgr *dmabuf_mgr;
 	void *rc;
+	const int sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
 
 	invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
 	if (res.result.status != OPTEE_SMC_RETURN_OK) {
-		pr_info("shm service not available\n");
+		pr_err("static shm service not available\n");
 		return ERR_PTR(-ENOENT);
 	}
 
@@ -465,28 +492,15 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
 	}
 	vaddr = (unsigned long)va;
 
-	/*
-	 * If OP-TEE can work with unregistered SHM, we will use own pool
-	 * for private shm
-	 */
-	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) {
-		rc = optee_shm_pool_alloc_pages();
-		if (IS_ERR(rc))
-			goto err_memunmap;
-		priv_mgr = rc;
-	} else {
-		const size_t sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
-
-		rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
-						    3 /* 8 bytes aligned */);
-		if (IS_ERR(rc))
-			goto err_memunmap;
-		priv_mgr = rc;
-
-		vaddr += sz;
-		paddr += sz;
-		size -= sz;
-	}
+	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
+					    3 /* 8 bytes aligned */);
+	if (IS_ERR(rc))
+		goto err_memunmap;
+	priv_mgr = rc;
+
+	vaddr += sz;
+	paddr += sz;
+	size -= sz;
 
 	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, size, PAGE_SHIFT);
 	if (IS_ERR(rc))
@@ -552,7 +566,7 @@ static optee_invoke_fn *get_invoke_func(struct device_node *np)
 static struct optee *optee_probe(struct device_node *np)
 {
 	optee_invoke_fn *invoke_fn;
-	struct tee_shm_pool *pool;
+	struct tee_shm_pool *pool = ERR_PTR(-EINVAL);
 	struct optee *optee = NULL;
 	void *memremaped_shm = NULL;
 	struct tee_device *teedev;
@@ -581,13 +595,17 @@ static struct optee *optee_probe(struct device_node *np)
 	}
 
 	/*
-	 * We have no other option for shared memory, if secure world
-	 * doesn't have any reserved memory we can use we can't continue.
+	 * Try to use dynamic shared memory if possible
 	 */
-	if (!(sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
-		return ERR_PTR(-EINVAL);
+	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
+		pool = optee_config_dyn_shm();
+
+	/*
+	 * If dynamic shared memory is not available or failed - try static one
+	 */
+	if (IS_ERR(pool) && (sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
+		pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm);
 
-	pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm, sec_caps);
 	if (IS_ERR(pool))
 		return (void *)pool;
 
-- 
2.7.4

[PATCH v3] optee: allow to work without static shared memory

[Volodymyr Babchuk]

From: Volodymyr Babchuk <vlad.babchuk@gmail.com>

On virtualized systems it is possible that OP-TEE will provide
only dynamic shared memory support. So it is fine to boot
without static SHM enabled if dymanic one is supported.

Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
---

 Changes from V2:
  - rebased onto upstream

 drivers/tee/optee/core.c | 80 ++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 31 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 0842b6e6af82..48963eab32f5 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -419,9 +419,35 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn,
 	return true;
 }
 
+static struct tee_shm_pool *optee_config_dyn_shm(void)
+{
+	struct tee_shm_pool_mgr *priv_mgr;
+	struct tee_shm_pool_mgr *dmabuf_mgr;
+	void *rc;
+
+	rc = optee_shm_pool_alloc_pages();
+	if (IS_ERR(rc))
+		return rc;
+	priv_mgr = rc;
+
+	rc = optee_shm_pool_alloc_pages();
+	if (IS_ERR(rc)) {
+		tee_shm_pool_mgr_destroy(priv_mgr);
+		return rc;
+	}
+	dmabuf_mgr = rc;
+
+	rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
+	if (IS_ERR(rc)) {
+		tee_shm_pool_mgr_destroy(priv_mgr);
+		tee_shm_pool_mgr_destroy(dmabuf_mgr);
+	}
+
+	return rc;
+}
+
 static struct tee_shm_pool *
-optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
-			  u32 sec_caps)
+optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm)
 {
 	union {
 		struct arm_smccc_res smccc;
@@ -436,10 +462,11 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
 	struct tee_shm_pool_mgr *priv_mgr;
 	struct tee_shm_pool_mgr *dmabuf_mgr;
 	void *rc;
+	const int sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
 
 	invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
 	if (res.result.status != OPTEE_SMC_RETURN_OK) {
-		pr_info("shm service not available\n");
+		pr_err("static shm service not available\n");
 		return ERR_PTR(-ENOENT);
 	}
 
@@ -465,28 +492,15 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
 	}
 	vaddr = (unsigned long)va;
 
-	/*
-	 * If OP-TEE can work with unregistered SHM, we will use own pool
-	 * for private shm
-	 */
-	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) {
-		rc = optee_shm_pool_alloc_pages();
-		if (IS_ERR(rc))
-			goto err_memunmap;
-		priv_mgr = rc;
-	} else {
-		const size_t sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
-
-		rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
-						    3 /* 8 bytes aligned */);
-		if (IS_ERR(rc))
-			goto err_memunmap;
-		priv_mgr = rc;
-
-		vaddr += sz;
-		paddr += sz;
-		size -= sz;
-	}
+	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
+					    3 /* 8 bytes aligned */);
+	if (IS_ERR(rc))
+		goto err_memunmap;
+	priv_mgr = rc;
+
+	vaddr += sz;
+	paddr += sz;
+	size -= sz;
 
 	rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, size, PAGE_SHIFT);
 	if (IS_ERR(rc))
@@ -552,7 +566,7 @@ static optee_invoke_fn *get_invoke_func(struct device_node *np)
 static struct optee *optee_probe(struct device_node *np)
 {
 	optee_invoke_fn *invoke_fn;
-	struct tee_shm_pool *pool;
+	struct tee_shm_pool *pool = ERR_PTR(-EINVAL);
 	struct optee *optee = NULL;
 	void *memremaped_shm = NULL;
 	struct tee_device *teedev;
@@ -581,13 +595,17 @@ static struct optee *optee_probe(struct device_node *np)
 	}
 
 	/*
-	 * We have no other option for shared memory, if secure world
-	 * doesn't have any reserved memory we can use we can't continue.
+	 * Try to use dynamic shared memory if possible
 	 */
-	if (!(sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
-		return ERR_PTR(-EINVAL);
+	if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
+		pool = optee_config_dyn_shm();
+
+	/*
+	 * If dynamic shared memory is not available or failed - try static one
+	 */
+	if (IS_ERR(pool) && (sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
+		pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm);
 
-	pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm, sec_caps);
 	if (IS_ERR(pool))
 		return (void *)pool;
 
-- 
2.21.0

Re: [PATCH v3] optee: allow to work without static shared memory

[Jens Wiklander]

Hi Volodymyr,

On Mon, Mar 11, 2019 at 2:04 PM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> From: Volodymyr Babchuk <vlad.babchuk@gmail.com>
>
> On virtualized systems it is possible that OP-TEE will provide
> only dynamic shared memory support. So it is fine to boot
> without static SHM enabled if dymanic one is supported.
>
> Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
> ---
>
>  Changes from V2:
>   - rebased onto upstream
>
>  drivers/tee/optee/core.c | 80 ++++++++++++++++++++++++----------------
>  1 file changed, 49 insertions(+), 31 deletions(-)
>

Looks good, I'm picking this up.

Thanks,
Jens

Re: [Tee-dev] [PATCH v3] optee: allow to work without static shared memory

[Sumit Garg]

Hi Volodymyr, Jens,

On Mon, 11 Mar 2019 at 18:34, Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> From: Volodymyr Babchuk <vlad.babchuk@gmail.com>
>
> On virtualized systems it is possible that OP-TEE will provide
> only dynamic shared memory support. So it is fine to boot
> without static SHM enabled if dymanic one is supported.
>
> Signed-off-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
> ---

Is this patch well tested?

This patch seems to broke "tee_shm_alloc()" API for all kernel
internal / user-space clients in case dynamic shared memory is enabled
in OP-TEE (capability: OPTEE_SMC_SEC_CAP_DYNAMIC_SHM is set).

How are kernel buffers allocated from "dmabuf_mgr" as per this patch
registered with OP-TEE?

Note here that "priv_mgr" seems to work well as OP-TEE provides a hook
to map the buffers returned during RPC calls into registered shm
space.

I think the correct way to enable intended feature as per description
of the patch would be to add an additional call to
"optee_shm_register()" during allocation from "dmabuf_mgr". So I have
proposed a fix patch here [1]. Please have a look.

[1] https://lkml.org/lkml/2019/10/31/382

Regards,
Sumit

>
>  Changes from V2:
>   - rebased onto upstream
>
>  drivers/tee/optee/core.c | 80 ++++++++++++++++++++++++----------------
>  1 file changed, 49 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 0842b6e6af82..48963eab32f5 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -419,9 +419,35 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn,
>         return true;
>  }
>
> +static struct tee_shm_pool *optee_config_dyn_shm(void)
> +{
> +       struct tee_shm_pool_mgr *priv_mgr;
> +       struct tee_shm_pool_mgr *dmabuf_mgr;
> +       void *rc;
> +
> +       rc = optee_shm_pool_alloc_pages();
> +       if (IS_ERR(rc))
> +               return rc;
> +       priv_mgr = rc;
> +
> +       rc = optee_shm_pool_alloc_pages();
> +       if (IS_ERR(rc)) {
> +               tee_shm_pool_mgr_destroy(priv_mgr);
> +               return rc;
> +       }
> +       dmabuf_mgr = rc;
> +
> +       rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
> +       if (IS_ERR(rc)) {
> +               tee_shm_pool_mgr_destroy(priv_mgr);
> +               tee_shm_pool_mgr_destroy(dmabuf_mgr);
> +       }
> +
> +       return rc;
> +}
> +
>  static struct tee_shm_pool *
> -optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
> -                         u32 sec_caps)
> +optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm)
>  {
>         union {
>                 struct arm_smccc_res smccc;
> @@ -436,10 +462,11 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
>         struct tee_shm_pool_mgr *priv_mgr;
>         struct tee_shm_pool_mgr *dmabuf_mgr;
>         void *rc;
> +       const int sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
>
>         invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
>         if (res.result.status != OPTEE_SMC_RETURN_OK) {
> -               pr_info("shm service not available\n");
> +               pr_err("static shm service not available\n");
>                 return ERR_PTR(-ENOENT);
>         }
>
> @@ -465,28 +492,15 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
>         }
>         vaddr = (unsigned long)va;
>
> -       /*
> -        * If OP-TEE can work with unregistered SHM, we will use own pool
> -        * for private shm
> -        */
> -       if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) {
> -               rc = optee_shm_pool_alloc_pages();
> -               if (IS_ERR(rc))
> -                       goto err_memunmap;
> -               priv_mgr = rc;
> -       } else {
> -               const size_t sz = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
> -
> -               rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
> -                                                   3 /* 8 bytes aligned */);
> -               if (IS_ERR(rc))
> -                       goto err_memunmap;
> -               priv_mgr = rc;
> -
> -               vaddr += sz;
> -               paddr += sz;
> -               size -= sz;
> -       }
> +       rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz,
> +                                           3 /* 8 bytes aligned */);
> +       if (IS_ERR(rc))
> +               goto err_memunmap;
> +       priv_mgr = rc;
> +
> +       vaddr += sz;
> +       paddr += sz;
> +       size -= sz;
>
>         rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, size, PAGE_SHIFT);
>         if (IS_ERR(rc))
> @@ -552,7 +566,7 @@ static optee_invoke_fn *get_invoke_func(struct device_node *np)
>  static struct optee *optee_probe(struct device_node *np)
>  {
>         optee_invoke_fn *invoke_fn;
> -       struct tee_shm_pool *pool;
> +       struct tee_shm_pool *pool = ERR_PTR(-EINVAL);
>         struct optee *optee = NULL;
>         void *memremaped_shm = NULL;
>         struct tee_device *teedev;
> @@ -581,13 +595,17 @@ static struct optee *optee_probe(struct device_node *np)
>         }
>
>         /*
> -        * We have no other option for shared memory, if secure world
> -        * doesn't have any reserved memory we can use we can't continue.
> +        * Try to use dynamic shared memory if possible
>          */
> -       if (!(sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
> -               return ERR_PTR(-EINVAL);
> +       if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
> +               pool = optee_config_dyn_shm();
> +
> +       /*
> +        * If dynamic shared memory is not available or failed - try static one
> +        */
> +       if (IS_ERR(pool) && (sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM))
> +               pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm);
>
> -       pool = optee_config_shm_memremap(invoke_fn, &memremaped_shm, sec_caps);
>         if (IS_ERR(pool))
>                 return (void *)pool;
>
> --
> 2.21.0
> _______________________________________________
> Tee-dev mailing list
> Tee-dev@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/tee-dev