Messages in this thread | | | Date | Tue, 29 Aug 2023 22:17:41 +0530 | Subject | Re: [PATCH] tee: amdtee: add support for use of cma region | From | Devaraj Rangasamy <> |
| |
On 8/19/2023 4:03 PM, Alexey Kardashevskiy wrote: > On 19/8/23 00:42, Devaraj Rangasamy wrote: >> In systems with low memory configuration, memory gets fragmented >> easily, and any bigger size contiguous memory allocations are likely >> to fail. >> Contiguous Memory Allocator (CMA) is used to overcome this >> limitation, and to guarantee memory allocations. >> >> This patch adds support for CMA area exclusive to amdtee. >> The support can be enabled if kernel have CONFIG_CMA enabled. >> The size can be set via the AMDTEE_CMA_SIZE config option >> at compile time or with the "amdtee_cma" kernel parameter. >> (e.g. "amdtee_cma=32 for 32MB"). >> >> Also, cma zone is utilized only for buffer allocation bigger than >> 64k bytes. When such allocation fails, there is a fallback to the >> buddy allocator. Since CMA requires a boot time initialization, >> it is enabled only when amdtee is built as an inbuilt driver. >> >> Signed-off-by: Devaraj Rangasamy <Devaraj.Rangasamy@amd.com> >> Co-developed-by: SivaSangeetha SK <SivaSangeetha.SK@amd.com> >> Signed-off-by: SivaSangeetha SK <SivaSangeetha.SK@amd.com> >> Reviewed-by: Rijo Thomas <Rijo-john.Thomas@amd.com> >> --- >> .../admin-guide/kernel-parameters.txt | 7 ++ >> arch/x86/include/asm/setup.h | 6 ++ >> arch/x86/kernel/setup.c | 2 + >> drivers/tee/amdtee/Kconfig | 9 +++ >> drivers/tee/amdtee/Makefile | 1 + >> drivers/tee/amdtee/amdtee_private.h | 6 +- >> drivers/tee/amdtee/core.c | 6 +- >> drivers/tee/amdtee/shm_pool.c | 32 ++++++-- >> drivers/tee/amdtee/shm_pool_cma.c | 78 +++++++++++++++++++ >> drivers/tee/amdtee/shm_pool_cma.h | 38 +++++++++ >> 10 files changed, 176 insertions(+), 9 deletions(-) >> create mode 100644 drivers/tee/amdtee/shm_pool_cma.c >> create mode 100644 drivers/tee/amdtee/shm_pool_cma.h >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt >> b/Documentation/admin-guide/kernel-parameters.txt >> index 722b6eca2e93..5e38423f3d53 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -363,6 +363,13 @@ >> selects a performance level in this range and >> appropriate >> to the current workload. >> + amdtee_cma=nn [HW,TEE] >> + Sets the memory size reserved for contiguous memory >> + allocations, to be used by amdtee device driver. >> + Value is in MB and can range from 4 to 128 (MBs) >> + CMA will be active only when CMA is enabled, and amdtee is >> + built as inbuilt driver, and not loaded as module. >> + >> amijoy.map= [HW,JOY] Amiga joystick support >> Map of devices attached to JOY0DAT and JOY1DAT >> Format: <a>,<b> >> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h >> index f3495623ac99..bb5e4b7134a2 100644 >> --- a/arch/x86/include/asm/setup.h >> +++ b/arch/x86/include/asm/setup.h >> @@ -66,6 +66,12 @@ extern void x86_ce4100_early_setup(void); >> static inline void x86_ce4100_early_setup(void) { } >> #endif >> +#if IS_BUILTIN(CONFIG_AMDTEE) && IS_ENABLED(CONFIG_CMA) >> +void amdtee_cma_reserve(void); >> +#else >> +static inline void amdtee_cma_reserve(void) { } >> +#endif >> + >> #ifndef _SETUP >> #include <asm/espfix.h> >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index fd975a4a5200..e73433af3bfa 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -1223,6 +1223,8 @@ void __init setup_arch(char **cmdline_p) >> initmem_init(); >> dma_contiguous_reserve(max_pfn_mapped << PAGE_SHIFT); >> + amdtee_cma_reserve(); >> + >> if (boot_cpu_has(X86_FEATURE_GBPAGES)) >> hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT); >> diff --git a/drivers/tee/amdtee/Kconfig b/drivers/tee/amdtee/Kconfig >> index 191f9715fa9a..5843c739a7b8 100644 >> --- a/drivers/tee/amdtee/Kconfig >> +++ b/drivers/tee/amdtee/Kconfig >> @@ -6,3 +6,12 @@ config AMDTEE >> depends on CRYPTO_DEV_SP_PSP && CRYPTO_DEV_CCP_DD >> help >> This implements AMD's Trusted Execution Environment (TEE) >> driver. >> + >> +config AMDTEE_CMA_SIZE >> + int "Size of Memory in MiB reserved in CMA for AMD-TEE" >> + default "0" >> + depends on CMA && (AMDTEE=y) >> + help >> + Specify the default amount of memory in MiB reserved in CMA >> for AMD-TEE driver >> + Any amdtee shm buffer allocation larger than 64k will allocate >> memory from the CMA >> + The default can be overridden with the kernel commandline >> parameter "amdtee_cma". >> \ No newline at end of file >> diff --git a/drivers/tee/amdtee/Makefile b/drivers/tee/amdtee/Makefile >> index ff1485266117..a197839cfcf3 100644 >> --- a/drivers/tee/amdtee/Makefile >> +++ b/drivers/tee/amdtee/Makefile >> @@ -3,3 +3,4 @@ obj-$(CONFIG_AMDTEE) += amdtee.o >> amdtee-objs += core.o >> amdtee-objs += call.o >> amdtee-objs += shm_pool.o >> +amdtee-objs += shm_pool_cma.o >> diff --git a/drivers/tee/amdtee/amdtee_private.h >> b/drivers/tee/amdtee/amdtee_private.h >> index 6d0f7062bb87..9ba47795adb6 100644 >> --- a/drivers/tee/amdtee/amdtee_private.h >> +++ b/drivers/tee/amdtee/amdtee_private.h >> @@ -87,11 +87,13 @@ struct shmem_desc { >> * struct amdtee_shm_data - Shared memory data >> * @kaddr: Kernel virtual address of shared memory >> * @buf_id: Buffer id of memory mapped by TEE_CMD_ID_MAP_SHARED_MEM >> + * @is_cma: Indicates whether memory is allocated from cma region >> or not >> */ >> struct amdtee_shm_data { >> struct list_head shm_node; >> void *kaddr; >> u32 buf_id; >> + bool is_cma; >> }; >> /** >> @@ -145,9 +147,9 @@ int amdtee_invoke_func(struct tee_context *ctx, >> int amdtee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 >> session); >> -int amdtee_map_shmem(struct tee_shm *shm); >> +int amdtee_map_shmem(struct tee_shm *shm, bool is_cma); >> -void amdtee_unmap_shmem(struct tee_shm *shm); >> +void amdtee_unmap_shmem(struct tee_shm *shm, bool *is_cma); >> int handle_load_ta(void *data, u32 size, >> struct tee_ioctl_open_session_arg *arg); >> diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c >> index 372d64756ed6..448802dccf13 100644 >> --- a/drivers/tee/amdtee/core.c >> +++ b/drivers/tee/amdtee/core.c >> @@ -336,7 +336,7 @@ int amdtee_close_session(struct tee_context *ctx, >> u32 session) >> return 0; >> } >> -int amdtee_map_shmem(struct tee_shm *shm) >> +int amdtee_map_shmem(struct tee_shm *shm, bool is_cma) >> { >> struct amdtee_context_data *ctxdata; >> struct amdtee_shm_data *shmnode; >> @@ -368,6 +368,7 @@ int amdtee_map_shmem(struct tee_shm *shm) >> shmnode->kaddr = shm->kaddr; >> shmnode->buf_id = buf_id; >> + shmnode->is_cma = is_cma; >> ctxdata = shm->ctx->data; >> mutex_lock(&ctxdata->shm_mutex); >> list_add(&shmnode->shm_node, &ctxdata->shm_list); >> @@ -378,7 +379,7 @@ int amdtee_map_shmem(struct tee_shm *shm) >> return 0; >> } >> -void amdtee_unmap_shmem(struct tee_shm *shm) >> +void amdtee_unmap_shmem(struct tee_shm *shm, bool *is_cma) >> { >> struct amdtee_context_data *ctxdata; >> struct amdtee_shm_data *shmnode; >> @@ -395,6 +396,7 @@ void amdtee_unmap_shmem(struct tee_shm *shm) >> mutex_lock(&ctxdata->shm_mutex); >> list_for_each_entry(shmnode, &ctxdata->shm_list, shm_node) >> if (buf_id == shmnode->buf_id) { >> + *is_cma = shmnode->is_cma; >> list_del(&shmnode->shm_node); >> kfree(shmnode); >> break; >> diff --git a/drivers/tee/amdtee/shm_pool.c >> b/drivers/tee/amdtee/shm_pool.c >> index f0303126f199..9aad401387be 100644 >> --- a/drivers/tee/amdtee/shm_pool.c >> +++ b/drivers/tee/amdtee/shm_pool.c >> @@ -7,19 +7,30 @@ >> #include <linux/tee_drv.h> >> #include <linux/psp.h> >> #include "amdtee_private.h" >> +#include "shm_pool_cma.h" >> static int pool_op_alloc(struct tee_shm_pool *pool, struct >> tee_shm *shm, >> size_t size, size_t align) >> { >> unsigned int order = get_order(size); >> unsigned long va; >> + bool is_cma = false; >> int rc; >> /* >> * Ignore alignment since this is already going to be page aligned >> * and there's no need for any larger alignment. >> */ >> - va = __get_free_pages(GFP_KERNEL | __GFP_ZERO, order); >> + >> + /* if CMA is available, use it for higher order allocation */ >> + if (amdtee_get_cma_size() && order > 6) >> + va = amdtee_alloc_from_cma(shm, order); >> + >> + if (va) >> + is_cma = true; >> + else >> + va = __get_free_pages(GFP_KERNEL | __GFP_ZERO, order); >> + >> if (!va) >> return -ENOMEM; >> @@ -28,9 +39,13 @@ static int pool_op_alloc(struct tee_shm_pool >> *pool, struct tee_shm *shm, >> shm->size = PAGE_SIZE << order; >> /* Map the allocated memory in to TEE */ >> - rc = amdtee_map_shmem(shm); >> + rc = amdtee_map_shmem(shm, is_cma); >> if (rc) { >> - free_pages(va, order); >> + if (is_cma) >> + amdtee_free_from_cma(shm); >> + else >> + free_pages(va, order); >> + >> shm->kaddr = NULL; >> return rc; >> } >> @@ -40,9 +55,16 @@ static int pool_op_alloc(struct tee_shm_pool >> *pool, struct tee_shm *shm, >> static void pool_op_free(struct tee_shm_pool *pool, struct >> tee_shm *shm) >> { >> + bool is_cma = false; >> + >> /* Unmap the shared memory from TEE */ >> - amdtee_unmap_shmem(shm); >> - free_pages((unsigned long)shm->kaddr, get_order(shm->size)); >> + amdtee_unmap_shmem(shm, &is_cma); >> + >> + if (is_cma) > > No need in is_cma here and other places. Ack. cma_release() do have inbuilt validation, and returns true/false as result for valid cma buffer release. > > >> + amdtee_free_from_cma(shm); >> + else >> + free_pages((unsigned long)shm->kaddr, get_order(shm->size)); >> + >> shm->kaddr = NULL; >> } >> diff --git a/drivers/tee/amdtee/shm_pool_cma.c >> b/drivers/tee/amdtee/shm_pool_cma.c >> new file mode 100644 >> index 000000000000..99dda9adb1c6 >> --- /dev/null >> +++ b/drivers/tee/amdtee/shm_pool_cma.c >> @@ -0,0 +1,78 @@ >> +// SPDX-License-Identifier: MIT >> +/* >> + * Copyright 2023 Advanced Micro Devices, Inc. >> + */ >> + >> +#if IS_BUILTIN(CONFIG_AMDTEE) && IS_ENABLED(CONFIG_CMA) >> + >> +#define pr_fmt(fmt) "%s: " fmt, __func__ >> + >> +#include <linux/cma.h> >> +#include <linux/mm.h> >> +#include <linux/tee_drv.h> >> +#include "shm_pool_cma.h" >> + >> +static struct cma *amdtee_cma; >> +unsigned long amdtee_cma_size __initdata = CONFIG_AMDTEE_CMA_SIZE * >> SZ_1M; >> + >> +static int __init early_parse_amdtee_cma(char *p) >> +{ >> + int cmd_size; > > Not int but unsigned long long. Ack. We have reverted new exclusive cma zone. > >> + >> + if (!p) >> + return 1; >> + >> + cmd_size = memparse(p, NULL); >> + if (cmd_size >= 4 && cmd_size <= 256) >> + amdtee_cma_size = cmd_size * SZ_1M; > > Change the doc to use 4M and 256M instead of plain 4 and 256. Or stop > using memparse(). > > /me sad. > Ack. Thanks for review. > >> + else >> + pr_err("invalid amdtee_cma size: %lu\n", amdtee_cma_size); >> + >> + return 0; >> +} >> +early_param("amdtee_cma", early_parse_amdtee_cma); >> + >> +void __init amdtee_cma_reserve(void) >> +{ >> + int ret; >> + >> + ret = cma_declare_contiguous(0, amdtee_cma_size, 0, 0, 0, false, >> "amdtee", &amdtee_cma); >> + if (ret) >> + pr_err("Failed to reserve CMA region of size %lu\n", >> amdtee_cma_size); >> + else >> + pr_info("Reserved %lu bytes CMA for amdtee\n", >> amdtee_cma_size); >> +} >> + >> +unsigned long amdtee_alloc_from_cma(struct tee_shm *shm, unsigned >> int order) >> +{ >> + struct page *page = NULL; >> + unsigned long va = 0; >> + int nr_pages = 0; >> + >> + if (amdtee_cma) { >> + nr_pages = 1 << order; >> + page = cma_alloc(amdtee_cma, nr_pages, 0, false); >> + if (page) >> + va = (unsigned long)page_to_virt(page); >> + else >> + pr_debug("failed to allocate from CMA region\n"); >> + } else { >> + pr_debug("CMA region is not available\n"); >> + } >> + return va; >> +} >> + >> +void amdtee_free_from_cma(struct tee_shm *shm) >> +{ >> + struct page *page; >> + int nr_pages = 0; >> + >> + if (amdtee_cma) { >> + nr_pages = 1 << get_order(shm->size); >> + page = virt_to_page(shm->kaddr); >> + cma_release(amdtee_cma, page, nr_pages); >> + } else { >> + pr_err("CMA region is not available\n"); >> + } >> +} >> +#endif /* IS_BUILTIN(CONFIG_AMDTEE) && IS_ENABLED(CONFIG_CMA) */ >> diff --git a/drivers/tee/amdtee/shm_pool_cma.h >> b/drivers/tee/amdtee/shm_pool_cma.h >> new file mode 100644 >> index 000000000000..d1cde11cbede >> --- /dev/null >> +++ b/drivers/tee/amdtee/shm_pool_cma.h >> @@ -0,0 +1,38 @@ >> +/* SPDX-License-Identifier: MIT */ >> + >> +/* >> + * Copyright 2023 Advanced Micro Devices, Inc. >> + */ >> + >> +#ifndef SHM_POOL_CMA_H >> +#define SHM_POOL_CMA_H >> + >> +#if IS_BUILTIN(CONFIG_AMDTEE) && IS_ENABLED(CONFIG_CMA) >> + >> +extern unsigned long amdtee_cma_size; >> + >> +static inline unsigned long amdtee_get_cma_size(void) >> +{ >> + return amdtee_cma_size; >> +} >> + >> +unsigned long amdtee_alloc_from_cma(struct tee_shm *shm, unsigned >> int order); >> + >> +void amdtee_free_from_cma(struct tee_shm *shm); >> + >> +#else /* IS_BUILTIN(CONFIG_AMDTEE) && IS_ENABLED(CONFIG_CMA) */ >> + >> +static inline unsigned long amdtee_get_cma_size(void) >> +{ >> + return 0; >> +} >> + >> +static inline unsigned long amdtee_alloc_from_cma(struct tee_shm >> *shm, unsigned int order) >> +{ >> + return 0; >> +} >> + >> +static inline void amdtee_free_from_cma(struct tee_shm *shm) { } >> + >> +#endif /* IS_BUILTIN(CONFIG_AMDTEE) && IS_ENABLED(CONFIG_CMA) */ >> +#endif /* SHM_POOL_CMA_H */ >
| |