Re: [PATCH] tee: optee: Fix dynamic shm pool allocations

[Sumit Garg]

Hi Jens,

On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <jens.wiklander@xxxxxxxxxx> 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 <sumit.garg@xxxxxxxxxx>
>
> 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 <etienne.carriere@xxxxxxxxxx>
>

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
> >