From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50257) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bgpPx-00033n-CC for qemu-devel@nongnu.org; Mon, 05 Sep 2016 04:38:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bgpPt-000530-7y for qemu-devel@nongnu.org; Mon, 05 Sep 2016 04:38:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58898) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bgpPt-00052i-30 for qemu-devel@nongnu.org; Mon, 05 Sep 2016 04:38:09 -0400 Date: Mon, 5 Sep 2016 16:38:04 +0800 From: Peter Xu Message-ID: <20160905083804.GB7761@pxdev.xzpeter.org> References: <1473060081-17835-1-git-send-email-peterx@redhat.com> <1473060081-17835-3-git-send-email-peterx@redhat.com> <2112298c-fe2a-c74f-7a68-a92625cd3533@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2112298c-fe2a-c74f-7a68-a92625cd3533@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, mst@redhat.com, jasowang@redhat.com, vkaplans@redhat.com, alex.williamson@redhat.com, wexu@redhat.com, cornelia.huck@de.ibm.com, dgibson@redhat.com 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