Re: [Qemu-devel] [PATCH 7/8] vfio: Introduce VFIO address spaces

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

On Mon, May 13, 2013 at 03:32:59PM -0600, Alex Williamson wrote:
> On Mon, 2013-05-13 at 20:54 +1000, David Gibson wrote:
> > The only model so far supported for VFIO passthrough devices is the model
> > usually used on x86, where all of the guest's RAM is mapped into the
> > (host) IOMMU and there is no IOMMU visible in the guest.
> > 
> > This patch begins to relax this model, introducing the notion of a
> > VFIOAddressSpace.  This represents a logical DMA address space which will
> > be visible to one or more VFIO devices by appropriate mapping in the (host)
> > IOMMU.  Thus the currently global list of containers becomes local to
> > a VFIOAddressSpace, and we verify that we don't attempt to add a VFIO
> > group to multiple address spaces.
> > 
> > For now, only one VFIOAddressSpace is created and used, corresponding to
> > main system memory, that will change in future patches.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/misc/vfio.c |   67 ++++++++++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 50 insertions(+), 17 deletions(-)
> > 
> > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> > index c4a8853..b1e9220 100644
> > --- a/hw/misc/vfio.c
> > +++ b/hw/misc/vfio.c
> > @@ -113,9 +113,17 @@ enum {
> >      VFIO_INT_MSIX = 3,
> >  };
> >  
> > +typedef struct VFIOAddressSpace {
> > +    AddressSpace *as;
> > +    QLIST_HEAD(, VFIOContainer) containers;
> > +} VFIOAddressSpace;
> > +
> > +static VFIOAddressSpace vfio_address_space_memory;
> > +
> >  struct VFIOGroup;
> >  
> >  typedef struct VFIOContainer {
> > +    VFIOAddressSpace *vas;
> 
> "space" maybe?

Ok, "space" it is.

> >      int fd; /* /dev/vfio/vfio, empowered by the attached groups */
> >      struct {
> >          /* enable abstraction to support various iommu backends */
> > @@ -178,9 +186,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);
> >  
> > @@ -2624,16 +2629,28 @@ static int vfio_load_rom(VFIODevice *vdev)
> >      return 0;
> >  }
> >  
> > -static int vfio_connect_container(VFIOGroup *group)
> > +static void vfio_address_space_init(VFIOAddressSpace *vas, AddressSpace *as)
> > +{
> > +    vas->as = as;
> > +    QLIST_INIT(&vas->containers);
> > +}
> > +
> > +static int vfio_connect(VFIOGroup *group, VFIOAddressSpace *vas)
> 
> Connect what?  Verb, no object.  This is still trying to attach a group
> to an existing container or create a new container, the address space is
> just a parameter in that matching.  So I'm not sure why this isn't still
> vfio_connect_container.

Uh, yeah, fair enough.  I've reverted to connect_container in the
next version.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]