[PATCH v4 09/31] accel: ethosu: Guard operations after unplug

[Rob Herring (Arm)]

drm_dev_is_unplugged() only checks the device state and can race with
removal. Use drm_dev_enter() around open and all private operations which
access driver state, and use drm_dev_unplug() during removal to wait for
them to complete.

Drop the drm_dev_enter() from the BO wait ioctl as that could
unnecessarily delay stopping the scheduler by waiting on a BO fence
signal timeout.

Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
---
v4:
 - New patch
---
 drivers/accel/ethosu/ethosu_drv.c     |  28 ++++----
 drivers/accel/ethosu/ethosu_job.c     |  21 ++++--
 drivers/accel/ethosu/ethosu_perfmon.c | 117 ++++++++++++++++++++++------------
 3 files changed, 107 insertions(+), 59 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
index 6875ed17a004..91b2c2ab0a03 100644
--- a/drivers/accel/ethosu/ethosu_drv.c
+++ b/drivers/accel/ethosu/ethosu_drv.c
@@ -80,19 +80,12 @@ static int ethosu_ioctl_bo_wait(struct drm_device *ddev, void *data,
 				struct drm_file *file)
 {
 	struct drm_ethosu_bo_wait *args = data;
-	int cookie, ret;
 	unsigned long timeout = drm_timeout_abs_to_jiffies(args->timeout_ns);
 
 	if (args->pad)
 		return -EINVAL;
 
-	if (!drm_dev_enter(ddev, &cookie))
-		return -ENODEV;
-
-	ret = drm_gem_dma_resv_wait(file, args->handle, true, timeout);
-
-	drm_dev_exit(cookie);
-	return ret;
+	return drm_gem_dma_resv_wait(file, args->handle, true, timeout);
 }
 
 static int ethosu_ioctl_bo_mmap_offset(struct drm_device *ddev, void *data,
@@ -139,27 +132,34 @@ static int ethosu_ioctl_cmdstream_bo_create(struct drm_device *ddev, void *data,
 
 static int ethosu_open(struct drm_device *ddev, struct drm_file *file)
 {
-	int ret = 0;
+	struct ethosu_file_priv __free(kfree) * priv = NULL;
+	int cookie, ret;
 
 	if (!try_module_get(THIS_MODULE))
 		return -EINVAL;
+	if (!drm_dev_enter(ddev, &cookie)) {
+		module_put(THIS_MODULE);
+		return -ENODEV;
+	}
 
-	struct ethosu_file_priv __free(kfree) *priv = kzalloc_obj(*priv);
+	priv = kzalloc_obj(*priv);
 	if (!priv) {
 		ret = -ENOMEM;
-		goto err_put_mod;
+		goto err_dev_exit;
 	}
 	priv->edev = to_ethosu_device(ddev);
 
 	ret = ethosu_job_open(priv);
 	if (ret)
-		goto err_put_mod;
+		goto err_dev_exit;
 
 	ethosu_perfmon_open_file(priv);
 	file->driver_priv = no_free_ptr(priv);
+	drm_dev_exit(cookie);
 	return 0;
 
-err_put_mod:
+err_dev_exit:
+	drm_dev_exit(cookie);
 	module_put(THIS_MODULE);
 	return ret;
 }
@@ -398,7 +398,7 @@ static void ethosu_remove(struct platform_device *pdev)
 {
 	struct ethosu_device *ethosudev = dev_get_drvdata(&pdev->dev);
 
-	drm_dev_unregister(&ethosudev->base);
+	drm_dev_unplug(&ethosudev->base);
 	ethosu_job_fini(ethosudev);
 
 	pm_runtime_barrier(&pdev->dev);
diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index f22cacb19e3d..8ffdc83d06f9 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -10,6 +10,7 @@
 #include <linux/pm_runtime.h>
 
 #include <drm/drm_file.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_dma_helper.h>
 #include <drm/drm_print.h>
@@ -557,26 +558,34 @@ static int ethosu_ioctl_submit_job(struct drm_device *dev, struct drm_file *file
 int ethosu_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *file)
 {
 	struct drm_ethosu_submit *args = data;
-	int ret = 0;
+	int cookie, ret = 0;
 	unsigned int i = 0;
 
+	if (!drm_dev_enter(dev, &cookie))
+		return -ENODEV;
+
 	struct drm_ethosu_job __free(kvfree) *jobs =
 		kvmalloc_objs(*jobs, args->job_count);
-	if (!jobs)
-		return -ENOMEM;
+	if (!jobs) {
+		ret = -ENOMEM;
+		goto out_dev_exit;
+	}
 
 	if (copy_from_user(jobs,
 			   (void __user *)(uintptr_t)args->jobs,
 			   args->job_count * sizeof(*jobs))) {
 		drm_dbg(dev, "Failed to copy incoming job array\n");
-		return -EFAULT;
+		ret = -EFAULT;
+		goto out_dev_exit;
 	}
 
 	for (i = 0; i < args->job_count; i++) {
 		ret = ethosu_ioctl_submit_job(dev, file, &jobs[i], args->perfmon_id);
 		if (ret)
-			return ret;
+			goto out_dev_exit;
 	}
 
-	return 0;
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }
diff --git a/drivers/accel/ethosu/ethosu_perfmon.c b/drivers/accel/ethosu/ethosu_perfmon.c
index 2a9b3cc1447b..8d3635ba3347 100644
--- a/drivers/accel/ethosu/ethosu_perfmon.c
+++ b/drivers/accel/ethosu/ethosu_perfmon.c
@@ -10,6 +10,7 @@
 #include <linux/uaccess.h>
 
 #include <drm/drm_file.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_ioctl.h>
 
 #include <uapi/drm/ethosu_accel.h>
@@ -162,27 +163,36 @@ int ethosu_ioctl_perfmon_create(struct drm_device *dev, void *data,
 	struct ethosu_device *ethosu = to_ethosu_device(dev);
 	struct ethosu_perfmon *perfmon;
 	unsigned int i, event_max;
-	int ret;
+	int cookie, ret;
 	u32 id;
 
+	if (!drm_dev_enter(dev, &cookie))
+		return -ENODEV;
+
 	/* Number of monitored counters cannot exceed HW or UAPI limits. */
 	if (req->ncounters > ethosu->npu_info.pmu_counters ||
-	    req->ncounters > DRM_ETHOSU_MAX_PERF_EVENT_COUNTERS)
-		return -EINVAL;
+	    req->ncounters > DRM_ETHOSU_MAX_PERF_EVENT_COUNTERS) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	/* Make sure all counters are valid. */
 	event_max = ethosu_is_u65(ethosu) ? 433 : 671;
 	for (i = 0; i < req->ncounters; i++) {
-		if (req->counters[i] > event_max)
-			return -EINVAL;
+		if (req->counters[i] > event_max) {
+			ret = -EINVAL;
+			goto out_dev_exit;
+		}
 	}
 
 	/* Add 1 more counter for cycle counter */
 	req->ncounters++;
 
 	perfmon = kzalloc_flex(*perfmon, values, req->ncounters);
-	if (!perfmon)
-		return -ENOMEM;
+	if (!perfmon) {
+		ret = -ENOMEM;
+		goto out_dev_exit;
+	}
 
 	for (i = 0; i < req->ncounters - 1; i++)
 		perfmon->counters[i] = req->counters[i];
@@ -196,12 +206,14 @@ int ethosu_ioctl_perfmon_create(struct drm_device *dev, void *data,
 
 	if (ret < 0) {
 		kfree(perfmon);
-		return ret;
+		goto out_dev_exit;
 	}
 
 	req->id = id;
 
-	return 0;
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }
 
 int ethosu_ioctl_perfmon_destroy(struct drm_device *dev, void *data,
@@ -210,14 +222,22 @@ int ethosu_ioctl_perfmon_destroy(struct drm_device *dev, void *data,
 	struct ethosu_file_priv *ethosu_priv = file_priv->driver_priv;
 	struct drm_ethosu_perfmon_destroy *req = data;
 	struct ethosu_perfmon *perfmon;
+	int cookie, ret = 0;
+
+	if (!drm_dev_enter(dev, &cookie))
+		return -ENODEV;
 
 	perfmon = xa_erase(&ethosu_priv->perfmons, req->id);
-	if (!perfmon)
-		return -EINVAL;
+	if (!perfmon) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	ethosu_perfmon_delete(ethosu_priv, perfmon);
 
-	return 0;
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }
 
 int ethosu_ioctl_perfmon_get_values(struct drm_device *dev, void *data,
@@ -227,19 +247,26 @@ int ethosu_ioctl_perfmon_get_values(struct drm_device *dev, void *data,
 	struct ethosu_file_priv *ethosu_priv = file_priv->driver_priv;
 	struct drm_ethosu_perfmon_get_values *req = data;
 	struct ethosu_perfmon *perfmon;
-	int ret = 0;
+	int cookie, ret = 0;
 
-	if (req->pad != 0)
-		return -EINVAL;
+	if (!drm_dev_enter(dev, &cookie))
+		return -ENODEV;
+
+	if (req->pad != 0) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	perfmon = ethosu_perfmon_find(ethosu_priv, req->id);
-	if (!perfmon)
-		return -EINVAL;
+	if (!perfmon) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	ret = pm_runtime_resume_and_get(dev->dev);
 	if (ret) {
 		ethosu_perfmon_put(perfmon);
-		return ret;
+		goto out_dev_exit;
 	}
 	ethosu_perfmon_stop(ethosu, perfmon, true);
 
@@ -251,6 +278,8 @@ int ethosu_ioctl_perfmon_get_values(struct drm_device *dev, void *data,
 
 	ethosu_perfmon_put(perfmon);
 
+out_dev_exit:
+	drm_dev_exit(cookie);
 	return ret;
 }
 
@@ -260,44 +289,54 @@ int ethosu_ioctl_perfmon_set_global(struct drm_device *dev, void *data,
 	struct ethosu_file_priv *ethosu_priv = file_priv->driver_priv;
 	struct drm_ethosu_perfmon_set_global *req = data;
 	struct ethosu_device *ethosu = to_ethosu_device(dev);
-	struct ethosu_perfmon *perfmon;
+	struct ethosu_perfmon *old = NULL, *perfmon;
+	int cookie, ret = 0;
 
-	if (req->flags & ~DRM_ETHOSU_PERFMON_CLEAR_GLOBAL)
-		return -EINVAL;
+	if (!drm_dev_enter(dev, &cookie))
+		return -ENODEV;
+
+	if (req->flags & ~DRM_ETHOSU_PERFMON_CLEAR_GLOBAL) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	perfmon = ethosu_perfmon_find(ethosu_priv, req->id);
-	if (!perfmon)
-		return -EINVAL;
+	if (!perfmon) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	/* If the request is to clear the global performance monitor */
 	if (req->flags & DRM_ETHOSU_PERFMON_CLEAR_GLOBAL) {
-		struct ethosu_perfmon *old;
-
 		scoped_guard(mutex, &ethosu->perfmon_state.lock) {
 			old = ethosu->global_perfmon;
-			if (!old) {
-				ethosu_perfmon_put(perfmon);
-				return -EINVAL;
+			if (old) {
+				ethosu->global_perfmon = NULL;
+				ethosu_perfmon_stop_locked(ethosu, old, true);
 			}
+		}
 
-			ethosu->global_perfmon = NULL;
-			ethosu_perfmon_stop_locked(ethosu, old, true);
+		ethosu_perfmon_put(perfmon);
+		if (!old) {
+			ret = -EINVAL;
+			goto out_dev_exit;
 		}
 
 		ethosu_perfmon_put(old);
-		ethosu_perfmon_put(perfmon);
 
-		return 0;
+		goto out_dev_exit;
 	}
 
 	scoped_guard(mutex, &ethosu->perfmon_state.lock) {
-		if (ethosu->perfmon_state.active || ethosu->global_perfmon) {
-			ethosu_perfmon_put(perfmon);
-			return -EBUSY;
-		}
-
-		ethosu->global_perfmon = perfmon;
+		if (ethosu->perfmon_state.active || ethosu->global_perfmon)
+			ret = -EBUSY;
+		else
+			ethosu->global_perfmon = perfmon;
 	}
+	if (ret)
+		ethosu_perfmon_put(perfmon);
 
-	return 0;
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }

-- 
2.53.0