drm/radeon: Make sure CS mutex is held across GPU reset.

This was only the case if the GPU reset was triggered from the CS ioctl, otherwise other processes could happily enter the CS ioctl and wreak havoc during the GPU reset. This is a little complicated because the GPU reset can be triggered from the CS ioctl, in which case we're already holding the mutex, or from other call paths, in which case we need to lock the mutex. AFAICT the mutex API doesn't allow recursive locking or finding out the mutex owner, so we need to handle this with helper functions which allow recursive locking from the same process. Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> Reviewed-by: Jerome Glisse <jglisse@redhat.com> Signed-off-by: Dave Airlie <airlied@redhat.com>

drm/radeon: Make sure CS mutex is held across GPU reset.
This was only the case if the GPU reset was triggered from the CS ioctl, otherwise other processes could happily enter the CS ioctl and wreak havoc during the GPU reset. This is a little complicated because the GPU reset can be triggered from the CS ioctl, in which case we're already holding the mutex, or from other call paths, in which case we need to lock the mutex. AFAICT the mutex API doesn't allow recursive locking or finding out the mutex owner, so we need to handle this with helper functions which allow recursive locking from the same process. Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> Reviewed-by: Jerome Glisse <jglisse@redhat.com> Signed-off-by: Dave Airlie <airlied@redhat.com>
Michel Dänzer · Dave Airlie
1 parent 471dd2ef37
Showing 3 changed files with 62 additions and 12 deletions Side-by-side Diff
drivers/gpu/drm/radeon/radeon.h
drivers/gpu/drm/radeon/radeon_cs.c
drivers/gpu/drm/radeon/radeon_device.c
@@ -1142,7 +1142,49 @@
 	u64				gpu_addr;
 };
  
+
 /*
+ * Mutex which allows recursive locking from the same process.
+ */
+struct radeon_mutex {
+	struct mutex		mutex;
+	struct task_struct	*owner;
+	int			level;
+};
+
+static inline void radeon_mutex_init(struct radeon_mutex *mutex)
+{
+	mutex_init(&mutex->mutex);
+	mutex->owner = NULL;
+	mutex->level = 0;
+}
+
+static inline void radeon_mutex_lock(struct radeon_mutex *mutex)
+{
+	if (mutex_trylock(&mutex->mutex)) {
+		/* The mutex was unlocked before, so it's ours now */
+		mutex->owner = current;
+	} else if (mutex->owner != current) {
+		/* Another process locked the mutex, take it */
+		mutex_lock(&mutex->mutex);
+		mutex->owner = current;
+	}
+	/* Otherwise the mutex was already locked by this process */
+
+	mutex->level++;
+}
+
+static inline void radeon_mutex_unlock(struct radeon_mutex *mutex)
+{
+	if (--mutex->level > 0)
+		return;
+
+	mutex->owner = NULL;
+	mutex_unlock(&mutex->mutex);
+}
+
+
+/*
  * Core structure, functions and helpers.
  */
 typedef uint32_t (*radeon_rreg_t)(struct radeon_device*, uint32_t);
@@ -1197,7 +1239,7 @@
 	struct radeon_gem		gem;
 	struct radeon_pm		pm;
 	uint32_t			bios_scratch[RADEON_BIOS_NUM_SCRATCH];
-	struct mutex			cs_mutex;
+	struct radeon_mutex		cs_mutex;
 	struct radeon_wb		wb;
 	struct radeon_dummy_page	dummy_page;
 	bool				gpu_lockup;
@@ -222,7 +222,7 @@
 	struct radeon_cs_chunk *ib_chunk;
 	int r;
  
-	mutex_lock(&rdev->cs_mutex);
+	radeon_mutex_lock(&rdev->cs_mutex);
 	/* initialize parser */
 	memset(&parser, 0, sizeof(struct radeon_cs_parser));
 	parser.filp = filp;
  
@@ -233,14 +233,14 @@
 	if (r) {
 		DRM_ERROR("Failed to initialize parser !\n");
 		radeon_cs_parser_fini(&parser, r);
-		mutex_unlock(&rdev->cs_mutex);
+		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
 	r =  radeon_ib_get(rdev, &parser.ib);
 	if (r) {
 		DRM_ERROR("Failed to get ib !\n");
 		radeon_cs_parser_fini(&parser, r);
-		mutex_unlock(&rdev->cs_mutex);
+		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
 	r = radeon_cs_parser_relocs(&parser);
@@ -248,7 +248,7 @@
 		if (r != -ERESTARTSYS)
 			DRM_ERROR("Failed to parse relocation %d!\n", r);
 		radeon_cs_parser_fini(&parser, r);
-		mutex_unlock(&rdev->cs_mutex);
+		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
 	/* Copy the packet into the IB, the parser will read from the
  
@@ -260,14 +260,14 @@
 	if (r || parser.parser_error) {
 		DRM_ERROR("Invalid command stream !\n");
 		radeon_cs_parser_fini(&parser, r);
-		mutex_unlock(&rdev->cs_mutex);
+		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
 	r = radeon_cs_finish_pages(&parser);
 	if (r) {
 		DRM_ERROR("Invalid command stream !\n");
 		radeon_cs_parser_fini(&parser, r);
-		mutex_unlock(&rdev->cs_mutex);
+		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
 	r = radeon_ib_schedule(rdev, parser.ib);
@@ -275,7 +275,7 @@
 		DRM_ERROR("Failed to schedule IB !\n");
 	}
 	radeon_cs_parser_fini(&parser, r);
-	mutex_unlock(&rdev->cs_mutex);
+	radeon_mutex_unlock(&rdev->cs_mutex);
 	return r;
 }
  
@@ -716,7 +716,7 @@
  
 	/* mutex initialization are all done here so we
 	 * can recall function without having locking issues */
-	mutex_init(&rdev->cs_mutex);
+	radeon_mutex_init(&rdev->cs_mutex);
 	mutex_init(&rdev->ib_pool.mutex);
 	mutex_init(&rdev->cp.mutex);
 	mutex_init(&rdev->dc_hw_i2c_mutex);
@@ -955,6 +955,9 @@
 	int r;
 	int resched;
  
+	/* Prevent CS ioctl from interfering */
+	radeon_mutex_lock(&rdev->cs_mutex);
+
 	radeon_save_bios_scratch_regs(rdev);
 	/* block TTM */
 	resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
  
@@ -967,10 +970,15 @@
 		radeon_restore_bios_scratch_regs(rdev);
 		drm_helper_resume_force_mode(rdev->ddev);
 		ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
-		return 0;
 	}
-	/* bad news, how to tell it to userspace ? */
-	dev_info(rdev->dev, "GPU reset failed\n");
+
+	radeon_mutex_unlock(&rdev->cs_mutex);
+
+	if (r) {
+		/* bad news, how to tell it to userspace ? */
+		dev_info(rdev->dev, "GPU reset failed\n");
+	}
+
 	return r;
 }
...	...	@@ -1142,7 +1142,49 @@
1142	1142	u64 gpu_addr;
1143	1143	};
1144	1144
	1145	+
1145	1146	/*
	1147	+ * Mutex which allows recursive locking from the same process.
	1148	+ */
	1149	+struct radeon_mutex {
	1150	+ struct mutex mutex;
	1151	+ struct task_struct *owner;
	1152	+ int level;
	1153	+};
	1154	+
	1155	+static inline void radeon_mutex_init(struct radeon_mutex *mutex)
	1156	+{
	1157	+ mutex_init(&mutex->mutex);
	1158	+ mutex->owner = NULL;
	1159	+ mutex->level = 0;
	1160	+}
	1161	+
	1162	+static inline void radeon_mutex_lock(struct radeon_mutex *mutex)
	1163	+{
	1164	+ if (mutex_trylock(&mutex->mutex)) {
	1165	+ /* The mutex was unlocked before, so it's ours now */
	1166	+ mutex->owner = current;
	1167	+ } else if (mutex->owner != current) {
	1168	+ /* Another process locked the mutex, take it */
	1169	+ mutex_lock(&mutex->mutex);
	1170	+ mutex->owner = current;
	1171	+ }
	1172	+ /* Otherwise the mutex was already locked by this process */
	1173	+
	1174	+ mutex->level++;
	1175	+}
	1176	+
	1177	+static inline void radeon_mutex_unlock(struct radeon_mutex *mutex)
	1178	+{
	1179	+ if (--mutex->level > 0)
	1180	+ return;
	1181	+
	1182	+ mutex->owner = NULL;
	1183	+ mutex_unlock(&mutex->mutex);
	1184	+}
	1185	+
	1186	+
	1187	+/*
1146	1188	* Core structure, functions and helpers.
1147	1189	*/
1148	1190	typedef uint32_t (radeon_rreg_t)(struct radeon_device, uint32_t);
...	...	@@ -1197,7 +1239,7 @@
1197	1239	struct radeon_gem gem;
1198	1240	struct radeon_pm pm;
1199	1241	uint32_t bios_scratch[RADEON_BIOS_NUM_SCRATCH];
1200		- struct mutex cs_mutex;
	1242	+ struct radeon_mutex cs_mutex;
1201	1243	struct radeon_wb wb;
1202	1244	struct radeon_dummy_page dummy_page;
1203	1245	bool gpu_lockup;
...	...	@@ -222,7 +222,7 @@
222	222	struct radeon_cs_chunk *ib_chunk;
223	223	int r;
224	224
225		- mutex_lock(&rdev->cs_mutex);
	225	+ radeon_mutex_lock(&rdev->cs_mutex);
226	226	/* initialize parser */
227	227	memset(&parser, 0, sizeof(struct radeon_cs_parser));
228	228	parser.filp = filp;
229	229
...	...	@@ -233,14 +233,14 @@
233	233	if (r) {
234	234	DRM_ERROR("Failed to initialize parser !\n");
235	235	radeon_cs_parser_fini(&parser, r);
236		- mutex_unlock(&rdev->cs_mutex);
	236	+ radeon_mutex_unlock(&rdev->cs_mutex);
237	237	return r;
238	238	}
239	239	r = radeon_ib_get(rdev, &parser.ib);
240	240	if (r) {
241	241	DRM_ERROR("Failed to get ib !\n");
242	242	radeon_cs_parser_fini(&parser, r);
243		- mutex_unlock(&rdev->cs_mutex);
	243	+ radeon_mutex_unlock(&rdev->cs_mutex);
244	244	return r;
245	245	}
246	246	r = radeon_cs_parser_relocs(&parser);
...	...	@@ -248,7 +248,7 @@
248	248	if (r != -ERESTARTSYS)
249	249	DRM_ERROR("Failed to parse relocation %d!\n", r);
250	250	radeon_cs_parser_fini(&parser, r);
251		- mutex_unlock(&rdev->cs_mutex);
	251	+ radeon_mutex_unlock(&rdev->cs_mutex);
252	252	return r;
253	253	}
254	254	/* Copy the packet into the IB, the parser will read from the
255	255
...	...	@@ -260,14 +260,14 @@
260	260	if (r \|\| parser.parser_error) {
261	261	DRM_ERROR("Invalid command stream !\n");
262	262	radeon_cs_parser_fini(&parser, r);
263		- mutex_unlock(&rdev->cs_mutex);
	263	+ radeon_mutex_unlock(&rdev->cs_mutex);
264	264	return r;
265	265	}
266	266	r = radeon_cs_finish_pages(&parser);
267	267	if (r) {
268	268	DRM_ERROR("Invalid command stream !\n");
269	269	radeon_cs_parser_fini(&parser, r);
270		- mutex_unlock(&rdev->cs_mutex);
	270	+ radeon_mutex_unlock(&rdev->cs_mutex);
271	271	return r;
272	272	}
273	273	r = radeon_ib_schedule(rdev, parser.ib);
...	...	@@ -275,7 +275,7 @@
275	275	DRM_ERROR("Failed to schedule IB !\n");
276	276	}
277	277	radeon_cs_parser_fini(&parser, r);
278		- mutex_unlock(&rdev->cs_mutex);
	278	+ radeon_mutex_unlock(&rdev->cs_mutex);
279	279	return r;
280	280	}
281	281
...	...	@@ -716,7 +716,7 @@
716	716
717	717	/* mutex initialization are all done here so we
718	718	* can recall function without having locking issues */
719		- mutex_init(&rdev->cs_mutex);
	719	+ radeon_mutex_init(&rdev->cs_mutex);
720	720	mutex_init(&rdev->ib_pool.mutex);
721	721	mutex_init(&rdev->cp.mutex);
722	722	mutex_init(&rdev->dc_hw_i2c_mutex);
...	...	@@ -955,6 +955,9 @@
955	955	int r;
956	956	int resched;
957	957
	958	+ /* Prevent CS ioctl from interfering */
	959	+ radeon_mutex_lock(&rdev->cs_mutex);
	960	+
958	961	radeon_save_bios_scratch_regs(rdev);
959	962	/* block TTM */
960	963	resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
961	964
...	...	@@ -967,10 +970,15 @@
967	970	radeon_restore_bios_scratch_regs(rdev);
968	971	drm_helper_resume_force_mode(rdev->ddev);
969	972	ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
970		- return 0;
971	973	}
972		- /* bad news, how to tell it to userspace ? */
973		- dev_info(rdev->dev, "GPU reset failed\n");
	974	+
	975	+ radeon_mutex_unlock(&rdev->cs_mutex);
	976	+
	977	+ if (r) {
	978	+ /* bad news, how to tell it to userspace ? */
	979	+ dev_info(rdev->dev, "GPU reset failed\n");
	980	+ }
	981	+
974	982	return r;
975	983	}
976	984