amdgpu_device_gpu_recover() removes the user doorbell and MMIO mappings
and calls amdgpu_amdkfd_pre_reset() before it checks whether the guilty
job has signaled. If the job has already signaled by then, the ASIC
reset is skipped, but KFD has been suspended for a reset that will not
happen: user doorbell and MMIO mappings are gone, and with the reset
domain held the HIQ and user queues stay mapped. On SR-IOV nothing
resumes KFD, because the only amdgpu_amdkfd_post_reset() for a VF is in
amdgpu_device_reset_sriov(). kfd_locked is leaked and later
open("/dev/kfd") fails with -EINVAL.
Move the unmapping and amdgpu_amdkfd_pre_reset() after the signal check,
so that they only run when the ASIC is reset, and skip
amdgpu_amdkfd_post_reset() on bare metal when the job has signaled.
Emergency restart does not check the fence and still runs them.
amdgpu_pci_error_detected() now calls amdgpu_amdkfd_pre_reset() itself,
since amdgpu_device_halt_activities() no longer does.
The check can also see a job that was never submitted to the ring: when
its finished fence already carries an error, for example one copied from
its entity by amdgpu_job_prepare_job(), amdgpu_job_run() skips it, and
the job stays on the pending list until it is freed. Its HW fence was
never initialized, and dma_fence_get_status() would take its NULL lock.
Such a job cannot have hung the ring, so skip the ASIC reset for it as
for a signaled job.
Fixes: 9f4f2c1a3524 ("drm/amd/amdgpu: fix the kfd pre_reset sequence in sriov")
Signed-off-by: Filip Adzic <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 47 ++++++++++++++--------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 44091ac12fa9..01648b0f28fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5541,8 +5541,6 @@ static void amdgpu_device_halt_activities(struct
amdgpu_device *adev,
cancel_delayed_work_sync(&tmp_adev->delayed_init_work);
- amdgpu_amdkfd_pre_reset(tmp_adev, reset_context);
-
/*
* Mark these ASICs to be reset as untracked first
* And add them back after reset completed
@@ -5687,13 +5685,15 @@ static int amdgpu_device_sched_resume(struct list_head
*device_list,
static void amdgpu_device_gpu_resume(struct amdgpu_device *adev,
struct list_head *device_list,
- bool need_emergency_restart)
+ bool need_emergency_restart,
+ bool job_signaled)
{
struct amdgpu_device *tmp_adev = NULL;
list_for_each_entry(tmp_adev, device_list, reset_list) {
/* unlock kfd: SRIOV would do it separately */
- if (!need_emergency_restart && !amdgpu_sriov_vf(tmp_adev))
+ if (!need_emergency_restart && !job_signaled &&
+ !amdgpu_sriov_vf(tmp_adev))
amdgpu_amdkfd_post_reset(tmp_adev);
/* kfd_post_reset will do nothing if kfd device is not
initialized,
@@ -5730,6 +5730,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
struct amdgpu_reset_context *reset_context)
{
struct list_head device_list;
+ struct amdgpu_device *tmp_adev = NULL;
bool job_signaled = false;
struct amdgpu_hive_info *hive = NULL;
int r = 0;
@@ -5794,28 +5795,36 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
*adev,
/* We need to lock reset domain only once both for XGMI and single
device */
amdgpu_device_recovery_get_reset_lock(adev, &device_list);
- /* unmap all the mappings of doorbell and framebuffer to prevent user
space from
- * accessing them
- */
- unmap_mapping_range(adev->ddev.anon_inode->i_mapping, 0, 0, 1);
- amdgpu_amdkfd_clear_kfd_mapping(adev);
-
amdgpu_device_halt_activities(adev, job, reset_context, &device_list,
hive, need_emergency_restart);
- if (need_emergency_restart)
- goto skip_sched_resume;
/*
* Must check guilty signal here since after this point all old
- * HW fences are force signaled.
+ * HW fences are force signaled. A job whose HW fence was never
+ * initialized was not submitted to the ring, so it cannot have hung it.
*
* job->base holds a reference to parent fence
*/
- if (job && (dma_fence_get_status(&job->hw_fence->base) > 0)) {
+ if (job && !need_emergency_restart &&
+ (!dma_fence_was_initialized(&job->hw_fence->base) ||
+ dma_fence_get_status(&job->hw_fence->base) > 0)) {
job_signaled = true;
- dev_info(adev->dev, "Guilty job already signaled, skipping HW
reset");
+ dev_info(adev->dev, "Guilty job %s, skipping HW reset",
+ dma_fence_was_initialized(&job->hw_fence->base) ?
+ "already signaled" : "was not submitted");
goto skip_hw_reset;
}
+ /* unmap all the mappings of doorbell and framebuffer to prevent user
space from
+ * accessing them
+ */
+ unmap_mapping_range(adev->ddev.anon_inode->i_mapping, 0, 0, 1);
+ amdgpu_amdkfd_clear_kfd_mapping(adev);
+ list_for_each_entry(tmp_adev, &device_list, reset_list)
+ amdgpu_amdkfd_pre_reset(tmp_adev, reset_context);
+
+ if (need_emergency_restart)
+ goto skip_sched_resume;
+
r = amdgpu_device_asic_reset(adev, &device_list, reset_context);
if (r)
goto reset_unlock;
@@ -5824,7 +5833,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
if (r)
goto reset_unlock;
skip_sched_resume:
- amdgpu_device_gpu_resume(adev, &device_list, need_emergency_restart);
+ amdgpu_device_gpu_resume(adev, &device_list, need_emergency_restart,
+ job_signaled);
reset_unlock:
amdgpu_device_recovery_put_reset_lock(adev, &device_list);
amdgpu_ras_post_reset(adev, &device_list);
@@ -6214,6 +6224,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev
*pdev, pci_channel_sta
struct amdgpu_hive_info *hive __free(xgmi_put_hive) =
amdgpu_get_xgmi_hive(adev);
struct amdgpu_reset_context reset_context;
+ struct amdgpu_device *tmp_adev;
struct list_head device_list;
dev_info(adev->dev, "PCI error: detected callback!!\n");
@@ -6254,6 +6265,8 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev
*pdev, pci_channel_sta
amdgpu_device_recovery_get_reset_lock(adev, &device_list);
amdgpu_device_halt_activities(adev, NULL, &reset_context,
&device_list,
hive, false);
+ list_for_each_entry(tmp_adev, &device_list, reset_list)
+ amdgpu_amdkfd_pre_reset(tmp_adev, &reset_context);
if (hive)
mutex_unlock(&hive->hive_lock);
return PCI_ERS_RESULT_NEED_RESET;
@@ -6428,7 +6441,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
list_add_tail(&adev->reset_list, &device_list);
}
amdgpu_device_sched_resume(&device_list, NULL, NULL);
- amdgpu_device_gpu_resume(adev, &device_list, false);
+ amdgpu_device_gpu_resume(adev, &device_list, false, false);
amdgpu_device_recovery_put_reset_lock(adev, &device_list);
if (hive) {
--
2.43.0
