Re: [Qemu-devel] [PATCH 5/7] memory: Allow replay of IOMMU mapping notifications

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

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

On Fri, Sep 25, 2015 at 01:20:12PM +0200, Paolo Bonzini wrote:
> 
> 
> On 24/09/2015 06:33, David Gibson wrote:
> > When we have guest visible IOMMUs, we allow notifiers to be registered
> > which will be informed of all changes to IOMMU mappings.  This is used by
> > vfio to keep the host IOMMU mappings in sync with guest IOMMU mappings.
> > 
> > However, unlike with a memory region listener, an iommu notifier won't be
> > told about any mappings which already exist in the (guest) IOMMU at the
> > time it is registered.  This can cause problems if hotplugging a VFIO
> > device onto a guest bus which had existing guest IOMMU mappings, but didn't
> > previously have an VFIO devices (and hence no host IOMMU mappings).
> > 
> > This adds a memory_region_register_iommu_notifier_replay() function to
> > handle this case.  As well as registering the new notifier it replays
> > existing mappings.  Because the IOMMU memory region doesn't internally
> > remember the granularity of the guest IOMMU it has a small hack where the
> > caller must specify a granularity at which to replay mappings.
> > 
> > If there are finer mappings in the guest IOMMU these will be reported in
> > the iotlb structures passed to the notifier which it must handle (probably
> > causing it to flag an error).  This isn't new - the VFIO iommu notifier
> > must already handle notifications about guest IOMMU mappings too short
> > for it to represent in the host IOMMU.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> The patch is okay, just two questions:
> 
> 1) Is there a case where using the no-replay functions makes sense?

I'm not sure.  I think vfio is the only user so far, so I guess that's
technically a no.  I was reluctant to change the interface and
semantics just off the bat, though.

> 2) You could add a ->replay function to the iommu_ops to optimize it, if
> you want.

Yes.  Originally I was going to implement with a ->replay function,
before I realised I didn't actually need it.  I figure we can optimize
when and if it proves necessary.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]