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
