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

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

On 2025/2/28 16:30, Vasant Hegde wrote:
> Hi Yi,
> 
> 
> On 2/28/2025 1:13 PM, Yi Liu wrote:
>> On 2025/2/28 14:32, Tian, Kevin wrote:
>>>> From: Vasant Hegde <vasant.hegde@amd.com>
>>>> Sent: Thursday, February 27, 2025 11:28 PM
>>>>
>>>> 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?
>>>
>>> yes
>>
>> in case this is the direction. This might be a per iommu_group knob. do we
> 
> 
>> want a default value per some info that can be probed by kernel?
> 
> I prefer kernel probe/setting default values so that we can enforce HW
> restrictions (like SNP, untrusted device etc) and then....

I got untrusted device. How is SNP probed? Is there any bit for it?

> 
>> or we just fully rely on admin to program it?
> 
> have a admin option to make choice (I prefer global enforcement option). As
> Kevin mentioned for fine grained control, we can have per group know as well.
> (may be we can hook it to /sys/kernel/iommu_groups/<x>/<ats knob> ?
yeah, this suit my thought. ATS only exists when iommu is enabled. Hence
Secure ATS. iommu_group is the granular where iommu can isolate devices. So
per iommu group knob sounds reasonable.

-- 
Regards,
Yi Liu