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

[Lu Baolu <baolu.lu@linux.intel.com>]

On 2022/4/12 15:44, Tian, Kevin wrote:
>> From: Lu Baolu<baolu.lu@linux.intel.com>
>> Sent: Saturday, April 9, 2022 8:51 PM
>>
>> 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.
> that actually makes sense, right? w/o device attached it's pointless to
> call that interface on a domain...

Agreed. Return -EOPNOTSUPP or -EINVAL to tell the caller that this
operation is invalid before any device attachment.

Best regards,
baolu