hisi_acc_vfio_pci: Remove the deferred_reset logic

The deferred_reset logic was added to vfio migration drivers to prevent
a circular locking dependency with respect to mm_lock and state mutex.
This is mainly because of the copy_to/from_user() functions(which takes
mm_lock) invoked under state mutex. But for HiSilicon driver, the only
place where we now hold the state mutex for copy_to_user is during the
PRE_COPY IOCTL. So for pre_copy, release the lock as soon as we have
updated the data and perform copy_to_user without state mutex. By this,
we can get rid of the deferred_reset logic.

Link: https://lore.kernel.org/kvm/20240220132459.GM13330@nvidia.com/
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/20240229091152.56664-1-shameerali.kolothum.thodi@huawei.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index 4d27465c8f1a893352bae1498303a5da7e4ebb6d..9a3e97108ace812543cd87eff0b7bca6f190d9dc 100644 (file)
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -630,25 +630,11 @@ static void hisi_acc_vf_disable_fds(struct hisi_acc_vf_core_device *hisi_acc_vde
        }
 }
 
-/*
- * This function is called in all state_mutex unlock cases to
- * handle a 'deferred_reset' if exists.
- */
-static void
-hisi_acc_vf_state_mutex_unlock(struct hisi_acc_vf_core_device *hisi_acc_vdev)
+static void hisi_acc_vf_reset(struct hisi_acc_vf_core_device *hisi_acc_vdev)
 {
-again:
-       spin_lock(&hisi_acc_vdev->reset_lock);
-       if (hisi_acc_vdev->deferred_reset) {
-               hisi_acc_vdev->deferred_reset = false;
-               spin_unlock(&hisi_acc_vdev->reset_lock);
-               hisi_acc_vdev->vf_qm_state = QM_NOT_READY;
-               hisi_acc_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
-               hisi_acc_vf_disable_fds(hisi_acc_vdev);
-               goto again;
-       }
-       mutex_unlock(&hisi_acc_vdev->state_mutex);
-       spin_unlock(&hisi_acc_vdev->reset_lock);
+       hisi_acc_vdev->vf_qm_state = QM_NOT_READY;
+       hisi_acc_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
+       hisi_acc_vf_disable_fds(hisi_acc_vdev);
 }
 
 static void hisi_acc_vf_start_device(struct hisi_acc_vf_core_device *hisi_acc_vdev)
@@ -804,8 +790,10 @@ static long hisi_acc_vf_precopy_ioctl(struct file *filp,
 
        info.dirty_bytes = 0;
        info.initial_bytes = migf->total_length - *pos;
+       mutex_unlock(&migf->lock);
+       mutex_unlock(&hisi_acc_vdev->state_mutex);
 
-       ret = copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
+       return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
 out:
        mutex_unlock(&migf->lock);
        mutex_unlock(&hisi_acc_vdev->state_mutex);
@@ -1071,7 +1059,7 @@ hisi_acc_vfio_pci_set_device_state(struct vfio_device *vdev,
                        break;
                }
        }
-       hisi_acc_vf_state_mutex_unlock(hisi_acc_vdev);
+       mutex_unlock(&hisi_acc_vdev->state_mutex);
        return res;
 }
 
@@ -1092,7 +1080,7 @@ hisi_acc_vfio_pci_get_device_state(struct vfio_device *vdev,
 
        mutex_lock(&hisi_acc_vdev->state_mutex);
        *curr_state = hisi_acc_vdev->mig_state;
-       hisi_acc_vf_state_mutex_unlock(hisi_acc_vdev);
+       mutex_unlock(&hisi_acc_vdev->state_mutex);
        return 0;
 }
 
@@ -1104,21 +1092,9 @@ static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev)
                                VFIO_MIGRATION_STOP_COPY)
                return;
 
-       /*
-        * As the higher VFIO layers are holding locks across reset and using
-        * those same locks with the mm_lock we need to prevent ABBA deadlock
-        * with the state_mutex and mm_lock.
-        * In case the state_mutex was taken already we defer the cleanup work
-        * to the unlock flow of the other running context.
-        */
-       spin_lock(&hisi_acc_vdev->reset_lock);
-       hisi_acc_vdev->deferred_reset = true;
-       if (!mutex_trylock(&hisi_acc_vdev->state_mutex)) {
-               spin_unlock(&hisi_acc_vdev->reset_lock);
-               return;
-       }
-       spin_unlock(&hisi_acc_vdev->reset_lock);
-       hisi_acc_vf_state_mutex_unlock(hisi_acc_vdev);
+       mutex_lock(&hisi_acc_vdev->state_mutex);
+       hisi_acc_vf_reset(hisi_acc_vdev);
+       mutex_unlock(&hisi_acc_vdev->state_mutex);
 }
 
 static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device *hisi_acc_vdev)

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
index dcabfeec6ca19d4334e746536c5e15a3572972d5..5bab46602fad261ee602e4524245f6551fa2af15 100644 (file)
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
@@ -98,8 +98,8 @@ struct hisi_acc_vf_migration_file {
 
 struct hisi_acc_vf_core_device {
        struct vfio_pci_core_device core_device;
-       u8 match_done:1;
-       u8 deferred_reset:1;
+       u8 match_done;
+
        /* For migration state */
        struct mutex state_mutex;
        enum vfio_device_mig_state mig_state;
@@ -109,8 +109,6 @@ struct hisi_acc_vf_core_device {
        struct hisi_qm vf_qm;
        u32 vf_qm_state;
        int vf_id;
-       /* For reset handler */
-       spinlock_t reset_lock;
        struct hisi_acc_vf_migration_file *resuming_migf;
        struct hisi_acc_vf_migration_file *saving_migf;
 };