1. Patchew
  2. QEMU
  3. View series

[PATCH] hw/vfio: fix iteration over global VFIODevice list

Volker Rümelin posted 1 patch 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231229203854.8281-1-vr._5Fqemu@t-online.de
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/common.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] hw/vfio: fix iteration over global VFIODevice list
Posted by Volker Rümelin 11 months ago 
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(-)

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
Re: [PATCH] hw/vfio: fix iteration over global VFIODevice list
Posted by Eric Auger 10 months, 4 weeks ago 
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);
>          }
Re: [PATCH] hw/vfio: fix iteration over global VFIODevice list
Posted by Cédric Le Goater 10 months, 4 weeks ago 
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);
>           }
RE: [PATCH] hw/vfio: fix iteration over global VFIODevice list
Posted by Duan, Zhenzhong 10 months, 4 weeks ago 

>-----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
Re: [PATCH] hw/vfio: fix iteration over global VFIODevice list
Posted by Cédric Le Goater 10 months, 4 weeks ago 
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
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.