[PATCH v1 1/3] drm/amdgpu: add wedge event implementation

[PATCH v1 1/3] 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. 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 to /sys/kernel/debug/dri/X/amdgpu_wedge to enable/disable
  at runtime for one GPU

The implementation uses at atomic to store the wedge status.
Runtime power management is disabled when the decided is wedged
(because we can't submit any work to the GPU).

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 |  6 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  5 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c  | 35 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c    | 43 ++++++++++++++++++++--
 6 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7974f9b7944f..83f4a86303bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -957,6 +957,11 @@ struct amdgpu_device {
 
 	struct amdgpu_uma_carveout_info uma_info;
 
+	struct {
+		atomic_t flag;
+		int boot_gpu_recovery;
+	} wedged;
+
 	/* KFD
 	 * Must be last --ends in a flexible-array member.
 	 */
@@ -1342,4 +1347,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->wedged.flag);
+}
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 964efec0d335..c636a94d869c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4034,6 +4034,10 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 		}
 	}
 
+	adev->wedged.boot_gpu_recovery = amdgpu_gpu_recovery;
+	if (atomic_xchg(&adev->wedged.flag, 0))
+		pm_runtime_put_autosuspend(adev->dev);
+
 fence_driver_init:
 	/* Fence driver */
 	r = amdgpu_fence_driver_sw_init(adev);
@@ -5740,6 +5744,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 		dev_info(adev->dev, "GPU reset end with ret = %d\n", r);
 
 	atomic_set(&adev->reset_domain->reset_res, r);
+	if (atomic_xchg(&adev->wedged.flag, 0))
+		pm_runtime_put_autosuspend(adev->dev);
 
 	if (!r) {
 		struct amdgpu_task_info *ti = NULL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0ab380ca7e64..399e935df7b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -3006,10 +3006,15 @@ long amdgpu_drm_ioctl(struct file *filp,
 		      unsigned int cmd, unsigned long arg)
 {
 	struct drm_file *file_priv = filp->private_data;
+	struct amdgpu_device *adev;
 	struct drm_device *dev;
 	long ret;
 
 	dev = file_priv->minor->dev;
+	adev = drm_to_adev(dev);
+	if (amdgpu_device_is_wedged(adev))
+		return -ENODEV;
+
 	ret = pm_runtime_get_sync(dev->dev);
 	if (ret < 0)
 		goto out;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 3043ad041bb4..b24106298576 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -974,6 +974,39 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file *m, void *unused)
 	return 0;
 }
 
+/*
+ * amdgpu_debugfs_wedge - enable/disable "wedged" device framework.
+ *
+ * When enabled, gpu auto recovery will be disabled and the device has to
+ * be reset manually (equivalent to amdgpu.gpu_recovery=0).
+ *
+ * Read will return if the device is currently wedged.
+ */
+static int amdgpu_device_wedged_get(void *data, u64 *val)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)data;
+	*val = amdgpu_device_is_wedged(adev);
+	return 0;
+}
+static int amdgpu_device_wedged_set(void *data, u64 val)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)data;
+
+	if (amdgpu_device_is_wedged(adev))
+		return -EINVAL;
+
+	if (val)
+		amdgpu_gpu_recovery = 0;
+	else
+		amdgpu_gpu_recovery = adev->wedged.boot_gpu_recovery;
+
+	return 0;
+}
+
+
+DEFINE_DEBUGFS_ATTRIBUTE(amdgpu_debugfs_wedge_fops, amdgpu_device_wedged_get, amdgpu_device_wedged_set,
+			 "%lld\n");
+
 /*
  * amdgpu_debugfs_gpu_recover - manually trigger a gpu reset & recover
  *
@@ -1039,6 +1072,8 @@ void amdgpu_debugfs_fence_init(struct amdgpu_device *adev)
 		INIT_WORK(&adev->reset_work, amdgpu_debugfs_reset_work);
 		debugfs_create_file("amdgpu_gpu_recover", 0444, root, adev,
 				    &amdgpu_debugfs_gpu_recover_fops);
+		debugfs_create_file("amdgpu_wedge", 0644, root, adev,
+				    &amdgpu_debugfs_wedge_fops);
 	}
 #endif
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 3dc8faa091d7..f43d85ba4b78 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -716,6 +716,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 d97cf1212e0f..187eac9aab1f 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,44 @@ 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))
+		if (amdgpu_sriov_vf(adev)) {
+			drm_sched_suspend_timeout(&ring->sched);
 			adev->virt.tdr_debug = true;
+		} else {
+			/* Declare the device as wedged if it's not already. */
+			struct drm_gpu_scheduler *sched;
+			struct amdgpu_ring *ring;
+			struct amdgpu_fence *guilty_fence;
+			int i;
+
+			if (!atomic_xchg(&adev->wedged.flag, 1)) {
+				pm_runtime_get_sync(adev->dev);
+
+				pci_clear_master(adev->pdev);
+
+				drm_dev_wedged_event(&adev->ddev, DRM_WEDGE_RECOVERY_REBIND |
+						DRM_WEDGE_RECOVERY_BUS_RESET, NULL);
+			}
+
+			guilty_fence = to_amdgpu_job(s_job)->hw_fence;
+
+			for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
+				ring = adev->rings[i];
+
+				if (!amdgpu_ring_sched_ready(ring))
+					continue;
+
+				sched = &ring->sched;
+
+				/* Stop the scheduler to 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:
@@ -452,7 +488,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) {
-- 
2.43.0

[PATCH v1 2/3] drm/amdgpu: skip amdgpu_gart_unbind if the device is wedged

[Pierre-Eric Pelloux-Prayer]

Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index c4c21dbbbdbf..09ddfccb8174 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -315,7 +315,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
 	uint64_t flags = 0;
 	int idx;
 
-	if (!adev->gart.ptr)
+	if (!adev->gart.ptr || amdgpu_device_is_wedged(adev))
 		return;
 
 	if (!drm_dev_enter(adev_to_drm(adev), &idx))
-- 
2.43.0

Re: [PATCH v1 2/3] drm/amdgpu: skip amdgpu_gart_unbind if the device is wedged

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Skipping `amdgpu_gart_unbind` when the device is wedged leaves GART PTEs pointing to freed system memory, enabling runaway DMA corruption.
--

--- Patch [2]: [PATCH v1 2/3] drm/amdgpu: skip amdgpu_gart_unbind if the device is wedged ---
commit 6802482fc9c63b1193bceee62eda365971bd0506
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: skip amdgpu_gart_unbind if the device is wedged

This commit modifies amdgpu_gart_unbind() to return early if the device is
wedged. This avoids unbinding GART pages during a wedged state.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index c4c21dbbbdbf8..09ddfccb81743 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -315,7 +315,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>  	uint64_t flags = 0;
>  	int idx;
>  
> -	if (!adev->gart.ptr)
> +	if (!adev->gart.ptr || amdgpu_device_is_wedged(adev))
>  		return;

[Severity: Critical]
Does skipping this unbind leave GART PTEs pointing to freed system memory in
amdgpu_gart_unbind()?

When the GPU is wedged, the driver explicitly skips resetting the GPU but
signals all pending fences to unblock the system. TTM responds to these
signaled fences by freeing the backing memory of the completed or killed
jobs, returning the physical pages to the OS.

Because this new code returns early, it skips updating the GART PTEs to the
safe dummy_page_addr. If a userspace application hangs the GPU (e.g. an
infinite loop with memory writes), the application exits, and its memory is
reallocated to other processes or the kernel.

Could the hung GPU, which is deliberately left powered on and electrically
active by the wedge state, continue performing DMA writes through the stale
GART PTEs? This appears it would corrupt the newly allocated system memory,
causing silent corruption on systems where the IOMMU is disabled or running
in passthrough mode.

>  
>  	if (!drm_dev_enter(adev_to_drm(adev), &idx))
>  		return;

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

[PATCH v1 3/3] drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged

[Pierre-Eric Pelloux-Prayer]

Otherwise it'll fail with: "failed to reg_write_reg_wait" and we're
going to fill the MES buffer which will prevent the reset to succeed.

Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index f43d85ba4b78..c33320a6413e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -786,6 +786,9 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 	int r, cnt = 0;
 	uint32_t seq;
 
+	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.
-- 
2.43.0

Re: [PATCH v1 3/3] drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing deadlock risk: `amdgpu_gmc_flush_gpu_tlb_pasid()` uses plain `spin_lock()` for `ring_lock` while KIQ accessor functions use `spin_lock_irqsave()` and run in hardirq context.
--

--- Patch [3]: [PATCH v1 3/3] drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged ---
commit 24e4f92f6cbb016df09e7fbd1d0ec02c367162a1
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged

This commit skips the GPU TLB flush for a specific pasid when the device
is wedged. This avoids failing to wait for the register write, which could
fill the MES buffer and prevent a reset from succeeding.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index f43d85ba4b78b..c33320a6413e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -786,6 +786,9 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
>  	int r, cnt = 0;
>  	uint32_t seq;
>  
> +	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.

[ ... ]

>  	} else {
>  		/* 2 dwords flush + 8 dwords fence */
>  		ndw = kiq->pmf->invalidate_tlbs_size + 8;
> 
>  		if (adev->gmc.flush_tlb_needs_extra_type_2)
>  			ndw += kiq->pmf->invalidate_tlbs_size;
> 
>  		if (adev->gmc.flush_tlb_needs_extra_type_0)
>  			ndw += kiq->pmf->invalidate_tlbs_size;
> 
>  		spin_lock(&adev->gfx.kiq[inst].ring_lock);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed that
amdgpu_gmc_flush_gpu_tlb_pasid() acquires the KIQ ring_lock using plain
spin_lock() here, while other KIQ accessor functions like amdgpu_kiq_wreg()
acquire it with spin_lock_irqsave() because they can run in hardirq context.

If a hardware interrupt occurs on the same CPU while this lock is held,
could an interrupt handler attempting to read or write a register using KIQ
(such as via WREG32 on an SR-IOV guest) deadlock when trying to acquire the
same ring_lock?

Does this lock acquisition need to be changed to spin_lock_irqsave()?

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

Re: [PATCH v1 1/3] drm/amdgpu: add wedge event implementation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Concurrent execution of wedge recovery across multiple rings causes race conditions in DRM scheduler and fence driver state.
- [High] Unconditionally returning `-ENODEV` in `amdgpu_drm_ioctl` blocks all core DRM ioctls, breaking userspace tools, KMS, and resource cleanup.
- [Medium] The per-device debugfs node `amdgpu_wedge` incorrectly modifies a global module parameter, affecting all GPUs.
- [Medium] The PM runtime usage counter is leaked if the device is removed or unbound while in the wedged state.
- [Medium] A Time-of-Check to Time-of-Use (TOCTOU) race exists between `amdgpu_job_timedout` setting the wedge flag and `amdgpu_device_gpu_recover` clearing it.
--

--- Patch [1]: [PATCH v1 1/3] drm/amdgpu: add wedge event implementation ---
commit da2e81546012e13c20ccab9ed3ffded97a0f3ece
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Subject: drm/amdgpu: add wedge event implementation

This commit adds a wedge framework for amdgpu to disable gpu recovery and
signal pending fences upon a hang, allowing userspace to inspect hardware
state. The wedge status is tracked via an atomic flag, and users can
manually trigger recovery later.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 964efec0d3352..c636a94d869c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
[ ... ]
> @@ -5740,6 +5744,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>  		dev_info(adev->dev, "GPU reset end with ret = %d\n", r);
>  
>  	atomic_set(&adev->reset_domain->reset_res, r);
> +	if (atomic_xchg(&adev->wedged.flag, 0))
> +		pm_runtime_put_autosuspend(adev->dev);
>  

[Severity: Medium]
Is there a race condition between setting the wedge flag in
amdgpu_job_timedout() and clearing it here?

If a TDR thread in amdgpu_job_timedout() sets the flag but gets preempted
before calling pci_clear_master(), a concurrent call to
amdgpu_device_gpu_recover() could clear the flag.

If the TDR thread then resumes execution, could it execute
pci_clear_master() on a successfully recovered and active GPU?

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0ab380ca7e645..399e935df7b29 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -3006,10 +3006,15 @@ long amdgpu_drm_ioctl(struct file *filp,
>  		      unsigned int cmd, unsigned long arg)
>  {
>  	struct drm_file *file_priv = filp->private_data;
> +	struct amdgpu_device *adev;
>  	struct drm_device *dev;
>  	long ret;
>  
>  	dev = file_priv->minor->dev;
> +	adev = drm_to_adev(dev);
> +	if (amdgpu_device_is_wedged(adev))
> +		return -ENODEV;
> +

[Severity: High]
Does returning -ENODEV unconditionally here block all core DRM ioctls?

This seems like it would lock out standard KMS and modeset queries, as
well as diagnostic tools attempting to read hardware state.

Could this prevent the exact hardware inspection the commit message
mentions?

>  	ret = pm_runtime_get_sync(dev->dev);
>  	if (ret < 0)
>  		goto out;

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 3043ad041bb42..b241062985769 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
[ ... ]
> @@ -974,6 +974,39 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file *m, void *unused)
> +static int amdgpu_device_wedged_set(void *data, u64 val)
> +{
> +	struct amdgpu_device *adev = (struct amdgpu_device *)data;
> +
> +	if (amdgpu_device_is_wedged(adev))
> +		return -EINVAL;
> +
> +	if (val)
> +		amdgpu_gpu_recovery = 0;
> +	else
> +		amdgpu_gpu_recovery = adev->wedged.boot_gpu_recovery;
> +
> +	return 0;
> +}
> +

[Severity: Medium]
Does modifying amdgpu_gpu_recovery here affect all GPUs in a multi-GPU
system?

The commit message mentions this is to enable/disable at runtime for one
GPU, but amdgpu_gpu_recovery is a global module parameter.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index d97cf1212e0f9..187eac9aab1f0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
[ ... ]
> @@ -184,9 +185,44 @@ 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))
> +		if (amdgpu_sriov_vf(adev)) {
> +			drm_sched_suspend_timeout(&ring->sched);
>  			adev->virt.tdr_debug = true;
> +		} else {
> +			/* Declare the device as wedged if it's not already. */
> +			struct drm_gpu_scheduler *sched;
> +			struct amdgpu_ring *ring;
> +			struct amdgpu_fence *guilty_fence;
> +			int i;
> +
> +			if (!atomic_xchg(&adev->wedged.flag, 1)) {
> +				pm_runtime_get_sync(adev->dev);
> +

[Severity: Medium]
If the device is unbound or hot-unplugged while in the wedged state, will
this pm_runtime_get_sync() reference be leaked?

It appears there is no corresponding put if the device is torn down before
manual recovery is triggered.

> +				pci_clear_master(adev->pdev);
> +
> +				drm_dev_wedged_event(&adev->ddev, DRM_WEDGE_RECOVERY_REBIND |
> +						DRM_WEDGE_RECOVERY_BUS_RESET, NULL);
> +			}
> +
> +			guilty_fence = to_amdgpu_job(s_job)->hw_fence;
> +
> +			for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> +				ring = adev->rings[i];
> +
> +				if (!amdgpu_ring_sched_ready(ring))
> +					continue;
> +
> +				sched = &ring->sched;
> +
> +				/* Stop the scheduler to 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]
Since this loop iterating over AMDGPU_MAX_RINGS is outside the
atomic_xchg() check, can multiple concurrent TDR threads race on stopping
and starting schedulers?

If multiple rings hang concurrently, all their respective TDR threads will
attempt to stop/start the schedulers and force fence completions across
all rings simultaneously, which could corrupt the scheduler and fence
driver state.

> +		}
>  	}
>  
>  exit:

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