Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d088bbd7 authored by Jeremy Gebben's avatar Jeremy Gebben
Browse files

msm: kgsl: fix device->mutex recursions 



kgsl_mutex_lock() is a workaround for complicated locking related
to invalidating the iommu TLB during kgsl_mmu_map() and
kgsl_mmu_unmap().  But it worked around this problem by adding
the possibility of lock recursion, which risks many other problems.

The locking here was complicated because global memory regions,
like the ringbuffer were often mapped and unmapped with the
device mutex held, whereas userspace (or other non-global)
memory regions were mapped and unmapped with the device->mutex
NOT held.

However, there is no reason to invalidate the TLB for global
mappings. These mappings are only mapped or unmapped when
creating or destroying pagetables. And when this happens,
the pagetable being (un)mapped is NOT the active pagetable.
Additionally, the global mappings are supposed to be the
same in every pagetable, so there's no point in invalidating
their TLB entries at any point. When a pagetable becomes
active or inactive, the TLB will still be flushed by
the setstate code.

Addtionally, there were some places where userspace
buffers could get unmapped with the device mutex held,
which would deadlock on the device mutex. Fix these
by making sure that kgsl_mem_entry_put() is called
with the mutex open.

Change-Id: I17864bbcf8829d661d0709bd66feb4acaa3f134c
Signed-off-by: default avatarJeremy Gebben <jgebben@codeaurora.org>
parent 57b7c01b

drivers/gpu/msm/kgsl.c

+23 −39
Original line number Diff line number Diff line
@@ -580,6 +580,7 @@ EXPORT_SYMBOL(kgsl_context_init); 
int kgsl_context_detach(struct kgsl_context *context)

{

	int ret;

	struct kgsl_device *device;



	if (context == NULL)

		return -EINVAL;
@@ -592,16 +593,21 @@ int kgsl_context_detach(struct kgsl_context *context) 
	if (test_and_set_bit(KGSL_CONTEXT_PRIV_DETACHED, &context->priv))

		return -EINVAL;



	trace_kgsl_context_detach(context->device, context);

	device = context->device;



	trace_kgsl_context_detach(device, context);



	/* we need to hold device mutex to detach */

	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);

	ret = context->device->ftbl->drawctxt_detach(context);

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);



	/*

	 * Cancel all pending events after the device-specific context is

	 * detached, to avoid possibly freeing memory while it is still

	 * in use by the GPU.

	 */

	kgsl_cancel_events(context->device, &context->events);

	kgsl_cancel_events(device, &context->events);



	/* Remove the event group from the list */

	kgsl_del_event_group(&context->events);
@@ -938,6 +944,8 @@ error: 
static int kgsl_close_device(struct kgsl_device *device)

{

	int result = 0;



	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);

	device->open_count--;

	if (device->open_count == 0) {
@@ -952,6 +960,7 @@ static int kgsl_close_device(struct kgsl_device *device) 
		result = device->ftbl->stop(device);

		kgsl_pwrctrl_change_state(device, KGSL_STATE_INIT);

	}

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

	return result;



}
@@ -978,7 +987,6 @@ static int kgsl_release(struct inode *inodep, struct file *filep) 
		kgsl_syncsource_put(syncsource);

		next = next + 1;

	}

	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);



	next = 0;

	while (1) {
@@ -1028,7 +1036,6 @@ static int kgsl_release(struct inode *inodep, struct file *filep) 
	}



	result = kgsl_close_device(device);

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);



	kfree(dev_priv);
@@ -1041,6 +1048,8 @@ static int kgsl_release(struct inode *inodep, struct file *filep) 
static int kgsl_open_device(struct kgsl_device *device)

{

	int result = 0;



	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);

	if (device->open_count == 0) {

		/*

		 * active_cnt special case: we are starting up for the first
@@ -1071,6 +1080,7 @@ err: 
	if (result)

		atomic_dec(&device->active_cnt);



	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

	return result;

}
@@ -1084,11 +1094,6 @@ static int kgsl_open(struct inode *inodep, struct file *filep) 
	device = kgsl_get_minor(minor);

	BUG_ON(device == NULL);



	if (filep->f_flags & O_EXCL) {

		KGSL_DRV_ERR(device, "O_EXCL not allowed\n");

		return -EBUSY;

	}



	result = pm_runtime_get_sync(&device->pdev->dev);

	if (result < 0) {

		KGSL_DRV_ERR(device,
@@ -1101,18 +1106,15 @@ static int kgsl_open(struct inode *inodep, struct file *filep) 
	dev_priv = kzalloc(sizeof(struct kgsl_device_private), GFP_KERNEL);

	if (dev_priv == NULL) {

		result = -ENOMEM;

		goto err_pmruntime;

		goto err;

	}



	dev_priv->device = device;

	filep->private_data = dev_priv;



	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);



	result = kgsl_open_device(device);

	if (result)

		goto err_freedevpriv;

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

		goto err;



	/*

	 * Get file (per process) private struct. This must be done
@@ -1121,29 +1123,17 @@ static int kgsl_open(struct inode *inodep, struct file *filep) 
	 */

	dev_priv->process_priv = kgsl_get_process_private(device);

	if (dev_priv->process_priv ==  NULL) {

		kgsl_close_device(device);

		result = -ENOMEM;

		goto err_stop;

		goto err;

	}





	return result;



err_stop:

	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);

	device->open_count--;

	if (device->open_count == 0) {

		/* make sure power is on to stop the device */

		kgsl_pwrctrl_enable(device);

		device->ftbl->stop(device);

		kgsl_pwrctrl_change_state(device, KGSL_STATE_INIT);

		atomic_dec(&device->active_cnt);

	}

err_freedevpriv:

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

err:

	if (result) {

		filep->private_data = NULL;

		kfree(dev_priv);

err_pmruntime:

		pm_runtime_put(&device->pdev->dev);

	}

	return result;

}
@@ -2365,8 +2355,6 @@ long kgsl_ioctl_cmdstream_freememontimestamp_ctxtid( 
	if (param->type != KGSL_TIMESTAMP_RETIRED)

		return -EINVAL;



	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);



	context = kgsl_context_get_owner(dev_priv, param->context_id);

	if (context == NULL)

		goto out;
@@ -2401,7 +2389,6 @@ long kgsl_ioctl_cmdstream_freememontimestamp_ctxtid( 


out:

	kgsl_context_put(context);

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

	return result;

}
@@ -2430,17 +2417,14 @@ long kgsl_ioctl_drawctxt_destroy(struct kgsl_device_private *dev_priv, 
					unsigned int cmd, void *data)

{

	struct kgsl_drawctxt_destroy *param = data;

	struct kgsl_device *device = dev_priv->device;

	struct kgsl_context *context;

	long result;



	kgsl_mutex_lock(&device->mutex, &device->mutex_owner);

	context = kgsl_context_get_owner(dev_priv, param->drawctxt_id);



	result = kgsl_context_detach(context);



	kgsl_context_put(context);

	kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

	return result;

}

drivers/gpu/msm/kgsl_device.h

+7 −11
Original line number Diff line number Diff line
@@ -891,21 +891,18 @@ static inline int kgsl_sysfs_store(const char *buf, unsigned int *ptr) 
}



/**

 * kgsl_mutex_lock() -- try to acquire the mutex if current thread does not

 *                      already own it

 * kgsl_mutex_lock() -- mutex_lock() wrapper

 * @mutex: mutex to lock

 * @owner: current mutex owner

 *

 */

static inline int kgsl_mutex_lock(struct mutex *mutex, atomic64_t *owner)

{



	if (atomic64_read(owner) != (long)current) {

	/*

	 * owner is no longer used, but the interface must remain the same

	 * for now.

	 */

	mutex_lock(mutex);

		atomic64_set(owner, (long)current);

		/* Barrier to make sure owner is updated */

		smp_wmb();

		return 0;

	}

	return 1;

}
@@ -916,7 +913,6 @@ static inline int kgsl_mutex_lock(struct mutex *mutex, atomic64_t *owner) 
 */

static inline void kgsl_mutex_unlock(struct mutex *mutex, atomic64_t *owner)

{

	atomic64_set(owner, 0);

	mutex_unlock(mutex);

}

drivers/gpu/msm/kgsl_iommu.c

+19 −16
Original line number Diff line number Diff line
@@ -1668,22 +1668,13 @@ done: 
static void kgsl_iommu_flush_tlb_pt_current(struct kgsl_pagetable *pt,

				struct kgsl_memdesc *memdesc)

{

	int lock_taken = 0;

	struct kgsl_device *device = pt->mmu->device;

	struct kgsl_iommu *iommu = pt->mmu->priv;

	unsigned int flush_flags = KGSL_MMUFLAGS_TLBFLUSH;



	flush_flags |= ((kgsl_memdesc_is_secured(memdesc) ?

			KGSL_MMUFLAGS_TLBFLUSH_SECURE : 0));



	/*

	 * Check to see if the current thread already holds the device mutex.

	 * If it does not, then take the device mutex which is required for

	 * flushing the tlb

	 */

	if (!kgsl_mutex_lock(&device->mutex, &device->mutex_owner))

		lock_taken = 1;



	mutex_lock(&pt->mmu->device->mutex);

	/*

	 * Flush the tlb only if the iommu device is attached and the pagetable

	 * hasn't been switched yet
@@ -1693,9 +1684,7 @@ static void kgsl_iommu_flush_tlb_pt_current(struct kgsl_pagetable *pt, 
		kgsl_iommu_pt_equal(pt->mmu, pt,

		kgsl_iommu_get_current_ptbase(pt->mmu)))

		kgsl_iommu_default_setstate(pt->mmu, flush_flags);



	if (lock_taken)

		kgsl_mutex_unlock(&device->mutex, &device->mutex_owner);

	mutex_unlock(&pt->mmu->device->mutex);

}



static int
@@ -1727,6 +1716,12 @@ kgsl_iommu_unmap(struct kgsl_pagetable *pt, 
		return ret;

	}



	/*

	 * We only need to flush the TLB for non-global memory.

	 * This is because global mappings are only removed at pagetable destroy

	 * time, and the pagetable is not active in the TLB at this point.

	 */

	if (!kgsl_memdesc_is_global(memdesc))

		kgsl_iommu_flush_tlb_pt_current(pt, memdesc);



	return ret;
@@ -1789,10 +1784,15 @@ kgsl_iommu_map(struct kgsl_pagetable *pt, 
	 *  bus errors, or upstream cores being hung (because of garbage data

	 *  being read) -> causing TLB sync stuck issues. As a result SW must

	 *  implement the invalidate+map.

	 *

	 * We only need to flush the TLB for non-global memory.

	 * This is because global mappings are only created at pagetable create

	 * time, and the pagetable is not active in the TLB at this point.

	 */



	if (ADRENO_FEATURE(adreno_dev, IOMMU_FLUSH_TLB_ON_MAP))

		kgsl_iommu_flush_tlb_pt_current(pt, NULL);

	if (ADRENO_FEATURE(adreno_dev, IOMMU_FLUSH_TLB_ON_MAP)

		&& !kgsl_memdesc_is_global(memdesc))

		kgsl_iommu_flush_tlb_pt_current(pt, memdesc);



	return ret;

}
@@ -1927,6 +1927,9 @@ static int kgsl_iommu_default_setstate(struct kgsl_mmu *mmu, 
						mmu->hwpagetable);

	uint64_t pt_val;



	/* we must hold the mutex here to idle the GPU, and to write regs */

	BUG_ON(!mutex_is_locked(&mmu->device->mutex));



	kgsl_iommu_enable_clk(mmu, KGSL_IOMMU_MAX_UNITS);



	/* For v0 SMMU GPU needs to be idle for tlb invalidate as well */