2023-11-16 12:37:18

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] optee: allocate shared memory with alloc_pages_exact()

On Tue, 14 Nov 2023 at 15:22, Jens Wiklander <[email protected]> wrote:
>
> Allocate memory to share with the secure using alloc_pages_exact()

s/with the secure using/with the secure world using/

> instead of alloc_pages() for more efficient memory usage.
>
> Signed-off-by: Jens Wiklander <[email protected]>
> ---
> drivers/tee/optee/core.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>

Apart from nit above, feel free to add:

Reviewed-by: Sumit Garg <[email protected]>

-Sumit

> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 38ea2fecfc2e..4a4b03b4fc7d 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -26,10 +26,8 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> size_t num_pages,
> unsigned long start))
> {
> - unsigned int order = get_order(size);
> - unsigned int nr_pages = 1 << order;
> + size_t nr_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
> struct page **pages;
> - struct page *page;
> unsigned int i;
> int rc = 0;
>
> @@ -37,13 +35,13 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> * Ignore alignment since this is already going to be page aligned
> * and there's no need for any larger alignment.
> */
> - page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> - if (!page)
> + shm->kaddr = alloc_pages_exact(nr_pages * PAGE_SIZE,
> + GFP_KERNEL | __GFP_ZERO);
> + if (!shm->kaddr)
> return -ENOMEM;
>
> - shm->kaddr = page_address(page);
> - shm->paddr = page_to_phys(page);
> - shm->size = PAGE_SIZE << order;
> + shm->paddr = virt_to_phys(shm->kaddr);
> + shm->size = nr_pages * PAGE_SIZE;
>
> pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> if (!pages) {
> @@ -52,7 +50,7 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> }
>
> for (i = 0; i < nr_pages; i++)
> - pages[i] = page + i;
> + pages[i] = virt_to_page((u8 *)shm->kaddr + i * PAGE_SIZE);
>
> shm->pages = pages;
> shm->num_pages = nr_pages;
> @@ -66,7 +64,7 @@ int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
>
> return 0;
> err:
> - free_pages((unsigned long)shm->kaddr, order);
> + free_pages_exact(shm->kaddr, shm->size);
> shm->kaddr = NULL;
> return rc;
> }
> @@ -77,7 +75,7 @@ void optee_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
> {
> if (shm_unregister)
> shm_unregister(shm->ctx, shm);
> - free_pages((unsigned long)shm->kaddr, get_order(shm->size));
> + free_pages_exact(shm->kaddr, shm->size);
> shm->kaddr = NULL;
> kfree(shm->pages);
> shm->pages = NULL;
> --
> 2.34.1
>