Re: [PATCH v3 09/13] vfio/common: Extract code from vfio_get_dirty_bitmap() to new function

["Cédric Le Goater" <clg@redhat.com>]

On 3/4/23 02:43, Joao Martins wrote:
> From: Avihai Horon <avihaih@nvidia.com>
> 
> Extract the VFIO_IOMMU_DIRTY_PAGES ioctl code in vfio_get_dirty_bitmap()
> to its own function.
> 
> This will help the code to be more readable after next patch will add
> device dirty page bitmap sync functionality.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>



Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

> ---
>   hw/vfio/common.c | 57 +++++++++++++++++++++++++++++-------------------
>   1 file changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index aa0df0604704..b0c7d03279ab 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1579,26 +1579,13 @@ static void vfio_listener_log_global_stop(MemoryListener *listener)
>       }
>   }
>   
> -static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
> -                                 uint64_t size, ram_addr_t ram_addr)
> +static int vfio_query_dirty_bitmap(VFIOContainer *container, VFIOBitmap *vbmap,
> +                                   hwaddr iova, hwaddr size)
>   {
>       struct vfio_iommu_type1_dirty_bitmap *dbitmap;
>       struct vfio_iommu_type1_dirty_bitmap_get *range;
> -    VFIOBitmap vbmap;
>       int ret;
>   
> -    if (!container->dirty_pages_supported) {
> -        cpu_physical_memory_set_dirty_range(ram_addr, size,
> -                                            tcg_enabled() ? DIRTY_CLIENTS_ALL :
> -                                            DIRTY_CLIENTS_NOCODE);
> -        return 0;
> -    }
> -
> -    ret = vfio_bitmap_alloc(&vbmap, size);
> -    if (ret) {
> -        return -errno;
> -    }
> -
>       dbitmap = g_malloc0(sizeof(*dbitmap) + sizeof(*range));
>   
>       dbitmap->argsz = sizeof(*dbitmap) + sizeof(*range);
> @@ -1613,8 +1600,8 @@ static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
>        * to qemu_real_host_page_size.
>        */
>       range->bitmap.pgsize = qemu_real_host_page_size();
> -    range->bitmap.size = vbmap.size;
> -    range->bitmap.data = (__u64 *)vbmap.bitmap;
> +    range->bitmap.size = vbmap->size;
> +    range->bitmap.data = (__u64 *)vbmap->bitmap;
>   
>       ret = ioctl(container->fd, VFIO_IOMMU_DIRTY_PAGES, dbitmap);
>       if (ret) {
> @@ -1622,16 +1609,42 @@ static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
>           error_report("Failed to get dirty bitmap for iova: 0x%"PRIx64
>                   " size: 0x%"PRIx64" err: %d", (uint64_t)range->iova,
>                   (uint64_t)range->size, errno);
> -        goto err_out;
> +    }
> +
> +    g_free(dbitmap);
> +
> +    return ret;
> +}
> +
> +static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
> +                                 uint64_t size, ram_addr_t ram_addr)
> +{
> +    VFIOBitmap vbmap;
> +    int ret;
> +
> +    if (!container->dirty_pages_supported) {
> +        cpu_physical_memory_set_dirty_range(ram_addr, size,
> +                                            tcg_enabled() ? DIRTY_CLIENTS_ALL :
> +                                            DIRTY_CLIENTS_NOCODE);
> +        return 0;
> +    }
> +
> +    ret = vfio_bitmap_alloc(&vbmap, size);
> +    if (ret) {
> +        return -errno;
> +    }
> +
> +    ret = vfio_query_dirty_bitmap(container, &vbmap, iova, size);
> +    if (ret) {
> +        goto out;
>       }
>   
>       cpu_physical_memory_set_dirty_lebitmap(vbmap.bitmap, ram_addr,
>                                              vbmap.pages);
>   
> -    trace_vfio_get_dirty_bitmap(container->fd, range->iova, range->size,
> -                                range->bitmap.size, ram_addr);
> -err_out:
> -    g_free(dbitmap);
> +    trace_vfio_get_dirty_bitmap(container->fd, iova, size, vbmap.size,
> +                                ram_addr);
> +out:
>       g_free(vbmap.bitmap);
>   
>       return ret;