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

[Vasant Hegde <vasant.hegde@amd.com>]

Hi Kevin,


On 2/26/2025 12:35 PM, Tian, Kevin wrote:
>> From: Tian, Kevin
>> Sent: Wednesday, February 26, 2025 10:51 AM
>>
>>> From: Jason Gunthorpe <jgg@ziepe.ca>
>>> Sent: Wednesday, February 26, 2025 9:18 AM
>>>
>>> On Wed, Feb 26, 2025 at 01:12:38AM +0000, Tian, Kevin wrote:
>>>> If above understanding is true, my preference is to support a sats flag
>>>> in domain alloc (nested_parent only and PCI/CXL.io only, as the start). For
>>>> AMD it turns on the sats bit in the DTE. for Intel/ARM the permission
>>>> structure is created and updated according to the map/unmap calls
>>>> on the parent S2.
>>>
>>> They are functionally different things, and have different
>>> requirements on the PCI topologies supported (eg CXL.cache vs no
>>> CXL.cache)
>>>
>>> I think they need to be different flags
>>>
>>
>> Not exactly. They are functionally different but serving the same purpose
>> to the user. From user p.o.v. it's sufficient to have a general flag for sats
>> when allocating a domain. The underlying driver decides whether such
>> flag is supported based on the domain type and the associated device,
>> just like checks on other existing flags.
>>
> 
> Chatted with Yi offline. Having untrusted user control a security 
> feature doesn't make much sense. Probably what we really require
> is:
> 
> - IOMMU core exposes a separate sats knob per probed device, 
>   allowing the administrator to manage the sats policy which could
>   be no ats, unsecure ats and secure ats (might be further set per
>   domain type in case the hw doesn't support sats for all types or
>   the driver doesn't support all hw-supported types in one batch).

When you say "sats knob", you mean sysfs interface?

> 
>   It's also checked against the device type e.x. CXL.cache.
> 
> - Then when enabling ats for a given domain type and device, follow
>   the policy set by administrator: ats disabled, ats enabled as today,
>   ats enabled with hw enforced security;
> 
> - Optionally iommufd may support a domain alloc flag for ats,
>   which alone is a performance or functional dependency (by PRI), 
>   hence better to let the user opt. If opted, again follow the
>   admin policy whether it's allowed and requires sats.
> 
> Is this way reasonable?
> 

What I am really thinking is :
  - If HW has a enforcement required (like AMD SNP case where Secure ATS is
must), enable secure ATS. But do we need some way to inform core layer? If yes,
may be have an domain ops similar to def_domain_type() ?

  - For untrusted devices,  by default enforce Secure ATS (may be a domain flag)

  - Have a command line option, so that if administrator wants to enforce secure
ATS for all devices, he can do that.

-Vasant