Re: [Qemu-devel] [PATCH 4/5] vfio: Move container list to DMAContext

[Alex Williamson]

On Wed, 2013-04-24 at 22:01 +1000, David Gibson wrote:
> At the moment, vfio maintains a global list of containers that are assumed
> to be more or less interchangeable, since they are all set up with a
> MemoryListener to have all of system memory mapped.  However, that only
> makes sense if all the containers are used on devices which really do
> expect a dma address space identical to system memory.
> 
> This patch moves towards that by making the list of containers per
> DMAContext (which corresponds to a dma address space) instead of global.

This seems like an unnecessary intrusion into common code.  Why not
create a vfio specific list of dma objects, each with a list of
containers?  Thanks,

Alex

> Signed-off-by: David Gibson <address@hidden>
> ---
>  dma-helpers.c          |    2 ++
>  hw/misc/vfio.c         |   13 ++++++++-----
>  include/hw/misc/vfio.h |   28 ++++++++++++++++++++++++++++
>  include/sysemu/dma.h   |    2 ++
>  stubs/Makefile.objs    |    1 +
>  stubs/vfio.c           |    6 ++++++
>  6 files changed, 47 insertions(+), 5 deletions(-)
>  create mode 100644 include/hw/misc/vfio.h
>  create mode 100644 stubs/vfio.c
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index 272632f..f0c7866 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -11,6 +11,7 @@
>  #include "trace.h"
>  #include "qemu/range.h"
>  #include "qemu/thread.h"
> +#include "hw/misc/vfio.h"
>  
>  /* #define DEBUG_IOMMU */
>  
> @@ -386,6 +387,7 @@ void dma_context_init(DMAContext *dma, AddressSpace *as, 
> DMATranslateFunc transl
>      dma->translate = translate;
>      dma->map = map;
>      dma->unmap = unmap;
> +    dma_context_init_vfio(dma);
>  }
>  
>  void *iommu_dma_memory_map(DMAContext *dma, dma_addr_t addr, dma_addr_t *len,
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index f77a599..ab870a8 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -39,6 +39,7 @@
>  #include "qemu/range.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/misc/vfio.h"
>  
>  /* #define DEBUG_VFIO */
>  #ifdef DEBUG_VFIO
> @@ -179,9 +180,6 @@ typedef struct VFIOGroup {
>  
>  #define MSIX_CAP_LENGTH 12
>  
> -static QLIST_HEAD(, VFIOContainer)
> -    container_list = QLIST_HEAD_INITIALIZER(container_list);
> -
>  static QLIST_HEAD(, VFIOGroup)
>      group_list = QLIST_HEAD_INITIALIZER(group_list);
>  
> @@ -2613,6 +2611,11 @@ static int vfio_load_rom(VFIODevice *vdev)
>      return 0;
>  }
>  
> +void dma_context_init_vfio(DMAContext *dma)
> +{
> +    QLIST_INIT(&dma->vfio.containers);
> +}
> +
>  static int vfio_connect_context(VFIOGroup *group, DMAContext *dma)
>  {
>      VFIOContainer *container;
> @@ -2628,7 +2631,7 @@ static int vfio_connect_context(VFIOGroup *group, 
> DMAContext *dma)
>          }
>      }
>  
> -    QLIST_FOREACH(container, &container_list, next) {
> +    QLIST_FOREACH(container, &dma->vfio.containers, next) {
>          if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
>              group->container = container;
>              QLIST_INSERT_HEAD(&container->group_list, group, container_next);
> @@ -2683,7 +2686,7 @@ static int vfio_connect_context(VFIOGroup *group, 
> DMAContext *dma)
>      }
>  
>      QLIST_INIT(&container->group_list);
> -    QLIST_INSERT_HEAD(&container_list, container, next);
> +    QLIST_INSERT_HEAD(&dma->vfio.containers, container, next);
>  
>      group->container = container;
>      QLIST_INSERT_HEAD(&container->group_list, group, container_next);
> diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h
> new file mode 100644
> index 0000000..18fe144
> --- /dev/null
> +++ b/include/hw/misc/vfio.h
> @@ -0,0 +1,28 @@
> +/*
> + * vfio based device assignment
> + *
> + * Copyright 2013 David Gibson, IBM Corporation.
> + * Copyright Red Hat, Inc. 2012
> + *
> + * This work is licensed under the terms of the GNU GPL, version
> + * 2. See the COPYING file in the top-level directory.
> + */
> +#ifndef QEMU_VFIO_H
> +#define QEMU_VFIO_H
> +
> +#include "qemu/queue.h"
> +
> +typedef struct DMAContext DMAContext;
> +struct DMAContext;
> +
> +typedef struct VFIOContainer VFIOContainer;
> +struct VFIOContainer;
> +
> +typedef struct DMAContextVFIO DMAContextVFIO;
> +struct DMAContextVFIO {
> +    QLIST_HEAD(, VFIOContainer) containers;
> +};
> +
> +void dma_context_init_vfio(DMAContext *dma);
> +
> +#endif /* QEMU_VFIO_H */
> diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
> index a52c93a..8692d0a 100644
> --- a/include/sysemu/dma.h
> +++ b/include/sysemu/dma.h
> @@ -15,6 +15,7 @@
>  #include "hw/hw.h"
>  #include "block/block.h"
>  #include "sysemu/kvm.h"
> +#include "hw/misc/vfio.h"
>  
>  typedef struct DMAContext DMAContext;
>  typedef struct ScatterGatherEntry ScatterGatherEntry;
> @@ -66,6 +67,7 @@ struct DMAContext {
>      DMATranslateFunc *translate;
>      DMAMapFunc *map;
>      DMAUnmapFunc *unmap;
> +    DMAContextVFIO vfio;
>  };
>  
>  /* A global DMA context corresponding to the address_space_memory
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 9c55b34..858ca6b 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -20,6 +20,7 @@ stub-obj-y += reset.o
>  stub-obj-y += set-fd-handler.o
>  stub-obj-y += slirp.o
>  stub-obj-y += sysbus.o
> +stub-obj-y += vfio.o
>  stub-obj-y += vm-stop.o
>  stub-obj-y += vmstate.o
>  stub-obj-$(CONFIG_WIN32) += fd-register.o
> diff --git a/stubs/vfio.c b/stubs/vfio.c
> new file mode 100644
> index 0000000..6fe4a84
> --- /dev/null
> +++ b/stubs/vfio.c
> @@ -0,0 +1,6 @@
> +#include "hw/misc/vfio.h"
> +#include "sysemu/dma.h"
> +
> +void dma_context_init_vfio(DMAContext *dma)
> +{
> +}