[PATCH v4 00/18] drm/panthor: Fix the unplug logic

[PATCH v4 00/18] drm/panthor: Fix the unplug logic

[Boris Brezillon]

The current unplug logic is broken in multiple ways. This is an attempt
at addressing the various problems found along the way (some were
reported by Sashiko, others have been found while trying to address
Sashiko's concerns).

Sending a new version even though v2 didn't receive any human review
just to try and address the new stuff pointed out by Sashiko.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v4:
- Revisit the RPM teardown logic to properly control when the last call to
  panthor_device_{suspend,resume}() happens
- Revisit the logic to temporarily disable the reset work in order to simplify
  the state tracking/locking
- Make the cleanup_wq/pt_cache device-local instead of global
- Make sure an AS won't be put back in the reclaim list after the VM has
  been destroyed
- Assume SOFT_RESETs can never fail to simplify things (no more explicit memory
  leaks needed if this holds true)
- Make sure l2_power_off() errors are taken into account with a
  __must_check attribute
- Link to v3: https://patch.msgid.link/20260813-panthor-unplug-fixes-v3-0-3ed4e961bbe7@collabora.com

Changes in v3:
- Fix a race in the reset reschedule logic we added to
  panthor_device_resume() (missing smp_mb__after_atomic())
- Fix a VM leak when reset and suspend are racing with each other
- Add missing drm_dev_enter/exit() sections
- Insert the groups in the user_owned list even if the group creation
  happens during a reset
- Try to document why some of the issues pointed out by Sashiko are
  either not real issues, or are expected (either fixed in a later
  commits, or just expected behavior)
- Fix a race between panthor_device_unplug() and vm_prep_for_cleanup()
  (introduced in v2)
- Link to v2: https://patch.msgid.link/20260811-panthor-unplug-fixes-v2-0-6b583e37f9ae@collabora.com

Changes in v2:
- Fix UAFs caused by deferred cleanup works
- Fix UAFs caused by open FDs closed after unplug
- Fix deadlock when device_unplug() is called from the reset work
- Make sure reset requests are not lost in the resume and post_reset
  paths
- Fix a deadlock in the suspend path
- Fix a clk prepare_enable leak in the unplug path
- Don't use a drmm_action to flush the cleanup queue (this could cause
  UAFs)
- Drop the now unused panthor_vm::unusable field
- Keep track of user owned resources to prevent leaks and/or UAFs
- Link to v1: https://patch.msgid.link/20260804-panthor-unplug-fixes-v1-0-abbbd2d41b13@collabora.com

---
Boris Brezillon (18):
      drm/panthor: Disable reset work before unplug
      drm/panthor: Revisit the reset logic to avoid reset request loss
      drm/panthor: Make panthor_device::pm::state non-atomic
      drm/panthor: Flush the cleanup_wq in the unplug path
      drm/panthor: Make the page table cache and cleanup workqueue device-local
      drm/panthor: Drop unused vm argument passed to panthor_vm_prepare_sync_only_op_ctx()
      drm/panthor: Move the debugfs initialization to panthor_device.c
      drm/panthor: Split panthor_vm
      drm/panthor: Add fine-grained restrictions on VMs
      drm/panthor: Check AS state before disabling
      drm/panthor: Don't pre-allocate VMAs or page tables when preparing a full VM unmap
      drm/panthor: Let l2_power_off return errors and force users to check it
      drm/panthor: Complain if the SOFT_RESET fails
      drm/panthor: Make the VM cleanup path more robust against UAF
      drm/panthor: Track user owned VMs
      drm/panthor: Track user owned groups
      drm/panthor: Fix the unplug logic
      drm/panthor: Add debugfs knobs to simulate reset failures

 drivers/gpu/drm/panthor/panthor_device.c |  282 ++++--
 drivers/gpu/drm/panthor/panthor_device.h |   42 +-
 drivers/gpu/drm/panthor/panthor_drv.c    |  179 ++--
 drivers/gpu/drm/panthor/panthor_fw.c     |    9 +-
 drivers/gpu/drm/panthor/panthor_gpu.c    |   17 +-
 drivers/gpu/drm/panthor/panthor_gpu.h    |    2 +-
 drivers/gpu/drm/panthor/panthor_hw.h     |   19 +-
 drivers/gpu/drm/panthor/panthor_mmu.c    | 1566 ++++++++++++++++++------------
 drivers/gpu/drm/panthor/panthor_mmu.h    |    6 +-
 drivers/gpu/drm/panthor/panthor_pwr.c    |   22 +-
 drivers/gpu/drm/panthor/panthor_pwr.h    |    2 +-
 drivers/gpu/drm/panthor/panthor_sched.c  |  141 ++-
 12 files changed, 1455 insertions(+), 832 deletions(-)
---
base-commit: 44e9eb5a762142a4aa46c0b5da7c39bfeb78910e
change-id: 20260804-panthor-unplug-fixes-7927b3ddc2f9
prerequisite-change-id: 20260429-panthor-signal-from-irq-d33684f4d292:v7
prerequisite-patch-id: 35735cbaa1997e2df4b2aae61970d580a94e755a
prerequisite-patch-id: 1b18d1d374715eaf8d1141ac9e51a0e304cf01ba
prerequisite-patch-id: ec524c6adfca6c0d694e0a750dc19e8eac106f45
prerequisite-patch-id: 4dc1e1dd06539c8274987208ba8737d2ab9bec30
prerequisite-patch-id: 2c4b51b7853ae85888e3181b00425f5615eb81ac
prerequisite-patch-id: 7d0cc8e0ec61131f0c715a8c74cb73aceffb1124
prerequisite-patch-id: 05ef0af6f1bc9567b4e2ee4405d9cf8ce3e3ecc0
prerequisite-patch-id: e8134f9708010d4c679db257f022acb26ce2e9e7
prerequisite-patch-id: 655903bf9a0a260a2710ff18d812d3c979eedab4
prerequisite-patch-id: 209e8372941a405f50de769d92cde6d3550f4e01
prerequisite-patch-id: 2a204da753bafd464e1751392b63120480f3273f
prerequisite-patch-id: 58c2642c2468fa75e5f2132e09a3ced3b55d4adb
prerequisite-patch-id: 380faeb44cb4b192271235ed6a08ce3727ee8851
prerequisite-patch-id: 9efcb5ec489431979c45da11452c9c1765e70997
prerequisite-patch-id: c210d79f7d6bf76aaaa1142f7baf262aca8289fb
prerequisite-patch-id: 7c885da1b72b8efe9bff3af120fb14496a0327e9
prerequisite-patch-id: ee8eebffdd3caeb4ed21c16375f6b6cb11370fab
prerequisite-patch-id: 5d4289478982e6aa6c96b12ba051c4dc2eaf172f
prerequisite-patch-id: fdd0adb45d355662f5e0705e5bd42f41b5db68cd
prerequisite-patch-id: ddfb77436009ac891520f244f6628a57055a1bb3
prerequisite-patch-id: 8a8e7fcea2aed0893668fec395e71b37a0a8ae76
prerequisite-patch-id: b2f5eff9ffefff9feaede54c74a9fa78c03a3124
prerequisite-patch-id: 5e4dc08b9eede222f1dbc697cc030cb07fb4a6bc
prerequisite-patch-id: e932978d746d3bffdedbef47410049bab51c02a9
prerequisite-patch-id: 3dc5c92615aa215dcba8755659b6fb66b4663eb8
prerequisite-patch-id: c3ad370d5b3a3501edbaf3a36cc6cdedb5491b69
prerequisite-patch-id: b69fed0ac3c13bdfccc42f17ec7979c31d6b4866
prerequisite-patch-id: 2680125abbc0415771369c1755987cb7f6edf1ba
prerequisite-patch-id: 3e883d51ce7a70c57cc081ddf28be2636a347b47
prerequisite-patch-id: 1fd1fe22f8fb8cc1bb89211fe48727cf1f5cebce
prerequisite-patch-id: 602326650213640640f1e20a44a7d61e01dd9d57
prerequisite-patch-id: 8cbd2db8da9f956a7e5c41f3136c2ddbec59825f
prerequisite-patch-id: 99912d0bd174abefd133a01fd3cea9847d9bb3a4
prerequisite-patch-id: 7d576356820309ef320ae3420918546123aa6bfc
prerequisite-patch-id: 6ba2744f009ca7fbe05fc171b431907760861c24
prerequisite-patch-id: 28bf3b7eb553e9838a988bec26f021a67436d37f
prerequisite-patch-id: 6098f85b57866459e3f1c910985f4017a0b02480
prerequisite-patch-id: 583606866f8aae3e5429271abf352bdfe80eb6af
prerequisite-patch-id: 8345e72ae5ba8de92d422980143c11f4527e9517
prerequisite-patch-id: e5e47df8f9e416bdee9f8c7dddd55ea132d295f2
prerequisite-patch-id: 8310631a5a1f522d310e8d11645e49840713ab54
prerequisite-patch-id: a8f0c66891e2d9a619a2fe9662186645489654dc
prerequisite-patch-id: f75eef43b5cbe151cc4e1d6f361ef41597a82582
prerequisite-patch-id: da421d8737bd1fd84a3c954ac166a57d8ef00cea
prerequisite-patch-id: 3c7a7c8ff073984e6bd1d329e6c59685334f2703
prerequisite-patch-id: 2faecdc38e12ef87aa7a1d5aa063cc67b2c4c72f
prerequisite-patch-id: df7bf83ab2de05b0e38935f8d9e481f6fd1aef02
prerequisite-patch-id: 5179339320cdc5381beefd2733fe961185bba9bc
prerequisite-patch-id: 8176015aa4bd977a36116673ea2921d5e659c9df
prerequisite-patch-id: 9da6d54591d5c0db98070d62a55dee95e8245cfc
prerequisite-patch-id: ed025417c1b221930a9d9defd4caf72ed9007663
prerequisite-patch-id: cc44ec1f5cf026f560f35d60e9b314e8738f6b52
prerequisite-patch-id: aeb3c2f38dd8d3e2561a915ae046d5c1ca9b6fe8
prerequisite-patch-id: 15591002611d855b070236366d885690f15dd70d
prerequisite-patch-id: 40e60acb3fe63fb81249251538ad9e150c137384
prerequisite-patch-id: 521c7e7d8e1a4eae568410ac74e008b75675f5e7
prerequisite-patch-id: 020043b86caebb6a8ef08b83e9243c5a870eab6c
prerequisite-patch-id: 719e285f54f8bc3fa3869e2ebd444312275b0970
prerequisite-patch-id: 1c1ded1281059aae3b2a87a117b9a7afc697d132
prerequisite-patch-id: 54a23de4e88f7a4abeb3ea205b30e5ede017a809
prerequisite-patch-id: ce5dd51a1562240c3a7b31201da62a5bd2d2a48f
prerequisite-patch-id: 09d082023004a7b90621a2db3cf9732facbf211e
prerequisite-patch-id: 4db42306fbcb07e9fd2eb1a73f64fb19bf2b141c
prerequisite-patch-id: d3f04d3510049bedf8eec82c2e6adfd01cd1f460
prerequisite-patch-id: 4fcd5ea808977e11aa4ef85e75baa8c13103800e
prerequisite-patch-id: 8b27be15d44641eedc4b0daa097c750b00286cb5
prerequisite-patch-id: be81db0e4f0528ef028a50e861f9350e964ecb70
prerequisite-patch-id: 9455eed728beca427378866051d80d8e8f55a679
prerequisite-patch-id: 26b15a5819a92b0481b900175ff22a31608a9de5
prerequisite-patch-id: 79fbfdd0ca009249978c3c349e66df957ed140a0
prerequisite-patch-id: 98723c87b69e65f8602b6450b5e037486a78eb90
prerequisite-patch-id: fd9ad21628d14109f863fcb31f3a8bf31cf08370
prerequisite-patch-id: 08bc16eeba34a85020885a8a83c50017d7a1aa80
prerequisite-patch-id: 257cbc08f24a92b2734a3ec0f044bedde8e81b00
prerequisite-patch-id: 54aa12783bac8524b468e961b89157cbd64cb1e1
prerequisite-patch-id: 9b1f83c6c3141da339965ad96c17e5d31b98e5a6
prerequisite-patch-id: 0a36c69593ad7455cfc4f23abd273753f07f2d34
prerequisite-patch-id: c42c70c43731fc0ec612c7feb5eebec6e667fff2
prerequisite-patch-id: 42e95b3e450413d2306c11cee88e3a8787843759
prerequisite-patch-id: dd579c9d4cfd06a62ff58c4180410efa2bc51f24
prerequisite-patch-id: ab25d3567d5d3e32f4a7b42c69317d1bf4d86e16
prerequisite-patch-id: 75ee7cdc29cd45f924889757a56e144ab5ea4455
prerequisite-patch-id: 93b12e5839fe895a081c9d3eb88f4e6233cb26ab
prerequisite-patch-id: 79820e6740c0c456efc1dfa273de04e495515a1c
prerequisite-patch-id: f518e06627c0d6055453ae279add55bce203806d
prerequisite-patch-id: 944933d170f17be13a43e50588e35826974c76cc
prerequisite-patch-id: b0097ee5639defbadfe69cc19bf172e790021fe7
prerequisite-patch-id: 8a197a241c2904e99dc49d6b80d52ba1128a368b
prerequisite-patch-id: 1380f7788e59064b639513a4b5a27480e4e34b6a
prerequisite-patch-id: e1d60ca759b4607804e991a3e61b579742696fef
prerequisite-patch-id: d406492379391d3bc1c0984b5e87ca0d2e0674ad

Best regards,
--  
Boris Brezillon <boris.brezillon@collabora.com>

[PATCH v4 01/18] drm/panthor: Disable reset work before unplug

[Boris Brezillon]

Unplug is supposed to be the end of the road, so we need to make sure
reset works won't execute while we're cleaning up everything as part
of the unplug, otherwise it would mess up the internal state.

In order to be able to call disable_work_sync() in the unplug
path, we need to defer the unplug triggered by the reset logic,
otherwise we would deadlock.

Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c | 21 ++++++++++++++++++++-
 drivers/gpu/drm/panthor/panthor_device.h |  3 +++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 0b25abebb803..7c55d0c45cfd 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -79,6 +79,9 @@ void panthor_device_unplug(struct panthor_device *ptdev)
 		return;
 	}
 
+	/* Make sure we're not interrupted by resets while we're unplugging. */
+	disable_work_sync(&ptdev->reset.work);
+
 	drm_WARN_ON(&ptdev->base, pm_runtime_get_sync(ptdev->base.dev) < 0);
 
 	/* Call drm_dev_unplug() so any access to HW blocks happening after
@@ -91,6 +94,13 @@ void panthor_device_unplug(struct panthor_device *ptdev)
 	 */
 	mutex_unlock(&ptdev->unplug.lock);
 
+	/* Unplug triggered by a device removal might race with the deferred
+	 * one queued by the reset work. The function covers this concurrent
+	 * unplug situation, but if we can disable the work before its
+	 * execution, that's still better.
+	 */
+	disable_work(&ptdev->unplug.work);
+
 	/* Now, try to cleanly shutdown the GPU before the device resources
 	 * get reclaimed.
 	 */
@@ -114,6 +124,13 @@ void panthor_device_unplug(struct panthor_device *ptdev)
 	complete_all(&ptdev->unplug.done);
 }
 
+static void panthor_device_unplug_work(struct work_struct *work)
+{
+	struct panthor_device *ptdev = container_of(work, struct panthor_device, unplug.work);
+
+	panthor_device_unplug(ptdev);
+}
+
 static void panthor_device_reset_cleanup(struct drm_device *ddev, void *data)
 {
 	struct panthor_device *ptdev = container_of(ddev, struct panthor_device, base);
@@ -148,8 +165,9 @@ static void panthor_device_reset_work(struct work_struct *work)
 	drm_dev_exit(cookie);
 
 	if (ret) {
-		panthor_device_unplug(ptdev);
+		disable_work(&ptdev->reset.work);
 		drm_err(&ptdev->base, "Failed to boot MCU after reset, making device unusable.");
+		queue_work(ptdev->reset.wq, &ptdev->unplug.work);
 	}
 }
 
@@ -206,6 +224,7 @@ int panthor_device_init(struct panthor_device *ptdev)
 	 */
 	*dummy_page_virt = 1;
 
+	INIT_WORK(&ptdev->unplug.work, panthor_device_unplug_work);
 	INIT_WORK(&ptdev->reset.work, panthor_device_reset_work);
 	disable_work(&ptdev->reset.work);
 	ptdev->reset.wq = alloc_ordered_workqueue("panthor-reset-wq", 0);
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index b55a3f9edd41..6529e01e838d 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -268,6 +268,9 @@ struct panthor_device {
 		 * operation is done.
 		 */
 		struct completion done;
+
+		/** @work: Unplug work. */
+		struct work_struct work;
 	} unplug;
 
 	/** @reset: Reset related fields. */

-- 
2.55.0

[PATCH v4 02/18] drm/panthor: Revisit the reset logic to avoid reset request loss

[Boris Brezillon]

disable/enable_work() provide a ready to use mechanism to temporarily
disable a work item, so use that instead of the complex state machinery
based on the PM state.

This also allows us to automate the reset resubmission in case a reset
was received while the work item was disabled.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c | 36 +++++++++++++++++++++++++-------
 drivers/gpu/drm/panthor/panthor_device.h |  3 +--
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 7c55d0c45cfd..2974f4bc0bb1 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -181,6 +181,15 @@ static void panthor_device_free_page(struct drm_device *ddev, void *data)
 	__free_page(data);
 }
 
+static void enable_resets(struct panthor_device *ptdev)
+{
+	/* If a reset has been queued while the work was
+	 * disabled, reschedule it.
+	 */
+	if (enable_work(&ptdev->reset.work) && atomic_read(&ptdev->reset.pending))
+		queue_work(ptdev->reset.wq, &ptdev->reset.work);
+}
+
 int panthor_device_init(struct panthor_device *ptdev)
 {
 	u32 *dummy_page_virt;
@@ -256,6 +265,13 @@ int panthor_device_init(struct panthor_device *ptdev)
 
 	ptdev->phys_addr = res->start;
 
+	/* panthor_device_resume() calls enable_resets(), so we need to disable
+	 * the reset.work manually before this gets called to keep things
+	 * balanced. We don't bother re-enabling the work if the resume fails,
+	 * because the whole initialization will fail in that case, and the work
+	 * will vanish.
+	 */
+	disable_work(&ptdev->reset.work);
 	ret = devm_pm_runtime_enable(ptdev->base.dev);
 	if (ret)
 		return ret;
@@ -305,9 +321,6 @@ int panthor_device_init(struct panthor_device *ptdev)
 
 	panthor_gem_init(ptdev);
 
-	/* Now that everything is initialized, we can enable the reset work. */
-	enable_work(&ptdev->reset.work);
-
 	/* ~3 frames */
 	pm_runtime_set_autosuspend_delay(ptdev->base.dev, 50);
 	pm_runtime_use_autosuspend(ptdev->base.dev);
@@ -316,6 +329,9 @@ int panthor_device_init(struct panthor_device *ptdev)
 	if (ret)
 		goto err_disable_autosuspend;
 
+	/* Now that everything is initialized, we can enable the reset work. */
+	enable_resets(ptdev);
+
 	pm_runtime_put_autosuspend(ptdev->base.dev);
 	return 0;
 
@@ -534,10 +550,8 @@ int panthor_device_resume(struct device *dev)
 		/* If there was a reset pending at the time we suspended the
 		 * device, we force a slow reset.
 		 */
-		if (atomic_read(&ptdev->reset.pending)) {
+		if (atomic_cmpxchg(&ptdev->reset.pending, 1, 0))
 			ptdev->reset.fast = false;
-			atomic_set(&ptdev->reset.pending, 0);
-		}
 
 		ret = panthor_device_resume_hw_components(ptdev);
 		if (ret && ptdev->reset.fast) {
@@ -565,6 +579,9 @@ int panthor_device_resume(struct device *dev)
 			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
 	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_ACTIVE);
 	mutex_unlock(&ptdev->pm.mmio_lock);
+
+	/* Now that everything is resumed, we can re-enable the reset work. */
+	enable_resets(ptdev);
 	return 0;
 
 err_suspend_devfreq:
@@ -604,10 +621,13 @@ int panthor_device_suspend(struct device *dev)
 			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
 	mutex_unlock(&ptdev->pm.mmio_lock);
 
+	/* Make sure we're not interrupted by resets after that point
+	 * until the GPU is resumed.
+	 */
+	disable_work_sync(&ptdev->reset.work);
+
 	if (panthor_device_is_initialized(ptdev) &&
 	    drm_dev_enter(&ptdev->base, &cookie)) {
-		cancel_work_sync(&ptdev->reset.work);
-
 		/* We prepare everything as if we were resetting the GPU.
 		 * The end of the reset will happen in the resume path though.
 		 */
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index 6529e01e838d..f55baa21b25e 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -389,8 +389,7 @@ void panthor_device_unplug(struct panthor_device *ptdev);
  */
 static inline void panthor_device_schedule_reset(struct panthor_device *ptdev)
 {
-	if (!atomic_cmpxchg(&ptdev->reset.pending, 0, 1) &&
-	    atomic_read(&ptdev->pm.state) == PANTHOR_DEVICE_PM_STATE_ACTIVE)
+	if (!atomic_cmpxchg(&ptdev->reset.pending, 0, 1))
 		queue_work(ptdev->reset.wq, &ptdev->reset.work);
 }
 

-- 
2.55.0

[PATCH v4 03/18] drm/panthor: Make panthor_device::pm::state non-atomic

[Boris Brezillon]

Now that the reset logic has been reworked to use disable/enable_work(),
there's no need for panthor_device::pm::state to be an atomic. It can
simply be accessed under the same lock we use to touch MMIO mappings.

While at it, rename the lock to make it clear it protects more than just
the MMIO logic, and transition locked sections to scoped_guard().

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c | 97 +++++++++++++++++---------------
 drivers/gpu/drm/panthor/panthor_device.h | 16 ++++--
 2 files changed, 63 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 2974f4bc0bb1..133e3895cd0a 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -147,8 +147,10 @@ static void panthor_device_reset_work(struct work_struct *work)
 	/* If the device is entering suspend, we don't reset. A slow reset will
 	 * be forced at resume time instead.
 	 */
-	if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_ACTIVE)
-		return;
+	scoped_guard(mutex, &ptdev->pm.lock) {
+		if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_ACTIVE)
+			return;
+	}
 
 	if (!drm_dev_enter(&ptdev->base, &cookie))
 		return;
@@ -204,7 +206,7 @@ int panthor_device_init(struct panthor_device *ptdev)
 	if (ret)
 		return ret;
 
-	ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.mmio_lock);
+	ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.lock);
 	if (ret)
 		return ret;
 
@@ -213,7 +215,7 @@ int panthor_device_init(struct panthor_device *ptdev)
 	INIT_LIST_HEAD(&ptdev->gems.node);
 #endif
 
-	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
+	ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
 	p = alloc_page(GFP_KERNEL | __GFP_ZERO);
 	if (!p)
 		return -ENOMEM;
@@ -432,40 +434,39 @@ static vm_fault_t panthor_mmio_vm_fault(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct panthor_device *ptdev = vma->vm_private_data;
-	u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT;
-	unsigned long pfn;
-	pgprot_t pgprot;
 	vm_fault_t ret;
-	bool active;
 	int cookie;
 
 	if (!drm_dev_enter(&ptdev->base, &cookie))
 		return VM_FAULT_SIGBUS;
 
-	mutex_lock(&ptdev->pm.mmio_lock);
-	active = atomic_read(&ptdev->pm.state) == PANTHOR_DEVICE_PM_STATE_ACTIVE;
+	scoped_guard(mutex, &ptdev->pm.lock) {
+		bool active = ptdev->pm.state == PANTHOR_DEVICE_PM_STATE_ACTIVE;
+		u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT;
+		unsigned long pfn;
+		pgprot_t pgprot;
 
-	switch (offset) {
-	case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET:
+		switch (offset) {
+		case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET:
+			if (active)
+				pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID);
+			else
+				pfn = page_to_pfn(ptdev->pm.dummy_latest_flush);
+			break;
+
+		default:
+			ret = VM_FAULT_SIGBUS;
+			goto out_dev_exit;
+		}
+
+		pgprot = vma->vm_page_prot;
 		if (active)
-			pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID);
-		else
-			pfn = page_to_pfn(ptdev->pm.dummy_latest_flush);
-		break;
+			pgprot = pgprot_noncached(pgprot);
 
-	default:
-		ret = VM_FAULT_SIGBUS;
-		goto out_unlock;
+		ret = vmf_insert_pfn_prot(vma, vmf->address, pfn, pgprot);
 	}
 
-	pgprot = vma->vm_page_prot;
-	if (active)
-		pgprot = pgprot_noncached(pgprot);
-
-	ret = vmf_insert_pfn_prot(vma, vmf->address, pfn, pgprot);
-
-out_unlock:
-	mutex_unlock(&ptdev->pm.mmio_lock);
+out_dev_exit:
 	drm_dev_exit(cookie);
 	return ret;
 }
@@ -526,10 +527,13 @@ int panthor_device_resume(struct device *dev)
 	struct panthor_device *ptdev = dev_get_drvdata(dev);
 	int ret, cookie;
 
-	if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_SUSPENDED)
-		return -EINVAL;
+	scoped_guard(mutex, &ptdev->pm.lock) {
+		if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_SUSPENDED)
+			return -EINVAL;
+
+		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_RESUMING;
+	}
 
-	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_RESUMING);
 
 	ret = clk_prepare_enable(ptdev->clks.core);
 	if (ret)
@@ -574,11 +578,11 @@ int panthor_device_resume(struct device *dev)
 	 * are removed and the real iomem mapping will be restored on next
 	 * access.
 	 */
-	mutex_lock(&ptdev->pm.mmio_lock);
+	mutex_lock(&ptdev->pm.lock);
 	unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
 			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
-	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_ACTIVE);
-	mutex_unlock(&ptdev->pm.mmio_lock);
+	ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_ACTIVE;
+	mutex_unlock(&ptdev->pm.lock);
 
 	/* Now that everything is resumed, we can re-enable the reset work. */
 	enable_resets(ptdev);
@@ -595,7 +599,9 @@ int panthor_device_resume(struct device *dev)
 	clk_disable_unprepare(ptdev->clks.core);
 
 err_set_suspended:
-	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
+	scoped_guard(mutex, &ptdev->pm.lock)
+		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
+
 	atomic_set(&ptdev->pm.recovery_needed, 1);
 	return ret;
 }
@@ -605,21 +611,21 @@ int panthor_device_suspend(struct device *dev)
 	struct panthor_device *ptdev = dev_get_drvdata(dev);
 	int cookie;
 
-	if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_ACTIVE)
-		return -EINVAL;
-
 	/* Clear all IOMEM mappings pointing to this device before we
 	 * shutdown the power-domain and clocks. Failing to do that results
 	 * in external aborts when the process accesses the iomem region.
 	 * We change the state and call unmap_mapping_range() with the
-	 * mmio_lock held to make sure the vm_fault handler won't set up
+	 * lock held to make sure the vm_fault handler won't set up
 	 * invalid mappings.
 	 */
-	mutex_lock(&ptdev->pm.mmio_lock);
-	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDING);
-	unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
-			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
-	mutex_unlock(&ptdev->pm.mmio_lock);
+	scoped_guard(mutex, &ptdev->pm.lock) {
+		if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_ACTIVE)
+			return -EINVAL;
+
+		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDING;
+		unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
+				    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
+	}
 
 	/* Make sure we're not interrupted by resets after that point
 	 * until the GPU is resumed.
@@ -644,6 +650,9 @@ int panthor_device_suspend(struct device *dev)
 	clk_disable_unprepare(ptdev->clks.coregroup);
 	clk_disable_unprepare(ptdev->clks.stacks);
 	clk_disable_unprepare(ptdev->clks.core);
-	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
+
+	scoped_guard(mutex, &ptdev->pm.lock)
+		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index f55baa21b25e..217eec811bdb 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -298,18 +298,22 @@ struct panthor_device {
 
 	/** @pm: Power management related data. */
 	struct {
-		/** @state: Power state. */
-		atomic_t state;
+		/**
+		 * @state: Power state.
+		 *
+		 * Must be accessed with the panthor_device::pm::lock held.
+		 */
+		enum panthor_device_pm_state state;
 
 		/**
-		 * @mmio_lock: Lock protecting MMIO userspace CPU mappings.
+		 * @lock: Lock protecting PM related fields.
 		 *
 		 * This is needed to ensure we map the dummy IO pages when
 		 * the device is being suspended, and the real IO pages when
-		 * the device is being resumed. We can't just do with the
-		 * state atomicity to deal with this race.
+		 * the device is being resumed. We can't just do with an
+		 * atomic state to deal with this race.
 		 */
-		struct mutex mmio_lock;
+		struct mutex lock;
 
 		/**
 		 * @dummy_latest_flush: Dummy LATEST_FLUSH page.

-- 
2.55.0

[PATCH v4 04/18] drm/panthor: Flush the cleanup_wq in the unplug path

[Boris Brezillon]

If we don't do that, we might face various UAFs, because the resource
referenced by these work items might be gone by the time they get
executed.

In each subcomponent making use of the panthor_cleanup_wq, we add a
flush_workqueue() at the end of the _unplug() function. Note that this
assumes no more work items from this subcomponent gets queued after
that point, which is not yet guaranteed, but this will be fixed in
upcoming changes.

Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
Fixes: 647810ec2476 ("drm/panthor: Add the MMU/VM logical block")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260625-panthor-signal-from-irq-v5-0-8836a74e0ef9@collabora.com?part=2
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c   | 6 ++++++
 drivers/gpu/drm/panthor/panthor_sched.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index d43ba1c7cd2a..1385ee28bee5 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -3367,6 +3367,12 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
 		}
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
+
+	/* Ensure any pending job cleanup work are executed before returning,
+	 * otherwise those might access objects that are gone if the work is
+	 * executed after other components are unplugged.
+	 */
+	flush_workqueue(panthor_cleanup_wq);
 }
 
 static void panthor_mmu_release_wq(struct drm_device *ddev, void *res)
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 42adfc76b9e1..a6d57dc1b43e 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -4049,6 +4049,12 @@ void panthor_sched_unplug(struct panthor_device *ptdev)
 		sched->pm.has_ref = false;
 	}
 	mutex_unlock(&sched->lock);
+
+	/* Ensure any pending group release work are executed before returning,
+	 * otherwise those might access objects that are gone if the work is
+	 * executed after other components are unplugged.
+	 */
+	flush_workqueue(panthor_cleanup_wq);
 }
 
 static void panthor_sched_fini(struct drm_device *ddev, void *res)

-- 
2.55.0

[PATCH v4 05/18] drm/panthor: Make the page table cache and cleanup workqueue device-local

[Boris Brezillon]

There's no clue that we'll ever have an SoC with multiple Mali GPUs,
and even then, the gains of sharing the PT caches and cleanup workqueue
remains unclear.

On the other hand, binding those resources to the device allows us to
detect leaks or UAF at drm_device removal time instead of when the
module is removed.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c | 17 ++++---
 drivers/gpu/drm/panthor/panthor_device.h |  8 ++++
 drivers/gpu/drm/panthor/panthor_drv.c    | 47 +------------------
 drivers/gpu/drm/panthor/panthor_mmu.c    | 78 +++++++++++++++-----------------
 drivers/gpu/drm/panthor/panthor_mmu.h    |  3 --
 drivers/gpu/drm/panthor/panthor_sched.c  |  4 +-
 6 files changed, 58 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 133e3895cd0a..08bb4d410941 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -131,12 +131,9 @@ static void panthor_device_unplug_work(struct work_struct *work)
 	panthor_device_unplug(ptdev);
 }
 
-static void panthor_device_reset_cleanup(struct drm_device *ddev, void *data)
+static void destroy_wq(struct drm_device *ddev, void *wq)
 {
-	struct panthor_device *ptdev = container_of(ddev, struct panthor_device, base);
-
-	disable_work_sync(&ptdev->reset.work);
-	destroy_workqueue(ptdev->reset.wq);
+	destroy_workqueue(wq);
 }
 
 static void panthor_device_reset_work(struct work_struct *work)
@@ -242,7 +239,15 @@ int panthor_device_init(struct panthor_device *ptdev)
 	if (!ptdev->reset.wq)
 		return -ENOMEM;
 
-	ret = drmm_add_action_or_reset(&ptdev->base, panthor_device_reset_cleanup, NULL);
+	ret = drmm_add_action_or_reset(&ptdev->base, destroy_wq, ptdev->reset.wq);
+	if (ret)
+		return ret;
+
+	ptdev->cleanup_wq =  alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0);
+	if (!ptdev->cleanup_wq)
+		return -ENOMEM;
+
+	ret = drmm_add_action_or_reset(&ptdev->base, destroy_wq, ptdev->cleanup_wq);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index 217eec811bdb..a7ea475fb407 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -327,6 +327,14 @@ struct panthor_device {
 		atomic_t recovery_needed;
 	} pm;
 
+	/**
+	 * @cleanup_wq: Workqueue used for cleanup operations.
+	 *
+	 * We create a dedicated workqueue so we can flush on unplug and
+	 * make sure all resources are freed before we finish the unplug.
+	 */
+	struct workqueue_struct *cleanup_wq;
+
 	/** @profile_mask: User-set profiling flags for job accounting. */
 	u32 profile_mask;
 
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 46a3080b0b20..67db6701699b 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1904,52 +1904,7 @@ static struct platform_driver panthor_driver = {
 		.dev_groups = panthor_groups,
 	},
 };
-
-/*
- * Workqueue used to cleanup stuff.
- *
- * We create a dedicated workqueue so we can drain on unplug and
- * make sure all resources are freed before the module is unloaded.
- */
-struct workqueue_struct *panthor_cleanup_wq;
-
-static int __init panthor_init(void)
-{
-	int ret;
-
-	ret = panthor_mmu_pt_cache_init();
-	if (ret)
-		return ret;
-
-	panthor_cleanup_wq = alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0);
-	if (!panthor_cleanup_wq) {
-		pr_err("panthor: Failed to allocate the workqueues");
-		ret = -ENOMEM;
-		goto err_mmu_pt_cache_fini;
-	}
-
-	ret = platform_driver_register(&panthor_driver);
-	if (ret)
-		goto err_destroy_cleanup_wq;
-
-	return 0;
-
-err_destroy_cleanup_wq:
-	destroy_workqueue(panthor_cleanup_wq);
-
-err_mmu_pt_cache_fini:
-	panthor_mmu_pt_cache_fini();
-	return ret;
-}
-module_init(panthor_init);
-
-static void __exit panthor_exit(void)
-{
-	platform_driver_unregister(&panthor_driver);
-	destroy_workqueue(panthor_cleanup_wq);
-	panthor_mmu_pt_cache_fini();
-}
-module_exit(panthor_exit);
+module_platform_driver(panthor_driver);
 
 MODULE_AUTHOR("Panthor Project Developers");
 MODULE_DESCRIPTION("Panthor DRM Driver");
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 1385ee28bee5..b4e41556247a 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -61,6 +61,16 @@ struct panthor_mmu {
 	/** @irq: The MMU irq. */
 	struct panthor_irq irq;
 
+	/**
+	 * @pt_cache: Cache used to allocate MMU page tables.
+	 *
+	 * The pre-allocation pattern forces us to over-allocate to plan for
+	 * the worst case scenario, and return the pages we didn't use.
+	 *
+	 * Having a kmem_cache allows us to speed allocations.
+	 */
+	struct kmem_cache *pt_cache;
+
 	/**
 	 * @as: Address space related fields.
 	 *
@@ -440,16 +450,6 @@ struct panthor_vm_bind_job {
 	struct panthor_vm_op_ctx ctx;
 };
 
-/*
- * @pt_cache: Cache used to allocate MMU page tables.
- *
- * The pre-allocation pattern forces us to over-allocate to plan for
- * the worst case scenario, and return the pages we didn't use.
- *
- * Having a kmem_cache allows us to speed allocations.
- */
-static struct kmem_cache *pt_cache;
-
 /**
  * alloc_pt() - Custom page table allocator
  * @cookie: Cookie passed at page table allocation time.
@@ -530,7 +530,7 @@ static void free_pt(void *cookie, void *data, size_t size)
 		return;
 
 	/* Return the page to the pt_cache. */
-	kmem_cache_free(pt_cache, data);
+	kmem_cache_free(vm->ptdev->mmu->pt_cache, data);
 }
 
 static int wait_ready(struct panthor_device *ptdev, u32 as_nr)
@@ -1179,7 +1179,8 @@ static void panthor_vm_cleanup_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 				     !op_ctx->map.bo;
 
 	if (remaining_pt_count) {
-		kmem_cache_free_bulk(pt_cache, remaining_pt_count,
+		kmem_cache_free_bulk(vm->ptdev->mmu->pt_cache,
+				     remaining_pt_count,
 				     op_ctx->rsvd_page_tables.pages +
 				     op_ctx->rsvd_page_tables.ptr);
 	}
@@ -1279,7 +1280,8 @@ static void panthor_vm_init_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	op_ctx->va.addr = va;
 }
 
-static int panthor_vm_op_ctx_prealloc_pts(struct panthor_vm_op_ctx *op_ctx)
+static int panthor_vm_op_ctx_prealloc_pts(struct panthor_device *ptdev,
+					  struct panthor_vm_op_ctx *op_ctx)
 {
 	u64 size = op_ctx->va.range;
 	u64 va = op_ctx->va.addr;
@@ -1298,7 +1300,7 @@ static int panthor_vm_op_ctx_prealloc_pts(struct panthor_vm_op_ctx *op_ctx)
 	if (!op_ctx->rsvd_page_tables.pages)
 		return -ENOMEM;
 
-	if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
+	if (!kmem_cache_alloc_bulk(ptdev->mmu->pt_cache, GFP_KERNEL, pt_count,
 				   op_ctx->rsvd_page_tables.pages)) {
 		op_ctx->rsvd_page_tables.count = 0;
 		return -ENOMEM;
@@ -1385,7 +1387,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	op_ctx->map.vm_bo = drm_gpuvm_bo_obtain_prealloc(preallocated_vm_bo);
 	op_ctx->map.bo_offset = op->bo_offset;
 
-	ret = panthor_vm_op_ctx_prealloc_pts(op_ctx);
+	ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx);
 	if (ret)
 		goto err_cleanup;
 
@@ -1444,8 +1446,9 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 			goto err_cleanup;
 		}
 
-		if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
-				op_ctx->rsvd_page_tables.pages)) {
+		if (!kmem_cache_alloc_bulk(vm->ptdev->mmu->pt_cache,
+					   GFP_KERNEL, pt_count,
+					   op_ctx->rsvd_page_tables.pages)) {
 			ret = -ENOMEM;
 			goto err_cleanup;
 		}
@@ -2580,7 +2583,7 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
 	bool found = false;
 	int ret;
 
-	ret = panthor_vm_op_ctx_prealloc_pts(op_ctx);
+	ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx);
 	if (ret)
 		goto out_cleanup;
 
@@ -2821,7 +2824,7 @@ panthor_vm_bind_free_job(struct drm_sched_job *sched_job)
 	/* Do the heavy cleanups asynchronously, so we're out of the
 	 * dma-signaling path and can acquire dma-resv locks safely.
 	 */
-	queue_work(panthor_cleanup_wq, &job->cleanup_op_ctx_work);
+	queue_work(job->vm->ptdev->cleanup_wq, &job->cleanup_op_ctx_work);
 }
 
 static enum drm_gpu_sched_stat
@@ -3372,7 +3375,7 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
 	 * otherwise those might access objects that are gone if the work is
 	 * executed after other components are unplugged.
 	 */
-	flush_workqueue(panthor_cleanup_wq);
+	flush_workqueue(ptdev->cleanup_wq);
 }
 
 static void panthor_mmu_release_wq(struct drm_device *ddev, void *res)
@@ -3385,6 +3388,11 @@ static void panthor_mmu_info_init(struct panthor_device *ptdev)
 	ptdev->mmu_info.page_size_bitmap = SZ_4K | SZ_2M;
 }
 
+static void free_pt_cache(struct drm_device *, void *pt_cache)
+{
+	kmem_cache_destroy(pt_cache);
+}
+
 /**
  * panthor_mmu_init() - Initialize the MMU logic.
  * @ptdev: Device.
@@ -3414,6 +3422,14 @@ int panthor_mmu_init(struct panthor_device *ptdev)
 	if (ret)
 		return ret;
 
+	mmu->pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, NULL);
+	if (!mmu->pt_cache)
+		return -ENOMEM;
+
+	ret = drmm_add_action_or_reset(&ptdev->base, free_pt_cache, mmu->pt_cache);
+	if (ret)
+		return ret;
+
 	mmu->iomem = ptdev->iomem + MMU_AS_BASE;
 	ptdev->mmu = mmu;
 
@@ -3498,25 +3514,3 @@ void panthor_mmu_debugfs_init(struct drm_minor *minor)
 				 minor->debugfs_root, minor);
 }
 #endif /* CONFIG_DEBUG_FS */
-
-/**
- * panthor_mmu_pt_cache_init() - Initialize the page table cache.
- *
- * Return: 0 on success, a negative error code otherwise.
- */
-int panthor_mmu_pt_cache_init(void)
-{
-	pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, NULL);
-	if (!pt_cache)
-		return -ENOMEM;
-
-	return 0;
-}
-
-/**
- * panthor_mmu_pt_cache_fini() - Destroy the page table cache.
- */
-void panthor_mmu_pt_cache_fini(void)
-{
-	kmem_cache_destroy(pt_cache);
-}
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
index 3522fbbce369..de6b4ee4e41a 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.h
+++ b/drivers/gpu/drm/panthor/panthor_mmu.h
@@ -104,9 +104,6 @@ void panthor_vm_update_resvs(struct panthor_vm *vm, struct drm_exec *exec,
 			     enum dma_resv_usage private_usage,
 			     enum dma_resv_usage extobj_usage);
 
-int panthor_mmu_pt_cache_init(void);
-void panthor_mmu_pt_cache_fini(void);
-
 #ifdef CONFIG_DEBUG_FS
 void panthor_mmu_debugfs_init(struct drm_minor *minor);
 #endif
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index a6d57dc1b43e..4ea16b40d6b9 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -972,7 +972,7 @@ static void group_release(struct kref *kref)
 	drm_WARN_ON(&ptdev->base, !list_empty(&group->run_node));
 	drm_WARN_ON(&ptdev->base, !list_empty(&group->wait_node));
 
-	queue_work(panthor_cleanup_wq, &group->release_work);
+	queue_work(ptdev->cleanup_wq, &group->release_work);
 }
 
 static void group_put(struct panthor_group *group)
@@ -4054,7 +4054,7 @@ void panthor_sched_unplug(struct panthor_device *ptdev)
 	 * otherwise those might access objects that are gone if the work is
 	 * executed after other components are unplugged.
 	 */
-	flush_workqueue(panthor_cleanup_wq);
+	flush_workqueue(ptdev->cleanup_wq);
 }
 
 static void panthor_sched_fini(struct drm_device *ddev, void *res)

-- 
2.55.0

[PATCH v4 06/18] drm/panthor: Drop unused vm argument passed to panthor_vm_prepare_sync_only_op_ctx()

[Boris Brezillon]

No need to pass a panthor_vm around if it's unused.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index b4e41556247a..44a44602b8ba 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1462,8 +1462,8 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	return ret;
 }
 
-static void panthor_vm_prepare_sync_only_op_ctx(struct panthor_vm_op_ctx *op_ctx,
-						struct panthor_vm *vm)
+static void
+panthor_vm_prepare_sync_only_op_ctx(struct panthor_vm_op_ctx *op_ctx)
 {
 	memset(op_ctx, 0, sizeof(*op_ctx));
 	op_ctx->flags = DRM_PANTHOR_VM_BIND_OP_TYPE_SYNC_ONLY;
@@ -3031,7 +3031,7 @@ panthor_vm_bind_prepare_op_ctx(struct drm_file *file,
 		if (!op->syncs.count)
 			return -EINVAL;
 
-		panthor_vm_prepare_sync_only_op_ctx(op_ctx, vm);
+		panthor_vm_prepare_sync_only_op_ctx(op_ctx);
 		return 0;
 
 	default:

-- 
2.55.0

[PATCH v4 07/18] drm/panthor: Move the debugfs initialization to panthor_device.c

[Boris Brezillon]

Those are per-device debugfs-files, so it makes sense to have the
initialization logic in panthor_device.c.

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c |  9 +++++++++
 drivers/gpu/drm/panthor/panthor_device.h |  4 ++++
 drivers/gpu/drm/panthor/panthor_drv.c    | 10 +---------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 08bb4d410941..19d3669f5ec7 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -13,6 +13,7 @@
 #include <linux/reset.h>
 
 #include <drm/drm_drv.h>
+#include <drm/drm_file.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_print.h>
 
@@ -661,3 +662,11 @@ int panthor_device_suspend(struct device *dev)
 
 	return 0;
 }
+
+#ifdef CONFIG_DEBUG_FS
+void panthor_device_debugfs_init(struct drm_minor *minor)
+{
+	panthor_mmu_debugfs_init(minor);
+	panthor_gem_debugfs_init(minor);
+}
+#endif
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index a7ea475fb407..e12049961912 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -421,6 +421,10 @@ int panthor_device_mmap_io(struct panthor_device *ptdev,
 int panthor_device_resume(struct device *dev);
 int panthor_device_suspend(struct device *dev);
 
+#ifdef CONFIG_DEBUG_FS
+void panthor_device_debugfs_init(struct drm_minor *minor);
+#endif
+
 static inline int panthor_device_resume_and_get(struct panthor_device *ptdev)
 {
 	int ret = pm_runtime_resume_and_get(ptdev->base.dev);
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 67db6701699b..3993e1a81495 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1764,14 +1764,6 @@ static const struct file_operations panthor_drm_driver_fops = {
 	.fop_flags = FOP_UNSIGNED_OFFSET,
 };
 
-#ifdef CONFIG_DEBUG_FS
-static void panthor_debugfs_init(struct drm_minor *minor)
-{
-	panthor_mmu_debugfs_init(minor);
-	panthor_gem_debugfs_init(minor);
-}
-#endif
-
 /*
  * PanCSF driver version:
  * - 1.0 - initial interface
@@ -1807,7 +1799,7 @@ static const struct drm_driver panthor_drm_driver = {
 	.gem_prime_import_sg_table = panthor_gem_prime_import_sg_table,
 	.gem_prime_import = panthor_gem_prime_import,
 #ifdef CONFIG_DEBUG_FS
-	.debugfs_init = panthor_debugfs_init,
+	.debugfs_init = panthor_device_debugfs_init,
 #endif
 };
 

-- 
2.55.0

[PATCH v4 08/18] drm/panthor: Split panthor_vm

[Boris Brezillon]

The way things are currently defined makes the cleanup procedure harder
because the panthor_vm object cleanup happens after drm_gpuvm_fini() has
been called, and sometimes we need a drm_gpuvm to undo things.
This has been worked around by things like the panthor_vm_unmap_range()
call in panthor_vm_destroy(), but there are still situations where this
is problematic, like the show_each_vm() where we walk a list of VM and
call drm_debugfs_gpuva_info() on each, with the risk of hitting an object
that had drm_gpuvm_fini() called on it already.

There's more of these tricky situations to come when we get to making
the unplug logic more robust, so let's address the problem ahead of it
and split the panthor_vm object in two:

- panthor_as: this is the object embedding drm_gpuvm and more
  generally dealing with page table updates/residency
- panthor_vm: this is the user-visible object wrapping around
  panthor_as. Among other things, it contains the scheduler for
  the bind queue and the drm_mm tree for kernel BO allocation.
  This object owns a drm_gpuvm ref.

With this in place, we can do the cleanup steps that need a valid
drm_gpuvm object in panthor_vm_release(), and the rest is cleaned up
in panthor_as_free().

Note that there's a bunch of s/as[_nr]/slot/ variable/argument renames
to clear the confusion between the AS slot number and the newly
introduced panthor_as object.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 1085 ++++++++++++++++++---------------
 1 file changed, 580 insertions(+), 505 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 44a44602b8ba..933cb820926d 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -41,14 +41,14 @@
 
 #define MAX_AS_SLOTS			32
 
-struct panthor_vm;
+struct panthor_as;
 
 /**
  * struct panthor_as_slot - Address space slot
  */
 struct panthor_as_slot {
-	/** @vm: VM bound to this slot. NULL is no VM is bound. */
-	struct panthor_vm *vm;
+	/** @as: AS bound to this slot. NULL if no AS is bound. */
+	struct panthor_as *as;
 };
 
 /**
@@ -87,19 +87,19 @@ struct panthor_mmu {
 		/** @as.faulty_mask: Bitmask encoding the faulty slots. */
 		unsigned long faulty_mask;
 
-		/** @as.slots: VMs currently bound to the AS slots. */
+		/** @as.slots: AS currently bound to the AS slots. */
 		struct panthor_as_slot slots[MAX_AS_SLOTS];
 
 		/**
-		 * @as.lru_list: List of least recently used VMs.
+		 * @as.lru_list: List of least recently used AS.
 		 *
-		 * We use this list to pick a VM to evict when all slots are
+		 * We use this list to pick an AS to evict when all slots are
 		 * used.
 		 *
-		 * There should be no more active VMs than there are AS slots,
-		 * so this LRU is just here to keep VMs bound until there's
-		 * a need to release a slot, thus avoid unnecessary TLB/cache
-		 * flushes.
+		 * There should be no more active AS than there are AS slots,
+		 * so this LRU is just here to keep page tables bound until
+		 * there's a need to release a slot, thus avoiding unnecessary
+		 * TLB/cache flushes.
 		 */
 		struct list_head lru_list;
 	} as;
@@ -163,9 +163,9 @@ struct panthor_vma {
 };
 
 /**
- * struct panthor_vm_op_ctx - VM operation context
+ * struct panthor_as_op_ctx - AS operation context
  *
- * With VM operations potentially taking place in a dma-signaling path, we
+ * With AS operations potentially taking place in a dma-signaling path, we
  * need to make sure everything that might require resource allocation is
  * pre-allocated upfront. This is what this operation context is far.
  *
@@ -173,7 +173,7 @@ struct panthor_vma {
  * asynchronously, and let the VM_BIND scheduler process the next VM_BIND
  * request.
  */
-struct panthor_vm_op_ctx {
+struct panthor_as_op_ctx {
 	/** @rsvd_page_tables: Pages reserved for the MMU page table update. */
 	struct {
 		/** @rsvd_page_tables.count: Number of pages reserved. */
@@ -225,13 +225,130 @@ struct panthor_vm_op_ctx {
 	} map;
 };
 
+/**
+ * struct panthor_as - Used to managed a GPU address space.
+ */
+struct panthor_as {
+	/**
+	 * @base: Inherit from drm_gpuvm.
+	 *
+	 * We delegate all the VA management to the common drm_gpuvm framework
+	 * and only implement hooks to update the MMU page table.
+	 */
+	struct drm_gpuvm base;
+
+	/** @memattr: Value to program to the AS_MEMATTR register. */
+	u64 memattr;
+
+	/** @pt: Page table fields. */
+	struct {
+		/** @pt.ops: Page table ops. */
+		struct io_pgtable_ops *ops;
+
+		/** @pt.root: Page table root. */
+		void *root;
+	} pt;
+
+	/**
+	 * @op_lock: Lock used to serialize operations on the AS.
+	 *
+	 * The serialization of jobs queued to the VM_BIND queue is already
+	 * taken care of by drm_sched, but we need to serialize synchronous
+	 * and asynchronous VM_BIND request. This is what this lock is for.
+	 */
+	struct mutex op_lock;
+
+	/**
+	 * @op_ctx: The context attached to the currently executing operation.
+	 *
+	 * NULL when no operation is in progress.
+	 */
+	struct panthor_as_op_ctx *op_ctx;
+
+	/** @active_cnt: Number of active users of this address space. */
+	refcount_t active_cnt;
+
+	/** @hw_slot: Hardware slot related fields. */
+	struct {
+		/**
+		 * @hw_slot.id: ID of the slot this AS is bound to.
+		 *
+		 * A value of -1 means the AS is inactive/not bound.
+		 */
+		int id;
+
+		/**
+		 * @hw_slot.lru_node: Used to insert the AS in panthor_mmu::as::lru_list.
+		 *
+		 * Active ASs should not be inserted in the LRU list.
+		 */
+		struct list_head lru_node;
+	} hw_slot;
+
+	/**
+	 * @unusable: True if the AS has turned unusable because something
+	 * bad happened during an asynchronous request.
+	 *
+	 * We don't try to recover from such failures, because this implies
+	 * informing userspace about the specific operation that failed, and
+	 * hoping the userspace driver can replay things from there. This all
+	 * sounds very complicated for little gain.
+	 *
+	 * Instead, we should just flag the AS as unusable, and fail any
+	 * further request targeting this AS.
+	 *
+	 * We also provide a way to query an AS state, so userspace can
+	 * destroy it and create a new one.
+	 *
+	 * As an analogy, this would be mapped to a VK_ERROR_DEVICE_LOST
+	 * situation, where the logical device needs to be re-created.
+	 */
+	bool unusable;
+
+	/**
+	 * @unhandled_fault: Unhandled fault happened.
+	 *
+	 * This should be reported to the scheduler, and the queue/group be
+	 * flagged as faulty as a result.
+	 */
+	bool unhandled_fault;
+
+	/** @locked_region: Information about the currently locked region currently. */
+	struct {
+		/** @locked_region.start: Start of the locked region. */
+		u64 start;
+
+		/** @locked_region.size: Size of the locked region. */
+		u64 size;
+	} locked_region;
+
+	/** @reclaim: Fields related to BO reclaim. */
+	struct {
+		/** @reclaim.lru: LRU of BOs that are only mapped to this AS. */
+		struct drm_gem_lru lru;
+
+		/**
+		 * @reclaim.lru_node: Node used to insert the AS in
+		 * panthor_device::reclaim::vms.
+		 */
+		struct list_head lru_node;
+
+		/**
+		 * @skip: Skip reclaim on this AS.
+		 *
+		 * This is set to true when the AS is about to be
+		 * released to ensure this VM won't participate to reclaim.
+		 * The VM should go away pretty soon and release its
+		 * resources anyway.
+		 */
+		bool skip;
+	} reclaim;
+};
+
 /**
  * struct panthor_vm - VM object
  *
  * A VM is an object representing a GPU (or MCU) virtual address space.
- * It embeds the MMU page table for this address space, a tree containing
- * all the virtual mappings of GEM objects, and other things needed to manage
- * the VM.
  *
  * Except for the MCU VM, which is managed by the kernel, all other VMs are
  * created by userspace and mostly managed by userspace, using the
@@ -243,13 +360,11 @@ struct panthor_vm_op_ctx {
  * by default).
  */
 struct panthor_vm {
-	/**
-	 * @base: Inherit from drm_gpuvm.
-	 *
-	 * We delegate all the VA management to the common drm_gpuvm framework
-	 * and only implement hooks to update the MMU page table.
-	 */
-	struct drm_gpuvm base;
+	/** @refcount: VM refcount. */
+	struct kref refcount;
+
+	/** @as: VM address space. */
+	struct panthor_as *as;
 
 	/**
 	 * @sched: Scheduler used for asynchronous VM_BIND request.
@@ -266,34 +381,6 @@ struct panthor_vm {
 	 */
 	struct drm_sched_entity entity;
 
-	/** @ptdev: Device. */
-	struct panthor_device *ptdev;
-
-	/** @memattr: Value to program to the AS_MEMATTR register. */
-	u64 memattr;
-
-	/** @pgtbl_ops: Page table operations. */
-	struct io_pgtable_ops *pgtbl_ops;
-
-	/** @root_page_table: Stores the root page table pointer. */
-	void *root_page_table;
-
-	/**
-	 * @op_lock: Lock used to serialize operations on a VM.
-	 *
-	 * The serialization of jobs queued to the VM_BIND queue is already
-	 * taken care of by drm_sched, but we need to serialize synchronous
-	 * and asynchronous VM_BIND request. This is what this lock is for.
-	 */
-	struct mutex op_lock;
-
-	/**
-	 * @op_ctx: The context attached to the currently executing VM operation.
-	 *
-	 * NULL when no operation is in progress.
-	 */
-	struct panthor_vm_op_ctx *op_ctx;
-
 	/**
 	 * @mm: Memory management object representing the auto-VA/kernel-VA.
 	 *
@@ -323,26 +410,6 @@ struct panthor_vm {
 	/** @user_va_range: Upper boundary of VAs VM users can map objects against. */
 	u64 user_va_range;
 
-	/** @as: Address space related fields. */
-	struct {
-		/**
-		 * @as.id: ID of the address space this VM is bound to.
-		 *
-		 * A value of -1 means the VM is inactive/not bound.
-		 */
-		int id;
-
-		/** @as.active_cnt: Number of active users of this VM. */
-		refcount_t active_cnt;
-
-		/**
-		 * @as.lru_node: Used to instead the VM in the panthor_mmu::as::lru_list.
-		 *
-		 * Active VMs should not be inserted in the LRU list.
-		 */
-		struct list_head lru_node;
-	} as;
-
 	/**
 	 * @heaps: Tiler heap related fields.
 	 */
@@ -371,55 +438,6 @@ struct panthor_vm {
 	 */
 	bool destroyed;
 
-	/**
-	 * @unusable: True if the VM has turned unusable because something
-	 * bad happened during an asynchronous request.
-	 *
-	 * We don't try to recover from such failures, because this implies
-	 * informing userspace about the specific operation that failed, and
-	 * hoping the userspace driver can replay things from there. This all
-	 * sounds very complicated for little gain.
-	 *
-	 * Instead, we should just flag the VM as unusable, and fail any
-	 * further request targeting this VM.
-	 *
-	 * We also provide a way to query a VM state, so userspace can destroy
-	 * it and create a new one.
-	 *
-	 * As an analogy, this would be mapped to a VK_ERROR_DEVICE_LOST
-	 * situation, where the logical device needs to be re-created.
-	 */
-	bool unusable;
-
-	/**
-	 * @unhandled_fault: Unhandled fault happened.
-	 *
-	 * This should be reported to the scheduler, and the queue/group be
-	 * flagged as faulty as a result.
-	 */
-	bool unhandled_fault;
-
-	/** @locked_region: Information about the currently locked region currently. */
-	struct {
-		/** @locked_region.start: Start of the locked region. */
-		u64 start;
-
-		/** @locked_region.size: Size of the locked region. */
-		u64 size;
-	} locked_region;
-
-	/** @reclaim: Fields related to BO reclaim. */
-	struct {
-		/** @reclaim.lru: LRU of BOs that are only mapped to this VM. */
-		struct drm_gem_lru lru;
-
-		/**
-		 * @reclaim.lru_node: Node used to insert the VM in
-		 * panthor_device::reclaim::vms.
-		 */
-		struct list_head lru_node;
-	} reclaim;
-
 	/**
 	 * @dummy: Dummy object used for sparse mappings.
 	 *
@@ -447,7 +465,7 @@ struct panthor_vm_bind_job {
 	struct panthor_vm *vm;
 
 	/** @ctx: Operation context. */
-	struct panthor_vm_op_ctx ctx;
+	struct panthor_as_op_ctx ctx;
 };
 
 /**
@@ -466,36 +484,37 @@ struct panthor_vm_bind_job {
  */
 static void *alloc_pt(void *cookie, size_t size, gfp_t gfp)
 {
-	struct panthor_vm *vm = cookie;
+	struct panthor_as *as = cookie;
+	struct panthor_as_op_ctx *op_ctx = as->op_ctx;
+	struct drm_device *ddev = as->base.drm;
 	void *page;
 
 	/* Allocation of the root page table happening during init. */
-	if (unlikely(!vm->root_page_table)) {
+	if (unlikely(!as->pt.root)) {
+		struct device *dev = drm_dev_dma_dev(ddev);
 		struct page *p;
 
-		drm_WARN_ON(&vm->ptdev->base, vm->op_ctx);
-		p = alloc_pages_node(dev_to_node(vm->ptdev->base.dev),
-				     gfp | __GFP_ZERO, get_order(size));
+		drm_WARN_ON(ddev, op_ctx);
+		p = alloc_pages_node(dev_to_node(dev), gfp | __GFP_ZERO, get_order(size));
 		page = p ? page_address(p) : NULL;
-		vm->root_page_table = page;
+		as->pt.root = page;
 		return page;
 	}
 
 	/* We're not supposed to have anything bigger than 4k here, because we picked a
 	 * 4k granule size at init time.
 	 */
-	if (drm_WARN_ON(&vm->ptdev->base, size != SZ_4K))
+	if (drm_WARN_ON(ddev, size != SZ_4K))
 		return NULL;
 
 	/* We must have some op_ctx attached to the VM and it must have at least one
 	 * free page.
 	 */
-	if (drm_WARN_ON(&vm->ptdev->base, !vm->op_ctx) ||
-	    drm_WARN_ON(&vm->ptdev->base,
-			vm->op_ctx->rsvd_page_tables.ptr >= vm->op_ctx->rsvd_page_tables.count))
+	if (drm_WARN_ON(ddev, !op_ctx) ||
+	    drm_WARN_ON(ddev, op_ctx->rsvd_page_tables.ptr >= op_ctx->rsvd_page_tables.count))
 		return NULL;
 
-	page = vm->op_ctx->rsvd_page_tables.pages[vm->op_ctx->rsvd_page_tables.ptr++];
+	page = op_ctx->rsvd_page_tables.pages[op_ctx->rsvd_page_tables.ptr++];
 	memset(page, 0, SZ_4K);
 
 	/* Page table entries don't use virtual addresses, which trips out
@@ -518,22 +537,23 @@ static void *alloc_pt(void *cookie, size_t size, gfp_t gfp)
  */
 static void free_pt(void *cookie, void *data, size_t size)
 {
-	struct panthor_vm *vm = cookie;
+	struct panthor_as *as = cookie;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 
-	if (unlikely(vm->root_page_table == data)) {
+	if (unlikely(as->pt.root == data)) {
 		free_pages((unsigned long)data, get_order(size));
-		vm->root_page_table = NULL;
+		as->pt.root = NULL;
 		return;
 	}
 
-	if (drm_WARN_ON(&vm->ptdev->base, size != SZ_4K))
+	if (drm_WARN_ON(&ptdev->base, size != SZ_4K))
 		return;
 
 	/* Return the page to the pt_cache. */
-	kmem_cache_free(vm->ptdev->mmu->pt_cache, data);
+	kmem_cache_free(ptdev->mmu->pt_cache, data);
 }
 
-static int wait_ready(struct panthor_device *ptdev, u32 as_nr)
+static int wait_ready(struct panthor_device *ptdev, u32 slot)
 {
 	struct panthor_mmu *mmu = ptdev->mmu;
 	int ret;
@@ -542,7 +562,7 @@ static int wait_ready(struct panthor_device *ptdev, u32 as_nr)
 	/* Wait for the MMU status to indicate there is no active command, in
 	 * case one is pending.
 	 */
-	ret = gpu_read_relaxed_poll_timeout_atomic(mmu->iomem, AS_STATUS(as_nr), val,
+	ret = gpu_read_relaxed_poll_timeout_atomic(mmu->iomem, AS_STATUS(slot), val,
 						   !(val & AS_STATUS_AS_ACTIVE), 10, 100000);
 
 	if (ret) {
@@ -553,15 +573,15 @@ static int wait_ready(struct panthor_device *ptdev, u32 as_nr)
 	return ret;
 }
 
-static int as_send_cmd_and_wait(struct panthor_device *ptdev, u32 as_nr, u32 cmd)
+static int as_send_cmd_and_wait(struct panthor_device *ptdev, u32 slot, u32 cmd)
 {
 	int status;
 
 	/* write AS_COMMAND when MMU is ready to accept another command */
-	status = wait_ready(ptdev, as_nr);
+	status = wait_ready(ptdev, slot);
 	if (!status) {
-		gpu_write(ptdev->mmu->iomem, AS_COMMAND(as_nr), cmd);
-		status = wait_ready(ptdev, as_nr);
+		gpu_write(ptdev->mmu->iomem, AS_COMMAND(slot), cmd);
+		status = wait_ready(ptdev, slot);
 	}
 
 	return status;
@@ -596,37 +616,37 @@ static u64 pack_region_range(struct panthor_device *ptdev, u64 *region_start, u6
 	return region_width | *region_start;
 }
 
-static u32 panthor_mmu_as_fault_mask(struct panthor_device *ptdev, u32 as)
+static u32 panthor_mmu_as_fault_mask(struct panthor_device *ptdev, u32 slot)
 {
-	return BIT(as);
+	return BIT(slot);
 }
 
-static int panthor_mmu_as_enable(struct panthor_device *ptdev, u32 as_nr,
+static int panthor_mmu_as_enable(struct panthor_device *ptdev, u32 slot,
 				 u64 transtab, u64 transcfg, u64 memattr)
 {
 	struct panthor_mmu *mmu = ptdev->mmu;
 
 	panthor_irq_enable_events(&ptdev->mmu->irq,
-				  panthor_mmu_as_fault_mask(ptdev, as_nr));
+				  panthor_mmu_as_fault_mask(ptdev, slot));
 
-	gpu_write64(mmu->iomem, AS_TRANSTAB(as_nr), transtab);
-	gpu_write64(mmu->iomem, AS_MEMATTR(as_nr), memattr);
-	gpu_write64(mmu->iomem, AS_TRANSCFG(as_nr), transcfg);
+	gpu_write64(mmu->iomem, AS_TRANSTAB(slot), transtab);
+	gpu_write64(mmu->iomem, AS_MEMATTR(slot), memattr);
+	gpu_write64(mmu->iomem, AS_TRANSCFG(slot), transcfg);
 
-	return as_send_cmd_and_wait(ptdev, as_nr, AS_COMMAND_UPDATE);
+	return as_send_cmd_and_wait(ptdev, slot, AS_COMMAND_UPDATE);
 }
 
-static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
+static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 slot,
 				  bool recycle_slot)
 {
 	struct panthor_mmu *mmu = ptdev->mmu;
-	struct panthor_vm *vm = ptdev->mmu->as.slots[as_nr].vm;
+	struct panthor_as *as = ptdev->mmu->as.slots[slot].as;
 	int ret;
 
 	lockdep_assert_held(&ptdev->mmu->as.slots_lock);
 
 	panthor_irq_disable_events(&ptdev->mmu->irq,
-				   panthor_mmu_as_fault_mask(ptdev, as_nr));
+				   panthor_mmu_as_fault_mask(ptdev, slot));
 
 	/* Flush+invalidate RW caches, invalidate RO ones. */
 	ret = panthor_gpu_flush_caches(ptdev, CACHE_CLEAN | CACHE_INV,
@@ -634,9 +654,9 @@ static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
 	if (ret)
 		return ret;
 
-	if (vm && vm->locked_region.size) {
+	if (as && as->locked_region.size) {
 		/* Unlock the region if there's a lock pending. */
-		ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_UNLOCK);
+		ret = as_send_cmd_and_wait(ptdev, slot, AS_COMMAND_UNLOCK);
 		if (ret)
 			return ret;
 	}
@@ -647,11 +667,11 @@ static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
 	if (recycle_slot)
 		return 0;
 
-	gpu_write64(mmu->iomem, AS_TRANSTAB(as_nr), 0);
-	gpu_write64(mmu->iomem, AS_MEMATTR(as_nr), 0);
-	gpu_write64(mmu->iomem, AS_TRANSCFG(as_nr), AS_TRANSCFG_ADRMODE_UNMAPPED);
+	gpu_write64(mmu->iomem, AS_TRANSTAB(slot), 0);
+	gpu_write64(mmu->iomem, AS_MEMATTR(slot), 0);
+	gpu_write64(mmu->iomem, AS_TRANSCFG(slot), AS_TRANSCFG_ADRMODE_UNMAPPED);
 
-	return as_send_cmd_and_wait(ptdev, as_nr, AS_COMMAND_UPDATE);
+	return as_send_cmd_and_wait(ptdev, slot, AS_COMMAND_UPDATE);
 }
 
 static u32 panthor_mmu_fault_mask(struct panthor_device *ptdev, u32 value)
@@ -668,7 +688,7 @@ static u32 panthor_mmu_fault_mask(struct panthor_device *ptdev, u32 value)
  */
 bool panthor_vm_has_unhandled_faults(struct panthor_vm *vm)
 {
-	return vm->unhandled_fault;
+	return vm->as->unhandled_fault;
 }
 
 /**
@@ -679,23 +699,23 @@ bool panthor_vm_has_unhandled_faults(struct panthor_vm *vm)
  */
 bool panthor_vm_is_unusable(struct panthor_vm *vm)
 {
-	return vm->unusable;
+	return vm->as->unusable;
 }
 
-static void panthor_vm_release_as_locked(struct panthor_vm *vm)
+static void panthor_as_release_hw_slot_locked(struct panthor_as *as)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 
 	lockdep_assert_held(&ptdev->mmu->as.slots_lock);
 
-	if (drm_WARN_ON(&ptdev->base, vm->as.id < 0))
+	if (drm_WARN_ON(&ptdev->base, as->hw_slot.id < 0))
 		return;
 
-	ptdev->mmu->as.slots[vm->as.id].vm = NULL;
-	clear_bit(vm->as.id, &ptdev->mmu->as.alloc_mask);
-	refcount_set(&vm->as.active_cnt, 0);
-	list_del_init(&vm->as.lru_node);
-	vm->as.id = -1;
+	ptdev->mmu->as.slots[as->hw_slot.id].as = NULL;
+	clear_bit(as->hw_slot.id, &ptdev->mmu->as.alloc_mask);
+	refcount_set(&as->active_cnt, 0);
+	list_del_init(&as->hw_slot.lru_node);
+	as->hw_slot.id = -1;
 }
 
 /**
@@ -708,17 +728,18 @@ static void panthor_vm_release_as_locked(struct panthor_vm *vm)
  */
 int panthor_vm_active(struct panthor_vm *vm)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct panthor_as *as = vm->as;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	u32 va_bits = GPU_MMU_FEATURES_VA_BITS(ptdev->gpu_info.mmu_features);
-	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(vm->pgtbl_ops)->cfg;
-	int ret = 0, as, cookie;
+	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(as->pt.ops)->cfg;
+	int ret = 0, slot, cookie;
 	u64 transtab, transcfg;
 	u32 fault_mask;
 
 	if (!drm_dev_enter(&ptdev->base, &cookie))
 		return -ENODEV;
 
-	if (refcount_inc_not_zero(&vm->as.active_cnt))
+	if (refcount_inc_not_zero(&as->active_cnt))
 		goto out_dev_exit;
 
 	/* As soon as active is called, we place the VM at the end of the VM LRU.
@@ -727,25 +748,25 @@ int panthor_vm_active(struct panthor_vm *vm)
 	 * that's an acceptable trade-off.
 	 */
 	mutex_lock(&ptdev->base.gem_lru_mutex);
-	if (vm->reclaim.lru.count)
-		list_move_tail(&vm->reclaim.lru_node, &ptdev->reclaim.vms);
+	if (as->reclaim.lru.count && !as->reclaim.skip)
+		list_move_tail(&as->reclaim.lru_node, &ptdev->reclaim.vms);
 	mutex_unlock(&ptdev->base.gem_lru_mutex);
 
 	/* Make sure we don't race with lock/unlock_region() calls
 	 * happening around VM bind operations.
 	 */
-	mutex_lock(&vm->op_lock);
+	mutex_lock(&as->op_lock);
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 
-	if (refcount_inc_not_zero(&vm->as.active_cnt))
+	if (refcount_inc_not_zero(&as->active_cnt))
 		goto out_unlock;
 
-	as = vm->as.id;
-	if (as >= 0) {
+	slot = as->hw_slot.id;
+	if (slot >= 0) {
 		/* Unhandled pagefault on this AS, the MMU was disabled. We need to
 		 * re-enable the MMU after clearing+unmasking the AS interrupts.
 		 */
-		if (ptdev->mmu->as.faulty_mask & panthor_mmu_as_fault_mask(ptdev, as))
+		if (ptdev->mmu->as.faulty_mask & panthor_mmu_as_fault_mask(ptdev, slot))
 			goto out_enable_as;
 
 		goto out_make_active;
@@ -754,36 +775,36 @@ int panthor_vm_active(struct panthor_vm *vm)
 	/* Check for a free AS */
 	if (vm->for_mcu) {
 		drm_WARN_ON(&ptdev->base, ptdev->mmu->as.alloc_mask & BIT(0));
-		as = 0;
+		slot = 0;
 	} else {
-		as = ffz(ptdev->mmu->as.alloc_mask | BIT(0));
+		slot = ffz(ptdev->mmu->as.alloc_mask | BIT(0));
 	}
 
-	if (!(BIT(as) & ptdev->gpu_info.as_present)) {
-		struct panthor_vm *lru_vm;
+	if (!(BIT(slot) & ptdev->gpu_info.as_present)) {
+		struct panthor_as *lru_as;
 
-		lru_vm = list_first_entry_or_null(&ptdev->mmu->as.lru_list,
-						  struct panthor_vm,
-						  as.lru_node);
-		if (drm_WARN_ON(&ptdev->base, !lru_vm)) {
+		lru_as = list_first_entry_or_null(&ptdev->mmu->as.lru_list,
+						  struct panthor_as,
+						  hw_slot.lru_node);
+		if (drm_WARN_ON(&ptdev->base, !lru_as)) {
 			ret = -EBUSY;
 			goto out_unlock;
 		}
 
-		drm_WARN_ON(&ptdev->base, refcount_read(&lru_vm->as.active_cnt));
-		as = lru_vm->as.id;
+		drm_WARN_ON(&ptdev->base, refcount_read(&lru_as->active_cnt));
+		slot = lru_as->hw_slot.id;
 
-		ret = panthor_mmu_as_disable(ptdev, as, true);
+		ret = panthor_mmu_as_disable(ptdev, slot, true);
 		if (ret)
 			goto out_unlock;
 
-		panthor_vm_release_as_locked(lru_vm);
+		panthor_as_release_hw_slot_locked(lru_as);
 	}
 
 	/* Assign the free or reclaimed AS to the FD */
-	vm->as.id = as;
-	set_bit(as, &ptdev->mmu->as.alloc_mask);
-	ptdev->mmu->as.slots[as].vm = vm;
+	as->hw_slot.id = slot;
+	set_bit(slot, &ptdev->mmu->as.alloc_mask);
+	ptdev->mmu->as.slots[slot].as = as;
 
 out_enable_as:
 	transtab = cfg->arm_lpae_s1_cfg.ttbr;
@@ -795,12 +816,12 @@ int panthor_vm_active(struct panthor_vm *vm)
 		transcfg |= AS_TRANSCFG_PTW_SH_OS;
 
 	/* If the VM is re-activated, we clear the fault. */
-	vm->unhandled_fault = false;
+	as->unhandled_fault = false;
 
 	/* Unhandled pagefault on this AS, clear the fault and enable the AS,
 	 * which re-enables interrupts.
 	 */
-	fault_mask = panthor_mmu_as_fault_mask(ptdev, as);
+	fault_mask = panthor_mmu_as_fault_mask(ptdev, slot);
 	if (ptdev->mmu->as.faulty_mask & fault_mask) {
 		gpu_write(ptdev->mmu->irq.iomem, INT_CLEAR, fault_mask);
 		ptdev->mmu->as.faulty_mask &= ~fault_mask;
@@ -809,18 +830,18 @@ int panthor_vm_active(struct panthor_vm *vm)
 	/* The VM update is guarded by ::op_lock, which we take at the beginning
 	 * of this function, so we don't expect any locked region here.
 	 */
-	drm_WARN_ON(&vm->ptdev->base, vm->locked_region.size > 0);
-	ret = panthor_mmu_as_enable(vm->ptdev, vm->as.id, transtab, transcfg, vm->memattr);
+	drm_WARN_ON(&ptdev->base, as->locked_region.size > 0);
+	ret = panthor_mmu_as_enable(ptdev, as->hw_slot.id, transtab, transcfg, as->memattr);
 
 out_make_active:
 	if (!ret) {
-		refcount_set(&vm->as.active_cnt, 1);
-		list_del_init(&vm->as.lru_node);
+		refcount_set(&as->active_cnt, 1);
+		list_del_init(&as->hw_slot.lru_node);
 	}
 
 out_unlock:
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
-	mutex_unlock(&vm->op_lock);
+	mutex_unlock(&as->op_lock);
 
 out_dev_exit:
 	drm_dev_exit(cookie);
@@ -842,21 +863,22 @@ int panthor_vm_active(struct panthor_vm *vm)
  */
 void panthor_vm_idle(struct panthor_vm *vm)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct panthor_as *as = vm->as;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 
-	if (!refcount_dec_and_mutex_lock(&vm->as.active_cnt, &ptdev->mmu->as.slots_lock))
+	if (!refcount_dec_and_mutex_lock(&as->active_cnt, &ptdev->mmu->as.slots_lock))
 		return;
 
-	if (!drm_WARN_ON(&ptdev->base, vm->as.id == -1 || !list_empty(&vm->as.lru_node)))
-		list_add_tail(&vm->as.lru_node, &ptdev->mmu->as.lru_list);
+	if (!drm_WARN_ON(&ptdev->base, as->hw_slot.id == -1 || !list_empty(&as->hw_slot.lru_node)))
+		list_add_tail(&as->hw_slot.lru_node, &ptdev->mmu->as.lru_list);
 
-	refcount_set(&vm->as.active_cnt, 0);
+	refcount_set(&as->active_cnt, 0);
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
 }
 
 u32 panthor_vm_page_size(struct panthor_vm *vm)
 {
-	const struct io_pgtable *pgt = io_pgtable_ops_to_pgtable(vm->pgtbl_ops);
+	const struct io_pgtable *pgt = io_pgtable_ops_to_pgtable(vm->as->pt.ops);
 	u32 pg_shift = ffs(pgt->cfg.pgsize_bitmap) - 1;
 
 	return 1u << pg_shift;
@@ -880,7 +902,7 @@ static void panthor_vm_start(struct panthor_vm *vm)
  */
 int panthor_vm_as(struct panthor_vm *vm)
 {
-	return vm->as.id;
+	return vm->as->hw_slot.id;
 }
 
 static size_t get_pgsize(u64 addr, size_t size, size_t *count)
@@ -904,43 +926,43 @@ static size_t get_pgsize(u64 addr, size_t size, size_t *count)
 	return SZ_2M;
 }
 
-static void panthor_vm_declare_unusable(struct panthor_vm *vm)
+static void panthor_as_declare_unusable(struct panthor_as *as)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	int cookie;
 
-	if (vm->unusable)
+	if (as->unusable)
 		return;
 
-	vm->unusable = true;
+	as->unusable = true;
 	mutex_lock(&ptdev->mmu->as.slots_lock);
-	if (vm->as.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) {
-		panthor_mmu_as_disable(ptdev, vm->as.id, false);
+	if (as->hw_slot.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) {
+		panthor_mmu_as_disable(ptdev, as->hw_slot.id, false);
 		drm_dev_exit(cookie);
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
 }
 
-static void panthor_vm_unmap_pages(struct panthor_vm *vm, u64 iova, u64 size)
+static void panthor_as_unmap_pages(struct panthor_as *as, u64 iova, u64 size)
 {
-	struct panthor_device *ptdev = vm->ptdev;
-	struct io_pgtable_ops *ops = vm->pgtbl_ops;
+	struct drm_device *ddev = as->base.drm;
+	struct io_pgtable_ops *ops = as->pt.ops;
 	u64 start_iova = iova;
 	u64 offset = 0;
 
 	if (!size)
 		return;
 
-	drm_WARN_ON(&ptdev->base,
-		    (iova < vm->locked_region.start) ||
-		    (iova + size > vm->locked_region.start + vm->locked_region.size));
+	drm_WARN_ON(ddev,
+		    (iova < as->locked_region.start) ||
+		    (iova + size > as->locked_region.start + as->locked_region.size));
 
 	while (offset < size) {
 		size_t unmapped_sz = 0, pgcount;
 		size_t pgsize = get_pgsize(iova + offset, size - offset, &pgcount);
 
 		unmapped_sz = ops->unmap_pages(ops, iova + offset, pgsize, pgcount, NULL);
-		if (drm_WARN_ON_ONCE(&ptdev->base, unmapped_sz != pgsize * pgcount)) {
+		if (drm_WARN_ON_ONCE(ddev, unmapped_sz != pgsize * pgcount)) {
 			/* Gracefully handle sparsely unmapped regions to avoid leaving
 			 * page table pages behind when the drm_gpuvm and VM page table
 			 * are out-of-sync. This is not supposed to happen, hence the
@@ -954,33 +976,32 @@ static void panthor_vm_unmap_pages(struct panthor_vm *vm, u64 iova, u64 size)
 			 * so flag the VM unusable to make sure it's not going
 			 * to be used anymore.
 			 */
-			panthor_vm_declare_unusable(vm);
+			panthor_as_declare_unusable(as);
 
 			/* If we don't make progress, we're screwed. That also means
 			 * something else prevents us from unmapping the region, but
 			 * there's not much we can do here: time for debugging.
 			 */
-			if (drm_WARN_ON_ONCE(&ptdev->base, !unmapped_sz))
+			if (drm_WARN_ON_ONCE(ddev, !unmapped_sz))
 				return;
 		}
 
-		drm_dbg(&ptdev->base,
-			"unmap: as=%d, iova=0x%llx, sz=%llu, va=0x%llx, pgcnt=%zu, pgsz=%zu",
-			vm->as.id, start_iova, size, iova + offset,
-			unmapped_sz / pgsize, pgsize);
+		drm_dbg(ddev,
+			"unmap: iova=0x%llx, sz=%llu, va=0x%llx, pgcnt=%zu, pgsz=%zu",
+			start_iova, size, iova + offset, unmapped_sz / pgsize, pgsize);
 
 		offset += unmapped_sz;
 	}
 }
 
 static int
-panthor_vm_map_pages(struct panthor_vm *vm, u64 iova, int prot,
+panthor_as_map_pages(struct panthor_as *as, u64 iova, int prot,
 		     struct sg_table *sgt, u64 offset, u64 size)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct drm_device *ddev = as->base.drm;
 	unsigned int count;
 	struct scatterlist *sgl;
-	struct io_pgtable_ops *ops = vm->pgtbl_ops;
+	struct io_pgtable_ops *ops = as->pt.ops;
 	u64 start_iova = iova;
 	u64 start_size = size;
 	int ret;
@@ -988,9 +1009,9 @@ panthor_vm_map_pages(struct panthor_vm *vm, u64 iova, int prot,
 	if (!size)
 		return 0;
 
-	drm_WARN_ON(&ptdev->base,
-		    (iova < vm->locked_region.start) ||
-		    (iova + size > vm->locked_region.start + vm->locked_region.size));
+	drm_WARN_ON(ddev,
+		    (iova < as->locked_region.start) ||
+		    (iova + size > as->locked_region.start + as->locked_region.size));
 
 	for_each_sgtable_dma_sg(sgt, sgl, count) {
 		dma_addr_t paddr = sg_dma_address(sgl);
@@ -1013,10 +1034,9 @@ panthor_vm_map_pages(struct panthor_vm *vm, u64 iova, int prot,
 			ret = ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot,
 					     GFP_KERNEL, &mapped);
 
-			drm_dbg(&ptdev->base,
-				"map: as=%d, iova=0x%llx, sz=%llu, va=0x%llx, pa=%pad, pgcnt=%zu, pgsz=%zu",
-				vm->as.id, start_iova, start_size, iova, &paddr,
-				mapped / pgsize, pgsize);
+			drm_dbg(ddev,
+				"map: iova=0x%llx, sz=%llu, va=0x%llx, pa=%pad, pgcnt=%zu, pgsz=%zu",
+				start_iova, start_size, iova, &paddr, mapped / pgsize, pgsize);
 
 			iova += mapped;
 			paddr += mapped;
@@ -1027,12 +1047,12 @@ panthor_vm_map_pages(struct panthor_vm *vm, u64 iova, int prot,
 				ret = -ENOMEM;
 
 			/* If something fails, we stop there, and flag the VM unusable. */
-			if (drm_WARN_ON_ONCE(&ptdev->base, ret)) {
+			if (drm_WARN_ON_ONCE(ddev, ret)) {
 				/* Unmap what we've already mapped to avoid leaving page
 				 * table pages behind.
 				 */
-				panthor_vm_unmap_pages(vm, start_iova, iova - start_iova);
-				panthor_vm_declare_unusable(vm);
+				panthor_as_unmap_pages(as, start_iova, iova - start_iova);
+				panthor_as_declare_unusable(as);
 				return ret;
 			}
 		}
@@ -1047,8 +1067,8 @@ panthor_vm_map_pages(struct panthor_vm *vm, u64 iova, int prot,
 }
 
 static int
-panthor_vm_map_sparse(struct panthor_vm *vm, u64 iova, int prot,
-		      struct sg_table *sgt, u64 size)
+panthor_as_map_sparse(struct panthor_as *as, u64 iova,
+		      int prot, struct sg_table *sgt, u64 size)
 {
 	u64 mapped = 0;
 	int ret;
@@ -1057,10 +1077,10 @@ panthor_vm_map_sparse(struct panthor_vm *vm, u64 iova, int prot,
 		u64 addr = iova + mapped;
 		u32 chunk_size = min(size - mapped, SZ_2M - (addr & (SZ_2M - 1)));
 
-		ret = panthor_vm_map_pages(vm, addr, prot, sgt,
+		ret = panthor_as_map_pages(as, addr, prot, sgt,
 					   addr % SZ_2M, chunk_size);
 		if (ret) {
-			panthor_vm_unmap_pages(vm, iova, mapped);
+			panthor_as_unmap_pages(as, iova, mapped);
 			return ret;
 		}
 
@@ -1162,9 +1182,10 @@ static void panthor_vm_bo_free(struct drm_gpuvm_bo *vm_bo)
 	kfree(vm_bo);
 }
 
-static void panthor_vm_cleanup_op_ctx(struct panthor_vm_op_ctx *op_ctx,
-				      struct panthor_vm *vm)
+static void panthor_as_cleanup_op_ctx(struct panthor_as_op_ctx *op_ctx,
+				      struct panthor_as *as)
 {
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	u32 remaining_pt_count = op_ctx->rsvd_page_tables.count -
 				 op_ctx->rsvd_page_tables.ptr;
 	u32 op_type = op_ctx->flags & DRM_PANTHOR_VM_BIND_OP_TYPE_MASK;
@@ -1179,7 +1200,7 @@ static void panthor_vm_cleanup_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 				     !op_ctx->map.bo;
 
 	if (remaining_pt_count) {
-		kmem_cache_free_bulk(vm->ptdev->mmu->pt_cache,
+		kmem_cache_free_bulk(ptdev->mmu->pt_cache,
 				     remaining_pt_count,
 				     op_ctx->rsvd_page_tables.pages +
 				     op_ctx->rsvd_page_tables.ptr);
@@ -1199,11 +1220,11 @@ static void panthor_vm_cleanup_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 		kfree(op_ctx->preallocated_vmas[i]);
 
 	if (!skip_deferred_cleanup)
-		drm_gpuvm_bo_deferred_cleanup(&vm->base);
+		drm_gpuvm_bo_deferred_cleanup(&as->base);
 }
 
 static void
-panthor_vm_op_ctx_return_vma(struct panthor_vm_op_ctx *op_ctx,
+panthor_as_op_ctx_return_vma(struct panthor_as_op_ctx *op_ctx,
 			     struct panthor_vma *vma)
 {
 	for (u32 i = 0; i < ARRAY_SIZE(op_ctx->preallocated_vmas); i++) {
@@ -1217,7 +1238,7 @@ panthor_vm_op_ctx_return_vma(struct panthor_vm_op_ctx *op_ctx,
 }
 
 static struct panthor_vma *
-panthor_vm_op_ctx_get_vma(struct panthor_vm_op_ctx *op_ctx)
+panthor_as_op_ctx_get_vma(struct panthor_as_op_ctx *op_ctx)
 {
 	for (u32 i = 0; i < ARRAY_SIZE(op_ctx->preallocated_vmas); i++) {
 		struct panthor_vma *vma = op_ctx->preallocated_vmas[i];
@@ -1232,7 +1253,7 @@ panthor_vm_op_ctx_get_vma(struct panthor_vm_op_ctx *op_ctx)
 }
 
 static int
-panthor_vm_op_ctx_prealloc_vmas(struct panthor_vm_op_ctx *op_ctx)
+panthor_as_op_ctx_prealloc_vmas(struct panthor_as_op_ctx *op_ctx)
 {
 	u32 vma_count;
 
@@ -1271,7 +1292,7 @@ panthor_vm_op_ctx_prealloc_vmas(struct panthor_vm_op_ctx *op_ctx)
 	return 0;
 }
 
-static void panthor_vm_init_op_ctx(struct panthor_vm_op_ctx *op_ctx,
+static void panthor_vm_init_op_ctx(struct panthor_as_op_ctx *op_ctx,
 				   u64 size, u64 va, u32 flags)
 {
 	memset(op_ctx, 0, sizeof(*op_ctx));
@@ -1280,8 +1301,8 @@ static void panthor_vm_init_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	op_ctx->va.addr = va;
 }
 
-static int panthor_vm_op_ctx_prealloc_pts(struct panthor_device *ptdev,
-					  struct panthor_vm_op_ctx *op_ctx)
+static int panthor_as_op_ctx_prealloc_pts(struct panthor_device *ptdev,
+					  struct panthor_as_op_ctx *op_ctx)
 {
 	u64 size = op_ctx->va.range;
 	u64 va = op_ctx->va.addr;
@@ -1317,11 +1338,12 @@ static int panthor_vm_op_ctx_prealloc_pts(struct panthor_device *ptdev,
 	 DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE | \
 	 DRM_PANTHOR_VM_BIND_OP_TYPE_MASK)
 
-static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
-					 struct panthor_vm *vm,
+static int panthor_as_prepare_map_op_ctx(struct panthor_as_op_ctx *op_ctx,
+					 struct panthor_as *as,
 					 struct panthor_gem_object *bo,
 					 const struct drm_panthor_vm_bind_op *op)
 {
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	bool is_sparse = op->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
 	struct drm_gpuvm_bo *preallocated_vm_bo;
 	struct sg_table *sgt = NULL;
@@ -1353,12 +1375,12 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 
 	/* If the BO has an exclusive VM attached, it can't be mapped to other VMs. */
 	if (bo->exclusive_vm_root_gem &&
-	    bo->exclusive_vm_root_gem != panthor_vm_root_gem(vm))
+	    bo->exclusive_vm_root_gem != as->base.r_obj)
 		return -EINVAL;
 
 	panthor_vm_init_op_ctx(op_ctx, op->size, op->va, op->flags);
 
-	ret = panthor_vm_op_ctx_prealloc_vmas(op_ctx);
+	ret = panthor_as_op_ctx_prealloc_vmas(op_ctx);
 	if (ret)
 		goto err_cleanup;
 
@@ -1378,7 +1400,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 		goto err_cleanup;
 	}
 
-	preallocated_vm_bo = drm_gpuvm_bo_create(&vm->base, &bo->base);
+	preallocated_vm_bo = drm_gpuvm_bo_create(&as->base, &bo->base);
 	if (!preallocated_vm_bo) {
 		ret = -ENOMEM;
 		goto err_cleanup;
@@ -1387,15 +1409,15 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	op_ctx->map.vm_bo = drm_gpuvm_bo_obtain_prealloc(preallocated_vm_bo);
 	op_ctx->map.bo_offset = op->bo_offset;
 
-	ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx);
+	ret = panthor_as_op_ctx_prealloc_pts(ptdev, op_ctx);
 	if (ret)
 		goto err_cleanup;
 
 	/* Insert BO into the extobj list last, when we know nothing can fail. */
-	if (bo->base.resv != panthor_vm_resv(vm)) {
-		dma_resv_lock(panthor_vm_resv(vm), NULL);
+	if (bo->base.resv != drm_gpuvm_resv(&as->base)) {
+		dma_resv_lock(drm_gpuvm_resv(&as->base), NULL);
 		drm_gpuvm_bo_extobj_add(op_ctx->map.vm_bo);
-		dma_resv_unlock(panthor_vm_resv(vm));
+		dma_resv_unlock(drm_gpuvm_resv(&as->base));
 	}
 
 	/* And finally update the BO state. */
@@ -1408,14 +1430,15 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	return 0;
 
 err_cleanup:
-	panthor_vm_cleanup_op_ctx(op_ctx, vm);
+	panthor_as_cleanup_op_ctx(op_ctx, as);
 	return ret;
 }
 
-static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
-					   struct panthor_vm *vm,
+static int panthor_as_prepare_unmap_op_ctx(struct panthor_as_op_ctx *op_ctx,
+					   struct panthor_as *as,
 					   u64 va, u64 size)
 {
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	u32 pt_count = 0;
 	int ret;
 
@@ -1434,7 +1457,7 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	    ALIGN(va + size, SZ_2M) != ALIGN(va, SZ_2M))
 		pt_count++;
 
-	ret = panthor_vm_op_ctx_prealloc_vmas(op_ctx);
+	ret = panthor_as_op_ctx_prealloc_vmas(op_ctx);
 	if (ret)
 		goto err_cleanup;
 
@@ -1446,7 +1469,7 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 			goto err_cleanup;
 		}
 
-		if (!kmem_cache_alloc_bulk(vm->ptdev->mmu->pt_cache,
+		if (!kmem_cache_alloc_bulk(ptdev->mmu->pt_cache,
 					   GFP_KERNEL, pt_count,
 					   op_ctx->rsvd_page_tables.pages)) {
 			ret = -ENOMEM;
@@ -1458,12 +1481,12 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
 	return 0;
 
 err_cleanup:
-	panthor_vm_cleanup_op_ctx(op_ctx, vm);
+	panthor_as_cleanup_op_ctx(op_ctx, as);
 	return ret;
 }
 
 static void
-panthor_vm_prepare_sync_only_op_ctx(struct panthor_vm_op_ctx *op_ctx)
+panthor_as_prepare_sync_only_op_ctx(struct panthor_as_op_ctx *op_ctx)
 {
 	memset(op_ctx, 0, sizeof(*op_ctx));
 	op_ctx->flags = DRM_PANTHOR_VM_BIND_OP_TYPE_SYNC_ONLY;
@@ -1491,8 +1514,8 @@ panthor_vm_get_bo_for_va(struct panthor_vm *vm, u64 va, u64 *bo_offset)
 	struct panthor_vma *vma;
 
 	/* Take the VM lock to prevent concurrent map/unmap operations. */
-	mutex_lock(&vm->op_lock);
-	gpuva = drm_gpuva_find_first(&vm->base, va, 1);
+	mutex_lock(&vm->as->op_lock);
+	gpuva = drm_gpuva_find_first(&vm->as->base, va, 1);
 	vma = gpuva ? container_of(gpuva, struct panthor_vma, base) : NULL;
 	if (vma && vma->base.gem.obj) {
 		drm_gem_object_get(vma->base.gem.obj);
@@ -1501,7 +1524,7 @@ panthor_vm_get_bo_for_va(struct panthor_vm *vm, u64 va, u64 *bo_offset)
 			vma->base.gem.offset + (va - vma->base.va.addr) :
 			va & (SZ_2M - 1);
 	}
-	mutex_unlock(&vm->op_lock);
+	mutex_unlock(&vm->as->op_lock);
 
 	return bo;
 }
@@ -1621,22 +1644,24 @@ int panthor_vm_pool_create_vm(struct panthor_device *ptdev,
 
 static void panthor_vm_destroy(struct panthor_vm *vm)
 {
+	struct panthor_as *as;
+	struct panthor_device *ptdev;
+
 	if (!vm)
 		return;
 
+	as = vm->as;
+	ptdev = container_of(as->base.drm, struct panthor_device, base);
 	vm->destroyed = true;
 
 	/* Tell scheduler to stop all GPU work related to this VM */
-	if (refcount_read(&vm->as.active_cnt) > 0)
-		panthor_sched_prepare_for_vm_destruction(vm->ptdev);
+	if (refcount_read(&as->active_cnt) > 0)
+		panthor_sched_prepare_for_vm_destruction(ptdev);
 
 	mutex_lock(&vm->heaps.lock);
 	panthor_heap_pool_destroy(vm->heaps.pool);
 	vm->heaps.pool = NULL;
 	mutex_unlock(&vm->heaps.lock);
-
-	drm_WARN_ON(&vm->ptdev->base,
-		    panthor_vm_unmap_range(vm, vm->base.mm_start, vm->base.mm_range));
 	panthor_vm_put(vm);
 }
 
@@ -1776,17 +1801,18 @@ static const char *access_type_name(struct panthor_device *ptdev,
 	}
 }
 
-static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
+static int panthor_as_lock_region(struct panthor_as *as, u64 start, u64 size)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct drm_device *ddev = as->base.drm;
+	struct panthor_device *ptdev = container_of(ddev, struct panthor_device, base);
 	int ret = 0;
 
-	/* sm_step_remap() can call panthor_vm_lock_region() to account for
+	/* sm_step_remap() can call panthor_as_lock_region() to account for
 	 * the wider unmap needed when doing a partial huge page unamp. We
 	 * need to ignore the lock if it's already part of the locked region.
 	 */
-	if (start >= vm->locked_region.start &&
-	    start + size <= vm->locked_region.start + vm->locked_region.size)
+	if (start >= as->locked_region.start &&
+	    start + size <= as->locked_region.start + as->locked_region.size)
 		return 0;
 
 	/* sm_step_remap() may need a locked region that isn't a strict superset
@@ -1797,42 +1823,42 @@ static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
 	 * boundaries in a remap operation can only shift up or down respectively,
 	 * but never otherwise.
 	 */
-	if (vm->locked_region.size) {
-		u64 end = max(vm->locked_region.start + vm->locked_region.size,
+	if (as->locked_region.size) {
+		u64 end = max(as->locked_region.start + as->locked_region.size,
 			      start + size);
 
-		drm_WARN_ON_ONCE(&vm->ptdev->base, (start + size <= vm->locked_region.start) ||
-				 (start >= vm->locked_region.start + vm->locked_region.size));
+		drm_WARN_ON_ONCE(ddev, (start + size <= as->locked_region.start) ||
+				 (start >= as->locked_region.start + as->locked_region.size));
 
-		start = min(start, vm->locked_region.start);
+		start = min(start, as->locked_region.start);
 		size = end - start;
 	}
 
 	mutex_lock(&ptdev->mmu->as.slots_lock);
-	if (vm->as.id >= 0 && size) {
+	if (as->hw_slot.id >= 0 && size) {
 		/* Lock the region that needs to be updated */
-		gpu_write64(ptdev->mmu->iomem, AS_LOCKADDR(vm->as.id),
+		gpu_write64(ptdev->mmu->iomem, AS_LOCKADDR(as->hw_slot.id),
 			    pack_region_range(ptdev, &start, &size));
 
 		/* If the lock succeeded, update the locked_region info. */
-		ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_LOCK);
+		ret = as_send_cmd_and_wait(ptdev, as->hw_slot.id, AS_COMMAND_LOCK);
 	}
 
 	if (!ret) {
-		vm->locked_region.start = start;
-		vm->locked_region.size = size;
+		as->locked_region.start = start;
+		as->locked_region.size = size;
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
 
 	return ret;
 }
 
-static void panthor_vm_unlock_region(struct panthor_vm *vm)
+static void panthor_as_unlock_region(struct panthor_as *as)
 {
-	struct panthor_device *ptdev = vm->ptdev;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 
 	mutex_lock(&ptdev->mmu->as.slots_lock);
-	if (vm->as.id >= 0) {
+	if (as->hw_slot.id >= 0) {
 		int ret;
 
 		/* flush+invalidate RW caches and invalidate RO ones.
@@ -1845,7 +1871,7 @@ static void panthor_vm_unlock_region(struct panthor_vm *vm)
 
 		/* Unlock the region if the flush is effective. */
 		if (!ret)
-			ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_UNLOCK);
+			ret = as_send_cmd_and_wait(ptdev, as->hw_slot.id, AS_COMMAND_UNLOCK);
 
 		/* If we fail to flush or unlock the region, schedule a GPU reset
 		 * to unblock the situation.
@@ -1853,8 +1879,8 @@ static void panthor_vm_unlock_region(struct panthor_vm *vm)
 		if (ret)
 			panthor_device_schedule_reset(ptdev);
 	}
-	vm->locked_region.start = 0;
-	vm->locked_region.size = 0;
+	as->locked_region.start = 0;
+	as->locked_region.size = 0;
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
 }
 
@@ -1908,8 +1934,8 @@ static void panthor_mmu_irq_handler(struct panthor_irq *pirq, u32 status)
 		 */
 		gpu_write(mmu->irq.iomem, INT_CLEAR, mask);
 
-		if (ptdev->mmu->as.slots[as].vm)
-			ptdev->mmu->as.slots[as].vm->unhandled_fault = true;
+		if (ptdev->mmu->as.slots[as].as)
+			ptdev->mmu->as.slots[as].as->unhandled_fault = true;
 
 		/* Disable the MMU to kill jobs on this AS. */
 		panthor_mmu_as_disable(ptdev, as, false);
@@ -1942,12 +1968,12 @@ void panthor_mmu_suspend(struct panthor_device *ptdev)
 {
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
-		struct panthor_vm *vm = ptdev->mmu->as.slots[i].vm;
+		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
 
-		if (vm) {
+		if (as) {
 			drm_WARN_ON(&ptdev->base,
 				    panthor_mmu_as_disable(ptdev, i, false));
-			panthor_vm_release_as_locked(vm);
+			panthor_as_release_hw_slot_locked(as);
 		}
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
@@ -2017,10 +2043,10 @@ void panthor_mmu_post_reset(struct panthor_device *ptdev)
 	ptdev->mmu->as.faulty_mask = 0;
 
 	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
-		struct panthor_vm *vm = ptdev->mmu->as.slots[i].vm;
+		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
 
-		if (vm)
-			panthor_vm_release_as_locked(vm);
+		if (as)
+			panthor_as_release_hw_slot_locked(as);
 	}
 
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
@@ -2036,15 +2062,24 @@ void panthor_mmu_post_reset(struct panthor_device *ptdev)
 	mutex_unlock(&ptdev->mmu->vm.lock);
 }
 
-static void panthor_vm_free(struct drm_gpuvm *gpuvm)
+static void panthor_vm_release(struct kref *kref)
 {
-	struct panthor_vm *vm = container_of(gpuvm, struct panthor_vm, base);
-	struct panthor_device *ptdev = vm->ptdev;
+	struct panthor_vm *vm = container_of(kref, struct panthor_vm, refcount);
+	struct panthor_as *as = vm->as;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 
+	/* Make sure the page table behind this VM doesn't participate in reclaim
+	 * after that point, since we're about to release everything anyway.
+	 */
 	mutex_lock(&ptdev->base.gem_lru_mutex);
-	list_del_init(&vm->reclaim.lru_node);
+	as->reclaim.skip = true;
+	list_del_init(&as->reclaim.lru_node);
 	mutex_unlock(&ptdev->base.gem_lru_mutex);
 
+	/* Unmap everything in case some BOs were still mapped. */
+	drm_WARN_ON(&ptdev->base,
+		    panthor_vm_unmap_range(vm, as->base.mm_start, as->base.mm_range));
+
 	mutex_lock(&vm->heaps.lock);
 	if (drm_WARN_ON(&ptdev->base, vm->heaps.pool))
 		panthor_heap_pool_destroy(vm->heaps.pool);
@@ -2065,29 +2100,26 @@ static void panthor_vm_free(struct drm_gpuvm *gpuvm)
 	drm_sched_entity_destroy(&vm->entity);
 	drm_sched_fini(&vm->sched);
 
-	mutex_lock(&vm->op_lock);
+	mutex_lock(&vm->as->op_lock);
 	mutex_lock(&ptdev->mmu->as.slots_lock);
-	if (vm->as.id >= 0) {
+	if (as->hw_slot.id >= 0) {
 		int cookie;
 
 		if (drm_dev_enter(&ptdev->base, &cookie)) {
-			panthor_mmu_as_disable(ptdev, vm->as.id, false);
+			panthor_mmu_as_disable(ptdev, as->hw_slot.id, false);
 			drm_dev_exit(cookie);
 		}
 
-		ptdev->mmu->as.slots[vm->as.id].vm = NULL;
-		clear_bit(vm->as.id, &ptdev->mmu->as.alloc_mask);
-		list_del(&vm->as.lru_node);
+		panthor_as_release_hw_slot_locked(as);
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
-	mutex_unlock(&vm->op_lock);
-
-	free_io_pgtable_ops(vm->pgtbl_ops);
+	mutex_unlock(&vm->as->op_lock);
 
 	if (vm->dummy)
 		drm_gem_object_put(&vm->dummy->base);
 
 	drm_mm_takedown(&vm->mm);
+	drm_gpuvm_put(&as->base);
 	kfree(vm);
 }
 
@@ -2097,7 +2129,8 @@ static void panthor_vm_free(struct drm_gpuvm *gpuvm)
  */
 void panthor_vm_put(struct panthor_vm *vm)
 {
-	drm_gpuvm_put(vm ? &vm->base : NULL);
+	if (vm)
+		kref_put(&vm->refcount, panthor_vm_release);
 }
 
 /**
@@ -2109,7 +2142,7 @@ void panthor_vm_put(struct panthor_vm *vm)
 struct panthor_vm *panthor_vm_get(struct panthor_vm *vm)
 {
 	if (vm)
-		drm_gpuvm_get(&vm->base);
+		kref_get(&vm->refcount);
 
 	return vm;
 }
@@ -2130,6 +2163,8 @@ struct panthor_vm *panthor_vm_get(struct panthor_vm *vm)
  */
 struct panthor_heap_pool *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool create)
 {
+	struct panthor_device *ptdev = container_of(vm->as->base.drm,
+						    struct panthor_device, base);
 	struct panthor_heap_pool *pool;
 
 	mutex_lock(&vm->heaps.lock);
@@ -2137,7 +2172,7 @@ struct panthor_heap_pool *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool c
 		if (vm->destroyed)
 			pool = ERR_PTR(-EINVAL);
 		else
-			pool = panthor_heap_pool_create(vm->ptdev, vm);
+			pool = panthor_heap_pool_create(ptdev, vm);
 
 		if (!IS_ERR(pool))
 			vm->heaps.pool = panthor_heap_pool_get(pool);
@@ -2172,7 +2207,7 @@ void panthor_vm_heaps_sizes(struct panthor_file *pfile, struct drm_memory_stats
 	xa_for_each(&pfile->vms->xa, i, vm) {
 		size_t size = panthor_heap_pool_size(vm->heaps.pool);
 		stats->resident += size;
-		if (vm->as.id >= 0)
+		if (vm->as->hw_slot.id >= 0)
 			stats->active += size;
 	}
 	xa_unlock(&pfile->vms->xa);
@@ -2220,8 +2255,7 @@ static u64 mair_to_memattr(u64 mair, bool coherent)
 	return memattr;
 }
 
-static void panthor_vma_link(struct panthor_vm *vm,
-			     struct panthor_vma *vma,
+static void panthor_vma_link(struct panthor_vma *vma,
 			     struct drm_gpuvm_bo *vm_bo)
 {
 	struct panthor_gem_object *bo = to_panthor_bo(vma->base.gem.obj);
@@ -2257,25 +2291,25 @@ panthor_fix_sparse_map_offset(struct drm_gpuva_op_map *op, u32 flags)
 }
 
 static int
-panthor_vm_exec_map_op(struct panthor_vm *vm, u32 flags,
+panthor_as_exec_map_op(struct panthor_as *as, u32 flags,
 		       const struct drm_gpuva_op_map *op)
 {
 	struct panthor_gem_object *bo = to_panthor_bo(op->gem.obj);
 	int prot = flags_to_prot(flags);
 
 	if (flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE)
-		return panthor_vm_map_sparse(vm, op->va.addr, prot,
+		return panthor_as_map_sparse(as, op->va.addr, prot,
 					     bo->dmap.sgt, op->va.range);
 
-	return panthor_vm_map_pages(vm, op->va.addr, prot, bo->dmap.sgt,
+	return panthor_as_map_pages(as, op->va.addr, prot, bo->dmap.sgt,
 				    op->gem.offset, op->va.range);
 }
 
 static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
 {
-	struct panthor_vm *vm = priv;
-	struct panthor_vm_op_ctx *op_ctx = vm->op_ctx;
-	struct panthor_vma *vma = panthor_vm_op_ctx_get_vma(op_ctx);
+	struct panthor_as *as = priv;
+	struct panthor_as_op_ctx *op_ctx = as->op_ctx;
+	struct panthor_vma *vma = panthor_as_op_ctx_get_vma(op_ctx);
 	int ret;
 
 	if (!vma)
@@ -2284,14 +2318,14 @@ static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
 	panthor_vma_init(vma, op_ctx->flags & PANTHOR_VM_MAP_FLAGS);
 	panthor_fix_sparse_map_offset(&op->map, vma->flags);
 
-	ret = panthor_vm_exec_map_op(vm, vma->flags, &op->map);
+	ret = panthor_as_exec_map_op(as, vma->flags, &op->map);
 	if (ret) {
-		panthor_vm_op_ctx_return_vma(op_ctx, vma);
+		panthor_as_op_ctx_return_vma(op_ctx, vma);
 		return ret;
 	}
 
-	drm_gpuva_map(&vm->base, &vma->base, &op->map);
-	panthor_vma_link(vm, vma, op_ctx->map.vm_bo);
+	drm_gpuva_map(&as->base, &vma->base, &op->map);
+	panthor_vma_link(vma, op_ctx->map.vm_bo);
 
 	drm_gpuvm_bo_put_deferred(op_ctx->map.vm_bo);
 	op_ctx->map.vm_bo = NULL;
@@ -2352,8 +2386,8 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 				       void *priv)
 {
 	struct panthor_vma *unmap_vma = container_of(op->remap.unmap->va, struct panthor_vma, base);
-	struct panthor_vm *vm = priv;
-	struct panthor_vm_op_ctx *op_ctx = vm->op_ctx;
+	struct panthor_as *as = priv;
+	struct panthor_as_op_ctx *op_ctx = as->op_ctx;
 	struct panthor_vma *prev_vma = NULL, *next_vma = NULL;
 	u64 unmap_start, unmap_range;
 	int ret;
@@ -2379,8 +2413,8 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 		 * atomicity. panthor_vm_lock_region() bails out early if the new region
 		 * is already part of the locked region, so no need to do this check here.
 		 */
-		panthor_vm_lock_region(vm, unmap_start, unmap_range);
-		panthor_vm_unmap_pages(vm, unmap_start, unmap_range);
+		panthor_as_lock_region(as, unmap_start, unmap_range);
+		panthor_as_unmap_pages(as, unmap_start, unmap_range);
 	}
 
 	if (op->remap.prev) {
@@ -2396,12 +2430,12 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 			};
 			panthor_fix_sparse_map_offset(&map_op, unmap_vma->flags);
 
-			ret = panthor_vm_exec_map_op(vm, unmap_vma->flags, &map_op);
+			ret = panthor_as_exec_map_op(as, unmap_vma->flags, &map_op);
 			if (ret)
 				return ret;
 		}
 
-		prev_vma = panthor_vm_op_ctx_get_vma(op_ctx);
+		prev_vma = panthor_as_op_ctx_get_vma(op_ctx);
 		panthor_vma_init(prev_vma, unmap_vma->flags);
 		prev_vma->evicted = unmap_vma->evicted;
 	}
@@ -2419,12 +2453,12 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 			};
 			panthor_fix_sparse_map_offset(&map_op, unmap_vma->flags);
 
-			ret = panthor_vm_exec_map_op(vm, unmap_vma->flags, &map_op);
+			ret = panthor_as_exec_map_op(as, unmap_vma->flags, &map_op);
 			if (ret)
 				return ret;
 		}
 
-		next_vma = panthor_vm_op_ctx_get_vma(op_ctx);
+		next_vma = panthor_as_op_ctx_get_vma(op_ctx);
 		panthor_vma_init(next_vma, unmap_vma->flags);
 		next_vma->evicted = unmap_vma->evicted;
 	}
@@ -2439,11 +2473,11 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 		 * owned by the old mapping which will be released when this
 		 * mapping is destroyed, we need to grab a ref here.
 		 */
-		panthor_vma_link(vm, prev_vma, op->remap.unmap->va->vm_bo);
+		panthor_vma_link(prev_vma, op->remap.unmap->va->vm_bo);
 	}
 
 	if (next_vma) {
-		panthor_vma_link(vm, next_vma, op->remap.unmap->va->vm_bo);
+		panthor_vma_link(next_vma, op->remap.unmap->va->vm_bo);
 	}
 
 	panthor_vma_unlink(unmap_vma);
@@ -2454,10 +2488,10 @@ static int panthor_gpuva_sm_step_unmap(struct drm_gpuva_op *op,
 				       void *priv)
 {
 	struct panthor_vma *unmap_vma = container_of(op->unmap.va, struct panthor_vma, base);
-	struct panthor_vm *vm = priv;
+	struct panthor_as *as = priv;
 
 	if (!unmap_vma->evicted) {
-		panthor_vm_unmap_pages(vm, unmap_vma->base.va.addr,
+		panthor_as_unmap_pages(as, unmap_vma->base.va.addr,
 				       unmap_vma->base.va.range);
 	}
 
@@ -2469,7 +2503,7 @@ static int panthor_gpuva_sm_step_unmap(struct drm_gpuva_op *op,
 void panthor_vm_update_bo_reclaim_lru_locked(struct panthor_gem_object *bo)
 {
 	struct panthor_device *ptdev = container_of(bo->base.dev, struct panthor_device, base);
-	struct panthor_vm *vm = NULL;
+	struct panthor_as *as = NULL;
 	struct drm_gpuvm_bo *vm_bo;
 
 	dma_resv_assert_held(bo->base.resv);
@@ -2482,27 +2516,34 @@ void panthor_vm_update_bo_reclaim_lru_locked(struct panthor_gem_object *bo)
 		/* We're only supposed to have one non-evicted vm_bo in the list if we get
 		 * there.
 		 */
-		drm_WARN_ON(&ptdev->base, vm);
-		vm = container_of(vm_bo->vm, struct panthor_vm, base);
+		drm_WARN_ON(&ptdev->base, as);
+		as = container_of(vm_bo->vm, struct panthor_as, base);
 
 		mutex_lock(&ptdev->base.gem_lru_mutex);
-		drm_gem_lru_move_tail_locked(&vm->reclaim.lru, &bo->base);
-		if (list_empty(&vm->reclaim.lru_node))
-			list_move(&vm->reclaim.lru_node, &ptdev->reclaim.vms);
+		drm_gem_lru_move_tail_locked(&as->reclaim.lru, &bo->base);
+		if (list_empty(&as->reclaim.lru_node) && !as->reclaim.skip)
+			list_move(&as->reclaim.lru_node, &ptdev->reclaim.vms);
 		mutex_unlock(&ptdev->base.gem_lru_mutex);
 	}
 }
 
 int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
 {
+	struct panthor_device *ptdev = container_of(bo->base.dev, struct panthor_device, base);
 	struct drm_gpuvm_bo *vm_bo;
 	int ret = 0;
 
 	drm_gem_for_each_gpuvm_bo(vm_bo, &bo->base) {
-		struct panthor_vm *vm = container_of(vm_bo->vm, struct panthor_vm, base);
+		struct panthor_as *as = container_of(vm_bo->vm, struct panthor_as, base);
 		struct drm_gpuva *va;
 
-		if (!mutex_trylock(&vm->op_lock))
+		scoped_guard(mutex, &ptdev->base.gem_lru_mutex) {
+			/* The VM is going away, skip this BO and get back to it later. */
+			if (!as->reclaim.skip)
+				return -EAGAIN;
+		}
+
+		if (!mutex_trylock(&as->op_lock))
 			return -EDEADLK;
 
 		/* It can be that the vm_bo was already evicted but a new
@@ -2531,16 +2572,16 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
 			 * will be validated, causing all its evicted VMAs to be repopulated
 			 * before the job runs. So no GPU fault expected.
 			 */
-			ret = panthor_vm_lock_region(vm, va->va.addr, va->va.range);
+			ret = panthor_as_lock_region(as, va->va.addr, va->va.range);
 			if (ret)
 				break;
 
-			panthor_vm_unmap_pages(vm, va->va.addr, va->va.range);
-			panthor_vm_unlock_region(vm);
+			panthor_as_unmap_pages(as, va->va.addr, va->va.range);
+			panthor_as_unlock_region(as);
 			vma->evicted = true;
 		}
 
-		mutex_unlock(&vm->op_lock);
+		mutex_unlock(&as->op_lock);
 
 		if (ret)
 			break;
@@ -2550,14 +2591,14 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
 }
 
 static struct panthor_vma *select_evicted_vma(struct drm_gpuvm_bo *vm_bo,
-					      struct panthor_vm_op_ctx *op_ctx)
+					      struct panthor_as_op_ctx *op_ctx)
 {
-	struct panthor_vm *vm = container_of(vm_bo->vm, struct panthor_vm, base);
+	struct panthor_as *as = container_of(vm_bo->vm, struct panthor_as, base);
 	struct panthor_vma *first_evicted_vma = NULL;
 	struct drm_gpuva *va;
 
 	/* Take op_lock to protect against va insertion/removal. */
-	mutex_lock(&vm->op_lock);
+	mutex_lock(&as->op_lock);
 	drm_gpuvm_bo_for_each_va(va, vm_bo) {
 		struct panthor_vma *vma = container_of(va, struct panthor_vma, base);
 
@@ -2568,22 +2609,23 @@ static struct panthor_vma *select_evicted_vma(struct drm_gpuvm_bo *vm_bo,
 			break;
 		}
 	}
-	mutex_unlock(&vm->op_lock);
+	mutex_unlock(&as->op_lock);
 
 	return first_evicted_vma;
 }
 
 static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
 			     struct panthor_vma *evicted_vma,
-			     struct panthor_vm_op_ctx *op_ctx)
+			     struct panthor_as_op_ctx *op_ctx)
 {
-	struct panthor_vm *vm = container_of(vm_bo->vm, struct panthor_vm, base);
+	struct panthor_as *as = container_of(vm_bo->vm, struct panthor_as, base);
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	struct panthor_gem_object *bo = to_panthor_bo(vm_bo->obj);
 	struct drm_gpuva *va;
 	bool found = false;
 	int ret;
 
-	ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx);
+	ret = panthor_as_op_ctx_prealloc_pts(ptdev, op_ctx);
 	if (ret)
 		goto out_cleanup;
 
@@ -2592,7 +2634,7 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
 	 * to release it so we can allocate PTs, because this very same lock
 	 * is taken in a DMA-signalling path.
 	 */
-	mutex_lock(&vm->op_lock);
+	mutex_lock(&as->op_lock);
 	drm_gpuvm_bo_for_each_va(va, vm_bo) {
 		struct panthor_vma *vma = container_of(va, struct panthor_vma, base);
 
@@ -2612,8 +2654,8 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
 	}
 
 	if (found) {
-		vm->op_ctx = op_ctx;
-		ret = panthor_vm_lock_region(vm, evicted_vma->base.va.addr,
+		as->op_ctx = op_ctx;
+		ret = panthor_as_lock_region(as, evicted_vma->base.va.addr,
 					     evicted_vma->base.va.range);
 		if (!ret) {
 			struct drm_gpuva_op_map map_op = {
@@ -2622,34 +2664,37 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
 				.gem.obj = &bo->base,
 				.gem.offset = evicted_vma->base.gem.offset,
 			};
-			if (evicted_vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE)
-				drm_WARN_ON_ONCE(&vm->ptdev->base, map_op.gem.offset !=
-						 (map_op.va.addr & (SZ_2M - 1)));
+			if (evicted_vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE) {
+				u64 expected_offset = map_op.va.addr & (SZ_2M - 1);
 
-			ret = panthor_vm_exec_map_op(vm, evicted_vma->flags, &map_op);
+				drm_WARN_ON_ONCE(as->base.drm,
+						 map_op.gem.offset != expected_offset);
+			}
+
+			ret = panthor_as_exec_map_op(as, evicted_vma->flags, &map_op);
 			if (!ret)
 				evicted_vma->evicted = false;
 
-			panthor_vm_unlock_region(vm);
+			panthor_as_unlock_region(as);
 		}
 
-		vm->op_ctx = NULL;
+		as->op_ctx = NULL;
 	}
 
-	mutex_unlock(&vm->op_lock);
+	mutex_unlock(&as->op_lock);
 
 out_cleanup:
-	panthor_vm_cleanup_op_ctx(op_ctx, vm);
+	panthor_as_cleanup_op_ctx(op_ctx, as);
 	return ret;
 }
 
 static int panthor_vm_restore_vmas(struct drm_gpuvm_bo *vm_bo)
 {
-	struct panthor_vm *vm = container_of(vm_bo->vm, struct panthor_vm, base);
+	struct panthor_as *as = container_of(vm_bo->vm, struct panthor_as, base);
 	struct panthor_gem_object *bo = to_panthor_bo(vm_bo->obj);
-	struct panthor_vm_op_ctx op_ctx;
+	struct panthor_as_op_ctx op_ctx;
 
-	if (drm_WARN_ON_ONCE(&vm->ptdev->base, !bo->dmap.sgt))
+	if (drm_WARN_ON_ONCE(as->base.drm, !bo->dmap.sgt))
 		return -EINVAL;
 
 	for (struct panthor_vma *vma = select_evicted_vma(vm_bo, &op_ctx);
@@ -2685,8 +2730,19 @@ static int panthor_vm_bo_validate(struct drm_gpuvm_bo *vm_bo,
 	return 0;
 }
 
+static void panthor_as_free(struct drm_gpuvm *gpuvm)
+{
+	struct panthor_as *as = container_of(gpuvm, struct panthor_as, base);
+
+	if (as->pt.ops)
+		free_io_pgtable_ops(as->pt.ops);
+
+	mutex_destroy(&as->op_lock);
+	kfree(as);
+}
+
 static const struct drm_gpuvm_ops panthor_gpuvm_ops = {
-	.vm_free = panthor_vm_free,
+	.vm_free = panthor_as_free,
 	.vm_bo_free = panthor_vm_bo_free,
 	.sm_step_map = panthor_gpuva_sm_step_map,
 	.sm_step_remap = panthor_gpuva_sm_step_remap,
@@ -2702,7 +2758,7 @@ static const struct drm_gpuvm_ops panthor_gpuvm_ops = {
  */
 struct dma_resv *panthor_vm_resv(struct panthor_vm *vm)
 {
-	return drm_gpuvm_resv(&vm->base);
+	return drm_gpuvm_resv(&vm->as->base);
 }
 
 struct drm_gem_object *panthor_vm_root_gem(struct panthor_vm *vm)
@@ -2710,12 +2766,12 @@ struct drm_gem_object *panthor_vm_root_gem(struct panthor_vm *vm)
 	if (!vm)
 		return NULL;
 
-	return vm->base.r_obj;
+	return vm->as->base.r_obj;
 }
 
-static int
-panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
-		   bool flag_vm_unusable_on_failure)
+static int panthor_as_exec_op(struct panthor_as *as,
+			      struct panthor_as_op_ctx *op,
+			      bool flag_vm_unusable_on_failure)
 {
 	u32 op_type = op->flags & DRM_PANTHOR_VM_BIND_OP_TYPE_MASK;
 	int ret;
@@ -2723,10 +2779,10 @@ panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
 	if (op_type == DRM_PANTHOR_VM_BIND_OP_TYPE_SYNC_ONLY)
 		return 0;
 
-	mutex_lock(&vm->op_lock);
-	vm->op_ctx = op;
+	mutex_lock(&as->op_lock);
+	as->op_ctx = op;
 
-	ret = panthor_vm_lock_region(vm, op->va.addr, op->va.range);
+	ret = panthor_as_lock_region(as, op->va.addr, op->va.range);
 	if (ret)
 		goto out;
 
@@ -2739,17 +2795,17 @@ panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
 			.map.gem.offset = op->map.bo_offset,
 		};
 
-		if (vm->unusable) {
+		if (as->unusable) {
 			ret = -EINVAL;
 			break;
 		}
 
-		ret = drm_gpuvm_sm_map(&vm->base, vm, &map_req);
+		ret = drm_gpuvm_sm_map(&as->base, as, &map_req);
 		break;
 	}
 
 	case DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP:
-		ret = drm_gpuvm_sm_unmap(&vm->base, vm, op->va.addr, op->va.range);
+		ret = drm_gpuvm_sm_unmap(&as->base, as, op->va.addr, op->va.range);
 		break;
 
 	default:
@@ -2757,14 +2813,14 @@ panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
 		break;
 	}
 
-	panthor_vm_unlock_region(vm);
+	panthor_as_unlock_region(as);
 
 out:
 	if (ret && flag_vm_unusable_on_failure)
-		panthor_vm_declare_unusable(vm);
+		panthor_as_declare_unusable(as);
 
-	vm->op_ctx = NULL;
-	mutex_unlock(&vm->op_lock);
+	as->op_ctx = NULL;
+	mutex_unlock(&as->op_lock);
 
 	return ret;
 }
@@ -2782,7 +2838,7 @@ panthor_vm_bind_run_job(struct drm_sched_job *sched_job)
 	 * to be destroyed and recreated.
 	 */
 	cookie = dma_fence_begin_signalling();
-	ret = panthor_vm_exec_op(job->vm, &job->ctx, true);
+	ret = panthor_as_exec_op(job->vm->as, &job->ctx, true);
 	dma_fence_end_signalling(cookie);
 
 	return ret ? ERR_PTR(ret) : NULL;
@@ -2795,7 +2851,7 @@ static void panthor_vm_bind_job_release(struct kref *kref)
 	if (job->base.s_fence)
 		drm_sched_job_cleanup(&job->base);
 
-	panthor_vm_cleanup_op_ctx(&job->ctx, job->vm);
+	panthor_as_cleanup_op_ctx(&job->ctx, job->vm->as);
 	panthor_vm_put(job->vm);
 	kfree(job);
 }
@@ -2818,13 +2874,15 @@ panthor_vm_bind_free_job(struct drm_sched_job *sched_job)
 {
 	struct panthor_vm_bind_job *job =
 		container_of(sched_job, struct panthor_vm_bind_job, base);
+	struct panthor_device *ptdev =
+		 container_of(job->vm->as->base.drm, struct panthor_device, base);
 
 	drm_sched_job_cleanup(sched_job);
 
 	/* Do the heavy cleanups asynchronously, so we're out of the
 	 * dma-signaling path and can acquire dma-resv locks safely.
 	 */
-	queue_work(job->vm->ptdev->cleanup_wq, &job->cleanup_op_ctx_work);
+	queue_work(ptdev->cleanup_wq, &job->cleanup_op_ctx_work);
 }
 
 static enum drm_gpu_sched_stat
@@ -2840,6 +2898,62 @@ static const struct drm_sched_backend_ops panthor_vm_bind_ops = {
 	.timedout_job = panthor_vm_bind_timedout_job,
 };
 
+static struct panthor_as *
+panthor_as_create(struct panthor_device *ptdev, const char *name,
+		  u64 min_va, u64 va_range)
+{
+	struct io_pgtable_cfg as_cfg = {
+		.pgsize_bitmap	= ptdev->mmu_info.page_size_bitmap,
+		.ias		= GPU_MMU_FEATURES_VA_BITS(ptdev->gpu_info.mmu_features),
+		.oas		= GPU_MMU_FEATURES_PA_BITS(ptdev->gpu_info.mmu_features),
+		.coherent_walk	= ptdev->coherent,
+		.tlb		= &mmu_tlb_ops,
+		.iommu_dev	= drm_dev_dma_dev(&ptdev->base),
+		.alloc		= alloc_pt,
+		.free		= free_pt,
+	};
+	struct drm_gem_object *dummy_gem;
+	struct panthor_as *as;
+	u64 mair;
+
+	/* We allocate a dummy GEM for the VM. */
+	dummy_gem = drm_gpuvm_resv_object_alloc(&ptdev->base);
+	if (!dummy_gem)
+		return ERR_PTR(-ENOMEM);
+
+	as = kzalloc_obj(*as);
+	if (!as) {
+		drm_gem_object_put(dummy_gem);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	mutex_init(&as->op_lock);
+	drm_gem_lru_init(&as->reclaim.lru);
+	INIT_LIST_HEAD(&as->reclaim.lru_node);
+	INIT_LIST_HEAD(&as->hw_slot.lru_node);
+	as->hw_slot.id = -1;
+	refcount_set(&as->active_cnt, 0);
+
+	/* We intentionally leave the reserved range to zero, because we want kernel VMAs
+	 * to be handled the same way user VMAs are.
+	 */
+	drm_gpuvm_init(&as->base, name,
+		       DRM_GPUVM_RESV_PROTECTED | DRM_GPUVM_IMMEDIATE_MODE,
+		       &ptdev->base, dummy_gem, min_va, va_range, 0, 0,
+		       &panthor_gpuvm_ops);
+	drm_gem_object_put(dummy_gem);
+
+	as->pt.ops = alloc_io_pgtable_ops(ARM_64_LPAE_S1, &as_cfg, as);
+	if (!as->pt.ops) {
+		drm_gpuvm_put(&as->base);
+		return ERR_PTR(-EINVAL);
+	}
+
+	mair = io_pgtable_ops_to_pgtable(as->pt.ops)->cfg.arm_lpae_s1_cfg.mair;
+	as->memattr = mair_to_memattr(mair, ptdev->coherent);
+	return as;
+}
+
 /**
  * panthor_vm_create() - Create a VM
  * @ptdev: Device.
@@ -2857,9 +2971,8 @@ panthor_vm_create(struct panthor_device *ptdev, bool for_mcu,
 		  u64 auto_kernel_va_start, u64 auto_kernel_va_size)
 {
 	u32 va_bits = GPU_MMU_FEATURES_VA_BITS(ptdev->gpu_info.mmu_features);
-	u32 pa_bits = GPU_MMU_FEATURES_PA_BITS(ptdev->gpu_info.mmu_features);
+	const char *name = for_mcu ? "panthor-MCU-VM" : "panthor-GPU-VM";
 	u64 full_va_range = 1ull << va_bits;
-	struct drm_gem_object *dummy_gem;
 	struct drm_gpu_scheduler *sched;
 	const struct drm_sched_init_args sched_args = {
 		.ops = &panthor_vm_bind_ops,
@@ -2870,27 +2983,11 @@ panthor_vm_create(struct panthor_device *ptdev, bool for_mcu,
 		.name = "panthor-vm-bind",
 		.dev = ptdev->base.dev,
 	};
-	struct io_pgtable_cfg pgtbl_cfg;
-	u64 mair, min_va, va_range;
+	struct panthor_as *as;
 	struct panthor_vm *vm;
+	u64 min_va, va_range;
 	int ret;
 
-	vm = kzalloc_obj(*vm);
-	if (!vm)
-		return ERR_PTR(-ENOMEM);
-
-	/* We allocate a dummy GEM for the VM. */
-	dummy_gem = drm_gpuvm_resv_object_alloc(&ptdev->base);
-	if (!dummy_gem) {
-		ret = -ENOMEM;
-		goto err_free_vm;
-	}
-
-	mutex_init(&vm->heaps.lock);
-	vm->for_mcu = for_mcu;
-	vm->ptdev = ptdev;
-	mutex_init(&vm->op_lock);
-
 	if (for_mcu) {
 		/* CSF MCU is a cortex M7, and can only address 4G */
 		min_va = 0;
@@ -2900,49 +2997,35 @@ panthor_vm_create(struct panthor_device *ptdev, bool for_mcu,
 		va_range = full_va_range;
 	}
 
+	as = panthor_as_create(ptdev, name, min_va, va_range);
+	if (IS_ERR(as))
+		return ERR_CAST(as);
+
+	vm = kzalloc_obj(*vm);
+	if (!vm) {
+		ret = -ENOMEM;
+		goto err_put_as;
+	}
+
 	vm->user_va_range = kernel_va_start;
+	vm->as = as;
+	mutex_init(&vm->heaps.lock);
+	vm->for_mcu = for_mcu;
 
 	mutex_init(&vm->mm_lock);
 	drm_mm_init(&vm->mm, kernel_va_start, kernel_va_size);
 	vm->kernel_auto_va.start = auto_kernel_va_start;
 	vm->kernel_auto_va.end = vm->kernel_auto_va.start + auto_kernel_va_size - 1;
 
-	drm_gem_lru_init(&vm->reclaim.lru);
-	INIT_LIST_HEAD(&vm->reclaim.lru_node);
-	INIT_LIST_HEAD(&vm->node);
-	INIT_LIST_HEAD(&vm->as.lru_node);
-	vm->as.id = -1;
-	refcount_set(&vm->as.active_cnt, 0);
-
-	pgtbl_cfg = (struct io_pgtable_cfg) {
-		.pgsize_bitmap	= ptdev->mmu_info.page_size_bitmap,
-		.ias		= va_bits,
-		.oas		= pa_bits,
-		.coherent_walk	= ptdev->coherent,
-		.tlb		= &mmu_tlb_ops,
-		.iommu_dev	= ptdev->base.dev,
-		.alloc		= alloc_pt,
-		.free		= free_pt,
-	};
-
-	vm->pgtbl_ops = alloc_io_pgtable_ops(ARM_64_LPAE_S1, &pgtbl_cfg, vm);
-	if (!vm->pgtbl_ops) {
-		ret = -EINVAL;
-		goto err_mm_takedown;
-	}
-
 	ret = drm_sched_init(&vm->sched, &sched_args);
 	if (ret)
-		goto err_free_io_pgtable;
+		goto err_free_vm;
 
 	sched = &vm->sched;
 	ret = drm_sched_entity_init(&vm->entity, 0, &sched, 1, NULL);
 	if (ret)
 		goto err_sched_fini;
 
-	mair = io_pgtable_ops_to_pgtable(vm->pgtbl_ops)->cfg.arm_lpae_s1_cfg.mair;
-	vm->memattr = mair_to_memattr(mair, ptdev->coherent);
-
 	mutex_lock(&ptdev->mmu->vm.lock);
 	list_add_tail(&vm->node, &ptdev->mmu->vm.list);
 
@@ -2951,28 +3034,20 @@ panthor_vm_create(struct panthor_device *ptdev, bool for_mcu,
 		panthor_vm_stop(vm);
 	mutex_unlock(&ptdev->mmu->vm.lock);
 
-	/* We intentionally leave the reserved range to zero, because we want kernel VMAs
-	 * to be handled the same way user VMAs are.
-	 */
-	drm_gpuvm_init(&vm->base, for_mcu ? "panthor-MCU-VM" : "panthor-GPU-VM",
-		       DRM_GPUVM_RESV_PROTECTED | DRM_GPUVM_IMMEDIATE_MODE,
-		       &ptdev->base, dummy_gem, min_va, va_range, 0, 0,
-		       &panthor_gpuvm_ops);
-	drm_gem_object_put(dummy_gem);
+	kref_init(&vm->refcount);
 	return vm;
 
 err_sched_fini:
 	drm_sched_fini(&vm->sched);
 
-err_free_io_pgtable:
-	free_io_pgtable_ops(vm->pgtbl_ops);
-
-err_mm_takedown:
-	drm_mm_takedown(&vm->mm);
-	drm_gem_object_put(dummy_gem);
-
 err_free_vm:
+	drm_mm_takedown(&vm->mm);
+	mutex_destroy(&vm->mm_lock);
+	mutex_destroy(&vm->heaps.lock);
 	kfree(vm);
+
+err_put_as:
+	drm_gpuvm_put(&as->base);
 	return ERR_PTR(ret);
 }
 
@@ -2980,7 +3055,7 @@ static int
 panthor_vm_bind_prepare_op_ctx(struct drm_file *file,
 			       struct panthor_vm *vm,
 			       const struct drm_panthor_vm_bind_op *op,
-			       struct panthor_vm_op_ctx *op_ctx)
+			       struct panthor_as_op_ctx *op_ctx)
 {
 	ssize_t vm_pgsz = panthor_vm_page_size(vm);
 	struct drm_gem_object *gem;
@@ -3003,7 +3078,7 @@ panthor_vm_bind_prepare_op_ctx(struct drm_file *file,
 			drm_gem_object_get(&vm->dummy->base);
 		}
 
-		ret = panthor_vm_prepare_map_op_ctx(op_ctx, vm,
+		ret = panthor_as_prepare_map_op_ctx(op_ctx, vm->as,
 						    gem ? to_panthor_bo(gem) : NULL,
 						    op);
 		drm_gem_object_put(gem);
@@ -3016,7 +3091,7 @@ panthor_vm_bind_prepare_op_ctx(struct drm_file *file,
 		if (op->bo_handle || op->bo_offset)
 			return -EINVAL;
 
-		return panthor_vm_prepare_unmap_op_ctx(op_ctx, vm, op->va, op->size);
+		return panthor_as_prepare_unmap_op_ctx(op_ctx, vm->as, op->va, op->size);
 
 	case DRM_PANTHOR_VM_BIND_OP_TYPE_SYNC_ONLY:
 		if (op->flags & ~DRM_PANTHOR_VM_BIND_OP_TYPE_MASK)
@@ -3031,7 +3106,7 @@ panthor_vm_bind_prepare_op_ctx(struct drm_file *file,
 		if (!op->syncs.count)
 			return -EINVAL;
 
-		panthor_vm_prepare_sync_only_op_ctx(op_ctx);
+		panthor_as_prepare_sync_only_op_ctx(op_ctx);
 		return 0;
 
 	default:
@@ -3066,7 +3141,7 @@ panthor_vm_bind_job_create(struct drm_file *file,
 	if (!vm)
 		return ERR_PTR(-EINVAL);
 
-	if (vm->destroyed || vm->unusable)
+	if (vm->destroyed || vm->as->unusable)
 		return ERR_PTR(-EINVAL);
 
 	job = kzalloc_obj(*job);
@@ -3112,7 +3187,7 @@ int panthor_vm_bind_job_prepare_resvs(struct drm_exec *exec,
 	int ret;
 
 	/* Acquire the VM lock an reserve a slot for this VM bind job. */
-	ret = drm_gpuvm_prepare_vm(&job->vm->base, exec, 1);
+	ret = drm_gpuvm_prepare_vm(&job->vm->as->base, exec, 1);
 	if (ret)
 		return ret;
 
@@ -3137,7 +3212,7 @@ void panthor_vm_bind_job_update_resvs(struct drm_exec *exec,
 	struct panthor_vm_bind_job *job = container_of(sched_job, struct panthor_vm_bind_job, base);
 
 	/* Explicit sync => we just register our job finished fence as bookkeep. */
-	drm_gpuvm_resv_add_fence(&job->vm->base, exec,
+	drm_gpuvm_resv_add_fence(&job->vm->as->base, exec,
 				 &sched_job->s_fence->finished,
 				 DMA_RESV_USAGE_BOOKKEEP,
 				 DMA_RESV_USAGE_BOOKKEEP);
@@ -3148,7 +3223,7 @@ void panthor_vm_update_resvs(struct panthor_vm *vm, struct drm_exec *exec,
 			     enum dma_resv_usage private_usage,
 			     enum dma_resv_usage extobj_usage)
 {
-	drm_gpuvm_resv_add_fence(&vm->base, exec, fence, private_usage, extobj_usage);
+	drm_gpuvm_resv_add_fence(&vm->as->base, exec, fence, private_usage, extobj_usage);
 }
 
 /**
@@ -3163,7 +3238,7 @@ int panthor_vm_bind_exec_sync_op(struct drm_file *file,
 				 struct panthor_vm *vm,
 				 struct drm_panthor_vm_bind_op *op)
 {
-	struct panthor_vm_op_ctx op_ctx;
+	struct panthor_as_op_ctx op_ctx;
 	int ret;
 
 	/* No sync objects allowed on synchronous operations. */
@@ -3177,8 +3252,8 @@ int panthor_vm_bind_exec_sync_op(struct drm_file *file,
 	if (ret)
 		return ret;
 
-	ret = panthor_vm_exec_op(vm, &op_ctx, false);
-	panthor_vm_cleanup_op_ctx(&op_ctx, vm);
+	ret = panthor_as_exec_op(vm->as, &op_ctx, false);
+	panthor_as_cleanup_op_ctx(&op_ctx, vm->as);
 
 	return ret;
 }
@@ -3207,18 +3282,18 @@ int panthor_vm_map_bo_range(struct panthor_vm *vm, struct panthor_gem_object *bo
 		.va = va,
 		.flags = flags,
 	};
-	struct panthor_vm_op_ctx op_ctx;
+	struct panthor_as_op_ctx op_ctx;
 	int ret;
 
-	if (drm_WARN_ON(&vm->ptdev->base, flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE))
+	if (drm_WARN_ON(vm->as->base.drm, flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE))
 		return -EINVAL;
 
-	ret = panthor_vm_prepare_map_op_ctx(&op_ctx, vm, bo, &op);
+	ret = panthor_as_prepare_map_op_ctx(&op_ctx, vm->as, bo, &op);
 	if (ret)
 		return ret;
 
-	ret = panthor_vm_exec_op(vm, &op_ctx, false);
-	panthor_vm_cleanup_op_ctx(&op_ctx, vm);
+	ret = panthor_as_exec_op(vm->as, &op_ctx, false);
+	panthor_as_cleanup_op_ctx(&op_ctx, vm->as);
 
 	return ret;
 }
@@ -3236,15 +3311,15 @@ int panthor_vm_map_bo_range(struct panthor_vm *vm, struct panthor_gem_object *bo
  */
 int panthor_vm_unmap_range(struct panthor_vm *vm, u64 va, u64 size)
 {
-	struct panthor_vm_op_ctx op_ctx;
+	struct panthor_as_op_ctx op_ctx;
 	int ret;
 
-	ret = panthor_vm_prepare_unmap_op_ctx(&op_ctx, vm, va, size);
+	ret = panthor_as_prepare_unmap_op_ctx(&op_ctx, vm->as, va, size);
 	if (ret)
 		return ret;
 
-	ret = panthor_vm_exec_op(vm, &op_ctx, false);
-	panthor_vm_cleanup_op_ctx(&op_ctx, vm);
+	ret = panthor_as_exec_op(vm->as, &op_ctx, false);
+	panthor_as_cleanup_op_ctx(&op_ctx, vm->as);
 
 	return ret;
 }
@@ -3268,15 +3343,15 @@ int panthor_vm_prepare_mapped_bos_resvs(struct drm_exec *exec, struct panthor_vm
 	int ret;
 
 	/* Acquire the VM lock and reserve a slot for this GPU job. */
-	ret = drm_gpuvm_prepare_vm(&vm->base, exec, slot_count);
+	ret = drm_gpuvm_prepare_vm(&vm->as->base, exec, slot_count);
 	if (ret)
 		return ret;
 
-	ret = drm_gpuvm_prepare_objects(&vm->base, exec, slot_count);
+	ret = drm_gpuvm_prepare_objects(&vm->as->base, exec, slot_count);
 	if (ret)
 		return ret;
 
-	return drm_gpuvm_validate(&vm->base, exec);
+	return drm_gpuvm_validate(&vm->as->base, exec);
 }
 
 unsigned long
@@ -3293,21 +3368,21 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device *ptdev,
 	list_splice_init(&ptdev->reclaim.vms, &vms);
 
 	while (freed < nr_to_scan) {
-		struct panthor_vm *vm;
+		struct panthor_as *as;
 
-		vm = list_first_entry_or_null(&vms, typeof(*vm),
+		as = list_first_entry_or_null(&vms, typeof(*as),
 					      reclaim.lru_node);
-		if (!vm)
+		if (!as)
 			break;
 
-		if (!kref_get_unless_zero(&vm->base.kref)) {
-			list_del_init(&vm->reclaim.lru_node);
+		if (!kref_get_unless_zero(&as->base.kref)) {
+			list_del_init(&as->reclaim.lru_node);
 			continue;
 		}
 
 		mutex_unlock(&ptdev->base.gem_lru_mutex);
 
-		freed += drm_gem_lru_scan(&ptdev->base, &vm->reclaim.lru,
+		freed += drm_gem_lru_scan(&ptdev->base, &as->reclaim.lru,
 					  nr_to_scan - freed,
 					  remaining, shrink, NULL);
 
@@ -3316,20 +3391,20 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device *ptdev,
 		/* If the VM is still in the temporary list, remove it so we
 		 * can proceed with the next VM.
 		 */
-		if (vm == list_first_entry_or_null(&vms, typeof(*vm), reclaim.lru_node)) {
-			list_del_init(&vm->reclaim.lru_node);
+		if (as == list_first_entry_or_null(&vms, typeof(*as), reclaim.lru_node)) {
+			list_del_init(&as->reclaim.lru_node);
 
 			/* Keep the VM around if there are still things to
 			 * reclaim, so we can preserve the LRU order when
 			 * re-inserting in ptdev->reclaim.vms at the end.
 			 */
-			if (vm->reclaim.lru.count > 0)
-				list_add_tail(&vm->reclaim.lru_node, &remaining_vms);
+			if (as->reclaim.lru.count > 0)
+				list_add_tail(&as->reclaim.lru_node, &remaining_vms);
 		}
 
 		mutex_unlock(&ptdev->base.gem_lru_mutex);
 
-		panthor_vm_put(vm);
+		drm_gpuvm_put(&as->base);
 
 		mutex_lock(&ptdev->base.gem_lru_mutex);
 	}
@@ -3361,12 +3436,12 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
 
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
-		struct panthor_vm *vm = ptdev->mmu->as.slots[i].vm;
+		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
 
-		if (vm) {
+		if (as) {
 			drm_WARN_ON(&ptdev->base,
 				    panthor_mmu_as_disable(ptdev, i, false));
-			panthor_vm_release_as_locked(vm);
+			panthor_as_release_hw_slot_locked(as);
 		}
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
@@ -3470,9 +3545,9 @@ static int show_vm_gpuvas(struct panthor_vm *vm, struct seq_file *m)
 {
 	int ret;
 
-	mutex_lock(&vm->op_lock);
-	ret = drm_debugfs_gpuva_info(m, &vm->base);
-	mutex_unlock(&vm->op_lock);
+	mutex_lock(&vm->as->op_lock);
+	ret = drm_debugfs_gpuva_info(m, &vm->as->base);
+	mutex_unlock(&vm->as->op_lock);
 
 	return ret;
 }

-- 
2.55.0

[PATCH v4 09/18] drm/panthor: Add fine-grained restrictions on VMs

[Boris Brezillon]

We currently restrict what a VM is allowed to do based on two states:
panthor_vm::destroyed and panthor_vm_pgtable::unusable, but we'll soon
need a no-unmap restriction to fix the unplug logic.

Instead of adding a third boolean that would reflect this new limitation,
let's overhaul the current restriction logic by adding separate
restriction flags representing the operations we want to prevent (map,
unmap and use).

Map and use restrictions are set everywhere we were previously
calling panthor_vm_pgtable_declare_unusable() or setting ::destroyed
to true, since that's what those two flags were preventing.

We also add restriction checks in
panthor_vm_pgtable_prepare_[un]map_op_ctx() and
panthor_vm_pgtable_exec_op() and drop the ones we had in
panthor_vm_bind_job_create() since they are redundant.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 134 ++++++++++++++++++++++------------
 1 file changed, 88 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 933cb820926d..3d9f9bf29e1d 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -225,6 +225,25 @@ struct panthor_as_op_ctx {
 	} map;
 };
 
+/**
+ * enum panthor_as_restriction - List of restrictions that can apply to an AS.
+ *
+ * An AS always starts unrestricted, but based on the faults or device state
+ * changes, restrictions can be added over time. Restrictions can't be removed
+ * though. Once a VM is restricted, a new one must be created to lift the
+ * restrictions.
+ */
+enum panthor_as_restriction {
+	/** @PANTHOR_AS_FORBID_MAP: The AS can't map new buffers. */
+	PANTHOR_AS_FORBID_MAP = BIT(0),
+
+	/** @PANTHOR_AS_FORBID_UNMAP: The AS can't remove existing mappings. */
+	PANTHOR_AS_FORBID_UNMAP = BIT(1),
+
+	/** @PANTHOR_AS_FORBID_USE: The AS can't become active again. */
+	PANTHOR_AS_FORBID_USE = BIT(2),
+};
+
 /**
  * struct panthor_as - Used to managed a GPU address space.
  */
@@ -285,25 +304,8 @@ struct panthor_as {
 		struct list_head lru_node;
 	} hw_slot;
 
-	/**
-	 * @unusable: True if the AS has turned unusable because something
-	 * bad happened during an asynchronous request.
-	 *
-	 * We don't try to recover from such failures, because this implies
-	 * informing userspace about the specific operation that failed, and
-	 * hoping the userspace driver can replay things from there. This all
-	 * sounds very complicated for little gain.
-	 *
-	 * Instead, we should just flag the AS as unusable, and fail any
-	 * further request targeting this AS.
-	 *
-	 * We also provide a way to query an AS state, so userspace can
-	 * destroy it and create a new one.
-	 *
-	 * As an analogy, this would be mapped to a VK_ERROR_DEVICE_LOST
-	 * situation, where the logical device needs to be re-created.
-	 */
-	bool unusable;
+	/** @restrictions: Bitmask of panthor_as_restriction flags. */
+	atomic_t restrictions;
 
 	/**
 	 * @unhandled_fault: Unhandled fault happened.
@@ -431,13 +433,6 @@ struct panthor_vm {
 	/** @for_mcu: True if this is the MCU VM. */
 	bool for_mcu;
 
-	/**
-	 * @destroyed: True if the VM was destroyed.
-	 *
-	 * No further bind requests should be queued to a destroyed VM.
-	 */
-	bool destroyed;
-
 	/**
 	 * @dummy: Dummy object used for sparse mappings.
 	 *
@@ -699,7 +694,9 @@ bool panthor_vm_has_unhandled_faults(struct panthor_vm *vm)
  */
 bool panthor_vm_is_unusable(struct panthor_vm *vm)
 {
-	return vm->as->unusable;
+	return (atomic_read(&vm->as->restrictions) &
+		(PANTHOR_AS_FORBID_USE | PANTHOR_AS_FORBID_MAP |
+		 PANTHOR_AS_FORBID_UNMAP));
 }
 
 static void panthor_as_release_hw_slot_locked(struct panthor_as *as)
@@ -758,6 +755,11 @@ int panthor_vm_active(struct panthor_vm *vm)
 	mutex_lock(&as->op_lock);
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 
+	if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_USE) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	if (refcount_inc_not_zero(&as->active_cnt))
 		goto out_unlock;
 
@@ -926,21 +928,29 @@ static size_t get_pgsize(u64 addr, size_t size, size_t *count)
 	return SZ_2M;
 }
 
-static void panthor_as_declare_unusable(struct panthor_as *as)
+static void panthor_as_restrict_usage_locked(struct panthor_as *as,
+					     u32 new_restrictions)
 {
 	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 	int cookie;
 
-	if (as->unusable)
-		return;
+	lockdep_assert_held(&as->op_lock);
 
-	as->unusable = true;
-	mutex_lock(&ptdev->mmu->as.slots_lock);
-	if (as->hw_slot.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) {
-		panthor_mmu_as_disable(ptdev, as->hw_slot.id, false);
-		drm_dev_exit(cookie);
+	if (new_restrictions & PANTHOR_AS_FORBID_USE) {
+		guard(mutex)(&ptdev->mmu->as.slots_lock);
+		if (as->hw_slot.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) {
+			/* Try to disable the AS. If as_disable() passed, this should cause
+			 * a fault on the next memory access. If it failed, a reset is
+			 * scheduled to recover from the GPU hang.
+			 * We intentionally don't call release_as_locked() here, because
+			 * this would mess up with the active_cnt refcount.
+			 */
+			panthor_mmu_as_disable(ptdev, as->hw_slot.id, false);
+			drm_dev_exit(cookie);
+		}
 	}
-	mutex_unlock(&ptdev->mmu->as.slots_lock);
+
+	atomic_or(new_restrictions, &as->restrictions);
 }
 
 static void panthor_as_unmap_pages(struct panthor_as *as, u64 iova, u64 size)
@@ -976,7 +986,9 @@ static void panthor_as_unmap_pages(struct panthor_as *as, u64 iova, u64 size)
 			 * so flag the VM unusable to make sure it's not going
 			 * to be used anymore.
 			 */
-			panthor_as_declare_unusable(as);
+			panthor_as_restrict_usage_locked(as,
+							 PANTHOR_AS_FORBID_USE |
+							 PANTHOR_AS_FORBID_MAP);
 
 			/* If we don't make progress, we're screwed. That also means
 			 * something else prevents us from unmapping the region, but
@@ -1052,7 +1064,9 @@ panthor_as_map_pages(struct panthor_as *as, u64 iova, int prot,
 				 * table pages behind.
 				 */
 				panthor_as_unmap_pages(as, start_iova, iova - start_iova);
-				panthor_as_declare_unusable(as);
+				panthor_as_restrict_usage_locked(as,
+								 PANTHOR_AS_FORBID_USE |
+								 PANTHOR_AS_FORBID_MAP);
 				return ret;
 			}
 		}
@@ -1349,6 +1363,9 @@ static int panthor_as_prepare_map_op_ctx(struct panthor_as_op_ctx *op_ctx,
 	struct sg_table *sgt = NULL;
 	int ret;
 
+	if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_MAP)
+		return -EINVAL;
+
 	if (!bo)
 		return -EINVAL;
 
@@ -1442,6 +1459,9 @@ static int panthor_as_prepare_unmap_op_ctx(struct panthor_as_op_ctx *op_ctx,
 	u32 pt_count = 0;
 	int ret;
 
+	if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP)
+		return -EINVAL;
+
 	memset(op_ctx, 0, sizeof(*op_ctx));
 	op_ctx->va.range = size;
 	op_ctx->va.addr = va;
@@ -1652,7 +1672,12 @@ static void panthor_vm_destroy(struct panthor_vm *vm)
 
 	as = vm->as;
 	ptdev = container_of(as->base.drm, struct panthor_device, base);
-	vm->destroyed = true;
+
+	scoped_guard(mutex, &as->op_lock) {
+		panthor_as_restrict_usage_locked(as,
+						 PANTHOR_AS_FORBID_USE |
+						 PANTHOR_AS_FORBID_MAP);
+	}
 
 	/* Tell scheduler to stop all GPU work related to this VM */
 	if (refcount_read(&as->active_cnt) > 0)
@@ -2169,7 +2194,7 @@ struct panthor_heap_pool *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool c
 
 	mutex_lock(&vm->heaps.lock);
 	if (!vm->heaps.pool && create) {
-		if (vm->destroyed)
+		if (panthor_vm_is_unusable(vm))
 			pool = ERR_PTR(-EINVAL);
 		else
 			pool = panthor_heap_pool_create(ptdev, vm);
@@ -2546,6 +2571,17 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
 		if (!mutex_trylock(&as->op_lock))
 			return -EDEADLK;
 
+		/* Unmaps are forbidden when we failed to communicate with the GPU,
+		 * meaning we can't guarantee that the GPU will see our page table
+		 * updates which might lead to UAF situations. In that case, we
+		 * just skip eviction on this VM. Things should go back to normal
+		 * after a GPU reset.
+		 */
+		if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP) {
+			ret = -EBUSY;
+			goto unlock_op;
+		}
+
 		/* It can be that the vm_bo was already evicted but a new
 		 * mapping pointing to this BO got created in the meantime,
 		 * thus turning the vm_bo in partially evicted state. In that case
@@ -2581,6 +2617,7 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
 			vma->evicted = true;
 		}
 
+unlock_op:
 		mutex_unlock(&as->op_lock);
 
 		if (ret)
@@ -2795,7 +2832,7 @@ static int panthor_as_exec_op(struct panthor_as *as,
 			.map.gem.offset = op->map.bo_offset,
 		};
 
-		if (as->unusable) {
+		if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_MAP) {
 			ret = -EINVAL;
 			break;
 		}
@@ -2805,6 +2842,11 @@ static int panthor_as_exec_op(struct panthor_as *as,
 	}
 
 	case DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP:
+		if (atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP) {
+			ret = -EINVAL;
+			break;
+		}
+
 		ret = drm_gpuvm_sm_unmap(&as->base, as, op->va.addr, op->va.range);
 		break;
 
@@ -2816,8 +2858,11 @@ static int panthor_as_exec_op(struct panthor_as *as,
 	panthor_as_unlock_region(as);
 
 out:
-	if (ret && flag_vm_unusable_on_failure)
-		panthor_as_declare_unusable(as);
+	if (ret && flag_vm_unusable_on_failure) {
+		panthor_as_restrict_usage_locked(as,
+						 PANTHOR_AS_FORBID_USE |
+						 PANTHOR_AS_FORBID_MAP);
+	}
 
 	as->op_ctx = NULL;
 	mutex_unlock(&as->op_lock);
@@ -3141,9 +3186,6 @@ panthor_vm_bind_job_create(struct drm_file *file,
 	if (!vm)
 		return ERR_PTR(-EINVAL);
 
-	if (vm->destroyed || vm->as->unusable)
-		return ERR_PTR(-EINVAL);
-
 	job = kzalloc_obj(*job);
 	if (!job)
 		return ERR_PTR(-ENOMEM);

-- 
2.55.0

[PATCH v4 10/18] drm/panthor: Check AS state before disabling

[Boris Brezillon]

Use TRANSTAB == 0 as a way to detect if an AS slot is idle. This
allows us to make panthor_mmu_as_disable() a NOP when it's called
after a SOFT_RESET, which will be needed for our unplug rework.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 3d9f9bf29e1d..58f0bd2ac7a2 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -640,6 +640,10 @@ static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 slot,
 
 	lockdep_assert_held(&ptdev->mmu->as.slots_lock);
 
+	/* The AS was disabled already, nothing to do. */
+	if (!gpu_read64(mmu->iomem, AS_TRANSTAB(slot)))
+		return 0;
+
 	panthor_irq_disable_events(&ptdev->mmu->irq,
 				   panthor_mmu_as_fault_mask(ptdev, slot));
 
@@ -662,11 +666,17 @@ static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 slot,
 	if (recycle_slot)
 		return 0;
 
-	gpu_write64(mmu->iomem, AS_TRANSTAB(slot), 0);
-	gpu_write64(mmu->iomem, AS_MEMATTR(slot), 0);
 	gpu_write64(mmu->iomem, AS_TRANSCFG(slot), AS_TRANSCFG_ADRMODE_UNMAPPED);
+	ret = as_send_cmd_and_wait(ptdev, slot, AS_COMMAND_UPDATE);
+	if (ret)
+		return ret;
 
-	return as_send_cmd_and_wait(ptdev, slot, AS_COMMAND_UPDATE);
+	/* We reset the other fields late to ensure that, if something fails,
+	 * the page table is considered active (TRANSTAB != NULL).
+	 */
+	gpu_write64(mmu->iomem, AS_MEMATTR(slot), 0);
+	gpu_write64(mmu->iomem, AS_TRANSTAB(slot), 0);
+	return 0;
 }
 
 static u32 panthor_mmu_fault_mask(struct panthor_device *ptdev, u32 value)

-- 
2.55.0

[PATCH v4 11/18] drm/panthor: Don't pre-allocate VMAs or page tables when preparing a full VM unmap

[Boris Brezillon]

In the cleanup path, we unmap the full VA range to make sure things are
clean before the VM is released. I'd rather not fail on memory
allocation in that path, so let's make sure
panthor_vm_pgtable_prepare_unmap_op_ctx() doesn't allocate VMAs or
page tables when the unmap range matches the VM virtual address range.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 58f0bd2ac7a2..11a9bbe87986 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1477,6 +1477,10 @@ static int panthor_as_prepare_unmap_op_ctx(struct panthor_as_op_ctx *op_ctx,
 	op_ctx->va.addr = va;
 	op_ctx->flags = DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP;
 
+	/* Unmap on the whole VM range don't need new VMAs or page tables. */
+	if (va == as->base.mm_start && size == as->base.mm_range)
+		return 0;
+
 	/* Pre-allocate L3 page tables to account for the split-2M-block
 	 * situation on unmap.
 	 */

-- 
2.55.0

[PATCH v4 12/18] drm/panthor: Let l2_power_off return errors and force users to check it

[Boris Brezillon]

The MMU logic assumes that, after a suspend, all the AS that were
resident before the suspend are free to leave their slot because the HW
is inactive until the next resume request. This doesn't hold if we
ignore failures to power-off the L2 block.

Let's propagate the error from the PWR backend to
panthor_gpu_l2_power_off(), and adjust panthor_gpu_suspend() to
escalate to slow reset when a fast reset is not possible. Add a
__must_check on panthor_hw_l2_power_off() to make sure new users
don't forget that they have to check the returned value.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_fw.c  |  2 +-
 drivers/gpu/drm/panthor/panthor_gpu.c | 15 ++++++++++-----
 drivers/gpu/drm/panthor/panthor_gpu.h |  2 +-
 drivers/gpu/drm/panthor/panthor_hw.h  |  7 ++++---
 drivers/gpu/drm/panthor/panthor_pwr.c | 20 ++++++++++++--------
 drivers/gpu/drm/panthor/panthor_pwr.h |  2 +-
 6 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 68965175105f..731da736e372 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -1308,7 +1308,7 @@ void panthor_fw_unplug(struct panthor_device *ptdev)
 	ptdev->fw->vm = NULL;
 
 	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev))
-		panthor_hw_l2_power_off(ptdev);
+		drm_WARN_ON(&ptdev->base, panthor_hw_l2_power_off(ptdev));
 }
 
 /**
diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index a383b04f101e..09ebe0294691 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.c
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -288,9 +288,9 @@ int panthor_gpu_block_power_on(struct panthor_device *ptdev,
 	return 0;
 }
 
-void panthor_gpu_l2_power_off(struct panthor_device *ptdev)
+int panthor_gpu_l2_power_off(struct panthor_device *ptdev)
 {
-	panthor_gpu_power_off(ptdev, L2, ptdev->gpu_info.l2_present, 20000);
+	return panthor_gpu_power_off(ptdev, L2, ptdev->gpu_info.l2_present, 20000);
 }
 
 /**
@@ -446,11 +446,16 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
  */
 void panthor_gpu_suspend(struct panthor_device *ptdev)
 {
-	/* On a fast reset, simply power down the L2. */
+	/* On a fast reset, simply power down the L2. If it fails, escalate to
+	 * a slow reset.
+	 */
+	if (ptdev->reset.fast && panthor_hw_l2_power_off(ptdev)) {
+		drm_warn(&ptdev->base, "L2 power-off failed, escalating to a slow reset.");
+		ptdev->reset.fast = false;
+	}
+
 	if (!ptdev->reset.fast)
 		panthor_hw_soft_reset(ptdev);
-	else
-		panthor_hw_l2_power_off(ptdev);
 
 	panthor_irq_suspend(&ptdev->gpu->irq);
 }
diff --git a/drivers/gpu/drm/panthor/panthor_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h
index f615feb05609..4b8bae363efb 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.h
+++ b/drivers/gpu/drm/panthor/panthor_gpu.h
@@ -46,7 +46,7 @@ int panthor_gpu_block_power_off(struct panthor_device *ptdev,
 				   type ## _PWRTRANS, \
 				   mask, timeout_us)
 
-void panthor_gpu_l2_power_off(struct panthor_device *ptdev);
+int panthor_gpu_l2_power_off(struct panthor_device *ptdev);
 int panthor_gpu_l2_power_on(struct panthor_device *ptdev);
 int panthor_gpu_flush_caches(struct panthor_device *ptdev,
 			     u32 l2, u32 lsc, u32 other);
diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
index f797663893b2..4531c1239cb6 100644
--- a/drivers/gpu/drm/panthor/panthor_hw.h
+++ b/drivers/gpu/drm/panthor/panthor_hw.h
@@ -15,7 +15,7 @@ struct panthor_hw_ops {
 	int (*soft_reset)(struct panthor_device *ptdev);
 
 	/** @l2_power_off: L2 power off function pointer */
-	void (*l2_power_off)(struct panthor_device *ptdev);
+	int (*l2_power_off)(struct panthor_device *ptdev);
 
 	/** @l2_power_on: L2 power on function pointer */
 	int (*l2_power_on)(struct panthor_device *ptdev);
@@ -51,9 +51,10 @@ static inline int panthor_hw_l2_power_on(struct panthor_device *ptdev)
 	return ptdev->hw->ops.l2_power_on(ptdev);
 }
 
-static inline void panthor_hw_l2_power_off(struct panthor_device *ptdev)
+static inline int __must_check
+panthor_hw_l2_power_off(struct panthor_device *ptdev)
 {
-	ptdev->hw->ops.l2_power_off(ptdev);
+	return ptdev->hw->ops.l2_power_off(ptdev);
 }
 
 static inline bool panthor_hw_has_pwr_ctrl(struct panthor_device *ptdev)
diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c
index dd7b6ef8ea20..c81e2cc053db 100644
--- a/drivers/gpu/drm/panthor/panthor_pwr.c
+++ b/drivers/gpu/drm/panthor/panthor_pwr.c
@@ -512,16 +512,17 @@ int panthor_pwr_reset_soft(struct panthor_device *ptdev)
 	return panthor_pwr_reset(ptdev, PWR_COMMAND_RESET_SOFT);
 }
 
-void panthor_pwr_l2_power_off(struct panthor_device *ptdev)
+int panthor_pwr_l2_power_off(struct panthor_device *ptdev)
 {
 	struct panthor_pwr *pwr = ptdev->pwr;
 	const u64 l2_allow_mask = PWR_STATUS_DOMAIN_ALLOWED(PWR_COMMAND_DOMAIN_L2);
 	const u64 pwr_status = gpu_read64(pwr->iomem, PWR_STATUS);
+	int ret;
 
 	/* Abort if L2 power off constraints are not satisfied */
 	if (!(pwr_status & l2_allow_mask)) {
 		drm_warn(&ptdev->base, "Power off L2 domain not allowed");
-		return;
+		return -EOPNOTSUPP;
 	}
 
 	/* It is expected that when halting the MCU, it would power down its
@@ -530,14 +531,17 @@ void panthor_pwr_l2_power_off(struct panthor_device *ptdev)
 	 * host control to be powered down in the right order before powering
 	 * down the L2.
 	 */
-	if (panthor_pwr_domain_force_off(ptdev, PWR_COMMAND_DOMAIN_TILER))
-		return;
+	ret = panthor_pwr_domain_force_off(ptdev, PWR_COMMAND_DOMAIN_TILER);
+	if (ret)
+		return ret;
 
-	if (panthor_pwr_domain_force_off(ptdev, PWR_COMMAND_DOMAIN_SHADER))
-		return;
+	ret = panthor_pwr_domain_force_off(ptdev, PWR_COMMAND_DOMAIN_SHADER);
+	if (ret)
+		return ret;
 
-	panthor_pwr_domain_power_off(ptdev, PWR_COMMAND_DOMAIN_L2, ptdev->gpu_info.l2_present,
-				     PWR_TRANSITION_TIMEOUT_US);
+	return panthor_pwr_domain_power_off(ptdev, PWR_COMMAND_DOMAIN_L2,
+					    ptdev->gpu_info.l2_present,
+					    PWR_TRANSITION_TIMEOUT_US);
 }
 
 int panthor_pwr_l2_power_on(struct panthor_device *ptdev)
diff --git a/drivers/gpu/drm/panthor/panthor_pwr.h b/drivers/gpu/drm/panthor/panthor_pwr.h
index adf1f6136abc..98a9a5b24270 100644
--- a/drivers/gpu/drm/panthor/panthor_pwr.h
+++ b/drivers/gpu/drm/panthor/panthor_pwr.h
@@ -12,7 +12,7 @@ int panthor_pwr_init(struct panthor_device *ptdev);
 
 int panthor_pwr_reset_soft(struct panthor_device *ptdev);
 
-void panthor_pwr_l2_power_off(struct panthor_device *ptdev);
+int panthor_pwr_l2_power_off(struct panthor_device *ptdev);
 
 int panthor_pwr_l2_power_on(struct panthor_device *ptdev);
 

-- 
2.55.0

[PATCH v4 13/18] drm/panthor: Complain if the SOFT_RESET fails

[Boris Brezillon]

We rely on a functioning SOFT_RESET to avoid HW UAFs when the GPU was
in a state where AS commands were no longer accepted. If we silently
ignore RESET failures, we're just pretending to be safe while exposing
ourselves to the very UAFs we were trying to avoid. On the other hand,
there's basically nothing we can do if both the SOFT_RESET and the
AS_COMMAND(UNMAPPED) fail, so do what we do best: complain loudly and
taint the kernel with a WARN_ON().

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_hw.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
index 4531c1239cb6..f13fd7b335c1 100644
--- a/drivers/gpu/drm/panthor/panthor_hw.h
+++ b/drivers/gpu/drm/panthor/panthor_hw.h
@@ -41,9 +41,17 @@ int panthor_hw_init(struct panthor_device *ptdev);
 int panthor_hw_power_status_register(void);
 void panthor_hw_power_status_unregister(void);
 
-static inline int panthor_hw_soft_reset(struct panthor_device *ptdev)
+static inline void
+panthor_hw_soft_reset(struct panthor_device *ptdev)
 {
-	return ptdev->hw->ops.soft_reset(ptdev);
+	/* We're relying on the SOFT_RESET to reset the MMU block if some AS
+	 * were stuck for some reason. Failing to reset the MMU/L2 means we're
+	 * exposing ourselves to HW UAFs. On the other hand, there's basically
+	 * nothing we can do if both the SOFT_RESET and
+	 * the AS_COMMAND(UNMAPPED) fail, so do what we do best: complain loudly
+	 * and taint the kernel.
+	 */
+	drm_WARN_ON(&ptdev->base, ptdev->hw->ops.soft_reset(ptdev));
 }
 
 static inline int panthor_hw_l2_power_on(struct panthor_device *ptdev)

-- 
2.55.0

[PATCH v4 14/18] drm/panthor: Make the VM cleanup path more robust against UAF

[Boris Brezillon]

The VM cleanup logic tries to gracefully evict the page table from its
AS slot to make sure the HW doesn't have access to the memory anymore.
But it might happen that the eviction fails because the HW hung, and
in that case, we have no guarantee that the HW won't access the memory
until we've properly reset the GPU.

Defer the cleanup of VMs after the reset is effective when this situation
happens.

Note that this introduces a leak if the VM is released while being
assigned an AS slot and the device was unplugged, because we don't
clear the FORBID_UNMAP restriction in that case. It's just one more
issue in an ocean of unplug bugs. These unplug issues will be addressed
in an upcoming commit.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c |   6 +-
 drivers/gpu/drm/panthor/panthor_mmu.c    | 308 +++++++++++++++++++++++--------
 drivers/gpu/drm/panthor/panthor_mmu.h    |   3 +-
 3 files changed, 239 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 19d3669f5ec7..817312f598f3 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -522,9 +522,10 @@ static int panthor_device_resume_hw_components(struct panthor_device *ptdev)
 	if (!ret)
 		return 0;
 
-	panthor_mmu_suspend(ptdev);
+	panthor_mmu_pre_suspend(ptdev);
 	panthor_gpu_suspend(ptdev);
 	panthor_pwr_suspend(ptdev);
+	panthor_mmu_post_suspend(ptdev);
 	return ret;
 }
 
@@ -645,9 +646,10 @@ int panthor_device_suspend(struct device *dev)
 		 */
 		panthor_sched_suspend(ptdev);
 		panthor_fw_suspend(ptdev);
-		panthor_mmu_suspend(ptdev);
+		panthor_mmu_pre_suspend(ptdev);
 		panthor_gpu_suspend(ptdev);
 		panthor_pwr_suspend(ptdev);
+		panthor_mmu_post_suspend(ptdev);
 		drm_dev_exit(cookie);
 	}
 
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 11a9bbe87986..47c57b39bd12 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -102,6 +102,17 @@ struct panthor_mmu {
 		 * TLB/cache flushes.
 		 */
 		struct list_head lru_list;
+
+		/**
+		 * @cleanup_list: List containing VMs waiting for cleanup.
+		 *
+		 * This list is used to keep track of VMs that got released but
+		 * couldn't be evicted from their AS slot because the HW hanged.
+		 * In that case, we add the VM to the list, and wait for the next
+		 * post_reset, at which point we're sure the HW is idle and the
+		 * VM resources can go away.
+		 */
+		struct list_head cleanup_list;
 	} as;
 
 	/** @vm: VMs management fields */
@@ -117,6 +128,12 @@ struct panthor_mmu {
 
 		/** @vm.wq: Workqueue used for the VM_BIND queues. */
 		struct workqueue_struct *wq;
+
+		/**
+		 * @vm.cleanup_work: Used to cleanup the VMs that are in
+		 * panthor_mmu::as::cleanup_list.
+		 */
+		struct work_struct cleanup_work;
 	} vm;
 };
 
@@ -1994,7 +2011,7 @@ static irqreturn_t panthor_mmu_irq_threaded_handler(int irq, void *data)
 }
 
 /**
- * panthor_mmu_suspend() - Suspend the MMU logic
+ * panthor_mmu_pre_suspend() - Prepare the MMU block for a suspend
  * @ptdev: Device.
  *
  * All we do here is de-assign the AS slots on all active VMs, so things
@@ -2003,16 +2020,27 @@ static irqreturn_t panthor_mmu_irq_threaded_handler(int irq, void *data)
  *
  * We also suspend the MMU IRQ.
  */
-void panthor_mmu_suspend(struct panthor_device *ptdev)
+void panthor_mmu_pre_suspend(struct panthor_device *ptdev)
 {
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
 		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
+		int ret;
 
-		if (as) {
-			drm_WARN_ON(&ptdev->base,
-				    panthor_mmu_as_disable(ptdev, i, false));
+		if (!as)
+			continue;
+
+		/* If the disable fails, leave the AS on its slot so we can
+		 * properly evict it when we're sure the GPU is off.
+		 */
+		ret = panthor_mmu_as_disable(ptdev, i, false);
+		if (!ret) {
 			panthor_as_release_hw_slot_locked(as);
+		} else {
+			atomic_or(PANTHOR_AS_FORBID_USE |
+				  PANTHOR_AS_FORBID_MAP |
+				  PANTHOR_AS_FORBID_UNMAP,
+				  &as->restrictions);
 		}
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
@@ -2020,6 +2048,35 @@ void panthor_mmu_suspend(struct panthor_device *ptdev)
 	panthor_irq_suspend(&ptdev->mmu->irq);
 }
 
+static void mmu_post_reset_cleanup(struct panthor_device *ptdev)
+{
+	guard(mutex)(&ptdev->mmu->as.slots_lock);
+
+	/* Now that the reset is effective, we can assume that none of the
+	 * AS slots are setup, and clear the faulty flags too.
+	 */
+	ptdev->mmu->as.alloc_mask = 0;
+	ptdev->mmu->as.faulty_mask = 0;
+
+	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
+		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
+
+		if (!as)
+			continue;
+
+		panthor_as_release_hw_slot_locked(as);
+		atomic_and(~PANTHOR_AS_FORBID_UNMAP, &as->restrictions);
+	}
+
+	if (!list_empty(&ptdev->mmu->as.cleanup_list))
+		queue_work(ptdev->cleanup_wq, &ptdev->mmu->vm.cleanup_work);
+}
+
+void panthor_mmu_post_suspend(struct panthor_device *ptdev)
+{
+	mmu_post_reset_cleanup(ptdev);
+}
+
 /**
  * panthor_mmu_resume() - Resume the MMU logic
  * @ptdev: Device.
@@ -2031,11 +2088,7 @@ void panthor_mmu_suspend(struct panthor_device *ptdev)
  */
 void panthor_mmu_resume(struct panthor_device *ptdev)
 {
-	mutex_lock(&ptdev->mmu->as.slots_lock);
-	ptdev->mmu->as.alloc_mask = 0;
-	ptdev->mmu->as.faulty_mask = 0;
-	mutex_unlock(&ptdev->mmu->as.slots_lock);
-
+	mmu_post_reset_cleanup(ptdev);
 	panthor_irq_resume(&ptdev->mmu->irq);
 }
 
@@ -2073,22 +2126,7 @@ void panthor_mmu_post_reset(struct panthor_device *ptdev)
 {
 	struct panthor_vm *vm;
 
-	mutex_lock(&ptdev->mmu->as.slots_lock);
-
-	/* Now that the reset is effective, we can assume that none of the
-	 * AS slots are setup, and clear the faulty flags too.
-	 */
-	ptdev->mmu->as.alloc_mask = 0;
-	ptdev->mmu->as.faulty_mask = 0;
-
-	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
-		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
-
-		if (as)
-			panthor_as_release_hw_slot_locked(as);
-	}
-
-	mutex_unlock(&ptdev->mmu->as.slots_lock);
+	mmu_post_reset_cleanup(ptdev);
 
 	panthor_irq_resume(&ptdev->mmu->irq);
 
@@ -2101,59 +2139,41 @@ void panthor_mmu_post_reset(struct panthor_device *ptdev)
 	mutex_unlock(&ptdev->mmu->vm.lock);
 }
 
-static void panthor_vm_release(struct kref *kref)
+static void vm_cleanup(struct panthor_vm *vm)
 {
-	struct panthor_vm *vm = container_of(kref, struct panthor_vm, refcount);
 	struct panthor_as *as = vm->as;
 	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
 
-	/* Make sure the page table behind this VM doesn't participate in reclaim
-	 * after that point, since we're about to release everything anyway.
+	if (!(atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP)) {
+		/* Unmap everything in case some BOs were still mapped. */
+		drm_WARN_ON(&ptdev->base,
+			    panthor_vm_unmap_range(vm, as->base.mm_start, as->base.mm_range));
+	}
+
+	/* It's safe to destroy the heaps and return the dummy BO even if
+	 * the FORBID_UNMAP restriction stands because this BO will be retained
+	 * by the drm_gpuvm_bo held by the active drm_gpuva entries, which
+	 * were preserved because the panthor_vm_unmap_range() was skipped.
+	 * The drm_gpuvm object itself is retained by the live drm_gpuvas
+	 * living there. So this is one of the rare occasions where we actually
+	 * want the circular referencing to silently leak objects.
+	 * Another side effect of this is that the drm_device is retained too,
+	 * because drm_gpuvm holds a ref on this object. This is okay, because
+	 * drm_dev_unregister() makes sure this device is no longer exposed
+	 * to userspace, so it's basically a zombie drm_device.
+	 *
+	 * TLDR; the memory a rogue GPU might have its hands on is leaked,
+	 * even though we explicitly destroy the objects owning these resources.
 	 */
-	mutex_lock(&ptdev->base.gem_lru_mutex);
-	as->reclaim.skip = true;
-	list_del_init(&as->reclaim.lru_node);
-	mutex_unlock(&ptdev->base.gem_lru_mutex);
-
-	/* Unmap everything in case some BOs were still mapped. */
-	drm_WARN_ON(&ptdev->base,
-		    panthor_vm_unmap_range(vm, as->base.mm_start, as->base.mm_range));
-
-	mutex_lock(&vm->heaps.lock);
-	if (drm_WARN_ON(&ptdev->base, vm->heaps.pool))
-		panthor_heap_pool_destroy(vm->heaps.pool);
-	mutex_unlock(&vm->heaps.lock);
+	scoped_guard(mutex, &vm->heaps.lock) {
+		if (drm_WARN_ON(&ptdev->base, vm->heaps.pool))
+			panthor_heap_pool_destroy(vm->heaps.pool);
+	}
 	mutex_destroy(&vm->heaps.lock);
 
-	mutex_lock(&ptdev->mmu->vm.lock);
-	list_del(&vm->node);
-	/* Restore the scheduler state so we can call drm_sched_entity_destroy()
-	 * and drm_sched_fini(). If get there, that means we have no job left
-	 * and no new jobs can be queued, so we can start the scheduler without
-	 * risking interfering with the reset.
-	 */
-	if (ptdev->mmu->vm.reset_in_progress)
-		panthor_vm_start(vm);
-	mutex_unlock(&ptdev->mmu->vm.lock);
-
 	drm_sched_entity_destroy(&vm->entity);
 	drm_sched_fini(&vm->sched);
 
-	mutex_lock(&vm->as->op_lock);
-	mutex_lock(&ptdev->mmu->as.slots_lock);
-	if (as->hw_slot.id >= 0) {
-		int cookie;
-
-		if (drm_dev_enter(&ptdev->base, &cookie)) {
-			panthor_mmu_as_disable(ptdev, as->hw_slot.id, false);
-			drm_dev_exit(cookie);
-		}
-
-		panthor_as_release_hw_slot_locked(as);
-	}
-	mutex_unlock(&ptdev->mmu->as.slots_lock);
-	mutex_unlock(&vm->as->op_lock);
-
 	if (vm->dummy)
 		drm_gem_object_put(&vm->dummy->base);
 
@@ -2162,6 +2182,98 @@ static void panthor_vm_release(struct kref *kref)
 	kfree(vm);
 }
 
+static bool vm_prep_for_cleanup(struct panthor_vm *vm)
+{
+	struct panthor_as *as = vm->as;
+	struct panthor_device *ptdev = container_of(as->base.drm, struct panthor_device, base);
+	bool ready_for_cleanup;
+	int cookie, ret;
+
+	/* First we forbid any kind of use on the VM that's about to be
+	 * released. UNMAP will be restored later if we manage to evict
+	 * the page table from its AS slot.
+	 */
+	atomic_or(PANTHOR_AS_FORBID_USE |
+		  PANTHOR_AS_FORBID_MAP |
+		  PANTHOR_AS_FORBID_UNMAP,
+		  &vm->as->restrictions);
+
+	/* Make sure the page table behind this VM doesn't participate in reclaim
+	 * after that point, since we're about to release everything anyway.
+	 */
+	scoped_guard(mutex, &ptdev->base.gem_lru_mutex) {
+		as->reclaim.skip = true;
+		list_del_init(&as->reclaim.lru_node);
+	}
+
+	scoped_guard(mutex, &ptdev->mmu->vm.lock) {
+		/* Remove the VM from the list early, so it can't be seen by the VM list
+		 * walkers after that point.
+		 */
+		list_del(&vm->node);
+
+		/* Restore the scheduler state so we can call drm_sched_entity_destroy()
+		 * and drm_sched_fini(). If get there, that means we have no job left
+		 * and no new jobs can be queued, so we can start the scheduler without
+		 * risking interfering with the reset.
+		 */
+		if (ptdev->mmu->vm.reset_in_progress)
+			panthor_vm_start(vm);
+	}
+
+	if (!drm_dev_enter(&ptdev->base, &cookie)) {
+		guard(mutex)(&ptdev->mmu->as.slots_lock);
+
+		/* If we're still on a slot after an unplug, it means
+		 * drm_dev_unplug() has returned but the part in
+		 * panthor_mmu_unplug() that evicts all resident AS has
+		 * not been executed yet. In that case, we simply queue
+		 * the VM to the cleanup list and wait for
+		 * panthor_mmu_unplug() to do its job.
+		 */
+		if (as->hw_slot.id >= 0) {
+			list_add_tail(&vm->node, &ptdev->mmu->as.cleanup_list);
+			return false;
+		}
+
+		/* If the page table is not resident, we can drop the
+		 * FORBID_UNMAP restriction.
+		 */
+		atomic_and(~PANTHOR_AS_FORBID_UNMAP, &as->restrictions);
+		return true;
+	}
+
+	scoped_guard(mutex, &ptdev->mmu->as.slots_lock) {
+		if (as->hw_slot.id >= 0) {
+			ret = panthor_mmu_as_disable(ptdev, as->hw_slot.id, false);
+			if (!ret) {
+				panthor_as_release_hw_slot_locked(as);
+			} else {
+				list_add_tail(&vm->node, &ptdev->mmu->as.cleanup_list);
+				panthor_device_schedule_reset(ptdev);
+			}
+		}
+
+		/* Page table is no longer resident, we can relax the no-unmap
+		 * restriction.
+		 */
+		ready_for_cleanup = as->hw_slot.id < 0;
+		if (ready_for_cleanup)
+			atomic_and(~PANTHOR_AS_FORBID_UNMAP, &as->restrictions);
+	}
+
+	drm_dev_exit(cookie);
+	return ready_for_cleanup;
+}
+
+static void panthor_vm_release(struct kref *kref)
+{
+	struct panthor_vm *vm = container_of(kref, struct panthor_vm, refcount);
+
+	if (vm_prep_for_cleanup(vm))
+		vm_cleanup(vm);
+}
+
 /**
  * panthor_vm_put() - Release a reference on a VM
  * @vm: VM to release the reference on. Can be NULL.
@@ -2785,7 +2897,11 @@ static void panthor_as_free(struct drm_gpuvm *gpuvm)
 {
 	struct panthor_as *as = container_of(gpuvm, struct panthor_as, base);
 
-	if (as->pt.ops)
+	/* If we get to that point and we're still not allowed to unmap,
+	 * this means the HW is still running and has a access to the page
+	 * table, so we just leak it to avoid UAF.
+	 */
+	if (as->pt.ops && !(atomic_read(&as->restrictions) & PANTHOR_AS_FORBID_UNMAP))
 		free_io_pgtable_ops(as->pt.ops);
 
 	mutex_destroy(&as->op_lock);
@@ -3493,15 +3609,34 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
 		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
+		int ret;
 
-		if (as) {
-			drm_WARN_ON(&ptdev->base,
-				    panthor_mmu_as_disable(ptdev, i, false));
-			panthor_as_release_hw_slot_locked(as);
-		}
+		if (!as)
+			continue;
+
+		ret = panthor_mmu_as_disable(ptdev, i, false);
+		drm_WARN_ON(&ptdev->base, ret);
+
+		/* Drop the unmap restriction if the disabled worked, so we
+		 * don't leak resources in the normal situation.
+		 */
+		if (!ret)
+			atomic_and(~PANTHOR_AS_FORBID_UNMAP, &as->restrictions);
+
+		panthor_as_release_hw_slot_locked(as);
 	}
+
+	if (!list_empty(&ptdev->mmu->as.cleanup_list))
+		queue_work(ptdev->cleanup_wq, &ptdev->mmu->vm.cleanup_work);
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
 
+	/* Make sure pending VM cleanups are processed before leaving. Those
+	 * cleanups might schedule vm_bind_job cleanups, so keep this
+	 * flush_work() before the final flush_workqueue(panthor_cleanup_wq).
+	 */
+	flush_work(&ptdev->mmu->vm.cleanup_work);
+	drm_WARN_ON(&ptdev->base, !list_empty(&ptdev->mmu->as.cleanup_list));
+
 	/* Ensure any pending job cleanup work are executed before returning,
 	 * otherwise those might access objects that are gone if the work is
 	 * executed after other components are unplugged.
@@ -3519,6 +3654,27 @@ static void panthor_mmu_info_init(struct panthor_device *ptdev)
 	ptdev->mmu_info.page_size_bitmap = SZ_4K | SZ_2M;
 }
 
+static void mmu_cleanup_vms_work(struct work_struct *work)
+{
+	struct panthor_mmu *mmu =
+		container_of(work, struct panthor_mmu, vm.cleanup_work);
+	struct panthor_vm *vm, *tmp;
+	LIST_HEAD(cleanup_list);
+
+	/* Collect the VMs to cleanup first. */
+	scoped_guard(mutex, &mmu->as.slots_lock) {
+		list_for_each_entry_safe(vm, tmp, &mmu->as.cleanup_list, node) {
+			if (vm->as->hw_slot.id < 0)
+				list_move_tail(&vm->node, &cleanup_list);
+		}
+	}
+
+	list_for_each_entry_safe(vm, tmp, &cleanup_list, node) {
+		list_del(&vm->node);
+		vm_cleanup(vm);
+	}
+}
+
 static void free_pt_cache(struct drm_device *, void *pt_cache)
 {
 	kmem_cache_destroy(pt_cache);
@@ -3542,7 +3698,9 @@ int panthor_mmu_init(struct panthor_device *ptdev)
 	if (!mmu)
 		return -ENOMEM;
 
+	INIT_WORK(&mmu->vm.cleanup_work, mmu_cleanup_vms_work);
 	INIT_LIST_HEAD(&mmu->as.lru_list);
+	INIT_LIST_HEAD(&mmu->as.cleanup_list);
 
 	ret = drmm_mutex_init(&ptdev->base, &mmu->as.slots_lock);
 	if (ret)
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
index de6b4ee4e41a..1ee5958569b6 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.h
+++ b/drivers/gpu/drm/panthor/panthor_mmu.h
@@ -21,7 +21,8 @@ int panthor_mmu_init(struct panthor_device *ptdev);
 void panthor_mmu_unplug(struct panthor_device *ptdev);
 void panthor_mmu_pre_reset(struct panthor_device *ptdev);
 void panthor_mmu_post_reset(struct panthor_device *ptdev);
-void panthor_mmu_suspend(struct panthor_device *ptdev);
+void panthor_mmu_pre_suspend(struct panthor_device *ptdev);
+void panthor_mmu_post_suspend(struct panthor_device *ptdev);
 void panthor_mmu_resume(struct panthor_device *ptdev);
 
 int panthor_vm_map_bo_range(struct panthor_vm *vm, struct panthor_gem_object *bo,

-- 
2.55.0

[PATCH v4 15/18] drm/panthor: Track user owned VMs

[Boris Brezillon]

We will soon need this to fix the unplug logic and make sure panthor_vm
objects are not left behind after an unplug.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 47c57b39bd12..6368bf57b8f5 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -117,12 +117,15 @@ struct panthor_mmu {
 
 	/** @vm: VMs management fields */
 	struct {
-		/** @vm.lock: Lock protecting access to list. */
+		/** @vm.lock: Lock protecting access to list and user_owned. */
 		struct mutex lock;
 
 		/** @vm.list: List containing all VMs. */
 		struct list_head list;
 
+		/** @vm.list: List containing VMs with a valid handle. */
+		struct list_head user_owned;
+
 		/** @vm.reset_in_progress: True if a reset is in progress. */
 		bool reset_in_progress;
 
@@ -447,6 +450,9 @@ struct panthor_vm {
 	/** @node: Used to insert the VM in the panthor_mmu::vm::list. */
 	struct list_head node;
 
+	/* @user_node: Used to insert the VM in the panthor_mmu::vm::user_owned list. */
+	struct list_head user_node;
+
 	/** @for_mcu: True if this is the MCU VM. */
 	bool for_mcu;
 
@@ -1681,10 +1687,19 @@ int panthor_vm_pool_create_vm(struct panthor_device *ptdev,
 	drm_gem_object_get(&pool->dummy->base);
 	vm->dummy = pool->dummy;
 
+	/* Insert in the list before xa_alloc() so we can't race with
+	 * panthor_vm_pool_destroy_vm() have the VM inserted in the
+	 * user_owned list after it's been destroyed.
+	 */
+	scoped_guard(mutex, &ptdev->mmu->vm.lock)
+		list_add_tail(&vm->user_node, &ptdev->mmu->vm.user_owned);
+
 	ret = xa_alloc(&pool->xa, &id, vm,
 		       XA_LIMIT(1, PANTHOR_MAX_VMS_PER_FILE), GFP_KERNEL);
 
 	if (ret) {
+		scoped_guard(mutex, &ptdev->mmu->vm.lock)
+			list_del_init(&vm->user_node);
 		panthor_vm_put(vm);
 		return ret;
 	}
@@ -1739,13 +1754,19 @@ static void panthor_vm_destroy(struct panthor_vm *vm)
  */
 int panthor_vm_pool_destroy_vm(struct panthor_vm_pool *pool, u32 handle)
 {
+	struct panthor_device *ptdev;
 	struct panthor_vm *vm;
 
 	vm = xa_erase(&pool->xa, handle);
+	if (!vm)
+		return -EINVAL;
+
+	ptdev = container_of(vm->as->base.drm, struct panthor_device, base);
+	scoped_guard(mutex, &ptdev->mmu->vm.lock)
+		list_del_init(&vm->user_node);
 
 	panthor_vm_destroy(vm);
-
-	return vm ? 0 : -EINVAL;
+	return 0;
 }
 
 /**
@@ -1785,7 +1806,7 @@ void panthor_vm_pool_destroy(struct panthor_file *pfile)
 		return;
 
 	xa_for_each(&pfile->vms->xa, i, vm)
-		panthor_vm_destroy(vm);
+		panthor_vm_pool_destroy_vm(pfile->vms, i);
 
 	if (pfile->vms->dummy)
 		drm_gem_object_put(&pfile->vms->dummy->base);
@@ -3182,6 +3203,7 @@ panthor_vm_create(struct panthor_device *ptdev, bool for_mcu,
 		goto err_put_as;
 	}
 
+	INIT_LIST_HEAD(&vm->user_node);
 	vm->user_va_range = kernel_va_start;
 	vm->as = as;
 	mutex_init(&vm->heaps.lock);
@@ -3707,6 +3729,7 @@ int panthor_mmu_init(struct panthor_device *ptdev)
 		return ret;
 
 	INIT_LIST_HEAD(&mmu->vm.list);
+	INIT_LIST_HEAD(&mmu->vm.user_owned);
 	ret = drmm_mutex_init(&ptdev->base, &mmu->vm.lock);
 	if (ret)
 		return ret;

-- 
2.55.0

[PATCH v4 16/18] drm/panthor: Track user owned groups

[Boris Brezillon]

A group can outlive its user handle because of internal refs. In order
to fix the unplug logic, we need to keep track of groups that have a
valid user handle so we can release the references that were owned by
the user processes in the unplug path.

This is the prep work to keep track of user owned groups. Note that
the destroyed attribute is dropped because it's equivalent to checking
whether the group is inserted in the user_owned list now.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_sched.c | 43 ++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 4ea16b40d6b9..bd5dcf4cb580 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -235,6 +235,15 @@ struct panthor_scheduler {
 		 * This list is evaluated in the @sync_upd_work work.
 		 */
 		struct list_head waiting;
+
+		/**
+		 * @user_owned: List of groups that have a valid user handle.
+		 *
+		 * All groups are inserted in this list at creation time through their
+		 * panthor_group;:user_node, and evicted from this list when
+		 * panthor_group_destroy() is called.
+		 */
+		struct list_head user_owned;
 	} groups;
 
 	/**
@@ -586,15 +595,6 @@ struct panthor_group {
 	 */
 	int csg_id;
 
-	/**
-	 * @destroyed: True when the group has been destroyed.
-	 *
-	 * If a group is destroyed it becomes useless: no further jobs can be submitted
-	 * to its queues. We simply wait for all references to be dropped so we can
-	 * release the group object.
-	 */
-	bool destroyed;
-
 	/**
 	 * @timedout: True when a timeout occurred on any of the queues owned by
 	 * this group.
@@ -707,6 +707,17 @@ struct panthor_group {
 	 * panthor_group::groups::waiting list.
 	 */
 	struct list_head wait_node;
+
+	/**
+	 * @user_node: Used to insert the group in the panthor_scheduler::groups::user_owned list.
+	 *
+	 * When the group is created, it's inserted in panthor_scheduler::groups::user_owned,
+	 * and when panthor_group_destroy, the group is remove from this list.
+	 *
+	 * When the device is unplugged, all groups that remain in this list must have an extra
+	 * put_group() called on them to release the reference owned by the per-file group pool.
+	 */
+	struct list_head user_node;
 };
 
 struct panthor_job_profiling_data {
@@ -969,6 +980,7 @@ static void group_release(struct kref *kref)
 	struct panthor_device *ptdev = group->ptdev;
 
 	drm_WARN_ON(&ptdev->base, group->csg_id >= 0);
+	drm_WARN_ON(&ptdev->base, !list_empty(&group->user_node));
 	drm_WARN_ON(&ptdev->base, !list_empty(&group->run_node));
 	drm_WARN_ON(&ptdev->base, !list_empty(&group->wait_node));
 
@@ -1003,7 +1015,7 @@ group_can_run(struct panthor_group *group)
 {
 	return group->state != PANTHOR_CS_GROUP_TERMINATED &&
 	       group->state != PANTHOR_CS_GROUP_UNKNOWN_STATE &&
-	       !group->destroyed &&
+	       !list_empty(&group->user_node) &&
 	       !atomic_read(&group->fatal_queues) &&
 	       !atomic_read(&group->timedout);
 }
@@ -2472,7 +2484,7 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
 			 * re-evaluate as soon as possible and get rid of
 			 * this dangling group.
 			 */
-			if (group->destroyed)
+			if (list_empty(&group->user_node))
 				ctx->immediate_tick = true;
 			group_put(group);
 		}
@@ -3667,6 +3679,7 @@ int panthor_group_create(struct panthor_file *pfile,
 	group->tiler_core_mask = group_args->tiler_core_mask;
 	group->priority = group_args->priority;
 
+	INIT_LIST_HEAD(&group->user_node);
 	INIT_LIST_HEAD(&group->wait_node);
 	INIT_LIST_HEAD(&group->run_node);
 	INIT_WORK(&group->term_work, group_term_work);
@@ -3735,8 +3748,13 @@ int panthor_group_create(struct panthor_file *pfile,
 	mutex_lock(&sched->reset.lock);
 	if (atomic_read(&sched->reset.in_progress)) {
 		panthor_group_stop(group);
+
+		mutex_lock(&sched->lock);
+		list_add_tail(&group->user_node, &sched->groups.user_owned);
+		mutex_unlock(&sched->lock);
 	} else {
 		mutex_lock(&sched->lock);
+		list_add_tail(&group->user_node, &sched->groups.user_owned);
 		list_add_tail(&group->run_node,
 			      &sched->groups.idle[group->priority]);
 		mutex_unlock(&sched->lock);
@@ -3776,7 +3794,7 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
 
 	mutex_lock(&sched->reset.lock);
 	mutex_lock(&sched->lock);
-	group->destroyed = true;
+	list_del_init(&group->user_node);
 	if (group->csg_id >= 0) {
 		sched_queue_delayed_work(sched, tick, 0);
 	} else if (!atomic_read(&sched->reset.in_progress)) {
@@ -4143,6 +4161,7 @@ int panthor_sched_init(struct panthor_device *ptdev)
 		INIT_LIST_HEAD(&sched->groups.idle[prio]);
 	}
 	INIT_LIST_HEAD(&sched->groups.waiting);
+	INIT_LIST_HEAD(&sched->groups.user_owned);
 
 	ret = drmm_mutex_init(&ptdev->base, &sched->reset.lock);
 	if (ret)

-- 
2.55.0

[PATCH v4 17/18] drm/panthor: Fix the unplug logic

[Boris Brezillon]

The current unplug logic is broken in multiple subtle ways:

1. it assumes that the HW is still accessible in multiple places,
   which goes against the very concept of hot-unplug
2. it doesn't take into account the fact the stop is a failible
   operation, and that we theoretically have no guarantee that the HW
   is actually stopped after we've released the resources

Those issues are hard to reason about because Mali GPUs are on a
platform bus, which is not hot-pluggable, so they are in practice
always accessible as long as we can enable their dependencies (clocks,
power-domain, ...). The problem is, if the GPU is in such a bad state
it can't properly reset/resume, there are various operations that can't
be done properly, and the unplug logic is clearly not ready for that.
And more importantly, if we can't guarantee the reset was effective,
we have to assume the HW still has access to the resource we passed to
it, meaning we can't return these resources to the system without
risking a UAF.

This patch does several things:

- it resets the GPU before calling the <component>_unplug() functions
- it drops the pm_get/put that around the sub-component unplug calls
  (no longer needed if we assume the HW is gone and can't be accessed
  anymore)
- it changes the _unplug() implementations to not touch the HW anymore
- it let's each component know whether it should leak resources the HW
  might have its hands on at the time the unplug happens
- it releases all resources at unplug time even if open FDs exist. This
  is needed otherwise we could have deferred cleanup work accessing
  objects that have been freed

Unfortunately, I couldn't find a way to break things into multiple
commits while preserving bisectability.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c |  50 +++++++++----
 drivers/gpu/drm/panthor/panthor_drv.c    | 122 ++++++++++++++++++++++++-------
 drivers/gpu/drm/panthor/panthor_fw.c     |   9 +--
 drivers/gpu/drm/panthor/panthor_gpu.c    |   2 +-
 drivers/gpu/drm/panthor/panthor_mmu.c    |  72 +++++++++++-------
 drivers/gpu/drm/panthor/panthor_pwr.c    |   2 +-
 drivers/gpu/drm/panthor/panthor_sched.c  |  90 +++++++++++++++++++++--
 7 files changed, 267 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 817312f598f3..328e601d80e8 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -83,13 +83,28 @@ void panthor_device_unplug(struct panthor_device *ptdev)
 	/* Make sure we're not interrupted by resets while we're unplugging. */
 	disable_work_sync(&ptdev->reset.work);
 
-	drm_WARN_ON(&ptdev->base, pm_runtime_get_sync(ptdev->base.dev) < 0);
+	/* Disable RPM callbacks early, so we're sure external RPM calls won't
+	 * interfere with our unplug logic.
+	 */
+	pm_runtime_dont_use_autosuspend(ptdev->base.dev);
+	pm_runtime_disable(ptdev->base.dev);
 
 	/* Call drm_dev_unplug() so any access to HW blocks happening after
 	 * that point get rejected.
 	 */
 	drm_dev_unplug(&ptdev->base);
 
+	/* If the device is suspended we know for sure the device is idle and
+	 * we can proceed with the rest of the unplug without issuing a
+	 * SOFT_RESET.
+	 */
+	if (!pm_runtime_status_suspended(ptdev->base.dev)) {
+		/* A soft-reset should guarantee that all components of the HW
+		 * are off, meaning we can proceed with the rest of the unplug.
+		 */
+		panthor_hw_soft_reset(ptdev);
+	}
+
 	/* We do the rest of the unplug with the unplug lock released,
 	 * future callers will wait on ptdev->unplug.done anyway.
 	 */
@@ -112,13 +127,21 @@ void panthor_device_unplug(struct panthor_device *ptdev)
 	panthor_gpu_unplug(ptdev);
 	panthor_pwr_unplug(ptdev);
 
-	pm_runtime_dont_use_autosuspend(ptdev->base.dev);
-	pm_runtime_put_sync_suspend(ptdev->base.dev);
-
-	/* If PM is disabled, we need to call the suspend handler manually. */
-	if (!IS_ENABLED(CONFIG_PM))
+	/* If the device is not suspended, suspend it now. Note that it covers
+	 * the !CONFIG_PM case because pm_runtime_status_suspended() always
+	 * returns false in that case.
+	 */
+	if (!pm_runtime_status_suspended(ptdev->base.dev)) {
+		/* RPM callbacks were disabled at the beginning of this function,
+		 * so we need to call the suspend hook manually to return the
+		 * clks/regulators refs we still own.
+		 */
 		panthor_device_suspend(ptdev->base.dev);
 
+		/* Make sure the device is considered suspended by the PM core. */
+		pm_runtime_set_suspended(ptdev->base.dev);
+	}
+
 	/* Report the unplug operation as done to unblock concurrent
 	 * panthor_device_unplug() callers.
 	 */
@@ -280,13 +303,6 @@ int panthor_device_init(struct panthor_device *ptdev)
 	 * will vanish.
 	 */
 	disable_work(&ptdev->reset.work);
-	ret = devm_pm_runtime_enable(ptdev->base.dev);
-	if (ret)
-		return ret;
-
-	ret = pm_runtime_resume_and_get(ptdev->base.dev);
-	if (ret)
-		return ret;
 
 	/* If PM is disabled, we need to call panthor_device_resume() manually. */
 	if (!IS_ENABLED(CONFIG_PM)) {
@@ -295,6 +311,11 @@ int panthor_device_init(struct panthor_device *ptdev)
 			return ret;
 	}
 
+	pm_runtime_enable(ptdev->base.dev);
+	ret = pm_runtime_resume_and_get(ptdev->base.dev);
+	if (ret)
+		goto err_rpm_disable;
+
 	ret = panthor_hw_init(ptdev);
 	if (ret)
 		goto err_rpm_put;
@@ -364,6 +385,9 @@ int panthor_device_init(struct panthor_device *ptdev)
 
 err_rpm_put:
 	pm_runtime_put_sync_suspend(ptdev->base.dev);
+
+err_rpm_disable:
+	pm_runtime_disable(ptdev->base.dev);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 3993e1a81495..f34050aae3fe 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1025,11 +1025,21 @@ static int panthor_ioctl_vm_destroy(struct drm_device *ddev, void *data,
 {
 	struct panthor_file *pfile = file->driver_priv;
 	struct drm_panthor_vm_destroy *args = data;
+	int cookie, ret;
 
-	if (args->pad)
-		return -EINVAL;
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
 
-	return panthor_vm_pool_destroy_vm(pfile->vms, args->id);
+	if (args->pad) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
+
+	ret = panthor_vm_pool_destroy_vm(pfile->vms, args->id);
+
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }
 
 #define PANTHOR_BO_FLAGS		(DRM_PANTHOR_BO_NO_MMAP | \
@@ -1219,11 +1229,21 @@ static int panthor_ioctl_group_destroy(struct drm_device *ddev, void *data,
 {
 	struct panthor_file *pfile = file->driver_priv;
 	struct drm_panthor_group_destroy *args = data;
+	int cookie, ret;
 
-	if (args->pad)
-		return -EINVAL;
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
 
-	return panthor_group_destroy(pfile, args->group_handle);
+	if (args->pad) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
+
+	ret = panthor_group_destroy(pfile, args->group_handle);
+
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }
 
 static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
@@ -1232,27 +1252,36 @@ static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
 	struct panthor_file *pfile = file->driver_priv;
 	struct drm_panthor_group_create *args = data;
 	struct drm_panthor_queue_create *queue_args;
-	int ret;
+	int cookie, ret;
 
-	if (!args->queues.count || args->queues.count > MAX_CS_PER_CSG)
-		return -EINVAL;
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
+
+	if (!args->queues.count || args->queues.count > MAX_CS_PER_CSG) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	ret = PANTHOR_UOBJ_GET_ARRAY(queue_args, &args->queues);
 	if (ret)
-		return ret;
+		goto out_dev_exit;
 
 	ret = group_priority_permit(file, args->priority);
 	if (ret)
-		goto out;
+		goto out_free_args;
 
 	ret = panthor_group_create(pfile, args, queue_args, file->client_id);
 	if (ret < 0)
-		goto out;
+		goto out_free_args;
+
 	args->group_handle = ret;
 	ret = 0;
 
-out:
+out_free_args:
 	kvfree(queue_args);
+
+out_dev_exit:
+	drm_dev_exit(cookie);
 	return ret;
 }
 
@@ -1261,8 +1290,15 @@ static int panthor_ioctl_group_get_state(struct drm_device *ddev, void *data,
 {
 	struct panthor_file *pfile = file->driver_priv;
 	struct drm_panthor_group_get_state *args = data;
+	int cookie, ret;
 
-	return panthor_group_get_state(pfile, args);
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
+
+	ret = panthor_group_get_state(pfile, args);
+
+	drm_dev_exit(cookie);
+	return ret;
 }
 
 static int panthor_ioctl_tiler_heap_create(struct drm_device *ddev, void *data,
@@ -1272,11 +1308,16 @@ static int panthor_ioctl_tiler_heap_create(struct drm_device *ddev, void *data,
 	struct drm_panthor_tiler_heap_create *args = data;
 	struct panthor_heap_pool *pool;
 	struct panthor_vm *vm;
-	int ret;
+	int cookie, ret;
+
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
 
 	vm = panthor_vm_pool_get_vm(pfile->vms, args->vm_id);
-	if (!vm)
-		return -EINVAL;
+	if (!vm) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	pool = panthor_vm_get_heap_pool(vm, true);
 	if (IS_ERR(pool)) {
@@ -1305,6 +1346,9 @@ static int panthor_ioctl_tiler_heap_create(struct drm_device *ddev, void *data,
 
 out_put_vm:
 	panthor_vm_put(vm);
+
+out_dev_exit:
+	drm_dev_exit(cookie);
 	return ret;
 }
 
@@ -1315,14 +1359,21 @@ static int panthor_ioctl_tiler_heap_destroy(struct drm_device *ddev, void *data,
 	struct drm_panthor_tiler_heap_destroy *args = data;
 	struct panthor_heap_pool *pool;
 	struct panthor_vm *vm;
-	int ret;
+	int cookie, ret;
 
-	if (args->pad)
-		return -EINVAL;
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
+
+	if (args->pad) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	vm = panthor_vm_pool_get_vm(pfile->vms, args->handle >> 16);
-	if (!vm)
-		return -EINVAL;
+	if (!vm) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	pool = panthor_vm_get_heap_pool(vm, false);
 	if (IS_ERR(pool)) {
@@ -1335,6 +1386,9 @@ static int panthor_ioctl_tiler_heap_destroy(struct drm_device *ddev, void *data,
 
 out_put_vm:
 	panthor_vm_put(vm);
+
+out_dev_exit:
+	drm_dev_exit(cookie);
 	return ret;
 }
 
@@ -1466,10 +1520,16 @@ static int panthor_ioctl_vm_get_state(struct drm_device *ddev, void *data,
 	struct panthor_file *pfile = file->driver_priv;
 	struct drm_panthor_vm_get_state *args = data;
 	struct panthor_vm *vm;
+	int cookie, ret;
+
+	if (!drm_dev_enter(ddev, &cookie))
+		return -ENODEV;
 
 	vm = panthor_vm_pool_get_vm(pfile->vms, args->vm_id);
-	if (!vm)
-		return -EINVAL;
+	if (!vm) {
+		ret = -EINVAL;
+		goto out_dev_exit;
+	}
 
 	if (panthor_vm_is_unusable(vm))
 		args->state = DRM_PANTHOR_VM_STATE_UNUSABLE;
@@ -1477,7 +1537,11 @@ static int panthor_ioctl_vm_get_state(struct drm_device *ddev, void *data,
 		args->state = DRM_PANTHOR_VM_STATE_USABLE;
 
 	panthor_vm_put(vm);
-	return 0;
+	ret = 0;
+
+out_dev_exit:
+	drm_dev_exit(cookie);
+	return ret;
 }
 
 static int panthor_ioctl_bo_set_label(struct drm_device *ddev, void *data,
@@ -1730,9 +1794,13 @@ static void panthor_show_internal_memory_stats(struct drm_printer *p, struct drm
 	char *drv_name = file->minor->dev->driver->name;
 	struct panthor_file *pfile = file->driver_priv;
 	struct drm_memory_stats stats = {0};
+	int cookie;
 
-	panthor_fdinfo_gather_group_mem_info(pfile, &stats);
-	panthor_vm_heaps_sizes(pfile, &stats);
+	if (drm_dev_enter(&pfile->ptdev->base, &cookie)) {
+		panthor_fdinfo_gather_group_mem_info(pfile, &stats);
+		panthor_vm_heaps_sizes(pfile, &stats);
+		drm_dev_exit(cookie);
+	}
 
 	drm_fdinfo_print_size(p, drv_name, "resident", "memory", stats.resident);
 	drm_fdinfo_print_size(p, drv_name, "active", "memory", stats.active);
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 731da736e372..aafe0a7daac1 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -1290,11 +1290,9 @@ void panthor_fw_unplug(struct panthor_device *ptdev)
 
 	disable_delayed_work_sync(&ptdev->fw->watchdog.ping_work);
 
-	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev)) {
-		/* Make sure the IRQ handler cannot be called after that point. */
+	/* Make sure the IRQ handler cannot be called after that point. */
+	if (!pm_runtime_status_suspended(ptdev->base.dev))
 		panthor_irq_suspend(&ptdev->fw->irq);
-		panthor_fw_stop(ptdev);
-	}
 
 	list_for_each_entry(section, &ptdev->fw->sections, node)
 		panthor_kernel_bo_destroy(section->mem);
@@ -1306,9 +1304,6 @@ void panthor_fw_unplug(struct panthor_device *ptdev)
 	 */
 	panthor_vm_put(ptdev->fw->vm);
 	ptdev->fw->vm = NULL;
-
-	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev))
-		drm_WARN_ON(&ptdev->base, panthor_hw_l2_power_off(ptdev));
 }
 
 /**
diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index 09ebe0294691..1cd19a02d2f2 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.c
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -132,7 +132,7 @@ void panthor_gpu_unplug(struct panthor_device *ptdev)
 	unsigned long flags;
 
 	/* Make sure the IRQ handler is not running after that point. */
-	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev))
+	if (!pm_runtime_status_suspended(ptdev->base.dev))
 		panthor_irq_suspend(&ptdev->gpu->irq);
 
 	/* Wake-up all waiters. */
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 6368bf57b8f5..429aa96d699f 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1799,17 +1799,27 @@ panthor_vm_pool_get_vm(struct panthor_vm_pool *pool, u32 handle)
  */
 void panthor_vm_pool_destroy(struct panthor_file *pfile)
 {
+	struct panthor_device *ptdev = pfile->ptdev;
 	struct panthor_vm *vm;
 	unsigned long i;
+	int cookie;
 
 	if (!pfile->vms)
 		return;
 
-	xa_for_each(&pfile->vms->xa, i, vm)
-		panthor_vm_pool_destroy_vm(pfile->vms, i);
+	/* If device is gone VMs have been destroyed already, and the XArray
+	 * contains pointers to objects that have been freed.
+	 */
+	if (drm_dev_enter(&ptdev->base, &cookie)) {
+		xa_for_each(&pfile->vms->xa, i, vm)
+			panthor_vm_pool_destroy_vm(pfile->vms, i);
+
+		drm_dev_exit(cookie);
+	}
 
 	if (pfile->vms->dummy)
 		drm_gem_object_put(&pfile->vms->dummy->base);
+
 	xa_destroy(&pfile->vms->xa);
 	kfree(pfile->vms);
 }
@@ -2243,6 +2253,11 @@ static bool vm_prep_for_cleanup(struct panthor_vm *vm)
 	}
 
 	if (!drm_dev_enter(&ptdev->base, &cookie)) {
+		/* Device is gone, take the unplug lock to make sure
+		 * panthor_device_stop_before_unplug() has run and
+		 * ::leak_active_resources is valid.
+		 */
+		guard(mutex)(&ptdev->unplug.lock);
 		guard(mutex)(&ptdev->mmu->as.slots_lock);
 
 		/* If we're still on a slot after an unplug, it means
@@ -3616,6 +3631,20 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device *ptdev,
 	return freed;
 }
 
+static struct panthor_vm *
+pop_user_owned_vm(struct panthor_device *ptdev)
+{
+	struct panthor_vm *vm;
+
+	guard(mutex)(&ptdev->mmu->vm.lock);
+	vm = list_first_entry_or_null(&ptdev->mmu->vm.user_owned,
+				      struct panthor_vm, user_node);
+	if (vm)
+		list_del_init(&vm->user_node);
+
+	return vm;
+}
+
 /**
  * panthor_mmu_unplug() - Unplug the MMU logic
  * @ptdev: Device.
@@ -3625,32 +3654,21 @@ panthor_mmu_reclaim_priv_bos(struct panthor_device *ptdev,
  */
 void panthor_mmu_unplug(struct panthor_device *ptdev)
 {
-	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev))
-		panthor_irq_suspend(&ptdev->mmu->irq);
-
-	mutex_lock(&ptdev->mmu->as.slots_lock);
-	for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
-		struct panthor_as *as = ptdev->mmu->as.slots[i].as;
-		int ret;
-
-		if (!as)
-			continue;
-
-		ret = panthor_mmu_as_disable(ptdev, i, false);
-		drm_WARN_ON(&ptdev->base, ret);
-
-		/* Drop the unmap restriction if the disabled worked, so we
-		 * don't leak resources in the normal situation.
-		 */
-		if (!ret)
-			atomic_and(~PANTHOR_AS_FORBID_UNMAP, &as->restrictions);
-
-		panthor_as_release_hw_slot_locked(as);
+	/* Collect non-destroyed user VMs so we can return the ref owned by the
+	 * XArray. If we don't do that, we leak all user VMs that were still
+	 * alive at the point drm_dev_unplug() was called, because
+	 * panthor_ioctl_vm_destroy() bails out early if the device is
+	 * unplugged.
+	 */
+	for (struct panthor_vm *vm = pop_user_owned_vm(ptdev); vm;
+	     vm = pop_user_owned_vm(ptdev)) {
+		panthor_vm_destroy(vm);
 	}
 
-	if (!list_empty(&ptdev->mmu->as.cleanup_list))
-		queue_work(ptdev->cleanup_wq, &ptdev->mmu->vm.cleanup_work);
-	mutex_unlock(&ptdev->mmu->as.slots_lock);
+	if (!pm_runtime_status_suspended(ptdev->base.dev))
+		panthor_irq_suspend(&ptdev->mmu->irq);
+
+	mmu_post_reset_cleanup(ptdev);
 
 	/* Make sure pending VM cleanups are processed before leaving. Those
 	 * cleanups might schedule vm_bind_job cleanups, so keep this
@@ -3658,6 +3676,8 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
 	 */
 	flush_work(&ptdev->mmu->vm.cleanup_work);
 	drm_WARN_ON(&ptdev->base, !list_empty(&ptdev->mmu->as.cleanup_list));
+	drm_WARN_ON(&ptdev->base, !list_empty(&ptdev->mmu->vm.list));
+	drm_WARN_ON(&ptdev->base, !list_empty(&ptdev->mmu->vm.user_owned));
 
 	/* Ensure any pending job cleanup work are executed before returning,
 	 * otherwise those might access objects that are gone if the work is
diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c
index c81e2cc053db..49eb6ff47620 100644
--- a/drivers/gpu/drm/panthor/panthor_pwr.c
+++ b/drivers/gpu/drm/panthor/panthor_pwr.c
@@ -458,7 +458,7 @@ void panthor_pwr_unplug(struct panthor_device *ptdev)
 		return;
 
 	/* Make sure the IRQ handler is not running after that point. */
-	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev))
+	if (!pm_runtime_status_suspended(ptdev->base.dev))
 		panthor_irq_suspend(&ptdev->pwr->irq);
 
 	/* Wake-up all waiters. */
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index bd5dcf4cb580..7d2815b4db0f 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -2548,7 +2548,7 @@ static void tick_work(struct work_struct *work)
 		return;
 
 	ret = panthor_device_resume_and_get(ptdev);
-	if (drm_WARN_ON(&ptdev->base, ret))
+	if (ret)
 		goto out_dev_exit;
 
 	/* If the tick is stopped, calculate when the next tick would be */
@@ -3100,12 +3100,17 @@ void panthor_sched_post_reset(struct panthor_device *ptdev, bool reset_failed)
 void panthor_fdinfo_gather_group_samples(struct panthor_file *pfile)
 {
 	struct panthor_group_pool *gpool = pfile->groups;
+	struct panthor_device *ptdev = pfile->ptdev;
 	struct panthor_group *group;
 	unsigned long i;
+	int cookie;
 
 	if (IS_ERR_OR_NULL(gpool))
 		return;
 
+	if (!drm_dev_enter(&ptdev->base, &cookie))
+		return;
+
 	xa_lock(&gpool->xa);
 	xa_for_each_marked(&gpool->xa, i, group, GROUP_REGISTERED) {
 		guard(spinlock)(&group->fdinfo.lock);
@@ -3115,6 +3120,8 @@ void panthor_fdinfo_gather_group_samples(struct panthor_file *pfile)
 		group->fdinfo.data.time = 0;
 	}
 	xa_unlock(&gpool->xa);
+
+	drm_dev_exit(cookie);
 }
 
 struct panthor_job_ringbuf_instrs {
@@ -3320,7 +3327,7 @@ queue_run_job(struct drm_sched_job *sched_job)
 	}
 
 	ret = panthor_device_resume_and_get(ptdev);
-	if (drm_WARN_ON(&ptdev->base, ret))
+	if (ret)
 		return ERR_PTR(ret);
 
 	mutex_lock(&sched->lock);
@@ -3873,14 +3880,23 @@ int panthor_group_pool_create(struct panthor_file *pfile)
 void panthor_group_pool_destroy(struct panthor_file *pfile)
 {
 	struct panthor_group_pool *gpool = pfile->groups;
+	struct panthor_device *ptdev = pfile->ptdev;
 	struct panthor_group *group;
 	unsigned long i;
+	int cookie;
 
 	if (IS_ERR_OR_NULL(gpool))
 		return;
 
-	xa_for_each(&gpool->xa, i, group)
-		panthor_group_destroy(pfile, i);
+	/* If device is gone groups have been destroyed already, and the XArray
+	 * contains pointers to objects that have been freed.
+	 */
+	if (drm_dev_enter(&ptdev->base, &cookie)) {
+		xa_for_each(&gpool->xa, i, group)
+			panthor_group_destroy(pfile, i);
+
+		drm_dev_exit(cookie);
+	}
 
 	xa_destroy(&gpool->xa);
 	kfree(gpool);
@@ -3899,11 +3915,16 @@ panthor_fdinfo_gather_group_mem_info(struct panthor_file *pfile,
 				     struct drm_memory_stats *stats)
 {
 	struct panthor_group_pool *gpool = pfile->groups;
+	struct panthor_device *ptdev = pfile->ptdev;
 	struct panthor_group *group;
 	unsigned long i;
+	int cookie;
+
+	if (!drm_dev_enter(&ptdev->base, &cookie))
+		return;
 
 	if (IS_ERR_OR_NULL(gpool))
-		return;
+		goto out_dev_exit;
 
 	xa_lock(&gpool->xa);
 	xa_for_each_marked(&gpool->xa, i, group, GROUP_REGISTERED) {
@@ -3912,6 +3933,9 @@ panthor_fdinfo_gather_group_mem_info(struct panthor_file *pfile,
 			stats->active += group->fdinfo.kbo_sizes;
 	}
 	xa_unlock(&gpool->xa);
+
+out_dev_exit:
+	drm_dev_exit(cookie);
 }
 
 static void job_release(struct kref *ref)
@@ -4057,22 +4081,78 @@ void panthor_job_update_resvs(struct drm_exec *exec, struct drm_sched_job *sched
 void panthor_sched_unplug(struct panthor_device *ptdev)
 {
 	struct panthor_scheduler *sched = ptdev->scheduler;
+	struct panthor_group *group, *tmp_group;
+	LIST_HEAD(groups);
 
 	disable_delayed_work_sync(&sched->tick_work);
 	disable_work_sync(&sched->sync_upd_work);
 
 	mutex_lock(&sched->lock);
+
+	/* Do a pass on the on-slot groups, and schedule termination. */
+	for (u32 i = 0; i < sched->csg_slot_count; i++) {
+		struct panthor_csg_slot *csg_slot = &sched->csg_slots[i];
+		struct panthor_group *group = csg_slot->group;
+
+		if (!group)
+			continue;
+
+		group_get(group);
+		group->state = PANTHOR_CS_GROUP_TERMINATED;
+		group_unbind_locked(group);
+		list_del_init(&group->wait_node);
+		group_queue_work(group, term);
+
+		group_put(group);
+	}
+
+	/* Now take care of the non-resident groups. */
+	for (u32 i = 0; i < ARRAY_SIZE(sched->groups.runnable); i++)
+		list_splice_init(&sched->groups.runnable[i], &groups);
+
+	for (u32 i = 0; i < ARRAY_SIZE(sched->groups.idle); i++)
+		list_splice_init(&sched->groups.idle[i], &groups);
+
+	list_for_each_entry_safe(group, tmp_group, &groups, run_node) {
+		list_del_init(&group->run_node);
+		list_del_init(&group->wait_node);
+		group_queue_work(group, term);
+	}
+
+	/* All groups that still have a user handle need a group_put()
+	 * because after drm_dev_unplug() has been called those handles
+	 * can't be released through the GROUP_DESTROY IOCTL anymore.
+	 */
+	list_for_each_entry_safe(group, tmp_group, &sched->groups.user_owned, user_node) {
+		list_del_init(&group->user_node);
+		group_put(group);
+	}
+
 	if (sched->pm.has_ref) {
 		pm_runtime_put(ptdev->base.dev);
 		sched->pm.has_ref = false;
 	}
 	mutex_unlock(&sched->lock);
 
+	/* Ensure all term work are done. */
+	flush_workqueue(sched->wq);
+
+	/* Ensure all tiler OOM work are done. */
+	flush_workqueue(sched->heap_alloc_wq);
+
 	/* Ensure any pending group release work are executed before returning,
 	 * otherwise those might access objects that are gone if the work is
 	 * executed after other components are unplugged.
 	 */
 	flush_workqueue(ptdev->cleanup_wq);
+
+	/* After we've flushed the workqueues, all lists should be empty. */
+	drm_WARN_ON(&ptdev->base, !list_empty(&sched->groups.user_owned));
+	for (u32 i = 0; i < ARRAY_SIZE(sched->groups.runnable); i++)
+		drm_WARN_ON(&ptdev->base, !list_empty(&sched->groups.runnable[i]));
+
+	for (u32 i = 0; i < ARRAY_SIZE(sched->groups.idle); i++)
+		drm_WARN_ON(&ptdev->base, !list_empty(&sched->groups.idle[i]));
 }
 
 static void panthor_sched_fini(struct drm_device *ddev, void *res)

-- 
2.55.0

[PATCH v4 18/18] drm/panthor: Add debugfs knobs to simulate reset failures

[Boris Brezillon]

It's almost impossible to trigger a situation where the reset
doesn't work now that the driver is more mature, so let's add two
knobs to exercise this error path:

- a knob to trigger a reset
- a knob to fake an error in the reset path

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c | 48 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/panthor/panthor_device.h |  8 ++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 328e601d80e8..8bdc511310c0 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -5,6 +5,7 @@
 /* Copyright 2025 ARM Limited. All rights reserved. */
 
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/mm.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
@@ -182,7 +183,10 @@ static void panthor_device_reset_work(struct work_struct *work)
 	panthor_hw_soft_reset(ptdev);
 	panthor_hw_l2_power_on(ptdev);
 	panthor_mmu_post_reset(ptdev);
-	ret = panthor_fw_post_reset(ptdev);
+	if (ptdev->reset.fake_failure)
+		ret = -EIO;
+	else
+		ret = panthor_fw_post_reset(ptdev);
 	atomic_set(&ptdev->reset.pending, 0);
 	panthor_sched_post_reset(ptdev, ret != 0);
 	drm_dev_exit(cookie);
@@ -690,8 +694,50 @@ int panthor_device_suspend(struct device *dev)
 }
 
 #ifdef CONFIG_DEBUG_FS
+static int panthor_device_fake_fw_reset_failure_get(void *data, u64 *val)
+{
+	struct panthor_device *ptdev = data;
+
+	*val = ptdev->reset.fake_failure ? 1 : 0;
+	return 0;
+}
+
+static int panthor_device_fake_fw_reset_failure_set(void *data, u64 val)
+{
+	struct panthor_device *ptdev = data;
+
+	ptdev->reset.fake_failure = val ? true : false;
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(panthor_device_fake_fw_reset_failure_fops,
+			 panthor_device_fake_fw_reset_failure_get,
+			 panthor_device_fake_fw_reset_failure_set, "%llu\n");
+
+static ssize_t panthor_device_reset_file_write(struct file *file,
+					       const char __user *, size_t size,
+					       loff_t *)
+{
+	struct panthor_device *ptdev = file_inode(file)->i_private;
+
+	panthor_device_schedule_reset(ptdev);
+	return size;
+}
+
+static const struct debugfs_short_fops panthor_device_reset_fops = {
+	.write = panthor_device_reset_file_write,
+};
+
 void panthor_device_debugfs_init(struct drm_minor *minor)
 {
+	struct panthor_device *ptdev = container_of(minor->dev, struct panthor_device, base);
+
+	debugfs_create_file("fake_fw_reset_failure", 0644,
+			    minor->debugfs_root, ptdev,
+			    &panthor_device_fake_fw_reset_failure_fops);
+	debugfs_create_file("reset", 0200,
+			    minor->debugfs_root, ptdev,
+			    &panthor_device_reset_fops);
 	panthor_mmu_debugfs_init(minor);
 	panthor_gem_debugfs_init(minor);
 }
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index e12049961912..82ec34347eba 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -294,6 +294,14 @@ struct panthor_device {
 		 * all FW sections to make sure we start from a fresh state.
 		 */
 		bool fast;
+
+		/**
+		 * @fake_failure: When true, pretend the FW boot in the reset path failed.
+		 *
+		 * This is important to check that we're doing the right thing in this very
+		 * unlikely case.
+		 */
+		bool fake_failure;
 	} reset;
 
 	/** @pm: Power management related data. */

-- 
2.55.0

Re: [PATCH v4 01/18] drm/panthor: Disable reset work before unplug

[Liviu Dudau]

On Wed, Aug 26, 2026 at 04:56:00PM +0200, Boris Brezillon wrote:
> Unplug is supposed to be the end of the road, so we need to make sure
> reset works won't execute while we're cleaning up everything as part
> of the unplug, otherwise it would mess up the internal state.
> 
> In order to be able to call disable_work_sync() in the unplug
> path, we need to defer the unplug triggered by the reset logic,
> otherwise we would deadlock.
> 
> Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block")
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_device.c | 21 ++++++++++++++++++++-
>  drivers/gpu/drm/panthor/panthor_device.h |  3 +++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 0b25abebb803..7c55d0c45cfd 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -79,6 +79,9 @@ void panthor_device_unplug(struct panthor_device *ptdev)
>  		return;
>  	}
>  
> +	/* Make sure we're not interrupted by resets while we're unplugging. */
> +	disable_work_sync(&ptdev->reset.work);
> +
>  	drm_WARN_ON(&ptdev->base, pm_runtime_get_sync(ptdev->base.dev) < 0);
>  
>  	/* Call drm_dev_unplug() so any access to HW blocks happening after
> @@ -91,6 +94,13 @@ void panthor_device_unplug(struct panthor_device *ptdev)
>  	 */
>  	mutex_unlock(&ptdev->unplug.lock);
>  
> +	/* Unplug triggered by a device removal might race with the deferred
> +	 * one queued by the reset work. The function covers this concurrent
> +	 * unplug situation, but if we can disable the work before its
> +	 * execution, that's still better.
> +	 */
> +	disable_work(&ptdev->unplug.work);
> +

That's being super cautious and I'm not convinced it is necessary. Anything
queued at this moment should end up waiting for unplug.done.

With the above call removed, I'm OK with the rest of the patch.

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu


>  	/* Now, try to cleanly shutdown the GPU before the device resources
>  	 * get reclaimed.
>  	 */
> @@ -114,6 +124,13 @@ void panthor_device_unplug(struct panthor_device *ptdev)
>  	complete_all(&ptdev->unplug.done);
>  }
>  
> +static void panthor_device_unplug_work(struct work_struct *work)
> +{
> +	struct panthor_device *ptdev = container_of(work, struct panthor_device, unplug.work);
> +
> +	panthor_device_unplug(ptdev);
> +}
> +
>  static void panthor_device_reset_cleanup(struct drm_device *ddev, void *data)
>  {
>  	struct panthor_device *ptdev = container_of(ddev, struct panthor_device, base);
> @@ -148,8 +165,9 @@ static void panthor_device_reset_work(struct work_struct *work)
>  	drm_dev_exit(cookie);
>  
>  	if (ret) {
> -		panthor_device_unplug(ptdev);
> +		disable_work(&ptdev->reset.work);
>  		drm_err(&ptdev->base, "Failed to boot MCU after reset, making device unusable.");
> +		queue_work(ptdev->reset.wq, &ptdev->unplug.work);
>  	}
>  }
>  
> @@ -206,6 +224,7 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	 */
>  	*dummy_page_virt = 1;
>  
> +	INIT_WORK(&ptdev->unplug.work, panthor_device_unplug_work);
>  	INIT_WORK(&ptdev->reset.work, panthor_device_reset_work);
>  	disable_work(&ptdev->reset.work);
>  	ptdev->reset.wq = alloc_ordered_workqueue("panthor-reset-wq", 0);
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index b55a3f9edd41..6529e01e838d 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -268,6 +268,9 @@ struct panthor_device {
>  		 * operation is done.
>  		 */
>  		struct completion done;
> +
> +		/** @work: Unplug work. */
> +		struct work_struct work;
>  	} unplug;
>  
>  	/** @reset: Reset related fields. */
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v4 02/18] drm/panthor: Revisit the reset logic to avoid reset request loss

[Liviu Dudau]

On Wed, Aug 26, 2026 at 04:56:01PM +0200, Boris Brezillon wrote:
> disable/enable_work() provide a ready to use mechanism to temporarily
> disable a work item, so use that instead of the complex state machinery
> based on the PM state.
> 
> This also allows us to automate the reset resubmission in case a reset
> was received while the work item was disabled.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_device.c | 36 +++++++++++++++++++++++++-------
>  drivers/gpu/drm/panthor/panthor_device.h |  3 +--
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 7c55d0c45cfd..2974f4bc0bb1 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -181,6 +181,15 @@ static void panthor_device_free_page(struct drm_device *ddev, void *data)
>  	__free_page(data);
>  }
>  
> +static void enable_resets(struct panthor_device *ptdev)
> +{
> +	/* If a reset has been queued while the work was
> +	 * disabled, reschedule it.
> +	 */
> +	if (enable_work(&ptdev->reset.work) && atomic_read(&ptdev->reset.pending))
> +		queue_work(ptdev->reset.wq, &ptdev->reset.work);
> +}
> +
>  int panthor_device_init(struct panthor_device *ptdev)
>  {
>  	u32 *dummy_page_virt;
> @@ -256,6 +265,13 @@ int panthor_device_init(struct panthor_device *ptdev)
>  
>  	ptdev->phys_addr = res->start;
>  
> +	/* panthor_device_resume() calls enable_resets(), so we need to disable
> +	 * the reset.work manually before this gets called to keep things
> +	 * balanced. We don't bother re-enabling the work if the resume fails,
> +	 * because the whole initialization will fail in that case, and the work
> +	 * will vanish.
> +	 */
> +	disable_work(&ptdev->reset.work);
>  	ret = devm_pm_runtime_enable(ptdev->base.dev);
>  	if (ret)
>  		return ret;
> @@ -305,9 +321,6 @@ int panthor_device_init(struct panthor_device *ptdev)
>  
>  	panthor_gem_init(ptdev);
>  
> -	/* Now that everything is initialized, we can enable the reset work. */
> -	enable_work(&ptdev->reset.work);
> -
>  	/* ~3 frames */
>  	pm_runtime_set_autosuspend_delay(ptdev->base.dev, 50);
>  	pm_runtime_use_autosuspend(ptdev->base.dev);
> @@ -316,6 +329,9 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	if (ret)
>  		goto err_disable_autosuspend;
>  
> +	/* Now that everything is initialized, we can enable the reset work. */
> +	enable_resets(ptdev);
> +
>  	pm_runtime_put_autosuspend(ptdev->base.dev);
>  	return 0;
>  
> @@ -534,10 +550,8 @@ int panthor_device_resume(struct device *dev)
>  		/* If there was a reset pending at the time we suspended the
>  		 * device, we force a slow reset.
>  		 */
> -		if (atomic_read(&ptdev->reset.pending)) {
> +		if (atomic_cmpxchg(&ptdev->reset.pending, 1, 0))
>  			ptdev->reset.fast = false;
> -			atomic_set(&ptdev->reset.pending, 0);
> -		}
>  
>  		ret = panthor_device_resume_hw_components(ptdev);
>  		if (ret && ptdev->reset.fast) {
> @@ -565,6 +579,9 @@ int panthor_device_resume(struct device *dev)
>  			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
>  	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_ACTIVE);
>  	mutex_unlock(&ptdev->pm.mmio_lock);
> +
> +	/* Now that everything is resumed, we can re-enable the reset work. */
> +	enable_resets(ptdev);
>  	return 0;
>  
>  err_suspend_devfreq:
> @@ -604,10 +621,13 @@ int panthor_device_suspend(struct device *dev)
>  			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
>  	mutex_unlock(&ptdev->pm.mmio_lock);
>  
> +	/* Make sure we're not interrupted by resets after that point
> +	 * until the GPU is resumed.
> +	 */
> +	disable_work_sync(&ptdev->reset.work);
> +
>  	if (panthor_device_is_initialized(ptdev) &&
>  	    drm_dev_enter(&ptdev->base, &cookie)) {
> -		cancel_work_sync(&ptdev->reset.work);
> -
>  		/* We prepare everything as if we were resetting the GPU.
>  		 * The end of the reset will happen in the resume path though.
>  		 */
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 6529e01e838d..f55baa21b25e 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -389,8 +389,7 @@ void panthor_device_unplug(struct panthor_device *ptdev);
>   */
>  static inline void panthor_device_schedule_reset(struct panthor_device *ptdev)
>  {
> -	if (!atomic_cmpxchg(&ptdev->reset.pending, 0, 1) &&
> -	    atomic_read(&ptdev->pm.state) == PANTHOR_DEVICE_PM_STATE_ACTIVE)
> +	if (!atomic_cmpxchg(&ptdev->reset.pending, 0, 1))
>  		queue_work(ptdev->reset.wq, &ptdev->reset.work);
>  }
>  
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v4 03/18] drm/panthor: Make panthor_device::pm::state non-atomic

[Liviu Dudau]

On Wed, Aug 26, 2026 at 04:56:02PM +0200, Boris Brezillon wrote:
> Now that the reset logic has been reworked to use disable/enable_work(),
> there's no need for panthor_device::pm::state to be an atomic. It can
> simply be accessed under the same lock we use to touch MMIO mappings.
> 
> While at it, rename the lock to make it clear it protects more than just
> the MMIO logic, and transition locked sections to scoped_guard().
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_device.c | 97 +++++++++++++++++---------------
>  drivers/gpu/drm/panthor/panthor_device.h | 16 ++++--
>  2 files changed, 63 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 2974f4bc0bb1..133e3895cd0a 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -147,8 +147,10 @@ static void panthor_device_reset_work(struct work_struct *work)
>  	/* If the device is entering suspend, we don't reset. A slow reset will
>  	 * be forced at resume time instead.
>  	 */
> -	if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_ACTIVE)
> -		return;
> +	scoped_guard(mutex, &ptdev->pm.lock) {
> +		if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_ACTIVE)
> +			return;
> +	}
>  
>  	if (!drm_dev_enter(&ptdev->base, &cookie))
>  		return;
> @@ -204,7 +206,7 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> -	ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.mmio_lock);
> +	ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.lock);
>  	if (ret)
>  		return ret;
>  
> @@ -213,7 +215,7 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	INIT_LIST_HEAD(&ptdev->gems.node);
>  #endif
>  
> -	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> +	ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
>  	p = alloc_page(GFP_KERNEL | __GFP_ZERO);
>  	if (!p)
>  		return -ENOMEM;
> @@ -432,40 +434,39 @@ static vm_fault_t panthor_mmio_vm_fault(struct vm_fault *vmf)
>  {
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct panthor_device *ptdev = vma->vm_private_data;
> -	u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT;
> -	unsigned long pfn;
> -	pgprot_t pgprot;
>  	vm_fault_t ret;
> -	bool active;
>  	int cookie;
>  
>  	if (!drm_dev_enter(&ptdev->base, &cookie))
>  		return VM_FAULT_SIGBUS;
>  
> -	mutex_lock(&ptdev->pm.mmio_lock);
> -	active = atomic_read(&ptdev->pm.state) == PANTHOR_DEVICE_PM_STATE_ACTIVE;
> +	scoped_guard(mutex, &ptdev->pm.lock) {
> +		bool active = ptdev->pm.state == PANTHOR_DEVICE_PM_STATE_ACTIVE;
> +		u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT;
> +		unsigned long pfn;
> +		pgprot_t pgprot;
>  
> -	switch (offset) {
> -	case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET:
> +		switch (offset) {
> +		case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET:
> +			if (active)
> +				pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID);
> +			else
> +				pfn = page_to_pfn(ptdev->pm.dummy_latest_flush);
> +			break;
> +
> +		default:
> +			ret = VM_FAULT_SIGBUS;
> +			goto out_dev_exit;
> +		}
> +
> +		pgprot = vma->vm_page_prot;
>  		if (active)
> -			pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID);
> -		else
> -			pfn = page_to_pfn(ptdev->pm.dummy_latest_flush);
> -		break;
> +			pgprot = pgprot_noncached(pgprot);
>  
> -	default:
> -		ret = VM_FAULT_SIGBUS;
> -		goto out_unlock;
> +		ret = vmf_insert_pfn_prot(vma, vmf->address, pfn, pgprot);
>  	}
>  
> -	pgprot = vma->vm_page_prot;
> -	if (active)
> -		pgprot = pgprot_noncached(pgprot);
> -
> -	ret = vmf_insert_pfn_prot(vma, vmf->address, pfn, pgprot);
> -
> -out_unlock:
> -	mutex_unlock(&ptdev->pm.mmio_lock);
> +out_dev_exit:
>  	drm_dev_exit(cookie);
>  	return ret;
>  }
> @@ -526,10 +527,13 @@ int panthor_device_resume(struct device *dev)
>  	struct panthor_device *ptdev = dev_get_drvdata(dev);
>  	int ret, cookie;
>  
> -	if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_SUSPENDED)
> -		return -EINVAL;
> +	scoped_guard(mutex, &ptdev->pm.lock) {
> +		if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_SUSPENDED)
> +			return -EINVAL;
> +
> +		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_RESUMING;
> +	}
>  
> -	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_RESUMING);
>  
>  	ret = clk_prepare_enable(ptdev->clks.core);
>  	if (ret)
> @@ -574,11 +578,11 @@ int panthor_device_resume(struct device *dev)
>  	 * are removed and the real iomem mapping will be restored on next
>  	 * access.
>  	 */
> -	mutex_lock(&ptdev->pm.mmio_lock);
> +	mutex_lock(&ptdev->pm.lock);
>  	unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
>  			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
> -	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_ACTIVE);
> -	mutex_unlock(&ptdev->pm.mmio_lock);
> +	ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_ACTIVE;
> +	mutex_unlock(&ptdev->pm.lock);

Can this also use scoped_guard()?

Otherwise, this looks good to me!

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

>  
>  	/* Now that everything is resumed, we can re-enable the reset work. */
>  	enable_resets(ptdev);
> @@ -595,7 +599,9 @@ int panthor_device_resume(struct device *dev)
>  	clk_disable_unprepare(ptdev->clks.core);
>  
>  err_set_suspended:
> -	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> +	scoped_guard(mutex, &ptdev->pm.lock)
> +		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
> +
>  	atomic_set(&ptdev->pm.recovery_needed, 1);
>  	return ret;
>  }
> @@ -605,21 +611,21 @@ int panthor_device_suspend(struct device *dev)
>  	struct panthor_device *ptdev = dev_get_drvdata(dev);
>  	int cookie;
>  
> -	if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_ACTIVE)
> -		return -EINVAL;
> -
>  	/* Clear all IOMEM mappings pointing to this device before we
>  	 * shutdown the power-domain and clocks. Failing to do that results
>  	 * in external aborts when the process accesses the iomem region.
>  	 * We change the state and call unmap_mapping_range() with the
> -	 * mmio_lock held to make sure the vm_fault handler won't set up
> +	 * lock held to make sure the vm_fault handler won't set up
>  	 * invalid mappings.
>  	 */
> -	mutex_lock(&ptdev->pm.mmio_lock);
> -	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDING);
> -	unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
> -			    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
> -	mutex_unlock(&ptdev->pm.mmio_lock);
> +	scoped_guard(mutex, &ptdev->pm.lock) {
> +		if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_ACTIVE)
> +			return -EINVAL;
> +
> +		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDING;
> +		unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
> +				    DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
> +	}
>  
>  	/* Make sure we're not interrupted by resets after that point
>  	 * until the GPU is resumed.
> @@ -644,6 +650,9 @@ int panthor_device_suspend(struct device *dev)
>  	clk_disable_unprepare(ptdev->clks.coregroup);
>  	clk_disable_unprepare(ptdev->clks.stacks);
>  	clk_disable_unprepare(ptdev->clks.core);
> -	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> +
> +	scoped_guard(mutex, &ptdev->pm.lock)
> +		ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index f55baa21b25e..217eec811bdb 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -298,18 +298,22 @@ struct panthor_device {
>  
>  	/** @pm: Power management related data. */
>  	struct {
> -		/** @state: Power state. */
> -		atomic_t state;
> +		/**
> +		 * @state: Power state.
> +		 *
> +		 * Must be accessed with the panthor_device::pm::lock held.
> +		 */
> +		enum panthor_device_pm_state state;
>  
>  		/**
> -		 * @mmio_lock: Lock protecting MMIO userspace CPU mappings.
> +		 * @lock: Lock protecting PM related fields.
>  		 *
>  		 * This is needed to ensure we map the dummy IO pages when
>  		 * the device is being suspended, and the real IO pages when
> -		 * the device is being resumed. We can't just do with the
> -		 * state atomicity to deal with this race.
> +		 * the device is being resumed. We can't just do with an
> +		 * atomic state to deal with this race.
>  		 */
> -		struct mutex mmio_lock;
> +		struct mutex lock;
>  
>  		/**
>  		 * @dummy_latest_flush: Dummy LATEST_FLUSH page.
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v4 04/18] drm/panthor: Flush the cleanup_wq in the unplug path

[Liviu Dudau]

On Wed, Aug 26, 2026 at 04:56:03PM +0200, Boris Brezillon wrote:
> If we don't do that, we might face various UAFs, because the resource
> referenced by these work items might be gone by the time they get
> executed.
> 
> In each subcomponent making use of the panthor_cleanup_wq, we add a
> flush_workqueue() at the end of the _unplug() function. Note that this
> assumes no more work items from this subcomponent gets queued after
> that point, which is not yet guaranteed, but this will be fixed in
> upcoming changes.
> 
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Fixes: 647810ec2476 ("drm/panthor: Add the MMU/VM logical block")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://sashiko.dev/#/patchset/20260625-panthor-signal-from-irq-v5-0-8836a74e0ef9@collabora.com?part=2
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c   | 6 ++++++
>  drivers/gpu/drm/panthor/panthor_sched.c | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index d43ba1c7cd2a..1385ee28bee5 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -3367,6 +3367,12 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
>  		}
>  	}
>  	mutex_unlock(&ptdev->mmu->as.slots_lock);
> +
> +	/* Ensure any pending job cleanup work are executed before returning,
> +	 * otherwise those might access objects that are gone if the work is
> +	 * executed after other components are unplugged.
> +	 */
> +	flush_workqueue(panthor_cleanup_wq);
>  }
>  
>  static void panthor_mmu_release_wq(struct drm_device *ddev, void *res)
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 42adfc76b9e1..a6d57dc1b43e 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -4049,6 +4049,12 @@ void panthor_sched_unplug(struct panthor_device *ptdev)
>  		sched->pm.has_ref = false;
>  	}
>  	mutex_unlock(&sched->lock);
> +
> +	/* Ensure any pending group release work are executed before returning,
> +	 * otherwise those might access objects that are gone if the work is
> +	 * executed after other components are unplugged.
> +	 */
> +	flush_workqueue(panthor_cleanup_wq);
>  }
>  
>  static void panthor_sched_fini(struct drm_device *ddev, void *res)
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v4 05/18] drm/panthor: Make the page table cache and cleanup workqueue device-local

[Liviu Dudau]

On Wed, Aug 26, 2026 at 04:56:04PM +0200, Boris Brezillon wrote:
> There's no clue that we'll ever have an SoC with multiple Mali GPUs,
> and even then, the gains of sharing the PT caches and cleanup workqueue
> remains unclear.
> 
> On the other hand, binding those resources to the device allows us to
> detect leaks or UAF at drm_device removal time instead of when the
> module is removed.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_device.c | 17 ++++---
>  drivers/gpu/drm/panthor/panthor_device.h |  8 ++++
>  drivers/gpu/drm/panthor/panthor_drv.c    | 47 +------------------
>  drivers/gpu/drm/panthor/panthor_mmu.c    | 78 +++++++++++++++-----------------
>  drivers/gpu/drm/panthor/panthor_mmu.h    |  3 --
>  drivers/gpu/drm/panthor/panthor_sched.c  |  4 +-
>  6 files changed, 58 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 133e3895cd0a..08bb4d410941 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -131,12 +131,9 @@ static void panthor_device_unplug_work(struct work_struct *work)
>  	panthor_device_unplug(ptdev);
>  }
>  
> -static void panthor_device_reset_cleanup(struct drm_device *ddev, void *data)
> +static void destroy_wq(struct drm_device *ddev, void *wq)
>  {
> -	struct panthor_device *ptdev = container_of(ddev, struct panthor_device, base);
> -
> -	disable_work_sync(&ptdev->reset.work);
> -	destroy_workqueue(ptdev->reset.wq);
> +	destroy_workqueue(wq);
>  }
>  
>  static void panthor_device_reset_work(struct work_struct *work)
> @@ -242,7 +239,15 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	if (!ptdev->reset.wq)
>  		return -ENOMEM;
>  
> -	ret = drmm_add_action_or_reset(&ptdev->base, panthor_device_reset_cleanup, NULL);
> +	ret = drmm_add_action_or_reset(&ptdev->base, destroy_wq, ptdev->reset.wq);
> +	if (ret)
> +		return ret;
> +
> +	ptdev->cleanup_wq =  alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0);
> +	if (!ptdev->cleanup_wq)
> +		return -ENOMEM;
> +
> +	ret = drmm_add_action_or_reset(&ptdev->base, destroy_wq, ptdev->cleanup_wq);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 217eec811bdb..a7ea475fb407 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -327,6 +327,14 @@ struct panthor_device {
>  		atomic_t recovery_needed;
>  	} pm;
>  
> +	/**
> +	 * @cleanup_wq: Workqueue used for cleanup operations.
> +	 *
> +	 * We create a dedicated workqueue so we can flush on unplug and
> +	 * make sure all resources are freed before we finish the unplug.
> +	 */
> +	struct workqueue_struct *cleanup_wq;
> +
>  	/** @profile_mask: User-set profiling flags for job accounting. */
>  	u32 profile_mask;
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 46a3080b0b20..67db6701699b 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1904,52 +1904,7 @@ static struct platform_driver panthor_driver = {
>  		.dev_groups = panthor_groups,
>  	},
>  };
> -
> -/*
> - * Workqueue used to cleanup stuff.
> - *
> - * We create a dedicated workqueue so we can drain on unplug and
> - * make sure all resources are freed before the module is unloaded.
> - */
> -struct workqueue_struct *panthor_cleanup_wq;
> -
> -static int __init panthor_init(void)
> -{
> -	int ret;
> -
> -	ret = panthor_mmu_pt_cache_init();
> -	if (ret)
> -		return ret;
> -
> -	panthor_cleanup_wq = alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0);
> -	if (!panthor_cleanup_wq) {
> -		pr_err("panthor: Failed to allocate the workqueues");
> -		ret = -ENOMEM;
> -		goto err_mmu_pt_cache_fini;
> -	}
> -
> -	ret = platform_driver_register(&panthor_driver);
> -	if (ret)
> -		goto err_destroy_cleanup_wq;
> -
> -	return 0;
> -
> -err_destroy_cleanup_wq:
> -	destroy_workqueue(panthor_cleanup_wq);
> -
> -err_mmu_pt_cache_fini:
> -	panthor_mmu_pt_cache_fini();
> -	return ret;
> -}
> -module_init(panthor_init);
> -
> -static void __exit panthor_exit(void)
> -{
> -	platform_driver_unregister(&panthor_driver);
> -	destroy_workqueue(panthor_cleanup_wq);
> -	panthor_mmu_pt_cache_fini();
> -}
> -module_exit(panthor_exit);
> +module_platform_driver(panthor_driver);
>  
>  MODULE_AUTHOR("Panthor Project Developers");
>  MODULE_DESCRIPTION("Panthor DRM Driver");
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 1385ee28bee5..b4e41556247a 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -61,6 +61,16 @@ struct panthor_mmu {
>  	/** @irq: The MMU irq. */
>  	struct panthor_irq irq;
>  
> +	/**
> +	 * @pt_cache: Cache used to allocate MMU page tables.
> +	 *
> +	 * The pre-allocation pattern forces us to over-allocate to plan for
> +	 * the worst case scenario, and return the pages we didn't use.
> +	 *
> +	 * Having a kmem_cache allows us to speed allocations.
> +	 */
> +	struct kmem_cache *pt_cache;
> +
>  	/**
>  	 * @as: Address space related fields.
>  	 *
> @@ -440,16 +450,6 @@ struct panthor_vm_bind_job {
>  	struct panthor_vm_op_ctx ctx;
>  };
>  
> -/*
> - * @pt_cache: Cache used to allocate MMU page tables.
> - *
> - * The pre-allocation pattern forces us to over-allocate to plan for
> - * the worst case scenario, and return the pages we didn't use.
> - *
> - * Having a kmem_cache allows us to speed allocations.
> - */
> -static struct kmem_cache *pt_cache;
> -
>  /**
>   * alloc_pt() - Custom page table allocator
>   * @cookie: Cookie passed at page table allocation time.
> @@ -530,7 +530,7 @@ static void free_pt(void *cookie, void *data, size_t size)
>  		return;
>  
>  	/* Return the page to the pt_cache. */
> -	kmem_cache_free(pt_cache, data);
> +	kmem_cache_free(vm->ptdev->mmu->pt_cache, data);
>  }
>  
>  static int wait_ready(struct panthor_device *ptdev, u32 as_nr)
> @@ -1179,7 +1179,8 @@ static void panthor_vm_cleanup_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  				     !op_ctx->map.bo;
>  
>  	if (remaining_pt_count) {
> -		kmem_cache_free_bulk(pt_cache, remaining_pt_count,
> +		kmem_cache_free_bulk(vm->ptdev->mmu->pt_cache,
> +				     remaining_pt_count,
>  				     op_ctx->rsvd_page_tables.pages +
>  				     op_ctx->rsvd_page_tables.ptr);
>  	}
> @@ -1279,7 +1280,8 @@ static void panthor_vm_init_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  	op_ctx->va.addr = va;
>  }
>  
> -static int panthor_vm_op_ctx_prealloc_pts(struct panthor_vm_op_ctx *op_ctx)
> +static int panthor_vm_op_ctx_prealloc_pts(struct panthor_device *ptdev,
> +					  struct panthor_vm_op_ctx *op_ctx)
>  {
>  	u64 size = op_ctx->va.range;
>  	u64 va = op_ctx->va.addr;
> @@ -1298,7 +1300,7 @@ static int panthor_vm_op_ctx_prealloc_pts(struct panthor_vm_op_ctx *op_ctx)
>  	if (!op_ctx->rsvd_page_tables.pages)
>  		return -ENOMEM;
>  
> -	if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
> +	if (!kmem_cache_alloc_bulk(ptdev->mmu->pt_cache, GFP_KERNEL, pt_count,
>  				   op_ctx->rsvd_page_tables.pages)) {
>  		op_ctx->rsvd_page_tables.count = 0;
>  		return -ENOMEM;
> @@ -1385,7 +1387,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  	op_ctx->map.vm_bo = drm_gpuvm_bo_obtain_prealloc(preallocated_vm_bo);
>  	op_ctx->map.bo_offset = op->bo_offset;
>  
> -	ret = panthor_vm_op_ctx_prealloc_pts(op_ctx);
> +	ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx);
>  	if (ret)
>  		goto err_cleanup;
>  
> @@ -1444,8 +1446,9 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  			goto err_cleanup;
>  		}
>  
> -		if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
> -				op_ctx->rsvd_page_tables.pages)) {
> +		if (!kmem_cache_alloc_bulk(vm->ptdev->mmu->pt_cache,
> +					   GFP_KERNEL, pt_count,
> +					   op_ctx->rsvd_page_tables.pages)) {
>  			ret = -ENOMEM;
>  			goto err_cleanup;
>  		}
> @@ -2580,7 +2583,7 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
>  	bool found = false;
>  	int ret;
>  
> -	ret = panthor_vm_op_ctx_prealloc_pts(op_ctx);
> +	ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx);
>  	if (ret)
>  		goto out_cleanup;
>  
> @@ -2821,7 +2824,7 @@ panthor_vm_bind_free_job(struct drm_sched_job *sched_job)
>  	/* Do the heavy cleanups asynchronously, so we're out of the
>  	 * dma-signaling path and can acquire dma-resv locks safely.
>  	 */
> -	queue_work(panthor_cleanup_wq, &job->cleanup_op_ctx_work);
> +	queue_work(job->vm->ptdev->cleanup_wq, &job->cleanup_op_ctx_work);
>  }
>  
>  static enum drm_gpu_sched_stat
> @@ -3372,7 +3375,7 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
>  	 * otherwise those might access objects that are gone if the work is
>  	 * executed after other components are unplugged.
>  	 */
> -	flush_workqueue(panthor_cleanup_wq);
> +	flush_workqueue(ptdev->cleanup_wq);
>  }
>  
>  static void panthor_mmu_release_wq(struct drm_device *ddev, void *res)
> @@ -3385,6 +3388,11 @@ static void panthor_mmu_info_init(struct panthor_device *ptdev)
>  	ptdev->mmu_info.page_size_bitmap = SZ_4K | SZ_2M;
>  }
>  
> +static void free_pt_cache(struct drm_device *, void *pt_cache)
> +{
> +	kmem_cache_destroy(pt_cache);
> +}
> +
>  /**
>   * panthor_mmu_init() - Initialize the MMU logic.
>   * @ptdev: Device.
> @@ -3414,6 +3422,14 @@ int panthor_mmu_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> +	mmu->pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, NULL);
> +	if (!mmu->pt_cache)
> +		return -ENOMEM;
> +
> +	ret = drmm_add_action_or_reset(&ptdev->base, free_pt_cache, mmu->pt_cache);
> +	if (ret)
> +		return ret;
> +
>  	mmu->iomem = ptdev->iomem + MMU_AS_BASE;
>  	ptdev->mmu = mmu;
>  
> @@ -3498,25 +3514,3 @@ void panthor_mmu_debugfs_init(struct drm_minor *minor)
>  				 minor->debugfs_root, minor);
>  }
>  #endif /* CONFIG_DEBUG_FS */
> -
> -/**
> - * panthor_mmu_pt_cache_init() - Initialize the page table cache.
> - *
> - * Return: 0 on success, a negative error code otherwise.
> - */
> -int panthor_mmu_pt_cache_init(void)
> -{
> -	pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, NULL);
> -	if (!pt_cache)
> -		return -ENOMEM;
> -
> -	return 0;
> -}
> -
> -/**
> - * panthor_mmu_pt_cache_fini() - Destroy the page table cache.
> - */
> -void panthor_mmu_pt_cache_fini(void)
> -{
> -	kmem_cache_destroy(pt_cache);
> -}
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
> index 3522fbbce369..de6b4ee4e41a 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.h
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.h
> @@ -104,9 +104,6 @@ void panthor_vm_update_resvs(struct panthor_vm *vm, struct drm_exec *exec,
>  			     enum dma_resv_usage private_usage,
>  			     enum dma_resv_usage extobj_usage);
>  
> -int panthor_mmu_pt_cache_init(void);
> -void panthor_mmu_pt_cache_fini(void);
> -
>  #ifdef CONFIG_DEBUG_FS
>  void panthor_mmu_debugfs_init(struct drm_minor *minor);
>  #endif
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index a6d57dc1b43e..4ea16b40d6b9 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -972,7 +972,7 @@ static void group_release(struct kref *kref)
>  	drm_WARN_ON(&ptdev->base, !list_empty(&group->run_node));
>  	drm_WARN_ON(&ptdev->base, !list_empty(&group->wait_node));
>  
> -	queue_work(panthor_cleanup_wq, &group->release_work);
> +	queue_work(ptdev->cleanup_wq, &group->release_work);
>  }
>  
>  static void group_put(struct panthor_group *group)
> @@ -4054,7 +4054,7 @@ void panthor_sched_unplug(struct panthor_device *ptdev)
>  	 * otherwise those might access objects that are gone if the work is
>  	 * executed after other components are unplugged.
>  	 */
> -	flush_workqueue(panthor_cleanup_wq);
> +	flush_workqueue(ptdev->cleanup_wq);
>  }
>  
>  static void panthor_sched_fini(struct drm_device *ddev, void *res)
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v4 06/18] drm/panthor: Drop unused vm argument passed to panthor_vm_prepare_sync_only_op_ctx()

[Liviu Dudau]

On Wed, Aug 26, 2026 at 04:56:05PM +0200, Boris Brezillon wrote:
> No need to pass a panthor_vm around if it's unused.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index b4e41556247a..44a44602b8ba 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1462,8 +1462,8 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  	return ret;
>  }
>  
> -static void panthor_vm_prepare_sync_only_op_ctx(struct panthor_vm_op_ctx *op_ctx,
> -						struct panthor_vm *vm)
> +static void
> +panthor_vm_prepare_sync_only_op_ctx(struct panthor_vm_op_ctx *op_ctx)
>  {
>  	memset(op_ctx, 0, sizeof(*op_ctx));
>  	op_ctx->flags = DRM_PANTHOR_VM_BIND_OP_TYPE_SYNC_ONLY;
> @@ -3031,7 +3031,7 @@ panthor_vm_bind_prepare_op_ctx(struct drm_file *file,
>  		if (!op->syncs.count)
>  			return -EINVAL;
>  
> -		panthor_vm_prepare_sync_only_op_ctx(op_ctx, vm);
> +		panthor_vm_prepare_sync_only_op_ctx(op_ctx);
>  		return 0;
>  
>  	default:
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯