Re: [PATCH v5 08/15] vfio/common: Add helper to consolidate iova/end calculation

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

On 3/7/23 13:54, Joao Martins wrote:
> In preparation to be used in device dirty tracking, move the code that
> calculate a iova/end range from the container/section.  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>

Thanks,

C.



> ---
>   hw/vfio/common.c | 41 +++++++++++++++++++++++++++--------------
>   1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index d38b7c1969b9..63831eab78a1 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -962,6 +962,31 @@ static bool vfio_listener_valid_section(MemoryRegionSection *section,
>       return true;
>   }
>   
> +static bool vfio_get_section_iova_range(VFIOContainer *container,
> +                                        MemoryRegionSection *section,
> +                                        hwaddr *out_iova, hwaddr *out_end,
> +                                        Int128 *out_llend)
> +{
> +    Int128 llend;
> +    hwaddr iova;
> +
> +    iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
> +    llend = int128_make64(section->offset_within_address_space);
> +    llend = int128_add(llend, section->size);
> +    llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
> +
> +    if (int128_ge(int128_make64(iova), llend)) {
> +        return false;
> +    }
> +
> +    *out_iova = iova;
> +    *out_end = int128_get64(int128_sub(llend, int128_one()));
> +    if (out_llend) {
> +        *out_llend = llend;
> +    }
> +    return true;
> +}
> +
>   static void vfio_listener_region_add(MemoryListener *listener,
>                                        MemoryRegionSection *section)
>   {
> @@ -977,12 +1002,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>           return;
>       }
>   
> -    iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
> -    llend = int128_make64(section->offset_within_address_space);
> -    llend = int128_add(llend, section->size);
> -    llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
> -
> -    if (int128_ge(int128_make64(iova), llend)) {
> +    if (!vfio_get_section_iova_range(container, section, &iova, &end, &llend)) {
>           if (memory_region_is_ram_device(section->mr)) {
>               trace_vfio_listener_region_add_no_dma_map(
>                   memory_region_name(section->mr),
> @@ -992,7 +1012,6 @@ static void vfio_listener_region_add(MemoryListener *listener,
>           }
>           return;
>       }
> -    end = int128_get64(int128_sub(llend, int128_one()));
>   
>       if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
>           hwaddr pgsize = 0;
> @@ -1219,15 +1238,9 @@ static void vfio_listener_region_del(MemoryListener *listener,
>            */
>       }
>   
> -    iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
> -    llend = int128_make64(section->offset_within_address_space);
> -    llend = int128_add(llend, section->size);
> -    llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
> -
> -    if (int128_ge(int128_make64(iova), llend)) {
> +    if (!vfio_get_section_iova_range(container, section, &iova, &end, &llend)) {
>           return;
>       }
> -    end = int128_get64(int128_sub(llend, int128_one()));
>   
>       llsize = int128_sub(llend, int128_make64(iova));
>