* [PATCH 1/6] vfio: Introduce .migration_reset_state() callback
2025-11-20 12:36 [PATCH 0/6] vfio: Reset migration device state using dedicated callback Michał Winiarski
@ 2025-11-20 12:36 ` Michał Winiarski
2025-11-21 7:13 ` Tian, Kevin
2025-11-21 18:16 ` Alex Williamson
2025-11-20 12:36 ` [PATCH 2/6] hisi_acc_vfio_pci: Use " Michał Winiarski
` (4 subsequent siblings)
5 siblings, 2 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-20 12:36 UTC (permalink / raw)
To: Jason Gunthorpe, Alex Williamson, Kevin Tian, Yishai Hadas,
Longfang Liu, Shameer Kolothum, Brett Creeley, Giovanni Cabiddu,
kvm, qat-linux, virtualization, linux-kernel
Cc: Michał Winiarski
Resetting the migration device state is typically delegated to PCI
.reset_done() callback.
With VFIO, reset is usually called under vdev->memory_lock, which causes
lockdep to report a following circular locking dependency scenario:
0: set_device_state
driver->state_mutex -> migf->lock
1: data_read
migf->lock -> mm->mmap_lock
2: vfio_pin_dma
mm->mmap_lock -> vdev->memory_lock
3: vfio_pci_ioctl_reset
vdev->memory_lock -> driver->state_mutex
Introduce a .migration_reset_state() callback called outside of
vdev->memory_lock to break the dependency chain.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/vfio/pci/vfio_pci_core.c | 25 ++++++++++++++++++++++---
include/linux/vfio.h | 4 ++++
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 7dcf5439dedc9..d919636558ec8 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -553,6 +553,16 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
}
EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
+static void vfio_pci_dev_migration_reset_state(struct vfio_pci_core_device *vdev)
+{
+ lockdep_assert_not_held(&vdev->memory_lock);
+
+ if (!vdev->vdev.mig_ops->migration_reset_state)
+ return;
+
+ vdev->vdev.mig_ops->migration_reset_state(&vdev->vdev);
+}
+
void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
{
struct pci_dev *pdev = vdev->pdev;
@@ -662,8 +672,10 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
* overwrite the previously restored configuration information.
*/
if (vdev->reset_works && pci_dev_trylock(pdev)) {
- if (!__pci_reset_function_locked(pdev))
+ if (!__pci_reset_function_locked(pdev)) {
vdev->needs_reset = false;
+ vfio_pci_dev_migration_reset_state(vdev);
+ }
pci_dev_unlock(pdev);
}
@@ -1230,6 +1242,8 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
ret = pci_try_reset_function(vdev->pdev);
up_write(&vdev->memory_lock);
+ vfio_pci_dev_migration_reset_state(vdev);
+
return ret;
}
@@ -2129,6 +2143,7 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
if (vdev->vdev.mig_ops) {
if (!(vdev->vdev.mig_ops->migration_get_state &&
vdev->vdev.mig_ops->migration_set_state &&
+ vdev->vdev.mig_ops->migration_reset_state &&
vdev->vdev.mig_ops->migration_get_data_size) ||
!(vdev->vdev.migration_flags & VFIO_MIGRATION_STOP_COPY))
return -EINVAL;
@@ -2486,8 +2501,10 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
err_undo:
list_for_each_entry_from_reverse(vdev, &dev_set->device_list,
- vdev.dev_set_list)
+ vdev.dev_set_list) {
up_write(&vdev->memory_lock);
+ vfio_pci_dev_migration_reset_state(vdev);
+ }
list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list)
pm_runtime_put(&vdev->pdev->dev);
@@ -2543,8 +2560,10 @@ static void vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set)
reset_done = true;
list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) {
- if (reset_done)
+ if (reset_done) {
cur->needs_reset = false;
+ vfio_pci_dev_migration_reset_state(cur);
+ }
if (!disable_idle_d3)
pm_runtime_put(&cur->pdev->dev);
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index eb563f538dee5..36aab2df40700 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -213,6 +213,9 @@ static inline bool vfio_device_cdev_opened(struct vfio_device *device)
* @migration_get_state: Optional callback to get the migration state for
* devices that support migration. It's mandatory for
* VFIO_DEVICE_FEATURE_MIGRATION migration support.
+ * @migration_reset_state: Optional callback to reset the migration state for
+ * devices that support migration. It's mandatory for
+ * VFIO_DEVICE_FEATURE_MIGRATION migration support.
* @migration_get_data_size: Optional callback to get the estimated data
* length that will be required to complete stop copy. It's mandatory for
* VFIO_DEVICE_FEATURE_MIGRATION migration support.
@@ -223,6 +226,7 @@ struct vfio_migration_ops {
enum vfio_device_mig_state new_state);
int (*migration_get_state)(struct vfio_device *device,
enum vfio_device_mig_state *curr_state);
+ void (*migration_reset_state)(struct vfio_device *device);
int (*migration_get_data_size)(struct vfio_device *device,
unsigned long *stop_copy_length);
};
--
2.51.2
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH 1/6] vfio: Introduce .migration_reset_state() callback
2025-11-20 12:36 ` [PATCH 1/6] vfio: Introduce .migration_reset_state() callback Michał Winiarski
@ 2025-11-21 7:13 ` Tian, Kevin
2025-11-24 21:29 ` Winiarski, Michal
2025-11-21 18:16 ` Alex Williamson
1 sibling, 1 reply; 11+ messages in thread
From: Tian, Kevin @ 2025-11-21 7:13 UTC (permalink / raw)
To: Winiarski, Michal, Jason Gunthorpe, Alex Williamson,
Yishai Hadas, Longfang Liu, Shameer Kolothum, Brett Creeley,
Cabiddu, Giovanni, kvm, qat-linux, virtualization, linux-kernel
> From: Winiarski, Michal <michal.winiarski@intel.com>
> Sent: Thursday, November 20, 2025 8:37 PM
>
> Resetting the migration device state is typically delegated to PCI
> .reset_done() callback.
> With VFIO, reset is usually called under vdev->memory_lock, which causes
> lockdep to report a following circular locking dependency scenario:
>
> 0: set_device_state
> driver->state_mutex -> migf->lock
> 1: data_read
> migf->lock -> mm->mmap_lock
> 2: vfio_pin_dma
> mm->mmap_lock -> vdev->memory_lock
> 3: vfio_pci_ioctl_reset
> vdev->memory_lock -> driver->state_mutex
>
> Introduce a .migration_reset_state() callback called outside of
> vdev->memory_lock to break the dependency chain.
so it kind of unifies the deferred_reset logic cross all drivers.
sounds reasonable as nobody should expect a concrete sequence of
a reset done vs. a racing set_device_state.
>
> +static void vfio_pci_dev_migration_reset_state(struct vfio_pci_core_device
> *vdev)
> +{
> + lockdep_assert_not_held(&vdev->memory_lock);
> +
> + if (!vdev->vdev.mig_ops->migration_reset_state)
> + return;
mig_ops could be NULL.
> @@ -1230,6 +1242,8 @@ static int vfio_pci_ioctl_reset(struct
> vfio_pci_core_device *vdev,
> ret = pci_try_reset_function(vdev->pdev);
> up_write(&vdev->memory_lock);
>
> + vfio_pci_dev_migration_reset_state(vdev);
> +
only if the previous reset succeeds.
> @@ -2486,8 +2501,10 @@ static int vfio_pci_dev_set_hot_reset(struct
> vfio_device_set *dev_set,
>
> err_undo:
> list_for_each_entry_from_reverse(vdev, &dev_set->device_list,
> - vdev.dev_set_list)
> + vdev.dev_set_list) {
> up_write(&vdev->memory_lock);
> + vfio_pci_dev_migration_reset_state(vdev);
> + }
ditto
btw some reset paths are missed in drivers/vfio/pci/vfio_pci_config.c,
e.g. for vFLR emulation.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/6] vfio: Introduce .migration_reset_state() callback
2025-11-21 7:13 ` Tian, Kevin
@ 2025-11-24 21:29 ` Winiarski, Michal
0 siblings, 0 replies; 11+ messages in thread
From: Winiarski, Michal @ 2025-11-24 21:29 UTC (permalink / raw)
To: Tian, Kevin
Cc: Jason Gunthorpe, Alex Williamson, Yishai Hadas, Longfang Liu,
Shameer Kolothum, Brett Creeley, Cabiddu, Giovanni, kvm,
qat-linux, virtualization, linux-kernel
On Fri, Nov 21, 2025 at 08:13:38AM +0100, Tian, Kevin wrote:
> > From: Winiarski, Michal <michal.winiarski@intel.com>
> > Sent: Thursday, November 20, 2025 8:37 PM
> >
> > Resetting the migration device state is typically delegated to PCI
> > .reset_done() callback.
> > With VFIO, reset is usually called under vdev->memory_lock, which causes
> > lockdep to report a following circular locking dependency scenario:
> >
> > 0: set_device_state
> > driver->state_mutex -> migf->lock
> > 1: data_read
> > migf->lock -> mm->mmap_lock
> > 2: vfio_pin_dma
> > mm->mmap_lock -> vdev->memory_lock
> > 3: vfio_pci_ioctl_reset
> > vdev->memory_lock -> driver->state_mutex
> >
> > Introduce a .migration_reset_state() callback called outside of
> > vdev->memory_lock to break the dependency chain.
>
> so it kind of unifies the deferred_reset logic cross all drivers.
>
> sounds reasonable as nobody should expect a concrete sequence of
> a reset done vs. a racing set_device_state.
>
> >
> > +static void vfio_pci_dev_migration_reset_state(struct vfio_pci_core_device
> > *vdev)
> > +{
> > + lockdep_assert_not_held(&vdev->memory_lock);
> > +
> > + if (!vdev->vdev.mig_ops->migration_reset_state)
> > + return;
>
> mig_ops could be NULL.
I'll replace it with pure mig_ops check, as the migration_reset_state()
is not optional.
>
> > @@ -1230,6 +1242,8 @@ static int vfio_pci_ioctl_reset(struct
> > vfio_pci_core_device *vdev,
> > ret = pci_try_reset_function(vdev->pdev);
> > up_write(&vdev->memory_lock);
> >
> > + vfio_pci_dev_migration_reset_state(vdev);
> > +
>
> only if the previous reset succeeds.
Ok.
>
> > @@ -2486,8 +2501,10 @@ static int vfio_pci_dev_set_hot_reset(struct
> > vfio_device_set *dev_set,
> >
> > err_undo:
> > list_for_each_entry_from_reverse(vdev, &dev_set->device_list,
> > - vdev.dev_set_list)
> > + vdev.dev_set_list) {
> > up_write(&vdev->memory_lock);
> > + vfio_pci_dev_migration_reset_state(vdev);
> > + }
>
> ditto
>
> btw some reset paths are missed in drivers/vfio/pci/vfio_pci_config.c,
> e.g. for vFLR emulation.
>
I'll add the missing paths in v2.
Thanks,
-Michał
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/6] vfio: Introduce .migration_reset_state() callback
2025-11-20 12:36 ` [PATCH 1/6] vfio: Introduce .migration_reset_state() callback Michał Winiarski
2025-11-21 7:13 ` Tian, Kevin
@ 2025-11-21 18:16 ` Alex Williamson
2025-11-24 21:44 ` Michał Winiarski
1 sibling, 1 reply; 11+ messages in thread
From: Alex Williamson @ 2025-11-21 18:16 UTC (permalink / raw)
To: Michał Winiarski
Cc: Jason Gunthorpe, Kevin Tian, Yishai Hadas, Longfang Liu,
Shameer Kolothum, Brett Creeley, Giovanni Cabiddu, kvm,
qat-linux, virtualization, linux-kernel
On Thu, 20 Nov 2025 13:36:42 +0100
Michał Winiarski <michal.winiarski@intel.com> wrote:
> Resetting the migration device state is typically delegated to PCI
> .reset_done() callback.
> With VFIO, reset is usually called under vdev->memory_lock, which causes
> lockdep to report a following circular locking dependency scenario:
>
> 0: set_device_state
> driver->state_mutex -> migf->lock
> 1: data_read
> migf->lock -> mm->mmap_lock
> 2: vfio_pin_dma
> mm->mmap_lock -> vdev->memory_lock
> 3: vfio_pci_ioctl_reset
> vdev->memory_lock -> driver->state_mutex
>
> Introduce a .migration_reset_state() callback called outside of
> vdev->memory_lock to break the dependency chain.
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/vfio/pci/vfio_pci_core.c | 25 ++++++++++++++++++++++---
> include/linux/vfio.h | 4 ++++
> 2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 7dcf5439dedc9..d919636558ec8 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -553,6 +553,16 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
> }
> EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
>
> +static void vfio_pci_dev_migration_reset_state(struct vfio_pci_core_device *vdev)
> +{
> + lockdep_assert_not_held(&vdev->memory_lock);
> +
> + if (!vdev->vdev.mig_ops->migration_reset_state)
mig_ops itself is generally NULL.
> + return;
> +
> + vdev->vdev.mig_ops->migration_reset_state(&vdev->vdev);
> +}
> +
> void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> {
> struct pci_dev *pdev = vdev->pdev;
> @@ -662,8 +672,10 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> * overwrite the previously restored configuration information.
> */
> if (vdev->reset_works && pci_dev_trylock(pdev)) {
> - if (!__pci_reset_function_locked(pdev))
> + if (!__pci_reset_function_locked(pdev)) {
> vdev->needs_reset = false;
> + vfio_pci_dev_migration_reset_state(vdev);
> + }
> pci_dev_unlock(pdev);
> }
>
> @@ -1230,6 +1242,8 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
> ret = pci_try_reset_function(vdev->pdev);
> up_write(&vdev->memory_lock);
>
> + vfio_pci_dev_migration_reset_state(vdev);
> +
> return ret;
> }
>
> @@ -2129,6 +2143,7 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
> if (vdev->vdev.mig_ops) {
> if (!(vdev->vdev.mig_ops->migration_get_state &&
> vdev->vdev.mig_ops->migration_set_state &&
> + vdev->vdev.mig_ops->migration_reset_state &&
For bisection purposes it would be better to enforce this after all the
drivers are converted.
> vdev->vdev.mig_ops->migration_get_data_size) ||
> !(vdev->vdev.migration_flags & VFIO_MIGRATION_STOP_COPY))
> return -EINVAL;
> @@ -2486,8 +2501,10 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
>
> err_undo:
> list_for_each_entry_from_reverse(vdev, &dev_set->device_list,
> - vdev.dev_set_list)
> + vdev.dev_set_list) {
> up_write(&vdev->memory_lock);
> + vfio_pci_dev_migration_reset_state(vdev);
> + }
>
> list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list)
> pm_runtime_put(&vdev->pdev->dev);
> @@ -2543,8 +2560,10 @@ static void vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set)
> reset_done = true;
>
> list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) {
> - if (reset_done)
> + if (reset_done) {
> cur->needs_reset = false;
> + vfio_pci_dev_migration_reset_state(cur);
> + }
This and the core_disable path above are only called in the
close/open-error path. Do we really need this behavior there? We
might need separate reconciliation vs .reset_done for these.
As Kevin also noted, we're missing the non-ioctl reset paths. This
approach seems a bit error prone. I wonder if instead we need a
counterpart of vfio_pci_zap_and_down_write_memory_lock(), ie.
vfio_pci_up_write_memory_lock_from_reset(). An equal mouthful, but
scopes the problem to be more manageable at memory_lock release.
Thanks,
Alex
>
> if (!disable_idle_d3)
> pm_runtime_put(&cur->pdev->dev);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index eb563f538dee5..36aab2df40700 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -213,6 +213,9 @@ static inline bool vfio_device_cdev_opened(struct vfio_device *device)
> * @migration_get_state: Optional callback to get the migration state for
> * devices that support migration. It's mandatory for
> * VFIO_DEVICE_FEATURE_MIGRATION migration support.
> + * @migration_reset_state: Optional callback to reset the migration state for
> + * devices that support migration. It's mandatory for
> + * VFIO_DEVICE_FEATURE_MIGRATION migration support.
> * @migration_get_data_size: Optional callback to get the estimated data
> * length that will be required to complete stop copy. It's mandatory for
> * VFIO_DEVICE_FEATURE_MIGRATION migration support.
> @@ -223,6 +226,7 @@ struct vfio_migration_ops {
> enum vfio_device_mig_state new_state);
> int (*migration_get_state)(struct vfio_device *device,
> enum vfio_device_mig_state *curr_state);
> + void (*migration_reset_state)(struct vfio_device *device);
> int (*migration_get_data_size)(struct vfio_device *device,
> unsigned long *stop_copy_length);
> };
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/6] vfio: Introduce .migration_reset_state() callback
2025-11-21 18:16 ` Alex Williamson
@ 2025-11-24 21:44 ` Michał Winiarski
0 siblings, 0 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-24 21:44 UTC (permalink / raw)
To: Alex Williamson
Cc: Jason Gunthorpe, Kevin Tian, Yishai Hadas, Longfang Liu,
Shameer Kolothum, Brett Creeley, Giovanni Cabiddu, kvm,
qat-linux, virtualization, linux-kernel
On Fri, Nov 21, 2025 at 11:16:03AM -0700, Alex Williamson wrote:
> On Thu, 20 Nov 2025 13:36:42 +0100
> Michał Winiarski <michal.winiarski@intel.com> wrote:
>
> > Resetting the migration device state is typically delegated to PCI
> > .reset_done() callback.
> > With VFIO, reset is usually called under vdev->memory_lock, which causes
> > lockdep to report a following circular locking dependency scenario:
> >
> > 0: set_device_state
> > driver->state_mutex -> migf->lock
> > 1: data_read
> > migf->lock -> mm->mmap_lock
> > 2: vfio_pin_dma
> > mm->mmap_lock -> vdev->memory_lock
> > 3: vfio_pci_ioctl_reset
> > vdev->memory_lock -> driver->state_mutex
> >
> > Introduce a .migration_reset_state() callback called outside of
> > vdev->memory_lock to break the dependency chain.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> > drivers/vfio/pci/vfio_pci_core.c | 25 ++++++++++++++++++++++---
> > include/linux/vfio.h | 4 ++++
> > 2 files changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > index 7dcf5439dedc9..d919636558ec8 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -553,6 +553,16 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
> > }
> > EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
> >
> > +static void vfio_pci_dev_migration_reset_state(struct vfio_pci_core_device *vdev)
> > +{
> > + lockdep_assert_not_held(&vdev->memory_lock);
> > +
> > + if (!vdev->vdev.mig_ops->migration_reset_state)
>
> mig_ops itself is generally NULL.
Yeah, I clearly didn't test it with plain vfio_pci.
I'll fix it in v2.
>
> > + return;
> > +
> > + vdev->vdev.mig_ops->migration_reset_state(&vdev->vdev);
> > +}
> > +
> > void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> > {
> > struct pci_dev *pdev = vdev->pdev;
> > @@ -662,8 +672,10 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> > * overwrite the previously restored configuration information.
> > */
> > if (vdev->reset_works && pci_dev_trylock(pdev)) {
> > - if (!__pci_reset_function_locked(pdev))
> > + if (!__pci_reset_function_locked(pdev)) {
> > vdev->needs_reset = false;
> > + vfio_pci_dev_migration_reset_state(vdev);
> > + }
> > pci_dev_unlock(pdev);
> > }
> >
> > @@ -1230,6 +1242,8 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
> > ret = pci_try_reset_function(vdev->pdev);
> > up_write(&vdev->memory_lock);
> >
> > + vfio_pci_dev_migration_reset_state(vdev);
> > +
> > return ret;
> > }
> >
> > @@ -2129,6 +2143,7 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
> > if (vdev->vdev.mig_ops) {
> > if (!(vdev->vdev.mig_ops->migration_get_state &&
> > vdev->vdev.mig_ops->migration_set_state &&
> > + vdev->vdev.mig_ops->migration_reset_state &&
>
> For bisection purposes it would be better to enforce this after all the
> drivers are converted.
Makes sense. I'll split this patch.
>
> > vdev->vdev.mig_ops->migration_get_data_size) ||
> > !(vdev->vdev.migration_flags & VFIO_MIGRATION_STOP_COPY))
> > return -EINVAL;
> > @@ -2486,8 +2501,10 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
> >
> > err_undo:
> > list_for_each_entry_from_reverse(vdev, &dev_set->device_list,
> > - vdev.dev_set_list)
> > + vdev.dev_set_list) {
> > up_write(&vdev->memory_lock);
> > + vfio_pci_dev_migration_reset_state(vdev);
> > + }
> >
> > list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list)
> > pm_runtime_put(&vdev->pdev->dev);
> > @@ -2543,8 +2560,10 @@ static void vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set)
> > reset_done = true;
> >
> > list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) {
> > - if (reset_done)
> > + if (reset_done) {
> > cur->needs_reset = false;
> > + vfio_pci_dev_migration_reset_state(cur);
> > + }
>
> This and the core_disable path above are only called in the
> close/open-error path. Do we really need this behavior there? We
> might need separate reconciliation vs .reset_done for these.
Some drivers do the migration state reset explicitly in their, some rely
on reset (and .reset_done) being invoked.
We can remove it from here if we just call migration_reset_state
consistently in each driver.
>
> As Kevin also noted, we're missing the non-ioctl reset paths. This
> approach seems a bit error prone. I wonder if instead we need a
> counterpart of vfio_pci_zap_and_down_write_memory_lock(), ie.
> vfio_pci_up_write_memory_lock_from_reset(). An equal mouthful, but
> scopes the problem to be more manageable at memory_lock release.
And it also needs to be called conditionally on certain paths.
Which means that it will take an extra argument (whether prior reset was
successful).
I'll try this approach in v2.
Thanks,
-Michał
> Thanks,
>
> Alex
>
>
> >
> > if (!disable_idle_d3)
> > pm_runtime_put(&cur->pdev->dev);
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index eb563f538dee5..36aab2df40700 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -213,6 +213,9 @@ static inline bool vfio_device_cdev_opened(struct vfio_device *device)
> > * @migration_get_state: Optional callback to get the migration state for
> > * devices that support migration. It's mandatory for
> > * VFIO_DEVICE_FEATURE_MIGRATION migration support.
> > + * @migration_reset_state: Optional callback to reset the migration state for
> > + * devices that support migration. It's mandatory for
> > + * VFIO_DEVICE_FEATURE_MIGRATION migration support.
> > * @migration_get_data_size: Optional callback to get the estimated data
> > * length that will be required to complete stop copy. It's mandatory for
> > * VFIO_DEVICE_FEATURE_MIGRATION migration support.
> > @@ -223,6 +226,7 @@ struct vfio_migration_ops {
> > enum vfio_device_mig_state new_state);
> > int (*migration_get_state)(struct vfio_device *device,
> > enum vfio_device_mig_state *curr_state);
> > + void (*migration_reset_state)(struct vfio_device *device);
> > int (*migration_get_data_size)(struct vfio_device *device,
> > unsigned long *stop_copy_length);
> > };
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/6] hisi_acc_vfio_pci: Use .migration_reset_state() callback
2025-11-20 12:36 [PATCH 0/6] vfio: Reset migration device state using dedicated callback Michał Winiarski
2025-11-20 12:36 ` [PATCH 1/6] vfio: Introduce .migration_reset_state() callback Michał Winiarski
@ 2025-11-20 12:36 ` Michał Winiarski
2025-11-20 12:36 ` [PATCH 3/6] vfio/pds: " Michał Winiarski
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-20 12:36 UTC (permalink / raw)
To: Jason Gunthorpe, Alex Williamson, Kevin Tian, Yishai Hadas,
Longfang Liu, Shameer Kolothum, Brett Creeley, Giovanni Cabiddu,
kvm, qat-linux, virtualization, linux-kernel
Cc: Michał Winiarski
Move the migration device state reset code from .reset_done() to
dedicated callback.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index fde33f54e99ec..eafdf62ee29ef 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -1169,9 +1169,10 @@ hisi_acc_vfio_pci_get_device_state(struct vfio_device *vdev,
return 0;
}
-static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev)
+static void
+hisi_acc_vfio_pci_reset_device_state(struct vfio_device *vdev)
{
- struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
+ struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(vdev);
if (hisi_acc_vdev->core_device.vdev.migration_flags !=
VFIO_MIGRATION_STOP_COPY)
@@ -1529,6 +1530,7 @@ static void hisi_acc_vfio_pci_close_device(struct vfio_device *core_vdev)
static const struct vfio_migration_ops hisi_acc_vfio_pci_migrn_state_ops = {
.migration_set_state = hisi_acc_vfio_pci_set_device_state,
.migration_get_state = hisi_acc_vfio_pci_get_device_state,
+ .migration_reset_state = hisi_acc_vfio_pci_reset_device_state,
.migration_get_data_size = hisi_acc_vfio_pci_get_data_size,
};
@@ -1689,7 +1691,6 @@ static const struct pci_device_id hisi_acc_vfio_pci_table[] = {
MODULE_DEVICE_TABLE(pci, hisi_acc_vfio_pci_table);
static const struct pci_error_handlers hisi_acc_vf_err_handlers = {
- .reset_done = hisi_acc_vf_pci_aer_reset_done,
.error_detected = vfio_pci_core_aer_err_detected,
};
--
2.51.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 3/6] vfio/pds: Use .migration_reset_state() callback
2025-11-20 12:36 [PATCH 0/6] vfio: Reset migration device state using dedicated callback Michał Winiarski
2025-11-20 12:36 ` [PATCH 1/6] vfio: Introduce .migration_reset_state() callback Michał Winiarski
2025-11-20 12:36 ` [PATCH 2/6] hisi_acc_vfio_pci: Use " Michał Winiarski
@ 2025-11-20 12:36 ` Michał Winiarski
2025-11-20 12:36 ` [PATCH 4/6] vfio/qat: " Michał Winiarski
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-20 12:36 UTC (permalink / raw)
To: Jason Gunthorpe, Alex Williamson, Kevin Tian, Yishai Hadas,
Longfang Liu, Shameer Kolothum, Brett Creeley, Giovanni Cabiddu,
kvm, qat-linux, virtualization, linux-kernel
Cc: Michał Winiarski
Move the migration device state reset code from .reset_done() to
dedicated callback.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/vfio/pci/pds/pci_drv.c | 10 ----------
drivers/vfio/pci/pds/vfio_dev.c | 12 ++++++++++++
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/vfio/pci/pds/pci_drv.c b/drivers/vfio/pci/pds/pci_drv.c
index 4923f18231263..4cf3d2e3767a6 100644
--- a/drivers/vfio/pci/pds/pci_drv.c
+++ b/drivers/vfio/pci/pds/pci_drv.c
@@ -162,17 +162,7 @@ static const struct pci_device_id pds_vfio_pci_table[] = {
};
MODULE_DEVICE_TABLE(pci, pds_vfio_pci_table);
-static void pds_vfio_pci_aer_reset_done(struct pci_dev *pdev)
-{
- struct pds_vfio_pci_device *pds_vfio = pds_vfio_pci_drvdata(pdev);
-
- mutex_lock(&pds_vfio->state_mutex);
- pds_vfio_reset(pds_vfio, VFIO_DEVICE_STATE_RUNNING);
- mutex_unlock(&pds_vfio->state_mutex);
-}
-
static const struct pci_error_handlers pds_vfio_pci_err_handlers = {
- .reset_done = pds_vfio_pci_aer_reset_done,
.error_detected = vfio_pci_core_aer_err_detected,
};
diff --git a/drivers/vfio/pci/pds/vfio_dev.c b/drivers/vfio/pci/pds/vfio_dev.c
index f3ccb0008f675..6b29641b5819b 100644
--- a/drivers/vfio/pci/pds/vfio_dev.c
+++ b/drivers/vfio/pci/pds/vfio_dev.c
@@ -101,9 +101,21 @@ static int pds_vfio_get_device_state_size(struct vfio_device *vdev,
return 0;
}
+static void pds_vfio_reset_device_state(struct vfio_device *vdev)
+{
+ struct pds_vfio_pci_device *pds_vfio =
+ container_of(vdev, struct pds_vfio_pci_device,
+ vfio_coredev.vdev);
+
+ mutex_lock(&pds_vfio->state_mutex);
+ pds_vfio_reset(pds_vfio, VFIO_DEVICE_STATE_RUNNING);
+ mutex_unlock(&pds_vfio->state_mutex);
+}
+
static const struct vfio_migration_ops pds_vfio_lm_ops = {
.migration_set_state = pds_vfio_set_device_state,
.migration_get_state = pds_vfio_get_device_state,
+ .migration_reset_state = pds_vfio_reset_device_state,
.migration_get_data_size = pds_vfio_get_device_state_size
};
--
2.51.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 4/6] vfio/qat: Use .migration_reset_state() callback
2025-11-20 12:36 [PATCH 0/6] vfio: Reset migration device state using dedicated callback Michał Winiarski
` (2 preceding siblings ...)
2025-11-20 12:36 ` [PATCH 3/6] vfio/pds: " Michał Winiarski
@ 2025-11-20 12:36 ` Michał Winiarski
2025-11-20 12:36 ` [PATCH 5/6] vfio/mlx5: " Michał Winiarski
2025-11-20 12:36 ` [PATCH 6/6] vfio/virtio: " Michał Winiarski
5 siblings, 0 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-20 12:36 UTC (permalink / raw)
To: Jason Gunthorpe, Alex Williamson, Kevin Tian, Yishai Hadas,
Longfang Liu, Shameer Kolothum, Brett Creeley, Giovanni Cabiddu,
kvm, qat-linux, virtualization, linux-kernel
Cc: Michał Winiarski
Move the migration device state reset code from .reset_done() to
dedicated callback.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/vfio/pci/qat/main.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/vfio/pci/qat/main.c b/drivers/vfio/pci/qat/main.c
index a19b68043eb2e..fe65301a4cdc8 100644
--- a/drivers/vfio/pci/qat/main.c
+++ b/drivers/vfio/pci/qat/main.c
@@ -474,13 +474,6 @@ static struct file *qat_vf_pci_step_device_state(struct qat_vf_core_device *qat_
return ERR_PTR(-EINVAL);
}
-static void qat_vf_reset_done(struct qat_vf_core_device *qat_vdev)
-{
- qat_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
- qat_vfmig_reset(qat_vdev->mdev);
- qat_vf_disable_fds(qat_vdev);
-}
-
static struct file *qat_vf_pci_set_device_state(struct vfio_device *vdev,
enum vfio_device_mig_state new_state)
{
@@ -526,6 +519,21 @@ static int qat_vf_pci_get_device_state(struct vfio_device *vdev,
return 0;
}
+static void qat_vf_pci_reset_device_state(struct vfio_device *vdev)
+{
+ struct qat_vf_core_device *qat_vdev = container_of(vdev,
+ struct qat_vf_core_device, core_device.vdev);
+
+ if (!qat_vdev->mdev)
+ return;
+
+ mutex_lock(&qat_vdev->state_mutex);
+ qat_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
+ qat_vfmig_reset(qat_vdev->mdev);
+ qat_vf_disable_fds(qat_vdev);
+ mutex_unlock(&qat_vdev->state_mutex);
+}
+
static int qat_vf_pci_get_data_size(struct vfio_device *vdev,
unsigned long *stop_copy_length)
{
@@ -542,6 +550,7 @@ static int qat_vf_pci_get_data_size(struct vfio_device *vdev,
static const struct vfio_migration_ops qat_vf_pci_mig_ops = {
.migration_set_state = qat_vf_pci_set_device_state,
.migration_get_state = qat_vf_pci_get_device_state,
+ .migration_reset_state = qat_vf_pci_reset_device_state,
.migration_get_data_size = qat_vf_pci_get_data_size,
};
@@ -628,18 +637,6 @@ static struct qat_vf_core_device *qat_vf_drvdata(struct pci_dev *pdev)
return container_of(core_device, struct qat_vf_core_device, core_device);
}
-static void qat_vf_pci_aer_reset_done(struct pci_dev *pdev)
-{
- struct qat_vf_core_device *qat_vdev = qat_vf_drvdata(pdev);
-
- if (!qat_vdev->mdev)
- return;
-
- mutex_lock(&qat_vdev->state_mutex);
- qat_vf_reset_done(qat_vdev);
- mutex_unlock(&qat_vdev->state_mutex);
-}
-
static int
qat_vf_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
@@ -683,7 +680,6 @@ static const struct pci_device_id qat_vf_vfio_pci_table[] = {
MODULE_DEVICE_TABLE(pci, qat_vf_vfio_pci_table);
static const struct pci_error_handlers qat_vf_err_handlers = {
- .reset_done = qat_vf_pci_aer_reset_done,
.error_detected = vfio_pci_core_aer_err_detected,
};
--
2.51.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 5/6] vfio/mlx5: Use .migration_reset_state() callback
2025-11-20 12:36 [PATCH 0/6] vfio: Reset migration device state using dedicated callback Michał Winiarski
` (3 preceding siblings ...)
2025-11-20 12:36 ` [PATCH 4/6] vfio/qat: " Michał Winiarski
@ 2025-11-20 12:36 ` Michał Winiarski
2025-11-20 12:36 ` [PATCH 6/6] vfio/virtio: " Michał Winiarski
5 siblings, 0 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-20 12:36 UTC (permalink / raw)
To: Jason Gunthorpe, Alex Williamson, Kevin Tian, Yishai Hadas,
Longfang Liu, Shameer Kolothum, Brett Creeley, Giovanni Cabiddu,
kvm, qat-linux, virtualization, linux-kernel
Cc: Michał Winiarski
Move the migration device state reset code from .reset_done() to
dedicated callback.
Remove the deferred reset mechanism, as it's no longer needed.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/vfio/pci/mlx5/cmd.c | 15 +++++----
drivers/vfio/pci/mlx5/cmd.h | 3 --
drivers/vfio/pci/mlx5/main.c | 59 +++++++++---------------------------
3 files changed, 22 insertions(+), 55 deletions(-)
diff --git a/drivers/vfio/pci/mlx5/cmd.c b/drivers/vfio/pci/mlx5/cmd.c
index a92b095b90f6a..de6d786ce7ed1 100644
--- a/drivers/vfio/pci/mlx5/cmd.c
+++ b/drivers/vfio/pci/mlx5/cmd.c
@@ -178,13 +178,13 @@ static int mlx5fv_vf_event(struct notifier_block *nb,
case MLX5_PF_NOTIFY_ENABLE_VF:
mutex_lock(&mvdev->state_mutex);
mvdev->mdev_detach = false;
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
break;
case MLX5_PF_NOTIFY_DISABLE_VF:
mlx5vf_cmd_close_migratable(mvdev);
mutex_lock(&mvdev->state_mutex);
mvdev->mdev_detach = true;
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
break;
default:
break;
@@ -203,7 +203,7 @@ void mlx5vf_cmd_close_migratable(struct mlx5vf_pci_core_device *mvdev)
mutex_lock(&mvdev->state_mutex);
mlx5vf_disable_fds(mvdev, NULL);
_mlx5vf_free_page_tracker_resources(mvdev);
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
}
void mlx5vf_cmd_remove_migratable(struct mlx5vf_pci_core_device *mvdev)
@@ -254,7 +254,6 @@ void mlx5vf_cmd_set_migratable(struct mlx5vf_pci_core_device *mvdev,
goto end;
mutex_init(&mvdev->state_mutex);
- spin_lock_init(&mvdev->reset_lock);
mvdev->nb.notifier_call = mlx5fv_vf_event;
ret = mlx5_sriov_blocking_notifier_register(mvdev->mdev, mvdev->vf_id,
&mvdev->nb);
@@ -1487,7 +1486,7 @@ int mlx5vf_stop_page_tracker(struct vfio_device *vdev)
_mlx5vf_free_page_tracker_resources(mvdev);
mvdev->log_active = false;
end:
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return 0;
}
@@ -1589,7 +1588,7 @@ int mlx5vf_start_page_tracker(struct vfio_device *vdev,
mlx5_eq_notifier_register(mdev, &tracker->nb);
*page_size = host_qp->tracked_page_size;
mvdev->log_active = true;
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return 0;
err_activate:
@@ -1605,7 +1604,7 @@ int mlx5vf_start_page_tracker(struct vfio_device *vdev,
err_uar:
mlx5_put_uars_page(mdev, tracker->uar);
end:
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return err;
}
@@ -1787,6 +1786,6 @@ int mlx5vf_tracker_read_and_clear(struct vfio_device *vdev, unsigned long iova,
if (tracker->is_err)
err = -EIO;
end:
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return err;
}
diff --git a/drivers/vfio/pci/mlx5/cmd.h b/drivers/vfio/pci/mlx5/cmd.h
index d7821b5ca7729..e36df1052cf5e 100644
--- a/drivers/vfio/pci/mlx5/cmd.h
+++ b/drivers/vfio/pci/mlx5/cmd.h
@@ -170,7 +170,6 @@ struct mlx5vf_pci_core_device {
int vf_id;
u16 vhca_id;
u8 migrate_cap:1;
- u8 deferred_reset:1;
u8 mdev_detach:1;
u8 log_active:1;
u8 chunk_mode:1;
@@ -178,8 +177,6 @@ struct mlx5vf_pci_core_device {
/* protect migration state */
struct mutex state_mutex;
enum vfio_device_mig_state mig_state;
- /* protect the reset_done flow */
- spinlock_t reset_lock;
struct mlx5_vf_migration_file *resuming_migf;
struct mlx5_vf_migration_file *saving_migf;
struct mlx5_vhca_page_tracker tracker;
diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c
index 7ec47e736a8e5..ddc6fa346f37c 100644
--- a/drivers/vfio/pci/mlx5/main.c
+++ b/drivers/vfio/pci/mlx5/main.c
@@ -325,7 +325,7 @@ static void mlx5vf_mig_file_save_work(struct work_struct *_work)
err:
mlx5vf_mark_err(migf);
end:
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
fput(migf->filp);
}
@@ -544,7 +544,7 @@ static long mlx5vf_precopy_ioctl(struct file *filp, unsigned int cmd,
}
done:
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
if (copy_to_user((void __user *)arg, &info, minsz))
return -EFAULT;
return 0;
@@ -552,7 +552,7 @@ static long mlx5vf_precopy_ioctl(struct file *filp, unsigned int cmd,
err_migf_unlock:
mutex_unlock(&migf->lock);
err_state_unlock:
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return ret;
}
@@ -972,7 +972,7 @@ static ssize_t mlx5vf_resume_write(struct file *filp, const char __user *buf,
if (ret)
migf->state = MLX5_MIGF_STATE_ERROR;
mutex_unlock(&migf->lock);
- mlx5vf_state_mutex_unlock(migf->mvdev);
+ mutex_unlock(&migf->mvdev->state_mutex);
return ret ? ret : done;
}
@@ -1191,25 +1191,6 @@ mlx5vf_pci_step_device_state_locked(struct mlx5vf_pci_core_device *mvdev,
return ERR_PTR(-EINVAL);
}
-/*
- * This function is called in all state_mutex unlock cases to
- * handle a 'deferred_reset' if exists.
- */
-void mlx5vf_state_mutex_unlock(struct mlx5vf_pci_core_device *mvdev)
-{
-again:
- spin_lock(&mvdev->reset_lock);
- if (mvdev->deferred_reset) {
- mvdev->deferred_reset = false;
- spin_unlock(&mvdev->reset_lock);
- mvdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
- mlx5vf_disable_fds(mvdev, NULL);
- goto again;
- }
- mutex_unlock(&mvdev->state_mutex);
- spin_unlock(&mvdev->reset_lock);
-}
-
static struct file *
mlx5vf_pci_set_device_state(struct vfio_device *vdev,
enum vfio_device_mig_state new_state)
@@ -1238,7 +1219,7 @@ mlx5vf_pci_set_device_state(struct vfio_device *vdev,
break;
}
}
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return res;
}
@@ -1256,7 +1237,7 @@ static int mlx5vf_pci_get_data_size(struct vfio_device *vdev,
&total_size, 0);
if (!ret)
*stop_copy_length = total_size;
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return ret;
}
@@ -1268,32 +1249,22 @@ static int mlx5vf_pci_get_device_state(struct vfio_device *vdev,
mutex_lock(&mvdev->state_mutex);
*curr_state = mvdev->mig_state;
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_unlock(&mvdev->state_mutex);
return 0;
}
-static void mlx5vf_pci_aer_reset_done(struct pci_dev *pdev)
+static void mlx5vf_pci_reset_device_state(struct vfio_device *vdev)
{
- struct mlx5vf_pci_core_device *mvdev = mlx5vf_drvdata(pdev);
+ struct mlx5vf_pci_core_device *mvdev = container_of(
+ vdev, struct mlx5vf_pci_core_device, core_device.vdev);
if (!mvdev->migrate_cap)
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(&mvdev->reset_lock);
- mvdev->deferred_reset = true;
- if (!mutex_trylock(&mvdev->state_mutex)) {
- spin_unlock(&mvdev->reset_lock);
- return;
- }
- spin_unlock(&mvdev->reset_lock);
- mlx5vf_state_mutex_unlock(mvdev);
+ mutex_lock(&mvdev->state_mutex);
+ mvdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
+ mlx5vf_disable_fds(mvdev, NULL);
+ mutex_unlock(&mvdev->state_mutex);
}
static int mlx5vf_pci_open_device(struct vfio_device *core_vdev)
@@ -1325,6 +1296,7 @@ static void mlx5vf_pci_close_device(struct vfio_device *core_vdev)
static const struct vfio_migration_ops mlx5vf_pci_mig_ops = {
.migration_set_state = mlx5vf_pci_set_device_state,
.migration_get_state = mlx5vf_pci_get_device_state,
+ .migration_reset_state = mlx5vf_pci_reset_device_state,
.migration_get_data_size = mlx5vf_pci_get_data_size,
};
@@ -1417,7 +1389,6 @@ static const struct pci_device_id mlx5vf_pci_table[] = {
MODULE_DEVICE_TABLE(pci, mlx5vf_pci_table);
static const struct pci_error_handlers mlx5vf_err_handlers = {
- .reset_done = mlx5vf_pci_aer_reset_done,
.error_detected = vfio_pci_core_aer_err_detected,
};
--
2.51.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 6/6] vfio/virtio: Use .migration_reset_state() callback
2025-11-20 12:36 [PATCH 0/6] vfio: Reset migration device state using dedicated callback Michał Winiarski
` (4 preceding siblings ...)
2025-11-20 12:36 ` [PATCH 5/6] vfio/mlx5: " Michał Winiarski
@ 2025-11-20 12:36 ` Michał Winiarski
5 siblings, 0 replies; 11+ messages in thread
From: Michał Winiarski @ 2025-11-20 12:36 UTC (permalink / raw)
To: Jason Gunthorpe, Alex Williamson, Kevin Tian, Yishai Hadas,
Longfang Liu, Shameer Kolothum, Brett Creeley, Giovanni Cabiddu,
kvm, qat-linux, virtualization, linux-kernel
Cc: Michał Winiarski
Move the migration device state reset code from .reset_done() to
dedicated callback.
Remove the deferred reset mechanism, as it's no longer needed.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/vfio/pci/virtio/common.h | 3 --
drivers/vfio/pci/virtio/main.c | 1 -
drivers/vfio/pci/virtio/migrate.c | 71 +++++++++----------------------
3 files changed, 21 insertions(+), 54 deletions(-)
diff --git a/drivers/vfio/pci/virtio/common.h b/drivers/vfio/pci/virtio/common.h
index c7d7e27af386e..cb27d3d2d3bb9 100644
--- a/drivers/vfio/pci/virtio/common.h
+++ b/drivers/vfio/pci/virtio/common.h
@@ -92,12 +92,9 @@ struct virtiovf_pci_core_device {
/* LM related */
u8 migrate_cap:1;
- u8 deferred_reset:1;
/* protect migration state */
struct mutex state_mutex;
enum vfio_device_mig_state mig_state;
- /* protect the reset_done flow */
- spinlock_t reset_lock;
struct virtiovf_migration_file *resuming_migf;
struct virtiovf_migration_file *saving_migf;
};
diff --git a/drivers/vfio/pci/virtio/main.c b/drivers/vfio/pci/virtio/main.c
index 8084f3e36a9f7..b80cb740f9a5d 100644
--- a/drivers/vfio/pci/virtio/main.c
+++ b/drivers/vfio/pci/virtio/main.c
@@ -203,7 +203,6 @@ static void virtiovf_pci_aer_reset_done(struct pci_dev *pdev)
#ifdef CONFIG_VIRTIO_VFIO_PCI_ADMIN_LEGACY
virtiovf_legacy_io_reset_done(pdev);
#endif
- virtiovf_migration_reset_done(pdev);
}
static const struct pci_error_handlers virtiovf_err_handlers = {
diff --git a/drivers/vfio/pci/virtio/migrate.c b/drivers/vfio/pci/virtio/migrate.c
index 7dd0ac866461d..5c7f9091d84e8 100644
--- a/drivers/vfio/pci/virtio/migrate.c
+++ b/drivers/vfio/pci/virtio/migrate.c
@@ -247,49 +247,6 @@ static void virtiovf_disable_fds(struct virtiovf_pci_core_device *virtvdev)
}
}
-/*
- * This function is called in all state_mutex unlock cases to
- * handle a 'deferred_reset' if exists.
- */
-static void virtiovf_state_mutex_unlock(struct virtiovf_pci_core_device *virtvdev)
-{
-again:
- spin_lock(&virtvdev->reset_lock);
- if (virtvdev->deferred_reset) {
- virtvdev->deferred_reset = false;
- spin_unlock(&virtvdev->reset_lock);
- virtvdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
- virtiovf_disable_fds(virtvdev);
- goto again;
- }
- mutex_unlock(&virtvdev->state_mutex);
- spin_unlock(&virtvdev->reset_lock);
-}
-
-void virtiovf_migration_reset_done(struct pci_dev *pdev)
-{
- struct virtiovf_pci_core_device *virtvdev = dev_get_drvdata(&pdev->dev);
-
- if (!virtvdev->migrate_cap)
- 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(&virtvdev->reset_lock);
- virtvdev->deferred_reset = true;
- if (!mutex_trylock(&virtvdev->state_mutex)) {
- spin_unlock(&virtvdev->reset_lock);
- return;
- }
- spin_unlock(&virtvdev->reset_lock);
- virtiovf_state_mutex_unlock(virtvdev);
-}
-
static int virtiovf_release_file(struct inode *inode, struct file *filp)
{
struct virtiovf_migration_file *migf = filp->private_data;
@@ -513,7 +470,7 @@ static long virtiovf_precopy_ioctl(struct file *filp, unsigned int cmd,
goto err_state_unlock;
done:
- virtiovf_state_mutex_unlock(virtvdev);
+ mutex_unlock(&virtvdev->state_mutex);
if (copy_to_user((void __user *)arg, &info, minsz))
return -EFAULT;
return 0;
@@ -521,7 +478,7 @@ static long virtiovf_precopy_ioctl(struct file *filp, unsigned int cmd,
err_migf_unlock:
mutex_unlock(&migf->lock);
err_state_unlock:
- virtiovf_state_mutex_unlock(virtvdev);
+ mutex_unlock(&virtvdev->state_mutex);
return ret;
}
@@ -1048,7 +1005,7 @@ static ssize_t virtiovf_resume_write(struct file *filp, const char __user *buf,
if (ret)
migf->state = VIRTIOVF_MIGF_STATE_ERROR;
mutex_unlock(&migf->lock);
- virtiovf_state_mutex_unlock(migf->virtvdev);
+ mutex_unlock(&migf->virtvdev->state_mutex);
return ret ? ret : done;
}
@@ -1245,7 +1202,7 @@ virtiovf_pci_set_device_state(struct vfio_device *vdev,
break;
}
}
- virtiovf_state_mutex_unlock(virtvdev);
+ mutex_unlock(&virtvdev->state_mutex);
return res;
}
@@ -1257,10 +1214,24 @@ static int virtiovf_pci_get_device_state(struct vfio_device *vdev,
mutex_lock(&virtvdev->state_mutex);
*curr_state = virtvdev->mig_state;
- virtiovf_state_mutex_unlock(virtvdev);
+ mutex_unlock(&virtvdev->state_mutex);
return 0;
}
+static void virtiovf_pci_reset_device_state(struct vfio_device *vdev)
+{
+ struct virtiovf_pci_core_device *virtvdev = container_of(
+ vdev, struct virtiovf_pci_core_device, core_device.vdev);
+
+ if (!virtvdev->migrate_cap)
+ return;
+
+ mutex_lock(&virtvdev->state_mutex);
+ virtvdev->mig_state = VFIO_DEVICE_STATE_RUNNING;
+ virtiovf_disable_fds(virtvdev);
+ mutex_unlock(&virtvdev->state_mutex);
+}
+
static int virtiovf_pci_get_data_size(struct vfio_device *vdev,
unsigned long *stop_copy_length)
{
@@ -1297,13 +1268,14 @@ static int virtiovf_pci_get_data_size(struct vfio_device *vdev,
if (!obj_id_exists)
virtiovf_pci_free_obj_id(virtvdev, obj_id);
end:
- virtiovf_state_mutex_unlock(virtvdev);
+ mutex_unlock(&virtvdev->state_mutex);
return ret;
}
static const struct vfio_migration_ops virtvdev_pci_mig_ops = {
.migration_set_state = virtiovf_pci_set_device_state,
.migration_get_state = virtiovf_pci_get_device_state,
+ .migration_reset_state = virtiovf_pci_reset_device_state,
.migration_get_data_size = virtiovf_pci_get_data_size,
};
@@ -1311,7 +1283,6 @@ void virtiovf_set_migratable(struct virtiovf_pci_core_device *virtvdev)
{
virtvdev->migrate_cap = 1;
mutex_init(&virtvdev->state_mutex);
- spin_lock_init(&virtvdev->reset_lock);
virtvdev->core_device.vdev.migration_flags =
VFIO_MIGRATION_STOP_COPY |
VFIO_MIGRATION_P2P |
--
2.51.2
^ permalink raw reply [flat|nested] 11+ messages in thread