[PATCH] iommu/amd: Add Secure ATS support

[PATCH] iommu/amd: Add Secure ATS support

[Vasant Hegde]

AMD IOMMU supports processing ATS requests as Secure ATS requests when IOTLB
is supported and enabled (See section "2.11 Secure ATS Supports" in AMD IOMMU
spec [1] for more detail).

With Secure ATS, for ATS requests IOMMU return GPA to device instead of SPA.
When receiving Translated DMA from a device the IOMMU performs in-line
GPA to SPA translation. This ensures that any malicious devices or
vulnerable IOTLB implementations do not have access to the full SPA
space even if they can generate Translated DMA with arbitrary addresses.
This provides security against untrusted devices.

Introduce new command line parameter (amd_iommu=sats) to enable Secure
ATS. This can be used to provide security against untrusted device ATS.
Also enable Secure ATS when SNP is enabled.

[1] https://www.amd.com/en/support/tech-docs/amd-io-virtualization-technology-iommu-specification

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h       |  1 +
 drivers/iommu/amd/amd_iommu_types.h |  2 ++
 drivers/iommu/amd/init.c            | 17 +++++++++++++++++
 drivers/iommu/amd/iommu.c           | 11 ++++++++++-
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 68debf5ee2d7..b9a6f5844e50 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -43,6 +43,7 @@ extern int amd_iommu_guest_ir;
 extern enum protection_domain_mode amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 extern unsigned long amd_iommu_pgsize_bitmap;
+extern bool amd_iommu_sats;
 
 /* Protection domain ops */
 void amd_iommu_init_identity_domain(void);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 23caea22f8dc..bffb6ebb3d3e 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -99,6 +99,7 @@
 #define FEATURE_GATS		GENMASK_ULL(13, 12)
 #define FEATURE_GLX		GENMASK_ULL(15, 14)
 #define FEATURE_GAM_VAPIC	BIT_ULL(21)
+#define FEATURE_SATSSUP		BIT_ULL(31)
 #define FEATURE_PASMAX		GENMASK_ULL(36, 32)
 #define FEATURE_GIOSUP		BIT_ULL(48)
 #define FEATURE_HASUP		BIT_ULL(49)
@@ -417,6 +418,7 @@
 #define DTE_FLAG_IOTLB	BIT_ULL(32)
 #define DTE_FLAG_MASK	(0x3ffULL << 32)
 #define DEV_DOMID_MASK	0xffffULL
+#define DTE_FLAG_SATS	BIT_ULL(42)
 
 #define DTE_GCR3_14_12	GENMASK_ULL(60, 58)
 #define DTE_GCR3_30_15	GENMASK_ULL(31, 16)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index cb536d372b12..9b28c1fd32bf 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -156,6 +156,9 @@ enum protection_domain_mode amd_iommu_pgtable = PD_MODE_V1;
 /* Guest page table level */
 int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
 
+/* Secure ATS support */
+bool amd_iommu_sats;
+
 int amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_VAPIC;
 static int amd_iommu_xt_mode = IRQ_REMAP_XAPIC_MODE;
 
@@ -3093,6 +3096,15 @@ static int __init early_amd_iommu_init(void)
 		}
 	}
 
+	/* Secure ATS supported when DTE[mode] != 0 */
+	if (amd_iommu_sats) {
+		if (!check_feature(FEATURE_SATSSUP) ||
+		    (amd_iommu_pgtable != PD_MODE_V1)) {
+			pr_info("Secure ATS is not supported");
+			amd_iommu_sats = false;
+		}
+	}
+
 	/* Disable any previously enabled IOMMUs */
 	if (!is_kdump_kernel() || amd_iommu_disabled)
 		disable_iommus();
@@ -3231,6 +3243,9 @@ static __init void iommu_snp_enable(void)
 		goto disable_snp;
 	}
 
+	if (check_feature(FEATURE_SATSSUP))
+		amd_iommu_sats = true;
+
 	pr_info("IOMMU SNP support enabled.\n");
 	return;
 
@@ -3526,6 +3541,8 @@ static int __init parse_amd_iommu_options(char *str)
 		} else if (strncmp(str, "v2_pgsizes_only", 15) == 0) {
 			pr_info("Restricting V1 page-sizes to 4KiB/2MiB/1GiB");
 			amd_iommu_pgsize_bitmap = AMD_IOMMU_PGSIZES_V2;
+		} else if (strncmp(str, "sats", 4) == 0) {
+			amd_iommu_sats = true;
 		} else {
 			pr_notice("Unknown option - '%s'\n", str);
 		}
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index b48a72bd7b23..c7c9b5112828 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2064,9 +2064,18 @@ static void set_dte_entry(struct amd_iommu *iommu,
 	if (domain->dirty_tracking)
 		new.data[0] |= DTE_FLAG_HAD;
 
-	if (dev_data->ats_enabled)
+	if (dev_data->ats_enabled) {
 		new.data[1] |= DTE_FLAG_IOTLB;
 
+		/* Secure ATS is supported when DTE[mode] != 0 */
+		if (amd_iommu_sats && domain->iop.mode != PAGE_MODE_NONE)
+			new.data[1] |= DTE_FLAG_SATS;
+		else if (amd_iommu_sats) {
+			pr_devel("Secure ATS not enabled for dev 0x%x as "
+				 "DTE[mode]=0\n", dev_data->devid);
+		}
+	}
+
 	old_domid = READ_ONCE(dte->data[1]) & DEV_DOMID_MASK;
 	new.data[1] |= domid;
 
-- 
2.31.1

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

[Yi Liu]

On 2025/2/25 18:58, Vasant Hegde wrote:
> AMD IOMMU supports processing ATS requests as Secure ATS requests when IOTLB
> is supported and enabled (See section "2.11 Secure ATS Supports" in AMD IOMMU
> spec [1] for more detail).

glad to see it. I'm also working on Secure ATS for VT-d. Intel Secure ATS
has an extra table named HPT (Host Permission Table) chapter 4.2.4 in
revision 5.0 spec. IOMMU uses this table to check the address in the
translated transactions to ensure the device has access permission.

We plan to support the HPT on S2 when nested translation is configured. But
we may let the userspace VMM to decide if it wants SATS on specific device.
The reason is that some devices might be considered as trusted. e.g.
integrated devices.

My thought is to add a flag in iommufd_hw_capabilities to report the
capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
know if HPT is needed when allocating S2 domain. Userspace attaches the
un-trusted devices to the domains with HPT. While trusted devices can be
attached to 'normal' S2 domains.  Will to send it out soon.:)

> With Secure ATS, for ATS requests IOMMU return GPA to device instead of SPA.

I suppose AMD Secure ATS only works under nested translation configuration.
is it? I guess admin may want some flexibility to opt-in it. :)

-- 
Regards,
Yi Liu

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

[Robin Murphy]

On 2025-02-25 12:30 pm, Yi Liu wrote:
> On 2025/2/25 18:58, Vasant Hegde wrote:
>> AMD IOMMU supports processing ATS requests as Secure ATS requests when 
>> IOTLB
>> is supported and enabled (See section "2.11 Secure ATS Supports" in 
>> AMD IOMMU
>> spec [1] for more detail).
> 
> glad to see it. I'm also working on Secure ATS for VT-d. Intel Secure ATS
> has an extra table named HPT (Host Permission Table) chapter 4.2.4 in
> revision 5.0 spec. IOMMU uses this table to check the address in the
> translated transactions to ensure the device has access permission.
> 
> We plan to support the HPT on S2 when nested translation is configured. But
> we may let the userspace VMM to decide if it wants SATS on specific device.
> The reason is that some devices might be considered as trusted. e.g.
> integrated devices.
> 
> My thought is to add a flag in iommufd_hw_capabilities to report the
> capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
> know if HPT is needed when allocating S2 domain. Userspace attaches the
> un-trusted devices to the domains with HPT. While trusted devices can be
> attached to 'normal' S2 domains.  Will to send it out soon.:)
> 
>> With Secure ATS, for ATS requests IOMMU return GPA to device instead 
>> of SPA.
> 
> I suppose AMD Secure ATS only works under nested translation configuration.
> is it? I guess admin may want some flexibility to opt-in it. :)

Heh, and in SMMUv3 we have both options, with split-stage ATS for nested 
translation, and our Device Permission Table for full ATS, so it seems 
like there should be room for some kind of generalised capability.

Thanks,
Robin.

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

[Yi Liu]

On 2025/2/25 21:18, Robin Murphy wrote:
> On 2025-02-25 12:30 pm, Yi Liu wrote:
>> On 2025/2/25 18:58, Vasant Hegde wrote:
>>> AMD IOMMU supports processing ATS requests as Secure ATS requests when 
>>> IOTLB
>>> is supported and enabled (See section "2.11 Secure ATS Supports" in AMD 
>>> IOMMU
>>> spec [1] for more detail).
>>
>> glad to see it. I'm also working on Secure ATS for VT-d. Intel Secure ATS
>> has an extra table named HPT (Host Permission Table) chapter 4.2.4 in
>> revision 5.0 spec. IOMMU uses this table to check the address in the
>> translated transactions to ensure the device has access permission.
>>
>> We plan to support the HPT on S2 when nested translation is configured. But
>> we may let the userspace VMM to decide if it wants SATS on specific device.
>> The reason is that some devices might be considered as trusted. e.g.
>> integrated devices.
>>
>> My thought is to add a flag in iommufd_hw_capabilities to report the
>> capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
>> know if HPT is needed when allocating S2 domain. Userspace attaches the
>> un-trusted devices to the domains with HPT. While trusted devices can be
>> attached to 'normal' S2 domains.  Will to send it out soon.:)
>>
>>> With Secure ATS, for ATS requests IOMMU return GPA to device instead of 
>>> SPA.
>>
>> I suppose AMD Secure ATS only works under nested translation configuration.
>> is it? I guess admin may want some flexibility to opt-in it. :)
> 
> Heh, and in SMMUv3 we have both options, with split-stage ATS for nested 
> translation, and our Device Permission Table for full ATS, so it seems like 
> there should be room for some kind of generalised capability.

I think so. HPT is a 4-level table which uses different bits of the PA as
index of each level. What in my mind so far is associating the HPT with
paging domains and managing the HPT per the map/unamp pages to paging
domain. Maybe we can have more discussions when my RFC is out. Should be
soon.

-- 
Regards,
Yi Liu

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

[Jason Gunthorpe]

On Tue, Feb 25, 2025 at 01:18:43PM +0000, Robin Murphy wrote:

> > My thought is to add a flag in iommufd_hw_capabilities to report the
> > capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
> > know if HPT is needed when allocating S2 domain. Userspace attaches the
> > un-trusted devices to the domains with HPT. While trusted devices can be
> > attached to 'normal' S2 domains.  Will to send it out soon.:)
> > 
> > > With Secure ATS, for ATS requests IOMMU return GPA to device instead
> > > of SPA.
> > 
> > I suppose AMD Secure ATS only works under nested translation configuration.
> > is it? I guess admin may want some flexibility to opt-in it. :)
> 
> Heh, and in SMMUv3 we have both options, with split-stage ATS for nested
> translation, and our Device Permission Table for full ATS, so it seems like
> there should be room for some kind of generalised capability.

Yes, lets not have another driver-only command line parameter but
somthing global managed by the core code.

What are the options here?
 1) Translated Address (TA) is a full physical address and IOMMU does
    do anything (today)

 2) IOMMU converts a S1 IOVA into a TA as a S2 IOVA address and the
    IOMMU runs it through the S2 to validate it. (ARM calls this
    split-stage). Requires nesting

 3) TA is an IOVA and the IOMMU runs it through the full translation
    to validate it. ATS is just used to signal non-present

 4) TA is an full physical address and the IOMMU validates the full
    physical using some kind of permission structure (ARM calls this
    Device Permission Table)

What are the three HWs doing? 

I see #2 and #4 clearly in the SMM spec. Is #3 a special case of #2 (a
STE with a S2 and S1DSS bypass)?

I think AMD is doing #3 from the docs.

What is Intel doing?

A domain alloc flag to put the domain into #3 mode seems like a good
start to me. The core code can decide to activate the flag for the
default domain. Suggest starting from pci->untrusted.

Also, #3 requires PCI topology support, the ACS flags need to be set
to route all TA's to the host. The core code should check and validate
this before turning it on.

I think #2 can be requested through the vSTE on ARM?

Not sure how to setup #4? The core code also needs to detect coherent
systems (ie CXL/etc) and refuse to do anything other than #1/#4..

Jason

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

[Yi Liu]

On 2025/2/25 22:55, Jason Gunthorpe wrote:
> On Tue, Feb 25, 2025 at 01:18:43PM +0000, Robin Murphy wrote:
> 
>>> My thought is to add a flag in iommufd_hw_capabilities to report the
>>> capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
>>> know if HPT is needed when allocating S2 domain. Userspace attaches the
>>> un-trusted devices to the domains with HPT. While trusted devices can be
>>> attached to 'normal' S2 domains.  Will to send it out soon.:)
>>>
>>>> With Secure ATS, for ATS requests IOMMU return GPA to device instead
>>>> of SPA.
>>>
>>> I suppose AMD Secure ATS only works under nested translation configuration.
>>> is it? I guess admin may want some flexibility to opt-in it. :)
>>
>> Heh, and in SMMUv3 we have both options, with split-stage ATS for nested
>> translation, and our Device Permission Table for full ATS, so it seems like
>> there should be room for some kind of generalised capability.
> 
> Yes, lets not have another driver-only command line parameter but
> somthing global managed by the core code.
> 
> What are the options here?
>   1) Translated Address (TA) is a full physical address and IOMMU does
>      do anything (today)
> 
do you mean "do nothing"?

>   2) IOMMU converts a S1 IOVA into a TA as a S2 IOVA address and the
>      IOMMU runs it through the S2 to validate it. (ARM calls this
>      split-stage). Requires nesting
> 
>   3) TA is an IOVA and the IOMMU runs it through the full translation
>      to validate it. ATS is just used to signal non-present
> 
>   4) TA is an full physical address and the IOMMU validates the full
>      physical using some kind of permission structure (ARM calls this
>      Device Permission Table)
> 
> What are the three HWs doing?
> 
> I see #2 and #4 clearly in the SMM spec. Is #3 a special case of #2 (a
> STE with a S2 and S1DSS bypass)?
> 
> I think AMD is doing #3 from the docs.
> 
> What is Intel doing?

Intel does #4. It has a Host Permission Table which validates the
physical address.

4.2.4 Host Permission Table
The Host Permission Table (HPT) allows software to control access to the 
physical address space when
one or more devices uses translated requests. System software may create 
one or more domains
which permit memory accesses from only those ATS-devices that are accepted 
as part of the domain’s
Trusted Computing Base (TCB). HPT is only supported when operating in 
scalable mode. The HPT is a
4-level structure described in Table 11 and an example HPT Walk in Figure 
4-2. Each table entry
corresponding to a valid page size can reference one or more valid pages 
using the Page Permission
(PPi) fields as well as providing a pointer to the subsequent paging structure.

> A domain alloc flag to put the domain into #3 mode seems like a good
> start to me. The core code can decide to activate the flag for the
> default domain. Suggest starting from pci->untrusted.
> 
> Also, #3 requires PCI topology support, the ACS flags need to be set
> to route all TA's to the host. The core code should check and validate
> this before turning it on.

yes. I think both #2 and #3 need to enforce it. The TA device got is
not really a PA in the two modes.

> I think #2 can be requested through the vSTE on ARM?
> 
> Not sure how to setup #4? The core code also needs to detect coherent
> systems (ie CXL/etc) and refuse to do anything other than #1/#4..

The DPT and the HPT validates physical addresses. So such tables need to be
programmed when a physical address is mapped in I/O page table. So the
choice may be associating it with paging domain (e.g. the S2 domain that
converts GPA to PA). If so, a domain alloc flag is also a good start.

-- 
Regards,
Yi Liu

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Tuesday, February 25, 2025 10:55 PM
> 
> On Tue, Feb 25, 2025 at 01:18:43PM +0000, Robin Murphy wrote:
> 
> > > My thought is to add a flag in iommufd_hw_capabilities to report the
> > > capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu
> driver
> > > know if HPT is needed when allocating S2 domain. Userspace attaches the
> > > un-trusted devices to the domains with HPT. While trusted devices can be
> > > attached to 'normal' S2 domains.  Will to send it out soon.:)
> > >
> > > > With Secure ATS, for ATS requests IOMMU return GPA to device instead
> > > > of SPA.
> > >
> > > I suppose AMD Secure ATS only works under nested translation
> configuration.
> > > is it? I guess admin may want some flexibility to opt-in it. :)
> >
> > Heh, and in SMMUv3 we have both options, with split-stage ATS for nested
> > translation, and our Device Permission Table for full ATS, so it seems like
> > there should be room for some kind of generalised capability.
> 
> Yes, lets not have another driver-only command line parameter but
> somthing global managed by the core code.
> 
> What are the options here?
>  1) Translated Address (TA) is a full physical address and IOMMU does
>     do anything (today)
> 
>  2) IOMMU converts a S1 IOVA into a TA as a S2 IOVA address and the
>     IOMMU runs it through the S2 to validate it. (ARM calls this
>     split-stage). Requires nesting

and this won't support CXL.cache which works on physical address.

> 
>  3) TA is an IOVA and the IOMMU runs it through the full translation
>     to validate it. ATS is just used to signal non-present
> 
>  4) TA is an full physical address and the IOMMU validates the full
>     physical using some kind of permission structure (ARM calls this
>     Device Permission Table)
> 
> What are the three HWs doing?
> 
> I see #2 and #4 clearly in the SMM spec. Is #3 a special case of #2 (a
> STE with a S2 and S1DSS bypass)?
> 
> I think AMD is doing #3 from the docs.

my reading of AMD spec was #2. also backed by Vasant's words:

"
With Secure ATS, for ATS requests IOMMU return GPA to device instead of SPA.
When receiving Translated DMA from a device the IOMMU performs in-line
GPA to SPA translation.
"

> 
> What is Intel doing?

Intel is doing #4. The permission structure is called Host Permission Table.

> 
> A domain alloc flag to put the domain into #3 mode seems like a good
> start to me. The core code can decide to activate the flag for the
> default domain. Suggest starting from pci->untrusted.
> 
> Also, #3 requires PCI topology support, the ACS flags need to be set
> to route all TA's to the host. The core code should check and validate
> this before turning it on.
> 
> I think #2 can be requested through the vSTE on ARM?
> 
> Not sure how to setup #4? The core code also needs to detect coherent
> systems (ie CXL/etc) and refuse to do anything other than #1/#4..
> 

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.

It provides a nice minimal start for all 3 platforms.

Later we can enable more domain/device types for #4 and beyond
iommufd if necessary.

Thanks
Kevin

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

[Jason Gunthorpe]

On Wed, Feb 26, 2025 at 09:09:29AM +0800, Yi Liu wrote:
> > What are the options here?
> >   1) Translated Address (TA) is a full physical address and IOMMU does
> >      do anything (today)

> do you mean "do nothing"?

Yes

> > Not sure how to setup #4? The core code also needs to detect coherent
> > systems (ie CXL/etc) and refuse to do anything other than #1/#4..
> 
> The DPT and the HPT validates physical addresses. So such tables need to be
> programmed when a physical address is mapped in I/O page table. So the
> choice may be associating it with paging domain (e.g. the S2 domain that
> converts GPA to PA). If so, a domain alloc flag is also a good start.

Hmm, so you imagine doing it as a side structure under the map
operation.. Maybe, that will complicate my effort to unify the page
tables somewhat, but maybe not too badly.

Jason

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

[Jason Gunthorpe]

On Wed, Feb 26, 2025 at 01:12:38AM +0000, Tian, Kevin wrote:
> > I see #2 and #4 clearly in the SMM spec. Is #3 a special case of #2 (a
> > STE with a S2 and S1DSS bypass)?
> > 
> > I think AMD is doing #3 from the docs.
> 
> my reading of AMD spec was #2. also backed by Vasant's words:
> 
> "
> With Secure ATS, for ATS requests IOMMU return GPA to device instead of SPA.
> When receiving Translated DMA from a device the IOMMU performs in-line
> GPA to SPA translation.
> "

Maybe it is both 2 and 3 if the GCR3 PASID 0 is setup to not translate
but do identity?

> 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

Jason

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

[Yi Liu]

On 2025/2/26 09:13, Jason Gunthorpe wrote:
> On Wed, Feb 26, 2025 at 09:09:29AM +0800, Yi Liu wrote:
>>> What are the options here?
>>>    1) Translated Address (TA) is a full physical address and IOMMU does
>>>       do anything (today)
> 
>> do you mean "do nothing"?
> 
> Yes
> 
>>> Not sure how to setup #4? The core code also needs to detect coherent
>>> systems (ie CXL/etc) and refuse to do anything other than #1/#4..
>>
>> The DPT and the HPT validates physical addresses. So such tables need to be
>> programmed when a physical address is mapped in I/O page table. So the
>> choice may be associating it with paging domain (e.g. the S2 domain that
>> converts GPA to PA). If so, a domain alloc flag is also a good start.
> 
> Hmm, so you imagine doing it as a side structure under the map
> operation.. Maybe,

yes.

> that will complicate my effort to unify the page
> tables somewhat, but maybe not too badly.

I'm not on purpose about this part :)

-- 
Regards,
Yi Liu

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

[Tian, Kevin]

> 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:
> > > I see #2 and #4 clearly in the SMM spec. Is #3 a special case of #2 (a
> > > STE with a S2 and S1DSS bypass)?
> > >
> > > I think AMD is doing #3 from the docs.
> >
> > my reading of AMD spec was #2. also backed by Vasant's words:
> >
> > "
> > With Secure ATS, for ATS requests IOMMU return GPA to device instead of
> SPA.
> > When receiving Translated DMA from a device the IOMMU performs in-line
> > GPA to SPA translation.
> > "
> 
> Maybe it is both 2 and 3 if the GCR3 PASID 0 is setup to not translate
> but do identity?

yes but conceptually it's still more a #2 thing from host p.o.v. The host
still sees a nested configuration no matter how the guest configures
each GCR3 entry.

> 
> > 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.

If necessary the driver could report certain restrictions about sats to the
user via GET_HW_INFO.

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Wednesday, February 26, 2025 9:13 AM
> 
> On Wed, Feb 26, 2025 at 09:09:29AM +0800, Yi Liu wrote:
> > > Not sure how to setup #4? The core code also needs to detect coherent
> > > systems (ie CXL/etc) and refuse to do anything other than #1/#4..
> >
> > The DPT and the HPT validates physical addresses. So such tables need to
> be
> > programmed when a physical address is mapped in I/O page table. So the
> > choice may be associating it with paging domain (e.g. the S2 domain that
> > converts GPA to PA). If so, a domain alloc flag is also a good start.
> 
> Hmm, so you imagine doing it as a side structure under the map
> operation.. Maybe, that will complicate my effort to unify the page
> tables somewhat, but maybe not too badly.
> 

might be OK. suppose some hooks in the map/unmap path allowing
the driver to register for updating the permission table.

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

[Vasant Hegde]

Hi Yi,


On 2/25/2025 6:00 PM, Yi Liu wrote:
> On 2025/2/25 18:58, Vasant Hegde wrote:
>> AMD IOMMU supports processing ATS requests as Secure ATS requests when IOTLB
>> is supported and enabled (See section "2.11 Secure ATS Supports" in AMD IOMMU
>> spec [1] for more detail).
> 
> glad to see it. I'm also working on Secure ATS for VT-d. Intel Secure ATS
> has an extra table named HPT (Host Permission Table) chapter 4.2.4 in
> revision 5.0 spec. IOMMU uses this table to check the address in the
> translated transactions to ensure the device has access permission.
> 
> We plan to support the HPT on S2 when nested translation is configured. But
> we may let the userspace VMM to decide if it wants SATS on specific device.
> The reason is that some devices might be considered as trusted. e.g.
> integrated devices.

Yeah. That's another option (ATS for trusted SoC devices and secure ATS for
other devices).

> 
> My thought is to add a flag in iommufd_hw_capabilities to report the
> capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
> know if HPT is needed when allocating S2 domain. Userspace attaches the
> un-trusted devices to the domains with HPT. While trusted devices can be
> attached to 'normal' S2 domains.  Will to send it out soon.:)
> 
>> With Secure ATS, for ATS requests IOMMU return GPA to device instead of SPA.
> 
> I suppose AMD Secure ATS only works under nested translation configuration.
> is it? I guess admin may want some flexibility to opt-in it. :)

AMD Secure ATS works under 3 scenarios :
  - Baremetal case w/ AMD Host (V1) page table (this include VFIO passthrough)
  - HW Nested setup where both page table is configured
    In above two setup, its opt-in feature

  - SNP (Secure Nested Paging) is ON
    Secure ATS is mandatory.

-Vasant

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

[Vasant Hegde]

Hi Jason,


On 2/25/2025 8:25 PM, Jason Gunthorpe wrote:
> On Tue, Feb 25, 2025 at 01:18:43PM +0000, Robin Murphy wrote:
> 
>>> My thought is to add a flag in iommufd_hw_capabilities to report the
>>> capability, and add a flag in iommufd_hwpt_alloc_flags to let iommu driver
>>> know if HPT is needed when allocating S2 domain. Userspace attaches the
>>> un-trusted devices to the domains with HPT. While trusted devices can be
>>> attached to 'normal' S2 domains.  Will to send it out soon.:)
>>>
>>>> With Secure ATS, for ATS requests IOMMU return GPA to device instead
>>>> of SPA.
>>>
>>> I suppose AMD Secure ATS only works under nested translation configuration.
>>> is it? I guess admin may want some flexibility to opt-in it. :)
>>
>> Heh, and in SMMUv3 we have both options, with split-stage ATS for nested
>> translation, and our Device Permission Table for full ATS, so it seems like
>> there should be room for some kind of generalised capability.
> 
> Yes, lets not have another driver-only command line parameter but
> somthing global managed by the core code.

I was not aware that intel/ARM has similar feature. So introduced AMD specific
option. Since everyone has similar feature, if it can be done in core layer and
satisfy the requirement then yes we should do that.


> 
> What are the options here?
>  1) Translated Address (TA) is a full physical address and IOMMU does
>     do anything (today)
> 
>  2) IOMMU converts a S1 IOVA into a TA as a S2 IOVA address and the
>     IOMMU runs it through the S2 to validate it. (ARM calls this
>     split-stage). Requires nesting
> 
>  3) TA is an IOVA and the IOMMU runs it through the full translation
>     to validate it. ATS is just used to signal non-present

Yes. AMD does this when Host page table is configured.
But in nested case, its slightly different. It would have finished guest page
table walk and sends GPA to device. when DMA request comes it walks final page
table (GPA -> SPA) again.

> 
>  4) TA is an full physical address and the IOMMU validates the full
>     physical using some kind of permission structure (ARM calls this
>     Device Permission Table)

I this this is similar to RMP validation on AMD.

> 
> What are the three HWs doing? 
> 
> I see #2 and #4 clearly in the SMM spec. Is #3 a special case of #2 (a
> STE with a S2 and S1DSS bypass)?
> 
> I think AMD is doing #3 from the docs.

yep.

> 
> What is Intel doing?
> 
> A domain alloc flag to put the domain into #3 mode seems like a good
> start to me. The core code can decide to activate the flag for the
> default domain. Suggest starting from pci->untrusted.

We need to consider various scenarios. For AMD:
  - Currently on baremetal, we cannot enable SVA and Secure ATS
  - Currently when SNP (Secure Nested Paging) is enabled, we have to enable
Secure ATS. So driver has to enforce.

Also what if user wants to enforce secure ATS for all devices? have kernel
command line option?


-Vasant

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

[Tian, Kevin]

> 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).

  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?

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

[Tian, Kevin]

> From: Vasant Hegde <vasant.hegde@amd.com>
> Sent: Wednesday, February 26, 2025 12:47 PM
>
> >
> >  3) TA is an IOVA and the IOMMU runs it through the full translation
> >     to validate it. ATS is just used to signal non-present
> 
> Yes. AMD does this when Host page table is configured.

Here 'signal non-present' implies to support PRI.

But...

> 
> We need to consider various scenarios. For AMD:
>   - Currently on baremetal, we cannot enable SVA and Secure ATS

... here it says SVA/SATS are incompatible. Any more background?

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

[Jason Gunthorpe]

On Wed, Feb 26, 2025 at 02:50:57AM +0000, Tian, Kevin wrote:
> > Maybe it is both 2 and 3 if the GCR3 PASID 0 is setup to not translate
> > but do identity?
> 
> yes but conceptually it's still more a #2 thing from host p.o.v. The host
> still sees a nested configuration no matter how the guest configures
> each GCR3 entry.

AMD doesn't even support nesting now, so if this patch works it means
it is scenario #2.

> 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.

We already discussed that #4 works with CXL and #2/#3 don't. They also
tolerate different PCI topology settings. The core code needs to know
what it is getting into and in the case of ARM it needs to instruct
the driver which of the two options to use.

#2/#3 seems to be prefered when possible because it uses less memory,
has less invalidation overhead, and apparently about the same
performance according to the SMMU spec.

So I think two flags is the right starting point..

Jason

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

[Jason Gunthorpe]

On Wed, Feb 26, 2025 at 07:05:06AM +0000, Tian, Kevin wrote:
> > 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:

So you don't want iommufd users to be able to opt out of it? That
makes sense to me

Jason

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

[Jason Gunthorpe]

On Wed, Feb 26, 2025 at 07:10:20AM +0000, Tian, Kevin wrote:
> > From: Vasant Hegde <vasant.hegde@amd.com>
> > Sent: Wednesday, February 26, 2025 12:47 PM
> >
> > >
> > >  3) TA is an IOVA and the IOMMU runs it through the full translation
> > >     to validate it. ATS is just used to signal non-present
> > 
> > Yes. AMD does this when Host page table is configured.
> 
> Here 'signal non-present' implies to support PRI.
> 
> But...
> 
> > We need to consider various scenarios. For AMD:
> >   - Currently on baremetal, we cannot enable SVA and Secure ATS
> 
> ... here it says SVA/SATS are incompatible. Any more background?

Yeah, +1. I don't know of use cases for #2/#3 without PRI. Why do you
want it?

Jason

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

[Jerry Snitselaar]

On Wed, Feb 26, 2025 at 07:10:20AM +0000, Tian, Kevin wrote:
> > From: Vasant Hegde <vasant.hegde@amd.com>
> > Sent: Wednesday, February 26, 2025 12:47 PM
> >
> > >
> > >  3) TA is an IOVA and the IOMMU runs it through the full translation
> > >     to validate it. ATS is just used to signal non-present
> > 
> > Yes. AMD does this when Host page table is configured.
> 
> Here 'signal non-present' implies to support PRI.
> 
> But...
> 
> > 
> > We need to consider various scenarios. For AMD:
> >   - Currently on baremetal, we cannot enable SVA and Secure ATS
> 
> ... here it says SVA/SATS are incompatible. Any more background?

Wouldn't this case be SVA with non-secure ATS and still use PRI?

"In normal non-secure processing of PCIe ATS requests, the IOMMU performs a full address transla-
tion and returns an SPA to the requesting device to be stored in that device's IOTLB."

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

[Vasant Hegde]

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

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

[Vasant Hegde]

Kevin, Jason,

On 2/26/2025 12:40 PM, Tian, Kevin wrote:
>> From: Vasant Hegde <vasant.hegde@amd.com>
>> Sent: Wednesday, February 26, 2025 12:47 PM
>>
>>>
>>>  3) TA is an IOVA and the IOMMU runs it through the full translation
>>>     to validate it. ATS is just used to signal non-present
>>
>> Yes. AMD does this when Host page table is configured.
> 
> Here 'signal non-present' implies to support PRI.
> 
> But...
> 
>>
>> We need to consider various scenarios. For AMD:
>>   - Currently on baremetal, we cannot enable SVA and Secure ATS
> 
> ... here it says SVA/SATS are incompatible. Any more background?

Sorry. I should have explained it better.

To support SVA (PASID/PRI), we have to configure domain with AMD Guest (v2) Page
table and host page table will not be set (DTE[Mode]=0).

  GVA -> GPA contains translation and GPA = SPA.

On ATS request it will send GPA back to device (which is actually a SPA). We can
support secure ATS, but ATS response will contain the SPA.

-Vasant

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

[Jason Gunthorpe]

On Thu, Feb 27, 2025 at 09:34:01PM +0530, Vasant Hegde wrote:
> Kevin, Jason,
> 
> On 2/26/2025 12:40 PM, Tian, Kevin wrote:
> >> From: Vasant Hegde <vasant.hegde@amd.com>
> >> Sent: Wednesday, February 26, 2025 12:47 PM
> >>
> >>>
> >>>  3) TA is an IOVA and the IOMMU runs it through the full translation
> >>>     to validate it. ATS is just used to signal non-present
> >>
> >> Yes. AMD does this when Host page table is configured.
> > 
> > Here 'signal non-present' implies to support PRI.
> > 
> > But...
> > 
> >>
> >> We need to consider various scenarios. For AMD:
> >>   - Currently on baremetal, we cannot enable SVA and Secure ATS
> > 
> > ... here it says SVA/SATS are incompatible. Any more background?
> 
> Sorry. I should have explained it better.
> 
> To support SVA (PASID/PRI), we have to configure domain with AMD Guest (v2) Page
> table and host page table will not be set (DTE[Mode]=0).
> 
>   GVA -> GPA contains translation and GPA = SPA.
> 
> On ATS request it will send GPA back to device (which is actually a SPA). We can
> support secure ATS, but ATS response will contain the SPA.

I see, that makes sense. The TA is not tagged with a PASID on the wire
so no iommu can support the #2 form with PASID.

Jason

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

[Baolu Lu]

On 2/28/25 00:04, Vasant Hegde wrote:
> On 2/26/2025 12:40 PM, Tian, Kevin wrote:
>>> From: Vasant Hegde<vasant.hegde@amd.com>
>>> Sent: Wednesday, February 26, 2025 12:47 PM
>>>
>>>>   3) TA is an IOVA and the IOMMU runs it through the full translation
>>>>      to validate it. ATS is just used to signal non-present
>>> Yes. AMD does this when Host page table is configured.
>> Here 'signal non-present' implies to support PRI.
>>
>> But...
>>
>>> We need to consider various scenarios. For AMD:
>>>    - Currently on baremetal, we cannot enable SVA and Secure ATS
>> ... here it says SVA/SATS are incompatible. Any more background?
> Sorry. I should have explained it better.
> 
> To support SVA (PASID/PRI), we have to configure domain with AMD Guest (v2) Page
> table and host page table will not be set (DTE[Mode]=0).
> 
>    GVA -> GPA contains translation and GPA = SPA.
> 
> On ATS request it will send GPA back to device (which is actually a SPA). We can
> support secure ATS, but ATS response will contain the SPA.

So, the hardware is configured to work in secure ATS mode, but the ATS
is not actually secure here, right? AMD's secure ATS relies on checking
GPA to SPA translation; however, GPA is always equal to SPA in the SVA
case.

Am I understanding this correctly?

Thanks,
baolu

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

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, February 28, 2025 9:48 AM
> 
> On 2/28/25 00:04, Vasant Hegde wrote:
> > On 2/26/2025 12:40 PM, Tian, Kevin wrote:
> >>> From: Vasant Hegde<vasant.hegde@amd.com>
> >>> Sent: Wednesday, February 26, 2025 12:47 PM
> >>>
> >>>>   3) TA is an IOVA and the IOMMU runs it through the full translation
> >>>>      to validate it. ATS is just used to signal non-present
> >>> Yes. AMD does this when Host page table is configured.
> >> Here 'signal non-present' implies to support PRI.
> >>
> >> But...
> >>
> >>> We need to consider various scenarios. For AMD:
> >>>    - Currently on baremetal, we cannot enable SVA and Secure ATS
> >> ... here it says SVA/SATS are incompatible. Any more background?
> > Sorry. I should have explained it better.
> >
> > To support SVA (PASID/PRI), we have to configure domain with AMD Guest
> (v2) Page
> > table and host page table will not be set (DTE[Mode]=0).
> >
> >    GVA -> GPA contains translation and GPA = SPA.
> >
> > On ATS request it will send GPA back to device (which is actually a SPA). We
> can
> > support secure ATS, but ATS response will contain the SPA.
> 
> So, the hardware is configured to work in secure ATS mode, but the ATS
> is not actually secure here, right? AMD's secure ATS relies on checking
> GPA to SPA translation; however, GPA is always equal to SPA in the SVA
> case.
> 
> Am I understanding this correctly?
> 

Looks so.

To support secure ATS, the IOMMU needs a structure to manage the
permission per the translated address (TA).

For #2 it reuses the S2 page table for permission track hence requires
nesting and TA = S2 IOVA. It cannot support bare metal SVA given
S2 is absent.

AMD is clearly this flavor. also supported by ARM.

For #3 it reuses the full translation (s1, s2, or nested) with TA =
untranslated IOVA. This supposes to work with all existing ATS
scenarios. Probably good for prototype development but no
real value given it loses all perf benefit from ATS or even worse
perf compared to no ats due to double walking.

Both #2/#3 have PCI topology restrictions e.g. CXL.

ARM probably supports #3 as a special case of #2.

For #4, it introduces a separate permission structure per real
physical address, hence TA = physical address. It's supposed
to work for all ATS scenarios and CXL, with some burden e.g.
sync with IO page table plus more invalidations, etc.

Intel/ARM supports it.

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Friday, February 28, 2025 8:04 AM
> 
> On Thu, Feb 27, 2025 at 09:34:01PM +0530, Vasant Hegde wrote:
> > Kevin, Jason,
> >
> > On 2/26/2025 12:40 PM, Tian, Kevin wrote:
> > >> From: Vasant Hegde <vasant.hegde@amd.com>
> > >> Sent: Wednesday, February 26, 2025 12:47 PM
> > >>
> > >>>
> > >>>  3) TA is an IOVA and the IOMMU runs it through the full translation
> > >>>     to validate it. ATS is just used to signal non-present
> > >>
> > >> Yes. AMD does this when Host page table is configured.
> > >
> > > Here 'signal non-present' implies to support PRI.
> > >
> > > But...
> > >
> > >>
> > >> We need to consider various scenarios. For AMD:
> > >>   - Currently on baremetal, we cannot enable SVA and Secure ATS
> > >
> > > ... here it says SVA/SATS are incompatible. Any more background?
> >
> > Sorry. I should have explained it better.
> >
> > To support SVA (PASID/PRI), we have to configure domain with AMD Guest
> (v2) Page
> > table and host page table will not be set (DTE[Mode]=0).
> >
> >   GVA -> GPA contains translation and GPA = SPA.
> >
> > On ATS request it will send GPA back to device (which is actually a SPA). We
> can
> > support secure ATS, but ATS response will contain the SPA.
> 
> I see, that makes sense. The TA is not tagged with a PASID on the wire
> so no iommu can support the #2 form with PASID.
> 

PCISIG has a ECN allowing translated request to include PASID.
Just not sure any device has supported it.

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

[Tian, Kevin]

> 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

> 
> >
> >   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.

> If yes,
> may be have an domain ops similar to def_domain_type() ?

the domain for trusted I/O will have a special flag.

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

or the admin may prefer to disabling ats.

> 
>   - 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
- ...

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

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

[Yi Liu]

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? or we just
fully rely on admin to program it?

-- 
Regards,
Yi Liu

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

[Vasant Hegde]

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

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

[Vasant Hegde]

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....

> 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> ?

-Vasant

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

[Vasant Hegde]

Baolu,


On 2/28/2025 7:17 AM, Baolu Lu wrote:
> On 2/28/25 00:04, Vasant Hegde wrote:
>> On 2/26/2025 12:40 PM, Tian, Kevin wrote:
>>>> From: Vasant Hegde<vasant.hegde@amd.com>
>>>> Sent: Wednesday, February 26, 2025 12:47 PM
>>>>
>>>>>   3) TA is an IOVA and the IOMMU runs it through the full translation
>>>>>      to validate it. ATS is just used to signal non-present
>>>> Yes. AMD does this when Host page table is configured.
>>> Here 'signal non-present' implies to support PRI.
>>>
>>> But...
>>>
>>>> We need to consider various scenarios. For AMD:
>>>>    - Currently on baremetal, we cannot enable SVA and Secure ATS
>>> ... here it says SVA/SATS are incompatible. Any more background?
>> Sorry. I should have explained it better.
>>
>> To support SVA (PASID/PRI), we have to configure domain with AMD Guest (v2) Page
>> table and host page table will not be set (DTE[Mode]=0).
>>
>>    GVA -> GPA contains translation and GPA = SPA.
>>
>> On ATS request it will send GPA back to device (which is actually a SPA). We can
>> support secure ATS, but ATS response will contain the SPA.
> 
> So, the hardware is configured to work in secure ATS mode, but the ATS
> is not actually secure here, right? AMD's secure ATS relies on checking
> GPA to SPA translation; however, GPA is always equal to SPA in the SVA
> case.
> 
> Am I understanding this correctly?

Right. This is how it works currently.

-Vasant

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

[Yi Liu]

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

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

[Vasant Hegde]

Hi Yi,


On 2/28/2025 2:17 PM, Yi Liu wrote:
> 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?

Yes. There is a IOMMU feature bit. see iommu_snp_enable().

-Vasant

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

[Vasant Hegde]

Kevin,


On 2/28/2025 11:45 AM, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@linux.intel.com>
>> Sent: Friday, February 28, 2025 9:48 AM
>>
>> On 2/28/25 00:04, Vasant Hegde wrote:
>>> On 2/26/2025 12:40 PM, Tian, Kevin wrote:
>>>>> From: Vasant Hegde<vasant.hegde@amd.com>
>>>>> Sent: Wednesday, February 26, 2025 12:47 PM
>>>>>
>>>>>>   3) TA is an IOVA and the IOMMU runs it through the full translation
>>>>>>      to validate it. ATS is just used to signal non-present
>>>>> Yes. AMD does this when Host page table is configured.
>>>> Here 'signal non-present' implies to support PRI.
>>>>
>>>> But...
>>>>
>>>>> We need to consider various scenarios. For AMD:
>>>>>    - Currently on baremetal, we cannot enable SVA and Secure ATS
>>>> ... here it says SVA/SATS are incompatible. Any more background?
>>> Sorry. I should have explained it better.
>>>
>>> To support SVA (PASID/PRI), we have to configure domain with AMD Guest
>> (v2) Page
>>> table and host page table will not be set (DTE[Mode]=0).
>>>
>>>    GVA -> GPA contains translation and GPA = SPA.
>>>
>>> On ATS request it will send GPA back to device (which is actually a SPA). We
>> can
>>> support secure ATS, but ATS response will contain the SPA.
>>
>> So, the hardware is configured to work in secure ATS mode, but the ATS
>> is not actually secure here, right? AMD's secure ATS relies on checking
>> GPA to SPA translation; however, GPA is always equal to SPA in the SVA
>> case.
>>
>> Am I understanding this correctly?
>>
> 
> Looks so.
> 
> To support secure ATS, the IOMMU needs a structure to manage the
> permission per the translated address (TA).
> 
> For #2 it reuses the S2 page table for permission track hence requires
> nesting and TA = S2 IOVA. It cannot support bare metal SVA given
> S2 is absent.
> 
> AMD is clearly this flavor. also supported by ARM.
> 
> For #3 it reuses the full translation (s1, s2, or nested) with TA =
> untranslated IOVA. This supposes to work with all existing ATS
> scenarios. Probably good for prototype development but no
> real value given it loses all perf benefit from ATS or even worse
> perf compared to no ats due to double walking.
> 
> Both #2/#3 have PCI topology restrictions e.g. CXL.
> 
> ARM probably supports #3 as a special case of #2.
> 
> For #4, it introduces a separate permission structure per real
> physical address, hence TA = physical address. It's supposed
> to work for all ATS scenarios and CXL, with some burden e.g.
> sync with IO page table plus more invalidations, etc.

Nice Summary. #2 / #4 is practically useful scenario's.

-Vasant

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

[Jason Gunthorpe]

On Fri, Feb 28, 2025 at 06:15:32AM +0000, Tian, Kevin wrote:

> scenarios. Probably good for prototype development but no
> real value given it loses all perf benefit from ATS or even worse
> perf compared to no ats due to double walking.

ARM's docs noted they expect about the same IO performance between #3
and #4 methods. Both are walking tables to validate the TA. The main
value of ATS here is not for performance but it supports a non-present
result.

So I thik given the choice we should prefer #3 over #4.

You must switch to #4 for CXL/etc/etc.

Jason

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

[Jason Gunthorpe]

On Thu, Feb 27, 2025 at 08:57:45PM +0530, Vasant Hegde wrote:

>   - 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() ?

Yeah I think that is probably the best option, some kind of callback
to the drive to check if a platform forced secure ATS is neccessary.

I assume if you need forced secure ATS then you also are disabling
IDENTITY and disabling PASID?

Jason

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

[Yi Liu]

On 2025/2/28 16:47, Vasant Hegde wrote:
> Hi Yi,
> 
> 
> On 2/28/2025 2:17 PM, Yi Liu wrote:
>> 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?
> 
> Yes. There is a IOMMU feature bit. see iommu_snp_enable().

sure.

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?

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.

Any ideas?

-- 
Regards,
Yi Liu

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Friday, February 28, 2025 10:53 PM
> 
> On Fri, Feb 28, 2025 at 06:15:32AM +0000, Tian, Kevin wrote:
> 
> > scenarios. Probably good for prototype development but no
> > real value given it loses all perf benefit from ATS or even worse
> > perf compared to no ats due to double walking.
> 
> ARM's docs noted they expect about the same IO performance between #3
> and #4 methods. Both are walking tables to validate the TA. The main
> value of ATS here is not for performance but it supports a non-present
> result.

Both are walking tables but the costs are not exactly same.

In concept #4 allows a more efficient table format to achieve a good
tradeoff between performance and security - perf lower than unsecure
ATS but higher than #3, with security enforced.

e.g. for VT-d the permission structure is a compacted format with
each 64 bit PTE covering 16 pages, i.e. only 4 perm bits per page.
So it's far less walks compared to walking a full nested I/O page
tables, plus a dedicated cache.

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Friday, February 28, 2025 10:57 PM
> 
> On Thu, Feb 27, 2025 at 08:57:45PM +0530, Vasant Hegde wrote:
> 
> >   - 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() ?
> 
> Yeah I think that is probably the best option, some kind of callback
> to the drive to check if a platform forced secure ATS is neccessary.
> 
> I assume if you need forced secure ATS then you also are disabling
> IDENTITY and disabling PASID?
> 

Out of curiosity here. The host is untrusted in SNP case. What would
happen if untrusted host disables secure ATS after SNP is enabled?
Is the bit locked down once SNP is enabled?

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

[Tian, Kevin]

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Sunday, March 2, 2025 4:11 PM
> 
> 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?

I was envisioning it as a policy knob, which can be updated only when
the device is not attached to any domain. There is a default value per
device, based on hw capability and platform enforcement like SNP.

Then when attaching the device to a new domain, turn on ats/sats
according to the policy.

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

[Vasant Hegde]

Hi Yi, Kevin, Jason,


On 3/2/2025 1:40 PM, Yi Liu wrote:
> On 2025/2/28 16:47, Vasant Hegde wrote:
>> Hi Yi,
>>
>>
>> On 2/28/2025 2:17 PM, Yi Liu wrote:
>>> 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?
>>
>> Yes. There is a IOMMU feature bit. see iommu_snp_enable().
> 
> sure.
> 
> 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?

Currently AMD driver enables device ATS and configures IOMMU DTE (per device
setup) during attach_device() path. There was a discussion to move device ATS
enablement to probe() path. We will move it to probe() path soon.


> 
> 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.

Right. Lets see if below flow covers all the requirement
  - Define new flags
    IOMMU_HWPT_FLAGS_SATS / IOMMU_HWPT_FLAGS_ATS

  - Enhance 'struct iommu_device' to include ATS flags (NO ATS, ATS, SECURE ATS)
    iommu_device->ats_flag

  - During probe() driver will update this field (similar to max_pasids)
    If (IOMMU needs Secure ATS always)
	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS
    else if (device supports ATS and IOMMU supports Secure ATS)
	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS | IOMMU_HWPT_FLAGS_ATS
    else if (device supports ATS and IOMMU supports ATS)
	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_ATS
    else
	iommu_device->ats_flag = <NO FLAGS> (May be have one more flag
IOMMU_HWPT_FLAGS_NO_ATS) ?


  - Based on default setting, kernel command line option and probe() data, core
layer can adjust ATS capability. i. e.
     - If we have kernel command line (say iommu.ats=<x>), use that data for all
ATS capable device.
     - If not, apply default
      untrusted device -> force secure ATS or no ats
      If HW needs Secure ATS always , enable Secure ATS
      For all other cases use ATS

   - For domain_alloc_paging_flags(), send appropriate ATS flags

     Q : We need to find a way to send this info to identity domain?

   - sysfs knob : core layer knows supported ATS flags. We can expose them via
sysfs. Then as Kevin mentioned, we can make sure changes happens only when
device in unbind to domain.

Does this flow works fine?

-Vasant

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

[Vasant Hegde]

Jason,

On 2/28/2025 8:26 PM, Jason Gunthorpe wrote:
> On Thu, Feb 27, 2025 at 08:57:45PM +0530, Vasant Hegde wrote:
> 
>>   - 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() ?
> 
> Yeah I think that is probably the best option, some kind of callback
> to the drive to check if a platform forced secure ATS is neccessary.
> 
> I assume if you need forced secure ATS then you also are disabling
> IDENTITY and disabling PASID?

Right. Currently SNP is enabled only when IOMMU is in Host (v1) page table mode.

-Vasant

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

[Jason Gunthorpe]

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

?

Jason

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

[Baolu Lu]

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

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

[Yi Liu]

On 2025/3/4 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?

sorry, a revise. Existing intel iommu driver enables ATS in the time of
domain attach. While Baolu has the below series [1] to move it to
probe_device().

[1] 
https://lore.kernel.org/linux-iommu/20250224051627.2956304-5-baolu.lu@linux.intel.com/

> 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..

will it affect Baolu's above refactoring [1]? :(

>> 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

this can be considered policy to the existing ATS. curious, why paging
domain is special here?

>   secure-always
>   secure-paging-only

and this is the policy of secure ATS.

A general question. Can admin op the knob if the device has been attached
to any domain?

-- 
Regards,
Yi Liu

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

[Yi Liu]

On 2025/3/3 11:00, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Sunday, March 2, 2025 4:11 PM
>>
>> 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?
> 
> I was envisioning it as a policy knob, which can be updated only when
> the device is not attached to any domain.

If the device is probed, there seems always a domain attached for probed
device. e.g. default_domain. Only when bound to some drivers like vfio,
there is a blocked_domain attached.

> There is a default value per
> device, based on hw capability and platform enforcement like SNP.

or maybe topology as well. if a device is plugged in a slot, it might
be considered as not trusted.

> Then when attaching the device to a new domain, turn on ats/sats
> according to the policy.

-- 
Regards,
Yi Liu

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

[Vasant Hegde]

Jason,


On 3/4/2025 12:08 AM, 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?

Yes. If we are dong knob then we should support normal ATS as well.

> 
>  never
>  always
>  paging-only
>  secure-always
>  secure-paging-only

Why not just expose supported modes (No ATS, ATS, Secure ATS) per group and
allow admin to select within supported modes?  We may have to add more logic in
sysfs store/show code path. But user will know what is supported and what they
can pick.

Also sysfs knob needs new domain ops to communicate changes to indivisual
driver. Like in store path
  - validate the input
  - make sure driver is unbond so that its safe to modify the
  - New ops to communicate to indivisual driver
	 something like ops->set_ats_mode(dev, mode)



-Vasant

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

[Vasant Hegde]

Hi,

On 3/4/2025 12:20 PM, Yi Liu wrote:
> On 2025/3/4 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?
> 
> sorry, a revise. Existing intel iommu driver enables ATS in the time of
> domain attach. While Baolu has the below series [1] to move it to
> probe_device().
> 
> [1] https://lore.kernel.org/linux-iommu/20250224051627.2956304-5-
> baolu.lu@linux.intel.com/
> 
>> 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..
> 
> will it affect Baolu's above refactoring [1]? :(
> 
>>> 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
> 
> this can be considered policy to the existing ATS. curious, why paging
> domain is special here?
> 
>>   secure-always
>>   secure-paging-only
> 
> and this is the policy of secure ATS.
> 
> A general question. Can admin op the knob if the device has been attached
> to any domain?

I'd say, it should be similar to "change domain" interface we have today.

-Vasant

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

[Jason Gunthorpe]

On Tue, Mar 04, 2025 at 10:16:45AM +0800, Baolu Lu wrote:
> 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.

It is not quite so simple, we need ATS in some virtualization cases
for VFIO even though it is not "using" ATS for PRI.

Today the iommu drivers always enable ATS for paging domains if ATS is
supported. This is what I'm wondering if it should be made optional

> 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".

But this philosophy makes sense

Jason

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

[Jason Gunthorpe]

On Tue, Mar 04, 2025 at 02:50:19PM +0800, Yi Liu wrote:

> > 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..
> 
> will it affect Baolu's above refactoring [1]? :(

I think no, but when we get some conclusion how ATS should work I'd
expect all IOMMU drivers to align

> >   never
> >   always
> >   paging-only
>
> this can be considered policy to the existing ATS. curious, why paging
> domain is special here?

Using ATS with an IDENTITY translation is not a performance win, so
you'd probably want to turn it off in that case.

Jason

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

[Jason Gunthorpe]

On Tue, Mar 04, 2025 at 03:45:54PM +0530, Vasant Hegde wrote:
> >  never
> >  always
> >  paging-only
> >  secure-always
> >  secure-paging-only
> 
> Why not just expose supported modes (No ATS, ATS, Secure ATS) per group and
> allow admin to select within supported modes?  We may have to add more logic in
> sysfs store/show code path. But user will know what is supported and what they
> can pick.

A sysfs type thing is what I'm thinking

Just trying to understand what options we should be providing to
userspace..

I am sensitive that we'd like to disble ATS for identity modes in many
common cases and we get that implicitly today.

> Also sysfs knob needs new domain ops to communicate changes to indivisual
> driver. Like in store path
>   - validate the input
>   - make sure driver is unbond so that its safe to modify the
>   - New ops to communicate to indivisual driver
> 	 something like ops->set_ats_mode(dev, mode)

Yeah this needs to be considered.. I wonder if a flags input to attach
is the right way to go.

That would be convenient for how SMMUv3 is structued now as it can just
switch modes hitlessly with its existing logic. I think all drivers
should have a similar design.

Jason

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

[Baolu Lu]

On 3/4/25 22:18, Jason Gunthorpe wrote:
> On Tue, Mar 04, 2025 at 10:16:45AM +0800, Baolu Lu wrote:
>> 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.
> It is not quite so simple, we need ATS in some virtualization cases
> for VFIO even though it is not "using" ATS for PRI.

In such cases, perhaps we could allocate a domain-allocation flag for
this purpose. Userspace would opt-in to this flag when allocating the
iommufd hwpt.

> 
> Today the iommu drivers always enable ATS for paging domains if ATS is
> supported. This is what I'm wondering if it should be made optional

The domain-allocation flag also applies here. The iommu core manages the
policy and determines whether to pass the ATS-required flag to the
domain allocation callback of the iommu drivers. The iommu core's sysfs
interface will include a node to modify the allocation policy.

Thanks,
baolu

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Tuesday, March 4, 2025 10:18 PM
> 
> On Tue, Mar 04, 2025 at 10:16:45AM +0800, Baolu Lu wrote:
> > 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.
> 
> It is not quite so simple, we need ATS in some virtualization cases
> for VFIO even though it is not "using" ATS for PRI.

As discussed before we probably want the user to opt for ATS via
a hwpt flag.

> 
> Today the iommu drivers always enable ATS for paging domains if ATS is
> supported. This is what I'm wondering if it should be made optional

that'd make sense.

> 
> > 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".
> 
> But this philosophy makes sense
> 

+1

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Tuesday, March 4, 2025 10:21 PM
> 
> On Tue, Mar 04, 2025 at 02:50:19PM +0800, Yi Liu wrote:
> 
> > > 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..
> >
> > will it affect Baolu's above refactoring [1]? :(
> 
> I think no, but when we get some conclusion how ATS should work I'd
> expect all IOMMU drivers to align
> 
> > >   never
> > >   always
> > >   paging-only
> >
> > this can be considered policy to the existing ATS. curious, why paging
> > domain is special here?
> 
> Using ATS with an IDENTITY translation is not a performance win, so
> you'd probably want to turn it off in that case.
> 

Probably we may also need separate policy between unmanaged and
dma types. In some scenarios the user may care more about potential
ATS attacks from userspace drivers but not kernel drivers. In that case
disabling secure ats for kernel drivers would avoid unnecessary cost
of address audit.

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

[Tian, Kevin]

> From: Vasant Hegde <vasant.hegde@amd.com>
> Sent: Monday, March 3, 2025 7:42 PM
> 
> Currently AMD driver enables device ATS and configures IOMMU DTE (per
> device
> setup) during attach_device() path. There was a discussion to move device
> ATS
> enablement to probe() path. We will move it to probe() path soon.

Based on discussion in this thread I'd expect doing it during
attach is the long-term direction.

> 
> 
> >
> > 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.
> 
> Right. Lets see if below flow covers all the requirement
>   - Define new flags
>     IOMMU_HWPT_FLAGS_SATS / IOMMU_HWPT_FLAGS_ATS

SATS policy should be managed by the sys admin, while ATS feature
itself can be opted by the user.

> 
>   - Enhance 'struct iommu_device' to include ATS flags (NO ATS, ATS, SECURE
> ATS)
>     iommu_device->ats_flag

as commented by Jason this needs to differentiate domain types,
either due to hw lacking of support for certain types or the admin
imposes different security levels across different types.

> 
>   - During probe() driver will update this field (similar to max_pasids)
>     If (IOMMU needs Secure ATS always)
> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS
>     else if (device supports ATS and IOMMU supports Secure ATS)
> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS |
> IOMMU_HWPT_FLAGS_ATS
>     else if (device supports ATS and IOMMU supports ATS)
> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_ATS
>     else
> 	iommu_device->ats_flag = <NO FLAGS> (May be have one more flag
> IOMMU_HWPT_FLAGS_NO_ATS) ?

Here the device->ats_flag should be more an ats_policy thing? At
probe time it's assigned with a default policy according to device
type, platform requirement, etc. and later exposed via sysfs for
admin opt.

Actual ats enabling/enforcement is done at attach time, according
to ats_policy.

> 
> 
>   - Based on default setting, kernel command line option and probe() data,
> core
> layer can adjust ATS capability. i. e.
>      - If we have kernel command line (say iommu.ats=<x>), use that data for
> all
> ATS capable device.
>      - If not, apply default
>       untrusted device -> force secure ATS or no ats
>       If HW needs Secure ATS always , enable Secure ATS
>       For all other cases use ATS

this is an interesting one, i.e. whether we still want a global iommu
option to enable sats for all applicable devices/domains. the pros
is that it is simple and allows underlying driver to add sats support
before a more flexible sysfs interface is ready. the downside is that
it's unclear which device/domain types are affected by sats on
different platforms w/o digging into the hw detail...

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

[Jason Gunthorpe]

On Wed, Mar 05, 2025 at 02:50:38AM +0000, Tian, Kevin wrote:

> Probably we may also need separate policy between unmanaged and
> dma types. In some scenarios the user may care more about potential
> ATS attacks from userspace drivers but not kernel drivers. In that case
> disabling secure ats for kernel drivers would avoid unnecessary cost
> of address audit.

That would be some specific iommufd/vfio policy IMHO

But really, if the device is being used by iommufd or not is well
known by the admin, they could set the general ATS to secure prior to
binding vfio and just be done with it. No need for more automation?

Jason

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

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Thursday, March 6, 2025 1:22 AM
> 
> On Wed, Mar 05, 2025 at 02:50:38AM +0000, Tian, Kevin wrote:
> 
> > Probably we may also need separate policy between unmanaged and
> > dma types. In some scenarios the user may care more about potential
> > ATS attacks from userspace drivers but not kernel drivers. In that case
> > disabling secure ats for kernel drivers would avoid unnecessary cost
> > of address audit.
> 
> That would be some specific iommufd/vfio policy IMHO
> 
> But really, if the device is being used by iommufd or not is well
> known by the admin, they could set the general ATS to secure prior to
> binding vfio and just be done with it. No need for more automation?
> 

Make sense.

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

[Vasant Hegde]

Hi Kevin,


On 3/3/2025 8:25 AM, Tian, Kevin wrote:
>> From: Jason Gunthorpe <jgg@ziepe.ca>
>> Sent: Friday, February 28, 2025 10:57 PM
>>
>> On Thu, Feb 27, 2025 at 08:57:45PM +0530, Vasant Hegde wrote:
>>
>>>   - 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() ?
>>
>> Yeah I think that is probably the best option, some kind of callback
>> to the drive to check if a platform forced secure ATS is neccessary.
>>
>> I assume if you need forced secure ATS then you also are disabling
>> IDENTITY and disabling PASID?
>>
> 
> Out of curiosity here. The host is untrusted in SNP case. What would
> happen if untrusted host disables secure ATS after SNP is enabled?

Once SNP is enabled, if host disabled Secure ATS for device , but keeps ATS ON,
then IOMMU will log ILLEGAL DTE Entry.

-Vasant

> Is the bit locked down once SNP is enabled?

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

[Vasant Hegde]

Jason,


On 3/4/2025 7:54 PM, Jason Gunthorpe wrote:
> On Tue, Mar 04, 2025 at 03:45:54PM +0530, Vasant Hegde wrote:
>>>  never
>>>  always
>>>  paging-only
>>>  secure-always
>>>  secure-paging-only
>>
>> Why not just expose supported modes (No ATS, ATS, Secure ATS) per group and
>> allow admin to select within supported modes?  We may have to add more logic in
>> sysfs store/show code path. But user will know what is supported and what they
>> can pick.
> 
> A sysfs type thing is what I'm thinking
> 
> Just trying to understand what options we should be providing to
> userspace..
> 
> I am sensitive that we'd like to disble ATS for identity modes in many
> common cases and we get that implicitly today.

In default boot, we can keep existing behavior right?

May be add more logic to sysfs show/store path? Meaning calculate per device
group supported options and expose only those. And in store path validate
against supported options?

> 
>> Also sysfs knob needs new domain ops to communicate changes to indivisual
>> driver. Like in store path
>>   - validate the input
>>   - make sure driver is unbond so that its safe to modify the
>>   - New ops to communicate to indivisual driver
>> 	 something like ops->set_ats_mode(dev, mode)
> 
> Yeah this needs to be considered.. I wonder if a flags input to attach
> is the right way to go.

You mean add `flag` to ops->attach_dev() ? That should work fine.

I said new ops because that would make updating existing domain/DTE becomes easy.

-Vasant

> 
> That would be convenient for how SMMUv3 is structued now as it can just
> switch modes hitlessly with its existing logic. I think all drivers
> should have a similar design.
> 
> Jason

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

[Vasant Hegde]

Kevin,


On 3/5/2025 8:54 AM, Tian, Kevin wrote:
>> From: Vasant Hegde <vasant.hegde@amd.com>
>> Sent: Monday, March 3, 2025 7:42 PM
>>
>> Currently AMD driver enables device ATS and configures IOMMU DTE (per
>> device
>> setup) during attach_device() path. There was a discussion to move device
>> ATS
>> enablement to probe() path. We will move it to probe() path soon.
> 
> Based on discussion in this thread I'd expect doing it during
> attach is the long-term direction.

AMD does actual enablement of ATS per device (via DTE setting) in attach device
path. So I see this as default requirement. Of course some of policy enforcement
we can implement later.

May be we should document full flow in detail and make sure it takes care of all
HW requirements. Then decide when to do each part.

> 
>>
>>
>>>
>>> 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.
>>
>> Right. Lets see if below flow covers all the requirement
>>   - Define new flags
>>     IOMMU_HWPT_FLAGS_SATS / IOMMU_HWPT_FLAGS_ATS
> 
> SATS policy should be managed by the sys admin, while ATS feature
> itself can be opted by the user.

"user" you mean like iommufd during domain allocation?

> 
>>
>>   - Enhance 'struct iommu_device' to include ATS flags (NO ATS, ATS, SECURE
>> ATS)
>>     iommu_device->ats_flag
> 
> as commented by Jason this needs to differentiate domain types,
> either due to hw lacking of support for certain types or the admin
> imposes different security levels across different types.

Right.

> 
>>
>>   - During probe() driver will update this field (similar to max_pasids)
>>     If (IOMMU needs Secure ATS always)
>> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS
>>     else if (device supports ATS and IOMMU supports Secure ATS)
>> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS |
>> IOMMU_HWPT_FLAGS_ATS
>>     else if (device supports ATS and IOMMU supports ATS)
>> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_ATS
>>     else
>> 	iommu_device->ats_flag = <NO FLAGS> (May be have one more flag
>> IOMMU_HWPT_FLAGS_NO_ATS) ?
> 
> Here the device->ats_flag should be more an ats_policy thing? At
> probe time it's assigned with a default policy according to device
> type, platform requirement, etc. and later exposed via sysfs for
> admin opt.

Right. We can call it as ats_policy (expected/desired mode by HW driver).
So now core knows HW requirement. Also default and global policy enforcement
(via kernel command line).

During domain allocation, we can enforce required domain specific policy.


> 
> Actual ats enabling/enforcement is done at attach time, according
> to ats_policy.

Then during attach_dev() we pass those flags as argument?

> 
>>
>>
>>   - Based on default setting, kernel command line option and probe() data,
>> core
>> layer can adjust ATS capability. i. e.
>>      - If we have kernel command line (say iommu.ats=<x>), use that data for
>> all
>> ATS capable device.
>>      - If not, apply default
>>       untrusted device -> force secure ATS or no ats
>>       If HW needs Secure ATS always , enable Secure ATS
>>       For all other cases use ATS
> 
> this is an interesting one, i.e. whether we still want a global iommu
> option to enable sats for all applicable devices/domains. the pros
> is that it is simple and allows underlying driver to add sats support
> before a more flexible sysfs interface is ready. the downside is that
> it's unclear which device/domain types are affected by sats on
> different platforms w/o digging into the hw detail...

I preferred global option (that's why this patch added amd_iommu=sats) because
as you said its simple, can be enforced at global level. Also w/ sysfs interface
we have to update for each device/group indivisual. Another drawback is, w/
sysfs first we have to unbind the driver (which may not be possible always),
modify ATS setting, then bind driver again.

In Summary, I visualize it three level:
  - Default policy implemented in kernel
  - Global policy enforcement by admin via kernel command line
  - Per group/device level enforcement via sysfs

Do we need something for iommufd/vfio? Where user can tell desired mode while
allocating HWPT domains?

-Vasant

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

[Tian, Kevin]

> From: Vasant Hegde <vasant.hegde@amd.com>
> Sent: Monday, March 10, 2025 10:14 PM
> 
> On 3/3/2025 8:25 AM, Tian, Kevin wrote:
> >> From: Jason Gunthorpe <jgg@ziepe.ca>
> >> Sent: Friday, February 28, 2025 10:57 PM
> >>
> >> On Thu, Feb 27, 2025 at 08:57:45PM +0530, Vasant Hegde wrote:
> >>
> >>>   - 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() ?
> >>
> >> Yeah I think that is probably the best option, some kind of callback
> >> to the drive to check if a platform forced secure ATS is neccessary.
> >>
> >> I assume if you need forced secure ATS then you also are disabling
> >> IDENTITY and disabling PASID?
> >>
> >
> > Out of curiosity here. The host is untrusted in SNP case. What would
> > happen if untrusted host disables secure ATS after SNP is enabled?
> 
> Once SNP is enabled, if host disabled Secure ATS for device , but keeps ATS
> ON,
> then IOMMU will log ILLEGAL DTE Entry.
> 

Make sense.

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

[Tian, Kevin]

> From: Vasant Hegde <vasant.hegde@amd.com>
> Sent: Tuesday, March 11, 2025 1:07 AM
> 
> On 3/5/2025 8:54 AM, Tian, Kevin wrote:
> >> From: Vasant Hegde <vasant.hegde@amd.com>
> >> Sent: Monday, March 3, 2025 7:42 PM
> >>
> >> Right. Lets see if below flow covers all the requirement
> >>   - Define new flags
> >>     IOMMU_HWPT_FLAGS_SATS / IOMMU_HWPT_FLAGS_ATS
> >
> > SATS policy should be managed by the sys admin, while ATS feature
> > itself can be opted by the user.
> 
> "user" you mean like iommufd during domain allocation?

yes

> >>
> >>   - During probe() driver will update this field (similar to max_pasids)
> >>     If (IOMMU needs Secure ATS always)
> >> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS
> >>     else if (device supports ATS and IOMMU supports Secure ATS)
> >> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS |
> >> IOMMU_HWPT_FLAGS_ATS
> >>     else if (device supports ATS and IOMMU supports ATS)
> >> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_ATS
> >>     else
> >> 	iommu_device->ats_flag = <NO FLAGS> (May be have one more flag
> >> IOMMU_HWPT_FLAGS_NO_ATS) ?
> >
> > Here the device->ats_flag should be more an ats_policy thing? At
> > probe time it's assigned with a default policy according to device
> > type, platform requirement, etc. and later exposed via sysfs for
> > admin opt.
> 
> Right. We can call it as ats_policy (expected/desired mode by HW driver).
> So now core knows HW requirement. Also default and global policy
> enforcement
> (via kernel command line).
> 
> During domain allocation, we can enforce required domain specific policy.

yes

> 
> 
> >
> > Actual ats enabling/enforcement is done at attach time, according
> > to ats_policy.
> 
> Then during attach_dev() we pass those flags as argument?

the policy is already stored in iommu_device so no need to pass.

and the domain also includes the ats flag at allocation time.

Then looks no need to pass additional flags at attach?

> 
> >
> >>
> >>
> >>   - Based on default setting, kernel command line option and probe() data,
> >> core
> >> layer can adjust ATS capability. i. e.
> >>      - If we have kernel command line (say iommu.ats=<x>), use that data
> for
> >> all
> >> ATS capable device.
> >>      - If not, apply default
> >>       untrusted device -> force secure ATS or no ats
> >>       If HW needs Secure ATS always , enable Secure ATS
> >>       For all other cases use ATS
> >
> > this is an interesting one, i.e. whether we still want a global iommu
> > option to enable sats for all applicable devices/domains. the pros
> > is that it is simple and allows underlying driver to add sats support
> > before a more flexible sysfs interface is ready. the downside is that
> > it's unclear which device/domain types are affected by sats on
> > different platforms w/o digging into the hw detail...
> 
> I preferred global option (that's why this patch added amd_iommu=sats)
> because
> as you said its simple, can be enforced at global level. Also w/ sysfs interface
> we have to update for each device/group indivisual. Another drawback is, w/
> sysfs first we have to unbind the driver (which may not be possible always),
> modify ATS setting, then bind driver again.
> 
> In Summary, I visualize it three level:
>   - Default policy implemented in kernel
>   - Global policy enforcement by admin via kernel command line
>   - Per group/device level enforcement via sysfs

that sounds like a path forward

> 
> Do we need something for iommufd/vfio? Where user can tell desired mode
> while
> allocating HWPT domains?
> 

we should not allow the user to change the ats mode/policy. Just the
option whether a hwpt needs ats, i.e. from functionality p.o.v.

Then whether this request can be satisfied subjects from the ats
policy/mode set by the administrator.

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

[Yi Liu]

On 2025/3/4 22:24, Jason Gunthorpe wrote:
> On Tue, Mar 04, 2025 at 03:45:54PM +0530, Vasant Hegde wrote:
>>>   never
>>>   always
>>>   paging-only
>>>   secure-always
>>>   secure-paging-only
>>
>> Why not just expose supported modes (No ATS, ATS, Secure ATS) per group and
>> allow admin to select within supported modes?  We may have to add more logic in
>> sysfs store/show code path. But user will know what is supported and what they
>> can pick.
> 
> A sysfs type thing is what I'm thinking
> 
> Just trying to understand what options we should be providing to
> userspace..
> 
> I am sensitive that we'd like to disble ATS for identity modes in many
> common cases and we get that implicitly today.
> 
>> Also sysfs knob needs new domain ops to communicate changes to indivisual
>> driver. Like in store path
>>    - validate the input
>>    - make sure driver is unbond so that its safe to modify the
>>    - New ops to communicate to indivisual driver
>> 	 something like ops->set_ats_mode(dev, mode)
> 
> Yeah this needs to be considered.. I wonder if a flags input to attach
> is the right way to go.

Do we expect this flag to setup the SATS in the underlying iommu driver?
If yes, I doubt if it is proper. My reason as below:

he reason is more for the #4 case you summarized in earlier mails of this
thread. Such HWs need to map the PA in the DPT/HPT table when a given PA is
mapped to IOVA. We may just hook the DPT/HPT map/unmap to the map/unmap of
a paging domain. Hence it's better to determine if the DPT/HPT is desired
per allocation. Otherwise it may requires upper layer to replay the
mapping. e.g. iommufd replay the mappings stored in IOAS->iopt

" 4) TA is an full physical address and the IOMMU validates the full
     physical using some kind of permission structure (ARM calls this
     Device Permission Table)
"

> 
> That would be convenient for how SMMUv3 is structued now as it can just
> switch modes hitlessly with its existing logic. I think all drivers
> should have a similar design.

I think this is more like we have done for the set_dev_pasid op. Now it
supports replacing domain. But as I replied in above, add a flag in the
attach op might not be a good idea.

-- 
Regards,
Yi Liu

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

[Yi Liu]

On 2025/3/4 18:15, Vasant Hegde wrote:
> Jason,
> 
> 
> On 3/4/2025 12:08 AM, 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?
> 
> Yes. If we are dong knob then we should support normal ATS as well.
> 
>>
>>   never
>>   always
>>   paging-only
>>   secure-always
>>   secure-paging-only
> 
> Why not just expose supported modes (No ATS, ATS, Secure ATS) per group and
> allow admin to select within supported modes?  We may have to add more logic in
> sysfs store/show code path. But user will know what is supported and what they
> can pick.
> 
> Also sysfs knob needs new domain ops to communicate changes to indivisual
> driver. Like in store path
>    - validate the input
>    - make sure driver is unbond so that its safe to modify the
>    - New ops to communicate to indivisual driver
> 	 something like ops->set_ats_mode(dev, mode)

Perhaps marking it in the core level is enough. Any domain that is going to
be used for this device should respect it. e.g. if admin change it to be
requiring SATS, then the default_domain should be reallocated with a flag
to indicate it.

-- 
Regards,
Yi Liu

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

[Yi Liu]

On 2025/3/4 22:20, Jason Gunthorpe wrote:
> On Tue, Mar 04, 2025 at 02:50:19PM +0800, Yi Liu wrote:
> 
>>> 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..
>>
>> will it affect Baolu's above refactoring [1]? :(
> 
> I think no, but when we get some conclusion how ATS should work I'd
> expect all IOMMU drivers to align
> 
>>>    never
>>>    always
>>>    paging-only
>>
>> this can be considered policy to the existing ATS. curious, why paging
>> domain is special here?
> 
> Using ATS with an IDENTITY translation is not a performance win, so
> you'd probably want to turn it off in that case.

got it. I see no value as well. :)

-- 
Regards,
Yi Liu

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

[Vasant Hegde]

Kevin,


On 3/12/2025 12:45 PM, Tian, Kevin wrote:
>> From: Vasant Hegde <vasant.hegde@amd.com>
>> Sent: Tuesday, March 11, 2025 1:07 AM
>>
>> On 3/5/2025 8:54 AM, Tian, Kevin wrote:
>>>> From: Vasant Hegde <vasant.hegde@amd.com>
>>>> Sent: Monday, March 3, 2025 7:42 PM
>>>>
>>>> Right. Lets see if below flow covers all the requirement
>>>>   - Define new flags
>>>>     IOMMU_HWPT_FLAGS_SATS / IOMMU_HWPT_FLAGS_ATS
>>>
>>> SATS policy should be managed by the sys admin, while ATS feature
>>> itself can be opted by the user.
>>
>> "user" you mean like iommufd during domain allocation?
> 
> yes
> 
>>>>
>>>>   - During probe() driver will update this field (similar to max_pasids)
>>>>     If (IOMMU needs Secure ATS always)
>>>> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS
>>>>     else if (device supports ATS and IOMMU supports Secure ATS)
>>>> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_SATS |
>>>> IOMMU_HWPT_FLAGS_ATS
>>>>     else if (device supports ATS and IOMMU supports ATS)
>>>> 	iommu_device->ats_flag = IOMMU_HWPT_FLAGS_ATS
>>>>     else
>>>> 	iommu_device->ats_flag = <NO FLAGS> (May be have one more flag
>>>> IOMMU_HWPT_FLAGS_NO_ATS) ?
>>>
>>> Here the device->ats_flag should be more an ats_policy thing? At
>>> probe time it's assigned with a default policy according to device
>>> type, platform requirement, etc. and later exposed via sysfs for
>>> admin opt.
>>
>> Right. We can call it as ats_policy (expected/desired mode by HW driver).
>> So now core knows HW requirement. Also default and global policy
>> enforcement
>> (via kernel command line).
>>
>> During domain allocation, we can enforce required domain specific policy.
> 
> yes
> 
>>
>>
>>>
>>> Actual ats enabling/enforcement is done at attach time, according
>>> to ats_policy.
>>
>> Then during attach_dev() we pass those flags as argument?
> 
> the policy is already stored in iommu_device so no need to pass.
> 
> and the domain also includes the ats flag at allocation time.
> 
> Then looks no need to pass additional flags at attach?

So when user request to change from ATS to secure ATS via sysfs :
  we tweak iommu_dev / domain and call attach_dev() ?



> 
>>
>>>
>>>>
>>>>
>>>>   - Based on default setting, kernel command line option and probe() data,
>>>> core
>>>> layer can adjust ATS capability. i. e.
>>>>      - If we have kernel command line (say iommu.ats=<x>), use that data
>> for
>>>> all
>>>> ATS capable device.
>>>>      - If not, apply default
>>>>       untrusted device -> force secure ATS or no ats
>>>>       If HW needs Secure ATS always , enable Secure ATS
>>>>       For all other cases use ATS
>>>
>>> this is an interesting one, i.e. whether we still want a global iommu
>>> option to enable sats for all applicable devices/domains. the pros
>>> is that it is simple and allows underlying driver to add sats support
>>> before a more flexible sysfs interface is ready. the downside is that
>>> it's unclear which device/domain types are affected by sats on
>>> different platforms w/o digging into the hw detail...
>>
>> I preferred global option (that's why this patch added amd_iommu=sats)
>> because
>> as you said its simple, can be enforced at global level. Also w/ sysfs interface
>> we have to update for each device/group indivisual. Another drawback is, w/
>> sysfs first we have to unbind the driver (which may not be possible always),
>> modify ATS setting, then bind driver again.
>>
>> In Summary, I visualize it three level:
>>   - Default policy implemented in kernel
>>   - Global policy enforcement by admin via kernel command line
>>   - Per group/device level enforcement via sysfs
> 
> that sounds like a path forward

Sure.

> 
>>
>> Do we need something for iommufd/vfio? Where user can tell desired mode
>> while
>> allocating HWPT domains?
>>
> 
> we should not allow the user to change the ats mode/policy. Just the
> option whether a hwpt needs ats, i.e. from functionality p.o.v.
> 
> Then whether this request can be satisfied subjects from the ats
> policy/mode set by the administrator.

Ok. Let me write down the details and then we can fine tune it.

-Vasant

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

[Jason Gunthorpe]

On Fri, Mar 14, 2025 at 08:09:37PM +0800, Yi Liu wrote:
> " 4) TA is an full physical address and the IOMMU validates the full
>     physical using some kind of permission structure (ARM calls this
>     Device Permission Table)
> "

I think selecting the two kinds of secure ATS would be done with
various flags and the DPT verions of it would only be selectable at
allocation time..

> > That would be convenient for how SMMUv3 is structued now as it can just
> > switch modes hitlessly with its existing logic. I think all drivers
> > should have a similar design.
> 
> I think this is more like we have done for the set_dev_pasid op. Now it
> supports replacing domain. But as I replied in above, add a flag in the
> attach op might not be a good idea.

Yes, you can't switch between DPT and non DPT modes hitlessly like
this as those would need new domain allocations.

But it could hitlessly switch between secure mode enabled/disabled or
ATS enabled/disabled without changing the active
struct iommu_domain *.

Jason

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

[Tian, Kevin]

> From: Vasant Hegde <vasant.hegde@amd.com>
> Sent: Monday, March 17, 2025 4:57 PM
> >>>
> >>> Actual ats enabling/enforcement is done at attach time, according
> >>> to ats_policy.
> >>
> >> Then during attach_dev() we pass those flags as argument?
> >
> > the policy is already stored in iommu_device so no need to pass.
> >
> > and the domain also includes the ats flag at allocation time.
> >
> > Then looks no need to pass additional flags at attach?
> 
> So when user request to change from ATS to secure ATS via sysfs :
>   we tweak iommu_dev / domain and call attach_dev() ?
> 

No. Attach is still initiated by iommufd and this is just an
attribute affecting the behavior of attach.

If the device is already attached then return -EBUSY in sysfs write.

otherwise update the attribute which will take effect at
next attach.