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

[Avihai Horon]

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