[PATCH v4 1/6] drm/amdkfd: avoid NULL kfd_process_device in kfd_profiler_pmc

[PATCH v4 1/6] drm/amdkfd: avoid NULL kfd_process_device in kfd_profiler_pmc

[Pierre-Eric Pelloux-Prayer]

All other callers of kfd_process_device_data_by_id deal with
a possible NULL return value except kfd_profiler_pmc.

Check it after the mutex unlock and return -EINVAL as others
place do.

Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=4
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 6c710e4a5220..ad086240a6dd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -3463,8 +3463,10 @@ static inline int kfd_profiler_pmc(struct kfd_process *p,
 	/* Enable/disable perfcount if requested */
 	mutex_lock(&p->mutex);
 	pdd = kfd_process_device_data_by_id(p, args->gpu_id);
-	dqm = pdd->dev->dqm;
 	mutex_unlock(&p->mutex);
+	if (!pdd)
+		return -EINVAL;
+	dqm = pdd->dev->dqm;
 
 	dqm->ops.set_perfcount(dqm, args->perfcount_enable);
 	return status;
-- 
2.43.0

[PATCH v4 2/6] drm/amdgpu: use pci_set_master in amdgpu_device_mode1_reset

[Pierre-Eric Pelloux-Prayer]

It's explicitly disabled above but never re-enabled, assuming
that amdgpu_device_load_pci_state would restore it. But if
pci_clear_master was called before entering amdgpu_device_mode1_reset
the saved state won't have the master status.

Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=4
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5069042b5279..730e9e93d078 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4981,6 +4981,10 @@ int amdgpu_device_mode1_reset(struct amdgpu_device *adev)
 	smp_mb();
 
 	amdgpu_device_load_pci_state(adev->pdev);
+
+	/* enable BM back */
+	pci_set_master(adev->pdev);
+
 	ret = amdgpu_psp_wait_for_bootloader(adev);
 	if (ret)
 		goto mode1_reset_failed;
-- 
2.43.0

Re: [PATCH v4 2/6] drm/amdgpu: use pci_set_master in amdgpu_device_mode1_reset

[Christian König]

On 11/9/26 18:13, Pierre-Eric Pelloux-Prayer wrote:
> It's explicitly disabled above but never re-enabled, assuming
> that amdgpu_device_load_pci_state would restore it. But if
> pci_clear_master was called before entering amdgpu_device_mode1_reset
> the saved state won't have the master status.
> 
> Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=4
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 5069042b5279..730e9e93d078 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4981,6 +4981,10 @@ int amdgpu_device_mode1_reset(struct amdgpu_device *adev)
>  	smp_mb();
>  
>  	amdgpu_device_load_pci_state(adev->pdev);
> +
> +	/* enable BM back */
> +	pci_set_master(adev->pdev);
> +
>  	ret = amdgpu_psp_wait_for_bootloader(adev);
>  	if (ret)
>  		goto mode1_reset_failed;

[PATCH v4 3/6] drm/amdgpu: always clear stable_pstate_ctx in amdgpu_ctx_fini

[Pierre-Eric Pelloux-Prayer]

Even if the hardware is not available we should clear the pointer when
the owning context is destroyed, otherwise nobody will be able to
use this feature again.

Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=5
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 25b1cf7dc988..2b318ccc9aa6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -426,15 +426,15 @@ void amdgpu_ctx_fini(struct kref *ref)
 		}
 	}
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
-		mutex_lock(&adev->pm.stable_pstate_ctx_lock);
-		if (adev->pm.stable_pstate_ctx == ctx) {
+	mutex_lock(&adev->pm.stable_pstate_ctx_lock);
+	if (adev->pm.stable_pstate_ctx == ctx) {
+		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
 			__amdgpu_ctx_set_stable_pstate(ctx, ctx->stable_pstate);
-			adev->pm.stable_pstate_ctx = NULL;
+			drm_dev_exit(idx);
 		}
-		mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
-		drm_dev_exit(idx);
+		adev->pm.stable_pstate_ctx = NULL;
 	}
+	mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
 
 	kfree(ctx);
 }
-- 
2.43.0

Re: [PATCH v4 3/6] drm/amdgpu: always clear stable_pstate_ctx in amdgpu_ctx_fini

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/amdgpu: missing Fixes trailer for amdgpu_ctx_fini() bug fix
--

commit b90ed859fa95af280f43e7bfda0184d737724134
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

> drm/amdgpu: always clear stable_pstate_ctx in amdgpu_ctx_fini
>
> Even if the hardware is not available we should clear the pointer when
> the owning context is destroyed, otherwise nobody will be able to
> use this feature again.
>
> Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=5
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

[Severity: Low]
Since this fixes leaving adev->pm.stable_pstate_ctx set when drm_dev_enter()
returns false in amdgpu_ctx_fini(), should it include a Fixes: tag?

  Fixes: c1dc4ccb82c9 ("drm/amdgpu: Fix context pstate override handling")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109171333.4329-1-pierre-eric.pelloux-prayer@amd.com?part=3

Re: [PATCH v4 3/6] drm/amdgpu: always clear stable_pstate_ctx in amdgpu_ctx_fini

[Christian König]

On 11/9/26 18:13, Pierre-Eric Pelloux-Prayer wrote:
> Even if the hardware is not available we should clear the pointer when
> the owning context is destroyed, otherwise nobody will be able to
> use this feature again.
> 
> Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=5
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 25b1cf7dc988..2b318ccc9aa6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -426,15 +426,15 @@ void amdgpu_ctx_fini(struct kref *ref)
>  		}
>  	}
>  
> -	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> -		mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> -		if (adev->pm.stable_pstate_ctx == ctx) {
> +	mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> +	if (adev->pm.stable_pstate_ctx == ctx) {
> +		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>  			__amdgpu_ctx_set_stable_pstate(ctx, ctx->stable_pstate);
> -			adev->pm.stable_pstate_ctx = NULL;
> +			drm_dev_exit(idx);
>  		}
> -		mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
> -		drm_dev_exit(idx);
> +		adev->pm.stable_pstate_ctx = NULL;
>  	}
> +	mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
>  
>  	kfree(ctx);
>  }

[PATCH v4 4/6] drm/amdgpu: handle gpu_recovery per device

[Pierre-Eric Pelloux-Prayer]

Instead of having a module wide parameter, store gpu_recovery
in amdgpu_device. Its initial value is based on the boot param
and it can be overridden by writing to the existing
amdgpu_gpu_recover debugfs file, so a single device can have
auto-recovery disabled without affecting every other GPU in the
system.

amdgpu_device_should_recover_gpu() now only consider this param.

Assisted-by: LLM
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 11 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 92 +++++++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c     | 21 ++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c       |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c      |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c     |  4 +-
 drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c         |  2 +-
 drivers/gpu/drm/amd/amdgpu/soc15.c            |  2 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |  6 +-
 10 files changed, 81 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5a32b54b1e9b..a486ac746a5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -982,6 +982,9 @@ struct amdgpu_device {
 
 	struct amdgpu_uma_carveout_info uma_info;
 
+	/* Per device gpu_recovery setting. 0 means disabled. */
+	unsigned int gpu_recovery;
+
 	/* KFD
 	 * Must be last --ends in a flexible-array member.
 	 */
@@ -1195,7 +1198,6 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 
 /* Common functions */
 bool amdgpu_device_has_job_running(struct amdgpu_device *adev);
-bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
 int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 			      struct amdgpu_job *job,
 			      struct amdgpu_reset_context *reset_context);
@@ -1323,9 +1325,14 @@ int amdgpu_device_set_cg_state(struct amdgpu_device *adev,
 int amdgpu_device_set_pg_state(struct amdgpu_device *adev,
 			       enum amd_powergating_state state);
 
+static inline bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev)
+{
+	return adev->gpu_recovery != 0;
+}
+
 static inline bool amdgpu_device_has_timeouts_enabled(struct amdgpu_device *adev)
 {
-	return amdgpu_gpu_recovery != 0 &&
+	return amdgpu_device_should_recover_gpu(adev) &&
 		adev->gfx_timeout != MAX_SCHEDULE_TIMEOUT &&
 		adev->compute_timeout != MAX_SCHEDULE_TIMEOUT &&
 		adev->sdma_timeout != MAX_SCHEDULE_TIMEOUT &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 730e9e93d078..a812551b353f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3762,6 +3762,45 @@ amdgpu_device_should_register_switcheroo(struct amdgpu_device *adev, bool px)
 		       apple_gmux_detect(NULL, NULL)));
 }
 
+static void amdgpu_device_init_gpu_recovery(struct amdgpu_device *adev)
+{
+	adev->gpu_recovery = 0;
+
+	if (amdgpu_gpu_recovery == 0)
+		return;
+
+	/* Skip soft reset check in fatal error mode */
+	if (!amdgpu_ras_is_poison_mode_supported(adev) ||
+	    amdgpu_sriov_vf(adev)) {
+		adev->gpu_recovery = adev->gpu_recovery > 0 ?
+			adev->gpu_recovery :-1;
+	} else if (amdgpu_gpu_recovery == -1) {
+		switch (adev->asic_type) {
+#ifdef CONFIG_DRM_AMDGPU_SI
+		case CHIP_VERDE:
+		case CHIP_TAHITI:
+		case CHIP_PITCAIRN:
+		case CHIP_OLAND:
+		case CHIP_HAINAN:
+#endif
+#ifdef CONFIG_DRM_AMDGPU_CIK
+		case CHIP_KAVERI:
+		case CHIP_KABINI:
+		case CHIP_MULLINS:
+#endif
+		case CHIP_CARRIZO:
+		case CHIP_STONEY:
+		case CHIP_CYAN_SKILLFISH:
+			return;
+		default:
+			adev->gpu_recovery = 1;
+			break;
+		}
+	} else {
+		adev->gpu_recovery = amdgpu_gpu_recovery;
+	}
+}
+
 /**
  * amdgpu_device_init - initialize the driver
  *
@@ -4109,6 +4148,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 		}
 	}
 
+	amdgpu_device_init_gpu_recovery(adev);
+
 fence_driver_init:
 	/* Fence driver */
 	r = amdgpu_fence_driver_sw_init(adev);
@@ -4894,57 +4935,6 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
 	return false;
 }
 
-/**
- * amdgpu_device_should_recover_gpu - check if we should try GPU recovery
- *
- * @adev: amdgpu_device pointer
- *
- * Check amdgpu_gpu_recovery and SRIOV status to see if we should try to recover
- * a hung GPU.
- */
-bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev)
-{
-
-	if (amdgpu_gpu_recovery == 0)
-		goto disabled;
-
-	/* Skip soft reset check in fatal error mode */
-	if (!amdgpu_ras_is_poison_mode_supported(adev))
-		return true;
-
-	if (amdgpu_sriov_vf(adev))
-		return true;
-
-	if (amdgpu_gpu_recovery == -1) {
-		switch (adev->asic_type) {
-#ifdef CONFIG_DRM_AMDGPU_SI
-		case CHIP_VERDE:
-		case CHIP_TAHITI:
-		case CHIP_PITCAIRN:
-		case CHIP_OLAND:
-		case CHIP_HAINAN:
-#endif
-#ifdef CONFIG_DRM_AMDGPU_CIK
-		case CHIP_KAVERI:
-		case CHIP_KABINI:
-		case CHIP_MULLINS:
-#endif
-		case CHIP_CARRIZO:
-		case CHIP_STONEY:
-		case CHIP_CYAN_SKILLFISH:
-			goto disabled;
-		default:
-			break;
-		}
-	}
-
-	return true;
-
-disabled:
-		dev_info(adev->dev, "GPU recovery disabled.\n");
-		return false;
-}
-
 int amdgpu_device_mode1_reset(struct amdgpu_device *adev)
 {
 	u32 i;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index bff2d38f3571..a8e23f4e8113 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -980,9 +980,13 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file *m, void *unused)
 }
 
 /*
- * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover
+ * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover,
+ * and control whether this device is allowed to auto-recover from a hang.
  *
- * Manually trigger a gpu reset at the next fence wait.
+ * Read triggers a gpu reset at the next fence wait.
+ *
+ * Write 0/1 to disable/enable auto GPU recovery for this device
+ * (equivalent to amdgpu.gpu_recovery=0, but scoped to this device only).
  */
 static int gpu_recover_get(void *data, u64 *val)
 {
@@ -1006,8 +1010,17 @@ static int gpu_recover_get(void *data, u64 *val)
 	return 0;
 }
 
+static int gpu_recover_set(void *data, u64 val)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)data;
+
+	adev->gpu_recovery = !!val;
+
+	return 0;
+}
+
 DEFINE_SHOW_ATTRIBUTE(amdgpu_debugfs_fence_info);
-DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_gpu_recover_fops, gpu_recover_get, NULL,
+DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_gpu_recover_fops, gpu_recover_get, gpu_recover_set,
 			 "%lld\n");
 
 static void amdgpu_debugfs_reset_work(struct work_struct *work)
@@ -1042,7 +1055,7 @@ void amdgpu_debugfs_fence_init(struct amdgpu_device *adev)
 	if (!amdgpu_sriov_vf(adev)) {
 
 		INIT_WORK(&adev->reset_work, amdgpu_debugfs_reset_work);
-		debugfs_create_file("amdgpu_gpu_recover", 0444, root, adev,
+		debugfs_create_file("amdgpu_gpu_recover", 0644, root, adev,
 				    &amdgpu_debugfs_gpu_recover_fops);
 	}
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 0ae6e990c310..28f9048f806c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -2299,7 +2299,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
 {
 	int r = 0;
 
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return r;
 
 	if (adev->gfx.num_gfx_rings) {
@@ -2319,7 +2319,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
 
 static void amdgpu_gfx_sysfs_reset_mask_fini(struct amdgpu_device *adev)
 {
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return;
 
 	if (adev->gfx.num_gfx_rings)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 4d363bcfb6a5..16391cc2b9a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -111,7 +111,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 	if (!amdgpu_sriov_vf(adev))
 		amdgpu_job_core_dump(adev, job);
 
-	if (amdgpu_gpu_recovery &&
+	if (amdgpu_device_should_recover_gpu(adev) &&
 	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_SOFT_RECOVERY) &&
 	    amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) {
 		dev_err(adev->dev, "ring %s timeout, but soft recovered\n",
@@ -130,7 +130,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 	}
 
 	/* attempt a per ring reset */
-	if (amdgpu_gpu_recovery &&
+	if (amdgpu_device_should_recover_gpu(adev) &&
 	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_PER_QUEUE) &&
 	    ring->funcs->reset) {
 		dev_err(adev->dev, "Starting %s ring reset\n",
@@ -152,7 +152,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 	}
 
 	/* Attempt an IP block soft reset, if supported. */
-	if (amdgpu_gpu_recovery &&
+	if (amdgpu_device_should_recover_gpu(adev) &&
 	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET)) {
 		r = amdgpu_device_ip_soft_reset(ring, job->hw_fence);
 		if (!r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index 87ecbd429f95..8da7c8e91e25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -454,7 +454,7 @@ int amdgpu_sdma_sysfs_reset_mask_init(struct amdgpu_device *adev)
 {
 	int r = 0;
 
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return r;
 
 	if (adev->sdma.num_instances) {
@@ -468,7 +468,7 @@ int amdgpu_sdma_sysfs_reset_mask_init(struct amdgpu_device *adev)
 
 void amdgpu_sdma_sysfs_reset_mask_fini(struct amdgpu_device *adev)
 {
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return;
 
 	if (adev->dev->kobj.sd) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 1a2aa85c504e..64fb04ff2098 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -101,7 +101,7 @@ static void amdgpu_userq_mgr_reset_work(struct work_struct *work)
 	 * If GPU recovery feature is disabled system-wide,
 	 * skip all reset detection logic
 	 */
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return;
 
 	memset(&reset_context, 0, sizeof(reset_context));
@@ -137,7 +137,7 @@ static void amdgpu_userq_hang_detect_work(struct work_struct *work)
 	 * If GPU recovery feature is disabled system-wide,
 	 * skip all reset detection logic
 	 */
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return;
 
 	if (queue->vm && queue->vm->pasid) {
diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
index c3293e5a658c..907f18b195ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
@@ -550,7 +550,7 @@ static int xgpu_vi_mailbox_rcv_irq(struct amdgpu_device *adev,
 	int r;
 
 	/* trigger gpu-reset by hypervisor only if TDR disabled */
-	if (!amdgpu_gpu_recovery) {
+	if (!amdgpu_device_should_recover_gpu(adev)) {
 		/* see what event we get */
 		r = xgpu_vi_mailbox_rcv_msg(adev, IDH_FLR_NOTIFICATION);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index b2024d05aa5f..b35622e65981 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -570,7 +570,7 @@ soc15_asic_reset_method(struct amdgpu_device *adev)
 		 */
 		if (adev->pcie_reset_ctx.in_link_reset)
 			return AMD_RESET_METHOD_LINK;
-		if (amdgpu_gpu_recovery == 4 || amdgpu_gpu_recovery == 5)
+		if (adev->gpu_recovery == 4 || adev->gpu_recovery == 5)
 			return AMD_RESET_METHOD_MODE2;
 		else if (!(adev->flags & AMD_IS_APU))
 			return AMD_RESET_METHOD_MODE1;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index ce67e56efa8c..18fa7c8cc210 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -336,7 +336,7 @@ static int remove_queue_mes_on_reset_option(struct device_queue_manager *dqm, st
 
 	/* remove_hw_queue failure indicates a queue hang. reset the queue */
 	r = reset_queues_mes(dqm, q);
-	if (r && amdgpu_gpu_recovery) {
+	if (r && amdgpu_device_should_recover_gpu(adev)) {
 		dev_err(adev->dev, "failed to remove queue from MES, doorbell=0x%x\n",
 			q->properties.doorbell_off);
 		dev_err(adev->dev, "MES might be in unrecoverable state, issue a GPU reset\n");
@@ -494,7 +494,7 @@ static int reset_queues_mes(struct device_queue_manager *dqm, struct queue *q)
 	if (WARN_ON(dqm->detect_hang_count > 0))
 		return 0;
 
-	if (!amdgpu_gpu_recovery) {
+	if (!amdgpu_device_should_recover_gpu(adev)) {
 		r = -ENOTRECOVERABLE;
 		goto fail;
 	}
@@ -2806,7 +2806,7 @@ static int reset_queues_on_hws_hang(struct device_queue_manager *dqm, bool is_sd
 
 		return -EPERM;
 	}
-	if (!amdgpu_gpu_recovery)
+	if (!amdgpu_device_should_recover_gpu(adev))
 		return -ENOTRECOVERABLE;
 
 	return is_sdma ? reset_hung_queues_sdma(dqm) : reset_hung_queues(dqm);
-- 
2.43.0

Re: [PATCH v4 4/6] drm/amdgpu: handle gpu_recovery per device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] drm/amdgpu: broken gpu_recovery init and ordering in
  amdgpu_device_init()
- [Medium] drm/amdgpu: sysfs reset_mask leak when gpu_recovery is
  disabled at runtime
--

commit da82b226214b96b7716dea3ad2863a26a7b52c07
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: handle gpu_recovery per device

This commit moves the gpu_recovery setting from a global module parameter
check into struct amdgpu_device and makes the amdgpu_gpu_recover debugfs
file writable so auto-recovery can be toggled per device.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 9c6d51445f820..f284e433b72bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3769,6 +3769,45 @@ amdgpu_device_should_register_switcheroo(struct amdgpu_device *adev, bool px)
>  		       apple_gmux_detect(NULL, NULL)));
>  }
>  
> +static void amdgpu_device_init_gpu_recovery(struct amdgpu_device *adev)
> +{
> +	adev->gpu_recovery = 0;
> +
> +	if (amdgpu_gpu_recovery == 0)
> +		return;
> +
> +	/* Skip soft reset check in fatal error mode */
> +	if (!amdgpu_ras_is_poison_mode_supported(adev) ||
> +	    amdgpu_sriov_vf(adev)) {
> +		adev->gpu_recovery = adev->gpu_recovery > 0 ?
> +			adev->gpu_recovery :-1;
> +	} else if (amdgpu_gpu_recovery == -1) {
> +		switch (adev->asic_type) {
> +#ifdef CONFIG_DRM_AMDGPU_SI
> +		case CHIP_VERDE:
> +		case CHIP_TAHITI:
> +		case CHIP_PITCAIRN:
> +		case CHIP_OLAND:
> +		case CHIP_HAINAN:
> +#endif
> +#ifdef CONFIG_DRM_AMDGPU_CIK
> +		case CHIP_KAVERI:
> +		case CHIP_KABINI:
> +		case CHIP_MULLINS:
> +#endif
> +		case CHIP_CARRIZO:
> +		case CHIP_STONEY:
> +		case CHIP_CYAN_SKILLFISH:
> +			return;
> +		default:
> +			adev->gpu_recovery = 1;
> +			break;
> +		}
> +	} else {
> +		adev->gpu_recovery = amdgpu_gpu_recovery;
> +	}
> +}
> +
>  /**
>   * amdgpu_device_init - initialize the driver
>   *
> @@ -4117,6 +4156,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  		}
>  	}
>  
> +	amdgpu_device_init_gpu_recovery(adev);
> +
>  fence_driver_init:
>  	/* Fence driver */
>  	r = amdgpu_fence_driver_sw_init(adev);

[Severity: High]
Does amdgpu_device_init_gpu_recovery() always set adev->gpu_recovery to
(unsigned int)-1 whenever amdgpu_gpu_recovery != 0?

adev->gpu_recovery (declared as unsigned int in struct amdgpu_device) is set
to 0 at the top of the function, and then the first branch tests
adev->gpu_recovery > 0 instead of amdgpu_gpu_recovery > 0.  That overwrites
positive module parameter values such as amdgpu.gpu_recovery=4 or 5 with -1,
so the adev->gpu_recovery == 4 || adev->gpu_recovery == 5 check in
soc15_asic_reset_method() can never select Mode-2 reset.

In addition, amdgpu_device_init() calls amdgpu_device_init_gpu_recovery()
before amdgpu_device_ip_init() -> amdgpu_ras_init() allocates the RAS context
and sets con->poison_supported.  At this point
amdgpu_ras_is_poison_mode_supported(adev) always returns false (and legacy
SI/CIK/Carrizo/Stoney/Cyan Skillfish ASICs do not support RAS poison mode
either), making the else-if per-ASIC exclusion switch and the final else
branch unreachable dead code.  Placing the call here also leaves
adev->gpu_recovery at 0 during the earlier reset-on-init amdgpu_asic_reset()
call and skips it altogether when amdgpu_emu_mode == 1 jumps to
fence_driver_init.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 0ae6e990c3102..28f9048f806cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -2299,7 +2299,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
>  {
>  	int r = 0;
>  
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return r;
>  
>  	if (adev->gfx.num_gfx_rings) {
> @@ -2319,7 +2319,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
>  
>  static void amdgpu_gfx_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>  {
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return;
>  
>  	if (adev->gfx.num_gfx_rings)

[Severity: Medium]
Can disabling gpu_recovery at runtime via debugfs leak the reset_mask sysfs
files on teardown?

If GPU recovery is enabled during sw_init, amdgpu_gfx_sysfs_reset_mask_init()
and amdgpu_sdma_sysfs_reset_mask_init() create gfx_reset_mask,
compute_reset_mask, and sdma_reset_mask on adev->dev.  If a user then writes
0 to amdgpu_gpu_recover in debugfs before driver unbind or module unload,
amdgpu_device_should_recover_gpu(adev) is false in
amdgpu_gfx_sysfs_reset_mask_fini() and amdgpu_sdma_sysfs_reset_mask_fini(),
so device_remove_file() is skipped.  The dangling sysfs attributes remain on
the PCI device, causing a subsequent re-bind to fail with -EEXIST in
device_create_file() and allowing reads of those attributes to access freed
adev memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109171333.4329-1-pierre-eric.pelloux-prayer@amd.com?part=4

Re: [PATCH v4 4/6] drm/amdgpu: handle gpu_recovery per device

[Christian König]

On 11/9/26 18:13, Pierre-Eric Pelloux-Prayer wrote:
> Instead of having a module wide parameter, store gpu_recovery
> in amdgpu_device. Its initial value is based on the boot param
> and it can be overridden by writing to the existing
> amdgpu_gpu_recover debugfs file, so a single device can have
> auto-recovery disabled without affecting every other GPU in the
> system.
> 
> amdgpu_device_should_recover_gpu() now only consider this param.
> 
> Assisted-by: LLM
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 11 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 92 +++++++++----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c     | 21 ++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c       |  4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  6 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c      |  4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c     |  4 +-
>  drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c         |  2 +-
>  drivers/gpu/drm/amd/amdgpu/soc15.c            |  2 +-
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |  6 +-
>  10 files changed, 81 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5a32b54b1e9b..a486ac746a5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -982,6 +982,9 @@ struct amdgpu_device {
>  
>  	struct amdgpu_uma_carveout_info uma_info;
>  
> +	/* Per device gpu_recovery setting. 0 means disabled. */
> +	unsigned int gpu_recovery;
> +
>  	/* KFD
>  	 * Must be last --ends in a flexible-array member.
>  	 */
> @@ -1195,7 +1198,6 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
>  
>  /* Common functions */
>  bool amdgpu_device_has_job_running(struct amdgpu_device *adev);
> -bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
>  int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>  			      struct amdgpu_job *job,
>  			      struct amdgpu_reset_context *reset_context);
> @@ -1323,9 +1325,14 @@ int amdgpu_device_set_cg_state(struct amdgpu_device *adev,
>  int amdgpu_device_set_pg_state(struct amdgpu_device *adev,
>  			       enum amd_powergating_state state);
>  
> +static inline bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev)
> +{
> +	return adev->gpu_recovery != 0;
> +}
> +
>  static inline bool amdgpu_device_has_timeouts_enabled(struct amdgpu_device *adev)
>  {
> -	return amdgpu_gpu_recovery != 0 &&
> +	return amdgpu_device_should_recover_gpu(adev) &&
>  		adev->gfx_timeout != MAX_SCHEDULE_TIMEOUT &&
>  		adev->compute_timeout != MAX_SCHEDULE_TIMEOUT &&
>  		adev->sdma_timeout != MAX_SCHEDULE_TIMEOUT &&
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 730e9e93d078..a812551b353f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3762,6 +3762,45 @@ amdgpu_device_should_register_switcheroo(struct amdgpu_device *adev, bool px)
>  		       apple_gmux_detect(NULL, NULL)));
>  }
>  
> +static void amdgpu_device_init_gpu_recovery(struct amdgpu_device *adev)
> +{
> +	adev->gpu_recovery = 0;
> +
> +	if (amdgpu_gpu_recovery == 0)
> +		return;
> +
> +	/* Skip soft reset check in fatal error mode */
> +	if (!amdgpu_ras_is_poison_mode_supported(adev) ||
> +	    amdgpu_sriov_vf(adev)) {
> +		adev->gpu_recovery = adev->gpu_recovery > 0 ?
> +			adev->gpu_recovery :-1;
> +	} else if (amdgpu_gpu_recovery == -1) {
> +		switch (adev->asic_type) {
> +#ifdef CONFIG_DRM_AMDGPU_SI
> +		case CHIP_VERDE:
> +		case CHIP_TAHITI:
> +		case CHIP_PITCAIRN:
> +		case CHIP_OLAND:
> +		case CHIP_HAINAN:
> +#endif
> +#ifdef CONFIG_DRM_AMDGPU_CIK
> +		case CHIP_KAVERI:
> +		case CHIP_KABINI:
> +		case CHIP_MULLINS:
> +#endif
> +		case CHIP_CARRIZO:
> +		case CHIP_STONEY:
> +		case CHIP_CYAN_SKILLFISH:
> +			return;


Hui? We have recovery disabled by default for those devices?

> +		default:
> +			adev->gpu_recovery = 1;
> +			break;
> +		}
> +	} else {
> +		adev->gpu_recovery = amdgpu_gpu_recovery;
> +	}
> +}
> +
>  /**
>   * amdgpu_device_init - initialize the driver
>   *
> @@ -4109,6 +4148,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  		}
>  	}
>  
> +	amdgpu_device_init_gpu_recovery(adev);
> +
>  fence_driver_init:
>  	/* Fence driver */
>  	r = amdgpu_fence_driver_sw_init(adev);
> @@ -4894,57 +4935,6 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
>  	return false;
>  }
>  
> -/**
> - * amdgpu_device_should_recover_gpu - check if we should try GPU recovery
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Check amdgpu_gpu_recovery and SRIOV status to see if we should try to recover
> - * a hung GPU.
> - */
> -bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev)
> -{
> -
> -	if (amdgpu_gpu_recovery == 0)
> -		goto disabled;
> -
> -	/* Skip soft reset check in fatal error mode */
> -	if (!amdgpu_ras_is_poison_mode_supported(adev))
> -		return true;
> -
> -	if (amdgpu_sriov_vf(adev))
> -		return true;
> -
> -	if (amdgpu_gpu_recovery == -1) {
> -		switch (adev->asic_type) {
> -#ifdef CONFIG_DRM_AMDGPU_SI
> -		case CHIP_VERDE:
> -		case CHIP_TAHITI:
> -		case CHIP_PITCAIRN:
> -		case CHIP_OLAND:
> -		case CHIP_HAINAN:
> -#endif
> -#ifdef CONFIG_DRM_AMDGPU_CIK
> -		case CHIP_KAVERI:
> -		case CHIP_KABINI:
> -		case CHIP_MULLINS:
> -#endif
> -		case CHIP_CARRIZO:
> -		case CHIP_STONEY:
> -		case CHIP_CYAN_SKILLFISH:
> -			goto disabled;
> -		default:
> -			break;
> -		}
> -	}
> -
> -	return true;
> -
> -disabled:
> -		dev_info(adev->dev, "GPU recovery disabled.\n");
> -		return false;
> -}
> -
>  int amdgpu_device_mode1_reset(struct amdgpu_device *adev)
>  {
>  	u32 i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index bff2d38f3571..a8e23f4e8113 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -980,9 +980,13 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file *m, void *unused)
>  }
>  
>  /*
> - * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover
> + * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover,
> + * and control whether this device is allowed to auto-recover from a hang.
>   *
> - * Manually trigger a gpu reset at the next fence wait.
> + * Read triggers a gpu reset at the next fence wait.
> + *
> + * Write 0/1 to disable/enable auto GPU recovery for this device
> + * (equivalent to amdgpu.gpu_recovery=0, but scoped to this device only).
>   */
>  static int gpu_recover_get(void *data, u64 *val)
>  {
> @@ -1006,8 +1010,17 @@ static int gpu_recover_get(void *data, u64 *val)
>  	return 0;
>  }
>  
> +static int gpu_recover_set(void *data, u64 val)
> +{
> +	struct amdgpu_device *adev = (struct amdgpu_device *)data;
> +
> +	adev->gpu_recovery = !!val;
> +
> +	return 0;
> +}
> +
>  DEFINE_SHOW_ATTRIBUTE(amdgpu_debugfs_fence_info);
> -DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_gpu_recover_fops, gpu_recover_get, NULL,
> +DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_gpu_recover_fops, gpu_recover_get, gpu_recover_set,
>  			 "%lld\n");
>  
>  static void amdgpu_debugfs_reset_work(struct work_struct *work)
> @@ -1042,7 +1055,7 @@ void amdgpu_debugfs_fence_init(struct amdgpu_device *adev)
>  	if (!amdgpu_sriov_vf(adev)) {
>  
>  		INIT_WORK(&adev->reset_work, amdgpu_debugfs_reset_work);
> -		debugfs_create_file("amdgpu_gpu_recover", 0444, root, adev,
> +		debugfs_create_file("amdgpu_gpu_recover", 0644, root, adev,
>  				    &amdgpu_debugfs_gpu_recover_fops);

You can also use debugfs_create_bool() here and drop the extra recover fops.

Apart from that looks good to me,
Christian.

>  	}
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 0ae6e990c310..28f9048f806c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -2299,7 +2299,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
>  {
>  	int r = 0;
>  
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return r;
>  
>  	if (adev->gfx.num_gfx_rings) {
> @@ -2319,7 +2319,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
>  
>  static void amdgpu_gfx_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>  {
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return;
>  
>  	if (adev->gfx.num_gfx_rings)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 4d363bcfb6a5..16391cc2b9a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -111,7 +111,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  	if (!amdgpu_sriov_vf(adev))
>  		amdgpu_job_core_dump(adev, job);
>  
> -	if (amdgpu_gpu_recovery &&
> +	if (amdgpu_device_should_recover_gpu(adev) &&
>  	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_SOFT_RECOVERY) &&
>  	    amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) {
>  		dev_err(adev->dev, "ring %s timeout, but soft recovered\n",
> @@ -130,7 +130,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  	}
>  
>  	/* attempt a per ring reset */
> -	if (amdgpu_gpu_recovery &&
> +	if (amdgpu_device_should_recover_gpu(adev) &&
>  	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_PER_QUEUE) &&
>  	    ring->funcs->reset) {
>  		dev_err(adev->dev, "Starting %s ring reset\n",
> @@ -152,7 +152,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  	}
>  
>  	/* Attempt an IP block soft reset, if supported. */
> -	if (amdgpu_gpu_recovery &&
> +	if (amdgpu_device_should_recover_gpu(adev) &&
>  	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET)) {
>  		r = amdgpu_device_ip_soft_reset(ring, job->hw_fence);
>  		if (!r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 87ecbd429f95..8da7c8e91e25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -454,7 +454,7 @@ int amdgpu_sdma_sysfs_reset_mask_init(struct amdgpu_device *adev)
>  {
>  	int r = 0;
>  
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return r;
>  
>  	if (adev->sdma.num_instances) {
> @@ -468,7 +468,7 @@ int amdgpu_sdma_sysfs_reset_mask_init(struct amdgpu_device *adev)
>  
>  void amdgpu_sdma_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>  {
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return;
>  
>  	if (adev->dev->kobj.sd) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 1a2aa85c504e..64fb04ff2098 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -101,7 +101,7 @@ static void amdgpu_userq_mgr_reset_work(struct work_struct *work)
>  	 * If GPU recovery feature is disabled system-wide,
>  	 * skip all reset detection logic
>  	 */
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return;
>  
>  	memset(&reset_context, 0, sizeof(reset_context));
> @@ -137,7 +137,7 @@ static void amdgpu_userq_hang_detect_work(struct work_struct *work)
>  	 * If GPU recovery feature is disabled system-wide,
>  	 * skip all reset detection logic
>  	 */
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return;
>  
>  	if (queue->vm && queue->vm->pasid) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
> index c3293e5a658c..907f18b195ba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
> @@ -550,7 +550,7 @@ static int xgpu_vi_mailbox_rcv_irq(struct amdgpu_device *adev,
>  	int r;
>  
>  	/* trigger gpu-reset by hypervisor only if TDR disabled */
> -	if (!amdgpu_gpu_recovery) {
> +	if (!amdgpu_device_should_recover_gpu(adev)) {
>  		/* see what event we get */
>  		r = xgpu_vi_mailbox_rcv_msg(adev, IDH_FLR_NOTIFICATION);
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index b2024d05aa5f..b35622e65981 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -570,7 +570,7 @@ soc15_asic_reset_method(struct amdgpu_device *adev)
>  		 */
>  		if (adev->pcie_reset_ctx.in_link_reset)
>  			return AMD_RESET_METHOD_LINK;
> -		if (amdgpu_gpu_recovery == 4 || amdgpu_gpu_recovery == 5)
> +		if (adev->gpu_recovery == 4 || adev->gpu_recovery == 5)
>  			return AMD_RESET_METHOD_MODE2;
>  		else if (!(adev->flags & AMD_IS_APU))
>  			return AMD_RESET_METHOD_MODE1;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index ce67e56efa8c..18fa7c8cc210 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -336,7 +336,7 @@ static int remove_queue_mes_on_reset_option(struct device_queue_manager *dqm, st
>  
>  	/* remove_hw_queue failure indicates a queue hang. reset the queue */
>  	r = reset_queues_mes(dqm, q);
> -	if (r && amdgpu_gpu_recovery) {
> +	if (r && amdgpu_device_should_recover_gpu(adev)) {
>  		dev_err(adev->dev, "failed to remove queue from MES, doorbell=0x%x\n",
>  			q->properties.doorbell_off);
>  		dev_err(adev->dev, "MES might be in unrecoverable state, issue a GPU reset\n");
> @@ -494,7 +494,7 @@ static int reset_queues_mes(struct device_queue_manager *dqm, struct queue *q)
>  	if (WARN_ON(dqm->detect_hang_count > 0))
>  		return 0;
>  
> -	if (!amdgpu_gpu_recovery) {
> +	if (!amdgpu_device_should_recover_gpu(adev)) {
>  		r = -ENOTRECOVERABLE;
>  		goto fail;
>  	}
> @@ -2806,7 +2806,7 @@ static int reset_queues_on_hws_hang(struct device_queue_manager *dqm, bool is_sd
>  
>  		return -EPERM;
>  	}
> -	if (!amdgpu_gpu_recovery)
> +	if (!amdgpu_device_should_recover_gpu(adev))
>  		return -ENOTRECOVERABLE;
>  
>  	return is_sdma ? reset_hung_queues_sdma(dqm) : reset_hung_queues(dqm);

Re: [PATCH v4 4/6] drm/amdgpu: handle gpu_recovery per device

[Pierre-Eric Pelloux-Prayer]

Le 10/11/2026 à 10:07, Christian König a écrit :
> On 11/9/26 18:13, Pierre-Eric Pelloux-Prayer wrote:
>> Instead of having a module wide parameter, store gpu_recovery
>> in amdgpu_device. Its initial value is based on the boot param
>> and it can be overridden by writing to the existing
>> amdgpu_gpu_recover debugfs file, so a single device can have
>> auto-recovery disabled without affecting every other GPU in the
>> system.
>>
>> amdgpu_device_should_recover_gpu() now only consider this param.
>>
>> Assisted-by: LLM
>> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 11 ++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 92 +++++++++----------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c     | 21 ++++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c       |  4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |  6 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c      |  4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c     |  4 +-
>>   drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c         |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/soc15.c            |  2 +-
>>   .../drm/amd/amdkfd/kfd_device_queue_manager.c |  6 +-
>>   10 files changed, 81 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 5a32b54b1e9b..a486ac746a5b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -982,6 +982,9 @@ struct amdgpu_device {
>>   
>>   	struct amdgpu_uma_carveout_info uma_info;
>>   
>> +	/* Per device gpu_recovery setting. 0 means disabled. */
>> +	unsigned int gpu_recovery;
>> +
>>   	/* KFD
>>   	 * Must be last --ends in a flexible-array member.
>>   	 */
>> @@ -1195,7 +1198,6 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
>>   
>>   /* Common functions */
>>   bool amdgpu_device_has_job_running(struct amdgpu_device *adev);
>> -bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
>>   int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>   			      struct amdgpu_job *job,
>>   			      struct amdgpu_reset_context *reset_context);
>> @@ -1323,9 +1325,14 @@ int amdgpu_device_set_cg_state(struct amdgpu_device *adev,
>>   int amdgpu_device_set_pg_state(struct amdgpu_device *adev,
>>   			       enum amd_powergating_state state);
>>   
>> +static inline bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev)
>> +{
>> +	return adev->gpu_recovery != 0;
>> +}
>> +
>>   static inline bool amdgpu_device_has_timeouts_enabled(struct amdgpu_device *adev)
>>   {
>> -	return amdgpu_gpu_recovery != 0 &&
>> +	return amdgpu_device_should_recover_gpu(adev) &&
>>   		adev->gfx_timeout != MAX_SCHEDULE_TIMEOUT &&
>>   		adev->compute_timeout != MAX_SCHEDULE_TIMEOUT &&
>>   		adev->sdma_timeout != MAX_SCHEDULE_TIMEOUT &&
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 730e9e93d078..a812551b353f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3762,6 +3762,45 @@ amdgpu_device_should_register_switcheroo(struct amdgpu_device *adev, bool px)
>>   		       apple_gmux_detect(NULL, NULL)));
>>   }
>>   
>> +static void amdgpu_device_init_gpu_recovery(struct amdgpu_device *adev)
>> +{
>> +	adev->gpu_recovery = 0;
>> +
>> +	if (amdgpu_gpu_recovery == 0)
>> +		return;
>> +
>> +	/* Skip soft reset check in fatal error mode */
>> +	if (!amdgpu_ras_is_poison_mode_supported(adev) ||
>> +	    amdgpu_sriov_vf(adev)) {
>> +		adev->gpu_recovery = adev->gpu_recovery > 0 ?
>> +			adev->gpu_recovery :-1;
>> +	} else if (amdgpu_gpu_recovery == -1) {
>> +		switch (adev->asic_type) {
>> +#ifdef CONFIG_DRM_AMDGPU_SI
>> +		case CHIP_VERDE:
>> +		case CHIP_TAHITI:
>> +		case CHIP_PITCAIRN:
>> +		case CHIP_OLAND:
>> +		case CHIP_HAINAN:
>> +#endif
>> +#ifdef CONFIG_DRM_AMDGPU_CIK
>> +		case CHIP_KAVERI:
>> +		case CHIP_KABINI:
>> +		case CHIP_MULLINS:
>> +#endif
>> +		case CHIP_CARRIZO:
>> +		case CHIP_STONEY:
>> +		case CHIP_CYAN_SKILLFISH:
>> +			return;
> 
> 
> Hui? We have recovery disabled by default for those devices?

Yes, this commit doesn't change the existing logic.

> 
>> +		default:
>> +			adev->gpu_recovery = 1;
>> +			break;
>> +		}
>> +	} else {
>> +		adev->gpu_recovery = amdgpu_gpu_recovery;
>> +	}
>> +}
>> +
>>   /**
>>    * amdgpu_device_init - initialize the driver
>>    *
>> @@ -4109,6 +4148,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>   		}
>>   	}
>>   
>> +	amdgpu_device_init_gpu_recovery(adev);
>> +
>>   fence_driver_init:
>>   	/* Fence driver */
>>   	r = amdgpu_fence_driver_sw_init(adev);
>> @@ -4894,57 +4935,6 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
>>   	return false;
>>   }
>>   
>> -/**
>> - * amdgpu_device_should_recover_gpu - check if we should try GPU recovery
>> - *
>> - * @adev: amdgpu_device pointer
>> - *
>> - * Check amdgpu_gpu_recovery and SRIOV status to see if we should try to recover
>> - * a hung GPU.
>> - */
>> -bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev)
>> -{
>> -
>> -	if (amdgpu_gpu_recovery == 0)
>> -		goto disabled;
>> -
>> -	/* Skip soft reset check in fatal error mode */
>> -	if (!amdgpu_ras_is_poison_mode_supported(adev))
>> -		return true;
>> -
>> -	if (amdgpu_sriov_vf(adev))
>> -		return true;
>> -
>> -	if (amdgpu_gpu_recovery == -1) {
>> -		switch (adev->asic_type) {
>> -#ifdef CONFIG_DRM_AMDGPU_SI
>> -		case CHIP_VERDE:
>> -		case CHIP_TAHITI:
>> -		case CHIP_PITCAIRN:
>> -		case CHIP_OLAND:
>> -		case CHIP_HAINAN:
>> -#endif
>> -#ifdef CONFIG_DRM_AMDGPU_CIK
>> -		case CHIP_KAVERI:
>> -		case CHIP_KABINI:
>> -		case CHIP_MULLINS:
>> -#endif
>> -		case CHIP_CARRIZO:
>> -		case CHIP_STONEY:
>> -		case CHIP_CYAN_SKILLFISH:
>> -			goto disabled;
>> -		default:
>> -			break;
>> -		}
>> -	}
>> -
>> -	return true;
>> -
>> -disabled:
>> -		dev_info(adev->dev, "GPU recovery disabled.\n");
>> -		return false;
>> -}
>> -
>>   int amdgpu_device_mode1_reset(struct amdgpu_device *adev)
>>   {
>>   	u32 i;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> index bff2d38f3571..a8e23f4e8113 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> @@ -980,9 +980,13 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file *m, void *unused)
>>   }
>>   
>>   /*
>> - * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover
>> + * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover,
>> + * and control whether this device is allowed to auto-recover from a hang.
>>    *
>> - * Manually trigger a gpu reset at the next fence wait.
>> + * Read triggers a gpu reset at the next fence wait.
>> + *
>> + * Write 0/1 to disable/enable auto GPU recovery for this device
>> + * (equivalent to amdgpu.gpu_recovery=0, but scoped to this device only).
>>    */
>>   static int gpu_recover_get(void *data, u64 *val)
>>   {
>> @@ -1006,8 +1010,17 @@ static int gpu_recover_get(void *data, u64 *val)
>>   	return 0;
>>   }
>>   
>> +static int gpu_recover_set(void *data, u64 val)
>> +{
>> +	struct amdgpu_device *adev = (struct amdgpu_device *)data;
>> +
>> +	adev->gpu_recovery = !!val;
>> +
>> +	return 0;
>> +}
>> +
>>   DEFINE_SHOW_ATTRIBUTE(amdgpu_debugfs_fence_info);
>> -DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_gpu_recover_fops, gpu_recover_get, NULL,
>> +DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_gpu_recover_fops, gpu_recover_get, gpu_recover_set,
>>   			 "%lld\n");
>>   
>>   static void amdgpu_debugfs_reset_work(struct work_struct *work)
>> @@ -1042,7 +1055,7 @@ void amdgpu_debugfs_fence_init(struct amdgpu_device *adev)
>>   	if (!amdgpu_sriov_vf(adev)) {
>>   
>>   		INIT_WORK(&adev->reset_work, amdgpu_debugfs_reset_work);
>> -		debugfs_create_file("amdgpu_gpu_recover", 0444, root, adev,
>> +		debugfs_create_file("amdgpu_gpu_recover", 0644, root, adev,
>>   				    &amdgpu_debugfs_gpu_recover_fops);
> 
> You can also use debugfs_create_bool() here and drop the extra recover fops.

We need a custom getter because it triggers the GPU recovery so 
debugfs_create_bool won't work.


Pierre-Erioc

> 
> Apart from that looks good to me,
> Christian.
> 
>>   	}
>>   #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index 0ae6e990c310..28f9048f806c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -2299,7 +2299,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
>>   {
>>   	int r = 0;
>>   
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return r;
>>   
>>   	if (adev->gfx.num_gfx_rings) {
>> @@ -2319,7 +2319,7 @@ static int amdgpu_gfx_sysfs_reset_mask_init(struct amdgpu_device *adev)
>>   
>>   static void amdgpu_gfx_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>>   {
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return;
>>   
>>   	if (adev->gfx.num_gfx_rings)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> index 4d363bcfb6a5..16391cc2b9a5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> @@ -111,7 +111,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>>   	if (!amdgpu_sriov_vf(adev))
>>   		amdgpu_job_core_dump(adev, job);
>>   
>> -	if (amdgpu_gpu_recovery &&
>> +	if (amdgpu_device_should_recover_gpu(adev) &&
>>   	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_SOFT_RECOVERY) &&
>>   	    amdgpu_ring_soft_recovery(ring, job->vmid, s_job->s_fence->parent)) {
>>   		dev_err(adev->dev, "ring %s timeout, but soft recovered\n",
>> @@ -130,7 +130,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>>   	}
>>   
>>   	/* attempt a per ring reset */
>> -	if (amdgpu_gpu_recovery &&
>> +	if (amdgpu_device_should_recover_gpu(adev) &&
>>   	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_PER_QUEUE) &&
>>   	    ring->funcs->reset) {
>>   		dev_err(adev->dev, "Starting %s ring reset\n",
>> @@ -152,7 +152,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>>   	}
>>   
>>   	/* Attempt an IP block soft reset, if supported. */
>> -	if (amdgpu_gpu_recovery &&
>> +	if (amdgpu_device_should_recover_gpu(adev) &&
>>   	    amdgpu_ring_is_reset_type_supported(ring, AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET)) {
>>   		r = amdgpu_device_ip_soft_reset(ring, job->hw_fence);
>>   		if (!r) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> index 87ecbd429f95..8da7c8e91e25 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>> @@ -454,7 +454,7 @@ int amdgpu_sdma_sysfs_reset_mask_init(struct amdgpu_device *adev)
>>   {
>>   	int r = 0;
>>   
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return r;
>>   
>>   	if (adev->sdma.num_instances) {
>> @@ -468,7 +468,7 @@ int amdgpu_sdma_sysfs_reset_mask_init(struct amdgpu_device *adev)
>>   
>>   void amdgpu_sdma_sysfs_reset_mask_fini(struct amdgpu_device *adev)
>>   {
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return;
>>   
>>   	if (adev->dev->kobj.sd) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index 1a2aa85c504e..64fb04ff2098 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -101,7 +101,7 @@ static void amdgpu_userq_mgr_reset_work(struct work_struct *work)
>>   	 * If GPU recovery feature is disabled system-wide,
>>   	 * skip all reset detection logic
>>   	 */
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return;
>>   
>>   	memset(&reset_context, 0, sizeof(reset_context));
>> @@ -137,7 +137,7 @@ static void amdgpu_userq_hang_detect_work(struct work_struct *work)
>>   	 * If GPU recovery feature is disabled system-wide,
>>   	 * skip all reset detection logic
>>   	 */
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return;
>>   
>>   	if (queue->vm && queue->vm->pasid) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>> index c3293e5a658c..907f18b195ba 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>> @@ -550,7 +550,7 @@ static int xgpu_vi_mailbox_rcv_irq(struct amdgpu_device *adev,
>>   	int r;
>>   
>>   	/* trigger gpu-reset by hypervisor only if TDR disabled */
>> -	if (!amdgpu_gpu_recovery) {
>> +	if (!amdgpu_device_should_recover_gpu(adev)) {
>>   		/* see what event we get */
>>   		r = xgpu_vi_mailbox_rcv_msg(adev, IDH_FLR_NOTIFICATION);
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> index b2024d05aa5f..b35622e65981 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> @@ -570,7 +570,7 @@ soc15_asic_reset_method(struct amdgpu_device *adev)
>>   		 */
>>   		if (adev->pcie_reset_ctx.in_link_reset)
>>   			return AMD_RESET_METHOD_LINK;
>> -		if (amdgpu_gpu_recovery == 4 || amdgpu_gpu_recovery == 5)
>> +		if (adev->gpu_recovery == 4 || adev->gpu_recovery == 5)
>>   			return AMD_RESET_METHOD_MODE2;
>>   		else if (!(adev->flags & AMD_IS_APU))
>>   			return AMD_RESET_METHOD_MODE1;
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index ce67e56efa8c..18fa7c8cc210 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -336,7 +336,7 @@ static int remove_queue_mes_on_reset_option(struct device_queue_manager *dqm, st
>>   
>>   	/* remove_hw_queue failure indicates a queue hang. reset the queue */
>>   	r = reset_queues_mes(dqm, q);
>> -	if (r && amdgpu_gpu_recovery) {
>> +	if (r && amdgpu_device_should_recover_gpu(adev)) {
>>   		dev_err(adev->dev, "failed to remove queue from MES, doorbell=0x%x\n",
>>   			q->properties.doorbell_off);
>>   		dev_err(adev->dev, "MES might be in unrecoverable state, issue a GPU reset\n");
>> @@ -494,7 +494,7 @@ static int reset_queues_mes(struct device_queue_manager *dqm, struct queue *q)
>>   	if (WARN_ON(dqm->detect_hang_count > 0))
>>   		return 0;
>>   
>> -	if (!amdgpu_gpu_recovery) {
>> +	if (!amdgpu_device_should_recover_gpu(adev)) {
>>   		r = -ENOTRECOVERABLE;
>>   		goto fail;
>>   	}
>> @@ -2806,7 +2806,7 @@ static int reset_queues_on_hws_hang(struct device_queue_manager *dqm, bool is_sd
>>   
>>   		return -EPERM;
>>   	}
>> -	if (!amdgpu_gpu_recovery)
>> +	if (!amdgpu_device_should_recover_gpu(adev))
>>   		return -ENOTRECOVERABLE;
>>   
>>   	return is_sdma ? reset_hung_queues_sdma(dqm) : reset_hung_queues(dqm);
> 

[PATCH v4 5/6] drm/amdgpu: add wedge event implementation

[Pierre-Eric Pelloux-Prayer]

When a job times out the kernel tries to reset the GPU to get
the system back to a normal state.
Sometimes it's useful to disable this process - for instance to
be able to inspect the hardware state at the time of the hang.
Since leaving fence unsignalled might affect the system's
stability, this commit enables the drm wedge framework for amdgpu.

When a hang condition is detected, the GPU isn't reset but all
the pending fences are signalled and the hung GPU cannot receive
new work. For KFD it's implemented by making
kfd_process_device_data_by_id return NULL if the requested GPU
is wedged.

When the user is done, it's possible to trigger a GPU
reset.

The auto-recovery is still enabled by default, and to use this
feature one has to either:
* boot with amdgpu.gpu_recovery=0: not recommended as it disables
  recovery for all GPUs
* write 0 to /sys/kernel/debug/dri/X/amdgpu_gpu_recover to disable
  auto-recovery for one GPU only

Runtime power management is disabled when the device is wedged
(because we can't submit any work to the GPU).

Assisted-by: LLM
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 13 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     |  3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  8 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  8 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c    | 31 +++++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 12 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   |  5 ----
 drivers/gpu/drm/amd/amdkfd/kfd_process.c   |  6 ++++-
 10 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a486ac746a5b..cfbb5ab5a2fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -985,6 +985,13 @@ struct amdgpu_device {
 	/* Per device gpu_recovery setting. 0 means disabled. */
 	unsigned int gpu_recovery;
 
+	/* Set to 1 when the device has been declared wedged following a
+	 * hang: it no longer accepts new work and pending fences have been
+	 * force-signalled. Reset to 0 once the device is reset back to a
+	 * working state.
+	 */
+	atomic_t wedge_status;
+
 	/* KFD
 	 * Must be last --ends in a flexible-array member.
 	 */
@@ -1374,4 +1381,10 @@ void amdgpu_device_set_uid(struct amdgpu_uid *uid_info,
 			   uint64_t uid);
 uint64_t amdgpu_device_get_uid(struct amdgpu_uid *uid_info,
 			       enum amdgpu_uid_type type, uint8_t inst);
+
+static inline bool amdgpu_device_is_wedged(struct amdgpu_device *adev)
+{
+	return atomic_read(&adev->wedge_status);
+}
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index fa301c71b782..d119de477284 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1465,6 +1465,9 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	if (!adev->accel_working)
 		return -EBUSY;
 
+	if (amdgpu_device_is_wedged(adev))
+		return -ENODEV;
+
 	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, lock);
 
 	r = PM_RUNTIME_ACQUIRE_ERR(&lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a812551b353f..7335f9c4d86f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4149,6 +4149,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	}
 
 	amdgpu_device_init_gpu_recovery(adev);
+	atomic_set(&adev->wedge_status, 0);
 
 fence_driver_init:
 	/* Fence driver */
@@ -4401,6 +4402,9 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 		amdgpu_sync_free(&adev->isolation[i].prev);
 	}
 
+	if (atomic_xchg(&adev->wedge_status, 0))
+		pm_runtime_put_autosuspend(adev->dev);
+
 	amdgpu_reset_fini(adev);
 
 	/* free i2c buses */
@@ -5144,6 +5148,10 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
 	r = 0;
 	list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
 		amdgpu_set_init_level(tmp_adev, init_level);
+
+		if (atomic_xchg(&tmp_adev->wedge_status, 0))
+			pm_runtime_put_autosuspend(tmp_adev->dev);
+
 		if (full_reset) {
 			/* post card */
 			amdgpu_reset_set_dpc_status(tmp_adev, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb60a99f43c1..9b4c76f198f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -459,6 +459,9 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 	uint32_t handle, initial_domain;
 	int r;
 
+	if (amdgpu_device_is_wedged(adev))
+		return -ENODEV;
+
 	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, lock);
 
 	r = PM_RUNTIME_ACQUIRE_ERR(&lock);
@@ -967,6 +970,11 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	if (r)
 		goto error_put_gobj;
 
+	if (amdgpu_device_is_wedged(adev)) {
+		r = -ENODEV;
+		goto error_put_gobj;
+	}
+
 	/* Add input syncobj fences (if any) for synchronization. */
 	r = amdgpu_gem_add_input_fence(filp,
 				       args->input_fence_syncobj_handles,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 4204c3c750a8..53fa9fa83ae7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -730,6 +730,9 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	struct amdgpu_job *job;
 	int r;
 
+	if (amdgpu_device_is_wedged(adev))
+		return;
+
 	ring = to_amdgpu_ring(adev->mman.buffer_funcs_scheds[0]);
 
 	if (!hub->sdma_invalidation_workaround || vmid ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 16391cc2b9a5..61e8f479556e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -24,6 +24,7 @@
 #include <linux/kthread.h>
 #include <linux/wait.h>
 #include <linux/sched.h>
+#include <linux/pm_runtime.h>
 
 #include <drm/drm_drv.h>
 
@@ -184,9 +185,30 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
 		if (r)
 			dev_err(adev->dev, "GPU Recovery Failed: %d\n", r);
 	} else {
-		drm_sched_suspend_timeout(&ring->sched);
-		if (amdgpu_sriov_vf(adev))
-			adev->virt.tdr_debug = true;
+		struct drm_gpu_scheduler *sched;
+		struct amdgpu_fence *guilty_fence;
+
+		/* Declare the device as wedged if it's not already. */
+		if (!atomic_xchg(&adev->wedge_status, 1)) {
+			/* This ref will be dropped in amdgpu_device_reinit_after_reset. */
+			pm_runtime_get_noresume(adev->dev);
+
+			pci_clear_master(adev->pdev);
+
+			drm_dev_wedged_event(&adev->ddev, DRM_WEDGE_RECOVERY_REBIND, info);
+		}
+
+		guilty_fence = to_amdgpu_job(s_job)->hw_fence;
+
+		sched = &ring->sched;
+
+		/* Prevent anybody else from touching the ring buffer. */
+		drm_sched_wqueue_stop(sched);
+
+		amdgpu_fence_driver_force_completion(ring, &guilty_fence->base);
+
+		/* Start the scheduler again */
+		drm_sched_wqueue_start(sched);
 	}
 
 exit:
@@ -438,7 +460,8 @@ static struct dma_fence *amdgpu_job_run(struct drm_sched_job *sched_job)
 
 	/* Skip job if VRAM is lost and never resubmit gangs */
 	if (job->generation != amdgpu_vm_generation(adev, job->vm) ||
-	    (job->job_run_counter && job->gang_submit))
+	    (job->job_run_counter && job->gang_submit) ||
+	    amdgpu_device_is_wedged(adev))
 		dma_fence_set_error(finished, -ECANCELED);
 
 	if (finished->error < 0) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 14a72a406d37..2393cad36ac0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -781,6 +781,9 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		return copy_to_user(out, &count, min(size, 4u)) ? -EFAULT : 0;
 	}
 	case AMDGPU_INFO_TIMESTAMP: {
+		if (amdgpu_device_is_wedged(adev))
+			return -ENODEV;
+
 		PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, pm);
 
 		ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
@@ -913,6 +916,9 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		if (info->read_mmr_reg.count > 128)
 			return -EINVAL;
 
+		if (amdgpu_device_is_wedged(adev))
+			return -ENODEV;
+
 		PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, pm);
 
 		ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
@@ -953,6 +959,9 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		uint64_t vm_size;
 		uint32_t pcie_gen_mask, pcie_width_mask;
 
+		if (amdgpu_device_is_wedged(adev))
+			return -ENODEV;
+
 		/* The clock limits are only available while the SMU is up. */
 		PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, pm);
 
@@ -1187,6 +1196,9 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		}
 	}
 	case AMDGPU_INFO_SENSOR: {
+		if (amdgpu_device_is_wedged(adev))
+			return -ENODEV;
+
 		PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, pm);
 
 		ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 4b1b42fee6b4..6ffd9e45e3ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -1255,12 +1255,12 @@ void amdgpu_virt_init(struct amdgpu_device *adev)
 
 static bool amdgpu_virt_access_debugfs_is_mmio(struct amdgpu_device *adev)
 {
-	return amdgpu_sriov_is_debug(adev) ? true : false;
+	return amdgpu_device_is_wedged(adev) ? true : false;
 }
 
 static bool amdgpu_virt_access_debugfs_is_kiq(struct amdgpu_device *adev)
 {
-	return amdgpu_sriov_is_normal(adev) ? true : false;
+	return !amdgpu_device_is_wedged(adev) ? true : false;
 }
 
 int amdgpu_virt_enable_access_debugfs(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index a2c1aa838737..665d483de731 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -315,7 +315,6 @@ struct amdgpu_virt {
 	uint32_t gim_feature;
 	uint32_t reg_access_mode;
 	int req_init_data_ver;
-	bool tdr_debug;
 	struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
 	bool ras_init_done;
 	uint32_t reg_access;
@@ -446,10 +445,6 @@ static inline bool is_virtual_machine(void)
 	((adev)->virt.gim_feature & AMDGIM_FEATURE_PP_ONE_VF)
 #define amdgpu_sriov_multi_vf_mode(adev) \
 	(amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev))
-#define amdgpu_sriov_is_debug(adev) \
-	((!amdgpu_in_reset(adev)) && adev->virt.tdr_debug)
-#define amdgpu_sriov_is_normal(adev) \
-	((!amdgpu_in_reset(adev)) && (!adev->virt.tdr_debug))
 #define amdgpu_sriov_is_av1_support(adev) \
 	((adev)->virt.gim_feature & AMDGIM_FEATURE_AV1_SUPPORT)
 #define amdgpu_sriov_is_vcn_rb_decouple(adev) \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index ac18c2be6ecb..b95dddeaccc5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -2424,8 +2424,12 @@ struct kfd_process_device *kfd_process_device_data_by_id(struct kfd_process *p,
 		for (i = 0; i < p->n_pdds; i++) {
 			struct kfd_process_device *pdd = p->pdds[i];
 
-			if (pdd->user_gpu_id == gpu_id)
+			if (pdd->user_gpu_id == gpu_id) {
+				if (amdgpu_device_is_wedged(pdd->dev->adev))
+					return NULL;
+
 				return pdd;
+			}
 		}
 	}
 	return NULL;
-- 
2.43.0

Re: [PATCH v4 5/6] drm/amdgpu: add wedge event implementation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential
issues to consider:
- [High] drm/amdgpu: wedge_status and runtime PM ref not reset on Mode-2
  or SR-IOV reset
- [High] drm/amdgpu: pci_clear_master() in amdgpu_job_timedout() breaks
  reset recovery
- [High] drm/amdgpu: amdgpu_job_timedout() only force-completes the
  timed-out ring
- [High] drm/amdkfd: returning NULL in kfd_process_device_data_by_id()
  breaks cleanup
- [Medium] drm/amdgpu: dropped amdgpu_in_reset() guard in SR-IOV debugfs
  access helpers
--

commit 0aba431562381113abbf37e2dd1f6f92109a2746
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: add wedge event implementation

This commit implements DRM wedge event support for amdgpu when GPU
recovery is disabled, force-completing fences on job timeout and
rejecting new work until a reset is triggered.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f284e433b72bf..24547ea842d49 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
[ ... ]
> @@ -5152,6 +5156,10 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)
>  	r = 0;
>  	list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
>  		amdgpu_set_init_level(tmp_adev, init_level);
> +
> +		if (atomic_xchg(&tmp_adev->wedge_status, 0))
> +			pm_runtime_put_autosuspend(tmp_adev->dev);
> +
>  		if (full_reset) {
>  			/* post card */
>  			amdgpu_reset_set_dpc_status(tmp_adev, false);

[Severity: High]
Does clearing wedge_status and dropping the runtime PM reference only in
amdgpu_device_reinit_after_reset() leave the GPU permanently wedged and
leak the runtime PM reference on ASICs that use reset_cntl Mode-2 handlers
or SR-IOV reset?

In amdgpu_do_asic_reset(), amdgpu_reset_perform_reset() runs first and
returns early whenever a reset_cntl handler is registered (such as on
Aldebaran, Sienna Cichlid, SMU v13.0.10, and SOC v1.0), bypassing
amdgpu_device_reinit_after_reset() entirely:

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:amdgpu_do_asic_reset() {
    ...
	r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
	/* If reset handler not implemented, continue; otherwise return */
	if (r == -EOPNOTSUPP)
		r = 0;
	else
		return r;
    ...
}

SR-IOV VF resets also bypass amdgpu_do_asic_reset() via
amdgpu_device_reset_sriov().  On these paths, GART TLB flushes in
amdgpu_gmc_flush_gpu_tlb() are skipped during recovery, the runtime PM
reference from amdgpu_job_timedout() is leaked, and the device continues
rejecting CS, GEM, INFO, and KFD ioctls and canceling jobs after the reset
completes.

In addition, on paths that do reach amdgpu_device_reinit_after_reset(),
clearing wedge_status at the start of the loop exposes the device as
unwedged before ASIC init, firmware loading, IP resume, and IB ring tests
have succeeded.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index d01df35de11b2..1caae49f93369 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
[ ... ]
> @@ -184,9 +185,30 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct drm_sched_job *s_job)
>  		if (r)
>  			dev_err(adev->dev, "GPU Recovery Failed: %d\n", r);
>  	} else {
> -		drm_sched_suspend_timeout(&ring->sched);
> -		if (amdgpu_sriov_vf(adev))
> -			adev->virt.tdr_debug = true;
> +		struct drm_gpu_scheduler *sched;
> +		struct amdgpu_fence *guilty_fence;
> +
> +		/* Declare the device as wedged if it's not already. */
> +		if (!atomic_xchg(&adev->wedge_status, 1)) {
> +			/* This ref will be dropped in amdgpu_device_reinit_after_reset. */
> +			pm_runtime_get_noresume(adev->dev);
> +
> +			pci_clear_master(adev->pdev);

[Severity: High]
Does calling pci_clear_master() here break subsequent GPU reset recovery?

Clearing PCI_COMMAND_MASTER when wedging the device causes
amdgpu_device_mode1_reset() and nv_asic_mode2_reset() to cache
adev->pci_state with bus mastering disabled, so later calls to
amdgpu_device_load_pci_state() restore PCI_COMMAND without bus mastering:

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:amdgpu_device_mode1_reset() {
    ...
	/* Cache the state before bus master disable. The saved config space
	 * values are used in other cases like restore after mode-2 reset.
	 */
	amdgpu_device_cache_pci_state(adev->pdev);

	/* disable BM */
	pci_clear_master(adev->pdev);
    ...
}

It also leaves bus mastering disabled during pre-reset IP suspend, during
post-reset PSP firmware loading in amdgpu_device_fw_loading() (which runs
before amdgpu_device_ip_resume_phase2() re-enables bus mastering via IH
resume), and permanently across Mode-2 resets such as
smu_v13_0_10_mode2_restore_ip() that only resume GFX, MES, and SDMA without
calling pci_set_master().  While wedged, disabling bus mastering also
prevents KIQ fence writebacks in amdgpu_kiq_rreg() and amdgpu_kiq_wreg()
from completing during debugfs register inspection.

> +
> +			drm_dev_wedged_event(&adev->ddev, DRM_WEDGE_RECOVERY_REBIND, info);
> +		}
> +
> +		guilty_fence = to_amdgpu_job(s_job)->hw_fence;
> +
> +		sched = &ring->sched;
> +
> +		/* Prevent anybody else from touching the ring buffer. */
> +		drm_sched_wqueue_stop(sched);
> +
> +		amdgpu_fence_driver_force_completion(ring, &guilty_fence->base);
> +
> +		/* Start the scheduler again */
> +		drm_sched_wqueue_start(sched);
>  	}

[Severity: High]
What happens to in-flight fences on other rings when the device wedges?

pci_clear_master() disables DMA fence writebacks and interrupts across the
entire PCI device before drm_sched_wqueue_stop() stops even the timed-out
ring, and amdgpu_fence_driver_force_completion() is only called on that
single ring.  Pending fences on rings with infinite timeouts (such as
compute rings with MAX_SCHEDULE_TIMEOUT) or non-scheduler rings (KIQ and
MES) will never signal.  Other active scheduler rings will suffer secondary
timeouts that call amdgpu_job_core_dump() at the top of
amdgpu_job_timedout(), overwriting the initial hang's IP state dump and
marking their fences with -ETIME instead of -ECANCELED.

Also, because amdgpu_job_prepare_job() does not check
amdgpu_device_is_wedged(), restarting the scheduler queue allows queued
jobs on the wedged ring to block in amdgpu_job_prepare_job() on unsignaled
gang, isolation, or VMID fences from other rings instead of reaching
amdgpu_job_run() to be canceled.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 21a935d9d32dd..b6af5284f4466 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -1255,12 +1255,12 @@ void amdgpu_virt_init(struct amdgpu_device *adev)
>  
>  static bool amdgpu_virt_access_debugfs_is_mmio(struct amdgpu_device *adev)
>  {
> -	return amdgpu_sriov_is_debug(adev) ? true : false;
> +	return amdgpu_device_is_wedged(adev) ? true : false;
>  }
>  
>  static bool amdgpu_virt_access_debugfs_is_kiq(struct amdgpu_device *adev)
>  {
> -	return amdgpu_sriov_is_normal(adev) ? true : false;
> +	return !amdgpu_device_is_wedged(adev) ? true : false;
>  }
>  
>  int amdgpu_virt_enable_access_debugfs(struct amdgpu_device *adev)

[Severity: Medium]
Should amdgpu_virt_access_debugfs_is_mmio() and
amdgpu_virt_access_debugfs_is_kiq() still check !amdgpu_in_reset(adev)?

Previously, amdgpu_sriov_is_debug() and amdgpu_sriov_is_normal() both
required !amdgpu_in_reset(adev), so amdgpu_virt_enable_access_debugfs()
returned -EPERM while an SR-IOV VF reset was in progress.  Without that
check, the two helpers are boolean complements, making the -EPERM branch in
amdgpu_virt_enable_access_debugfs() dead code and allowing debugfs register
reads during a VF reset to set AMDGPU_SRIOV_CAPS_RUNTIME via
amdgpu_virt_disable_access_debugfs() while the VF is in exclusive reset
mode.

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 6663718fb83ce..b83f6994394c1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -2449,8 +2449,12 @@ struct kfd_process_device *kfd_process_device_data_by_id(struct kfd_process *p,
>  		for (i = 0; i < p->n_pdds; i++) {
>  			struct kfd_process_device *pdd = p->pdds[i];
>  
> -			if (pdd->user_gpu_id == gpu_id)
> +			if (pdd->user_gpu_id == gpu_id) {
> +				if (amdgpu_device_is_wedged(pdd->dev->adev))
> +					return NULL;
> +
>  				return pdd;
> +			}
>  		}
>  	}
>  	return NULL;

[Severity: High]
Does returning NULL from kfd_process_device_data_by_id() when the GPU is
wedged break KFD resource cleanup, SVM eviction, and WARN invariants?

Several KFD paths use kfd_process_device_data_by_id() to tear down state or
complete multi-step operations on an existing process device:

1. In svm_range_set_attr(), svm_range_check_attr() validates the GPU ID via
kfd_process_gpuidx_from_gpuid() (which does not check
amdgpu_device_is_wedged()), and svm_range_best_prefetch_location() then
looks up the node via svm_range_get_node_by_id() ->
kfd_process_device_data_by_id(), hitting WARN_ONCE(1, "failed to get valid
kfd node at id%x\n", best_loc).

2. In kfd_ioctl_unmap_memory_from_gpu(), if the GPU wedges before the
post-sync loop, kfd_process_device_data_by_id() returns NULL, triggering
WARN_ON_ONCE(!peer_pdd) and skipping amdgpu_amdkfd_gpuvm_dmaunmap_mem().
Similarly, kfd_ioctl_free_memory_of_gpu() and
kfd_ioctl_unmap_memory_from_gpu() fail with -EINVAL, preventing userspace
from freeing BOs on the wedged GPU or unmapping and freeing BOs on healthy
peer GPUs that were mapped to the wedged GPU.

3. In svm_range_evict_svm_bo(), prange is unlinked from svm_bo->range_list
before calling svm_migrate_vram_to_ram(), which fails with -ENODEV when
kfd_process_device_data_by_id() returns NULL, leaving prange detached with
its svm_bo and VRAM node unreleased.

4. In profile_lock_device(), kfd_profiler_pmc(), and
kfd_profiler_ptl_control(), returning NULL prevents unlocking
kfd->profiler_process or releasing ptl->disable_ref on a wedged GPU.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109171333.4329-1-pierre-eric.pelloux-prayer@amd.com?part=5

[PATCH v4 6/6] drm/amdgpu: introduce amdgpu_dev_enter / exit

[Pierre-Eric Pelloux-Prayer]

And use them instead of drm_dev_enter / exit.

Every place that needs the GPU should already call the drm variant.
The amdgpu version also checks for the wedge status of the device.

amdgpu_device_skip_hw_access is not used as it's not meant to be
used outside interrupt handlers and reset handler.

Direct uses of amdgpu_device_is_wedged are still ok for paths where
drm_dev_enter is used like drm ioctls.

As Sashiko bot pointed out in the v3 review, some places should still
use the drm_dev_enter variant:
* amdgpu_job_timedout: this function has to force-finish fences for
  the other rings.
* amdgpu_device_mm_access: accessing memory should be possible for
  wedged devices to allow debugging

Link: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=5
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 14 ++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  6 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c   | 16 ++++++++--------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 18 +++++++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c    | 10 +++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c    | 12 ++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c    | 12 ++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 10 +++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c     |  6 +++---
 drivers/gpu/drm/amd/amdgpu/psp_v13_0.c     |  6 +++---
 drivers/gpu/drm/amd/amdgpu/psp_v14_0.c     |  6 +++---
 drivers/gpu/drm/amd/amdgpu/psp_v15_0_3.c   |  6 +++---
 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c      |  8 ++++----
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c      |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c      |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c      |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c      |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_2.c    |  4 ++--
 29 files changed, 112 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index cfbb5ab5a2fe..75989767f9b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,6 +50,7 @@
 #include <drm/ttm/ttm_placement.h>
 
 #include <drm/amdgpu_drm.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_ioctl.h>
 
@@ -1387,4 +1388,17 @@ static inline bool amdgpu_device_is_wedged(struct amdgpu_device *adev)
 	return atomic_read(&adev->wedge_status);
 }
 
+static inline bool amdgpu_dev_enter(struct amdgpu_device *adev, int *idx)
+{
+	if (amdgpu_device_is_wedged(adev))
+		return false;
+
+	return drm_dev_enter(adev_to_drm(adev), idx);
+}
+
+static inline void amdgpu_dev_exit(struct amdgpu_device *adev, int idx)
+{
+	drm_dev_exit(idx);
+}
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 2b318ccc9aa6..ab3b62379e38 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -428,9 +428,9 @@ void amdgpu_ctx_fini(struct kref *ref)
 
 	mutex_lock(&adev->pm.stable_pstate_ctx_lock);
 	if (adev->pm.stable_pstate_ctx == ctx) {
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			__amdgpu_ctx_set_stable_pstate(ctx, ctx->stable_pstate);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		}
 		adev->pm.stable_pstate_ctx = NULL;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7335f9c4d86f..a66e504f7023 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3025,12 +3025,18 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
 {
 	struct amdgpu_device *adev =
 		container_of(work, struct amdgpu_device, gfx.gfx_off_delay_work.work);
+	int idx;
 
 	WARN_ON_ONCE(adev->gfx.gfx_off_state);
 	WARN_ON_ONCE(adev->gfx.gfx_off_req_count);
 
+	if (!amdgpu_dev_enter(adev, &idx))
+		return;
+
 	if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true, 0))
 		adev->gfx.gfx_off_state = true;
+
+	amdgpu_dev_exit(adev, idx);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3e5634de077f..4c3aa4e4d4c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -3030,13 +3030,13 @@ static int amdgpu_drm_release(struct inode *inode, struct file *filp)
 	struct drm_device *dev = file_priv->minor->dev;
 	int idx;
 
-	if (fpriv && drm_dev_enter(dev, &idx)) {
+	if (fpriv && amdgpu_dev_enter(drm_to_adev(dev), &idx)) {
 		amdgpu_evf_mgr_shutdown(&fpriv->evf_mgr);
 		amdgpu_userq_mgr_cancel_resume(&fpriv->userq_mgr);
 		amdgpu_evf_mgr_flush_suspend(&fpriv->evf_mgr);
 		amdgpu_userq_mgr_fini(&fpriv->userq_mgr);
 		amdgpu_evf_mgr_fini(&fpriv->evf_mgr);
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(drm_to_adev(dev), idx);
 	}
 
 	return drm_release(inode, filp);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index c4c21dbbbdbf..62e98e50bf35 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -318,7 +318,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
 	if (!adev->gart.ptr)
 		return;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return;
 
 	t = offset / AMDGPU_GPU_PAGE_SIZE;
@@ -335,7 +335,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
 	}
 	amdgpu_gart_invalidate_tlb(adev);
 
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 }
 
 /**
@@ -359,7 +359,7 @@ void amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
 	unsigned i, j, t;
 	int idx;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return;
 
 	t = offset / AMDGPU_GPU_PAGE_SIZE;
@@ -371,7 +371,7 @@ void amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
 			page_base += AMDGPU_GPU_PAGE_SIZE;
 		}
 	}
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 }
 
 /**
@@ -400,7 +400,7 @@ void amdgpu_gart_map_vram_range(struct amdgpu_device *adev, uint64_t pa,
 	/* The SYSTEM flag indicates the pages aren't in VRAM. */
 	WARN_ON_ONCE(flags & AMDGPU_PTE_SYSTEM);
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return;
 
 	page_base = pa;
@@ -411,7 +411,7 @@ void amdgpu_gart_map_vram_range(struct amdgpu_device *adev, uint64_t pa,
 		}
 	}
 
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 }
 
 /**
@@ -440,7 +440,7 @@ void amdgpu_gart_map_gfx9_mqd(struct amdgpu_device *adev, uint64_t offset,
 	if (!adev->gart.ptr)
 		return;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return;
 
 	t = offset / AMDGPU_GPU_PAGE_SIZE;
@@ -455,7 +455,7 @@ void amdgpu_gart_map_gfx9_mqd(struct amdgpu_device *adev, uint64_t offset,
 			page_base += AMDGPU_GPU_PAGE_SIZE;
 		}
 	}
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9b4c76f198f2..700e4da9cab8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -128,12 +128,12 @@ static vm_fault_t amdgpu_gem_fault(struct vm_fault *vmf)
 	if (ret)
 		return ret;
 
-	if (drm_dev_enter(ddev, &idx)) {
+	if (amdgpu_dev_enter(drm_to_adev(ddev), &idx)) {
 		pm_runtime_get_noresume(ddev->dev);
 
 		ret = amdgpu_bo_fault_reserve_notify(bo);
 		if (ret) {
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(drm_to_adev(ddev), idx);
 			goto unlock;
 		}
 
@@ -141,7 +141,7 @@ static vm_fault_t amdgpu_gem_fault(struct vm_fault *vmf)
 					       TTM_BO_VM_NUM_PREFAULT);
 
 		pm_runtime_put_noidle(ddev->dev);
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(drm_to_adev(ddev), idx);
 	} else {
 		ret = ttm_bo_vm_dummy_page(vmf, vmf->vma->vm_page_prot);
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 53fa9fa83ae7..94a77bf14c76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -728,9 +728,9 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	struct amdgpu_vmhub *hub = &adev->vmhub[vmhub];
 	struct dma_fence *fence;
 	struct amdgpu_job *job;
-	int r;
+	int r, idx;
 
-	if (amdgpu_device_is_wedged(adev))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return;
 
 	ring = to_amdgpu_ring(adev->mman.buffer_funcs_scheds[0]);
@@ -742,8 +742,10 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 		 * A GPU reset should flush all TLBs anyway, so no need to do
 		 * this while one is ongoing.
 		 */
-		if (!down_read_trylock(&adev->reset_domain->sem))
+		if (!down_read_trylock(&adev->reset_domain->sem)) {
+			amdgpu_dev_exit(adev, idx);
 			return;
+		}
 
 		if (adev->gmc.flush_tlb_needs_extra_type_2)
 			adev->gmc.gmc_funcs->flush_gpu_tlb(adev, vmid,
@@ -756,6 +758,7 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 		adev->gmc.gmc_funcs->flush_gpu_tlb(adev, vmid, vmhub,
 						   flush_type);
 		up_read(&adev->reset_domain->sem);
+		amdgpu_dev_exit(adev, idx);
 		return;
 	}
 
@@ -782,11 +785,13 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 
 	dma_fence_wait(fence, false);
 	dma_fence_put(fence);
+	amdgpu_dev_exit(adev, idx);
 
 	return;
 
 error_alloc:
 	mutex_unlock(&adev->mman.default_entity.lock);
+	amdgpu_dev_exit(adev, idx);
 	dev_err(adev->dev, "Error flushing GPU TLB using the SDMA (%d)!\n", r);
 }
 
@@ -947,6 +952,9 @@ int amdgpu_gmc_flush_gpu_tlb_pasid_xccs(struct amdgpu_device *adev, u16 pasid,
 	unsigned long pending = 0;
 	int xcc, r = 0, err;
 
+	if (amdgpu_device_is_wedged(adev))
+		return -ENODEV;
+
 	/*
 	 * A GPU reset should flush all TLBs anyway, so no need to do
 	 * this while one is ongoing.
@@ -1283,7 +1291,7 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	u64 gart_ptb_gpu_pa = amdgpu_gmc_vram_pa(adev, adev->gart.bo);
 	int idx;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return;
 
 	flags |= AMDGPU_PTE_VALID | AMDGPU_PTE_READABLE;
@@ -1315,7 +1323,7 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PDE_BFS_FLAG(adev, 0);
 	/* Requires gart_ptb_gpu_pa to be 4K aligned */
 	amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, gart_ptb_gpu_pa, flags);
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6aa895f63aea..e0c1a876d76a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1144,12 +1144,12 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
 
 	amdgpu_ttm_fini(adev);
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		if (!adev->gmc.xgmi.connected_to_cpu && !adev->gmc.is_app_apu) {
 			arch_phys_wc_del(adev->gmc.vram_mtrr);
 			arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);
 		}
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 01dca6858833..a0fbb9af818f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -5109,7 +5109,7 @@ static ssize_t psp_usbc_pd_fw_sysfs_write(struct device *dev,
 		return -EBUSY;
 	}
 
-	if (!drm_dev_enter(ddev, &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return -ENODEV;
 
 	ret = amdgpu_ucode_request(adev, &usbc_pd_fw, AMDGPU_UCODE_REQUIRED,
@@ -5142,7 +5142,7 @@ static ssize_t psp_usbc_pd_fw_sysfs_write(struct device *dev,
 		count = ret;
 	}
 
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 	return count;
 }
 
@@ -5150,19 +5150,19 @@ int psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t bin_size)
 {
 	int idx;
 
-	if (!drm_dev_enter(adev_to_drm(psp->adev), &idx))
+	if (!amdgpu_dev_enter(psp->adev, &idx))
 		return -ENODEV;
 
 	if (!bin_size || bin_size > PSP_1_MEG) {
 		dev_err(psp->adev->dev, "PSP firmware is invalid\n");
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(psp->adev, idx);
 		return -EINVAL;
 	}
 
 	memset(psp->fw_pri_buf, 0, PSP_1_MEG);
 	memcpy(psp->fw_pri_buf, start_addr, bin_size);
 
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(psp->adev, idx);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9c3efc27b363..5209143e7c24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1624,7 +1624,7 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo,
 	if (!adev->mman.sdma_access_ptr)
 		return -EACCES;
 
-	if (!adev->mman.buffer_funcs_enabled || !drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!adev->mman.buffer_funcs_enabled || !amdgpu_dev_enter(adev, &idx))
 		return -ENODEV;
 
 	if (write)
@@ -1660,7 +1660,7 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo,
 	if (!(r || write))
 		memcpy(buf, adev->mman.sdma_access_ptr, len);
 out:
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index cd35ff7ca864..8668684e8e42 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -456,14 +456,14 @@ int amdgpu_uvd_prepare_suspend(struct amdgpu_device *adev)
 		if (!adev->uvd.inst[j].saved_bo)
 			return -ENOMEM;
 
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			/* re-write 0 since err_event_athub will corrupt VCPU buffer */
 			if (amdgpu_ras_intr_triggered())
 				memset(adev->uvd.inst[j].saved_bo, 0, size);
 			else
 				memcpy_fromio(adev->uvd.inst[j].saved_bo, ptr, size);
 
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		}
 	}
 
@@ -495,9 +495,9 @@ int amdgpu_uvd_resume(struct amdgpu_device *adev)
 		ptr = adev->uvd.inst[i].cpu_addr;
 
 		if (adev->uvd.inst[i].saved_bo != NULL) {
-			if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+			if (amdgpu_dev_enter(adev, &idx)) {
 				memcpy_toio(ptr, adev->uvd.inst[i].saved_bo, size);
-				drm_dev_exit(idx);
+				amdgpu_dev_exit(adev, idx);
 			}
 			kvfree(adev->uvd.inst[i].saved_bo);
 			adev->uvd.inst[i].saved_bo = NULL;
@@ -508,10 +508,10 @@ int amdgpu_uvd_resume(struct amdgpu_device *adev)
 			hdr = (const struct common_firmware_header *)adev->uvd.fw->data;
 			if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
 				offset = le32_to_cpu(hdr->ucode_array_offset_bytes);
-				if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+				if (amdgpu_dev_enter(adev, &idx)) {
 					memcpy_toio(adev->uvd.inst[i].cpu_addr, adev->uvd.fw->data + offset,
 						    le32_to_cpu(hdr->ucode_size_bytes));
-					drm_dev_exit(idx);
+					amdgpu_dev_exit(adev, idx);
 				}
 				size -= le32_to_cpu(hdr->ucode_size_bytes);
 				ptr += le32_to_cpu(hdr->ucode_size_bytes);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 3952af9ae115..b583c4d41a1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -337,11 +337,11 @@ int amdgpu_vce_resume(struct amdgpu_device *adev)
 	hdr = (const struct common_firmware_header *)adev->vce.fw->data;
 	offset = le32_to_cpu(hdr->ucode_array_offset_bytes);
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		memset_io(adev->vce.cpu_addr, 0, amdgpu_bo_size(adev->vce.vcpu_bo));
 		memcpy_toio(adev->vce.cpu_addr, adev->vce.fw->data + offset,
 			    adev->vce.fw->size - offset);
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 34e1242d66ea..c286df7c6faf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -332,9 +332,9 @@ static int amdgpu_vcn_save_vcpu_bo_inst(struct amdgpu_device *adev, int i)
 	if (!adev->vcn.inst[i].saved_bo)
 		return -ENOMEM;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		memcpy_fromio(adev->vcn.inst[i].saved_bo, ptr, size);
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	return 0;
@@ -383,9 +383,9 @@ int amdgpu_vcn_resume(struct amdgpu_device *adev, int i)
 	ptr = adev->vcn.inst[i].cpu_addr;
 
 	if (adev->vcn.inst[i].saved_bo != NULL) {
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			memcpy_toio(ptr, adev->vcn.inst[i].saved_bo, size);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		}
 		kvfree(adev->vcn.inst[i].saved_bo);
 		adev->vcn.inst[i].saved_bo = NULL;
@@ -396,11 +396,11 @@ int amdgpu_vcn_resume(struct amdgpu_device *adev, int i)
 		hdr = (const struct common_firmware_header *)adev->vcn.inst[i].fw->data;
 		if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
 			offset = le32_to_cpu(hdr->ucode_array_offset_bytes);
-			if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+			if (amdgpu_dev_enter(adev, &idx)) {
 				memcpy_toio(adev->vcn.inst[i].cpu_addr,
 					    adev->vcn.inst[i].fw->data + offset,
 					    le32_to_cpu(hdr->ucode_size_bytes));
-				drm_dev_exit(idx);
+				amdgpu_dev_exit(adev, idx);
 			}
 			size -= le32_to_cpu(hdr->ucode_size_bytes);
 			ptr += le32_to_cpu(hdr->ucode_size_bytes);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ea8688061237..01f00d55ad6f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1035,7 +1035,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
 	if (list_empty(&vm->kernel.needs_update))
 		return 0;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return -ENODEV;
 
 	memset(&params, 0, sizeof(params));
@@ -1069,7 +1069,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
 		amdgpu_vm_bo_idle(entry);
 
 error:
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 	return r;
 }
 
@@ -1170,12 +1170,12 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	struct amdgpu_res_cursor cursor;
 	int r, idx;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return -ENODEV;
 
 	tlb_cb = kmalloc_obj(*tlb_cb);
 	if (!tlb_cb) {
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 		return -ENOMEM;
 	}
 
@@ -1287,7 +1287,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 error_free:
 	kfree(tlb_cb);
 	amdgpu_vm_end_critical(&params);
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index c73eb416c0e2..9d1fbddcdd7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -387,7 +387,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	if (r)
 		return r;
 
-	if (!drm_dev_enter(adev_to_drm(adev), &idx))
+	if (!amdgpu_dev_enter(adev, &idx))
 		return -ENODEV;
 
 	r = vm->update_funcs->map_table(vmbo);
@@ -429,7 +429,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 
 	r = vm->update_funcs->commit(&params, &vm->last_update);
 exit:
-	drm_dev_exit(idx);
+	amdgpu_dev_exit(adev, idx);
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index bd3bb3bfb668..4d51e9c534f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -547,20 +547,20 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
 			return -ENOMEM;
 		}
 
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
 			ret = psp_v11_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
 			if (ret) {
 				DRM_ERROR("Send long training msg failed.\n");
 				vfree(buf);
-				drm_dev_exit(idx);
+				amdgpu_dev_exit(adev, idx);
 				return ret;
 			}
 
 			memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
 			amdgpu_device_flush_hdp(adev, NULL);
 			vfree(buf);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		} else {
 			vfree(buf);
 			return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
index f9a3fbfc0634..43bb89d4fa33 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
@@ -635,20 +635,20 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
 			return -ENOMEM;
 		}
 
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
 			ret = psp_v13_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
 			if (ret) {
 				DRM_ERROR("Send long training msg failed.\n");
 				vfree(buf);
-				drm_dev_exit(idx);
+				amdgpu_dev_exit(adev, idx);
 				return ret;
 			}
 
 			memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
 			amdgpu_device_flush_hdp(adev, NULL);
 			vfree(buf);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		} else {
 			vfree(buf);
 			return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v14_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v14_0.c
index 888c747d6c5c..1c8476fb387d 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v14_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v14_0.c
@@ -494,20 +494,20 @@ static int psp_v14_0_memory_training(struct psp_context *psp, uint32_t ops)
 			return -ENOMEM;
 		}
 
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
 			ret = psp_v14_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
 			if (ret) {
 				DRM_ERROR("Send long training msg failed.\n");
 				vfree(buf);
-				drm_dev_exit(idx);
+				amdgpu_dev_exit(adev, idx);
 				return ret;
 			}
 
 			memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
 			amdgpu_device_flush_hdp(adev, NULL);
 			vfree(buf);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		} else {
 			vfree(buf);
 			return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v15_0_3.c b/drivers/gpu/drm/amd/amdgpu/psp_v15_0_3.c
index 029d4f4b0961..739fba7a4751 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v15_0_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v15_0_3.c
@@ -483,20 +483,20 @@ static int psp_v15_0_3_memory_training(struct psp_context *psp, uint32_t ops)
 			return -ENOMEM;
 		}
 
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
 			ret = psp_v15_0_3_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
 			if (ret) {
 				DRM_ERROR("Send long training msg failed.\n");
 				vfree(buf);
-				drm_dev_exit(idx);
+				amdgpu_dev_exit(adev, idx);
 				return ret;
 			}
 
 			memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
 			amdgpu_device_flush_hdp(adev, NULL);
 			vfree(buf);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		} else {
 			vfree(buf);
 			return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
index ee445d8abe47..e67f53f36f51 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -566,14 +566,14 @@ static int vce_v4_0_suspend(struct amdgpu_ip_block *ip_block)
 	if (adev->vce.vcpu_bo == NULL)
 		return 0;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
 			unsigned size = amdgpu_bo_size(adev->vce.vcpu_bo);
 			void *ptr = adev->vce.cpu_addr;
 
 			memcpy_fromio(adev->vce.saved_bo, ptr, size);
 		}
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	/*
@@ -616,12 +616,12 @@ static int vce_v4_0_resume(struct amdgpu_ip_block *ip_block)
 
 	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
 
-		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+		if (amdgpu_dev_enter(adev, &idx)) {
 			unsigned size = amdgpu_bo_size(adev->vce.vcpu_bo);
 			void *ptr = adev->vce.cpu_addr;
 
 			memcpy_toio(ptr, adev->vce.saved_bo, size);
-			drm_dev_exit(idx);
+			amdgpu_dev_exit(adev, idx);
 		}
 	} else {
 		r = amdgpu_vce_resume(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index 7d2b9e4b913a..2e4b8b025b12 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -255,9 +255,9 @@ static int vcn_v2_0_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	struct amdgpu_fw_shared *fw_shared = adev->vcn.inst->fw_shared.cpu_addr;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		fw_shared->present_flag_0 = 0;
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	amdgpu_virt_free_mm_table(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 10ef5616b2f5..c10c3a672d5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -421,14 +421,14 @@ static int vcn_v2_5_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	struct amdgpu_fw_shared *fw_shared;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			if (adev->vcn.harvest_config & (1 << i))
 				continue;
 			fw_shared = adev->vcn.inst[i].fw_shared.cpu_addr;
 			fw_shared->present_flag_0 = 0;
 		}
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index 0f4f4d66e2e4..6189c096e32f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -326,7 +326,7 @@ static int vcn_v3_0_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_fw_shared *fw_shared;
 
@@ -337,7 +337,7 @@ static int vcn_v3_0_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->sw_ring.is_enabled = false;
 		}
 
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index 4b7f7bbe18da..3f0185eac593 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -277,7 +277,7 @@ static int vcn_v4_0_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_vcn4_fw_shared *fw_shared;
 
@@ -289,7 +289,7 @@ static int vcn_v4_0_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->sq.is_enabled = 0;
 		}
 
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
index 3f39aa83c319..ebc4729669c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
@@ -296,7 +296,7 @@ static int vcn_v4_0_3_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(&adev->ddev, &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_vcn4_fw_shared *fw_shared;
 
@@ -304,7 +304,7 @@ static int vcn_v4_0_3_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->present_flag_0 = 0;
 			fw_shared->sq.is_enabled = cpu_to_le32(false);
 		}
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
index f30f80c1b07f..3797038d9f72 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
@@ -248,7 +248,7 @@ static int vcn_v4_0_5_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_vcn4_fw_shared *fw_shared;
 
@@ -260,7 +260,7 @@ static int vcn_v4_0_5_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->sq.is_enabled = 0;
 		}
 
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
index 7b7c3bc9f046..5d6c3759fa72 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
@@ -214,7 +214,7 @@ static int vcn_v5_0_0_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_vcn5_fw_shared *fw_shared;
 
@@ -226,7 +226,7 @@ static int vcn_v5_0_0_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->sq.is_enabled = 0;
 		}
 
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c
index c9e990ab297d..270fb2ab42dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c
@@ -259,7 +259,7 @@ static int vcn_v5_0_1_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_vcn5_fw_shared *fw_shared;
 
@@ -268,7 +268,7 @@ static int vcn_v5_0_1_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->sq.is_enabled = 0;
 		}
 
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_2.c b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_2.c
index ed47ddd5b08c..3565772288b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_2.c
@@ -179,7 +179,7 @@ static int vcn_v5_0_2_sw_fini(struct amdgpu_ip_block *ip_block)
 	struct amdgpu_device *adev = ip_block->adev;
 	int i, r, idx;
 
-	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
+	if (amdgpu_dev_enter(adev, &idx)) {
 		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
 			struct amdgpu_vcn5_fw_shared *fw_shared;
 
@@ -188,7 +188,7 @@ static int vcn_v5_0_2_sw_fini(struct amdgpu_ip_block *ip_block)
 			fw_shared->sq.is_enabled = 0;
 		}
 
-		drm_dev_exit(idx);
+		amdgpu_dev_exit(adev, idx);
 	}
 
 	if (amdgpu_sriov_vf(adev))
-- 
2.43.0

Re: [PATCH v4 6/6] drm/amdgpu: introduce amdgpu_dev_enter / exit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 9 potential
issues to consider:
- [High] drm/amdgpu: UAF and leaks in amdgpu_drm_release() when device
  is wedged
- [High] drm/amdgpu: stale PTEs and broken open when wedged in GART/VM
  helpers
- [High] drm/amdgpu: uninitialized saved_bo copied to VRAM on reset of
  wedged GPU
- [High] drm/amdgpu: SR-IOV and Mode-2 resets fail to clear wedge_status
- [High] drm/amdgpu: NULL pointer dereference in amdgpu_vm_init() when
  wedged
- [High] drm/amdgpu: dangling entry->bo in amdgpu_vm_pt_alloc() when
  wedged
- [Medium] drm/amdgpu: WC memtype and MTRR leak in amdgpu_bo_fini() when
  wedged
- [Medium] drm/amdgpu: unplug deadlock in amdgpu_gmc_flush_gpu_tlb() on
  fence wait
- [Medium] drm/amdgpu: amdgpu_vram_mgr_dmem_reclaim() misses
  amdgpu_dev_enter()
--

commit 0d8a6bc2888d88350480e40fc5320cb5adf242ad
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: introduce amdgpu_dev_enter / exit

This commit introduces amdgpu_dev_enter() and amdgpu_dev_exit() wrappers
that check whether the device is wedged before calling drm_dev_enter(), and
converts existing drm_dev_enter() and drm_dev_exit() call sites across
amdgpu.

Link: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=5

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 45680878aa5a1..4ba1772d9eb62 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
[ ... ]
> @@ -1384,4 +1385,17 @@ static inline bool amdgpu_device_is_wedged(struct amdgpu_device *adev)
>  	return atomic_read(&adev->wedge_status);
>  }
>  
> +static inline bool amdgpu_dev_enter(struct amdgpu_device *adev, int *idx)
> +{
> +	if (amdgpu_device_is_wedged(adev))
> +		return false;
> +
> +	return drm_dev_enter(adev_to_drm(adev), idx);
> +}
> +
> +static inline void amdgpu_dev_exit(struct amdgpu_device *adev, int idx)
> +{
> +	drm_dev_exit(idx);
> +}
> +
>  #endif

[Severity: Medium]
Should amdgpu_vram_mgr_dmem_reclaim() also use amdgpu_dev_enter() and
amdgpu_dev_exit()?

It still calls drm_dev_enter().  When dmem cgroup reclaim runs on a wedged
GPU, ttm_resource_manager_dmem_reclaim() -> amdgpu_bo_move() ->
amdgpu_move_blit() submits an SDMA copy job whose fence amdgpu_job_run()
immediately cancels and signals without copying, causing
ttm_bo_move_accel_cleanup() to free the source VRAM resource and lose the
BO contents.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 3e5634de077f1..4c3aa4e4d4c95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -3030,13 +3030,13 @@ static int amdgpu_drm_release(struct inode *inode, struct file *filp)
>  	struct drm_device *dev = file_priv->minor->dev;
>  	int idx;
>  
> -	if (fpriv && drm_dev_enter(dev, &idx)) {
> +	if (fpriv && amdgpu_dev_enter(drm_to_adev(dev), &idx)) {
>  		amdgpu_evf_mgr_shutdown(&fpriv->evf_mgr);
>  		amdgpu_userq_mgr_cancel_resume(&fpriv->userq_mgr);
>  		amdgpu_evf_mgr_flush_suspend(&fpriv->evf_mgr);
>  		amdgpu_userq_mgr_fini(&fpriv->userq_mgr);
>  		amdgpu_evf_mgr_fini(&fpriv->evf_mgr);
> -		drm_dev_exit(idx);
> +		amdgpu_dev_exit(drm_to_adev(dev), idx);
>  	}
>  
>  	return drm_release(inode, filp);

[Severity: High]
Does skipping userq_mgr and evf_mgr teardown when the device is wedged
cause a use-after-free on fpriv?

When a process closes its DRM fd while the GPU is wedged,
amdgpu_dev_enter() returns false and skips tearing down fpriv->userq_mgr
and fpriv->evf_mgr, after which drm_release() ->
amdgpu_driver_postclose_kms() unconditionally frees fpriv.  Any usermode
queues remain in adev->userq_doorbell_xa with queue->userq_mgr pointing
into the freed fpriv, which is dereferenced during GPU reset or suspend:

drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c:amdgpu_userq_mgr_cancel_reset_work() {
	...
	xa_for_each(xa, queue_id, queue) {
		cancel_delayed_work(&queue->hang_detect_work);
		cancel_work(&queue->userq_mgr->reset_work);
	}
}

Skipping teardown also leaves fpriv->evf_mgr.suspend_work,
fpriv->userq_mgr.resume_work and fpriv->userq_mgr.reset_work uncanceled,
and leaves unsignaled eviction fences on BOs with ev_fence->evf_mgr
pointing into the freed fpriv when amdgpu_eviction_fence_enable_signaling()
schedules suspend_work.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index c4c21dbbbdbf8..62e98e50bf35a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -318,7 +318,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>  	if (!adev->gart.ptr)
>  		return;
>  
> -	if (!drm_dev_enter(adev_to_drm(adev), &idx))
> +	if (!amdgpu_dev_enter(adev, &idx))
>  		return;
>  
>  	t = offset / AMDGPU_GPU_PAGE_SIZE;
> @@ -335,7 +335,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>  	}
>  	amdgpu_gart_invalidate_tlb(adev);
>  
> -	drm_dev_exit(idx);
> +	amdgpu_dev_exit(adev, idx);
>  }

[Severity: High]
Does returning early from amdgpu_gart_unbind() when wedged leave stale GART
PTEs pointing to freed system pages across a subsequent GPU reset?

When a GTT BO is unbound while wedged, amdgpu_ttm_backend_unbind() marks
gtt->bound = false, TTM frees the backing system pages, and
amdgpu_gtt_mgr_del() frees the drm_mm_node without clearing the PTEs in
adev->gart.ptr to dummy_page_addr.  If the GPU later recovers via reset,
amdgpu_gtt_mgr_recover() only rewrites PTEs for still-allocated nodes in
mgr->mm, leaving the unallocated slots mapped to the freed pages when
adev->gart.ptr is in system RAM or preserved VRAM.

Similarly, returning -ENODEV when wedged from amdgpu_vm_update_range(),
amdgpu_vm_update_pdes() and amdgpu_vm_pt_clear() causes
amdgpu_vm_clear_freed() to drop mappings from vm->freed without clearing
their PTEs, logs dev_err in amdgpu_gem_object_close() on every closed BO,
and causes amdgpu_driver_open_kms() to fail with -ENODEV when opening the
DRM device node.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index b9afcbab02f9d..e435cb0c9d7f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -728,9 +728,9 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>  	struct amdgpu_vmhub *hub = &adev->vmhub[vmhub];
>  	struct dma_fence *fence;
>  	struct amdgpu_job *job;
> -	int r;
> +	int r, idx;
>  
> -	if (amdgpu_device_is_wedged(adev))
> +	if (!amdgpu_dev_enter(adev, &idx))
>  		return;
[ ... ]
> @@ -782,11 +785,13 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>  
>  	dma_fence_wait(fence, false);
>  	dma_fence_put(fence);
> +	amdgpu_dev_exit(adev, idx);
>  
>  	return;

[Severity: Medium]
Can holding amdgpu_dev_enter() across dma_fence_wait() deadlock with
drm_dev_unplug() on hot-unplug?

CPU0                              CPU1
amdgpu_gmc_flush_gpu_tlb()
  amdgpu_dev_enter()
  fence = amdgpu_job_submit(job)
                                  amdgpu_pci_remove()
                                    drm_dev_unplug()
                                      dev->unplugged = true
                                      synchronize_srcu(&drm_unplug_srcu)
  dma_fence_wait(fence, false)
                                  amdgpu_job_timedout()
                                    drm_dev_enter() -> false
                                    return DRM_GPU_SCHED_STAT_ENODEV
                                      <-- fence is never signaled

When hub->sdma_invalidation_workaround is used, amdgpu_job_timedout() bails
out once dev->unplugged is set without force-completing the job fence, and
amdgpu_pci_remove() cannot reach amdgpu_fence_driver_hw_fini() until
synchronize_srcu(&drm_unplug_srcu) completes.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6aa895f63aeaf..e0c1a876d76ad 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1144,12 +1144,12 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
>  
>  	amdgpu_ttm_fini(adev);
>  
> -	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> +	if (amdgpu_dev_enter(adev, &idx)) {
>  		if (!adev->gmc.xgmi.connected_to_cpu && !adev->gmc.is_app_apu) {
>  			arch_phys_wc_del(adev->gmc.vram_mtrr);
>  			arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);
>  		}
> -		drm_dev_exit(idx);
> +		amdgpu_dev_exit(adev, idx);
>  	}
>  }

[Severity: Medium]
Does amdgpu_bo_fini() leak adev->gmc.vram_mtrr and the aperture WC memtype
reservation when tearing down a wedged device?

In amdgpu_device_fini_sw(), amdgpu_device_ip_fini() runs amdgpu_bo_fini()
before atomic_xchg(&adev->wedge_status, 0) clears wedge_status, so
amdgpu_dev_enter() returns false and skips arch_phys_wc_del() and
arch_io_free_memtype_wc() on a connected PCI device.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 82a2684dbd2e1..703f41b350aa9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
[ ... ]
> @@ -5155,19 +5155,19 @@ int psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t bin_size)
>  {
>  	int idx;
>  
> -	if (!drm_dev_enter(adev_to_drm(psp->adev), &idx))
> +	if (!amdgpu_dev_enter(psp->adev, &idx))
>  		return -ENODEV;
>  
>  	if (!bin_size || bin_size > PSP_1_MEG) {
>  		dev_err(psp->adev->dev, "PSP firmware is invalid\n");
> -		drm_dev_exit(idx);
> +		amdgpu_dev_exit(psp->adev, idx);
>  		return -EINVAL;
>  	}
>  
>  	memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>  	memcpy(psp->fw_pri_buf, start_addr, bin_size);
>  
> -	drm_dev_exit(idx);
> +	amdgpu_dev_exit(psp->adev, idx);
>  	return 0;
>  }

[Severity: High]
Does converting psp_copy_fw() and amdgpu_gmc_init_pdb0() to
amdgpu_dev_enter() break SR-IOV and Mode-2 GPU reset recovery when the
device is wedged?

adev->wedge_status is only cleared during reset in
amdgpu_device_reinit_after_reset().  However, SR-IOV reset in
amdgpu_device_reset_sriov() and custom Mode-2 reset handlers in
amdgpu_do_asic_reset() never call amdgpu_device_reinit_after_reset().
During amdgpu_device_reset_sriov(), psp_copy_fw() fails with -ENODEV and
amdgpu_gmc_init_pdb0() silently skips PDB0 initialization because
wedge_status is still 1, and both reset paths leave the device wedged after
reset completes.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 6c12d8aa3c769..7477aed52eb1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -352,9 +352,9 @@ static int amdgpu_vcn_save_vcpu_bo_inst(struct amdgpu_device *adev, int i)
>  	if (!adev->vcn.inst[i].saved_bo)
>  		return -ENOMEM;
>  
> -	if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> +	if (amdgpu_dev_enter(adev, &idx)) {
>  		memcpy_fromio(adev->vcn.inst[i].saved_bo, ptr, size);
> -		drm_dev_exit(idx);
> +		amdgpu_dev_exit(adev, idx);
>  	}
>  
>  	return 0;
> @@ -403,9 +403,9 @@ int amdgpu_vcn_resume(struct amdgpu_device *adev, int i)
>  	ptr = adev->vcn.inst[i].cpu_addr;
>  
>  	if (adev->vcn.inst[i].saved_bo != NULL) {
> -		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> +		if (amdgpu_dev_enter(adev, &idx)) {
>  			memcpy_toio(ptr, adev->vcn.inst[i].saved_bo, size);
> -			drm_dev_exit(idx);
> +			amdgpu_dev_exit(adev, idx);
>  		}
>  		kvfree(adev->vcn.inst[i].saved_bo);
>  		adev->vcn.inst[i].saved_bo = NULL;

[Severity: High]
When resetting a wedged GPU, does this overwrite the VCPU buffer in VRAM
with uninitialized kernel memory?

During a reset of a wedged device, amdgpu_device_pre_asic_reset() runs
amdgpu_device_ip_suspend() while adev->wedge_status is still 1.
amdgpu_vcn_save_vcpu_bo_inst() allocates saved_bo with kvmalloc() and skips
memcpy_fromio() because amdgpu_dev_enter() returns false.  Then
amdgpu_device_reinit_after_reset() clears wedge_status to 0 and calls
amdgpu_vcn_resume(), where amdgpu_dev_enter() succeeds and memcpy_toio()
copies the uninitialized saved_bo into the VCPU BO.  The same pattern
occurs in amdgpu_uvd_prepare_suspend() / amdgpu_uvd_resume() and
vce_v4_0_suspend() / vce_v4_0_resume().

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> index c73eb416c0e26..9d1fbddcdd7c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -387,7 +387,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  	if (r)
>  		return r;
>  
> -	if (!drm_dev_enter(adev_to_drm(adev), &idx))
> +	if (!amdgpu_dev_enter(adev, &idx))
>  		return -ENODEV;
>  
>  	r = vm->update_funcs->map_table(vmbo);

[Severity: High]
Does failing amdgpu_vm_pt_clear() when wedged trigger a NULL pointer
dereference in amdgpu_vm_init() whenever the DRM device is opened?

When a process opens the DRM device while wedged, amdgpu_driver_open_kms()
calls amdgpu_vm_init(), which calls amdgpu_vm_pt_clear() and jumps to
error_free_root on -ENODEV:

drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:amdgpu_vm_init() {
	...
error_free_root:
	amdgpu_vm_pt_free_root(adev, vm);
	amdgpu_bo_unreserve(vm->root.bo);
	amdgpu_bo_unref(&root_bo);
}

amdgpu_vm_pt_free_root() calls amdgpu_vm_pt_free(&vm->root), which sets
vm->root.bo = NULL before amdgpu_bo_unreserve(vm->root.bo) dereferences it.

[Severity: High]
Can a -ENODEV return from amdgpu_vm_pt_clear() also leave a dangling
entry->bo pointer in amdgpu_vm_pt_alloc()?

In amdgpu_vm_pt_alloc(), vm->eviction_lock is dropped across
amdgpu_vm_pt_create(), during which a concurrent timeout can wedge the
device.  amdgpu_vm_pt_alloc() then initializes entry with pt_bo before
calling amdgpu_vm_pt_clear():

drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c:amdgpu_vm_pt_alloc() {
	...
	amdgpu_vm_bo_base_init(entry, p->vm, pt_bo);
	r = amdgpu_vm_pt_clear(p->adev, p->vm, pt, p->immediate);
	if (r)
		goto error_unpin;

	return 0;

error_unpin:
	if (p->vm->is_npa)
		amdgpu_bo_unpin(pt_bo);
	amdgpu_bo_unref(&pt_bo);
	return r;
}

When amdgpu_vm_pt_clear() returns -ENODEV, error_unpin frees pt_bo without
calling amdgpu_vm_pt_free(entry), leaving entry->bo pointing to freed
memory and entry->vm_status linked on the VM status list.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109171333.4329-1-pierre-eric.pelloux-prayer@amd.com?part=6

Re: [PATCH v4 1/6] drm/amdkfd: avoid NULL kfd_process_device in kfd_profiler_pmc

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] drm/amdkfd: un-reverted lock state on NULL pdd in
  kfd_profiler_pmc()
- [Low] drm/amdkfd: missing Fixes trailer in kfd_profiler_pmc() commit
--

commit 04f061e589d38a636c5063f9828ed5587bde69f6
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

> drm/amdkfd: avoid NULL kfd_process_device in kfd_profiler_pmc
>
> All other callers of kfd_process_device_data_by_id deal with
> a possible NULL return value except kfd_profiler_pmc.
>
> Check it after the mutex unlock and return -EINVAL as others
> place do.
>
> Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20261029152000.22001-1-pierre-eric.pelloux-prayer@amd.com?part=4
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

[Severity: Low]
Since this fixes a NULL pointer dereference in kfd_profiler_pmc(), should
the commit message include a Fixes tag?

  Fixes: a789761de305 ("amd/amdkfd: Add kfd_ioctl_profiler to contain profiler kernel driver changes")

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index d65ae53c81a5c..95921369abb26 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -3463,8 +3463,10 @@ static inline int kfd_profiler_pmc(struct kfd_process *p,
>  	/* Enable/disable perfcount if requested */
>  	mutex_lock(&p->mutex);
>  	pdd = kfd_process_device_data_by_id(p, args->gpu_id);
> -	dqm = pdd->dev->dqm;
>  	mutex_unlock(&p->mutex);
> +	if (!pdd)
> +		return -EINVAL;
> +	dqm = pdd->dev->dqm;
>  
>  	dqm->ops.set_perfcount(dqm, args->perfcount_enable);
>  	return status;

[Severity: Medium]
Does returning -EINVAL here leave the device profiler lock and PTL disable
request leaked when args->lock == 1?

profile_lock_device() has already looked up pdd under p->mutex, set
kfd->profiler_process = p, and called kfd_ptl_disable_request(pdd, p).
Because p->mutex is dropped between the two lookups, a concurrent
criu_restore_devices() call updating pdd->user_gpu_id (or the device
becoming wedged later in this series) can cause this second
kfd_process_device_data_by_id() call to return NULL.  Returning -EINVAL at
that point leaves kfd->profiler_process and the PTL disable state held
when locking, so later profiling attempts fail with -EALREADY or -EBUSY
(or skips updating perfcount on the queues after unlocking when
args->lock == 0).

Looking up pdd once or passing pdd from profile_lock_device() would avoid
the second lookup and the un-reverted state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109171333.4329-1-pierre-eric.pelloux-prayer@amd.com?part=1