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?
> I added a fixme about this.
>
> > This also seems like an issue relative to vfio compatibility that I
> > don't see mentioned in that patch. Thanks,
>
> 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,
Alex
