Re: [Qemu-devel] [PATCH v2 00/10] intel-iommu: nested vIOMMU, cleanups, bug fixes

[Peter Xu <peterx@redhat.com>]

On Wed, May 16, 2018 at 09:39:41PM -0600, Alex Williamson wrote:
> On Thu, 17 May 2018 10:45:44 +0800
> Peter Xu <peterx@redhat.com> wrote:
> 
> > On Wed, May 16, 2018 at 09:57:40PM +0800, Jason Wang wrote:
> > > 
> > > 
> > > On 2018年05月16日 14:30, Peter Xu wrote:  
> > > > On Fri, May 04, 2018 at 11:08:01AM +0800, Peter Xu wrote:  
> > > > > v2:
> > > > > - fix patchew code style warnings
> > > > > - interval tree: postpone malloc when inserting; simplify node remove
> > > > >    a bit where proper [Jason]
> > > > > - fix up comment and commit message for iommu lock patch [Kevin]
> > > > > - protect context cache too using the iommu lock [Kevin, Jason]
> > > > > - add vast comment in patch 8 to explain the modify-PTE problem
> > > > >    [Jason, Kevin]  
> > > > We can hold a bit on reviewing this series.  Jintack reported a scp
> > > > DMAR issue that might happen even with L1 guest with this series, and
> > > > the scp can stall after copied tens or hundreds of MBs randomly.  I'm
> > > > still investigating the problem.  This problem should be related to
> > > > deferred flushing of VT-d kernel driver, since the problem will go
> > > > away if we use "intel_iommu=on,strict".  However I'm still trying to
> > > > figure out what's the thing behind the scene even with that deferred
> > > > flushing feature.  
> > > 
> > > I vaguely remember recent upstream vfio support delayed flush, maybe it's
> > > related.  
> > 
> > I'm a bit confused on why vfio is related to the deferred flushing.
> > Could you provide a pointer for this?
> 
> Perhaps referring to this:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bd06f5a486c06023a618a86e8153b91d26f75f4
> 
> Rather than calling iommu_unmap() for each chunk of a mapping we'll
> make multiple calls to iommu_unmap_fast() and flush with
> iommu_tlb_sync() to defer and batch the hardware flushing.  Thanks,

Thanks for the link!

It seems to be a good performance enhancement for vfio, but it might
not related to current problem.

My latest clue shows that the issue is possibly caused by replaying
each IOMMU region twice on a single DSI.  For example, when we get one
DSI we'll actually call vtd_page_walk() twice (the IOMMU region is
splitted by the MSI region 0xfeeXXXXX, so we have two notifiers for
each vfio-pci device now... and memory_region_iommu_replay_all will
call vtd_page_walk twice). So that confused the IOVA tree a bit. I'll
verify that and see how I can fix it up.

(PS: it seems that in above patch unmapped_region_cnt is not needed in
 vfio_unmap_unpin considering that we already have
 unmapped_region_list there?  If that's correct, then we can remove
 all the references too, e.g. we don't need to pass in unmapped_cnt
 into unmap_unpin_fast as well.)

Thanks,

-- 
Peter Xu