diff mbox series

tee: optee: Fix dynamic shm pool allocations

Message ID 1572527274-21925-1-git-send-email-sumit.garg@linaro.org
State Superseded
Headers show
Series tee: optee: Fix dynamic shm pool allocations | expand

Commit Message

Sumit Garg Oct. 31, 2019, 1:07 p.m. UTC 
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@linaro.org>

---

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

-- 
2.7.4

Comments

Jens Wiklander Nov. 7, 2019, 11:06 a.m. UTC | #1 
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@linaro.org>


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@linaro.org>


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

>
Sumit Garg Nov. 7, 2019, 12:41 p.m. UTC | #2 
Hi Jens,

On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <jens.wiklander@linaro.org> 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@linaro.org>

>

> 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@linaro.org>

>


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

> >
Jens Wiklander Nov. 8, 2019, 10:39 a.m. UTC | #3 
Hi Sumit,

On Thu, Nov 7, 2019 at 1:41 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>

> Hi Jens,

>

> On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <jens.wiklander@linaro.org> 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@linaro.org>

> >

> > 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@linaro.org>

> >

>

> 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

> > >
Sumit Garg Nov. 8, 2019, 11:05 a.m. UTC | #4 
Hi Jens,

On Fri, 8 Nov 2019 at 16:09, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>

> Hi Sumit,

>

> On Thu, Nov 7, 2019 at 1:41 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> >

> > Hi Jens,

> >

> > On Thu, 7 Nov 2019 at 16:36, Jens Wiklander <jens.wiklander@linaro.org> 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@linaro.org>

> > >

> > > 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@linaro.org>

> > >

> >

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

> > > >
diff mbox series

Patch

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