On 2021-04-28 11:11 a.m., Andrey Grodzovsky wrote:
Make sure all fecens dependent on HW present are force signaled
when handling device removal. This helpes later to scope all HW
accesing code such as IOCTLs in drm_dev_enter/exit and use
drm_dev_unplug as synchronization point past which we know HW
will not be accessed anymore outside of pci remove driver callback.
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 98 ++++++++++++++++++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 12 +--
4 files changed, 103 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 0db0ba4fba89..df6c5ed676b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1374,6 +1374,8 @@ void amdgpu_pci_resume(struct pci_dev *pdev);
bool amdgpu_device_cache_pci_state(struct pci_dev *pdev);
bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
+void amdgpu_finilize_device_fences(struct drm_device *dev);
+
#include "amdgpu_object.h"
static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 33e8e9e1d1fe..55afc11c17e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3692,15 +3692,12 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
amdgpu_virt_fini_data_exchange(adev);
}
- /* disable all interrupts */
- amdgpu_irq_disable_all(adev);
if (adev->mode_info.mode_config_initialized){
if (!amdgpu_device_has_dc_support(adev))
drm_helper_force_disable_all(adev_to_drm(adev));
else
drm_atomic_helper_shutdown(adev_to_drm(adev));
}
- amdgpu_fence_driver_fini_hw(adev);
if (adev->pm_sysfs_en)
amdgpu_pm_sysfs_fini(adev);
@@ -4567,14 +4564,19 @@ static bool amdgpu_device_lock_adev(struct amdgpu_device *adev,
return true;
}
-static void amdgpu_device_unlock_adev(struct amdgpu_device *adev)
+static void amdgpu_device_unlock_adev_imp(struct amdgpu_device *adev, bool skip_in_gpu_reset)
{
amdgpu_vf_error_trans_all(adev);
adev->mp1_state = PP_MP1_STATE_NONE;
- atomic_set(&adev->in_gpu_reset, 0);
+ !skip_in_gpu_reset ? atomic_set(&adev->in_gpu_reset, 0) : 0;
up_write(&adev->reset_sem);
}
+static void amdgpu_device_unlock_adev(struct amdgpu_device *adev)
+{
+ amdgpu_device_unlock_adev_imp(adev, false);
+}
+
/*
* to lockup a list of amdgpu devices in a hive safely, if not a hive
* with multiple nodes, it will be similar as amdgpu_device_lock_adev.
@@ -5321,3 +5323,89 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev)
}
+static void amdgpu_finilize_schedulded_fences(struct amdgpu_ctx_mgr *mgr)
+{
+ struct amdgpu_ctx *ctx;
+ struct idr *idp;
+ uint32_t id, i, j;
+
+ idp = &mgr->ctx_handles;
+
+ idr_for_each_entry(idp, ctx, id) {
+ for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
+ for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
+ struct drm_sched_entity *entity;
+
+ if (!ctx->entities[i][j])
+ continue;
+
+ entity = &ctx->entities[i][j]->entity;
+ drm_sched_entity_kill_jobs(entity);
+ }
+ }
+ }
+}
+
+/**
+ * amdgpu_finilize_device_fences() - Finilize all device fences
+ * @pdev: pointer to PCI device
+ *
+ * Will disable and finilise ISRs and will signal all fences
+ * that might hang if HW is gone
+ */
+void amdgpu_finilize_device_fences(struct drm_device *dev)
+{
+ struct amdgpu_device *adev = drm_to_adev(dev);
+ struct drm_file *file;
+
+ /*
+ * Block TDRs from further execution by setting adev->in_gpu_reset
+ * instead of holding full reset lock in order to not deadlock
+ * further ahead against any thread locking the reset lock when we
+ * wait for it's completion
+ */
+ while (!amdgpu_device_lock_adev(adev, NULL))
+ amdgpu_cancel_all_tdr(adev);
+ amdgpu_device_unlock_adev_imp(adev, true);
+
+
+ /* disable all HW interrupts */
+ amdgpu_irq_disable_all(adev);
+
+ /* stop and flush all in flight HW interrupts handlers */
+ disable_irq(pci_irq_vector(adev->pdev, 0));
+
+ /*
+ * Stop SW GPU schedulers and force completion on all HW fences. Since
+ * in the prev. step all ISRs were disabled and completed the
+ * HW fence array is idle (no insertions or extractions) and so it's
+ * safe to iterate it bellow.
+ * After this step all HW fences in the system are signaled. As a result
+ * also all the scheduler 'finished' fences are also signaled.
+ */
+ amdgpu_fence_driver_fini_hw(adev);
+
+ /*
+ * Reject any further jobs to any scheduler entity queue. After this
+ * step no new insertions and because schedulers are stopped also no
+ * new extractions.
+ */
+ down_read(&adev->sched_fence_completion_sem);
+ adev->stop_job_submissions = true;
+ up_read(&adev->sched_fence_completion_sem);
FYI: Typo here - down_write and up_write obviously.
Andrey
+
+ /*
+ * Complete all scheduler 'scheduled' fences currently pending.
+ * It's OK if new contexts and sched entities are concurrently
+ * still created as they will fail in pushing jobs to SW queues
+ * and their schedule fences will be signaled with error
+ */
+ mutex_lock(&adev->ddev.filelist_mutex);
+ list_for_each_entry(file, &adev->ddev.filelist, lhead) {
+ struct amdgpu_fpriv *fpriv = file->driver_priv;
+ amdgpu_finilize_schedulded_fences(&fpriv->ctx_mgr);
+ }
+ mutex_unlock(&adev->ddev.filelist_mutex);
+}
+
+
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f799c40d7e72..8a19b8dd02ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1249,6 +1249,12 @@ amdgpu_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
+ /*
+ * Force completion of all device related fences that might hang us when
+ * synchronizing SRCU in the following step.
+ */
+ amdgpu_finilize_device_fences(dev);
+
drm_dev_unplug(dev);
amdgpu_driver_unload_kms(dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 2670201e78d3..af592b28cd35 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -526,7 +526,7 @@ int amdgpu_fence_driver_init(struct amdgpu_device *adev)
*/
void amdgpu_fence_driver_fini_hw(struct amdgpu_device *adev)
{
- int i, r;
+ int i;
for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
struct amdgpu_ring *ring = adev->rings[i];
@@ -535,18 +535,10 @@ void amdgpu_fence_driver_fini_hw(struct amdgpu_device *adev)
continue;
/* Stop any new job submissions from sched before flushing the ring */
- /* TODO Handle amdgpu_job_submit_direct and amdgpu_amdkfd_submit_ib */
if (!ring->no_scheduler)
drm_sched_fini(&ring->sched);
- /* You can't wait for HW to signal if it's gone */
- if (!drm_dev_is_unplugged(&adev->ddev))
- r = amdgpu_fence_wait_empty(ring);
- else
- r = -ENODEV;
- /* no need to trigger GPU reset as we are unloading */
- if (r)
- amdgpu_fence_driver_force_completion(ring);
+ amdgpu_fence_driver_force_completion(ring);
if (ring->fence_drv.irq_src)
amdgpu_irq_put(adev, ring->fence_drv.irq_src,
