On Wed, Sep 23, 2015 at 12:40:18PM +0200, Thomas Huth wrote: > On 17/09/15 15:09, 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 <da...@gibson.dropbear.id.au> > > --- > > include/exec/memory.h | 16 ++++++++++++++++ > > memory.c | 18 ++++++++++++++++++ > > 2 files changed, 34 insertions(+) > > > diff --git a/memory.c b/memory.c > > index 0d8b2d9..6b5a2f1 100644 > > --- a/memory.c > > +++ b/memory.c > > @@ -1403,6 +1403,24 @@ void > > memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n) > > notifier_list_add(&mr->iommu_notify, n); > > } > > > > +void memory_region_register_iommu_notifier_replay(MemoryRegion *mr, > > Notifier *n, > > + hwaddr granularity, bool > > is_write) > > granularity itself is not an address, but a size, isn't it? So using > "hwaddr" sounds wrong here.
As Laurent says, hwaddr is used for sizes through memory.c - I think
the rationale is that a size is a difference between two hwaddrs, so
should have the same type.
> > +{
> > + hwaddr addr;
>
> dma_addr_t ?
As mentioned in previus reply, I don't think so on consideration.
> > + IOMMUTLBEntry iotlb;
> > +
> > + memory_region_register_iommu_notifier(mr, n);
> > +
> > + for (addr = 0;
> > + int128_lt(int128_make64(addr), mr->size);
> > + addr += granularity) {
> > +
> > + iotlb = mr->iommu_ops->translate(mr, addr, is_write);
> > + if (iotlb.perm != IOMMU_NONE)
> > + n->notify(n, &iotlb);
>
> Missing curly braces.
Thanks, fixed.
>
> > + }
> > +}
> > +
>
> Thomas
>
--
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
pgp0a_gHpb_gO.pgp
Description: PGP signature
