On Wed, Mar 23, 2022 at 02:04:46PM -0600, Alex Williamson wrote:
> On Wed, 23 Mar 2022 16:34:39 -0300
> Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
>
> > On Wed, Mar 23, 2022 at 01:10:38PM -0600, Alex Williamson wrote:
> > > On Fri, 18 Mar 2022 14:27:33 -0300
> > > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> > >
> > > > +static int conv_iommu_prot(u32 map_flags)
> > > > +{
> > > > + int iommu_prot;
> > > > +
> > > > + /*
> > > > + * We provide no manual cache coherency ioctls to userspace and most
> > > > + * architectures make the CPU ops for cache flushing privileged.
> > > > + * Therefore we require the underlying IOMMU to support CPU coherent
> > > > + * operation.
> > > > + */
> > > > + iommu_prot = IOMMU_CACHE;
> > >
> > > Where is this requirement enforced? AIUI we'd need to test
> > > IOMMU_CAP_CACHE_COHERENCY somewhere since functions like
> > > intel_iommu_map() simply drop the flag when not supported by HW.
> >
> > You are right, the correct thing to do is to fail device
> > binding/attach entirely if IOMMU_CAP_CACHE_COHERENCY is not there,
> > however we can't do that because Intel abuses the meaning of
> > IOMMU_CAP_CACHE_COHERENCY to mean their special no-snoop behavior is
> > supported.
> >
> > I want Intel to split out their special no-snoop from IOMMU_CACHE and
> > IOMMU_CAP_CACHE_COHERENCY so these things have a consisent meaning in
> > all iommu drivers. Once this is done vfio and iommufd should both
> > always set IOMMU_CACHE and refuse to work without
> > IOMMU_CAP_CACHE_COHERENCY. (unless someone knows of an !IOMMU_CACHE
> > arch that does in fact work today with vfio, somehow, but I don't..)
>
> IIRC, the DMAR on Intel CPUs dedicated to IGD was where we'd often see
> lack of snoop-control support, causing us to have mixed coherent and
> non-coherent domains. I don't recall if you go back far enough in VT-d
> history if the primary IOMMU might have lacked this support. So I
> think there are systems we care about with IOMMUs that can't enforce
> DMA coherency.
>
> As it is today, if the IOMMU reports IOMMU_CAP_CACHE_COHERENCY and all
> mappings make use of IOMMU_CACHE, then all DMA is coherent. Are you
> suggesting IOMMU_CAP_CACHE_COHERENCY should indicate that all mappings
> are coherent regardless of mapping protection flags? What's the point
> of IOMMU_CACHE at that point?
IOMMU_CAP_CACHE_COHERENCY should return to what it was before Intel's
change.
It only means normal DMAs issued in a normal way are coherent with the
CPU and do not require special cache flushing instructions. ie DMA
issued by a kernel driver using the DMA API.
It does not mean that non-coherence DMA does not exist, or that
platform or device specific ways to trigger non-coherence are blocked.
Stated another way, any platform that wires dev_is_dma_coherent() to
true, like all x86 does, must support IOMMU_CACHE and report
IOMMU_CAP_CACHE_COHERENCY for every iommu_domain the platform
supports. The platform obviously declares it support this in order to
support the in-kernel DMA API.
Thus, a new cap indicating that 'all dma is coherent' or 'no-snoop
blocking' should be created to cover Intel's special need. From what I
know it is only implemented in the Intel driver and apparently only
for some IOMMUs connected to IGD.
> > Yes, it was missed in the notes for vfio compat that Intel no-snoop is
> > not working currently, I fixed it.
>
> Right, I see it in the comments relative to extensions, but missed in
> the commit log. Thanks,
Oh good, I remembered it was someplace..
Jason
