In case of dynamic shared memory pool, kernel memory allocated using
dmabuf_mgr pool needs to be registered with OP-TEE prior to its usage
during optee_open_session() or optee_invoke_func().
So fix dmabuf_mgr pool allocations via an additional call to
optee_shm_register().
Fixes: 9733b072a12a ("optee: allow to work without static shared memory")
Signed-off-by: Sumit Garg <[email protected]>
---
Depends on patch: https://lkml.org/lkml/2019/7/30/506 that adds support
to allow registration of kernel buffers with OP-TEE.
drivers/tee/optee/shm_pool.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
index de1d9b8..0332a53 100644
--- a/drivers/tee/optee/shm_pool.c
+++ b/drivers/tee/optee/shm_pool.c
@@ -17,6 +17,7 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
{
unsigned int order = get_order(size);
struct page *page;
+ int rc = 0;
page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
if (!page)
@@ -26,12 +27,21 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
shm->paddr = page_to_phys(page);
shm->size = PAGE_SIZE << order;
- return 0;
+ if (shm->flags & TEE_SHM_DMA_BUF) {
+ shm->flags |= TEE_SHM_REGISTER;
+ rc = optee_shm_register(shm->ctx, shm, &page, 1 << order,
+ (unsigned long)shm->kaddr);
+ }
+
+ return rc;
}
static void pool_op_free(struct tee_shm_pool_mgr *poolm,
struct tee_shm *shm)
{
+ if (shm->flags & TEE_SHM_DMA_BUF)
+ optee_shm_unregister(shm->ctx, shm);
+
free_pages((unsigned long)shm->kaddr, get_order(shm->size));
shm->kaddr = NULL;
}
--
2.7.4
Hi Sumit,
On Thu, Oct 31, 2019 at 06:37:54PM +0530, Sumit Garg wrote:
> In case of dynamic shared memory pool, kernel memory allocated using
> dmabuf_mgr pool needs to be registered with OP-TEE prior to its usage
> during optee_open_session() or optee_invoke_func().
>
> So fix dmabuf_mgr pool allocations via an additional call to
> optee_shm_register().
>
> Fixes: 9733b072a12a ("optee: allow to work without static shared memory")
> Signed-off-by: Sumit Garg <[email protected]>
Looks good, I'm picking this up. Etienne told me has tested this on some
of his hardware so I'll add:
Tested-by: Etienne Carriere <[email protected]>
Thanks,
Jens
> ---
>
> Depends on patch: https://lkml.org/lkml/2019/7/30/506 that adds support
> to allow registration of kernel buffers with OP-TEE.
>
> drivers/tee/optee/shm_pool.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
> index de1d9b8..0332a53 100644
> --- a/drivers/tee/optee/shm_pool.c
> +++ b/drivers/tee/optee/shm_pool.c
> @@ -17,6 +17,7 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> {
> unsigned int order = get_order(size);
> struct page *page;
> + int rc = 0;
>
> page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> if (!page)
> @@ -26,12 +27,21 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> shm->paddr = page_to_phys(page);
> shm->size = PAGE_SIZE << order;
>
> - return 0;
> + if (shm->flags & TEE_SHM_DMA_BUF) {
> + shm->flags |= TEE_SHM_REGISTER;
> + rc = optee_shm_register(shm->ctx, shm, &page, 1 << order,
> + (unsigned long)shm->kaddr);
> + }
> +
> + return rc;
> }
>
> static void pool_op_free(struct tee_shm_pool_mgr *poolm,
> struct tee_shm *shm)
> {
> + if (shm->flags & TEE_SHM_DMA_BUF)
> + optee_shm_unregister(shm->ctx, shm);
> +
> free_pages((unsigned long)shm->kaddr, get_order(shm->size));
> shm->kaddr = NULL;
> }
> --
> 2.7.4
>
Hi Jens,
On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <[email protected]> wrote:
>
> Hi Sumit,
>
> On Thu, Oct 31, 2019 at 06:37:54PM +0530, Sumit Garg wrote:
> > In case of dynamic shared memory pool, kernel memory allocated using
> > dmabuf_mgr pool needs to be registered with OP-TEE prior to its usage
> > during optee_open_session() or optee_invoke_func().
> >
> > So fix dmabuf_mgr pool allocations via an additional call to
> > optee_shm_register().
> >
> > Fixes: 9733b072a12a ("optee: allow to work without static shared memory")
> > Signed-off-by: Sumit Garg <[email protected]>
>
> Looks good, I'm picking this up. Etienne told me has tested this on some
> of his hardware so I'll add:
> Tested-by: Etienne Carriere <[email protected]>
>
Thanks for picking this up.
>
> > ---
> >
> > Depends on patch: https://lkml.org/lkml/2019/7/30/506 that adds support
> > to allow registration of kernel buffers with OP-TEE.
You also need to pick up the dependency patch too. The latest v3 of
this patch is here [1] although its unchanged from v2. Also, If you
pick that up I will drop it from future revisions of Trusted Keys
patchset [2].
[1] https://lkml.org/lkml/2019/10/31/431
[2] https://lkml.org/lkml/2019/10/31/430
-Sumit
> >
> > drivers/tee/optee/shm_pool.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
> > index de1d9b8..0332a53 100644
> > --- a/drivers/tee/optee/shm_pool.c
> > +++ b/drivers/tee/optee/shm_pool.c
> > @@ -17,6 +17,7 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> > {
> > unsigned int order = get_order(size);
> > struct page *page;
> > + int rc = 0;
> >
> > page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> > if (!page)
> > @@ -26,12 +27,21 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> > shm->paddr = page_to_phys(page);
> > shm->size = PAGE_SIZE << order;
> >
> > - return 0;
> > + if (shm->flags & TEE_SHM_DMA_BUF) {
> > + shm->flags |= TEE_SHM_REGISTER;
> > + rc = optee_shm_register(shm->ctx, shm, &page, 1 << order,
> > + (unsigned long)shm->kaddr);
> > + }
> > +
> > + return rc;
> > }
> >
> > static void pool_op_free(struct tee_shm_pool_mgr *poolm,
> > struct tee_shm *shm)
> > {
> > + if (shm->flags & TEE_SHM_DMA_BUF)
> > + optee_shm_unregister(shm->ctx, shm);
> > +
> > free_pages((unsigned long)shm->kaddr, get_order(shm->size));
> > shm->kaddr = NULL;
> > }
> > --
> > 2.7.4
> >
Hi Sumit,
On Thu, Nov 7, 2019 at 1:41 PM Sumit Garg <[email protected]> wrote:
>
> Hi Jens,
>
> On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <[email protected]> wrote:
> >
> > Hi Sumit,
> >
> > On Thu, Oct 31, 2019 at 06:37:54PM +0530, Sumit Garg wrote:
> > > In case of dynamic shared memory pool, kernel memory allocated using
> > > dmabuf_mgr pool needs to be registered with OP-TEE prior to its usage
> > > during optee_open_session() or optee_invoke_func().
> > >
> > > So fix dmabuf_mgr pool allocations via an additional call to
> > > optee_shm_register().
> > >
> > > Fixes: 9733b072a12a ("optee: allow to work without static shared memory")
> > > Signed-off-by: Sumit Garg <[email protected]>
> >
> > Looks good, I'm picking this up. Etienne told me has tested this on some
> > of his hardware so I'll add:
> > Tested-by: Etienne Carriere <[email protected]>
> >
>
> Thanks for picking this up.
>
> >
> > > ---
> > >
> > > Depends on patch: https://lkml.org/lkml/2019/7/30/506 that adds support
> > > to allow registration of kernel buffers with OP-TEE.
>
> You also need to pick up the dependency patch too. The latest v3 of
> this patch is here [1] although its unchanged from v2. Also, If you
> pick that up I will drop it from future revisions of Trusted Keys
> patchset [2].
>
> [1] https://lkml.org/lkml/2019/10/31/431
> [2] https://lkml.org/lkml/2019/10/31/430
So I missed taking [1] into account. I think that it would make more
sense to merge this patch ("tee: optee: Fix dynamic shm pool
allocations") with [1] ("tee: optee: allow kernel pages to register as
shm") into a new patch.
Cheers,
Jens
>
> -Sumit
>
> > >
> > > drivers/tee/optee/shm_pool.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
> > > index de1d9b8..0332a53 100644
> > > --- a/drivers/tee/optee/shm_pool.c
> > > +++ b/drivers/tee/optee/shm_pool.c
> > > @@ -17,6 +17,7 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> > > {
> > > unsigned int order = get_order(size);
> > > struct page *page;
> > > + int rc = 0;
> > >
> > > page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> > > if (!page)
> > > @@ -26,12 +27,21 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> > > shm->paddr = page_to_phys(page);
> > > shm->size = PAGE_SIZE << order;
> > >
> > > - return 0;
> > > + if (shm->flags & TEE_SHM_DMA_BUF) {
> > > + shm->flags |= TEE_SHM_REGISTER;
> > > + rc = optee_shm_register(shm->ctx, shm, &page, 1 << order,
> > > + (unsigned long)shm->kaddr);
> > > + }
> > > +
> > > + return rc;
> > > }
> > >
> > > static void pool_op_free(struct tee_shm_pool_mgr *poolm,
> > > struct tee_shm *shm)
> > > {
> > > + if (shm->flags & TEE_SHM_DMA_BUF)
> > > + optee_shm_unregister(shm->ctx, shm);
> > > +
> > > free_pages((unsigned long)shm->kaddr, get_order(shm->size));
> > > shm->kaddr = NULL;
> > > }
> > > --
> > > 2.7.4
> > >
Hi Jens,
On Fri, 8 Nov 2019 at 16:09, Jens Wiklander <[email protected]> wrote:
>
> Hi Sumit,
>
> On Thu, Nov 7, 2019 at 1:41 PM Sumit Garg <[email protected]> wrote:
> >
> > Hi Jens,
> >
> > On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <[email protected]> wrote:
> > >
> > > Hi Sumit,
> > >
> > > On Thu, Oct 31, 2019 at 06:37:54PM +0530, Sumit Garg wrote:
> > > > In case of dynamic shared memory pool, kernel memory allocated using
> > > > dmabuf_mgr pool needs to be registered with OP-TEE prior to its usage
> > > > during optee_open_session() or optee_invoke_func().
> > > >
> > > > So fix dmabuf_mgr pool allocations via an additional call to
> > > > optee_shm_register().
> > > >
> > > > Fixes: 9733b072a12a ("optee: allow to work without static shared memory")
> > > > Signed-off-by: Sumit Garg <[email protected]>
> > >
> > > Looks good, I'm picking this up. Etienne told me has tested this on some
> > > of his hardware so I'll add:
> > > Tested-by: Etienne Carriere <[email protected]>
> > >
> >
> > Thanks for picking this up.
> >
> > >
> > > > ---
> > > >
> > > > Depends on patch: https://lkml.org/lkml/2019/7/30/506 that adds support
> > > > to allow registration of kernel buffers with OP-TEE.
> >
> > You also need to pick up the dependency patch too. The latest v3 of
> > this patch is here [1] although its unchanged from v2. Also, If you
> > pick that up I will drop it from future revisions of Trusted Keys
> > patchset [2].
> >
> > [1] https://lkml.org/lkml/2019/10/31/431
> > [2] https://lkml.org/lkml/2019/10/31/430
>
> So I missed taking [1] into account. I think that it would make more
> sense to merge this patch ("tee: optee: Fix dynamic shm pool
> allocations") with [1] ("tee: optee: allow kernel pages to register as
> shm") into a new patch.
Sure, will send v2 as merged patch instead.
-Sumit
> Cheers,
> Jens
>
> >
> > -Sumit
> >
> > > >
> > > > drivers/tee/optee/shm_pool.c | 12 +++++++++++-
> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
> > > > index de1d9b8..0332a53 100644
> > > > --- a/drivers/tee/optee/shm_pool.c
> > > > +++ b/drivers/tee/optee/shm_pool.c
> > > > @@ -17,6 +17,7 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> > > > {
> > > > unsigned int order = get_order(size);
> > > > struct page *page;
> > > > + int rc = 0;
> > > >
> > > > page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> > > > if (!page)
> > > > @@ -26,12 +27,21 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
> > > > shm->paddr = page_to_phys(page);
> > > > shm->size = PAGE_SIZE << order;
> > > >
> > > > - return 0;
> > > > + if (shm->flags & TEE_SHM_DMA_BUF) {
> > > > + shm->flags |= TEE_SHM_REGISTER;
> > > > + rc = optee_shm_register(shm->ctx, shm, &page, 1 << order,
> > > > + (unsigned long)shm->kaddr);
> > > > + }
> > > > +
> > > > + return rc;
> > > > }
> > > >
> > > > static void pool_op_free(struct tee_shm_pool_mgr *poolm,
> > > > struct tee_shm *shm)
> > > > {
> > > > + if (shm->flags & TEE_SHM_DMA_BUF)
> > > > + optee_shm_unregister(shm->ctx, shm);
> > > > +
> > > > free_pages((unsigned long)shm->kaddr, get_order(shm->size));
> > > > shm->kaddr = NULL;
> > > > }
> > > > --
> > > > 2.7.4
> > > >