Re: [PATCH] iommu: Allow ATS to work on VFs when the PF uses IDENTITY

[Yi Liu <yi.l.liu@intel.com>]

On 2024/8/9 21:28, Jason Gunthorpe wrote:
> On Fri, Aug 09, 2024 at 02:36:14AM +0000, Tian, Kevin wrote:
>>> From: Jason Gunthorpe <jgg@nvidia.com>
>>> Sent: Thursday, August 8, 2024 2:19 AM
>>>
>>> PCI ATS has a global Smallest Translation Unit field that is located in
>>> the PF but shared by all of the VFs.
>>>
>>> The expectation is that the STU will be set to the root port's global STU
>>> capability which is driven by the IO page table configuration of the iommu
>>> HW. Today it becomes set when the iommu driver first enables ATS.
>>>
>>> Thus, to enable ATS on the VF, the PF must have already had the correct
>>> STU programmed, even if ATS is off on the PF.
>>>
>>> Unfortunately the PF only programs the STU when the PF enables ATS. The
>>> iommu drivers tend to leave ATS disabled when IDENTITY translation is
>>> being used.

I think this is the common practice as it is not necessary to enable ATS
since iommu is passthrough mode. :)

>>
>> Is there more context on this?
> 
> How do you mean?
> 
>> Looking at intel-iommu driver ATS is disabled for IDENETITY when
>> the iommu is in legacy mode:
>>
>> dmar_domain_attach_device()
>> {
>> 	...
>> 	if (sm_supported(info->iommu) || !domain_type_is_si(info->domain))
>> 		iommu_enable_pci_caps(info);
>> 	...
>> }
>>
>> But this follows what VT-d spec says (section 9.3):
>>
>> TT: Translate Type
>> 10b: Untranslated requests are processed as pass-through. The SSPTPTR
>> field is ignored by hardware. Translated and Translation Requests are
>> blocked.
> 
> Yes, HW like this is exactly the problem, it ends up not enabling ATS
> on the PF and then we don't have the STU programmed so the VF is
> effectively disabled too.
> 
> Ideally iommus would continue to work with translated requests when
> ATS is enabled. 

As Kevin's pasting, the Translated requests will be blocked. So it does
not work. :(

> Not supporting this configuration creates a nasty
> problem for devices that are using PASID.
>
> The PASID may require ATS to be enabled (ie SVA), but the RID may be
> IDENTITY for performance. The poor device has no idea it is not
> allowed to use ATS on the RID side :(

If this is the only problematic case, the intel iommu driver in this
patch could check the scalable mode before enabling ATS in the
probe_device() op. In this way, the legacy mode iommu would keep the old
ATS enable policy.

Seems an alternative is to use paging domain for IDENTITY. This means
the TT would not be 10b, hence it would work with ATS enabled.

-- 
Regards,
Yi Liu