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

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

Hi Keven,


On 2/28/2025 12:02 PM, 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

That means root user can change it during runtime?  And we have to expose
supported combination per group (like noats, ats, secure ATS).
Also runtime we have to update DTE (and intel case create new domain?) if we
switch from ats to secure ATS.

> 
>>
>>>
>>>   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?
> 
> It's not conflicting. When enabling a trusted device the driver can detect
> whether sats has been opted in by admin for the device. If not fail the
> request. If yes, turn on secure ATS as part of enforcement and return
> -EBUSY if the admin attempts to disable it in-fly.

Sorry. I am not sure if I get everything. When you say "admin" are you referring
to system admin to make choice?



> 
>> If yes,
>> may be have an domain ops similar to def_domain_type() ?
> 
> the domain for trusted I/O will have a special flag.

Right. we can pass special flag to domain allocation. But how does core layer
know's about HW requirement?

> 
>>
>>   - For untrusted devices,  by default enforce Secure ATS (may be a domain
>> flag)
> 
> or the admin may prefer to disabling ats.

Right.

> 
>>
>>   - Have a command line option, so that if administrator wants to enforce
>> secure
>> ATS for all devices, he can do that.
>>
> 
> whether to enable sats depends on various factors:
> 
> - is the device trusted?
> - does the overhead of enabling sats break the perf requirement of
>   the existing ats scenario?
> - different security policy based on domain types, e.g. only requiring
>   sats for iommufd but not default DMA API domain

Also HW restriction (Booted AMD system with SNP enabled).

> - ...
> 
> A global cmdline option for all devices/domains is hard to meet those
> requirements.

We will have default policy and based on global option we can do enforcement.
So that user wants global level enforcement he can do it.

-Vasant