msm: kgsl: Fix pagetable member of struct kgsl_memdesc

memdesc->pagetable is supposed to help ensure that memory gets
unmapped before it is freed, but the pagetable member is being
populated at create time not when the buffer gets mapped. This
forces the developer to ensure that the same pagetable is
used for both the create and map step. Instead, assign the
pagetable member when it is first used (to get a GPU address)
and put it away when the GPU address is released.

Change-Id: Ic0dedbad372fd9029b932dd99633a650049751ed
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

7 files changed, 60 insertions, 88 deletions

diff --git a/drivers/gpu/msm/adreno_a5xx.c b/drivers/gpu/msm/adreno_a5xx.c
index 2891940b8f5b..38ed2d1e8a99 100644
--- a/drivers/gpu/msm/adreno_a5xx.c
+++ b/drivers/gpu/msm/adreno_a5xx.c
@@ -252,7 +252,7 @@ static int a5xx_critical_packet_construct(struct adreno_device *adreno_dev)
 		return ret;
 
 	ret = kgsl_allocate_user(&adreno_dev->dev, &crit_pkts_refbuf0,
-					NULL, PAGE_SIZE, KGSL_MEMFLAGS_SECURE);
+		PAGE_SIZE, KGSL_MEMFLAGS_SECURE);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/msm/kgsl.c b/drivers/gpu/msm/kgsl.c
index add4590bbb90..01966bb31a09 100644
--- a/drivers/gpu/msm/kgsl.c
+++ b/drivers/gpu/msm/kgsl.c
@@ -380,24 +380,6 @@ kgsl_mem_entry_track_gpuaddr(struct kgsl_process_private *process,
 	return kgsl_mmu_get_gpuaddr(pagetable, &entry->memdesc);
 }
 
-/**
- * kgsl_mem_entry_untrack_gpuaddr() - Untrack memory that is previously tracked
- * process - Pointer to process private to which memory belongs
- * entry - Memory entry to untrack
- *
- * Function just does the opposite of kgsl_mem_entry_track_gpuaddr. Needs to be
- * called with processes spin lock held
- */
-static void
-kgsl_mem_entry_untrack_gpuaddr(struct kgsl_process_private *process,
-				struct kgsl_mem_entry *entry)
-{
-	struct kgsl_pagetable *pagetable = entry->memdesc.pagetable;
-
-	if (entry->memdesc.gpuaddr)
-		kgsl_mmu_put_gpuaddr(pagetable, &entry->memdesc);
-}
-
 /* Commit the entry to the process so it can be accessed by other operations */
 static void kgsl_mem_entry_commit_process(struct kgsl_mem_entry *entry)
 {
@@ -446,7 +428,7 @@ kgsl_mem_entry_attach_process(struct kgsl_mem_entry *entry,
 
 	if (id < 0) {
 		ret = id;
-		kgsl_mem_entry_untrack_gpuaddr(process, entry);
+		kgsl_mmu_put_gpuaddr(&entry->memdesc);
 		goto err_put_proc_priv;
 	}
 
@@ -485,7 +467,7 @@ err_put_proc_priv:
 static void kgsl_mem_entry_detach_process(struct kgsl_mem_entry *entry)
 {
 	unsigned int type;
-	int ret;
+
 	if (entry == NULL)
 		return;
 
@@ -502,14 +484,7 @@ static void kgsl_mem_entry_detach_process(struct kgsl_mem_entry *entry)
 	entry->priv->stats[type].cur -= entry->memdesc.size;
 	spin_unlock(&entry->priv->mem_lock);
 
-	ret = kgsl_mmu_unmap(entry->memdesc.pagetable, &entry->memdesc);
-	/*
-	 * Do not free the gpuaddr/size if unmap fails. Because if we try
-	 * to map this range in future, the iommu driver will throw
-	 * a BUG_ON() because it feels we are overwriting a mapping.
-	 */
-	if (ret == 0)
-		kgsl_mem_entry_untrack_gpuaddr(entry->priv, entry);
+	kgsl_mmu_put_gpuaddr(&entry->memdesc);
 
 	kgsl_process_private_put(entry->priv);
 
@@ -3048,7 +3023,7 @@ static struct kgsl_mem_entry *gpumem_alloc_entry(
 		entry->memdesc.priv |= KGSL_MEMDESC_SECURE;
 
 	ret = kgsl_allocate_user(dev_priv->device, &entry->memdesc,
-				private->pagetable, size, flags);
+		size, flags);
 	if (ret != 0)
 		goto err;
 
@@ -3251,7 +3226,7 @@ long kgsl_ioctl_sparse_phys_alloc(struct kgsl_device_private *dev_priv,
 	kgsl_memdesc_set_align(&entry->memdesc, ilog2(param->pagesize));
 
 	ret = kgsl_allocate_user(dev_priv->device, &entry->memdesc,
-			process->pagetable, param->size, entry->memdesc.flags);
+			param->size, entry->memdesc.flags);
 	if (ret)
 		goto err_remove_idr;
 
@@ -4016,11 +3991,11 @@ static unsigned long _gpu_set_svm_region(struct kgsl_process_private *private,
 		return ret;
 
 	entry->memdesc.gpuaddr = (uint64_t) addr;
+	entry->memdesc.pagetable = private->pagetable;
 
 	ret = kgsl_mmu_map(private->pagetable, &entry->memdesc);
 	if (ret) {
-		kgsl_mmu_put_gpuaddr(private->pagetable,
-				&entry->memdesc);
+		kgsl_mmu_put_gpuaddr(&entry->memdesc);
 		return ret;
 	}
 
diff --git a/drivers/gpu/msm/kgsl_iommu.c b/drivers/gpu/msm/kgsl_iommu.c
index 9f35a3197a4c..bc681057250d 100644
--- a/drivers/gpu/msm/kgsl_iommu.c
+++ b/drivers/gpu/msm/kgsl_iommu.c
@@ -1403,17 +1403,16 @@ static int _setstate_alloc(struct kgsl_device *device,
 {
 	int ret;
 
-	ret = kgsl_sharedmem_alloc_contig(device, &iommu->setstate, NULL,
-		PAGE_SIZE);
-	if (ret)
-		return ret;
+	ret = kgsl_sharedmem_alloc_contig(device, &iommu->setstate, PAGE_SIZE);
 
-	/* Mark the setstate memory as read only */
-	iommu->setstate.flags |= KGSL_MEMFLAGS_GPUREADONLY;
+	if (!ret) {
+		/* Mark the setstate memory as read only */
+		iommu->setstate.flags |= KGSL_MEMFLAGS_GPUREADONLY;
 
-	kgsl_sharedmem_set(device, &iommu->setstate, 0, 0, PAGE_SIZE);
+		kgsl_sharedmem_set(device, &iommu->setstate, 0, 0, PAGE_SIZE);
+	}
 
-	return 0;
+	return ret;
 }
 
 static int kgsl_iommu_init(struct kgsl_mmu *mmu)
@@ -1663,7 +1662,7 @@ static int _iommu_map_guard_page(struct kgsl_pagetable *pt,
 
 		if (!kgsl_secure_guard_page_memdesc.sgt) {
 			if (kgsl_allocate_user(KGSL_MMU_DEVICE(pt->mmu),
-					&kgsl_secure_guard_page_memdesc, pt,
+					&kgsl_secure_guard_page_memdesc,
 					sgp_size, KGSL_MEMFLAGS_SECURE)) {
 				KGSL_CORE_ERR(
 					"Secure guard page alloc failed\n");
@@ -2364,23 +2363,27 @@ static int kgsl_iommu_get_gpuaddr(struct kgsl_pagetable *pagetable,
 	}
 
 	ret = _insert_gpuaddr(pagetable, addr, size);
-	if (ret == 0)
+	if (ret == 0) {
 		memdesc->gpuaddr = addr;
+		memdesc->pagetable = pagetable;
+	}
 
 out:
 	spin_unlock(&pagetable->lock);
 	return ret;
 }
 
-static void kgsl_iommu_put_gpuaddr(struct kgsl_pagetable *pagetable,
-		struct kgsl_memdesc *memdesc)
+static void kgsl_iommu_put_gpuaddr(struct kgsl_memdesc *memdesc)
 {
-	spin_lock(&pagetable->lock);
+	if (memdesc->pagetable == NULL)
+		return;
+
+	spin_lock(&memdesc->pagetable->lock);
 
-	if (_remove_gpuaddr(pagetable, memdesc->gpuaddr))
+	if (_remove_gpuaddr(memdesc->pagetable, memdesc->gpuaddr))
 		BUG();
 
-	spin_unlock(&pagetable->lock);
+	spin_unlock(&memdesc->pagetable->lock);
 }
 
 static int kgsl_iommu_svm_range(struct kgsl_pagetable *pagetable,
diff --git a/drivers/gpu/msm/kgsl_mmu.c b/drivers/gpu/msm/kgsl_mmu.c
index ba564b2851f9..0df1ad02f16e 100644
--- a/drivers/gpu/msm/kgsl_mmu.c
+++ b/drivers/gpu/msm/kgsl_mmu.c
@@ -414,17 +414,29 @@ EXPORT_SYMBOL(kgsl_mmu_map);
  * @pagetable: Pagetable to release the memory from
  * @memdesc: Memory descriptor containing the GPU address to free
  */
-void kgsl_mmu_put_gpuaddr(struct kgsl_pagetable *pagetable,
-		struct kgsl_memdesc *memdesc)
+void kgsl_mmu_put_gpuaddr(struct kgsl_memdesc *memdesc)
 {
+	struct kgsl_pagetable *pagetable = memdesc->pagetable;
+	int unmap_fail = 0;
+
 	if (memdesc->size == 0 || memdesc->gpuaddr == 0)
 		return;
 
-	if (PT_OP_VALID(pagetable, put_gpuaddr))
-		pagetable->pt_ops->put_gpuaddr(pagetable, memdesc);
+	if (!kgsl_memdesc_is_global(memdesc))
+		unmap_fail = kgsl_mmu_unmap(pagetable, memdesc);
+
+	/*
+	 * Do not free the gpuaddr/size if unmap fails. Because if we
+	 * try to map this range in future, the iommu driver will throw
+	 * a BUG_ON() because it feels we are overwriting a mapping.
+	*/
+	if (PT_OP_VALID(pagetable, put_gpuaddr) && (unmap_fail == 0))
+		pagetable->pt_ops->put_gpuaddr(memdesc);
 
 	if (!kgsl_memdesc_is_global(memdesc))
 		memdesc->gpuaddr = 0;
+
+	memdesc->pagetable = NULL;
 }
 EXPORT_SYMBOL(kgsl_mmu_put_gpuaddr);
 
@@ -610,7 +622,12 @@ static int nommu_get_gpuaddr(struct kgsl_pagetable *pagetable,
 
 	memdesc->gpuaddr = (uint64_t) sg_phys(memdesc->sgt->sgl);
 
-	return memdesc->gpuaddr != 0 ? 0 : -ENOMEM;
+	if (memdesc->gpuaddr) {
+		memdesc->pagetable = pagetable;
+		return 0;
+	}
+
+	return -ENOMEM;
 }
 
 static struct kgsl_mmu_pt_ops nommu_pt_ops = {
diff --git a/drivers/gpu/msm/kgsl_mmu.h b/drivers/gpu/msm/kgsl_mmu.h
index 3e32c25b3dbe..bc448d424ccb 100644
--- a/drivers/gpu/msm/kgsl_mmu.h
+++ b/drivers/gpu/msm/kgsl_mmu.h
@@ -92,7 +92,7 @@ struct kgsl_mmu_pt_ops {
 	u64 (*get_ttbr0)(struct kgsl_pagetable *);
 	u32 (*get_contextidr)(struct kgsl_pagetable *);
 	int (*get_gpuaddr)(struct kgsl_pagetable *, struct kgsl_memdesc *);
-	void (*put_gpuaddr)(struct kgsl_pagetable *, struct kgsl_memdesc *);
+	void (*put_gpuaddr)(struct kgsl_memdesc *);
 	uint64_t (*find_svm_region)(struct kgsl_pagetable *, uint64_t, uint64_t,
 		uint64_t, uint64_t);
 	int (*set_svm_region)(struct kgsl_pagetable *, uint64_t, uint64_t);
@@ -181,8 +181,7 @@ int kgsl_mmu_map(struct kgsl_pagetable *pagetable,
 		 struct kgsl_memdesc *memdesc);
 int kgsl_mmu_unmap(struct kgsl_pagetable *pagetable,
 		    struct kgsl_memdesc *memdesc);
-void kgsl_mmu_put_gpuaddr(struct kgsl_pagetable *pagetable,
-		 struct kgsl_memdesc *memdesc);
+void kgsl_mmu_put_gpuaddr(struct kgsl_memdesc *memdesc);
 unsigned int kgsl_virtaddr_to_physaddr(void *virtaddr);
 unsigned int kgsl_mmu_log_fault_addr(struct kgsl_mmu *mmu,
 		u64 ttbr0, uint64_t addr);
diff --git a/drivers/gpu/msm/kgsl_sharedmem.c b/drivers/gpu/msm/kgsl_sharedmem.c
index 72895c18119f..b18934fb66ae 100644
--- a/drivers/gpu/msm/kgsl_sharedmem.c
+++ b/drivers/gpu/msm/kgsl_sharedmem.c
@@ -318,12 +318,11 @@ static int kgsl_cma_alloc_secure(struct kgsl_device *device,
 
 static int kgsl_allocate_secure(struct kgsl_device *device,
 				struct kgsl_memdesc *memdesc,
-				struct kgsl_pagetable *pagetable,
 				uint64_t size) {
 	int ret;
 
 	if (MMU_FEATURE(&device->mmu, KGSL_MMU_HYP_SECURE_ALLOC))
-		ret = kgsl_sharedmem_page_alloc_user(memdesc, pagetable, size);
+		ret = kgsl_sharedmem_page_alloc_user(memdesc, size);
 	else
 		ret = kgsl_cma_alloc_secure(device, memdesc, size);
 
@@ -332,7 +331,6 @@ static int kgsl_allocate_secure(struct kgsl_device *device,
 
 int kgsl_allocate_user(struct kgsl_device *device,
 		struct kgsl_memdesc *memdesc,
-		struct kgsl_pagetable *pagetable,
 		uint64_t size, uint64_t flags)
 {
 	int ret;
@@ -340,12 +338,11 @@ int kgsl_allocate_user(struct kgsl_device *device,
 	memdesc->flags = flags;
 
 	if (kgsl_mmu_get_mmutype(device) == KGSL_MMU_TYPE_NONE)
-		ret = kgsl_sharedmem_alloc_contig(device, memdesc,
-				pagetable, size);
+		ret = kgsl_sharedmem_alloc_contig(device, memdesc, size);
 	else if (flags & KGSL_MEMFLAGS_SECURE)
-		ret = kgsl_allocate_secure(device, memdesc, pagetable, size);
+		ret = kgsl_allocate_secure(device, memdesc, size);
 	else
-		ret = kgsl_sharedmem_page_alloc_user(memdesc, pagetable, size);
+		ret = kgsl_sharedmem_page_alloc_user(memdesc, size);
 
 	return ret;
 }
@@ -638,7 +635,6 @@ static inline int get_page_size(size_t size, unsigned int align)
 
 int
 kgsl_sharedmem_page_alloc_user(struct kgsl_memdesc *memdesc,
-			struct kgsl_pagetable *pagetable,
 			uint64_t size)
 {
 	int ret = 0;
@@ -672,7 +668,6 @@ kgsl_sharedmem_page_alloc_user(struct kgsl_memdesc *memdesc,
 
 	len_alloc = PAGE_ALIGN(size) >> PAGE_SHIFT;
 
-	memdesc->pagetable = pagetable;
 	memdesc->ops = &kgsl_page_alloc_ops;
 
 	/*
@@ -806,18 +801,8 @@ void kgsl_sharedmem_free(struct kgsl_memdesc *memdesc)
 	if (memdesc == NULL || memdesc->size == 0)
 		return;
 
-	if (memdesc->gpuaddr) {
-		int ret = 0;
-
-		ret = kgsl_mmu_unmap(memdesc->pagetable, memdesc);
-		/*
-		 * Do not free the gpuaddr/size if unmap fails. Because if we
-		 * try to map this range in future, the iommu driver will throw
-		 * a BUG_ON() because it feels we are overwriting a mapping.
-		 */
-		if (ret == 0)
-			kgsl_mmu_put_gpuaddr(memdesc->pagetable, memdesc);
-	}
+	/* Make sure the memory object has been unmapped */
+	kgsl_mmu_put_gpuaddr(memdesc);
 
 	if (memdesc->ops && memdesc->ops->free)
 		memdesc->ops->free(memdesc);
@@ -997,8 +982,7 @@ void kgsl_get_memory_usage(char *name, size_t name_size, uint64_t memflags)
 EXPORT_SYMBOL(kgsl_get_memory_usage);
 
 int kgsl_sharedmem_alloc_contig(struct kgsl_device *device,
-			struct kgsl_memdesc *memdesc,
-			struct kgsl_pagetable *pagetable, uint64_t size)
+			struct kgsl_memdesc *memdesc, uint64_t size)
 {
 	int result = 0;
 
@@ -1007,7 +991,6 @@ int kgsl_sharedmem_alloc_contig(struct kgsl_device *device,
 		return -EINVAL;
 
 	memdesc->size = size;
-	memdesc->pagetable = pagetable;
 	memdesc->ops = &kgsl_cma_ops;
 	memdesc->dev = device->dev->parent;
 
@@ -1098,7 +1081,6 @@ static int kgsl_cma_alloc_secure(struct kgsl_device *device,
 {
 	struct kgsl_iommu *iommu = KGSL_IOMMU_PRIV(device);
 	int result = 0;
-	struct kgsl_pagetable *pagetable = device->mmu.securepagetable;
 	size_t aligned;
 
 	/* Align size to 1M boundaries */
@@ -1118,7 +1100,6 @@ static int kgsl_cma_alloc_secure(struct kgsl_device *device,
 			memdesc->priv &= ~KGSL_MEMDESC_GUARD_PAGE;
 
 	memdesc->size = aligned;
-	memdesc->pagetable = pagetable;
 	memdesc->ops = &kgsl_cma_ops;
 	memdesc->dev = iommu->ctx[KGSL_IOMMU_CONTEXT_SECURE].dev;
 
diff --git a/drivers/gpu/msm/kgsl_sharedmem.h b/drivers/gpu/msm/kgsl_sharedmem.h
index 9e6817c76df8..03f278ead20f 100644
--- a/drivers/gpu/msm/kgsl_sharedmem.h
+++ b/drivers/gpu/msm/kgsl_sharedmem.h
@@ -26,7 +26,7 @@ struct kgsl_process_private;
 
 int kgsl_sharedmem_alloc_contig(struct kgsl_device *device,
 			struct kgsl_memdesc *memdesc,
-			struct kgsl_pagetable *pagetable, uint64_t size);
+			uint64_t size);
 
 void kgsl_sharedmem_free(struct kgsl_memdesc *memdesc);
 
@@ -66,13 +66,11 @@ void kgsl_sharedmem_uninit_sysfs(void);
 
 int kgsl_allocate_user(struct kgsl_device *device,
 		struct kgsl_memdesc *memdesc,
-		struct kgsl_pagetable *pagetable,
 		uint64_t size, uint64_t flags);
 
 void kgsl_get_memory_usage(char *str, size_t len, uint64_t memflags);
 
 int kgsl_sharedmem_page_alloc_user(struct kgsl_memdesc *memdesc,
-				struct kgsl_pagetable *pagetable,
 				uint64_t size);
 
 #define MEMFLAGS(_flags, _mask, _shift) \
@@ -287,11 +285,10 @@ static inline int kgsl_allocate_global(struct kgsl_device *device,
 	memdesc->priv = priv;
 
 	if ((memdesc->priv & KGSL_MEMDESC_CONTIG) != 0)
-		ret = kgsl_sharedmem_alloc_contig(device, memdesc, NULL,
+		ret = kgsl_sharedmem_alloc_contig(device, memdesc,
 						(size_t) size);
 	else {
-		ret = kgsl_sharedmem_page_alloc_user(memdesc, NULL,
-						(size_t) size);
+		ret = kgsl_sharedmem_page_alloc_user(memdesc, (size_t) size);
 		if (ret == 0)
 			kgsl_memdesc_map(memdesc);
 	}