diff mbox series

[v4,05/14] vfio/common: Add helper to validate iova/end against hostwin

Message ID 20230307020258.58215-6-joao.m.martins@oracle.com (mailing list archive)
State New, archived
Headers show
Series vfio/migration: Device dirty page tracking | expand

Commit Message

Joao Martins March 7, 2023, 2:02 a.m. UTC 
In preparation to be used in device dirty tracking, move the code that
finds the container host DMA window against a iova range.  This avoids
duplication on the common checks across listener callbacks.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/common.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

Comments

Avihai Horon March 7, 2023, 8:57 a.m. UTC | #1 
On 07/03/2023 4:02, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> In preparation to be used in device dirty tracking, move the code that
> finds the container host DMA window against a iova range.  This avoids
> duplication on the common checks across listener callbacks.

Eventually this isn't used by device dirty tracking, so "In preparation 
to be used in device dirty tracking" can be dropped.

Other than that, FWIW:

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

Thanks.

>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/common.c | 38 ++++++++++++++++++++------------------
>   1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index cec3de08d2b4..99acb998eb14 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -901,6 +901,22 @@ static void vfio_unregister_ram_discard_listener(VFIOContainer *container,
>       g_free(vrdl);
>   }
>
> +static VFIOHostDMAWindow *vfio_find_hostwin(VFIOContainer *container,
> +                                            hwaddr iova, hwaddr end)
> +{
> +    VFIOHostDMAWindow *hostwin;
> +    bool hostwin_found = false;
> +
> +    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
> +        if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
> +            hostwin_found = true;
> +            break;
> +        }
> +    }
> +
> +    return hostwin_found ? hostwin : NULL;
> +}
> +
>   static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
>   {
>       MemoryRegion *mr = section->mr;
> @@ -926,7 +942,6 @@ static void vfio_listener_region_add(MemoryListener *listener,
>       void *vaddr;
>       int ret;
>       VFIOHostDMAWindow *hostwin;
> -    bool hostwin_found;
>       Error *err = NULL;
>
>       if (vfio_listener_skipped_section(section)) {
> @@ -1027,15 +1042,8 @@ static void vfio_listener_region_add(MemoryListener *listener,
>   #endif
>       }
>
> -    hostwin_found = false;
> -    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
> -        if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
> -            hostwin_found = true;
> -            break;
> -        }
> -    }
> -
> -    if (!hostwin_found) {
> +    hostwin = vfio_find_hostwin(container, iova, end);
> +    if (!hostwin) {
>           error_setg(&err, "Container %p can't map guest IOVA region"
>                      " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx, container, iova, end);
>           goto fail;
> @@ -1237,15 +1245,9 @@ static void vfio_listener_region_del(MemoryListener *listener,
>       if (memory_region_is_ram_device(section->mr)) {
>           hwaddr pgmask;
>           VFIOHostDMAWindow *hostwin;
> -        bool hostwin_found = false;
>
> -        QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
> -            if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
> -                hostwin_found = true;
> -                break;
> -            }
> -        }
> -        assert(hostwin_found); /* or region_add() would have failed */
> +        hostwin = vfio_find_hostwin(container, iova, end);
> +        assert(hostwin); /* or region_add() would have failed */
>
>           pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>           try_unmap = !((iova & pgmask) || (int128_get64(llsize) & pgmask));
> --
> 2.17.2
>
Joao Martins March 7, 2023, 10:18 a.m. UTC | #2 
On 07/03/2023 08:57, Avihai Horon wrote:
> 
> On 07/03/2023 4:02, Joao Martins wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> In preparation to be used in device dirty tracking, move the code that
>> finds the container host DMA window against a iova range.  This avoids
>> duplication on the common checks across listener callbacks.
> 
> Eventually this isn't used by device dirty tracking, so "In preparation to be
> used in device dirty tracking" can be dropped.
> 
good catch, as this is here since the first range-version checks that still had
a over complicated version of vfio_get_section_range(). I'll remove it.

> Other than that, FWIW:
> 
> Reviewed-by: Avihai Horon <avihaih@nvidia.com>
> 
> Thanks.
> 
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/common.c | 38 ++++++++++++++++++++------------------
>>   1 file changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index cec3de08d2b4..99acb998eb14 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -901,6 +901,22 @@ static void
>> vfio_unregister_ram_discard_listener(VFIOContainer *container,
>>       g_free(vrdl);
>>   }
>>
>> +static VFIOHostDMAWindow *vfio_find_hostwin(VFIOContainer *container,
>> +                                            hwaddr iova, hwaddr end)
>> +{
>> +    VFIOHostDMAWindow *hostwin;
>> +    bool hostwin_found = false;
>> +
>> +    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
>> +        if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
>> +            hostwin_found = true;
>> +            break;
>> +        }
>> +    }
>> +
>> +    return hostwin_found ? hostwin : NULL;
>> +}
>> +
>>   static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
>>   {
>>       MemoryRegion *mr = section->mr;
>> @@ -926,7 +942,6 @@ static void vfio_listener_region_add(MemoryListener
>> *listener,
>>       void *vaddr;
>>       int ret;
>>       VFIOHostDMAWindow *hostwin;
>> -    bool hostwin_found;
>>       Error *err = NULL;
>>
>>       if (vfio_listener_skipped_section(section)) {
>> @@ -1027,15 +1042,8 @@ static void vfio_listener_region_add(MemoryListener
>> *listener,
>>   #endif
>>       }
>>
>> -    hostwin_found = false;
>> -    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
>> -        if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
>> -            hostwin_found = true;
>> -            break;
>> -        }
>> -    }
>> -
>> -    if (!hostwin_found) {
>> +    hostwin = vfio_find_hostwin(container, iova, end);
>> +    if (!hostwin) {
>>           error_setg(&err, "Container %p can't map guest IOVA region"
>>                      " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx, container, iova, end);
>>           goto fail;
>> @@ -1237,15 +1245,9 @@ static void vfio_listener_region_del(MemoryListener
>> *listener,
>>       if (memory_region_is_ram_device(section->mr)) {
>>           hwaddr pgmask;
>>           VFIOHostDMAWindow *hostwin;
>> -        bool hostwin_found = false;
>>
>> -        QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
>> -            if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
>> -                hostwin_found = true;
>> -                break;
>> -            }
>> -        }
>> -        assert(hostwin_found); /* or region_add() would have failed */
>> +        hostwin = vfio_find_hostwin(container, iova, end);
>> +        assert(hostwin); /* or region_add() would have failed */
>>
>>           pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>>           try_unmap = !((iova & pgmask) || (int128_get64(llsize) & pgmask));
>> -- 
>> 2.17.2
>>
diff mbox series

Patch

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index cec3de08d2b4..99acb998eb14 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -901,6 +901,22 @@  static void vfio_unregister_ram_discard_listener(VFIOContainer *container,
     g_free(vrdl);
 }
 
+static VFIOHostDMAWindow *vfio_find_hostwin(VFIOContainer *container,
+                                            hwaddr iova, hwaddr end)
+{
+    VFIOHostDMAWindow *hostwin;
+    bool hostwin_found = false;
+
+    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
+        if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
+            hostwin_found = true;
+            break;
+        }
+    }
+
+    return hostwin_found ? hostwin : NULL;
+}
+
 static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
 {
     MemoryRegion *mr = section->mr;
@@ -926,7 +942,6 @@  static void vfio_listener_region_add(MemoryListener *listener,
     void *vaddr;
     int ret;
     VFIOHostDMAWindow *hostwin;
-    bool hostwin_found;
     Error *err = NULL;
 
     if (vfio_listener_skipped_section(section)) {
@@ -1027,15 +1042,8 @@  static void vfio_listener_region_add(MemoryListener *listener,
 #endif
     }
 
-    hostwin_found = false;
-    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
-        if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
-            hostwin_found = true;
-            break;
-        }
-    }
-
-    if (!hostwin_found) {
+    hostwin = vfio_find_hostwin(container, iova, end);
+    if (!hostwin) {
         error_setg(&err, "Container %p can't map guest IOVA region"
                    " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx, container, iova, end);
         goto fail;
@@ -1237,15 +1245,9 @@  static void vfio_listener_region_del(MemoryListener *listener,
     if (memory_region_is_ram_device(section->mr)) {
         hwaddr pgmask;
         VFIOHostDMAWindow *hostwin;
-        bool hostwin_found = false;
 
-        QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
-            if (hostwin->min_iova <= iova && end <= hostwin->max_iova) {
-                hostwin_found = true;
-                break;
-            }
-        }
-        assert(hostwin_found); /* or region_add() would have failed */
+        hostwin = vfio_find_hostwin(container, iova, end);
+        assert(hostwin); /* or region_add() would have failed */
 
         pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
         try_unmap = !((iova & pgmask) || (int128_get64(llsize) & pgmask));