Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 0/2] drm/i915/selftests: Use safe userspace memory for mappings
@ 2026-05-08  8:02 Krzysztof Karas
  2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
  2026-05-08  8:02 ` [PATCH v10 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
  0 siblings, 2 replies; 8+ messages in thread
From: Krzysztof Karas @ 2026-05-08  8:02 UTC (permalink / raw)
  To: intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik, Krzysztof Karas

Test-with: 20260330102400.1157658-1-krzysztof.karas@intel.com

Currently, i915 selftests use unknown process's address space to
perform mappings to userspace memory. This is problematic,
because there is no control over lifetime of the memory of such
task, so the test would occasionally borrow memory scheduled for
or in the middle of a cleanup causing SIGBUS errors.

Utilize user-provided PID of running userspace process to
perfofm mapping in a safe environment.

Krzysztof Karas (2):
  drm/i915/selftests: Prevent userspace mapping invalidation
  drm/i915/selftests: Run vma tests only if current->mm is present

 .../drm/i915/gem/selftests/i915_gem_mman.c    | 16 ++--
 drivers/gpu/drm/i915/i915_selftest.h          |  1 +
 .../gpu/drm/i915/selftests/i915_selftest.c    | 75 ++++++++++++++++++-
 3 files changed, 83 insertions(+), 9 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-05-08  8:02 [PATCH v10 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
@ 2026-05-08  8:02 ` Krzysztof Karas
  2026-05-08  8:53   ` Janusz Krzysztofik
                     ` (3 more replies)
  2026-05-08  8:02 ` [PATCH v10 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
  1 sibling, 4 replies; 8+ messages in thread
From: Krzysztof Karas @ 2026-05-08  8:02 UTC (permalink / raw)
  To: intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik, Krzysztof Karas

Migration testing in i915 assumes current task's address space
to allocate new userspace mapping and uses it without
registering real user for that address space in mm_struct.
On single NUMA node setups PCI probe executes in the same
context as userspace process calling the test (i915_selftest
from IGT), but when multiple nodes are available, the PCI code
puts probe into a kernel workqueue. This switches execution to
a kworker, which does not have its own address space in
userspace and must borrow such memory from another process, so
"current->active_mm" is unknown at the start of the test.

It was observed that mm->mm_users would occasionally be 0
or drop to 0 during the test due to short delay between
scheduling and executing work in forked process, which reaped
userspace mappings, further leading to failures upon reading
from userland memory.

Prevent this by adding a PID parameter to a trusted task, so its
mm struct may be used if needed.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
v8:
 * Keep reference to mm open for the duration of test for
 readability. (Sebastian)
 * Be paranoic and explicit about keeping the mm reference,
 so we are **really** sure about userspace mappings not
 diappearing.

v9:
 * Drop "Fixes" tag. (Andi)
 * Revert to using a separate function for mm acquisition. (Andi)
 * Keep kthread_use/unuse and mmget/mmput calls symmetric. (Janusz)

v10:
 * Initialize PID variable to a negative value and check for
 user provided value. (Janusz)

 drivers/gpu/drm/i915/i915_selftest.h          |  1 +
 .../gpu/drm/i915/selftests/i915_selftest.c    | 75 ++++++++++++++++++-
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
index 72922028f4ba..e29ca298e7eb 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -35,6 +35,7 @@ struct i915_selftest {
 	unsigned long timeout_jiffies;
 	unsigned int timeout_ms;
 	unsigned int random_seed;
+	unsigned int userspace_pid;
 	char *filter;
 	int mock;
 	int live;
diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
index 8460f0a70d04..036328072e38 100644
--- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
+++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
@@ -181,13 +181,57 @@ __wait_gsc_huc_load_completed(struct drm_i915_private *i915)
 		pr_warn(DRIVER_NAME "Timed out waiting for huc load via GSC!\n");
 }
 
+static struct mm_struct *
+get_mm(int u_pid_nr)
+{
+	struct task_struct *task = NULL;
+	struct mm_struct *mm = NULL;
+	struct pid *u_pid = NULL;
+
+	if (u_pid_nr < 1)
+		return NULL;
+
+	u_pid = find_get_pid(u_pid_nr);
+	if (!u_pid) {
+		pr_warn("Could not find PID: %d\n", u_pid_nr);
+		return NULL;
+	}
+
+	task = get_pid_task(u_pid, PIDTYPE_PID);
+	put_pid(u_pid);
+	if (!task) {
+		pr_warn("Could not find task for PID: %d\n", u_pid_nr);
+		return NULL;
+	}
+
+	if (task->flags & PF_KTHREAD) {
+		pr_warn("Task not in userspace: %d\n", u_pid_nr);
+		put_task_struct(task);
+		return NULL;
+	}
+
+	mm = get_task_mm(task);
+	put_task_struct(task);
+	if (!mm) {
+		pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
+		return NULL;
+	}
+
+	return mm;
+}
+
 static int __run_selftests(const char *name,
 			   struct selftest *st,
 			   unsigned int count,
 			   void *data)
 {
+	struct mm_struct *mm = NULL;
+	int u_pid_nr = -1;
 	int err = 0;
 
+	if (i915_selftest.userspace_pid)
+		u_pid_nr = i915_selftest.userspace_pid;
+
 	while (!i915_selftest.random_seed)
 		i915_selftest.random_seed = get_random_u32();
 
@@ -201,14 +245,36 @@ static int __run_selftests(const char *name,
 	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
 		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
 
+	/*
+	 * If we are running in a kthread on a multi NUMA system and the user passed
+	 * a valid PID of a userspace task, then we may borrow its address space
+	 * to prepare a safe environment for the mmap selftests.
+	 */
+	if (!current->mm && u_pid_nr > 0) {
+		mm = get_mm(u_pid_nr);
+		if (mm) {
+			kthread_use_mm(mm);
+			if (unlikely(!current->mm)) {
+				mmput(mm);
+				mm = NULL;
+				pr_warn("Could not set mm as current->mm\n");
+			}
+		}
+	}
+
 	/* Tests are listed in order in i915_*_selftests.h */
 	for (; count--; st++) {
 		if (!st->enabled)
 			continue;
 
 		cond_resched();
-		if (signal_pending(current))
+		if (signal_pending(current)) {
+			if (mm) {
+				kthread_unuse_mm(mm);
+				mmput_async(mm);
+			}
 			return -EINTR;
+		}
 
 		pr_info(DRIVER_NAME ": Running %s\n", st->name);
 		if (data)
@@ -226,6 +292,11 @@ static int __run_selftests(const char *name,
 		 st->name, err))
 		err = -1;
 
+	if (mm) {
+		kthread_unuse_mm(mm);
+		mmput_async(mm);
+	}
+
 	return err;
 }
 
@@ -507,6 +578,8 @@ void igt_hexdump(const void *buf, size_t len)
 module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
 module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
 module_param_named(st_filter, i915_selftest.filter, charp, 0400);
+module_param_named(st_userspace_pid, i915_selftest.userspace_pid, uint, 0400);
+MODULE_PARM_DESC(st_userspace_pid, "For usage in tests that map userspace memory and require address space with controllable lifetime.");
 
 module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
 MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then leave dummy module)");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v10 2/2] drm/i915/selftests: Run vma tests only if current->mm is present
  2026-05-08  8:02 [PATCH v10 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
  2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
@ 2026-05-08  8:02 ` Krzysztof Karas
  2026-05-08  9:44   ` Michał Grzelak
  1 sibling, 1 reply; 8+ messages in thread
From: Krzysztof Karas @ 2026-05-08  8:02 UTC (permalink / raw)
  To: intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik, Krzysztof Karas

This set of tests require userspace memory to map objects, so
run them only if this that memory is available.

Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
 .../gpu/drm/i915/gem/selftests/i915_gem_mman.c   | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9d454d0b46f2..d01acfb7d93d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -1847,11 +1847,12 @@ static int igt_mmap_revoke(void *arg)
 int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 {
 	int ret;
-	bool unuse_mm = false;
 	static const struct i915_subtest tests[] = {
 		SUBTEST(igt_partial_tiling),
 		SUBTEST(igt_smoke_tiling),
 		SUBTEST(igt_mmap_offset_exhaustion),
+	};
+	static const struct i915_subtest vma_tests[] = {
 		SUBTEST(igt_mmap),
 		SUBTEST(igt_mmap_migrate),
 		SUBTEST(igt_mmap_access),
@@ -1859,15 +1860,14 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_mmap_gpu),
 	};
 
-	if (!current->mm) {
-		kthread_use_mm(current->active_mm);
-		unuse_mm = true;
-	}
-
 	ret = i915_live_subtests(tests, i915);
+	if (ret)
+		return ret;
 
-	if (unuse_mm)
-		kthread_unuse_mm(current->active_mm);
+	if (current->mm)
+		ret = i915_live_subtests(vma_tests, i915);
+	else
+		pr_warn("No current->mm to safely borrow userspace memory from. Skipping VMA tests.\n");
 
 	return ret;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
@ 2026-05-08  8:53   ` Janusz Krzysztofik
  2026-05-08  9:15   ` Sebastian Brzezinka
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Janusz Krzysztofik @ 2026-05-08  8:53 UTC (permalink / raw)
  To: Krzysztof Karas, intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec

On Fri, 2026-05-08 at 08:02 +0000, Krzysztof Karas wrote:
> Migration testing in i915 assumes current task's address space
> to allocate new userspace mapping and uses it without
> registering real user for that address space in mm_struct.
> On single NUMA node setups PCI probe executes in the same
> context as userspace process calling the test (i915_selftest
> from IGT), but when multiple nodes are available, the PCI code
> puts probe into a kernel workqueue. This switches execution to
> a kworker, which does not have its own address space in
> userspace and must borrow such memory from another process, so
> "current->active_mm" is unknown at the start of the test.
> 
> It was observed that mm->mm_users would occasionally be 0
> or drop to 0 during the test due to short delay between
> scheduling and executing work in forked process, which reaped
> userspace mappings, further leading to failures upon reading
> from userland memory.
> 
> Prevent this by adding a PID parameter to a trusted task, so its
> mm struct may be used if needed.
> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>

Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

> ---
> v8:
>  * Keep reference to mm open for the duration of test for
>  readability. (Sebastian)
>  * Be paranoic and explicit about keeping the mm reference,
>  so we are **really** sure about userspace mappings not
>  diappearing.
> 
> v9:
>  * Drop "Fixes" tag. (Andi)
>  * Revert to using a separate function for mm acquisition. (Andi)
>  * Keep kthread_use/unuse and mmget/mmput calls symmetric. (Janusz)
> 
> v10:
>  * Initialize PID variable to a negative value and check for
>  user provided value. (Janusz)
> 
>  drivers/gpu/drm/i915/i915_selftest.h          |  1 +
>  .../gpu/drm/i915/selftests/i915_selftest.c    | 75 ++++++++++++++++++-
>  2 files changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index 72922028f4ba..e29ca298e7eb 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -35,6 +35,7 @@ struct i915_selftest {
>  	unsigned long timeout_jiffies;
>  	unsigned int timeout_ms;
>  	unsigned int random_seed;
> +	unsigned int userspace_pid;
>  	char *filter;
>  	int mock;
>  	int live;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 8460f0a70d04..036328072e38 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -181,13 +181,57 @@ __wait_gsc_huc_load_completed(struct drm_i915_private *i915)
>  		pr_warn(DRIVER_NAME "Timed out waiting for huc load via GSC!\n");
>  }
>  
> +static struct mm_struct *
> +get_mm(int u_pid_nr)
> +{
> +	struct task_struct *task = NULL;
> +	struct mm_struct *mm = NULL;
> +	struct pid *u_pid = NULL;
> +
> +	if (u_pid_nr < 1)
> +		return NULL;
> +
> +	u_pid = find_get_pid(u_pid_nr);
> +	if (!u_pid) {
> +		pr_warn("Could not find PID: %d\n", u_pid_nr);
> +		return NULL;
> +	}
> +
> +	task = get_pid_task(u_pid, PIDTYPE_PID);
> +	put_pid(u_pid);
> +	if (!task) {
> +		pr_warn("Could not find task for PID: %d\n", u_pid_nr);
> +		return NULL;
> +	}
> +
> +	if (task->flags & PF_KTHREAD) {
> +		pr_warn("Task not in userspace: %d\n", u_pid_nr);
> +		put_task_struct(task);
> +		return NULL;
> +	}
> +
> +	mm = get_task_mm(task);
> +	put_task_struct(task);
> +	if (!mm) {
> +		pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
> +		return NULL;
> +	}
> +
> +	return mm;
> +}
> +
>  static int __run_selftests(const char *name,
>  			   struct selftest *st,
>  			   unsigned int count,
>  			   void *data)
>  {
> +	struct mm_struct *mm = NULL;
> +	int u_pid_nr = -1;
>  	int err = 0;
>  
> +	if (i915_selftest.userspace_pid)
> +		u_pid_nr = i915_selftest.userspace_pid;
> +
>  	while (!i915_selftest.random_seed)
>  		i915_selftest.random_seed = get_random_u32();
>  
> @@ -201,14 +245,36 @@ static int __run_selftests(const char *name,
>  	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
>  		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
>  
> +	/*
> +	 * If we are running in a kthread on a multi NUMA system and the user passed
> +	 * a valid PID of a userspace task, then we may borrow its address space
> +	 * to prepare a safe environment for the mmap selftests.
> +	 */
> +	if (!current->mm && u_pid_nr > 0) {
> +		mm = get_mm(u_pid_nr);
> +		if (mm) {
> +			kthread_use_mm(mm);
> +			if (unlikely(!current->mm)) {
> +				mmput(mm);
> +				mm = NULL;
> +				pr_warn("Could not set mm as current->mm\n");
> +			}
> +		}
> +	}
> +
>  	/* Tests are listed in order in i915_*_selftests.h */
>  	for (; count--; st++) {
>  		if (!st->enabled)
>  			continue;
>  
>  		cond_resched();
> -		if (signal_pending(current))
> +		if (signal_pending(current)) {
> +			if (mm) {
> +				kthread_unuse_mm(mm);
> +				mmput_async(mm);
> +			}
>  			return -EINTR;
> +		}
>  
>  		pr_info(DRIVER_NAME ": Running %s\n", st->name);
>  		if (data)
> @@ -226,6 +292,11 @@ static int __run_selftests(const char *name,
>  		 st->name, err))
>  		err = -1;
>  
> +	if (mm) {
> +		kthread_unuse_mm(mm);
> +		mmput_async(mm);
> +	}
> +
>  	return err;
>  }
>  
> @@ -507,6 +578,8 @@ void igt_hexdump(const void *buf, size_t len)
>  module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
>  module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
>  module_param_named(st_filter, i915_selftest.filter, charp, 0400);
> +module_param_named(st_userspace_pid, i915_selftest.userspace_pid, uint, 0400);
> +MODULE_PARM_DESC(st_userspace_pid, "For usage in tests that map userspace memory and require address space with controllable lifetime.");
>  
>  module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
>  MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then leave dummy module)");

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
  2026-05-08  8:53   ` Janusz Krzysztofik
@ 2026-05-08  9:15   ` Sebastian Brzezinka
  2026-05-08  9:41   ` Andi Shyti
  2026-05-08  9:42   ` Michał Grzelak
  3 siblings, 0 replies; 8+ messages in thread
From: Sebastian Brzezinka @ 2026-05-08  9:15 UTC (permalink / raw)
  To: Krzysztof Karas, intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik

On Fri May 8, 2026 at 10:02 AM CEST, Krzysztof Karas wrote:
> Migration testing in i915 assumes current task's address space
> to allocate new userspace mapping and uses it without
> registering real user for that address space in mm_struct.
> On single NUMA node setups PCI probe executes in the same
> context as userspace process calling the test (i915_selftest
> from IGT), but when multiple nodes are available, the PCI code
> puts probe into a kernel workqueue. This switches execution to
> a kworker, which does not have its own address space in
> userspace and must borrow such memory from another process, so
> "current->active_mm" is unknown at the start of the test.
>
> It was observed that mm->mm_users would occasionally be 0
> or drop to 0 during the test due to short delay between
> scheduling and executing work in forked process, which reaped
> userspace mappings, further leading to failures upon reading
> from userland memory.
>
> Prevent this by adding a PID parameter to a trusted task, so its
> mm struct may be used if needed.
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>

-- 
Best regards,
Sebastian


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
  2026-05-08  8:53   ` Janusz Krzysztofik
  2026-05-08  9:15   ` Sebastian Brzezinka
@ 2026-05-08  9:41   ` Andi Shyti
  2026-05-08  9:42   ` Michał Grzelak
  3 siblings, 0 replies; 8+ messages in thread
From: Andi Shyti @ 2026-05-08  9:41 UTC (permalink / raw)
  To: Krzysztof Karas
  Cc: intel-gfx, Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik

Hi Krzysztof,

...

> +static struct mm_struct *
> +get_mm(int u_pid_nr)

This fits in one line, no need to break it.

I'm not a big fan of the name "get_mm()". It's too generic and
sounds like a library function.

With the two above changed:

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

On the other hand I don't want to ask for v11: if it's OK with
you I would bring the lines together and change the function to
get_selftest_mm() before merging.

Let me know,
Andi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
                     ` (2 preceding siblings ...)
  2026-05-08  9:41   ` Andi Shyti
@ 2026-05-08  9:42   ` Michał Grzelak
  3 siblings, 0 replies; 8+ messages in thread
From: Michał Grzelak @ 2026-05-08  9:42 UTC (permalink / raw)
  To: Krzysztof Karas
  Cc: intel-gfx, Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik

[-- Attachment #1: Type: text/plain, Size: 6260 bytes --]

Hi Krzysztof,

On Fri, 8 May 2026, Krzysztof Karas wrote:
> Migration testing in i915 assumes current task's address space
> to allocate new userspace mapping and uses it without
> registering real user for that address space in mm_struct.
> On single NUMA node setups PCI probe executes in the same
> context as userspace process calling the test (i915_selftest
> from IGT), but when multiple nodes are available, the PCI code
> puts probe into a kernel workqueue. This switches execution to
> a kworker, which does not have its own address space in
> userspace and must borrow such memory from another process, so
> "current->active_mm" is unknown at the start of the test.
>
> It was observed that mm->mm_users would occasionally be 0
> or drop to 0 during the test due to short delay between
> scheduling and executing work in forked process, which reaped
> userspace mappings, further leading to failures upon reading
> from userland memory.
>
> Prevent this by adding a PID parameter to a trusted task, so its
> mm struct may be used if needed.

nit: I think it will sound better with s/mm struct/mm_struct/.

>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> v8:
> * Keep reference to mm open for the duration of test for
> readability. (Sebastian)
> * Be paranoic and explicit about keeping the mm reference,
> so we are **really** sure about userspace mappings not
> diappearing.

another nit: can we s/diappearing/disappearing/?

Other than that:
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>

BR,
Michał

>
> v9:
> * Drop "Fixes" tag. (Andi)
> * Revert to using a separate function for mm acquisition. (Andi)
> * Keep kthread_use/unuse and mmget/mmput calls symmetric. (Janusz)
>
> v10:
> * Initialize PID variable to a negative value and check for
> user provided value. (Janusz)
>
> drivers/gpu/drm/i915/i915_selftest.h          |  1 +
> .../gpu/drm/i915/selftests/i915_selftest.c    | 75 ++++++++++++++++++-
> 2 files changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index 72922028f4ba..e29ca298e7eb 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -35,6 +35,7 @@ struct i915_selftest {
> 	unsigned long timeout_jiffies;
> 	unsigned int timeout_ms;
> 	unsigned int random_seed;
> +	unsigned int userspace_pid;
> 	char *filter;
> 	int mock;
> 	int live;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 8460f0a70d04..036328072e38 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -181,13 +181,57 @@ __wait_gsc_huc_load_completed(struct drm_i915_private *i915)
> 		pr_warn(DRIVER_NAME "Timed out waiting for huc load via GSC!\n");
> }
>
> +static struct mm_struct *
> +get_mm(int u_pid_nr)
> +{
> +	struct task_struct *task = NULL;
> +	struct mm_struct *mm = NULL;
> +	struct pid *u_pid = NULL;
> +
> +	if (u_pid_nr < 1)
> +		return NULL;
> +
> +	u_pid = find_get_pid(u_pid_nr);
> +	if (!u_pid) {
> +		pr_warn("Could not find PID: %d\n", u_pid_nr);
> +		return NULL;
> +	}
> +
> +	task = get_pid_task(u_pid, PIDTYPE_PID);
> +	put_pid(u_pid);
> +	if (!task) {
> +		pr_warn("Could not find task for PID: %d\n", u_pid_nr);
> +		return NULL;
> +	}
> +
> +	if (task->flags & PF_KTHREAD) {
> +		pr_warn("Task not in userspace: %d\n", u_pid_nr);
> +		put_task_struct(task);
> +		return NULL;
> +	}
> +
> +	mm = get_task_mm(task);
> +	put_task_struct(task);
> +	if (!mm) {
> +		pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
> +		return NULL;
> +	}
> +
> +	return mm;
> +}
> +
> static int __run_selftests(const char *name,
> 			   struct selftest *st,
> 			   unsigned int count,
> 			   void *data)
> {
> +	struct mm_struct *mm = NULL;
> +	int u_pid_nr = -1;
> 	int err = 0;
>
> +	if (i915_selftest.userspace_pid)
> +		u_pid_nr = i915_selftest.userspace_pid;
> +
> 	while (!i915_selftest.random_seed)
> 		i915_selftest.random_seed = get_random_u32();
>
> @@ -201,14 +245,36 @@ static int __run_selftests(const char *name,
> 	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
> 		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
>
> +	/*
> +	 * If we are running in a kthread on a multi NUMA system and the user passed
> +	 * a valid PID of a userspace task, then we may borrow its address space
> +	 * to prepare a safe environment for the mmap selftests.
> +	 */
> +	if (!current->mm && u_pid_nr > 0) {
> +		mm = get_mm(u_pid_nr);
> +		if (mm) {
> +			kthread_use_mm(mm);
> +			if (unlikely(!current->mm)) {
> +				mmput(mm);
> +				mm = NULL;
> +				pr_warn("Could not set mm as current->mm\n");
> +			}
> +		}
> +	}
> +
> 	/* Tests are listed in order in i915_*_selftests.h */
> 	for (; count--; st++) {
> 		if (!st->enabled)
> 			continue;
>
> 		cond_resched();
> -		if (signal_pending(current))
> +		if (signal_pending(current)) {
> +			if (mm) {
> +				kthread_unuse_mm(mm);
> +				mmput_async(mm);
> +			}
> 			return -EINTR;
> +		}
>
> 		pr_info(DRIVER_NAME ": Running %s\n", st->name);
> 		if (data)
> @@ -226,6 +292,11 @@ static int __run_selftests(const char *name,
> 		 st->name, err))
> 		err = -1;
>
> +	if (mm) {
> +		kthread_unuse_mm(mm);
> +		mmput_async(mm);
> +	}
> +
> 	return err;
> }
>
> @@ -507,6 +578,8 @@ void igt_hexdump(const void *buf, size_t len)
> module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
> module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
> module_param_named(st_filter, i915_selftest.filter, charp, 0400);
> +module_param_named(st_userspace_pid, i915_selftest.userspace_pid, uint, 0400);
> +MODULE_PARM_DESC(st_userspace_pid, "For usage in tests that map userspace memory and require address space with controllable lifetime.");
>
> module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
> MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then leave dummy module)");
> -- 
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 2/2] drm/i915/selftests: Run vma tests only if current->mm is present
  2026-05-08  8:02 ` [PATCH v10 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
@ 2026-05-08  9:44   ` Michał Grzelak
  0 siblings, 0 replies; 8+ messages in thread
From: Michał Grzelak @ 2026-05-08  9:44 UTC (permalink / raw)
  To: Krzysztof Karas
  Cc: intel-gfx, Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

On Fri, 8 May 2026, Krzysztof Karas wrote:
> This set of tests require userspace memory to map objects, so
> run them only if this that memory is available.

I think the word "this" can be dropped from the description.

Otherwise,
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>

>
> Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> .../gpu/drm/i915/gem/selftests/i915_gem_mman.c   | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 9d454d0b46f2..d01acfb7d93d 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -1847,11 +1847,12 @@ static int igt_mmap_revoke(void *arg)
> int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
> {
> 	int ret;
> -	bool unuse_mm = false;
> 	static const struct i915_subtest tests[] = {
> 		SUBTEST(igt_partial_tiling),
> 		SUBTEST(igt_smoke_tiling),
> 		SUBTEST(igt_mmap_offset_exhaustion),
> +	};
> +	static const struct i915_subtest vma_tests[] = {
> 		SUBTEST(igt_mmap),
> 		SUBTEST(igt_mmap_migrate),
> 		SUBTEST(igt_mmap_access),
> @@ -1859,15 +1860,14 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
> 		SUBTEST(igt_mmap_gpu),
> 	};
>
> -	if (!current->mm) {
> -		kthread_use_mm(current->active_mm);
> -		unuse_mm = true;
> -	}
> -
> 	ret = i915_live_subtests(tests, i915);
> +	if (ret)
> +		return ret;
>
> -	if (unuse_mm)
> -		kthread_unuse_mm(current->active_mm);
> +	if (current->mm)
> +		ret = i915_live_subtests(vma_tests, i915);
> +	else
> +		pr_warn("No current->mm to safely borrow userspace memory from. Skipping VMA tests.\n");
>
> 	return ret;
> }
> -- 
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-08  9:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08  8:02 [PATCH v10 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
2026-05-08  8:02 ` [PATCH v10 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
2026-05-08  8:53   ` Janusz Krzysztofik
2026-05-08  9:15   ` Sebastian Brzezinka
2026-05-08  9:41   ` Andi Shyti
2026-05-08  9:42   ` Michał Grzelak
2026-05-08  8:02 ` [PATCH v10 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
2026-05-08  9:44   ` Michał Grzelak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox