Hi Jens,
On 3 Oct 2023, at 17:30, Jens Wiklander wrote:
On Wed, Sep 27, 2023 at 05:21:46PM +0200, Balint Dobszay wrote:
[snip]
diff --git a/drivers/tee/tstee/shm_pool.c b/drivers/tee/tstee/shm_pool.c new file mode 100644 index 000000000000..518c10dd0735 --- /dev/null +++ b/drivers/tee/tstee/shm_pool.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- Copyright (c) 2023, Arm Limited
- */
+#include <linux/arm_ffa.h> +#include <linux/device.h> +#include <linux/dma-buf.h> +#include <linux/genalloc.h> +#include <linux/slab.h> +#include <linux/tee_drv.h>
+#include "tstee_private.h"
+static int pool_op_alloc(struct tee_shm_pool *pool __always_unused, struct tee_shm *shm,
size_t size, size_t align __always_unused)+{
- unsigned int order, nr_pages, i;
- struct page *page, **pages;
- int rc;
- if (size == 0)
return -EINVAL;- order = get_order(size);
- nr_pages = 1 << order;
- page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
- if (!page)
return -ENOMEM;- shm->kaddr = page_address(page);
- shm->paddr = page_to_phys(page);
- shm->size = PAGE_SIZE << order;
- pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
- if (!pages) {
rc = -ENOMEM;goto err;- }
- for (i = 0; i < nr_pages; i++)
pages[i] = page + i;- rc = tstee_shm_register(shm->ctx, shm, pages, nr_pages, (unsigned long)shm->kaddr);
- kfree(pages);
- if (rc)
goto err;- return 0;
+err:
- __free_pages(page, order);
- return rc;
+}
+static void pool_op_free(struct tee_shm_pool *pool __always_unused, struct tee_shm *shm) +{
- int rc;
- rc = tstee_shm_unregister(shm->ctx, shm);
- if (rc)
pr_err("shm id 0x%llx unregister rc %d\n", shm->sec_world_id, rc);- free_pages((unsigned long)shm->kaddr, get_order(shm->size));
- shm->kaddr = NULL;
+}
+static void pool_op_destroy_pool(struct tee_shm_pool *pool) +{
- kfree(pool);
+}
These pool_op functions look very much like the optee_pool_op functions. Compare this with how the pool_ffa_ops is implemented in the OP-TEE driver. We should consider factoring out the optee_pool_op functions into the TEE subsystem instead so we can avoid reimplementing the same thing.
Makes sense, thanks. I'll propose a patch for this in the next version.
Regards, Balint