Re: [Qemu-devel] [PATCH qemu v18 4/5] vfio/spapr: Create DMA window dynamically (SPAPR IOMMU v2)

[Laurent Vivier]

On 21/06/2016 03:14, Alexey Kardashevskiy wrote:
> New VFIO_SPAPR_TCE_v2_IOMMU type supports dynamic DMA window management.
> This adds ability to VFIO common code to dynamically allocate/remove
> DMA windows in the host kernel when new VFIO container is added/removed.
> 
> This adds a helper to vfio_listener_region_add which makes
> VFIO_IOMMU_SPAPR_TCE_CREATE ioctl and adds just created IOMMU into
> the host IOMMU list; the opposite action is taken in
> vfio_listener_region_del.
> 
> When creating a new window, this uses heuristic to decide on the TCE table
> levels number.
> 
> This should cause no guest visible change in behavior.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> Changes:
> v18:
> * moved trace definitions under hw/vfio/spapr.c section
> * moved trace_vfio_spapr_remove_window to vfio_spapr_remove_window()
> * vfio_host_win_del() now checks for exact window size
> * one ctz() less in vfio_spapr_create_window()
> 
> v17:
> * moved spapr window create/remove helpers to separate file
> * added hw_error() if vfio_host_win_del() failed
> 
> v16:
> * used memory_region_iommu_get_page_sizes() in vfio_listener_region_add()
> * enforced no intersections between windows
> 
> v14:
> * new to the series
> ---
>  hw/vfio/common.c              | 79 
> +++++++++++++++++++++++++++++++++++++------
>  hw/vfio/spapr.c               | 71 ++++++++++++++++++++++++++++++++++++++
>  include/hw/vfio/vfio-common.h |  6 ++++
>  trace-events                  |  2 ++
>  4 files changed, 148 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index b53a1db..8e3466c 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -265,6 +265,21 @@ static void vfio_host_win_add(VFIOContainer *container,
>      QLIST_INSERT_HEAD(&container->hostwin_list, hostwin, hostwin_next);
>  }
>  
> +static int vfio_host_win_del(VFIOContainer *container, hwaddr min_iova,
> +                             hwaddr max_iova)
> +{
> +    VFIOHostDMAWindow *hostwin;
> +
> +    QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
> +        if (hostwin->min_iova == min_iova && hostwin->max_iova == max_iova) {
> +            QLIST_REMOVE(hostwin, hostwin_next);
> +            return 0;
> +        }
> +    }
> +
> +    return -1;
> +}
> +
>  static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>  {
>      return (!memory_region_is_ram(section->mr) &&
> @@ -380,6 +395,30 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>      }
>      end = int128_get64(int128_sub(llend, int128_one()));
>  
> +    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> +        VFIOHostDMAWindow *hostwin;
> +        hwaddr pgsize = 0;
> +
> +        /* For now intersections are not allowed, we may relax this later */
> +        QLIST_FOREACH(hostwin, &container->hostwin_list, hostwin_next) {
> +            if (ranges_overlap(hostwin->min_iova,
> +                               hostwin->max_iova - hostwin->min_iova + 1,
> +                               section->offset_within_address_space,
> +                               int128_get64(section->size))) {
> +                goto fail;

ret is not initialized and it is used in "fail:".

hw/vfio/common.c: In function ‘vfio_listener_region_add’:
hw/vfio/common.c:493:30: error: ‘ret’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
             container->error = ret;

Laurent