Re: [PATCH 2/8] iommu/arm-smmu-v3: Use S2FWB when available

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Aug 21, 2024 at 09:53:33AM +0000, Mostafa Saleh wrote:
> > Oh, from that perspective yes, but the entire point of S2FWB is that
> > VM's can not create non-coherent access so it is a bit nonsense to ask
> > for both S2FWB and try to assign a non-DMA coherent device.
> 
> Yes, but KVM sets FWB unconditionally and would use cacheable mapping
> for stage-2, and I expect the same for the nested SMMU.

Yes, you'd need some kind of handshake like Intel built for their GPU
cache incoherence to turn that off.

> > > What I mean is the master itself not the SMMU (the SID basically),
> > > so in that case the STE shouldn’t have FWB enabled.
> > 
> > That doesn't matter, those cases will not pass in IOMMU_CACHE and they
> > will work fine with S2FWB turned on.
> 
> But that won’t be the case in nested? Otherwise why we use FWB in the
> first place.

Right, without KVM support for guest cachability selection and cache
flushing in VFIO, is infeasible to allow non-coherent devices. It is a
medium sized problem if someone wants to tackle it.

> Maybe the SMMUv3 .capable, should be changed to check if the device is
> coherent (instead of using dev_is_dma_coherent, it can use lower level
> functions from the supported buses)

That would be the fix I expect. Either SMMUv3 does it, or the core
code adds it on top in the .capable wrapper. It makes sense to me that
the iommu driver should be aware of per-master coherence capability.

> Also, I think supporting IOMMU_CACHE is not enough, as the SMMU can
> support it but the device is still not coherent.

IOMMU_CACHE is defined as requiring no cache maintenance on that
memory.

If specific devices can't guarentee that then IOMMU_CACHE should not
be used on those devices and IOMMU_CAP_CACHE_COHERENCY for that device
should be false.

That is what I mean by support.

Anyhow, I'm going to continue to leave this problem alone for
nesting. Nothing gets worse by adding nesting on top of this. Even if
we wrongly permit VFIO to open non-coherent devices they won't
actually work correctly (VFIO forces IOMMU_CACHE and S2FWB). Most
likely anything trying to use them will just crash/malfunction due to
missing cache flushing.

Jason