Re: [Qemu-devel] [PATCH 3/4] vfio: Move container list to iommu MemoryRegion

[David Gibson]

On Sat, Apr 27, 2013 at 02:17:54PM +0200, Paolo Bonzini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Il 27/04/2013 11:49, David Gibson ha scritto:
> >> There's no fundamental reason for VFIO to use multiple 
> >> MemoryListeners.  It could use one for all VFIO instances.
> > 
> > Actually, there kind of is.  Using a new listener for each new 
> > container means the listener framework automatically invokes
> > callbacks for all the existing reasons, allowing us to map them
> > into the container.  With a single listener, we'd have to manually
> > write logic to map all the existing mappings into each new
> > container.
> 
> Ah, thanks for teaching me.  It is indeed more convenient.
> 
> >>> More importantly, what I'm working towards here is vfio support
> >>> for guest visible IOMMUs that don't have all of guest RAM
> >>> mapped into them initially.  In that case there won't (and
> >>> can't) be any MemoryListener at all.
> >> 
> >> Why?  All you want here is to look for appearance of an IOMMU 
> >> MemoryRegion in the flat representation of the AddressSpace.
> >> That's exactly what the MemoryListener does---of course that's a
> >> different MemoryListener implementation than VFIO's current one.
> >> 
> >> The MemoryListener is used for a lot of different things, I find
> >> it hard to believe that this is not a variation on one of them.
> > 
> > Uh.. so, yes we might actually want a memorylistener to watch for 
> > appearance of iommu regions, if there are memoryregion layers
> > between us and the iommu region itself.
> > 
> > But the point remains that from the code which implements the
> > guest side IOMMU we have to somehow invoke a hook which will make
> > parallel mappings in the vfio container.  That will need to go from
> > the iommu code's handle on the address space - a MemoryRegion - to
> > vfio's handle on the address space, the vfio container.  I don't
> > see a way to do that without having a vfio private pointer in the
> > memoryregion, that isn't much uglier than that minor encapsulation
> > breach.
> 
> Ok, knowing about changes that happen in the IOMMU mapping is indeed
> out of scope of MemoryListeners.  What about adding a NotifierList?
> Then VFIO can register a notifier and use it to learn about "events"
> in the IOMMU mapping.  The notifier data can be a MemoryRegionSection
> or IOMMUTableEntry, whatever you find more convenient.

For the generic case a Notifier could work in principle.  Neither of
those structures is suitable as the data though: constructing a
MemoryRegionSection for every page we map into the IOTLB is far too
heavyweight, and the IOMMUTLBEntry doesn't contain the IOVA.

There still might be complications with setup and teardown - on
pseries, for example, we need to do some bookkeeping when a container
is first attached to an iommu address space.  Likewise we may have
some vfio specific stuff to do to clean up when an iommu address space
is removed.

For pseries, we also have a complication to handle our accelerated
case, where the IOMMU interface (hypercalls, in our case) are handled
in kvm, and directly do the host IOMMU manipulations for VFIO, without
bouncing through qemu.  To do that we need to tell the kernel the
(qemu allocated) ID for the address space so it can associated that
with the VFIO container.

Thinking over, I think what that mostly amounts to, is that if the
VFIO aspects of the address space are already wired up by the host
bridge, then the individual vfio-pci devices need a way of going from
their qemu iommu address space (which they get from pci_dev->iommu) to
the vfio specific information about that address space.

-- 
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

signature.asc
Description: Digital signature