From d014ef60644d61b84c7eaf94e3ba4ff16ac36dcc Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Thu, 24 Sep 2020 09:24:12 -0600 Subject: [PATCH 1/3] msm: kgsl: Make sure that pool pages don't have any extra references Before putting a page back in the pool be sure that it doesn't have any additional references that would be a signal that somebody else is looking at the page and that it would be a bad idea to keep it around and run the risk of accidentally handing it to a different process. Change-Id: Ic0dedbad0cf2ffb34b76ad23e393c5a911114b82 Signed-off-by: Jordan Crouse --- drivers/gpu/msm/kgsl_pool.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/msm/kgsl_pool.c b/drivers/gpu/msm/kgsl_pool.c index 9f7e41660c1c..a6c726fbd990 100644 --- a/drivers/gpu/msm/kgsl_pool.c +++ b/drivers/gpu/msm/kgsl_pool.c @@ -96,6 +96,15 @@ _kgsl_pool_add_page(struct kgsl_page_pool *pool, struct page *p) if (!p) return; + /* + * Sanity check to make sure we don't re-pool a page that + * somebody else has a reference to. + */ + if (WARN_ON(unlikely(page_count(p) > 1))) { + __free_pages(p, pool->pool_order); + return; + } + spin_lock(&pool->list_lock); list_add_tail(&p->lru, &pool->page_list); pool->page_count++; From 0977dc6d53bc37d7add3d23f96b49f25b50e2816 Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Thu, 24 Sep 2020 15:43:12 -0600 Subject: [PATCH 2/3] msm: kgsl: Use system memory for kernel globals Kernel global GPU memory usually lasts for the life of the driver. It doesn't make any sense to take them out of the KGSL pool since they will never be recycled. Add a flag to allocate 4k pages from system memory and use them by default for global objects. Change-Id: Ic0dedbadd0ff1f558d5f9711ec9843451593aee4 Signed-off-by: Jordan Crouse --- drivers/gpu/msm/kgsl.h | 2 + drivers/gpu/msm/kgsl_sharedmem.c | 124 +++++++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/msm/kgsl.h b/drivers/gpu/msm/kgsl.h index 7116fedc5716..219b5d9d233b 100644 --- a/drivers/gpu/msm/kgsl.h +++ b/drivers/gpu/msm/kgsl.h @@ -176,6 +176,8 @@ struct kgsl_memdesc_ops { #define KGSL_MEMDESC_UCODE BIT(7) /* For global buffers, randomly assign an address from the region */ #define KGSL_MEMDESC_RANDOM BIT(8) +/* Allocate memory from the system instead of the pools */ +#define KGSL_MEMDESC_SYSMEM BIT(9) /** * struct kgsl_memdesc - GPU memory object descriptor diff --git a/drivers/gpu/msm/kgsl_sharedmem.c b/drivers/gpu/msm/kgsl_sharedmem.c index c4d3ee7e6967..f09b95701e5d 100644 --- a/drivers/gpu/msm/kgsl_sharedmem.c +++ b/drivers/gpu/msm/kgsl_sharedmem.c @@ -862,6 +862,38 @@ static void kgsl_contiguous_free(struct kgsl_memdesc *memdesc) } #if IS_ENABLED(CONFIG_QCOM_SECURE_BUFFER) +static void kgsl_free_secure_system_pages(struct kgsl_memdesc *memdesc) +{ + int i; + struct scatterlist *sg; + int ret = unlock_sgt(memdesc->sgt); + + if (ret) { + /* + * Unlock of the secure buffer failed. This buffer will + * be stuck in secure side forever and is unrecoverable. + * Give up on the buffer and don't return it to the + * pool. + */ + pr_err("kgsl: secure buf unlock failed: gpuaddr: %llx size: %llx ret: %d\n", + memdesc->gpuaddr, memdesc->size, ret); + return; + } + + atomic_long_sub(memdesc->size, &kgsl_driver.stats.secure); + + for_each_sg(memdesc->sgt->sgl, sg, memdesc->sgt->nents, i) { + struct page *page = sg_page(sg); + + __free_pages(page, get_order(PAGE_SIZE)); + } + + sg_free_table(memdesc->sgt); + kfree(memdesc->sgt); + + memdesc->sgt = NULL; +} + static void kgsl_free_secure_pool_pages(struct kgsl_memdesc *memdesc) { int ret = unlock_sgt(memdesc->sgt); @@ -904,6 +936,23 @@ static void kgsl_free_pool_pages(struct kgsl_memdesc *memdesc) memdesc->pages = NULL; } +static void kgsl_free_system_pages(struct kgsl_memdesc *memdesc) +{ + int i; + + kgsl_paged_unmap_kernel(memdesc); + WARN_ON(memdesc->hostptr); + + atomic_long_sub(memdesc->size, &kgsl_driver.stats.page_alloc); + + for (i = 0; i < memdesc->page_count; i++) + __free_pages(memdesc->pages[i], get_order(PAGE_SIZE)); + + memdesc->page_count = 0; + kvfree(memdesc->pages); + memdesc->pages = NULL; +} + static struct kgsl_memdesc_ops kgsl_contiguous_ops = { .free = kgsl_contiguous_free, .vmflags = VM_DONTDUMP | VM_PFNMAP | VM_DONTEXPAND | VM_DONTCOPY, @@ -911,6 +960,11 @@ static struct kgsl_memdesc_ops kgsl_contiguous_ops = { }; #if IS_ENABLED(CONFIG_QCOM_SECURE_BUFFER) +static struct kgsl_memdesc_ops kgsl_secure_system_ops = { + .free = kgsl_free_secure_system_pages, + /* FIXME: Make sure vmflags / vmfault does the right thing here */ +}; + static struct kgsl_memdesc_ops kgsl_secure_pool_ops = { .free = kgsl_free_secure_pool_pages, /* FIXME: Make sure vmflags / vmfault does the right thing here */ @@ -925,6 +979,49 @@ static struct kgsl_memdesc_ops kgsl_pool_ops = { .unmap_kernel = kgsl_paged_unmap_kernel, }; +static struct kgsl_memdesc_ops kgsl_system_ops = { + .free = kgsl_free_system_pages, + .vmflags = VM_DONTDUMP | VM_DONTEXPAND | VM_DONTCOPY | VM_MIXEDMAP, + .vmfault = kgsl_paged_vmfault, + .map_kernel = kgsl_paged_map_kernel, + .unmap_kernel = kgsl_paged_unmap_kernel, +}; + +static int kgsl_system_alloc_pages(u64 size, struct page ***pages, + struct device *dev) +{ + struct scatterlist sg; + struct page **local; + int i, npages = size >> PAGE_SHIFT; + + local = kvcalloc(npages, sizeof(*pages), GFP_KERNEL | __GFP_NORETRY); + if (!local) + return -ENOMEM; + + for (i = 0; i < npages; i++) { + gfp_t gfp = __GFP_ZERO | __GFP_HIGHMEM | + GFP_KERNEL | __GFP_NORETRY; + + local[i] = alloc_pages(gfp, get_order(PAGE_SIZE)); + if (!local[i]) { + for (i = i - 1; i >= 0; i--) + __free_pages(local[i], get_order(PAGE_SIZE)); + kvfree(local); + return -ENOMEM; + } + + /* Make sure the cache is clean */ + sg_init_table(&sg, 1); + sg_set_page(&sg, local[i], PAGE_SIZE, 0); + sg_dma_address(&sg) = page_to_phys(local[i]); + + dma_sync_sg_for_device(dev, &sg, 1, DMA_BIDIRECTIONAL); + } + + *pages = local; + return npages; +} + #if IS_ENABLED(CONFIG_QCOM_SECURE_BUFFER) static int kgsl_alloc_secure_pages(struct kgsl_device *device, struct kgsl_memdesc *memdesc, u64 size, u64 flags, u32 priv) @@ -942,8 +1039,13 @@ static int kgsl_alloc_secure_pages(struct kgsl_device *device, kgsl_memdesc_init(device, memdesc, flags); memdesc->priv |= priv; - memdesc->ops = &kgsl_secure_pool_ops; - count = kgsl_pool_alloc_pages(size, &pages, device->dev); + if (priv & KGSL_MEMDESC_SYSMEM) { + memdesc->ops = &kgsl_secure_system_ops; + count = kgsl_system_alloc_pages(size, &pages, device->dev); + } else { + memdesc->ops = &kgsl_secure_pool_ops; + count = kgsl_pool_alloc_pages(size, &pages, device->dev); + } if (count < 0) return count; @@ -997,8 +1099,13 @@ static int kgsl_alloc_pages(struct kgsl_device *device, kgsl_memdesc_init(device, memdesc, flags); memdesc->priv |= priv; - memdesc->ops = &kgsl_pool_ops; - count = kgsl_pool_alloc_pages(size, &pages, device->dev); + if (priv & KGSL_MEMDESC_SYSMEM) { + memdesc->ops = &kgsl_system_ops; + count = kgsl_system_alloc_pages(size, &pages, device->dev); + } else { + memdesc->ops = &kgsl_pool_ops; + count = kgsl_pool_alloc_pages(size, &pages, device->dev); + } if (count < 0) return count; @@ -1158,7 +1265,8 @@ kgsl_allocate_secure_global(struct kgsl_device *device, if (!md) return ERR_PTR(-ENOMEM); - priv |= KGSL_MEMDESC_GLOBAL; + /* Make sure that we get global memory from system memory */ + priv |= KGSL_MEMDESC_GLOBAL | KGSL_MEMDESC_SYSMEM; ret = kgsl_allocate_secure(device, &md->memdesc, size, flags, priv); if (ret) { @@ -1197,7 +1305,11 @@ struct kgsl_memdesc *kgsl_allocate_global(struct kgsl_device *device, if (!md) return ERR_PTR(-ENOMEM); - priv |= KGSL_MEMDESC_GLOBAL; + /* + * Make sure that we get global memory from system memory to keep from + * taking up pool memory for the life of the driver + */ + priv |= KGSL_MEMDESC_GLOBAL | KGSL_MEMDESC_SYSMEM; ret = kgsl_allocate_kernel(device, &md->memdesc, size, flags, priv); if (ret) { From c5f2680903225e12bcb0ce1806b7efb0c91bf1d5 Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Thu, 10 Sep 2020 09:59:48 -0600 Subject: [PATCH 3/3] msm: kgsl: Fix kvcalloc() calls Depending on user input the size of the page struct that we try to allocate with kvcalloc() could be way too big. Add __GFP_NOWARN and __GFP_NORETRY to keep from invoking the OOM killer in these cases. Also remove an unneeded usage of kvmalloc() since the maximum allocation size for the HFI command will only be 1k as enforced on the previous line. Change-Id: Ic0dedbad8d09544bdc0070b9b797616ee1f672d4 Signed-off-by: Jordan Crouse --- drivers/gpu/msm/adreno_a6xx_hwsched_hfi.c | 4 ++-- drivers/gpu/msm/kgsl.c | 3 ++- drivers/gpu/msm/kgsl_pool.c | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/msm/adreno_a6xx_hwsched_hfi.c b/drivers/gpu/msm/adreno_a6xx_hwsched_hfi.c index 10914ffe7e5b..1fe5af4075c6 100644 --- a/drivers/gpu/msm/adreno_a6xx_hwsched_hfi.c +++ b/drivers/gpu/msm/adreno_a6xx_hwsched_hfi.c @@ -1200,7 +1200,7 @@ int a6xx_hwsched_submit_cmdobj(struct adreno_device *adreno_dev, if (WARN_ON(cmd_sizebytes > HFI_MAX_MSG_SIZE)) return -EMSGSIZE; - cmd = kvmalloc(cmd_sizebytes, GFP_KERNEL); + cmd = kmalloc(cmd_sizebytes, GFP_KERNEL); if (cmd == NULL) return -ENOMEM; @@ -1264,7 +1264,7 @@ skipib: adreno_profile_submit_time(&time); free: - kvfree(cmd); + kfree(cmd); return ret; } diff --git a/drivers/gpu/msm/kgsl.c b/drivers/gpu/msm/kgsl.c index 3a466a20d8a6..c1ab0d79b616 100644 --- a/drivers/gpu/msm/kgsl.c +++ b/drivers/gpu/msm/kgsl.c @@ -2346,7 +2346,8 @@ static int memdesc_sg_virt(struct kgsl_memdesc *memdesc, struct file *vmfile) if (sglen == 0 || sglen >= LONG_MAX) return -EINVAL; - pages = kvcalloc(sglen, sizeof(*pages), GFP_KERNEL); + pages = kvcalloc(sglen, sizeof(*pages), + GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN); if (pages == NULL) return -ENOMEM; diff --git a/drivers/gpu/msm/kgsl_pool.c b/drivers/gpu/msm/kgsl_pool.c index a6c726fbd990..7e7326ee304b 100644 --- a/drivers/gpu/msm/kgsl_pool.c +++ b/drivers/gpu/msm/kgsl_pool.c @@ -442,7 +442,8 @@ int kgsl_pool_alloc_pages(u64 size, struct page ***pages, struct device *dev) { int count = 0; int npages = size >> PAGE_SHIFT; - struct page **local = kvcalloc(npages, sizeof(*local), GFP_KERNEL); + struct page **local = kvcalloc(npages, sizeof(*local), + GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN); u32 page_size, align; u64 len = size;