mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	Felix Kuehling <Felix.Kuehling@amd.com>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] drm/amdgpu: add wedge event implementation
Date: Mon, 2 Nov 2026 16:44:48 +0100	[thread overview]
Message-ID: <0c8f89c5-b02b-491b-99a5-f881ea82a804@amd.com> (raw)
In-Reply-To: <20261029152000.22001-4-pierre-eric.pelloux-prayer@amd.com>

On 10/29/26 16:19, Pierre-Eric Pelloux-Prayer wrote:
> 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>

Looks mostly reasonable to me. 

> ---
>  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    | 34 ++++++++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 12 ++++++++
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c   |  6 +++-
>  8 files changed, 83 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 34bbb30b6f0c..cd4959486c94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -989,6 +989,13 @@ struct amdgpu_device {
>  	 */
>  	bool gpu_recovery_enabled;
>  
> +	/* 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 ec2514079919..641b7fe24951 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4110,6 +4110,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	}
>  
>  	adev->gpu_recovery_enabled = true;
> +	atomic_set(&adev->wedge_status, 0);
>  
>  fence_driver_init:
>  	/* Fence driver */
> @@ -4362,6 +4363,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 */
> @@ -5155,6 +5159,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..617d4eec4e26 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);
> @@ -961,6 +964,11 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>  		abo = NULL;
>  	}
>  
> +	if (amdgpu_device_is_wedged(adev)) {
> +		r = -ENODEV;
> +		goto error_put_gobj;
> +	}
> +
>  	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->dev, lock);
>  
>  	r = PM_RUNTIME_ACQUIRE_ERR(&lock);
> 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 0b8d0a79dd45..f72c9c690c3b 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>
>  
> @@ -185,9 +186,35 @@ 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;

I think we should just completely drop the tdr_debug approach from the VF handling under SRIOV.

The wedge event approach should be completely sufficient to handle all of that.

Regards,
Christian.

> +		} else {
> +			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:
> @@ -439,7 +466,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/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;


  reply	other threads:[~2026-11-02 15:44 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 15:19 [PATCH v3 1/5] drm/amdgpu: fix NULL deref in error path Pierre-Eric Pelloux-Prayer
2026-10-29 15:19 ` [PATCH v3 2/5] drm/amdgpu: add gpu_recovery_enabled flag to amdgpu_device Pierre-Eric Pelloux-Prayer
2026-11-02 15:21   ` Christian König
2026-10-29 15:19 ` [PATCH v3 3/5] drm/amdgpu: honor gpu_recovery_enabled in amdgpu_job_timedout Pierre-Eric Pelloux-Prayer
2026-10-29 15:19 ` [PATCH v3 4/5] drm/amdgpu: add wedge event implementation Pierre-Eric Pelloux-Prayer
2026-11-02 15:44   ` Christian König [this message]
2026-11-02 16:44     ` Pierre-Eric Pelloux-Prayer
2026-11-02 17:04       ` Christian König
2026-10-29 15:19 ` [PATCH v3 5/5] drm/amdgpu: introduce amdgpu_dev_enter / exit Pierre-Eric Pelloux-Prayer
2026-11-02 15:46   ` Christian König
2026-10-30  9:32 ` [PATCH v3 1/5] drm/amdgpu: fix NULL deref in error path Christian König

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=0c8f89c5-b02b-491b-99a5-f881ea82a804@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-eric.pelloux-prayer@amd.com \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

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

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

all inboxes | Powered by JetHome®