Re: [PATCH] On unmap, flush IOMMU TLB and return correct size

[David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2342 bytes --]

On Wed, 2013-09-25 at 15:46 -0600, Alex Williamson wrote:
> On Wed, 2013-09-25 at 21:11 +0100, David Woodhouse wrote:
> > I wouldn't bother to go looking for opportunities to use super pages if
> > we remove the last non-SP-capable IOMMU from the domain.
> 
> I predict bugs getting filed if a guest sees a performance hit after
> adding a device that is not restored when the device is removed.  If
> only we could assume a similar feature set among IOMMUs in a system.

Ok, fine. As long as you don't have *too* much mapped, that shouldn't
suck too much.

> > > > FWIW we currently screw up the handling of cache-coherent vs.
> > > > non-coherent page tables too. That one wants a wbinvd somewhere when we
> > > > add a non-coherent IOMMU to the domain.
> > > 
> > > You're not selling the "trust the IOMMU driver" story very well here.
> > > Can we assume that the IOMMU_CACHE flag (SNP) is ignored appropriately
> > > by non-coherent IOMMUs?  Is there any downside to ignoring it and always
> > > setting SNP in the IOMMU page tables?  AMD IOMMU ignores it, but it's
> > > also always cache coherent.  Thanks,
> > 
> > SNP is a separate issue. I'm speaking of cache coherency of the hardware
> > page table walk — the feature bit that all the horrid clflush calls are
> > predicated on.
> > 
> > Again, this is just a bug. We *should* be getting this right, but don't
> > yet.
> 
> And for DMA_PTE_SNP?  intel_iommu_map() won't let us set this bit if the
> domain contains a hardware unit that doesn't support ecap.SC, but it
> also doesn't update existing mappings.  Barring hardware bugs, it seems
> much easier to unconditionally set DMA_PTE_SNP but still advertise
> IOMMU_CAP_CACHE_COHERENCY based on the composition of the domain.
> Otherwise we have to reject adding devices to a domain that change the
> coherency once DMA mappings are in play or never set IOMMU_CACHE and
> advertise to KVM that the domain is always non-coherent.  Thanks,

It's not that the domain is non-coherent, is it? The SNP bit allows you
to *force* a PCIe DMA request to snoop the CPU cache, even if it didn't
*request* that.

If your device is actually *trying* to do cache-coherent (i.e. snooping)
DMA, surely that works regardless of the DMA_PTE_SNP setting?

Or am I missing something?


-- 
dwmw2

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]