On 9/28/17 1:04 PM, Volodymyr Babchuk wrote:
From: Volodymyr Babchuk vlad.babchuk@gmail.com
Previous patches added various features that are needed for dynamic SHM. Dynamic SHM allows Normal World to share any buffers with OP-TEE. While original design suggested to use pre-allocated region (usually of 1M to 2M of size), this new approach allows to use all non-secure RAM for command buffers, RPC allocations and TA parameters.
This patch checks capability OPTEE_SMC_SEC_CAP_DYNAMIC_SHM. If it was set by OP-TEE, then kernel part of OP-TEE will use kernel page allocator to allocate command buffers. Also it will set TEE_GEN_CAP_REG_MEM capability to tell userspace that it supports shared memory registration.
Signed-off-by: Volodymyr Babchuk vlad.babchuk@gmail.com
drivers/tee/optee/core.c | 69 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 18 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 8e012ea..e8fd9af 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -28,6 +28,7 @@ #include <linux/uaccess.h> #include "optee_private.h" #include "optee_smc.h" +#include "shm_pool.h" #define DRIVER_NAME "optee" @@ -227,6 +228,10 @@ static void optee_get_version(struct tee_device *teedev, .impl_caps = TEE_OPTEE_CAP_TZ, .gen_caps = TEE_GEN_CAP_GP, };
- struct optee *optee = tee_get_drvdata(teedev);
- if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
v.gen_caps |= TEE_GEN_CAP_REG_MEM;@@ -405,21 +410,22 @@ static bool optee_msg_exchange_capabilities(optee_invoke_fn *invoke_fn, } static struct tee_shm_pool * -optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm) +optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm,
u32 sec_caps)
- struct tee_shm_pool *pool; unsigned long vaddr; phys_addr_t paddr; size_t size; phys_addr_t begin; phys_addr_t end; void *va;
- struct tee_shm_pool_mem_info priv_info;
- struct tee_shm_pool_mem_info dmabuf_info;
- struct tee_shm_pool_mgr *priv_mgr;
- struct tee_shm_pool_mgr *dmabuf_mgr;
- void *rc;
invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc); if (res.result.status != OPTEE_SMC_RETURN_OK) { @@ -449,22 +455,49 @@ optee_config_shm_memremap(optee_invoke_fn *invoke_fn, void **memremaped_shm) } vaddr = (unsigned long)va;
- priv_info.vaddr = vaddr;
- priv_info.paddr = paddr;
- priv_info.size = OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
- dmabuf_info.vaddr = vaddr + OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
- dmabuf_info.paddr = paddr + OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
- dmabuf_info.size = size - OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE;
- pool = tee_shm_pool_alloc_res_mem(&priv_info, &dmabuf_info);
- if (IS_ERR(pool)) {
memunmap(va);goto out;
Now that you removed the call to tee_shm_pool_alloc_res_mem() it is dead code and no longer used. Do we still need tee_shm_pool_alloc_res_mem at all?
- /*
* 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, size, PAGE_SHIFT);
- if (IS_ERR(rc))
goto err_free_priv_mgr;- dmabuf_mgr = rc;
- rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
- if (IS_ERR(rc))
goto err_free_dmabuf_mgr;- *memremaped_shm = va;
-out:
- return pool;
- return rc;
+err_free_dmabuf_mgr:
- tee_shm_pool_mgr_destroy(dmabuf_mgr);
+err_free_priv_mgr:
- tee_shm_pool_mgr_destroy(priv_mgr);
+err_memunmap:
- memunmap(va);
- return rc; }
This function now mixes dynamic and shared memory based allocation in a way that only applies to certain cases.
We're going to have the following cases: -Linux OP-TEE driver sees only static shared memory advertised (older versions of OP-TEE) -Linux OP-TEE driver sees only dynamic shared memory advertised (e.g. a guest kernel in a VM) -Linux OP-TEE driver sees both static and dynamic memory advertised
We are not handling the 'only dynamic shared memory' case currently and this code is going to have to be refactored again to support that. Since we are substantially re-working it anyway here, why don't we support all the cases.
It seems like we need logic along the lines of:
invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc); if (res.result.status == OPTEE_SMC_RETURN_OK) optee_static_shm = true; else optee_static_shm = false;
if (sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) optee_dynamic_shm = true; else optee_dynamic_shm = false;
/* allocate private pool */ if (optee_dynamic_shm) { rc = optee_shm_pool_alloc_pages(); priv_mgr = rc; } else { rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, sz, 3); priv_mgr = rc; }
/* allocate dmabuf pool */ if (optee_dynamic_shm && !optee_static_shm) { dmabuf_mgr = NULL; } else { rc = tee_shm_pool_mgr_alloc_res_mem(vaddr, paddr, size, PAGE_SHIFT); dmabuf_mgr = rc; }
rc = tee_shm_pool_alloc(priv_mgr, dmabuf_mgr);
/* Simple wrapper functions to be able to use a function pointer */ @@ -542,7 +575,7 @@ static struct optee *optee_probe(struct device_node *np) if (!(sec_caps & OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM)) return ERR_PTR(-EINVAL);
We should remove the above assumption that there must be static shared memory. It shouldn't be an error.
Thanks, Stuart