Messages in this thread | | | From | Sumit Garg <> | Date | Thu, 31 Oct 2019 18:39:45 +0530 | Subject | Re: [Tee-dev] [PATCH v3] optee: allow to work without static shared memory |
| |
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
| |