Re: [Qemu-devel] [RFC 3/3] memory: introduce IOMMU_NOTIFIER_USER_[UN]SET

[Peter Xu]

On Tue, Jun 05, 2018 at 02:54:18PM +0100, Peter Maydell wrote:
> On 5 June 2018 at 14:19, Peter Xu <address@hidden> wrote:
> > Add two more IOMMU notifier flags to selectively choose whether the
> > notifier would like to listen to an event that with USER bit set/unset.
> > Note that all existing notifiers should always been registered with both
> > of the flags set to make sure they'll receive the notification no matter
> > whether the USER bit is set or unset in the attributes.  To simplify
> > this procedure, some new macros are defined.  The old UNMAP-only
> > notifiers should now be registered with IOMMU_NOTIFIER_UNMAP_ALL
> > flags (rather than the old IOMMU_NOTIFIER_UNMAP flag), while the old
> > MAP+UNMAP case can keep to use the IOMMU_NOTIFIER_ALL flag.
> >
> > Now if a new notifier would like to register to only UNMAP notifications
> > with USER bit set, it should register with below flag:
> >
> >   IOMMU_NOTIFIER_UNMAP | IOMMU_NOTIFIER_USER_SET
> >
> > Then when we want to notify a DMA invalidation (we call it IOMMUTLBEntry
> > in QEMU), we should do this:
> >
> >   IOMMUTLBEntry entry;
> >   ... (set up the fields)
> >   entry.perm = IOMMU_NONE;
> >   entry.attrs.user = 1;
> >   memory_region_notify_iommu(mr, &entry);
> 
> I don't think this works, because there is not a single unique
> MemTxAttrs for a particular invalidation event. Invalidates
> will affect classes of transaction attributes. For instance
> in the MPC, changing the lookup table can invalidate cached
> IOMMU information for any translation that was done either
> with attrs.secure = 1, or for translations with attrs.unspecified = 1.
> 
> In general, at the point where the IOMMU calls notifiers it
> won't have a single MemTxAttrs it can use, so you don't want
> to put an attrs into the IOMMUTLBEntry.

So we have a requirement that we want to send an invalidation for
either (1) unspecified, or (2) secure=1.  We can't do that with a
single MemTxAttrs.

Could we just notify twice?  One with unspecified=1 and one with
secure=1?  Is this (reduce the calls to notify functions) the major
reason we introduce this IOMMU index idea into the memory API (besides
"avoid possible programming error" you mentioned in IRC discussion)?

Again, I think the more critical question would be whether we can pass
in MemTxAttrs into translate(), and whether we can avoid introducing
this IOMMU index idea into the memory API layer... For example, would
it add complexity to other architectures without much benefit?

Thanks,

-- 
Peter Xu