| Message ID | 20231229203854.8281-1-vr_qemu@t-online.de |
|---|---|
| State | New |
| Headers | show |
| Series | hw/vfio: fix iteration over global VFIODevice list | expand |
>-----Original Message----- >From: Volker Rümelin <vr_qemu@t-online.de> >Subject: [PATCH] hw/vfio: fix iteration over global VFIODevice list > >Commit 3d779abafe ("vfio/common: Introduce a global VFIODevice list") >introduced a global VFIODevice list, but forgot to update the list >element field name when iterating over the new list. Change the code >to use the correct list element field. > >Fixes: 3d779abafe ("vfio/common: Introduce a global VFIODevice list") >Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2061 >Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com> My fault, thanks for fixing! I also digged into why I didn't reproduce this issue with two vfio devices before. This issue only reproduce with two vfio devices in one reset group(same slot or bus) plus a third vfio device in a different reset group. Then the two vfio devices will miss reset due to this bug. BRs. Zhenzhong >--- > hw/vfio/common.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > >diff --git a/hw/vfio/common.c b/hw/vfio/common.c >index 08a3e57672..3ba892d7d3 100644 >--- a/hw/vfio/common.c >+++ b/hw/vfio/common.c >@@ -74,7 +74,7 @@ bool vfio_mig_active(void) > return false; > } > >- QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >+ QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->migration_blocker) { > return false; > } >@@ -95,7 +95,7 @@ static bool >vfio_multiple_devices_migration_is_supported(void) > unsigned int device_num = 0; > bool all_support_p2p = true; > >- QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >+ QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->migration) { > device_num++; > >@@ -1367,13 +1367,13 @@ void vfio_reset_handler(void *opaque) > { > VFIODevice *vbasedev; > >- QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >+ QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->dev->realized) { > vbasedev->ops->vfio_compute_needs_reset(vbasedev); > } > } > >- QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >+ QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->dev->realized && vbasedev->needs_reset) { > vbasedev->ops->vfio_hot_reset_multi(vbasedev); > } >-- >2.35.3
On 12/29/23 21:38, Volker Rümelin wrote: > Commit 3d779abafe ("vfio/common: Introduce a global VFIODevice list") > introduced a global VFIODevice list, but forgot to update the list > element field name when iterating over the new list. Change the code > to use the correct list element field. > > Fixes: 3d779abafe ("vfio/common: Introduce a global VFIODevice list") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2061 > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > hw/vfio/common.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 08a3e57672..3ba892d7d3 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -74,7 +74,7 @@ bool vfio_mig_active(void) > return false; > } > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->migration_blocker) { > return false; > } > @@ -95,7 +95,7 @@ static bool vfio_multiple_devices_migration_is_supported(void) > unsigned int device_num = 0; > bool all_support_p2p = true; > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->migration) { > device_num++; > > @@ -1367,13 +1367,13 @@ void vfio_reset_handler(void *opaque) > { > VFIODevice *vbasedev; > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->dev->realized) { > vbasedev->ops->vfio_compute_needs_reset(vbasedev); > } > } > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->dev->realized && vbasedev->needs_reset) { > vbasedev->ops->vfio_hot_reset_multi(vbasedev); > }
On 1/2/24 05:27, Duan, Zhenzhong wrote: > > >> -----Original Message----- >> From: Volker Rümelin <vr_qemu@t-online.de> >> Subject: [PATCH] hw/vfio: fix iteration over global VFIODevice list >> >> Commit 3d779abafe ("vfio/common: Introduce a global VFIODevice list") >> introduced a global VFIODevice list, but forgot to update the list >> element field name when iterating over the new list. Change the code >> to use the correct list element field. >> >> Fixes: 3d779abafe ("vfio/common: Introduce a global VFIODevice list") >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2061 >> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > > Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > > My fault, thanks for fixing! > > I also digged into why I didn't reproduce this issue with two vfio devices before. > This issue only reproduce with two vfio devices in one reset group(same slot or bus) > plus a third vfio device in a different reset group. Then the two vfio devices will > miss reset due to this bug. Thanks for digging. C. > BRs. > Zhenzhong > >> --- >> hw/vfio/common.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index 08a3e57672..3ba892d7d3 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -74,7 +74,7 @@ bool vfio_mig_active(void) >> return false; >> } >> >> - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >> + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { >> if (vbasedev->migration_blocker) { >> return false; >> } >> @@ -95,7 +95,7 @@ static bool >> vfio_multiple_devices_migration_is_supported(void) >> unsigned int device_num = 0; >> bool all_support_p2p = true; >> >> - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >> + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { >> if (vbasedev->migration) { >> device_num++; >> >> @@ -1367,13 +1367,13 @@ void vfio_reset_handler(void *opaque) >> { >> VFIODevice *vbasedev; >> >> - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >> + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { >> if (vbasedev->dev->realized) { >> vbasedev->ops->vfio_compute_needs_reset(vbasedev); >> } >> } >> >> - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { >> + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { >> if (vbasedev->dev->realized && vbasedev->needs_reset) { >> vbasedev->ops->vfio_hot_reset_multi(vbasedev); >> } >> -- >> 2.35.3 >
Hi Volker, On 12/29/23 21:38, Volker Rümelin wrote: > Commit 3d779abafe ("vfio/common: Introduce a global VFIODevice list") > introduced a global VFIODevice list, but forgot to update the list > element field name when iterating over the new list. Change the code > to use the correct list element field. > > Fixes: 3d779abafe ("vfio/common: Introduce a global VFIODevice list") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2061 > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks! Eric > --- > hw/vfio/common.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 08a3e57672..3ba892d7d3 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -74,7 +74,7 @@ bool vfio_mig_active(void) > return false; > } > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->migration_blocker) { > return false; > } > @@ -95,7 +95,7 @@ static bool vfio_multiple_devices_migration_is_supported(void) > unsigned int device_num = 0; > bool all_support_p2p = true; > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->migration) { > device_num++; > > @@ -1367,13 +1367,13 @@ void vfio_reset_handler(void *opaque) > { > VFIODevice *vbasedev; > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->dev->realized) { > vbasedev->ops->vfio_compute_needs_reset(vbasedev); > } > } > > - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { > + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { > if (vbasedev->dev->realized && vbasedev->needs_reset) { > vbasedev->ops->vfio_hot_reset_multi(vbasedev); > }
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 08a3e57672..3ba892d7d3 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -74,7 +74,7 @@ bool vfio_mig_active(void) return false; } - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { if (vbasedev->migration_blocker) { return false; } @@ -95,7 +95,7 @@ static bool vfio_multiple_devices_migration_is_supported(void) unsigned int device_num = 0; bool all_support_p2p = true; - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { if (vbasedev->migration) { device_num++; @@ -1367,13 +1367,13 @@ void vfio_reset_handler(void *opaque) { VFIODevice *vbasedev; - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { if (vbasedev->dev->realized) { vbasedev->ops->vfio_compute_needs_reset(vbasedev); } } - QLIST_FOREACH(vbasedev, &vfio_device_list, next) { + QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) { if (vbasedev->dev->realized && vbasedev->needs_reset) { vbasedev->ops->vfio_hot_reset_multi(vbasedev); }
Commit 3d779abafe ("vfio/common: Introduce a global VFIODevice list") introduced a global VFIODevice list, but forgot to update the list element field name when iterating over the new list. Change the code to use the correct list element field. Fixes: 3d779abafe ("vfio/common: Introduce a global VFIODevice list") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2061 Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> --- hw/vfio/common.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)