diff mbox series

[v5,06/10] vfio: Reverse test on vfio_get_dirty_bitmap()

Message ID 20240506092053.388578-7-clg@redhat.com (mailing list archive)
State New
Headers show
Series vfio: Improve error reporting (part 2) | expand

Commit Message

Cédric Le Goater May 6, 2024, 9:20 a.m. UTC 
It will simplify the changes coming after.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/common.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Comments

Avihai Horon May 13, 2024, 1:42 p.m. UTC | #1 
On 06/05/2024 12:20, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
Title should be: Reverse test on vfio_get_xlat_addr()?

> It will simplify the changes coming after.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/common.c | 22 +++++++++++++---------
>   1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index ed5ee6349ced78b3bde68d2ee506f78ba1a9dd9c..b929bb0b7ac60dcef34c0d5a098d5d91f75501dd 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1224,16 +1224,20 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>       }
>
>       rcu_read_lock();
> -    if (vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
> -        ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
> -                                    translated_addr);
> -        if (ret) {
> -            error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
> -                         "0x%"HWADDR_PRIx") = %d (%s)",
> -                         bcontainer, iova, iotlb->addr_mask + 1, ret,
> -                         strerror(-ret));
> -        }
> +    if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
> +        goto out_lock;
>       }
> +
> +    ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
> +                                translated_addr);
> +    if (ret) {
> +        error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
> +                     "0x%"HWADDR_PRIx") = %d (%s)",
> +                     bcontainer, iova, iotlb->addr_mask + 1, ret,
> +                     strerror(-ret));
> +    }
> +
> +out_lock:

s/out_lock/out_unlock?

With the above,

Reviewed-by: Avihai Horon <avihaih@nvidia.com>

>       rcu_read_unlock();
>
>   out:
> --
> 2.45.0
>
Cédric Le Goater May 14, 2024, 8:55 a.m. UTC | #2 
On 5/13/24 15:42, Avihai Horon wrote:
> 
> On 06/05/2024 12:20, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
> Title should be: Reverse test on vfio_get_xlat_addr()?

It could.

> 
>> It will simplify the changes coming after.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/common.c | 22 +++++++++++++---------
>>   1 file changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index ed5ee6349ced78b3bde68d2ee506f78ba1a9dd9c..b929bb0b7ac60dcef34c0d5a098d5d91f75501dd 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -1224,16 +1224,20 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>>       }
>>
>>       rcu_read_lock();
>> -    if (vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
>> -        ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
>> -                                    translated_addr);
>> -        if (ret) {
>> -            error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
>> -                         "0x%"HWADDR_PRIx") = %d (%s)",
>> -                         bcontainer, iova, iotlb->addr_mask + 1, ret,
>> -                         strerror(-ret));
>> -        }
>> +    if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
>> +        goto out_lock;
>>       }
>> +
>> +    ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
>> +                                translated_addr);
>> +    if (ret) {
>> +        error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
>> +                     "0x%"HWADDR_PRIx") = %d (%s)",
>> +                     bcontainer, iova, iotlb->addr_mask + 1, ret,
>> +                     strerror(-ret));
>> +    }
>> +
>> +out_lock:
> 
> s/out_lock/out_unlock?

yes. better naming.

> 
> With the above,

both done.


Thanks,

C.



> Reviewed-by: Avihai Horon <avihaih@nvidia.com>
> 
>>       rcu_read_unlock();
>>
>>   out:
>> -- 
>> 2.45.0
>>
>
diff mbox series

Patch

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index ed5ee6349ced78b3bde68d2ee506f78ba1a9dd9c..b929bb0b7ac60dcef34c0d5a098d5d91f75501dd 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1224,16 +1224,20 @@  static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
     }
 
     rcu_read_lock();
-    if (vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
-        ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
-                                    translated_addr);
-        if (ret) {
-            error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
-                         "0x%"HWADDR_PRIx") = %d (%s)",
-                         bcontainer, iova, iotlb->addr_mask + 1, ret,
-                         strerror(-ret));
-        }
+    if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
+        goto out_lock;
     }
+
+    ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
+                                translated_addr);
+    if (ret) {
+        error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
+                     "0x%"HWADDR_PRIx") = %d (%s)",
+                     bcontainer, iova, iotlb->addr_mask + 1, ret,
+                     strerror(-ret));
+    }
+
+out_lock:
     rcu_read_unlock();
 
 out: