On Mon, Sep 05, 2016 at 10:04:42AM +0200, Paolo Bonzini wrote:
>
>
> On 05/09/2016 09:21, Peter Xu wrote:
> > void memory_region_notify_iommu(MemoryRegion *mr,
> > - IOMMUTLBEntry entry)
> > + IOMMUTLBEntry entry, IOMMUAccessFlags flag)
> > {
> > assert(memory_region_is_iommu(mr));
> > + assert(flag == mr->iommu_notify_flag);
> > notifier_list_notify(&mr->iommu_notify, &entry);
> > }
>
> Shouldn't it be possible to have IOMMU_RW and IOMMU_NONE on the same
> IOMMU, if the IOMMU supports IOMMU_RW at all?
Yeah, this is a good point...
If we see IOMMU_NONE as a subset of IOMMU_RW, we should allow notify
IOMMU_NONE even if the cached flag is IOMMU_RW.
However in this patch I was not meant to do that. I made it an
exclusive flag to identify two different use cases. I don't know
whether this is good, but at least for Intel IOMMU's current use case,
these two types should be totally isolated from each other:
- IOMMU_NONE notification is used by future DMAR-enabled vhost, it
should only be notified with device-IOTLB invalidations, this will
only require "Device IOTLB" capability for Intel IOMMUs, and be
notified in Device IOTLB invalidation handlers.
- IOMMU_RW notifications should only be used for vfio-pci, notified
with IOTLB invalidations. This will only require Cache Mode (CM=1)
capability, and will be notified in common IOTLB invalidations (no
matter whether it's an cache invalidation or not, we will all use
IOMMU_RW flag for this kind of notifies).
Maybe here naming the flags as IOMMU_{RW_NONE} is a little bit
confusing (just to leverage existing access flags), but what I was
trying to do is to make the two things not overlapped at all, since I
didn't find a mixture use case.
Thanks,
-- peterx
