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

[Pierre-Eric Pelloux-Prayer <pierre-eric@damsy.net>]

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);
>