On Mon, Mar 28, 2022 at 11:17:23AM -0600, Alex Williamson wrote: > On Thu, 24 Mar 2022 10:46:22 -0300 > Jason Gunthorpe <j...@nvidia.com> wrote: > > > On Thu, Mar 24, 2022 at 07:25:03AM +0000, Tian, Kevin wrote: > > > > > Based on that here is a quick tweak of the force-snoop part (not > > > compiled). > > > > I liked your previous idea better, that IOMMU_CAP_CACHE_COHERENCY > > started out OK but got weird. So lets fix it back to the way it was. > > > > How about this: > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjgunthorpe%2Flinux%2Fcommits%2Fintel_no_snoop&data=04%7C01%7Cjgg%40nvidia.com%7C9d34426f1c1646af43a608da10ded6b5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637840846514240225%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%2ByHWyE8Yxcwxe8r8LoMQD9tPh5%2FZPaGfNsUkMlpRfWM%3D&reserved=0 > > > > b11c19a4b34c2a iommu: Move the Intel no-snoop control off of IOMMU_CACHE > > 5263947f9d5f36 vfio: Require that device support DMA cache coherence > > I have some issues with the argument here: > > This will block device/platform/iommu combinations that do not > support cache coherent DMA - but these never worked anyhow as VFIO > did not expose any interface to perform the required cache > maintenance operations. > > VFIO never intended to provide such operations, it only tried to make > the coherence of the device visible to userspace such that it can > perform operations via other means, for example via KVM. The "never > worked" statement here seems false.
VFIO is generic. I expect if DPDK connects to VFIO then it will work properly. That is definitely not the case today when dev_is_dma_coherent() is false. This is what the paragraph is talking about. Remember, x86 wires dev_is_dma_coherent() to true, so this above remark is not related to anything about x86. We don't have a way in VFIO to negotiate that 'vfio can only be used with kvm' so I hope no cases like that really do exist :( Do you know of any? > Commit b11c19a4b34c2a also appears to be a behavioral change. AIUI > vfio_domain.enforce_cache_coherency would only be set on Intel VT-d > where snoop-control is supported, this translates to KVM emulating > coherency instructions everywhere except VT-d w/ snoop-control. It seems so. > My understanding of AMD-Vi is that no-snoop TLPs are always coherent, so > this would trigger unnecessary wbinvd emulation on those platforms. I look in the AMD manual and it looks like it works the same as intel with a dedicated IOPTE bit: #define IOMMU_PTE_FC (1ULL << 60) https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf Pg 79: FC: Force Coherent. Software uses the FC bit in the PTE to indicate the source of the upstream coherent attribute state for an untranslated DMA transaction.1 = the IOMMU sets the coherent attribute state in the upstream request. 0 = the IOMMU passes on the coherent attribute state from the originating request. Device internal address/page table translations are considered "untranslated accesses" by IOMMU.The FC state is returned in the ATS response to the device endpoint via the state of the (N)oSnoop bit. So, currently AMD and Intel have exactly the same HW feature with a different kAPI.. I would say it is wrong that AMD creates kernel owned domains for the DMA-API to use that do not support snoop. > don't know if other archs need similar, but it seems we're changing > polarity wrt no-snoop TLPs from "everyone is coherent except this case > on Intel" to "everyone is non-coherent except this opposite case on > Intel". Yes. We should not assume no-snoop blocking is a HW feature without explicit knowledge that it is. >From a kAPI compat perspective IOMMU_CAP_CACHE_COHERENCY only has two impacts: - Only on x86 arch it controls kvm_arch_register_noncoherent_dma() - It triggers IOMMU_CACHE If we look at the list of places where IOMMU_CAP_CACHE_COHERENCY is set: drivers/iommu/intel/iommu.c Must have IOMMU_CACHE set/clear to control no-snoop blocking drivers/iommu/amd/iommu.c Always sets its no-snoop block, inconsistent with Intel drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c drivers/iommu/arm/arm-smmu/arm-smmu.c drivers/iommu/arm/arm-smmu/qcom_iommu.c Must have IOMMU_CACHE set, ARM arch has no kvm_arch_register_noncoherent_dma() From what I could tell in the manuals and the prior discussion SMMU doesn't block no-snoop. ie ARM lies about IOMMU_CAP_CACHE_COHERENCY because it needs IOMM_CACHE set to work. drivers/iommu/fsl_pamu_domain.c drivers/iommu/s390-iommu.c Ignore IOMM_CACHE, arch has no kvm_arch_register_noncoherent_dma() No idea if the HW blocks no-snoop or not, but it doesn't matter. So other than AMD, it is OK to change the sense and makes it clearer for future driver authors what they are expected to do with this. Thanks, Jason _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
