On 8/18/2023 8:56 PM, Dave Hansen wrote:
On 8/18/23 07:42, Devaraj Rangasamy wrote:
--- 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);
Right now, we have *A* global CMA pool set up in
dma_contiguous_reserve() that everyone shares.
Why does this *one* driver deserve to be a special snowflake and get its
own private CMA area and own command-line options?
It seems to me like you should just tell users to set
CONFIG_CMA_SIZE_MBYTES or use cma=size on the command-line and just use
the global pool. If you want to make it a special snowflake, there's a
much higher bar to clear, and there's zero justification for that right now.
Ack.
We had exclusive cma zone due to concern over contention for generic cma
zone, and to guarentee buffer allocation. We profiled and migrated to
generic cma zone.
Oh, and this:
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;
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);
is goofy. It either only needs powers-of-2 and can take "order" as an
argument instead of 'size', or it should be using alloc_pages_exact() to
avoid wasting memory.
Ack.
This was inherently introduced by __get_free_pages() requirement, and
constly indeed.
alloc_pages_exact() do call __get_free_pages() internally, and doesnot
prevent memory fragmentation. but at least unnecessary pages are
returned back to free pool.
We will update the section accordingly, and post updated patch. Thanks
for the review.
