Re: [PATCH] iommu/amd: Add Secure ATS support

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

On 3/4/25 02:38, Jason Gunthorpe wrote:
> On Sun, Mar 02, 2025 at 04:10:46PM +0800, Yi Liu wrote:
>> I'm also thinking about the impact of such a knob. How should we define
>> this knob. Should we allow it to disable/enable ATS? Especially in runtime.
>> e.g. if the device is identified to be ATS untrusted by the admin, while
>> the hw does not support SATS, it seems reasonable to disable ATS for
>> such device. This might change the driver behavior on ATS enabling. Intel
>> iommu driver enables ATS as long as it's available in the probe_device()
>> op. What about AMD and ARM?
> ARM enables ATS when a PAGING translation is set on the RID or any
> PASID is used. Otherwise it is off, eg for RID = IDENTITY
> 
> This seems like a reasonable thing to do to me..
> 
>> On the other hand, we may just define the knob as sats required or not.
>> This is just a knob to let kernel know if the iommu driver needs to enable
>> sats or not. While leave the ATS enabling policy unchanged. This means
>> we need to probe the sats capability of iommu hw before creating the knob
>> in sysfs.
> If we add some ATS control knob then maybe it should be able to
> disable/enable normal ATS as well?
> 
>   never
>   always
>   paging-only
>   secure-always
>   secure-paging-only
> 
> ?

My two cents' worth.

We could separate ATS security and enablement. ATS security is a
security policy that should be opt-in by the user through something like
sysfs nodes. We can follow what we have done for the default domain
type: identity domain (non-secure) and paging domain (secure with some
extra performance overhead). Users can specify the static default domain
type and tweak it in a per-iommu_group manner through sysfs nodes.

For ATS security, we can probably define two levels:

- Relaxed ATS: ATS could be enabled as long as the IOMMU supports the
   ATS service and the device supports ATC. This matches what most IOMMU
   drivers currently do.

- Secure ATS: ATS could only be enabled if the platform provides
   enumerable capabilities that can disallow arbitrary translated DMA
   requests. We need to let the user know that once ATS security is set
   to this level, some features like host SVA won't be supported
   currently.

For ATS enablement, I believe we have already reached some agreement
that ATS enablement is in an on-demand manner. ATS is enabled when the
first domain that requires ATS is attached and disabled when the last
domain is detached. That matches what we are doing for PRI.

When a domain attachment triggers ATS to be on, there might be some
cases:

- ATS does not impact functionality. For example, ATS could be enabled
   for the DMA domain for better performance. In this case, it's a
   successful case if the device supports ATS but the platform can't
   provide the secure ATS that is demanded by the user's ATS security
   level. ATS will not be enabled but attach returns success.

- ATS impacts functionality. For example, the domain requires PRI. In
   this case, it's a failure case when the device does not support ATS or
   the ATS security is insufficient.

- ATS compatibility should also be checked in domain attach path. If ATS
   policy between attaching domain and the device does not match, an
   -EINVAL should be returned to inform the caller that "domain is not
   compatible, suggest to allocate a dedicated domain for this device".

Thanks,
baolu