Re: [PATCH v2 2/4] vfio: Move the Intel no-snoop control off of IOMMU_CACHE

Lu Baolu
2022-04-09T12:51:00Z

On 2022/4/8 16:16, Tian, Kevin wrote:
>> From: Jason Gunthorpe <jgg@nvidia.com>
>> Sent: Thursday, April 7, 2022 11:24 PM
>>
>> IOMMU_CACHE means "normal DMA to this iommu_domain's IOVA should
>> be cache
>> coherent" and is used by the DMA API. The definition allows for special
>> non-coherent DMA to exist - ie processing of the no-snoop flag in PCIe
>> TLPs - so long as this behavior is opt-in by the device driver.
>>
>> The flag is mainly used by the DMA API to synchronize the IOMMU setting
>> with the expected cache behavior of the DMA master. eg based on
>> dev_is_dma_coherent() in some case.
>>
>> For Intel IOMMU IOMMU_CACHE was redefined to mean 'force all DMA to
>> be
>> cache coherent' which has the practical effect of causing the IOMMU to
>> ignore the no-snoop bit in a PCIe TLP.
>>
>> x86 platforms are always IOMMU_CACHE, so Intel should ignore this flag.
>>
>> Instead use the new domain op enforce_cache_coherency() which causes
>> every
>> IOPTE created in the domain to have the no-snoop blocking behavior.
>>
>> Reconfigure VFIO to always use IOMMU_CACHE and call
>> enforce_cache_coherency() to operate the special Intel behavior.
>>
>> Remove the IOMMU_CACHE test from Intel IOMMU.
>>
>> Ultimately VFIO plumbs the result of enforce_cache_coherency() back into
>> the x86 platform code through kvm_arch_register_noncoherent_dma()
>> which
>> controls if the WBINVD instruction is available in the guest. No other
>> arch implements kvm_arch_register_noncoherent_dma().
>>
>> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>
> btw as discussed in last version it is not necessarily to recalculate
> snoop control globally with this new approach. Will follow up to
> clean it up after this series is merged.

Agreed. But it also requires the enforce_cache_coherency() to be called
only after domain being attached to a device just as VFIO is doing.

Anyway, for this change in iommu/vt-d:

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu