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 <[email protected]>
> ---
> 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);
