Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Shyti <andi.shyti@linux.intel.com>
To: intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Cc: Chris Wilson <chris.p.wilson@linux.intel.com>,
	Tvrtko Ursulin <tursulin@ursulin.net>,
	Andi Shyti <andi.shyti@linux.intel.com>
Subject: [PATCH v3 12/15] drm/i915: Protect access to the UABI engines list with a mutex
Date: Fri, 23 Aug 2024 15:08:52 +0200	[thread overview]
Message-ID: <20240823130855.72436-13-andi.shyti@linux.intel.com> (raw)
In-Reply-To: <20240823130855.72436-1-andi.shyti@linux.intel.com>

Until now, the UABI engines list has been accessed in read-only
mode, as it was created once during boot and destroyed upon
module unload.

In upcoming commits, we will be modifying this list by changing
the CCS mode, allowing compute engines to be dynamically added
and removed at runtime based on user whims.

To ensure thread safety and prevent race conditions, we need to
protect the engine list with a mutex, thereby serializing access
to it.

Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |  3 +++
 drivers/gpu/drm/i915/gt/intel_engine_user.c |  7 +++++++
 drivers/gpu/drm/i915/gt/sysfs_engines.c     |  5 +++++
 drivers/gpu/drm/i915/i915_cmd_parser.c      |  2 ++
 drivers/gpu/drm/i915/i915_debugfs.c         |  4 ++++
 drivers/gpu/drm/i915/i915_drv.h             |  4 ++++
 drivers/gpu/drm/i915/i915_gem.c             |  4 ++++
 drivers/gpu/drm/i915/i915_perf.c            |  8 +++++---
 drivers/gpu/drm/i915/i915_pmu.c             | 11 +++++++++--
 drivers/gpu/drm/i915/i915_query.c           | 21 ++++++++++++++++-----
 10 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c0543c35cd6a..0ccbe447f51d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1124,6 +1124,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 	if (!e)
 		return ERR_PTR(-ENOMEM);
 
+	mutex_lock(&ctx->i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, ctx->i915) {
 		struct intel_context *ce;
 		struct intel_sseu sseu = {};
@@ -1155,9 +1156,11 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 
 	}
 
+	mutex_unlock(&ctx->i915->uabi_engines_mutex);
 	return e;
 
 free_engines:
+	mutex_unlock(&ctx->i915->uabi_engines_mutex);
 	free_engines(e);
 	return err;
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 8e5284af8335..209d5badbd3d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -210,6 +210,13 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
 	LIST_HEAD(engines);
 
 	sort_engines(i915, &engines);
+	mutex_init(&i915->uabi_engines_mutex);
+
+	/*
+	 * We are still booting i915 and we are sure we are running
+	 * single-threaded. We don't need at this point to protect the
+	 * uabi_engines access list with the mutex.
+	 */
 
 	prev = NULL;
 	p = &i915->uabi_engines.rb_node;
diff --git a/drivers/gpu/drm/i915/gt/sysfs_engines.c b/drivers/gpu/drm/i915/gt/sysfs_engines.c
index f67f76df1cfe..c1cc0981c8fb 100644
--- a/drivers/gpu/drm/i915/gt/sysfs_engines.c
+++ b/drivers/gpu/drm/i915/gt/sysfs_engines.c
@@ -508,6 +508,11 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915)
 
 	i915->sysfs_engine = dir;
 
+	/*
+	 * We are still booting i915 and we are sure we are running
+	 * single-threaded. We don't need at this point to protect the
+	 * uabi_engines access list with the mutex.
+	 */
 	for_each_uabi_engine(engine, i915) {
 		struct kobject *kobj;
 
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 2905df83e180..12987ece6f8e 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1592,12 +1592,14 @@ int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv)
 	bool active = false;
 
 	/* If the command parser is not enabled, report 0 - unsupported */
+	mutex_lock(&dev_priv->uabi_engines_mutex);
 	for_each_uabi_engine(engine, dev_priv) {
 		if (intel_engine_using_cmd_parser(engine)) {
 			active = true;
 			break;
 		}
 	}
+	mutex_unlock(&dev_priv->uabi_engines_mutex);
 	if (!active)
 		return 0;
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bc717cf544e4..8b5e365eb6bd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -459,8 +459,10 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 		   to_gt(i915)->clock_period_ns);
 
 	p = drm_seq_file_printer(m);
+	mutex_lock(&i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, i915)
 		intel_engine_dump(engine, &p, "%s\n", engine->name);
+	mutex_unlock(&i915->uabi_engines_mutex);
 
 	intel_gt_show_timelines(to_gt(i915), &p, i915_request_show_with_schedule);
 
@@ -474,6 +476,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
 	struct drm_i915_private *i915 = node_to_i915(m->private);
 	struct intel_engine_cs *engine;
 
+	mutex_lock(&i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, i915) {
 		const struct i915_wa_list *wal = &engine->ctx_wa_list;
 		const struct i915_wa *wa;
@@ -493,6 +496,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
 
 		seq_printf(m, "\n");
 	}
+	mutex_unlock(&i915->uabi_engines_mutex);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3a8a757f5bd5..5210e130ca0c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -231,6 +231,10 @@ struct drm_i915_private {
 		struct rb_root uabi_engines;
 	};
 	unsigned int engine_uabi_class_count[I915_LAST_UABI_ENGINE_CLASS + 1];
+	/*
+	 * Protect access to the uabi_engines list.
+	 */
+	struct mutex uabi_engines_mutex;
 
 	/* protects the irq masks */
 	spinlock_t irq_lock;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1391c01d7663..5256bd306455 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1263,7 +1263,11 @@ void i915_gem_driver_remove(struct drm_i915_private *dev_priv)
 	i915_gem_suspend_late(dev_priv);
 	for_each_gt(gt, dev_priv, i)
 		intel_gt_driver_remove(gt);
+
+	/* Let's make sure no one is using the uabi_engines list */
+	mutex_lock(&dev_priv->uabi_engines_mutex);
 	dev_priv->uabi_engines = RB_ROOT;
+	mutex_unlock(&dev_priv->uabi_engines_mutex);
 
 	/* Flush any outstanding unpin_work. */
 	i915_gem_drain_workqueue(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 025a79fe5920..e1a823c440eb 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2688,7 +2688,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
 	struct intel_engine_cs *engine;
 	struct intel_gt *gt = stream->engine->gt;
 	struct i915_gem_context *ctx, *cn;
-	int err;
+	int err = 0;
 
 	lockdep_assert_held(&gt->perf.lock);
 
@@ -2732,6 +2732,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
 	 * If we don't modify the kernel_context, we do not get events while
 	 * idle.
 	 */
+	mutex_lock(&i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, i915) {
 		struct intel_context *ce = engine->kernel_context;
 
@@ -2742,10 +2743,11 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
 
 		err = gen8_modify_self(ce, regs, num_regs, active);
 		if (err)
-			return err;
+			break;
 	}
+	mutex_unlock(&i915->uabi_engines_mutex);
 
-	return 0;
+	return err;
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 21eb0c5b320d..df5d319e8c38 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -1022,6 +1022,7 @@ create_event_attributes(struct i915_pmu *pmu)
 		}
 	}
 
+	mutex_lock(&i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, i915) {
 		for (i = 0; i < ARRAY_SIZE(engine_events); i++) {
 			if (!engine_event_status(engine,
@@ -1029,6 +1030,7 @@ create_event_attributes(struct i915_pmu *pmu)
 				count++;
 		}
 	}
+	mutex_unlock(&i915->uabi_engines_mutex);
 
 	/* Allocate attribute objects and table. */
 	i915_attr = kcalloc(count, sizeof(*i915_attr), GFP_KERNEL);
@@ -1086,6 +1088,7 @@ create_event_attributes(struct i915_pmu *pmu)
 	}
 
 	/* Initialize supported engine counters. */
+	mutex_lock(&i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, i915) {
 		for (i = 0; i < ARRAY_SIZE(engine_events); i++) {
 			char *str;
@@ -1097,7 +1100,7 @@ create_event_attributes(struct i915_pmu *pmu)
 			str = kasprintf(GFP_KERNEL, "%s-%s",
 					engine->name, engine_events[i].name);
 			if (!str)
-				goto err;
+				goto err_unlock;
 
 			*attr_iter++ = &i915_iter->attr.attr;
 			i915_iter =
@@ -1109,18 +1112,22 @@ create_event_attributes(struct i915_pmu *pmu)
 			str = kasprintf(GFP_KERNEL, "%s-%s.unit",
 					engine->name, engine_events[i].name);
 			if (!str)
-				goto err;
+				goto err_unlock;
 
 			*attr_iter++ = &pmu_iter->attr.attr;
 			pmu_iter = add_pmu_attr(pmu_iter, str, "ns");
 		}
 	}
+	mutex_unlock(&i915->uabi_engines_mutex);
 
 	pmu->i915_attr = i915_attr;
 	pmu->pmu_attr = pmu_attr;
 
 	return attr;
 
+err_unlock:
+	mutex_unlock(&i915->uabi_engines_mutex);
+
 err:;
 	for (attr_iter = attr; *attr_iter; attr_iter++)
 		kfree((*attr_iter)->name);
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 14d9ec0ed777..7c6669cc4c96 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -140,6 +140,7 @@ query_engine_info(struct drm_i915_private *i915,
 	if (query_item->flags)
 		return -EINVAL;
 
+	mutex_lock(&i915->uabi_engines_mutex);
 	for_each_uabi_engine(engine, i915)
 		num_uabi_engines++;
 
@@ -147,11 +148,13 @@ query_engine_info(struct drm_i915_private *i915,
 
 	ret = copy_query_item(&query, sizeof(query), len, query_item);
 	if (ret != 0)
-		return ret;
+		goto err;
 
 	if (query.num_engines || query.rsvd[0] || query.rsvd[1] ||
-	    query.rsvd[2])
-		return -EINVAL;
+	    query.rsvd[2]) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	info_ptr = &query_ptr->engines[0];
 
@@ -162,17 +165,25 @@ query_engine_info(struct drm_i915_private *i915,
 		info.capabilities = engine->uabi_capabilities;
 		info.logical_instance = ilog2(engine->logical_mask);
 
-		if (copy_to_user(info_ptr, &info, sizeof(info)))
-			return -EFAULT;
+		if (copy_to_user(info_ptr, &info, sizeof(info))) {
+			ret = -EFAULT;
+			goto err;
+		}
 
 		query.num_engines++;
 		info_ptr++;
 	}
+	mutex_unlock(&i915->uabi_engines_mutex);
 
 	if (copy_to_user(query_ptr, &query, sizeof(query)))
 		return -EFAULT;
 
 	return len;
+
+err:
+	mutex_unlock(&i915->uabi_engines_mutex);
+
+	return ret;
 }
 
 static int can_copy_perf_config_registers_or_number(u32 user_n_regs,
-- 
2.45.2


  parent reply	other threads:[~2024-08-23 13:10 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-23 13:08 [PATCH v3 00/15] CCS static load balance Andi Shyti
2024-08-23 13:08 ` [PATCH v3 01/15] drm/i915/gt: Avoid using masked workaround for CCS_MODE setting Andi Shyti
2024-08-23 13:08 ` [PATCH v3 02/15] drm/i915/gt: Move the CCS mode variable to a global position Andi Shyti
2024-08-23 13:08 ` [PATCH v3 03/15] drm/i915/gt: Allow the creation of multi-mode CCS masks Andi Shyti
2024-08-23 13:08 ` [PATCH v3 04/15] drm/i915/gt: Refactor uabi engine class/instance list creation Andi Shyti
2024-08-23 13:08 ` [PATCH v3 05/15] drm/i915/gem: Mark and verify UABI engine validity Andi Shyti
2024-08-23 13:08 ` [PATCH v3 06/15] drm/i915/gt: Introduce for_each_enabled_engine() and apply it in selftests Andi Shyti
2024-08-23 13:08 ` [PATCH v3 07/15] drm/i915/gt: Manage CCS engine creation within UABI exposure Andi Shyti
2024-08-23 13:08 ` [PATCH v3 08/15] drm/i915/gt: Remove cslices mask value from the CCS structure Andi Shyti
2024-08-23 13:08 ` [PATCH v3 09/15] drm/i915/gt: Expose the number of total CCS slices Andi Shyti
2024-08-23 13:08 ` [PATCH v3 10/15] drm/i915/gt: Store engine-related sysfs kobjects Andi Shyti
2024-08-23 13:08 ` [PATCH v3 11/15] drm/i915/gt: Store active CCS mask Andi Shyti
2024-08-23 13:08 ` Andi Shyti [this message]
2024-08-23 13:08 ` [PATCH v3 13/15] drm/i915/gt: Isolate single sysfs engine file creation Andi Shyti
2024-08-23 13:08 ` [PATCH v3 14/15] drm/i915/gt: Implement creation and removal routines for CCS engines Andi Shyti
2024-08-26 22:07   ` kernel test robot
2024-08-23 13:08 ` [PATCH v3 15/15] drm/i915/gt: Allow the user to change the CCS mode through sysfs Andi Shyti
2024-08-27 17:36   ` Daniel Vetter
2024-08-23 14:14 ` ✗ Fi.CI.CHECKPATCH: warning for CCS static load balance Patchwork
2024-08-23 14:14 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-08-23 14:26 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-08-23 15:48 ` ✗ Fi.CI.CHECKPATCH: warning for CCS static load balance (rev2) Patchwork
2024-08-23 15:48 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-08-23 16:04 ` ✓ Fi.CI.BAT: success " Patchwork
2024-08-24 14:44 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-08-27  9:25 ` [PATCH v3 00/15] CCS static load balance Andi Shyti
2024-08-27 17:31 ` Daniel Vetter
2024-08-28  8:20   ` Andi Shyti
2024-08-28 13:47     ` Daniel Vetter
2024-08-28 15:35       ` Andi Shyti
2024-09-02 11:13         ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240823130855.72436-13-andi.shyti@linux.intel.com \
    --to=andi.shyti@linux.intel.com \
    --cc=chris.p.wilson@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=tursulin@ursulin.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox