On Fri, Feb 10, 2017 at 12:12:22PM +1100, David Gibson wrote: > On Tue, Feb 07, 2017 at 04:28:04PM +0800, Peter Xu wrote: > > A cleanup for vfio_iommu_map_notify(). Now we will fetch vaddr even if > > the operation is unmap, but it won't hurt much. > > > > One thing to mention is that we need the RCU read lock to protect the > > whole translation and map/unmap procedure. > > > > Acked-by: Alex Williamson <alex.william...@redhat.com> > > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> > > Signed-off-by: Peter Xu <pet...@redhat.com> > > So, I know I reviewed this already, but looking again I'm confused. > > I'm not sure how the original code ever worked: if this is an unmap > (perm == IOMMU_NONE), then I wouldn't even expect > iotlb->translated_addr to have a valid value, but we're passing it to > address_space_translate() and failing if it it doesn't give us > sensible results.
Hmm, right. Looks like it is just because we have accidentally inited iotlb->translated_addr in all the callers of memory_region_notify_iommu (one is put_tce_emu(), the other one is rpcit_service_call()). If so, patch 3 (maybe, along with this one) would be more essential imho to make sure we don't have such an assumption. Thanks, -- peterx