Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

[Peter Xu]

On Wed, Sep 14, 2016 at 02:00:29PM +1000, David Gibson wrote:

[...]

> > > > > 
> > > > > > So
> > > > > > IIUC this is a question about "naming" but not the 
> > > > > > implementations...
> > > > > > I suppose it is really a matter of taste, and both work for me 
> > > > > > (either
> > > > > > INVALIDATION/CHANGE or UNMAP/MAP).
> > > > > 
> > > > > No.. it is a question of implementation.  My point is that I don't
> > > > > think the new permission is sufficient information to let you know if
> > > > > a notification is necessary.  You need to know if there was an
> > > > > existing mapping at that IOBA.
> > > > 
> > > > My understanding is that we don't need to know that. Because IIUC
> > > > there are only map_page() and unmap_page() in guest IOMMU driver
> > > > (please check dma_map_ops in kernel). There is no chance for anyone to
> > > > "change" the content of the mapping, unless it calls unmap_page() then
> > > > with a map_page(). In that case, we'll have two IOTLB invalidation
> > > > requests.
> > > 
> > > That's assuming a Linux guest using the current guest IOMMU model.
> > > 
> > > I don't think we do so in practice, but the PAPR hypercall interface
> > > allows in-place changing of a mapping.  The interface is just "set
> > > this IOPTE to this value".
> > 
> > I see. Even if so, QEMU IOMMU emulation codes can convert one CHANGE
> > request into UNMAP and a continuous MAP, right?
> 
> Yes, I guess so.  Why is that preferable to issuing a single
> notification to both "map" and "unmap" listeners though?

So I think we should be talking about the same thing here (please
correct me if I am wrong...). Please review v4 of this series and see
whether that works (I renamed CHANGE into MAP, so there will be
MAP/UNMAP, and I think things are clearer with it).

Thanks!

-- peterx