Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 15576 bytes --]

+ MM folks to seek guidance here.

On Thu, Mar 27, 2025 at 09:07:34AM +0100, Jens Wiklander wrote:
> Hi Sumit,
> 
> On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
> >
> > On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> > > Add support in the OP-TEE backend driver dynamic restricted memory
> > > allocation with FF-A.
> > >
> > > The restricted memory pools for dynamically allocated restrict memory
> > > are instantiated when requested by user-space. This instantiation can
> > > fail if OP-TEE doesn't support the requested use-case of restricted
> > > memory.
> > >
> > > Restricted memory pools based on a static carveout or dynamic allocation
> > > can coexist for different use-cases. We use only dynamic allocation with
> > > FF-A.
> > >
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > ---
> > >  drivers/tee/optee/Makefile        |   1 +
> > >  drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
> > >  drivers/tee/optee/optee_private.h |  13 +-
> > >  drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
> > >  4 files changed, 483 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/tee/optee/rstmem.c
> > >

<snip>

> > > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> > > new file mode 100644
> > > index 000000000000..ea27769934d4
> > > --- /dev/null
> > > +++ b/drivers/tee/optee/rstmem.c
> > > @@ -0,0 +1,329 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (c) 2025, Linaro Limited
> > > + */
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include <linux/errno.h>
> > > +#include <linux/genalloc.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > > +#include <linux/tee_core.h>
> > > +#include <linux/types.h>
> > > +#include "optee_private.h"
> > > +
> > > +struct optee_rstmem_cma_pool {
> > > +     struct tee_rstmem_pool pool;
> > > +     struct gen_pool *gen_pool;
> > > +     struct optee *optee;
> > > +     size_t page_count;
> > > +     u16 *end_points;
> > > +     u_int end_point_count;
> > > +     u_int align;
> > > +     refcount_t refcount;
> > > +     u32 use_case;
> > > +     struct tee_shm *rstmem;
> > > +     /* Protects when initializing and tearing down this struct */
> > > +     struct mutex mutex;
> > > +};
> > > +
> > > +static struct optee_rstmem_cma_pool *
> > > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> > > +{
> > > +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
> > > +}
> > > +
> > > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > +{
> > > +     int rc;
> > > +
> > > +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> > > +                                             rp->align);
> > > +     if (IS_ERR(rp->rstmem)) {
> > > +             rc = PTR_ERR(rp->rstmem);
> > > +             goto err_null_rstmem;
> > > +     }
> > > +
> > > +     /*
> > > +      * TODO unmap the memory range since the physical memory will
> > > +      * become inaccesible after the lend_rstmem() call.
> > > +      */
> >
> > What's your plan for this TODO? I think we need a CMA allocator here
> > which can allocate un-mapped memory such that any cache speculation
> > won't lead to CPU hangs once the memory restriction comes into picture.
> 
> What happens is platform-specific. For some platforms, it might be
> enough to avoid explicit access. Yes, a CMA allocator with unmapped
> memory or where memory can be unmapped is one option.

Did you get a chance to enable real memory protection on RockPi board?
This will atleast ensure that mapped restricted memory without explicit
access works fine. Since otherwise once people start to enable real
memory restriction in OP-TEE, there can be chances of random hang ups
due to cache speculation.

MM folks,

Basically what we are trying to achieve here is a "no-map" DT behaviour
[1] which is rather dynamic in  nature. The use-case here is that a memory
block allocated from CMA can be marked restricted at runtime where we
would like the Linux not being able to directly or indirectly (cache
speculation) access it. Once memory restriction use-case has been
completed, the memory block can be marked as normal and freed for
further CMA allocation.

It will be apprciated if you can guide us regarding the appropriate APIs
to use for un-mapping/mamping CMA allocations for this use-case.

[1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L79

-Sumit

> 
> >
> > > +     rc = rp->optee->ops->lend_rstmem(rp->optee, rp->rstmem, rp->end_points,
> > > +                                      rp->end_point_count, rp->use_case);
> > > +     if (rc)
> > > +             goto err_put_shm;
> > > +     rp->rstmem->flags |= TEE_SHM_DYNAMIC;
> > > +
> > > +     rp->gen_pool = gen_pool_create(PAGE_SHIFT, -1);
> > > +     if (!rp->gen_pool) {
> > > +             rc = -ENOMEM;
> > > +             goto err_reclaim;
> > > +     }
> > > +
> > > +     rc = gen_pool_add(rp->gen_pool, rp->rstmem->paddr,
> > > +                       rp->rstmem->size, -1);
> > > +     if (rc)
> > > +             goto err_free_pool;
> > > +
> > > +     refcount_set(&rp->refcount, 1);
> > > +     return 0;
> > > +
> > > +err_free_pool:
> > > +     gen_pool_destroy(rp->gen_pool);
> > > +     rp->gen_pool = NULL;
> > > +err_reclaim:
> > > +     rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> > > +err_put_shm:
> > > +     tee_shm_put(rp->rstmem);
> > > +err_null_rstmem:
> > > +     rp->rstmem = NULL;
> > > +     return rc;
> > > +}
> > > +
> > > +static int get_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > +{
> > > +     int rc = 0;
> > > +
> > > +     if (!refcount_inc_not_zero(&rp->refcount)) {
> > > +             mutex_lock(&rp->mutex);
> > > +             if (rp->gen_pool) {
> > > +                     /*
> > > +                      * Another thread has already initialized the pool
> > > +                      * before us, or the pool was just about to be torn
> > > +                      * down. Either way we only need to increase the
> > > +                      * refcount and we're done.
> > > +                      */
> > > +                     refcount_inc(&rp->refcount);
> > > +             } else {
> > > +                     rc = init_cma_rstmem(rp);
> > > +             }
> > > +             mutex_unlock(&rp->mutex);
> > > +     }
> > > +
> > > +     return rc;
> > > +}
> > > +
> > > +static void release_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > +{
> > > +     gen_pool_destroy(rp->gen_pool);
> > > +     rp->gen_pool = NULL;
> > > +
> > > +     rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> > > +     rp->rstmem->flags &= ~TEE_SHM_DYNAMIC;
> > > +
> > > +     WARN(refcount_read(&rp->rstmem->refcount) != 1, "Unexpected refcount");
> > > +     tee_shm_put(rp->rstmem);
> > > +     rp->rstmem = NULL;
> > > +}
> > > +
> > > +static void put_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > +{
> > > +     if (refcount_dec_and_test(&rp->refcount)) {
> > > +             mutex_lock(&rp->mutex);
> > > +             if (rp->gen_pool)
> > > +                     release_cma_rstmem(rp);
> > > +             mutex_unlock(&rp->mutex);
> > > +     }
> > > +}
> > > +
> > > +static int rstmem_pool_op_cma_alloc(struct tee_rstmem_pool *pool,
> > > +                                 struct sg_table *sgt, size_t size,
> > > +                                 size_t *offs)
> > > +{
> > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > +     size_t sz = ALIGN(size, PAGE_SIZE);
> > > +     phys_addr_t pa;
> > > +     int rc;
> > > +
> > > +     rc = get_cma_rstmem(rp);
> > > +     if (rc)
> > > +             return rc;
> > > +
> > > +     pa = gen_pool_alloc(rp->gen_pool, sz);
> > > +     if (!pa) {
> > > +             rc = -ENOMEM;
> > > +             goto err_put;
> > > +     }
> > > +
> > > +     rc = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > > +     if (rc)
> > > +             goto err_free;
> > > +
> > > +     sg_set_page(sgt->sgl, phys_to_page(pa), size, 0);
> > > +     *offs = pa - rp->rstmem->paddr;
> > > +
> > > +     return 0;
> > > +err_free:
> > > +     gen_pool_free(rp->gen_pool, pa, size);
> > > +err_put:
> > > +     put_cma_rstmem(rp);
> > > +
> > > +     return rc;
> > > +}
> > > +
> > > +static void rstmem_pool_op_cma_free(struct tee_rstmem_pool *pool,
> > > +                                 struct sg_table *sgt)
> > > +{
> > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > +     struct scatterlist *sg;
> > > +     int i;
> > > +
> > > +     for_each_sgtable_sg(sgt, sg, i)
> > > +             gen_pool_free(rp->gen_pool, sg_phys(sg), sg->length);
> > > +     sg_free_table(sgt);
> > > +     put_cma_rstmem(rp);
> > > +}
> > > +
> > > +static int rstmem_pool_op_cma_update_shm(struct tee_rstmem_pool *pool,
> > > +                                      struct sg_table *sgt, size_t offs,
> > > +                                      struct tee_shm *shm,
> > > +                                      struct tee_shm **parent_shm)
> > > +{
> > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > +
> > > +     *parent_shm = rp->rstmem;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static void pool_op_cma_destroy_pool(struct tee_rstmem_pool *pool)
> > > +{
> > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > +
> > > +     mutex_destroy(&rp->mutex);
> > > +     kfree(rp);
> > > +}
> > > +
> > > +static struct tee_rstmem_pool_ops rstmem_pool_ops_cma = {
> > > +     .alloc = rstmem_pool_op_cma_alloc,
> > > +     .free = rstmem_pool_op_cma_free,
> > > +     .update_shm = rstmem_pool_op_cma_update_shm,
> > > +     .destroy_pool = pool_op_cma_destroy_pool,
> > > +};
> > > +
> > > +static int get_rstmem_config(struct optee *optee, u32 use_case,
> > > +                          size_t *min_size, u_int *min_align,
> > > +                          u16 *end_points, u_int *ep_count)
> >
> > I guess this end points terminology is specific to FF-A ABI. Is there
> > any relevance for this in the common APIs?
> 
> Yes, endpoints are specific to FF-A ABI. The list of end-points must
> be presented to FFA_MEM_LEND. We're relying on the secure world to
> know which endpoints are needed for a specific use case.
> 
> Cheers,
> Jens
> 
> >
> > -Sumit
> >
> > > +{
> > > +     struct tee_param params[2] = {
> > > +             [0] = {
> > > +                     .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT,
> > > +                     .u.value.a = use_case,
> > > +             },
> > > +             [1] = {
> > > +                     .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT,
> > > +             },
> > > +     };
> > > +     struct optee_shm_arg_entry *entry;
> > > +     struct tee_shm *shm_param = NULL;
> > > +     struct optee_msg_arg *msg_arg;
> > > +     struct tee_shm *shm;
> > > +     u_int offs;
> > > +     int rc;
> > > +
> > > +     if (end_points && *ep_count) {
> > > +             params[1].u.memref.size = *ep_count * sizeof(*end_points);
> > > +             shm_param = tee_shm_alloc_priv_buf(optee->ctx,
> > > +                                                params[1].u.memref.size);
> > > +             if (IS_ERR(shm_param))
> > > +                     return PTR_ERR(shm_param);
> > > +             params[1].u.memref.shm = shm_param;
> > > +     }
> > > +
> > > +     msg_arg = optee_get_msg_arg(optee->ctx, ARRAY_SIZE(params), &entry,
> > > +                                 &shm, &offs);
> > > +     if (IS_ERR(msg_arg)) {
> > > +             rc = PTR_ERR(msg_arg);
> > > +             goto out_free_shm;
> > > +     }
> > > +     msg_arg->cmd = OPTEE_MSG_CMD_GET_RSTMEM_CONFIG;
> > > +
> > > +     rc = optee->ops->to_msg_param(optee, msg_arg->params,
> > > +                                   ARRAY_SIZE(params), params,
> > > +                                   false /*!update_out*/);
> > > +     if (rc)
> > > +             goto out_free_msg;
> > > +
> > > +     rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
> > > +     if (rc)
> > > +             goto out_free_msg;
> > > +     if (msg_arg->ret && msg_arg->ret != TEEC_ERROR_SHORT_BUFFER) {
> > > +             rc = -EINVAL;
> > > +             goto out_free_msg;
> > > +     }
> > > +
> > > +     rc = optee->ops->from_msg_param(optee, params, ARRAY_SIZE(params),
> > > +                                     msg_arg->params, true /*update_out*/);
> > > +     if (rc)
> > > +             goto out_free_msg;
> > > +
> > > +     if (!msg_arg->ret && end_points &&
> > > +         *ep_count < params[1].u.memref.size / sizeof(u16)) {
> > > +             rc = -EINVAL;
> > > +             goto out_free_msg;
> > > +     }
> > > +
> > > +     *min_size = params[0].u.value.a;
> > > +     *min_align = params[0].u.value.b;
> > > +     *ep_count = params[1].u.memref.size / sizeof(u16);
> > > +
> > > +     if (msg_arg->ret == TEEC_ERROR_SHORT_BUFFER) {
> > > +             rc = -ENOSPC;
> > > +             goto out_free_msg;
> > > +     }
> > > +
> > > +     if (end_points)
> > > +             memcpy(end_points, tee_shm_get_va(shm_param, 0),
> > > +                    params[1].u.memref.size);
> > > +
> > > +out_free_msg:
> > > +     optee_free_msg_arg(optee->ctx, entry, offs);
> > > +out_free_shm:
> > > +     if (shm_param)
> > > +             tee_shm_free(shm_param);
> > > +     return rc;
> > > +}
> > > +
> > > +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
> > > +                                                 enum tee_dma_heap_id id)
> > > +{
> > > +     struct optee_rstmem_cma_pool *rp;
> > > +     u32 use_case = id;
> > > +     size_t min_size;
> > > +     int rc;
> > > +
> > > +     rp = kzalloc(sizeof(*rp), GFP_KERNEL);
> > > +     if (!rp)
> > > +             return ERR_PTR(-ENOMEM);
> > > +     rp->use_case = use_case;
> > > +
> > > +     rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, NULL,
> > > +                            &rp->end_point_count);
> > > +     if (rc) {
> > > +             if (rc != -ENOSPC)
> > > +                     goto err;
> > > +             rp->end_points = kcalloc(rp->end_point_count,
> > > +                                      sizeof(*rp->end_points), GFP_KERNEL);
> > > +             if (!rp->end_points) {
> > > +                     rc = -ENOMEM;
> > > +                     goto err;
> > > +             }
> > > +             rc = get_rstmem_config(optee, use_case, &min_size, &rp->align,
> > > +                                    rp->end_points, &rp->end_point_count);
> > > +             if (rc)
> > > +                     goto err_kfree_eps;
> > > +     }
> > > +
> > > +     rp->pool.ops = &rstmem_pool_ops_cma;
> > > +     rp->optee = optee;
> > > +     rp->page_count = min_size / PAGE_SIZE;
> > > +     mutex_init(&rp->mutex);
> > > +
> > > +     return &rp->pool;
> > > +
> > > +err_kfree_eps:
> > > +     kfree(rp->end_points);
> > > +err:
> > > +     kfree(rp);
> > > +     return ERR_PTR(rc);
> > > +}
> > > --
> > > 2.43.0
> > >

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Jens Wiklander]

[-- Attachment #1: Type: text/plain, Size: 16764 bytes --]

On Tue, Apr 1, 2025 at 12:13 PM Sumit Garg <sumit.garg@kernel.org> wrote:
>
> + MM folks to seek guidance here.
>
> On Thu, Mar 27, 2025 at 09:07:34AM +0100, Jens Wiklander wrote:
> > Hi Sumit,
> >
> > On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
> > >
> > > On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> > > > Add support in the OP-TEE backend driver dynamic restricted memory
> > > > allocation with FF-A.
> > > >
> > > > The restricted memory pools for dynamically allocated restrict memory
> > > > are instantiated when requested by user-space. This instantiation can
> > > > fail if OP-TEE doesn't support the requested use-case of restricted
> > > > memory.
> > > >
> > > > Restricted memory pools based on a static carveout or dynamic allocation
> > > > can coexist for different use-cases. We use only dynamic allocation with
> > > > FF-A.
> > > >
> > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > > ---
> > > >  drivers/tee/optee/Makefile        |   1 +
> > > >  drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
> > > >  drivers/tee/optee/optee_private.h |  13 +-
> > > >  drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
> > > >  4 files changed, 483 insertions(+), 3 deletions(-)
> > > >  create mode 100644 drivers/tee/optee/rstmem.c
> > > >
>
> <snip>
>
> > > > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> > > > new file mode 100644
> > > > index 000000000000..ea27769934d4
> > > > --- /dev/null
> > > > +++ b/drivers/tee/optee/rstmem.c
> > > > @@ -0,0 +1,329 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * Copyright (c) 2025, Linaro Limited
> > > > + */
> > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > +
> > > > +#include <linux/errno.h>
> > > > +#include <linux/genalloc.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/string.h>
> > > > +#include <linux/tee_core.h>
> > > > +#include <linux/types.h>
> > > > +#include "optee_private.h"
> > > > +
> > > > +struct optee_rstmem_cma_pool {
> > > > +     struct tee_rstmem_pool pool;
> > > > +     struct gen_pool *gen_pool;
> > > > +     struct optee *optee;
> > > > +     size_t page_count;
> > > > +     u16 *end_points;
> > > > +     u_int end_point_count;
> > > > +     u_int align;
> > > > +     refcount_t refcount;
> > > > +     u32 use_case;
> > > > +     struct tee_shm *rstmem;
> > > > +     /* Protects when initializing and tearing down this struct */
> > > > +     struct mutex mutex;
> > > > +};
> > > > +
> > > > +static struct optee_rstmem_cma_pool *
> > > > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> > > > +{
> > > > +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
> > > > +}
> > > > +
> > > > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > +{
> > > > +     int rc;
> > > > +
> > > > +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> > > > +                                             rp->align);
> > > > +     if (IS_ERR(rp->rstmem)) {
> > > > +             rc = PTR_ERR(rp->rstmem);
> > > > +             goto err_null_rstmem;
> > > > +     }
> > > > +
> > > > +     /*
> > > > +      * TODO unmap the memory range since the physical memory will
> > > > +      * become inaccesible after the lend_rstmem() call.
> > > > +      */
> > >
> > > What's your plan for this TODO? I think we need a CMA allocator here
> > > which can allocate un-mapped memory such that any cache speculation
> > > won't lead to CPU hangs once the memory restriction comes into picture.
> >
> > What happens is platform-specific. For some platforms, it might be
> > enough to avoid explicit access. Yes, a CMA allocator with unmapped
> > memory or where memory can be unmapped is one option.
>
> Did you get a chance to enable real memory protection on RockPi board?

No, I don't think I have access to the needed documentation for the
board to set it up for relevant peripherals.

> This will atleast ensure that mapped restricted memory without explicit
> access works fine. Since otherwise once people start to enable real
> memory restriction in OP-TEE, there can be chances of random hang ups
> due to cache speculation.

A hypervisor in the normal world can also make the memory inaccessible
to the kernel. That shouldn't cause any hangups due to cache
speculation.

Cheers,
Jens

>
> MM folks,
>
> Basically what we are trying to achieve here is a "no-map" DT behaviour
> [1] which is rather dynamic in  nature. The use-case here is that a memory
> block allocated from CMA can be marked restricted at runtime where we
> would like the Linux not being able to directly or indirectly (cache
> speculation) access it. Once memory restriction use-case has been
> completed, the memory block can be marked as normal and freed for
> further CMA allocation.
>
> It will be apprciated if you can guide us regarding the appropriate APIs
> to use for un-mapping/mamping CMA allocations for this use-case.
>
> [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L79
>
> -Sumit
>
> >
> > >
> > > > +     rc = rp->optee->ops->lend_rstmem(rp->optee, rp->rstmem, rp->end_points,
> > > > +                                      rp->end_point_count, rp->use_case);
> > > > +     if (rc)
> > > > +             goto err_put_shm;
> > > > +     rp->rstmem->flags |= TEE_SHM_DYNAMIC;
> > > > +
> > > > +     rp->gen_pool = gen_pool_create(PAGE_SHIFT, -1);
> > > > +     if (!rp->gen_pool) {
> > > > +             rc = -ENOMEM;
> > > > +             goto err_reclaim;
> > > > +     }
> > > > +
> > > > +     rc = gen_pool_add(rp->gen_pool, rp->rstmem->paddr,
> > > > +                       rp->rstmem->size, -1);
> > > > +     if (rc)
> > > > +             goto err_free_pool;
> > > > +
> > > > +     refcount_set(&rp->refcount, 1);
> > > > +     return 0;
> > > > +
> > > > +err_free_pool:
> > > > +     gen_pool_destroy(rp->gen_pool);
> > > > +     rp->gen_pool = NULL;
> > > > +err_reclaim:
> > > > +     rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> > > > +err_put_shm:
> > > > +     tee_shm_put(rp->rstmem);
> > > > +err_null_rstmem:
> > > > +     rp->rstmem = NULL;
> > > > +     return rc;
> > > > +}
> > > > +
> > > > +static int get_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > +{
> > > > +     int rc = 0;
> > > > +
> > > > +     if (!refcount_inc_not_zero(&rp->refcount)) {
> > > > +             mutex_lock(&rp->mutex);
> > > > +             if (rp->gen_pool) {
> > > > +                     /*
> > > > +                      * Another thread has already initialized the pool
> > > > +                      * before us, or the pool was just about to be torn
> > > > +                      * down. Either way we only need to increase the
> > > > +                      * refcount and we're done.
> > > > +                      */
> > > > +                     refcount_inc(&rp->refcount);
> > > > +             } else {
> > > > +                     rc = init_cma_rstmem(rp);
> > > > +             }
> > > > +             mutex_unlock(&rp->mutex);
> > > > +     }
> > > > +
> > > > +     return rc;
> > > > +}
> > > > +
> > > > +static void release_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > +{
> > > > +     gen_pool_destroy(rp->gen_pool);
> > > > +     rp->gen_pool = NULL;
> > > > +
> > > > +     rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> > > > +     rp->rstmem->flags &= ~TEE_SHM_DYNAMIC;
> > > > +
> > > > +     WARN(refcount_read(&rp->rstmem->refcount) != 1, "Unexpected refcount");
> > > > +     tee_shm_put(rp->rstmem);
> > > > +     rp->rstmem = NULL;
> > > > +}
> > > > +
> > > > +static void put_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > +{
> > > > +     if (refcount_dec_and_test(&rp->refcount)) {
> > > > +             mutex_lock(&rp->mutex);
> > > > +             if (rp->gen_pool)
> > > > +                     release_cma_rstmem(rp);
> > > > +             mutex_unlock(&rp->mutex);
> > > > +     }
> > > > +}
> > > > +
> > > > +static int rstmem_pool_op_cma_alloc(struct tee_rstmem_pool *pool,
> > > > +                                 struct sg_table *sgt, size_t size,
> > > > +                                 size_t *offs)
> > > > +{
> > > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > > +     size_t sz = ALIGN(size, PAGE_SIZE);
> > > > +     phys_addr_t pa;
> > > > +     int rc;
> > > > +
> > > > +     rc = get_cma_rstmem(rp);
> > > > +     if (rc)
> > > > +             return rc;
> > > > +
> > > > +     pa = gen_pool_alloc(rp->gen_pool, sz);
> > > > +     if (!pa) {
> > > > +             rc = -ENOMEM;
> > > > +             goto err_put;
> > > > +     }
> > > > +
> > > > +     rc = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > > > +     if (rc)
> > > > +             goto err_free;
> > > > +
> > > > +     sg_set_page(sgt->sgl, phys_to_page(pa), size, 0);
> > > > +     *offs = pa - rp->rstmem->paddr;
> > > > +
> > > > +     return 0;
> > > > +err_free:
> > > > +     gen_pool_free(rp->gen_pool, pa, size);
> > > > +err_put:
> > > > +     put_cma_rstmem(rp);
> > > > +
> > > > +     return rc;
> > > > +}
> > > > +
> > > > +static void rstmem_pool_op_cma_free(struct tee_rstmem_pool *pool,
> > > > +                                 struct sg_table *sgt)
> > > > +{
> > > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > > +     struct scatterlist *sg;
> > > > +     int i;
> > > > +
> > > > +     for_each_sgtable_sg(sgt, sg, i)
> > > > +             gen_pool_free(rp->gen_pool, sg_phys(sg), sg->length);
> > > > +     sg_free_table(sgt);
> > > > +     put_cma_rstmem(rp);
> > > > +}
> > > > +
> > > > +static int rstmem_pool_op_cma_update_shm(struct tee_rstmem_pool *pool,
> > > > +                                      struct sg_table *sgt, size_t offs,
> > > > +                                      struct tee_shm *shm,
> > > > +                                      struct tee_shm **parent_shm)
> > > > +{
> > > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > > +
> > > > +     *parent_shm = rp->rstmem;
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static void pool_op_cma_destroy_pool(struct tee_rstmem_pool *pool)
> > > > +{
> > > > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > > > +
> > > > +     mutex_destroy(&rp->mutex);
> > > > +     kfree(rp);
> > > > +}
> > > > +
> > > > +static struct tee_rstmem_pool_ops rstmem_pool_ops_cma = {
> > > > +     .alloc = rstmem_pool_op_cma_alloc,
> > > > +     .free = rstmem_pool_op_cma_free,
> > > > +     .update_shm = rstmem_pool_op_cma_update_shm,
> > > > +     .destroy_pool = pool_op_cma_destroy_pool,
> > > > +};
> > > > +
> > > > +static int get_rstmem_config(struct optee *optee, u32 use_case,
> > > > +                          size_t *min_size, u_int *min_align,
> > > > +                          u16 *end_points, u_int *ep_count)
> > >
> > > I guess this end points terminology is specific to FF-A ABI. Is there
> > > any relevance for this in the common APIs?
> >
> > Yes, endpoints are specific to FF-A ABI. The list of end-points must
> > be presented to FFA_MEM_LEND. We're relying on the secure world to
> > know which endpoints are needed for a specific use case.
> >
> > Cheers,
> > Jens
> >
> > >
> > > -Sumit
> > >
> > > > +{
> > > > +     struct tee_param params[2] = {
> > > > +             [0] = {
> > > > +                     .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT,
> > > > +                     .u.value.a = use_case,
> > > > +             },
> > > > +             [1] = {
> > > > +                     .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT,
> > > > +             },
> > > > +     };
> > > > +     struct optee_shm_arg_entry *entry;
> > > > +     struct tee_shm *shm_param = NULL;
> > > > +     struct optee_msg_arg *msg_arg;
> > > > +     struct tee_shm *shm;
> > > > +     u_int offs;
> > > > +     int rc;
> > > > +
> > > > +     if (end_points && *ep_count) {
> > > > +             params[1].u.memref.size = *ep_count * sizeof(*end_points);
> > > > +             shm_param = tee_shm_alloc_priv_buf(optee->ctx,
> > > > +                                                params[1].u.memref.size);
> > > > +             if (IS_ERR(shm_param))
> > > > +                     return PTR_ERR(shm_param);
> > > > +             params[1].u.memref.shm = shm_param;
> > > > +     }
> > > > +
> > > > +     msg_arg = optee_get_msg_arg(optee->ctx, ARRAY_SIZE(params), &entry,
> > > > +                                 &shm, &offs);
> > > > +     if (IS_ERR(msg_arg)) {
> > > > +             rc = PTR_ERR(msg_arg);
> > > > +             goto out_free_shm;
> > > > +     }
> > > > +     msg_arg->cmd = OPTEE_MSG_CMD_GET_RSTMEM_CONFIG;
> > > > +
> > > > +     rc = optee->ops->to_msg_param(optee, msg_arg->params,
> > > > +                                   ARRAY_SIZE(params), params,
> > > > +                                   false /*!update_out*/);
> > > > +     if (rc)
> > > > +             goto out_free_msg;
> > > > +
> > > > +     rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
> > > > +     if (rc)
> > > > +             goto out_free_msg;
> > > > +     if (msg_arg->ret && msg_arg->ret != TEEC_ERROR_SHORT_BUFFER) {
> > > > +             rc = -EINVAL;
> > > > +             goto out_free_msg;
> > > > +     }
> > > > +
> > > > +     rc = optee->ops->from_msg_param(optee, params, ARRAY_SIZE(params),
> > > > +                                     msg_arg->params, true /*update_out*/);
> > > > +     if (rc)
> > > > +             goto out_free_msg;
> > > > +
> > > > +     if (!msg_arg->ret && end_points &&
> > > > +         *ep_count < params[1].u.memref.size / sizeof(u16)) {
> > > > +             rc = -EINVAL;
> > > > +             goto out_free_msg;
> > > > +     }
> > > > +
> > > > +     *min_size = params[0].u.value.a;
> > > > +     *min_align = params[0].u.value.b;
> > > > +     *ep_count = params[1].u.memref.size / sizeof(u16);
> > > > +
> > > > +     if (msg_arg->ret == TEEC_ERROR_SHORT_BUFFER) {
> > > > +             rc = -ENOSPC;
> > > > +             goto out_free_msg;
> > > > +     }
> > > > +
> > > > +     if (end_points)
> > > > +             memcpy(end_points, tee_shm_get_va(shm_param, 0),
> > > > +                    params[1].u.memref.size);
> > > > +
> > > > +out_free_msg:
> > > > +     optee_free_msg_arg(optee->ctx, entry, offs);
> > > > +out_free_shm:
> > > > +     if (shm_param)
> > > > +             tee_shm_free(shm_param);
> > > > +     return rc;
> > > > +}
> > > > +
> > > > +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
> > > > +                                                 enum tee_dma_heap_id id)
> > > > +{
> > > > +     struct optee_rstmem_cma_pool *rp;
> > > > +     u32 use_case = id;
> > > > +     size_t min_size;
> > > > +     int rc;
> > > > +
> > > > +     rp = kzalloc(sizeof(*rp), GFP_KERNEL);
> > > > +     if (!rp)
> > > > +             return ERR_PTR(-ENOMEM);
> > > > +     rp->use_case = use_case;
> > > > +
> > > > +     rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, NULL,
> > > > +                            &rp->end_point_count);
> > > > +     if (rc) {
> > > > +             if (rc != -ENOSPC)
> > > > +                     goto err;
> > > > +             rp->end_points = kcalloc(rp->end_point_count,
> > > > +                                      sizeof(*rp->end_points), GFP_KERNEL);
> > > > +             if (!rp->end_points) {
> > > > +                     rc = -ENOMEM;
> > > > +                     goto err;
> > > > +             }
> > > > +             rc = get_rstmem_config(optee, use_case, &min_size, &rp->align,
> > > > +                                    rp->end_points, &rp->end_point_count);
> > > > +             if (rc)
> > > > +                     goto err_kfree_eps;
> > > > +     }
> > > > +
> > > > +     rp->pool.ops = &rstmem_pool_ops_cma;
> > > > +     rp->optee = optee;
> > > > +     rp->page_count = min_size / PAGE_SIZE;
> > > > +     mutex_init(&rp->mutex);
> > > > +
> > > > +     return &rp->pool;
> > > > +
> > > > +err_kfree_eps:
> > > > +     kfree(rp->end_points);
> > > > +err:
> > > > +     kfree(rp);
> > > > +     return ERR_PTR(rc);
> > > > +}
> > > > --
> > > > 2.43.0
> > > >

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[David Hildenbrand]

[-- Attachment #1: Type: text/plain, Size: 5020 bytes --]

On 01.04.25 12:13, Sumit Garg wrote:
> + MM folks to seek guidance here.
> 
> On Thu, Mar 27, 2025 at 09:07:34AM +0100, Jens Wiklander wrote:
>> Hi Sumit,
>>
>> On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
>>>
>>> On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
>>>> Add support in the OP-TEE backend driver dynamic restricted memory
>>>> allocation with FF-A.
>>>>
>>>> The restricted memory pools for dynamically allocated restrict memory
>>>> are instantiated when requested by user-space. This instantiation can
>>>> fail if OP-TEE doesn't support the requested use-case of restricted
>>>> memory.
>>>>
>>>> Restricted memory pools based on a static carveout or dynamic allocation
>>>> can coexist for different use-cases. We use only dynamic allocation with
>>>> FF-A.
>>>>
>>>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>>>> ---
>>>>   drivers/tee/optee/Makefile        |   1 +
>>>>   drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
>>>>   drivers/tee/optee/optee_private.h |  13 +-
>>>>   drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
>>>>   4 files changed, 483 insertions(+), 3 deletions(-)
>>>>   create mode 100644 drivers/tee/optee/rstmem.c
>>>>
> 
> <snip>
> 
>>>> diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
>>>> new file mode 100644
>>>> index 000000000000..ea27769934d4
>>>> --- /dev/null
>>>> +++ b/drivers/tee/optee/rstmem.c
>>>> @@ -0,0 +1,329 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Copyright (c) 2025, Linaro Limited
>>>> + */
>>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>> +
>>>> +#include <linux/errno.h>
>>>> +#include <linux/genalloc.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/string.h>
>>>> +#include <linux/tee_core.h>
>>>> +#include <linux/types.h>
>>>> +#include "optee_private.h"
>>>> +
>>>> +struct optee_rstmem_cma_pool {
>>>> +     struct tee_rstmem_pool pool;
>>>> +     struct gen_pool *gen_pool;
>>>> +     struct optee *optee;
>>>> +     size_t page_count;
>>>> +     u16 *end_points;
>>>> +     u_int end_point_count;
>>>> +     u_int align;
>>>> +     refcount_t refcount;
>>>> +     u32 use_case;
>>>> +     struct tee_shm *rstmem;
>>>> +     /* Protects when initializing and tearing down this struct */
>>>> +     struct mutex mutex;
>>>> +};
>>>> +
>>>> +static struct optee_rstmem_cma_pool *
>>>> +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
>>>> +{
>>>> +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
>>>> +}
>>>> +
>>>> +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
>>>> +{
>>>> +     int rc;
>>>> +
>>>> +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
>>>> +                                             rp->align);
>>>> +     if (IS_ERR(rp->rstmem)) {
>>>> +             rc = PTR_ERR(rp->rstmem);
>>>> +             goto err_null_rstmem;
>>>> +     }
>>>> +
>>>> +     /*
>>>> +      * TODO unmap the memory range since the physical memory will
>>>> +      * become inaccesible after the lend_rstmem() call.
>>>> +      */
>>>
>>> What's your plan for this TODO? I think we need a CMA allocator here
>>> which can allocate un-mapped memory such that any cache speculation
>>> won't lead to CPU hangs once the memory restriction comes into picture.
>>
>> What happens is platform-specific. For some platforms, it might be
>> enough to avoid explicit access. Yes, a CMA allocator with unmapped
>> memory or where memory can be unmapped is one option.
> 
> Did you get a chance to enable real memory protection on RockPi board?
> This will atleast ensure that mapped restricted memory without explicit
> access works fine. Since otherwise once people start to enable real
> memory restriction in OP-TEE, there can be chances of random hang ups
> due to cache speculation.
> 
> MM folks,
> 
> Basically what we are trying to achieve here is a "no-map" DT behaviour
> [1] which is rather dynamic in  nature. The use-case here is that a memory
> block allocated from CMA can be marked restricted at runtime where we
> would like the Linux not being able to directly or indirectly (cache
> speculation) access it. Once memory restriction use-case has been
> completed, the memory block can be marked as normal and freed for
> further CMA allocation.
> 
> It will be apprciated if you can guide us regarding the appropriate APIs
> to use for un-mapping/mamping CMA allocations for this use-case.

Can we get some more information why that is even required, so we can 
decide if that is even the right thing to do? :)

Who would mark the memory block as restricted and for which purpose?

In arch/powerpc/platforms/powernv/memtrace.c we have some arch-specific 
code to remove the directmap after alloc_contig_pages(). See 
memtrace_alloc_node(). But it's very arch-specific ...

-- 
Cheers,

David / dhildenb

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 6661 bytes --]

Thanks David for your response.

On Wed, Apr 09, 2025 at 12:01:21PM +0200, David Hildenbrand wrote:
> On 01.04.25 12:13, Sumit Garg wrote:
> > + MM folks to seek guidance here.
> > 
> > On Thu, Mar 27, 2025 at 09:07:34AM +0100, Jens Wiklander wrote:
> > > Hi Sumit,
> > > 
> > > On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > 
> > > > On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> > > > > Add support in the OP-TEE backend driver dynamic restricted memory
> > > > > allocation with FF-A.
> > > > > 
> > > > > The restricted memory pools for dynamically allocated restrict memory
> > > > > are instantiated when requested by user-space. This instantiation can
> > > > > fail if OP-TEE doesn't support the requested use-case of restricted
> > > > > memory.
> > > > > 
> > > > > Restricted memory pools based on a static carveout or dynamic allocation
> > > > > can coexist for different use-cases. We use only dynamic allocation with
> > > > > FF-A.
> > > > > 
> > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > ---
> > > > >   drivers/tee/optee/Makefile        |   1 +
> > > > >   drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
> > > > >   drivers/tee/optee/optee_private.h |  13 +-
> > > > >   drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
> > > > >   4 files changed, 483 insertions(+), 3 deletions(-)
> > > > >   create mode 100644 drivers/tee/optee/rstmem.c
> > > > > 
> > 
> > <snip>
> > 
> > > > > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> > > > > new file mode 100644
> > > > > index 000000000000..ea27769934d4
> > > > > --- /dev/null
> > > > > +++ b/drivers/tee/optee/rstmem.c
> > > > > @@ -0,0 +1,329 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * Copyright (c) 2025, Linaro Limited
> > > > > + */
> > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > +
> > > > > +#include <linux/errno.h>
> > > > > +#include <linux/genalloc.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/string.h>
> > > > > +#include <linux/tee_core.h>
> > > > > +#include <linux/types.h>
> > > > > +#include "optee_private.h"
> > > > > +
> > > > > +struct optee_rstmem_cma_pool {
> > > > > +     struct tee_rstmem_pool pool;
> > > > > +     struct gen_pool *gen_pool;
> > > > > +     struct optee *optee;
> > > > > +     size_t page_count;
> > > > > +     u16 *end_points;
> > > > > +     u_int end_point_count;
> > > > > +     u_int align;
> > > > > +     refcount_t refcount;
> > > > > +     u32 use_case;
> > > > > +     struct tee_shm *rstmem;
> > > > > +     /* Protects when initializing and tearing down this struct */
> > > > > +     struct mutex mutex;
> > > > > +};
> > > > > +
> > > > > +static struct optee_rstmem_cma_pool *
> > > > > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> > > > > +{
> > > > > +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
> > > > > +}
> > > > > +
> > > > > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > > +{
> > > > > +     int rc;
> > > > > +
> > > > > +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> > > > > +                                             rp->align);
> > > > > +     if (IS_ERR(rp->rstmem)) {
> > > > > +             rc = PTR_ERR(rp->rstmem);
> > > > > +             goto err_null_rstmem;
> > > > > +     }
> > > > > +
> > > > > +     /*
> > > > > +      * TODO unmap the memory range since the physical memory will
> > > > > +      * become inaccesible after the lend_rstmem() call.
> > > > > +      */
> > > > 
> > > > What's your plan for this TODO? I think we need a CMA allocator here
> > > > which can allocate un-mapped memory such that any cache speculation
> > > > won't lead to CPU hangs once the memory restriction comes into picture.
> > > 
> > > What happens is platform-specific. For some platforms, it might be
> > > enough to avoid explicit access. Yes, a CMA allocator with unmapped
> > > memory or where memory can be unmapped is one option.
> > 
> > Did you get a chance to enable real memory protection on RockPi board?
> > This will atleast ensure that mapped restricted memory without explicit
> > access works fine. Since otherwise once people start to enable real
> > memory restriction in OP-TEE, there can be chances of random hang ups
> > due to cache speculation.
> > 
> > MM folks,
> > 
> > Basically what we are trying to achieve here is a "no-map" DT behaviour
> > [1] which is rather dynamic in  nature. The use-case here is that a memory
> > block allocated from CMA can be marked restricted at runtime where we
> > would like the Linux not being able to directly or indirectly (cache
> > speculation) access it. Once memory restriction use-case has been
> > completed, the memory block can be marked as normal and freed for
> > further CMA allocation.
> > 
> > It will be apprciated if you can guide us regarding the appropriate APIs
> > to use for un-mapping/mamping CMA allocations for this use-case.
> 
> Can we get some more information why that is even required, so we can decide
> if that is even the right thing to do? :)

The main reason which I can see is for memory re-use. Although we should
be able to carve out memory during boot and then mark it restricted for
the entire boot cycle but without re-use. Especially for secure media
pipeline use-case where the video buffers can be sufficiently large
enough which will benefit from memory re-use.

> 
> Who would mark the memory block as restricted and for which purpose?

It will be the higher privileged firmware/Trusted OS which can either be
the running on same CPU with higher privileges like Arm TrustZone or a
separate co-processor like AMD-TEE etc. The purpose is for secure media
pipeline, trusted UI or secure crypto use-cases where essentially the
motivation is that the Linux kernel shouldn't be able to access
decrypted content or key material in plain format but rather only the
allowed peripherals like media pipeline, crypto accelerators etc. able to
access them.

> 
> In arch/powerpc/platforms/powernv/memtrace.c we have some arch-specific code
> to remove the directmap after alloc_contig_pages(). See
> memtrace_alloc_node(). But it's very arch-specific ...

Thanks for the reference, we are looking for something like that but
with generic code along with capability to remap when the restricted
memory block is freed and available for normal kernel usage.

-Sumit

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 6036 bytes --]

On Tue, Apr 01, 2025 at 02:26:59PM +0200, Jens Wiklander wrote:
> On Tue, Apr 1, 2025 at 12:13 PM Sumit Garg <sumit.garg@kernel.org> wrote:
> >
> > + MM folks to seek guidance here.
> >
> > On Thu, Mar 27, 2025 at 09:07:34AM +0100, Jens Wiklander wrote:
> > > Hi Sumit,
> > >
> > > On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
> > > >
> > > > On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> > > > > Add support in the OP-TEE backend driver dynamic restricted memory
> > > > > allocation with FF-A.
> > > > >
> > > > > The restricted memory pools for dynamically allocated restrict memory
> > > > > are instantiated when requested by user-space. This instantiation can
> > > > > fail if OP-TEE doesn't support the requested use-case of restricted
> > > > > memory.
> > > > >
> > > > > Restricted memory pools based on a static carveout or dynamic allocation
> > > > > can coexist for different use-cases. We use only dynamic allocation with
> > > > > FF-A.
> > > > >
> > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > ---
> > > > >  drivers/tee/optee/Makefile        |   1 +
> > > > >  drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
> > > > >  drivers/tee/optee/optee_private.h |  13 +-
> > > > >  drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
> > > > >  4 files changed, 483 insertions(+), 3 deletions(-)
> > > > >  create mode 100644 drivers/tee/optee/rstmem.c
> > > > >
> >
> > <snip>
> >
> > > > > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> > > > > new file mode 100644
> > > > > index 000000000000..ea27769934d4
> > > > > --- /dev/null
> > > > > +++ b/drivers/tee/optee/rstmem.c
> > > > > @@ -0,0 +1,329 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * Copyright (c) 2025, Linaro Limited
> > > > > + */
> > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > +
> > > > > +#include <linux/errno.h>
> > > > > +#include <linux/genalloc.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/string.h>
> > > > > +#include <linux/tee_core.h>
> > > > > +#include <linux/types.h>
> > > > > +#include "optee_private.h"
> > > > > +
> > > > > +struct optee_rstmem_cma_pool {
> > > > > +     struct tee_rstmem_pool pool;
> > > > > +     struct gen_pool *gen_pool;
> > > > > +     struct optee *optee;
> > > > > +     size_t page_count;
> > > > > +     u16 *end_points;
> > > > > +     u_int end_point_count;
> > > > > +     u_int align;
> > > > > +     refcount_t refcount;
> > > > > +     u32 use_case;
> > > > > +     struct tee_shm *rstmem;
> > > > > +     /* Protects when initializing and tearing down this struct */
> > > > > +     struct mutex mutex;
> > > > > +};
> > > > > +
> > > > > +static struct optee_rstmem_cma_pool *
> > > > > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> > > > > +{
> > > > > +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
> > > > > +}
> > > > > +
> > > > > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > > +{
> > > > > +     int rc;
> > > > > +
> > > > > +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> > > > > +                                             rp->align);
> > > > > +     if (IS_ERR(rp->rstmem)) {
> > > > > +             rc = PTR_ERR(rp->rstmem);
> > > > > +             goto err_null_rstmem;
> > > > > +     }
> > > > > +
> > > > > +     /*
> > > > > +      * TODO unmap the memory range since the physical memory will
> > > > > +      * become inaccesible after the lend_rstmem() call.
> > > > > +      */
> > > >
> > > > What's your plan for this TODO? I think we need a CMA allocator here
> > > > which can allocate un-mapped memory such that any cache speculation
> > > > won't lead to CPU hangs once the memory restriction comes into picture.
> > >
> > > What happens is platform-specific. For some platforms, it might be
> > > enough to avoid explicit access. Yes, a CMA allocator with unmapped
> > > memory or where memory can be unmapped is one option.
> >
> > Did you get a chance to enable real memory protection on RockPi board?
> 
> No, I don't think I have access to the needed documentation for the
> board to set it up for relevant peripherals.
> 
> > This will atleast ensure that mapped restricted memory without explicit
> > access works fine. Since otherwise once people start to enable real
> > memory restriction in OP-TEE, there can be chances of random hang ups
> > due to cache speculation.
> 
> A hypervisor in the normal world can also make the memory inaccessible
> to the kernel. That shouldn't cause any hangups due to cache
> speculation.

The hypervisor should unmap the memory from EL2 translation tables which
I think should disallow the cache speculation to take place. However,
without hypervisor here the memory remains mapped in normal world which
can lead to cache speculation for restricted buffers. That's why we
should atleast test on one platform with real memory protection enabled
to rule out any assumptions we make.

-Sumit

> 
> Cheers,
> Jens
> 
> >
> > MM folks,
> >
> > Basically what we are trying to achieve here is a "no-map" DT behaviour
> > [1] which is rather dynamic in  nature. The use-case here is that a memory
> > block allocated from CMA can be marked restricted at runtime where we
> > would like the Linux not being able to directly or indirectly (cache
> > speculation) access it. Once memory restriction use-case has been
> > completed, the memory block can be marked as normal and freed for
> > further CMA allocation.
> >
> > It will be apprciated if you can guide us regarding the appropriate APIs
> > to use for un-mapping/mamping CMA allocations for this use-case.
> >
> > [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L79
> >
> > -Sumit

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Jens Wiklander]

[-- Attachment #1: Type: text/plain, Size: 6692 bytes --]

On Tue, Apr 8, 2025 at 11:20 AM Sumit Garg <sumit.garg@kernel.org> wrote:
>
> On Tue, Apr 01, 2025 at 02:26:59PM +0200, Jens Wiklander wrote:
> > On Tue, Apr 1, 2025 at 12:13 PM Sumit Garg <sumit.garg@kernel.org> wrote:
> > >
> > > + MM folks to seek guidance here.
> > >
> > > On Thu, Mar 27, 2025 at 09:07:34AM +0100, Jens Wiklander wrote:
> > > > Hi Sumit,
> > > >
> > > > On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
> > > > >
> > > > > On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> > > > > > Add support in the OP-TEE backend driver dynamic restricted memory
> > > > > > allocation with FF-A.
> > > > > >
> > > > > > The restricted memory pools for dynamically allocated restrict memory
> > > > > > are instantiated when requested by user-space. This instantiation can
> > > > > > fail if OP-TEE doesn't support the requested use-case of restricted
> > > > > > memory.
> > > > > >
> > > > > > Restricted memory pools based on a static carveout or dynamic allocation
> > > > > > can coexist for different use-cases. We use only dynamic allocation with
> > > > > > FF-A.
> > > > > >
> > > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > > ---
> > > > > >  drivers/tee/optee/Makefile        |   1 +
> > > > > >  drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
> > > > > >  drivers/tee/optee/optee_private.h |  13 +-
> > > > > >  drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
> > > > > >  4 files changed, 483 insertions(+), 3 deletions(-)
> > > > > >  create mode 100644 drivers/tee/optee/rstmem.c
> > > > > >
> > >
> > > <snip>
> > >
> > > > > > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..ea27769934d4
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/tee/optee/rstmem.c
> > > > > > @@ -0,0 +1,329 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > +/*
> > > > > > + * Copyright (c) 2025, Linaro Limited
> > > > > > + */
> > > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > > +
> > > > > > +#include <linux/errno.h>
> > > > > > +#include <linux/genalloc.h>
> > > > > > +#include <linux/slab.h>
> > > > > > +#include <linux/string.h>
> > > > > > +#include <linux/tee_core.h>
> > > > > > +#include <linux/types.h>
> > > > > > +#include "optee_private.h"
> > > > > > +
> > > > > > +struct optee_rstmem_cma_pool {
> > > > > > +     struct tee_rstmem_pool pool;
> > > > > > +     struct gen_pool *gen_pool;
> > > > > > +     struct optee *optee;
> > > > > > +     size_t page_count;
> > > > > > +     u16 *end_points;
> > > > > > +     u_int end_point_count;
> > > > > > +     u_int align;
> > > > > > +     refcount_t refcount;
> > > > > > +     u32 use_case;
> > > > > > +     struct tee_shm *rstmem;
> > > > > > +     /* Protects when initializing and tearing down this struct */
> > > > > > +     struct mutex mutex;
> > > > > > +};
> > > > > > +
> > > > > > +static struct optee_rstmem_cma_pool *
> > > > > > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> > > > > > +{
> > > > > > +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
> > > > > > +}
> > > > > > +
> > > > > > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > > > > > +{
> > > > > > +     int rc;
> > > > > > +
> > > > > > +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> > > > > > +                                             rp->align);
> > > > > > +     if (IS_ERR(rp->rstmem)) {
> > > > > > +             rc = PTR_ERR(rp->rstmem);
> > > > > > +             goto err_null_rstmem;
> > > > > > +     }
> > > > > > +
> > > > > > +     /*
> > > > > > +      * TODO unmap the memory range since the physical memory will
> > > > > > +      * become inaccesible after the lend_rstmem() call.
> > > > > > +      */
> > > > >
> > > > > What's your plan for this TODO? I think we need a CMA allocator here
> > > > > which can allocate un-mapped memory such that any cache speculation
> > > > > won't lead to CPU hangs once the memory restriction comes into picture.
> > > >
> > > > What happens is platform-specific. For some platforms, it might be
> > > > enough to avoid explicit access. Yes, a CMA allocator with unmapped
> > > > memory or where memory can be unmapped is one option.
> > >
> > > Did you get a chance to enable real memory protection on RockPi board?
> >
> > No, I don't think I have access to the needed documentation for the
> > board to set it up for relevant peripherals.
> >
> > > This will atleast ensure that mapped restricted memory without explicit
> > > access works fine. Since otherwise once people start to enable real
> > > memory restriction in OP-TEE, there can be chances of random hang ups
> > > due to cache speculation.
> >
> > A hypervisor in the normal world can also make the memory inaccessible
> > to the kernel. That shouldn't cause any hangups due to cache
> > speculation.
>
> The hypervisor should unmap the memory from EL2 translation tables which
> I think should disallow the cache speculation to take place. However,
> without hypervisor here the memory remains mapped in normal world which
> can lead to cache speculation for restricted buffers. That's why we
> should atleast test on one platform with real memory protection enabled
> to rule out any assumptions we make.

Do I hear a volunteer? ;-)

Anyway, this isn't something that can be enabled in the kernel alone.
Only platforms where the firmware has been updated will be affected.
If this can't be supported on a particular platform, there's still the
option with a static carveout.

Cheers,
Jens

>
> -Sumit
>
> >
> > Cheers,
> > Jens
> >
> > >
> > > MM folks,
> > >
> > > Basically what we are trying to achieve here is a "no-map" DT behaviour
> > > [1] which is rather dynamic in  nature. The use-case here is that a memory
> > > block allocated from CMA can be marked restricted at runtime where we
> > > would like the Linux not being able to directly or indirectly (cache
> > > speculation) access it. Once memory restriction use-case has been
> > > completed, the memory block can be marked as normal and freed for
> > > further CMA allocation.
> > >
> > > It will be apprciated if you can guide us regarding the appropriate APIs
> > > to use for un-mapping/mamping CMA allocations for this use-case.
> > >
> > > [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L79
> > >
> > > -Sumit

[PATCH v6 00/10] TEE subsystem for restricted dma-buf allocations

[Jens Wiklander]

[-- Attachment #1: Type: text/plain, Size: 7355 bytes --]

Hi,

This patch set allocates the restricted DMA-bufs from a DMA-heap
instantiated from the TEE subsystem.

The TEE subsystem handles the DMA-buf allocations since it is the TEE
(OP-TEE, AMD-TEE, TS-TEE, or perhaps a future QTEE) which sets up the
restrictions for the memory used for the DMA-bufs.

The DMA-heap uses a restricted memory pool provided by the backend TEE
driver, allowing it to choose how to allocate the restricted physical
memory.

The allocated DMA-bufs must be imported with a new TEE_IOC_SHM_REGISTER_FD
before they can be passed as arguments when requesting services from the
secure world.

Three use-cases (Secure Video Playback, Trusted UI, and Secure Video
Recording) has been identified so far to serve as examples of what can be
expected. The use-cases has predefined DMA-heap names,
"restricted,secure-video", "restricted,trusted-ui", and
"restricted,secure-video-record". The backend driver registers restricted
memory pools for the use-cases it supports.

Each use-case has it's own restricted memory pool since different use-cases
requires isolation from different parts of the system. A restricted memory
pool can be based on a static carveout instantiated while probing the TEE
backend driver, or dynamically allocated from CMA and made restricted as
needed by the TEE.

This can be tested on a RockPi 4B+ with the following steps:
repo init -u https://github.com/jenswi-linaro/manifest.git -m rockpi4.xml \
        -b prototype/sdp-v6
repo sync -j8
cd build
make toolchains -j$(nproc)
make all -j$(nproc)
# Copy ../out/rockpi4.img to an SD card and boot the RockPi from that
# Connect a monitor to the RockPi
# login and at the prompt:
gst-launch-1.0 videotestsrc ! \
        aesenc key=1f9423681beb9a79215820f6bda73d0f \
                iv=e9aa8e834d8d70b7e0d254ff670dd718 serialize-iv=true ! \
        aesdec key=1f9423681beb9a79215820f6bda73d0f ! \
        kmssink

The aesdec module has been hacked to use an OP-TEE TA to decrypt the stream
into restricted DMA-bufs which are consumed by the kmssink.

The primitive QEMU tests from previous patch set can be tested on RockPi
in the same way with:
xtest --sdp-basic

The primitive test are tested on QEMU with the following steps:
repo init -u https://github.com/jenswi-linaro/manifest.git -m qemu_v8.xml \
        -b prototype/sdp-v6
repo sync -j8
cd build
make toolchains -j$(nproc)
make SPMC_AT_EL=1 all -j$(nproc)
make SPMC_AT_EL=1 run-only
# login and at the prompt:
xtest --sdp-basic

The SPMC_AT_EL=1 parameter configures the build with FF-A and an SPMC at
S-EL1 inside OP-TEE. The parameter can be changed into SPMC_AT_EL=n to test
without FF-A using the original SMC ABI instead. Please remember to do
%rm -rf ../trusted-firmware-a/build/qemu
for TF-A to be rebuilt properly using the new configuration.

https://optee.readthedocs.io/en/latest/building/prerequisites.html
list dependencies needed to build the above.

The tests are pretty basic, mostly checking that a Trusted Application in
the secure world can access and manipulate the memory. There are also some
negative tests for out of bounds buffers etc.

Thanks,
Jens

Changes since V5:
* Removing "tee: add restricted memory allocation" and
  "tee: add TEE_IOC_RSTMEM_FD_INFO"
* Adding "tee: implement restricted DMA-heap",
  "tee: new ioctl to a register tee_shm from a dmabuf file descriptor",
  "tee: add tee_shm_alloc_cma_phys_mem()",
  "optee: pass parent device to tee_device_alloc()", and
  "tee: tee_device_alloc(): copy dma_mask from parent device"
* The two TEE driver OPs "rstmem_alloc()" and "rstmem_free()" are replaced
  with a struct tee_rstmem_pool abstraction.
* Replaced the the TEE_IOC_RSTMEM_ALLOC user space API with the DMA-heap API

Changes since V4:
* Adding the patch "tee: add TEE_IOC_RSTMEM_FD_INFO" needed by the
  GStreamer demo
* Removing the dummy CPU access and mmap functions from the dma_buf_ops
* Fixing a compile error in "optee: FF-A: dynamic restricted memory allocation"
  reported by kernel test robot <lkp@intel.com>

Changes since V3:
* Make the use_case and flags field in struct tee_shm u32's instead of
  u16's
* Add more description for TEE_IOC_RSTMEM_ALLOC in the header file
* Import namespace DMA_BUF in module tee, reported by lkp(a)intel.com
* Added a note in the commit message for "optee: account for direction
  while converting parameters" why it's needed
* Factor out dynamic restricted memory allocation from
  "optee: support restricted memory allocation" into two new commits
  "optee: FF-A: dynamic restricted memory allocation" and
  "optee: smc abi: dynamic restricted memory allocation"
* Guard CMA usage with #ifdef CONFIG_CMA, effectively disabling dynamic
  restricted memory allocate if CMA isn't configured

Changes since the V2 RFC:
* Based on v6.12
* Replaced the flags for SVP and Trusted UID memory with a u32 field with
  unique id for each use case
* Added dynamic allocation of restricted memory pools
* Added OP-TEE ABI both with and without FF-A for dynamic restricted memory
* Added support for FF-A with FFA_LEND

Changes since the V1 RFC:
* Based on v6.11
* Complete rewrite, replacing the restricted heap with TEE_IOC_RSTMEM_ALLOC

Changes since Olivier's post [2]:
* Based on Yong Wu's post [1] where much of dma-buf handling is done in
  the generic restricted heap
* Simplifications and cleanup
* New commit message for "dma-buf: heaps: add Linaro restricted dmabuf heap
  support"
* Replaced the word "secure" with "restricted" where applicable

Etienne Carriere (1):
  tee: new ioctl to a register tee_shm from a dmabuf file descriptor

Jens Wiklander (9):
  tee: tee_device_alloc(): copy dma_mask from parent device
  optee: pass parent device to tee_device_alloc()
  optee: account for direction while converting parameters
  optee: sync secure world ABI headers
  tee: implement restricted DMA-heap
  tee: add tee_shm_alloc_cma_phys_mem()
  optee: support restricted memory allocation
  optee: FF-A: dynamic restricted memory allocation
  optee: smc abi: dynamic restricted memory allocation

 drivers/tee/Makefile              |   1 +
 drivers/tee/optee/Makefile        |   1 +
 drivers/tee/optee/call.c          |  10 +-
 drivers/tee/optee/core.c          |   1 +
 drivers/tee/optee/ffa_abi.c       | 194 +++++++++++-
 drivers/tee/optee/optee_ffa.h     |  27 +-
 drivers/tee/optee/optee_msg.h     |  65 ++++-
 drivers/tee/optee/optee_private.h |  55 +++-
 drivers/tee/optee/optee_smc.h     |  71 ++++-
 drivers/tee/optee/rpc.c           |  31 +-
 drivers/tee/optee/rstmem.c        | 329 +++++++++++++++++++++
 drivers/tee/optee/smc_abi.c       | 190 ++++++++++--
 drivers/tee/tee_core.c            | 147 +++++++---
 drivers/tee/tee_heap.c            | 470 ++++++++++++++++++++++++++++++
 drivers/tee/tee_private.h         |   7 +
 drivers/tee/tee_shm.c             | 199 ++++++++++++-
 include/linux/tee_core.h          |  67 +++++
 include/linux/tee_drv.h           |  10 +
 include/uapi/linux/tee.h          |  29 ++
 19 files changed, 1781 insertions(+), 123 deletions(-)
 create mode 100644 drivers/tee/optee/rstmem.c
 create mode 100644 drivers/tee/tee_heap.c


base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
-- 
2.43.0

[PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Jens Wiklander]

[-- Attachment #1: Type: text/plain, Size: 17092 bytes --]

Add support in the OP-TEE backend driver dynamic restricted memory
allocation with FF-A.

The restricted memory pools for dynamically allocated restrict memory
are instantiated when requested by user-space. This instantiation can
fail if OP-TEE doesn't support the requested use-case of restricted
memory.

Restricted memory pools based on a static carveout or dynamic allocation
can coexist for different use-cases. We use only dynamic allocation with
FF-A.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 drivers/tee/optee/Makefile        |   1 +
 drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
 drivers/tee/optee/optee_private.h |  13 +-
 drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
 4 files changed, 483 insertions(+), 3 deletions(-)
 create mode 100644 drivers/tee/optee/rstmem.c

diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
index a6eff388d300..498969fb8e40 100644
--- a/drivers/tee/optee/Makefile
+++ b/drivers/tee/optee/Makefile
@@ -4,6 +4,7 @@ optee-objs += core.o
 optee-objs += call.o
 optee-objs += notif.o
 optee-objs += rpc.o
+optee-objs += rstmem.o
 optee-objs += supp.o
 optee-objs += device.o
 optee-objs += smc_abi.o
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
index e4b08cd195f3..6a55114232ef 100644
--- a/drivers/tee/optee/ffa_abi.c
+++ b/drivers/tee/optee/ffa_abi.c
@@ -672,6 +672,123 @@ static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
 	return optee_ffa_yielding_call(ctx, &data, rpc_arg, system_thread);
 }
 
+static int do_call_lend_rstmem(struct optee *optee, u64 cookie, u32 use_case)
+{
+	struct optee_shm_arg_entry *entry;
+	struct optee_msg_arg *msg_arg;
+	struct tee_shm *shm;
+	u_int offs;
+	int rc;
+
+	msg_arg = optee_get_msg_arg(optee->ctx, 1, &entry, &shm, &offs);
+	if (IS_ERR(msg_arg))
+		return PTR_ERR(msg_arg);
+
+	msg_arg->cmd = OPTEE_MSG_CMD_ASSIGN_RSTMEM;
+	msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT;
+	msg_arg->params[0].u.value.a = cookie;
+	msg_arg->params[0].u.value.b = use_case;
+
+	rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
+	if (rc)
+		goto out;
+	if (msg_arg->ret != TEEC_SUCCESS) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+out:
+	optee_free_msg_arg(optee->ctx, entry, offs);
+	return rc;
+}
+
+static int optee_ffa_lend_rstmem(struct optee *optee, struct tee_shm *rstmem,
+				 u16 *end_points, unsigned int ep_count,
+				 u32 use_case)
+{
+	struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
+	const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops;
+	const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops;
+	struct ffa_send_direct_data data;
+	struct ffa_mem_region_attributes *mem_attr;
+	struct ffa_mem_ops_args args = {
+		.use_txbuf = true,
+		.tag = use_case,
+	};
+	struct page *page;
+	struct scatterlist sgl;
+	unsigned int n;
+	int rc;
+
+	mem_attr = kcalloc(ep_count, sizeof(*mem_attr), GFP_KERNEL);
+	for (n = 0; n < ep_count; n++) {
+		mem_attr[n].receiver = end_points[n];
+		mem_attr[n].attrs = FFA_MEM_RW;
+	}
+	args.attrs = mem_attr;
+	args.nattrs = ep_count;
+
+	page = phys_to_page(rstmem->paddr);
+	sg_init_table(&sgl, 1);
+	sg_set_page(&sgl, page, rstmem->size, 0);
+
+	args.sg = &sgl;
+	rc = mem_ops->memory_lend(&args);
+	kfree(mem_attr);
+	if (rc)
+		return rc;
+
+	rc = do_call_lend_rstmem(optee, args.g_handle, use_case);
+	if (rc)
+		goto err_reclaim;
+
+	rc = optee_shm_add_ffa_handle(optee, rstmem, args.g_handle);
+	if (rc)
+		goto err_unreg;
+
+	rstmem->sec_world_id = args.g_handle;
+
+	return 0;
+
+err_unreg:
+	data = (struct ffa_send_direct_data){
+		.data0 = OPTEE_FFA_RELEASE_RSTMEM,
+		.data1 = (u32)args.g_handle,
+		.data2 = (u32)(args.g_handle >> 32),
+	};
+	msg_ops->sync_send_receive(ffa_dev, &data);
+err_reclaim:
+	mem_ops->memory_reclaim(args.g_handle, 0);
+	return rc;
+}
+
+static int optee_ffa_reclaim_rstmem(struct optee *optee, struct tee_shm *rstmem)
+{
+	struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
+	const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops;
+	const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops;
+	u64 global_handle = rstmem->sec_world_id;
+	struct ffa_send_direct_data data = {
+		.data0 = OPTEE_FFA_RELEASE_RSTMEM,
+		.data1 = (u32)global_handle,
+		.data2 = (u32)(global_handle >> 32)
+	};
+	int rc;
+
+	optee_shm_rem_ffa_handle(optee, global_handle);
+	rstmem->sec_world_id = 0;
+
+	rc = msg_ops->sync_send_receive(ffa_dev, &data);
+	if (rc)
+		pr_err("Release SHM id 0x%llx rc %d\n", global_handle, rc);
+
+	rc = mem_ops->memory_reclaim(global_handle, 0);
+	if (rc)
+		pr_err("mem_reclaim: 0x%llx %d", global_handle, rc);
+
+	return rc;
+}
+
 /*
  * 6. Driver initialization
  *
@@ -833,6 +950,8 @@ static const struct optee_ops optee_ffa_ops = {
 	.do_call_with_arg = optee_ffa_do_call_with_arg,
 	.to_msg_param = optee_ffa_to_msg_param,
 	.from_msg_param = optee_ffa_from_msg_param,
+	.lend_rstmem = optee_ffa_lend_rstmem,
+	.reclaim_rstmem = optee_ffa_reclaim_rstmem,
 };
 
 static void optee_ffa_remove(struct ffa_device *ffa_dev)
@@ -941,7 +1060,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
 				  optee->pool, optee);
 	if (IS_ERR(teedev)) {
 		rc = PTR_ERR(teedev);
-		goto err_free_pool;
+		goto err_free_shm_pool;
 	}
 	optee->teedev = teedev;
 
@@ -988,6 +1107,24 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
 			       rc);
 	}
 
+	if (IS_ENABLED(CONFIG_CMA) && !IS_MODULE(CONFIG_OPTEE) &&
+	    (sec_caps & OPTEE_FFA_SEC_CAP_RSTMEM)) {
+		enum tee_dma_heap_id id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY;
+		struct tee_rstmem_pool *pool;
+
+		pool = optee_rstmem_alloc_cma_pool(optee, id);
+		if (IS_ERR(pool)) {
+			rc = PTR_ERR(pool);
+			goto err_notif_uninit;
+		}
+
+		rc = tee_device_register_dma_heap(optee->teedev, id, pool);
+		if (rc) {
+			pool->ops->destroy_pool(pool);
+			goto err_notif_uninit;
+		}
+	}
+
 	rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
 	if (rc)
 		goto err_unregister_devices;
@@ -1001,6 +1138,8 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
 
 err_unregister_devices:
 	optee_unregister_devices();
+	tee_device_unregister_all_dma_heaps(optee->teedev);
+err_notif_uninit:
 	if (optee->ffa.bottom_half_value != U32_MAX)
 		notif_ops->notify_relinquish(ffa_dev,
 					     optee->ffa.bottom_half_value);
@@ -1018,7 +1157,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
 	tee_device_unregister(optee->supp_teedev);
 err_unreg_teedev:
 	tee_device_unregister(optee->teedev);
-err_free_pool:
+err_free_shm_pool:
 	tee_shm_pool_free(pool);
 err_free_optee:
 	kfree(optee);
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 20eda508dbac..faab31ad7c52 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -174,9 +174,14 @@ struct optee;
  * @do_call_with_arg:	enters OP-TEE in secure world
  * @to_msg_param:	converts from struct tee_param to OPTEE_MSG parameters
  * @from_msg_param:	converts from OPTEE_MSG parameters to struct tee_param
+ * @lend_rstmem:	lends physically contiguous memory as restricted
+ *			memory, inaccessible by the kernel
+ * @reclaim_rstmem:	reclaims restricted memory previously lent with
+ *			@lend_rstmem() and makes it accessible by the
+ *			kernel again
  *
  * These OPs are only supposed to be used internally in the OP-TEE driver
- * as a way of abstracting the different methogs of entering OP-TEE in
+ * as a way of abstracting the different methods of entering OP-TEE in
  * secure world.
  */
 struct optee_ops {
@@ -191,6 +196,10 @@ struct optee_ops {
 			      size_t num_params,
 			      const struct optee_msg_param *msg_params,
 			      bool update_out);
+	int (*lend_rstmem)(struct optee *optee, struct tee_shm *rstmem,
+			   u16 *end_points, unsigned int ep_count,
+			   u32 use_case);
+	int (*reclaim_rstmem)(struct optee *optee, struct tee_shm *rstmem);
 };
 
 /**
@@ -285,6 +294,8 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
 void optee_supp_init(struct optee_supp *supp);
 void optee_supp_uninit(struct optee_supp *supp);
 void optee_supp_release(struct optee_supp *supp);
+struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
+						    enum tee_dma_heap_id id);
 
 int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params,
 		    struct tee_param *param);
diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
new file mode 100644
index 000000000000..ea27769934d4
--- /dev/null
+++ b/drivers/tee/optee/rstmem.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2025, Linaro Limited
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/errno.h>
+#include <linux/genalloc.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/tee_core.h>
+#include <linux/types.h>
+#include "optee_private.h"
+
+struct optee_rstmem_cma_pool {
+	struct tee_rstmem_pool pool;
+	struct gen_pool *gen_pool;
+	struct optee *optee;
+	size_t page_count;
+	u16 *end_points;
+	u_int end_point_count;
+	u_int align;
+	refcount_t refcount;
+	u32 use_case;
+	struct tee_shm *rstmem;
+	/* Protects when initializing and tearing down this struct */
+	struct mutex mutex;
+};
+
+static struct optee_rstmem_cma_pool *
+to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
+{
+	return container_of(pool, struct optee_rstmem_cma_pool, pool);
+}
+
+static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
+{
+	int rc;
+
+	rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
+						rp->align);
+	if (IS_ERR(rp->rstmem)) {
+		rc = PTR_ERR(rp->rstmem);
+		goto err_null_rstmem;
+	}
+
+	/*
+	 * TODO unmap the memory range since the physical memory will
+	 * become inaccesible after the lend_rstmem() call.
+	 */
+	rc = rp->optee->ops->lend_rstmem(rp->optee, rp->rstmem, rp->end_points,
+					 rp->end_point_count, rp->use_case);
+	if (rc)
+		goto err_put_shm;
+	rp->rstmem->flags |= TEE_SHM_DYNAMIC;
+
+	rp->gen_pool = gen_pool_create(PAGE_SHIFT, -1);
+	if (!rp->gen_pool) {
+		rc = -ENOMEM;
+		goto err_reclaim;
+	}
+
+	rc = gen_pool_add(rp->gen_pool, rp->rstmem->paddr,
+			  rp->rstmem->size, -1);
+	if (rc)
+		goto err_free_pool;
+
+	refcount_set(&rp->refcount, 1);
+	return 0;
+
+err_free_pool:
+	gen_pool_destroy(rp->gen_pool);
+	rp->gen_pool = NULL;
+err_reclaim:
+	rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
+err_put_shm:
+	tee_shm_put(rp->rstmem);
+err_null_rstmem:
+	rp->rstmem = NULL;
+	return rc;
+}
+
+static int get_cma_rstmem(struct optee_rstmem_cma_pool *rp)
+{
+	int rc = 0;
+
+	if (!refcount_inc_not_zero(&rp->refcount)) {
+		mutex_lock(&rp->mutex);
+		if (rp->gen_pool) {
+			/*
+			 * Another thread has already initialized the pool
+			 * before us, or the pool was just about to be torn
+			 * down. Either way we only need to increase the
+			 * refcount and we're done.
+			 */
+			refcount_inc(&rp->refcount);
+		} else {
+			rc = init_cma_rstmem(rp);
+		}
+		mutex_unlock(&rp->mutex);
+	}
+
+	return rc;
+}
+
+static void release_cma_rstmem(struct optee_rstmem_cma_pool *rp)
+{
+	gen_pool_destroy(rp->gen_pool);
+	rp->gen_pool = NULL;
+
+	rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
+	rp->rstmem->flags &= ~TEE_SHM_DYNAMIC;
+
+	WARN(refcount_read(&rp->rstmem->refcount) != 1, "Unexpected refcount");
+	tee_shm_put(rp->rstmem);
+	rp->rstmem = NULL;
+}
+
+static void put_cma_rstmem(struct optee_rstmem_cma_pool *rp)
+{
+	if (refcount_dec_and_test(&rp->refcount)) {
+		mutex_lock(&rp->mutex);
+		if (rp->gen_pool)
+			release_cma_rstmem(rp);
+		mutex_unlock(&rp->mutex);
+	}
+}
+
+static int rstmem_pool_op_cma_alloc(struct tee_rstmem_pool *pool,
+				    struct sg_table *sgt, size_t size,
+				    size_t *offs)
+{
+	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
+	size_t sz = ALIGN(size, PAGE_SIZE);
+	phys_addr_t pa;
+	int rc;
+
+	rc = get_cma_rstmem(rp);
+	if (rc)
+		return rc;
+
+	pa = gen_pool_alloc(rp->gen_pool, sz);
+	if (!pa) {
+		rc = -ENOMEM;
+		goto err_put;
+	}
+
+	rc = sg_alloc_table(sgt, 1, GFP_KERNEL);
+	if (rc)
+		goto err_free;
+
+	sg_set_page(sgt->sgl, phys_to_page(pa), size, 0);
+	*offs = pa - rp->rstmem->paddr;
+
+	return 0;
+err_free:
+	gen_pool_free(rp->gen_pool, pa, size);
+err_put:
+	put_cma_rstmem(rp);
+
+	return rc;
+}
+
+static void rstmem_pool_op_cma_free(struct tee_rstmem_pool *pool,
+				    struct sg_table *sgt)
+{
+	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sgtable_sg(sgt, sg, i)
+		gen_pool_free(rp->gen_pool, sg_phys(sg), sg->length);
+	sg_free_table(sgt);
+	put_cma_rstmem(rp);
+}
+
+static int rstmem_pool_op_cma_update_shm(struct tee_rstmem_pool *pool,
+					 struct sg_table *sgt, size_t offs,
+					 struct tee_shm *shm,
+					 struct tee_shm **parent_shm)
+{
+	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
+
+	*parent_shm = rp->rstmem;
+
+	return 0;
+}
+
+static void pool_op_cma_destroy_pool(struct tee_rstmem_pool *pool)
+{
+	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
+
+	mutex_destroy(&rp->mutex);
+	kfree(rp);
+}
+
+static struct tee_rstmem_pool_ops rstmem_pool_ops_cma = {
+	.alloc = rstmem_pool_op_cma_alloc,
+	.free = rstmem_pool_op_cma_free,
+	.update_shm = rstmem_pool_op_cma_update_shm,
+	.destroy_pool = pool_op_cma_destroy_pool,
+};
+
+static int get_rstmem_config(struct optee *optee, u32 use_case,
+			     size_t *min_size, u_int *min_align,
+			     u16 *end_points, u_int *ep_count)
+{
+	struct tee_param params[2] = {
+		[0] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT,
+			.u.value.a = use_case,
+		},
+		[1] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT,
+		},
+	};
+	struct optee_shm_arg_entry *entry;
+	struct tee_shm *shm_param = NULL;
+	struct optee_msg_arg *msg_arg;
+	struct tee_shm *shm;
+	u_int offs;
+	int rc;
+
+	if (end_points && *ep_count) {
+		params[1].u.memref.size = *ep_count * sizeof(*end_points);
+		shm_param = tee_shm_alloc_priv_buf(optee->ctx,
+						   params[1].u.memref.size);
+		if (IS_ERR(shm_param))
+			return PTR_ERR(shm_param);
+		params[1].u.memref.shm = shm_param;
+	}
+
+	msg_arg = optee_get_msg_arg(optee->ctx, ARRAY_SIZE(params), &entry,
+				    &shm, &offs);
+	if (IS_ERR(msg_arg)) {
+		rc = PTR_ERR(msg_arg);
+		goto out_free_shm;
+	}
+	msg_arg->cmd = OPTEE_MSG_CMD_GET_RSTMEM_CONFIG;
+
+	rc = optee->ops->to_msg_param(optee, msg_arg->params,
+				      ARRAY_SIZE(params), params,
+				      false /*!update_out*/);
+	if (rc)
+		goto out_free_msg;
+
+	rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
+	if (rc)
+		goto out_free_msg;
+	if (msg_arg->ret && msg_arg->ret != TEEC_ERROR_SHORT_BUFFER) {
+		rc = -EINVAL;
+		goto out_free_msg;
+	}
+
+	rc = optee->ops->from_msg_param(optee, params, ARRAY_SIZE(params),
+					msg_arg->params, true /*update_out*/);
+	if (rc)
+		goto out_free_msg;
+
+	if (!msg_arg->ret && end_points &&
+	    *ep_count < params[1].u.memref.size / sizeof(u16)) {
+		rc = -EINVAL;
+		goto out_free_msg;
+	}
+
+	*min_size = params[0].u.value.a;
+	*min_align = params[0].u.value.b;
+	*ep_count = params[1].u.memref.size / sizeof(u16);
+
+	if (msg_arg->ret == TEEC_ERROR_SHORT_BUFFER) {
+		rc = -ENOSPC;
+		goto out_free_msg;
+	}
+
+	if (end_points)
+		memcpy(end_points, tee_shm_get_va(shm_param, 0),
+		       params[1].u.memref.size);
+
+out_free_msg:
+	optee_free_msg_arg(optee->ctx, entry, offs);
+out_free_shm:
+	if (shm_param)
+		tee_shm_free(shm_param);
+	return rc;
+}
+
+struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
+						    enum tee_dma_heap_id id)
+{
+	struct optee_rstmem_cma_pool *rp;
+	u32 use_case = id;
+	size_t min_size;
+	int rc;
+
+	rp = kzalloc(sizeof(*rp), GFP_KERNEL);
+	if (!rp)
+		return ERR_PTR(-ENOMEM);
+	rp->use_case = use_case;
+
+	rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, NULL,
+			       &rp->end_point_count);
+	if (rc) {
+		if (rc != -ENOSPC)
+			goto err;
+		rp->end_points = kcalloc(rp->end_point_count,
+					 sizeof(*rp->end_points), GFP_KERNEL);
+		if (!rp->end_points) {
+			rc = -ENOMEM;
+			goto err;
+		}
+		rc = get_rstmem_config(optee, use_case, &min_size, &rp->align,
+				       rp->end_points, &rp->end_point_count);
+		if (rc)
+			goto err_kfree_eps;
+	}
+
+	rp->pool.ops = &rstmem_pool_ops_cma;
+	rp->optee = optee;
+	rp->page_count = min_size / PAGE_SIZE;
+	mutex_init(&rp->mutex);
+
+	return &rp->pool;
+
+err_kfree_eps:
+	kfree(rp->end_points);
+err:
+	kfree(rp);
+	return ERR_PTR(rc);
+}
-- 
2.43.0

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 18757 bytes --]

On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> Add support in the OP-TEE backend driver dynamic restricted memory
> allocation with FF-A.
> 
> The restricted memory pools for dynamically allocated restrict memory
> are instantiated when requested by user-space. This instantiation can
> fail if OP-TEE doesn't support the requested use-case of restricted
> memory.
> 
> Restricted memory pools based on a static carveout or dynamic allocation
> can coexist for different use-cases. We use only dynamic allocation with
> FF-A.
> 
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  drivers/tee/optee/Makefile        |   1 +
>  drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
>  drivers/tee/optee/optee_private.h |  13 +-
>  drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
>  4 files changed, 483 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/tee/optee/rstmem.c
> 
> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> index a6eff388d300..498969fb8e40 100644
> --- a/drivers/tee/optee/Makefile
> +++ b/drivers/tee/optee/Makefile
> @@ -4,6 +4,7 @@ optee-objs += core.o
>  optee-objs += call.o
>  optee-objs += notif.o
>  optee-objs += rpc.o
> +optee-objs += rstmem.o
>  optee-objs += supp.o
>  optee-objs += device.o
>  optee-objs += smc_abi.o
> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> index e4b08cd195f3..6a55114232ef 100644
> --- a/drivers/tee/optee/ffa_abi.c
> +++ b/drivers/tee/optee/ffa_abi.c
> @@ -672,6 +672,123 @@ static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
>  	return optee_ffa_yielding_call(ctx, &data, rpc_arg, system_thread);
>  }
>  
> +static int do_call_lend_rstmem(struct optee *optee, u64 cookie, u32 use_case)
> +{
> +	struct optee_shm_arg_entry *entry;
> +	struct optee_msg_arg *msg_arg;
> +	struct tee_shm *shm;
> +	u_int offs;
> +	int rc;
> +
> +	msg_arg = optee_get_msg_arg(optee->ctx, 1, &entry, &shm, &offs);
> +	if (IS_ERR(msg_arg))
> +		return PTR_ERR(msg_arg);
> +
> +	msg_arg->cmd = OPTEE_MSG_CMD_ASSIGN_RSTMEM;
> +	msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT;
> +	msg_arg->params[0].u.value.a = cookie;
> +	msg_arg->params[0].u.value.b = use_case;
> +
> +	rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
> +	if (rc)
> +		goto out;
> +	if (msg_arg->ret != TEEC_SUCCESS) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +
> +out:
> +	optee_free_msg_arg(optee->ctx, entry, offs);
> +	return rc;
> +}
> +
> +static int optee_ffa_lend_rstmem(struct optee *optee, struct tee_shm *rstmem,
> +				 u16 *end_points, unsigned int ep_count,
> +				 u32 use_case)
> +{
> +	struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
> +	const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops;
> +	const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops;
> +	struct ffa_send_direct_data data;
> +	struct ffa_mem_region_attributes *mem_attr;
> +	struct ffa_mem_ops_args args = {
> +		.use_txbuf = true,
> +		.tag = use_case,
> +	};
> +	struct page *page;
> +	struct scatterlist sgl;
> +	unsigned int n;
> +	int rc;
> +
> +	mem_attr = kcalloc(ep_count, sizeof(*mem_attr), GFP_KERNEL);
> +	for (n = 0; n < ep_count; n++) {
> +		mem_attr[n].receiver = end_points[n];
> +		mem_attr[n].attrs = FFA_MEM_RW;
> +	}
> +	args.attrs = mem_attr;
> +	args.nattrs = ep_count;
> +
> +	page = phys_to_page(rstmem->paddr);
> +	sg_init_table(&sgl, 1);
> +	sg_set_page(&sgl, page, rstmem->size, 0);
> +
> +	args.sg = &sgl;
> +	rc = mem_ops->memory_lend(&args);
> +	kfree(mem_attr);
> +	if (rc)
> +		return rc;
> +
> +	rc = do_call_lend_rstmem(optee, args.g_handle, use_case);
> +	if (rc)
> +		goto err_reclaim;
> +
> +	rc = optee_shm_add_ffa_handle(optee, rstmem, args.g_handle);
> +	if (rc)
> +		goto err_unreg;
> +
> +	rstmem->sec_world_id = args.g_handle;
> +
> +	return 0;
> +
> +err_unreg:
> +	data = (struct ffa_send_direct_data){
> +		.data0 = OPTEE_FFA_RELEASE_RSTMEM,
> +		.data1 = (u32)args.g_handle,
> +		.data2 = (u32)(args.g_handle >> 32),
> +	};
> +	msg_ops->sync_send_receive(ffa_dev, &data);
> +err_reclaim:
> +	mem_ops->memory_reclaim(args.g_handle, 0);
> +	return rc;
> +}
> +
> +static int optee_ffa_reclaim_rstmem(struct optee *optee, struct tee_shm *rstmem)
> +{
> +	struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
> +	const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops;
> +	const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops;
> +	u64 global_handle = rstmem->sec_world_id;
> +	struct ffa_send_direct_data data = {
> +		.data0 = OPTEE_FFA_RELEASE_RSTMEM,
> +		.data1 = (u32)global_handle,
> +		.data2 = (u32)(global_handle >> 32)
> +	};
> +	int rc;
> +
> +	optee_shm_rem_ffa_handle(optee, global_handle);
> +	rstmem->sec_world_id = 0;
> +
> +	rc = msg_ops->sync_send_receive(ffa_dev, &data);
> +	if (rc)
> +		pr_err("Release SHM id 0x%llx rc %d\n", global_handle, rc);
> +
> +	rc = mem_ops->memory_reclaim(global_handle, 0);
> +	if (rc)
> +		pr_err("mem_reclaim: 0x%llx %d", global_handle, rc);
> +
> +	return rc;
> +}
> +
>  /*
>   * 6. Driver initialization
>   *
> @@ -833,6 +950,8 @@ static const struct optee_ops optee_ffa_ops = {
>  	.do_call_with_arg = optee_ffa_do_call_with_arg,
>  	.to_msg_param = optee_ffa_to_msg_param,
>  	.from_msg_param = optee_ffa_from_msg_param,
> +	.lend_rstmem = optee_ffa_lend_rstmem,
> +	.reclaim_rstmem = optee_ffa_reclaim_rstmem,
>  };
>  
>  static void optee_ffa_remove(struct ffa_device *ffa_dev)
> @@ -941,7 +1060,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>  				  optee->pool, optee);
>  	if (IS_ERR(teedev)) {
>  		rc = PTR_ERR(teedev);
> -		goto err_free_pool;
> +		goto err_free_shm_pool;
>  	}
>  	optee->teedev = teedev;
>  
> @@ -988,6 +1107,24 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>  			       rc);
>  	}
>  
> +	if (IS_ENABLED(CONFIG_CMA) && !IS_MODULE(CONFIG_OPTEE) &&

The CMA dependency should be managed via Kconfig.

> +	    (sec_caps & OPTEE_FFA_SEC_CAP_RSTMEM)) {
> +		enum tee_dma_heap_id id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY;
> +		struct tee_rstmem_pool *pool;
> +
> +		pool = optee_rstmem_alloc_cma_pool(optee, id);
> +		if (IS_ERR(pool)) {
> +			rc = PTR_ERR(pool);
> +			goto err_notif_uninit;
> +		}
> +
> +		rc = tee_device_register_dma_heap(optee->teedev, id, pool);
> +		if (rc) {
> +			pool->ops->destroy_pool(pool);
> +			goto err_notif_uninit;
> +		}
> +	}
> +
>  	rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
>  	if (rc)
>  		goto err_unregister_devices;
> @@ -1001,6 +1138,8 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>  
>  err_unregister_devices:
>  	optee_unregister_devices();
> +	tee_device_unregister_all_dma_heaps(optee->teedev);
> +err_notif_uninit:
>  	if (optee->ffa.bottom_half_value != U32_MAX)
>  		notif_ops->notify_relinquish(ffa_dev,
>  					     optee->ffa.bottom_half_value);
> @@ -1018,7 +1157,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>  	tee_device_unregister(optee->supp_teedev);
>  err_unreg_teedev:
>  	tee_device_unregister(optee->teedev);
> -err_free_pool:
> +err_free_shm_pool:
>  	tee_shm_pool_free(pool);
>  err_free_optee:
>  	kfree(optee);
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 20eda508dbac..faab31ad7c52 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -174,9 +174,14 @@ struct optee;
>   * @do_call_with_arg:	enters OP-TEE in secure world
>   * @to_msg_param:	converts from struct tee_param to OPTEE_MSG parameters
>   * @from_msg_param:	converts from OPTEE_MSG parameters to struct tee_param
> + * @lend_rstmem:	lends physically contiguous memory as restricted
> + *			memory, inaccessible by the kernel
> + * @reclaim_rstmem:	reclaims restricted memory previously lent with
> + *			@lend_rstmem() and makes it accessible by the
> + *			kernel again
>   *
>   * These OPs are only supposed to be used internally in the OP-TEE driver
> - * as a way of abstracting the different methogs of entering OP-TEE in
> + * as a way of abstracting the different methods of entering OP-TEE in
>   * secure world.
>   */
>  struct optee_ops {
> @@ -191,6 +196,10 @@ struct optee_ops {
>  			      size_t num_params,
>  			      const struct optee_msg_param *msg_params,
>  			      bool update_out);
> +	int (*lend_rstmem)(struct optee *optee, struct tee_shm *rstmem,
> +			   u16 *end_points, unsigned int ep_count,
> +			   u32 use_case);
> +	int (*reclaim_rstmem)(struct optee *optee, struct tee_shm *rstmem);
>  };
>  
>  /**
> @@ -285,6 +294,8 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
>  void optee_supp_init(struct optee_supp *supp);
>  void optee_supp_uninit(struct optee_supp *supp);
>  void optee_supp_release(struct optee_supp *supp);
> +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
> +						    enum tee_dma_heap_id id);
>  
>  int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params,
>  		    struct tee_param *param);
> diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> new file mode 100644
> index 000000000000..ea27769934d4
> --- /dev/null
> +++ b/drivers/tee/optee/rstmem.c
> @@ -0,0 +1,329 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2025, Linaro Limited
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/errno.h>
> +#include <linux/genalloc.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/tee_core.h>
> +#include <linux/types.h>
> +#include "optee_private.h"
> +
> +struct optee_rstmem_cma_pool {
> +	struct tee_rstmem_pool pool;
> +	struct gen_pool *gen_pool;
> +	struct optee *optee;
> +	size_t page_count;
> +	u16 *end_points;
> +	u_int end_point_count;
> +	u_int align;
> +	refcount_t refcount;
> +	u32 use_case;
> +	struct tee_shm *rstmem;
> +	/* Protects when initializing and tearing down this struct */
> +	struct mutex mutex;
> +};
> +
> +static struct optee_rstmem_cma_pool *
> +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> +{
> +	return container_of(pool, struct optee_rstmem_cma_pool, pool);
> +}
> +
> +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> +{
> +	int rc;
> +
> +	rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> +						rp->align);
> +	if (IS_ERR(rp->rstmem)) {
> +		rc = PTR_ERR(rp->rstmem);
> +		goto err_null_rstmem;
> +	}
> +
> +	/*
> +	 * TODO unmap the memory range since the physical memory will
> +	 * become inaccesible after the lend_rstmem() call.
> +	 */

What's your plan for this TODO? I think we need a CMA allocator here
which can allocate un-mapped memory such that any cache speculation
won't lead to CPU hangs once the memory restriction comes into picture.

> +	rc = rp->optee->ops->lend_rstmem(rp->optee, rp->rstmem, rp->end_points,
> +					 rp->end_point_count, rp->use_case);
> +	if (rc)
> +		goto err_put_shm;
> +	rp->rstmem->flags |= TEE_SHM_DYNAMIC;
> +
> +	rp->gen_pool = gen_pool_create(PAGE_SHIFT, -1);
> +	if (!rp->gen_pool) {
> +		rc = -ENOMEM;
> +		goto err_reclaim;
> +	}
> +
> +	rc = gen_pool_add(rp->gen_pool, rp->rstmem->paddr,
> +			  rp->rstmem->size, -1);
> +	if (rc)
> +		goto err_free_pool;
> +
> +	refcount_set(&rp->refcount, 1);
> +	return 0;
> +
> +err_free_pool:
> +	gen_pool_destroy(rp->gen_pool);
> +	rp->gen_pool = NULL;
> +err_reclaim:
> +	rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> +err_put_shm:
> +	tee_shm_put(rp->rstmem);
> +err_null_rstmem:
> +	rp->rstmem = NULL;
> +	return rc;
> +}
> +
> +static int get_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> +{
> +	int rc = 0;
> +
> +	if (!refcount_inc_not_zero(&rp->refcount)) {
> +		mutex_lock(&rp->mutex);
> +		if (rp->gen_pool) {
> +			/*
> +			 * Another thread has already initialized the pool
> +			 * before us, or the pool was just about to be torn
> +			 * down. Either way we only need to increase the
> +			 * refcount and we're done.
> +			 */
> +			refcount_inc(&rp->refcount);
> +		} else {
> +			rc = init_cma_rstmem(rp);
> +		}
> +		mutex_unlock(&rp->mutex);
> +	}
> +
> +	return rc;
> +}
> +
> +static void release_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> +{
> +	gen_pool_destroy(rp->gen_pool);
> +	rp->gen_pool = NULL;
> +
> +	rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> +	rp->rstmem->flags &= ~TEE_SHM_DYNAMIC;
> +
> +	WARN(refcount_read(&rp->rstmem->refcount) != 1, "Unexpected refcount");
> +	tee_shm_put(rp->rstmem);
> +	rp->rstmem = NULL;
> +}
> +
> +static void put_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> +{
> +	if (refcount_dec_and_test(&rp->refcount)) {
> +		mutex_lock(&rp->mutex);
> +		if (rp->gen_pool)
> +			release_cma_rstmem(rp);
> +		mutex_unlock(&rp->mutex);
> +	}
> +}
> +
> +static int rstmem_pool_op_cma_alloc(struct tee_rstmem_pool *pool,
> +				    struct sg_table *sgt, size_t size,
> +				    size_t *offs)
> +{
> +	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> +	size_t sz = ALIGN(size, PAGE_SIZE);
> +	phys_addr_t pa;
> +	int rc;
> +
> +	rc = get_cma_rstmem(rp);
> +	if (rc)
> +		return rc;
> +
> +	pa = gen_pool_alloc(rp->gen_pool, sz);
> +	if (!pa) {
> +		rc = -ENOMEM;
> +		goto err_put;
> +	}
> +
> +	rc = sg_alloc_table(sgt, 1, GFP_KERNEL);
> +	if (rc)
> +		goto err_free;
> +
> +	sg_set_page(sgt->sgl, phys_to_page(pa), size, 0);
> +	*offs = pa - rp->rstmem->paddr;
> +
> +	return 0;
> +err_free:
> +	gen_pool_free(rp->gen_pool, pa, size);
> +err_put:
> +	put_cma_rstmem(rp);
> +
> +	return rc;
> +}
> +
> +static void rstmem_pool_op_cma_free(struct tee_rstmem_pool *pool,
> +				    struct sg_table *sgt)
> +{
> +	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> +	struct scatterlist *sg;
> +	int i;
> +
> +	for_each_sgtable_sg(sgt, sg, i)
> +		gen_pool_free(rp->gen_pool, sg_phys(sg), sg->length);
> +	sg_free_table(sgt);
> +	put_cma_rstmem(rp);
> +}
> +
> +static int rstmem_pool_op_cma_update_shm(struct tee_rstmem_pool *pool,
> +					 struct sg_table *sgt, size_t offs,
> +					 struct tee_shm *shm,
> +					 struct tee_shm **parent_shm)
> +{
> +	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> +
> +	*parent_shm = rp->rstmem;
> +
> +	return 0;
> +}
> +
> +static void pool_op_cma_destroy_pool(struct tee_rstmem_pool *pool)
> +{
> +	struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> +
> +	mutex_destroy(&rp->mutex);
> +	kfree(rp);
> +}
> +
> +static struct tee_rstmem_pool_ops rstmem_pool_ops_cma = {
> +	.alloc = rstmem_pool_op_cma_alloc,
> +	.free = rstmem_pool_op_cma_free,
> +	.update_shm = rstmem_pool_op_cma_update_shm,
> +	.destroy_pool = pool_op_cma_destroy_pool,
> +};
> +
> +static int get_rstmem_config(struct optee *optee, u32 use_case,
> +			     size_t *min_size, u_int *min_align,
> +			     u16 *end_points, u_int *ep_count)

I guess this end points terminology is specific to FF-A ABI. Is there
any relevance for this in the common APIs?

-Sumit

> +{
> +	struct tee_param params[2] = {
> +		[0] = {
> +			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT,
> +			.u.value.a = use_case,
> +		},
> +		[1] = {
> +			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT,
> +		},
> +	};
> +	struct optee_shm_arg_entry *entry;
> +	struct tee_shm *shm_param = NULL;
> +	struct optee_msg_arg *msg_arg;
> +	struct tee_shm *shm;
> +	u_int offs;
> +	int rc;
> +
> +	if (end_points && *ep_count) {
> +		params[1].u.memref.size = *ep_count * sizeof(*end_points);
> +		shm_param = tee_shm_alloc_priv_buf(optee->ctx,
> +						   params[1].u.memref.size);
> +		if (IS_ERR(shm_param))
> +			return PTR_ERR(shm_param);
> +		params[1].u.memref.shm = shm_param;
> +	}
> +
> +	msg_arg = optee_get_msg_arg(optee->ctx, ARRAY_SIZE(params), &entry,
> +				    &shm, &offs);
> +	if (IS_ERR(msg_arg)) {
> +		rc = PTR_ERR(msg_arg);
> +		goto out_free_shm;
> +	}
> +	msg_arg->cmd = OPTEE_MSG_CMD_GET_RSTMEM_CONFIG;
> +
> +	rc = optee->ops->to_msg_param(optee, msg_arg->params,
> +				      ARRAY_SIZE(params), params,
> +				      false /*!update_out*/);
> +	if (rc)
> +		goto out_free_msg;
> +
> +	rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
> +	if (rc)
> +		goto out_free_msg;
> +	if (msg_arg->ret && msg_arg->ret != TEEC_ERROR_SHORT_BUFFER) {
> +		rc = -EINVAL;
> +		goto out_free_msg;
> +	}
> +
> +	rc = optee->ops->from_msg_param(optee, params, ARRAY_SIZE(params),
> +					msg_arg->params, true /*update_out*/);
> +	if (rc)
> +		goto out_free_msg;
> +
> +	if (!msg_arg->ret && end_points &&
> +	    *ep_count < params[1].u.memref.size / sizeof(u16)) {
> +		rc = -EINVAL;
> +		goto out_free_msg;
> +	}
> +
> +	*min_size = params[0].u.value.a;
> +	*min_align = params[0].u.value.b;
> +	*ep_count = params[1].u.memref.size / sizeof(u16);
> +
> +	if (msg_arg->ret == TEEC_ERROR_SHORT_BUFFER) {
> +		rc = -ENOSPC;
> +		goto out_free_msg;
> +	}
> +
> +	if (end_points)
> +		memcpy(end_points, tee_shm_get_va(shm_param, 0),
> +		       params[1].u.memref.size);
> +
> +out_free_msg:
> +	optee_free_msg_arg(optee->ctx, entry, offs);
> +out_free_shm:
> +	if (shm_param)
> +		tee_shm_free(shm_param);
> +	return rc;
> +}
> +
> +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
> +						    enum tee_dma_heap_id id)
> +{
> +	struct optee_rstmem_cma_pool *rp;
> +	u32 use_case = id;
> +	size_t min_size;
> +	int rc;
> +
> +	rp = kzalloc(sizeof(*rp), GFP_KERNEL);
> +	if (!rp)
> +		return ERR_PTR(-ENOMEM);
> +	rp->use_case = use_case;
> +
> +	rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, NULL,
> +			       &rp->end_point_count);
> +	if (rc) {
> +		if (rc != -ENOSPC)
> +			goto err;
> +		rp->end_points = kcalloc(rp->end_point_count,
> +					 sizeof(*rp->end_points), GFP_KERNEL);
> +		if (!rp->end_points) {
> +			rc = -ENOMEM;
> +			goto err;
> +		}
> +		rc = get_rstmem_config(optee, use_case, &min_size, &rp->align,
> +				       rp->end_points, &rp->end_point_count);
> +		if (rc)
> +			goto err_kfree_eps;
> +	}
> +
> +	rp->pool.ops = &rstmem_pool_ops_cma;
> +	rp->optee = optee;
> +	rp->page_count = min_size / PAGE_SIZE;
> +	mutex_init(&rp->mutex);
> +
> +	return &rp->pool;
> +
> +err_kfree_eps:
> +	kfree(rp->end_points);
> +err:
> +	kfree(rp);
> +	return ERR_PTR(rc);
> +}
> -- 
> 2.43.0
> 

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

[Jens Wiklander]

[-- Attachment #1: Type: text/plain, Size: 23528 bytes --]

Hi Sumit,

On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg@kernel.org> wrote:
>
> On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote:
> > Add support in the OP-TEE backend driver dynamic restricted memory
> > allocation with FF-A.
> >
> > The restricted memory pools for dynamically allocated restrict memory
> > are instantiated when requested by user-space. This instantiation can
> > fail if OP-TEE doesn't support the requested use-case of restricted
> > memory.
> >
> > Restricted memory pools based on a static carveout or dynamic allocation
> > can coexist for different use-cases. We use only dynamic allocation with
> > FF-A.
> >
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > ---
> >  drivers/tee/optee/Makefile        |   1 +
> >  drivers/tee/optee/ffa_abi.c       | 143 ++++++++++++-
> >  drivers/tee/optee/optee_private.h |  13 +-
> >  drivers/tee/optee/rstmem.c        | 329 ++++++++++++++++++++++++++++++
> >  4 files changed, 483 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/tee/optee/rstmem.c
> >
> > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > index a6eff388d300..498969fb8e40 100644
> > --- a/drivers/tee/optee/Makefile
> > +++ b/drivers/tee/optee/Makefile
> > @@ -4,6 +4,7 @@ optee-objs += core.o
> >  optee-objs += call.o
> >  optee-objs += notif.o
> >  optee-objs += rpc.o
> > +optee-objs += rstmem.o
> >  optee-objs += supp.o
> >  optee-objs += device.o
> >  optee-objs += smc_abi.o
> > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> > index e4b08cd195f3..6a55114232ef 100644
> > --- a/drivers/tee/optee/ffa_abi.c
> > +++ b/drivers/tee/optee/ffa_abi.c
> > @@ -672,6 +672,123 @@ static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
> >       return optee_ffa_yielding_call(ctx, &data, rpc_arg, system_thread);
> >  }
> >
> > +static int do_call_lend_rstmem(struct optee *optee, u64 cookie, u32 use_case)
> > +{
> > +     struct optee_shm_arg_entry *entry;
> > +     struct optee_msg_arg *msg_arg;
> > +     struct tee_shm *shm;
> > +     u_int offs;
> > +     int rc;
> > +
> > +     msg_arg = optee_get_msg_arg(optee->ctx, 1, &entry, &shm, &offs);
> > +     if (IS_ERR(msg_arg))
> > +             return PTR_ERR(msg_arg);
> > +
> > +     msg_arg->cmd = OPTEE_MSG_CMD_ASSIGN_RSTMEM;
> > +     msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT;
> > +     msg_arg->params[0].u.value.a = cookie;
> > +     msg_arg->params[0].u.value.b = use_case;
> > +
> > +     rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
> > +     if (rc)
> > +             goto out;
> > +     if (msg_arg->ret != TEEC_SUCCESS) {
> > +             rc = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +out:
> > +     optee_free_msg_arg(optee->ctx, entry, offs);
> > +     return rc;
> > +}
> > +
> > +static int optee_ffa_lend_rstmem(struct optee *optee, struct tee_shm *rstmem,
> > +                              u16 *end_points, unsigned int ep_count,
> > +                              u32 use_case)
> > +{
> > +     struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
> > +     const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops;
> > +     const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops;
> > +     struct ffa_send_direct_data data;
> > +     struct ffa_mem_region_attributes *mem_attr;
> > +     struct ffa_mem_ops_args args = {
> > +             .use_txbuf = true,
> > +             .tag = use_case,
> > +     };
> > +     struct page *page;
> > +     struct scatterlist sgl;
> > +     unsigned int n;
> > +     int rc;
> > +
> > +     mem_attr = kcalloc(ep_count, sizeof(*mem_attr), GFP_KERNEL);
> > +     for (n = 0; n < ep_count; n++) {
> > +             mem_attr[n].receiver = end_points[n];
> > +             mem_attr[n].attrs = FFA_MEM_RW;
> > +     }
> > +     args.attrs = mem_attr;
> > +     args.nattrs = ep_count;
> > +
> > +     page = phys_to_page(rstmem->paddr);
> > +     sg_init_table(&sgl, 1);
> > +     sg_set_page(&sgl, page, rstmem->size, 0);
> > +
> > +     args.sg = &sgl;
> > +     rc = mem_ops->memory_lend(&args);
> > +     kfree(mem_attr);
> > +     if (rc)
> > +             return rc;
> > +
> > +     rc = do_call_lend_rstmem(optee, args.g_handle, use_case);
> > +     if (rc)
> > +             goto err_reclaim;
> > +
> > +     rc = optee_shm_add_ffa_handle(optee, rstmem, args.g_handle);
> > +     if (rc)
> > +             goto err_unreg;
> > +
> > +     rstmem->sec_world_id = args.g_handle;
> > +
> > +     return 0;
> > +
> > +err_unreg:
> > +     data = (struct ffa_send_direct_data){
> > +             .data0 = OPTEE_FFA_RELEASE_RSTMEM,
> > +             .data1 = (u32)args.g_handle,
> > +             .data2 = (u32)(args.g_handle >> 32),
> > +     };
> > +     msg_ops->sync_send_receive(ffa_dev, &data);
> > +err_reclaim:
> > +     mem_ops->memory_reclaim(args.g_handle, 0);
> > +     return rc;
> > +}
> > +
> > +static int optee_ffa_reclaim_rstmem(struct optee *optee, struct tee_shm *rstmem)
> > +{
> > +     struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
> > +     const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops;
> > +     const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops;
> > +     u64 global_handle = rstmem->sec_world_id;
> > +     struct ffa_send_direct_data data = {
> > +             .data0 = OPTEE_FFA_RELEASE_RSTMEM,
> > +             .data1 = (u32)global_handle,
> > +             .data2 = (u32)(global_handle >> 32)
> > +     };
> > +     int rc;
> > +
> > +     optee_shm_rem_ffa_handle(optee, global_handle);
> > +     rstmem->sec_world_id = 0;
> > +
> > +     rc = msg_ops->sync_send_receive(ffa_dev, &data);
> > +     if (rc)
> > +             pr_err("Release SHM id 0x%llx rc %d\n", global_handle, rc);
> > +
> > +     rc = mem_ops->memory_reclaim(global_handle, 0);
> > +     if (rc)
> > +             pr_err("mem_reclaim: 0x%llx %d", global_handle, rc);
> > +
> > +     return rc;
> > +}
> > +
> >  /*
> >   * 6. Driver initialization
> >   *
> > @@ -833,6 +950,8 @@ static const struct optee_ops optee_ffa_ops = {
> >       .do_call_with_arg = optee_ffa_do_call_with_arg,
> >       .to_msg_param = optee_ffa_to_msg_param,
> >       .from_msg_param = optee_ffa_from_msg_param,
> > +     .lend_rstmem = optee_ffa_lend_rstmem,
> > +     .reclaim_rstmem = optee_ffa_reclaim_rstmem,
> >  };
> >
> >  static void optee_ffa_remove(struct ffa_device *ffa_dev)
> > @@ -941,7 +1060,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> >                                 optee->pool, optee);
> >       if (IS_ERR(teedev)) {
> >               rc = PTR_ERR(teedev);
> > -             goto err_free_pool;
> > +             goto err_free_shm_pool;
> >       }
> >       optee->teedev = teedev;
> >
> > @@ -988,6 +1107,24 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> >                              rc);
> >       }
> >
> > +     if (IS_ENABLED(CONFIG_CMA) && !IS_MODULE(CONFIG_OPTEE) &&
>
> The CMA dependency should be managed via Kconfig.

Yes, I'll fix it.

>
> > +         (sec_caps & OPTEE_FFA_SEC_CAP_RSTMEM)) {
> > +             enum tee_dma_heap_id id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY;
> > +             struct tee_rstmem_pool *pool;
> > +
> > +             pool = optee_rstmem_alloc_cma_pool(optee, id);
> > +             if (IS_ERR(pool)) {
> > +                     rc = PTR_ERR(pool);
> > +                     goto err_notif_uninit;
> > +             }
> > +
> > +             rc = tee_device_register_dma_heap(optee->teedev, id, pool);
> > +             if (rc) {
> > +                     pool->ops->destroy_pool(pool);
> > +                     goto err_notif_uninit;
> > +             }
> > +     }
> > +
> >       rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
> >       if (rc)
> >               goto err_unregister_devices;
> > @@ -1001,6 +1138,8 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> >
> >  err_unregister_devices:
> >       optee_unregister_devices();
> > +     tee_device_unregister_all_dma_heaps(optee->teedev);
> > +err_notif_uninit:
> >       if (optee->ffa.bottom_half_value != U32_MAX)
> >               notif_ops->notify_relinquish(ffa_dev,
> >                                            optee->ffa.bottom_half_value);
> > @@ -1018,7 +1157,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> >       tee_device_unregister(optee->supp_teedev);
> >  err_unreg_teedev:
> >       tee_device_unregister(optee->teedev);
> > -err_free_pool:
> > +err_free_shm_pool:
> >       tee_shm_pool_free(pool);
> >  err_free_optee:
> >       kfree(optee);
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index 20eda508dbac..faab31ad7c52 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -174,9 +174,14 @@ struct optee;
> >   * @do_call_with_arg:        enters OP-TEE in secure world
> >   * @to_msg_param:    converts from struct tee_param to OPTEE_MSG parameters
> >   * @from_msg_param:  converts from OPTEE_MSG parameters to struct tee_param
> > + * @lend_rstmem:     lends physically contiguous memory as restricted
> > + *                   memory, inaccessible by the kernel
> > + * @reclaim_rstmem:  reclaims restricted memory previously lent with
> > + *                   @lend_rstmem() and makes it accessible by the
> > + *                   kernel again
> >   *
> >   * These OPs are only supposed to be used internally in the OP-TEE driver
> > - * as a way of abstracting the different methogs of entering OP-TEE in
> > + * as a way of abstracting the different methods of entering OP-TEE in
> >   * secure world.
> >   */
> >  struct optee_ops {
> > @@ -191,6 +196,10 @@ struct optee_ops {
> >                             size_t num_params,
> >                             const struct optee_msg_param *msg_params,
> >                             bool update_out);
> > +     int (*lend_rstmem)(struct optee *optee, struct tee_shm *rstmem,
> > +                        u16 *end_points, unsigned int ep_count,
> > +                        u32 use_case);
> > +     int (*reclaim_rstmem)(struct optee *optee, struct tee_shm *rstmem);
> >  };
> >
> >  /**
> > @@ -285,6 +294,8 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
> >  void optee_supp_init(struct optee_supp *supp);
> >  void optee_supp_uninit(struct optee_supp *supp);
> >  void optee_supp_release(struct optee_supp *supp);
> > +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
> > +                                                 enum tee_dma_heap_id id);
> >
> >  int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params,
> >                   struct tee_param *param);
> > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c
> > new file mode 100644
> > index 000000000000..ea27769934d4
> > --- /dev/null
> > +++ b/drivers/tee/optee/rstmem.c
> > @@ -0,0 +1,329 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2025, Linaro Limited
> > + */
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/errno.h>
> > +#include <linux/genalloc.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/tee_core.h>
> > +#include <linux/types.h>
> > +#include "optee_private.h"
> > +
> > +struct optee_rstmem_cma_pool {
> > +     struct tee_rstmem_pool pool;
> > +     struct gen_pool *gen_pool;
> > +     struct optee *optee;
> > +     size_t page_count;
> > +     u16 *end_points;
> > +     u_int end_point_count;
> > +     u_int align;
> > +     refcount_t refcount;
> > +     u32 use_case;
> > +     struct tee_shm *rstmem;
> > +     /* Protects when initializing and tearing down this struct */
> > +     struct mutex mutex;
> > +};
> > +
> > +static struct optee_rstmem_cma_pool *
> > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool)
> > +{
> > +     return container_of(pool, struct optee_rstmem_cma_pool, pool);
> > +}
> > +
> > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > +{
> > +     int rc;
> > +
> > +     rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count,
> > +                                             rp->align);
> > +     if (IS_ERR(rp->rstmem)) {
> > +             rc = PTR_ERR(rp->rstmem);
> > +             goto err_null_rstmem;
> > +     }
> > +
> > +     /*
> > +      * TODO unmap the memory range since the physical memory will
> > +      * become inaccesible after the lend_rstmem() call.
> > +      */
>
> What's your plan for this TODO? I think we need a CMA allocator here
> which can allocate un-mapped memory such that any cache speculation
> won't lead to CPU hangs once the memory restriction comes into picture.

What happens is platform-specific. For some platforms, it might be
enough to avoid explicit access. Yes, a CMA allocator with unmapped
memory or where memory can be unmapped is one option.

>
> > +     rc = rp->optee->ops->lend_rstmem(rp->optee, rp->rstmem, rp->end_points,
> > +                                      rp->end_point_count, rp->use_case);
> > +     if (rc)
> > +             goto err_put_shm;
> > +     rp->rstmem->flags |= TEE_SHM_DYNAMIC;
> > +
> > +     rp->gen_pool = gen_pool_create(PAGE_SHIFT, -1);
> > +     if (!rp->gen_pool) {
> > +             rc = -ENOMEM;
> > +             goto err_reclaim;
> > +     }
> > +
> > +     rc = gen_pool_add(rp->gen_pool, rp->rstmem->paddr,
> > +                       rp->rstmem->size, -1);
> > +     if (rc)
> > +             goto err_free_pool;
> > +
> > +     refcount_set(&rp->refcount, 1);
> > +     return 0;
> > +
> > +err_free_pool:
> > +     gen_pool_destroy(rp->gen_pool);
> > +     rp->gen_pool = NULL;
> > +err_reclaim:
> > +     rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> > +err_put_shm:
> > +     tee_shm_put(rp->rstmem);
> > +err_null_rstmem:
> > +     rp->rstmem = NULL;
> > +     return rc;
> > +}
> > +
> > +static int get_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > +{
> > +     int rc = 0;
> > +
> > +     if (!refcount_inc_not_zero(&rp->refcount)) {
> > +             mutex_lock(&rp->mutex);
> > +             if (rp->gen_pool) {
> > +                     /*
> > +                      * Another thread has already initialized the pool
> > +                      * before us, or the pool was just about to be torn
> > +                      * down. Either way we only need to increase the
> > +                      * refcount and we're done.
> > +                      */
> > +                     refcount_inc(&rp->refcount);
> > +             } else {
> > +                     rc = init_cma_rstmem(rp);
> > +             }
> > +             mutex_unlock(&rp->mutex);
> > +     }
> > +
> > +     return rc;
> > +}
> > +
> > +static void release_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > +{
> > +     gen_pool_destroy(rp->gen_pool);
> > +     rp->gen_pool = NULL;
> > +
> > +     rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem);
> > +     rp->rstmem->flags &= ~TEE_SHM_DYNAMIC;
> > +
> > +     WARN(refcount_read(&rp->rstmem->refcount) != 1, "Unexpected refcount");
> > +     tee_shm_put(rp->rstmem);
> > +     rp->rstmem = NULL;
> > +}
> > +
> > +static void put_cma_rstmem(struct optee_rstmem_cma_pool *rp)
> > +{
> > +     if (refcount_dec_and_test(&rp->refcount)) {
> > +             mutex_lock(&rp->mutex);
> > +             if (rp->gen_pool)
> > +                     release_cma_rstmem(rp);
> > +             mutex_unlock(&rp->mutex);
> > +     }
> > +}
> > +
> > +static int rstmem_pool_op_cma_alloc(struct tee_rstmem_pool *pool,
> > +                                 struct sg_table *sgt, size_t size,
> > +                                 size_t *offs)
> > +{
> > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > +     size_t sz = ALIGN(size, PAGE_SIZE);
> > +     phys_addr_t pa;
> > +     int rc;
> > +
> > +     rc = get_cma_rstmem(rp);
> > +     if (rc)
> > +             return rc;
> > +
> > +     pa = gen_pool_alloc(rp->gen_pool, sz);
> > +     if (!pa) {
> > +             rc = -ENOMEM;
> > +             goto err_put;
> > +     }
> > +
> > +     rc = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > +     if (rc)
> > +             goto err_free;
> > +
> > +     sg_set_page(sgt->sgl, phys_to_page(pa), size, 0);
> > +     *offs = pa - rp->rstmem->paddr;
> > +
> > +     return 0;
> > +err_free:
> > +     gen_pool_free(rp->gen_pool, pa, size);
> > +err_put:
> > +     put_cma_rstmem(rp);
> > +
> > +     return rc;
> > +}
> > +
> > +static void rstmem_pool_op_cma_free(struct tee_rstmem_pool *pool,
> > +                                 struct sg_table *sgt)
> > +{
> > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > +     struct scatterlist *sg;
> > +     int i;
> > +
> > +     for_each_sgtable_sg(sgt, sg, i)
> > +             gen_pool_free(rp->gen_pool, sg_phys(sg), sg->length);
> > +     sg_free_table(sgt);
> > +     put_cma_rstmem(rp);
> > +}
> > +
> > +static int rstmem_pool_op_cma_update_shm(struct tee_rstmem_pool *pool,
> > +                                      struct sg_table *sgt, size_t offs,
> > +                                      struct tee_shm *shm,
> > +                                      struct tee_shm **parent_shm)
> > +{
> > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > +
> > +     *parent_shm = rp->rstmem;
> > +
> > +     return 0;
> > +}
> > +
> > +static void pool_op_cma_destroy_pool(struct tee_rstmem_pool *pool)
> > +{
> > +     struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool);
> > +
> > +     mutex_destroy(&rp->mutex);
> > +     kfree(rp);
> > +}
> > +
> > +static struct tee_rstmem_pool_ops rstmem_pool_ops_cma = {
> > +     .alloc = rstmem_pool_op_cma_alloc,
> > +     .free = rstmem_pool_op_cma_free,
> > +     .update_shm = rstmem_pool_op_cma_update_shm,
> > +     .destroy_pool = pool_op_cma_destroy_pool,
> > +};
> > +
> > +static int get_rstmem_config(struct optee *optee, u32 use_case,
> > +                          size_t *min_size, u_int *min_align,
> > +                          u16 *end_points, u_int *ep_count)
>
> I guess this end points terminology is specific to FF-A ABI. Is there
> any relevance for this in the common APIs?

Yes, endpoints are specific to FF-A ABI. The list of end-points must
be presented to FFA_MEM_LEND. We're relying on the secure world to
know which endpoints are needed for a specific use case.

Cheers,
Jens

>
> -Sumit
>
> > +{
> > +     struct tee_param params[2] = {
> > +             [0] = {
> > +                     .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT,
> > +                     .u.value.a = use_case,
> > +             },
> > +             [1] = {
> > +                     .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT,
> > +             },
> > +     };
> > +     struct optee_shm_arg_entry *entry;
> > +     struct tee_shm *shm_param = NULL;
> > +     struct optee_msg_arg *msg_arg;
> > +     struct tee_shm *shm;
> > +     u_int offs;
> > +     int rc;
> > +
> > +     if (end_points && *ep_count) {
> > +             params[1].u.memref.size = *ep_count * sizeof(*end_points);
> > +             shm_param = tee_shm_alloc_priv_buf(optee->ctx,
> > +                                                params[1].u.memref.size);
> > +             if (IS_ERR(shm_param))
> > +                     return PTR_ERR(shm_param);
> > +             params[1].u.memref.shm = shm_param;
> > +     }
> > +
> > +     msg_arg = optee_get_msg_arg(optee->ctx, ARRAY_SIZE(params), &entry,
> > +                                 &shm, &offs);
> > +     if (IS_ERR(msg_arg)) {
> > +             rc = PTR_ERR(msg_arg);
> > +             goto out_free_shm;
> > +     }
> > +     msg_arg->cmd = OPTEE_MSG_CMD_GET_RSTMEM_CONFIG;
> > +
> > +     rc = optee->ops->to_msg_param(optee, msg_arg->params,
> > +                                   ARRAY_SIZE(params), params,
> > +                                   false /*!update_out*/);
> > +     if (rc)
> > +             goto out_free_msg;
> > +
> > +     rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false);
> > +     if (rc)
> > +             goto out_free_msg;
> > +     if (msg_arg->ret && msg_arg->ret != TEEC_ERROR_SHORT_BUFFER) {
> > +             rc = -EINVAL;
> > +             goto out_free_msg;
> > +     }
> > +
> > +     rc = optee->ops->from_msg_param(optee, params, ARRAY_SIZE(params),
> > +                                     msg_arg->params, true /*update_out*/);
> > +     if (rc)
> > +             goto out_free_msg;
> > +
> > +     if (!msg_arg->ret && end_points &&
> > +         *ep_count < params[1].u.memref.size / sizeof(u16)) {
> > +             rc = -EINVAL;
> > +             goto out_free_msg;
> > +     }
> > +
> > +     *min_size = params[0].u.value.a;
> > +     *min_align = params[0].u.value.b;
> > +     *ep_count = params[1].u.memref.size / sizeof(u16);
> > +
> > +     if (msg_arg->ret == TEEC_ERROR_SHORT_BUFFER) {
> > +             rc = -ENOSPC;
> > +             goto out_free_msg;
> > +     }
> > +
> > +     if (end_points)
> > +             memcpy(end_points, tee_shm_get_va(shm_param, 0),
> > +                    params[1].u.memref.size);
> > +
> > +out_free_msg:
> > +     optee_free_msg_arg(optee->ctx, entry, offs);
> > +out_free_shm:
> > +     if (shm_param)
> > +             tee_shm_free(shm_param);
> > +     return rc;
> > +}
> > +
> > +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee,
> > +                                                 enum tee_dma_heap_id id)
> > +{
> > +     struct optee_rstmem_cma_pool *rp;
> > +     u32 use_case = id;
> > +     size_t min_size;
> > +     int rc;
> > +
> > +     rp = kzalloc(sizeof(*rp), GFP_KERNEL);
> > +     if (!rp)
> > +             return ERR_PTR(-ENOMEM);
> > +     rp->use_case = use_case;
> > +
> > +     rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, NULL,
> > +                            &rp->end_point_count);
> > +     if (rc) {
> > +             if (rc != -ENOSPC)
> > +                     goto err;
> > +             rp->end_points = kcalloc(rp->end_point_count,
> > +                                      sizeof(*rp->end_points), GFP_KERNEL);
> > +             if (!rp->end_points) {
> > +                     rc = -ENOMEM;
> > +                     goto err;
> > +             }
> > +             rc = get_rstmem_config(optee, use_case, &min_size, &rp->align,
> > +                                    rp->end_points, &rp->end_point_count);
> > +             if (rc)
> > +                     goto err_kfree_eps;
> > +     }
> > +
> > +     rp->pool.ops = &rstmem_pool_ops_cma;
> > +     rp->optee = optee;
> > +     rp->page_count = min_size / PAGE_SIZE;
> > +     mutex_init(&rp->mutex);
> > +
> > +     return &rp->pool;
> > +
> > +err_kfree_eps:
> > +     kfree(rp->end_points);
> > +err:
> > +     kfree(rp);
> > +     return ERR_PTR(rc);
> > +}
> > --
> > 2.43.0
> >