Device ATS is currently enabled when a domain is attached to the device
and disabled when the domain is detached. This creates a limitation:
when the IOMMU is operating in scalable mode and IOPF is enabled, the
device's domain cannot be changed.
Remove this limitation by moving ATS enablement to the device probe path.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/iommu.c | 78 ++++++++++++++++++-------------------
1 file changed, 38 insertions(+), 40 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 16dd8f0de76d..f52602bde742 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1172,34 +1172,6 @@ static bool dev_needs_extra_dtlb_flush(struct pci_dev *pdev)
return true;
}
-static void iommu_enable_pci_caps(struct device_domain_info *info)
-{
- struct pci_dev *pdev;
-
- if (!dev_is_pci(info->dev))
- return;
-
- pdev = to_pci_dev(info->dev);
- if (info->ats_supported && pci_ats_page_aligned(pdev) &&
- !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
- info->ats_enabled = 1;
-}
-
-static void iommu_disable_pci_caps(struct device_domain_info *info)
-{
- struct pci_dev *pdev;
-
- if (!dev_is_pci(info->dev))
- return;
-
- pdev = to_pci_dev(info->dev);
-
- if (info->ats_enabled) {
- pci_disable_ats(pdev);
- info->ats_enabled = 0;
- }
-}
-
static void intel_flush_iotlb_all(struct iommu_domain *domain)
{
cache_tag_flush_all(to_dmar_domain(domain));
@@ -1556,12 +1528,22 @@ domain_context_mapping(struct dmar_domain *domain, struct device *dev)
struct device_domain_info *info = dev_iommu_priv_get(dev);
struct intel_iommu *iommu = info->iommu;
u8 bus = info->bus, devfn = info->devfn;
+ struct pci_dev *pdev;
+ int ret;
if (!dev_is_pci(dev))
return domain_context_mapping_one(domain, iommu, bus, devfn);
- return pci_for_each_dma_alias(to_pci_dev(dev),
- domain_context_mapping_cb, domain);
+ pdev = to_pci_dev(dev);
+ ret = pci_for_each_dma_alias(pdev, domain_context_mapping_cb, domain);
+ if (ret)
+ return ret;
+
+ if (info->ats_supported && pci_ats_page_aligned(pdev) &&
+ !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
+ info->ats_enabled = 1;
+
+ return 0;
}
/* Return largest possible superpage level for a given mapping */
@@ -1843,8 +1825,6 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
if (ret)
goto out_block_translation;
- iommu_enable_pci_caps(info);
-
ret = cache_tag_assign_domain(domain, dev, IOMMU_NO_PASID);
if (ret)
goto out_block_translation;
@@ -3191,13 +3171,20 @@ static int domain_context_clear_one_cb(struct pci_dev *pdev, u16 alias, void *op
*/
static void domain_context_clear(struct device_domain_info *info)
{
+ struct pci_dev *pdev;
+
if (!dev_is_pci(info->dev)) {
domain_context_clear_one(info, info->bus, info->devfn);
return;
}
- pci_for_each_dma_alias(to_pci_dev(info->dev),
- &domain_context_clear_one_cb, info);
+ pdev = to_pci_dev(info->dev);
+ pci_for_each_dma_alias(pdev, &domain_context_clear_one_cb, info);
+
+ if (info->ats_enabled) {
+ pci_disable_ats(pdev);
+ info->ats_enabled = 0;
+ }
}
/*
@@ -3214,7 +3201,6 @@ void device_block_translation(struct device *dev)
if (info->domain)
cache_tag_unassign_domain(info->domain, dev, IOMMU_NO_PASID);
- iommu_disable_pci_caps(info);
if (!dev_is_real_dma_subdevice(dev)) {
if (sm_supported(iommu))
intel_pasid_tear_down_entry(iommu, dev,
@@ -3749,6 +3735,16 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
!pci_enable_pasid(pdev, info->pasid_supported & ~1))
info->pasid_enabled = 1;
+ if (sm_supported(iommu)) {
+ if (info->ats_supported && pci_ats_page_aligned(pdev)) {
+ ret = pci_enable_ats(pdev, VTD_PAGE_SHIFT);
+ if (ret)
+ pci_info(pdev, "Failed to enable ATS on device\n");
+ else
+ info->ats_enabled = 1;
+ }
+ }
+
return &iommu->iommu;
free_table:
intel_pasid_free_table(dev);
@@ -3765,6 +3761,11 @@ static void intel_iommu_release_device(struct device *dev)
struct device_domain_info *info = dev_iommu_priv_get(dev);
struct intel_iommu *iommu = info->iommu;
+ if (info->ats_enabled) {
+ pci_disable_ats(to_pci_dev(dev));
+ info->ats_enabled = 0;
+ }
+
if (info->pasid_enabled) {
pci_disable_pasid(to_pci_dev(dev));
info->pasid_enabled = 0;
@@ -4365,13 +4366,10 @@ static int identity_domain_attach_dev(struct iommu_domain *domain, struct device
if (dev_is_real_dma_subdevice(dev))
return 0;
- if (sm_supported(iommu)) {
+ if (sm_supported(iommu))
ret = intel_pasid_setup_pass_through(iommu, dev, IOMMU_NO_PASID);
- if (!ret)
- iommu_enable_pci_caps(info);
- } else {
+ else
ret = device_setup_pass_through(dev);
- }
return ret;
}
--
2.43.0On 2025/2/24 13:16, Lu Baolu wrote:
> Device ATS is currently enabled when a domain is attached to the device
> and disabled when the domain is detached. This creates a limitation:
> when the IOMMU is operating in scalable mode and IOPF is enabled, the
> device's domain cannot be changed.
>
> Remove this limitation by moving ATS enablement to the device probe path.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/iommu.c | 78 ++++++++++++++++++-------------------
> 1 file changed, 38 insertions(+), 40 deletions(-)
I'm ok with this patch. Just a heads up in case of anyone that is not aware
of a discussion in another threa which intends to enable ATS in domain
attach.
[1]
https://lore.kernel.org/linux-iommu/2c9ef073-fee5-43c6-8932-a8cae677970e@intel.com/
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 16dd8f0de76d..f52602bde742 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1172,34 +1172,6 @@ static bool dev_needs_extra_dtlb_flush(struct pci_dev *pdev)
> return true;
> }
>
> -static void iommu_enable_pci_caps(struct device_domain_info *info)
> -{
> - struct pci_dev *pdev;
> -
> - if (!dev_is_pci(info->dev))
> - return;
> -
> - pdev = to_pci_dev(info->dev);
> - if (info->ats_supported && pci_ats_page_aligned(pdev) &&
> - !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> - info->ats_enabled = 1;
> -}
> -
> -static void iommu_disable_pci_caps(struct device_domain_info *info)
> -{
> - struct pci_dev *pdev;
> -
> - if (!dev_is_pci(info->dev))
> - return;
> -
> - pdev = to_pci_dev(info->dev);
> -
> - if (info->ats_enabled) {
> - pci_disable_ats(pdev);
> - info->ats_enabled = 0;
> - }
> -}
> -
> static void intel_flush_iotlb_all(struct iommu_domain *domain)
> {
> cache_tag_flush_all(to_dmar_domain(domain));
> @@ -1556,12 +1528,22 @@ domain_context_mapping(struct dmar_domain *domain, struct device *dev)
> struct device_domain_info *info = dev_iommu_priv_get(dev);
> struct intel_iommu *iommu = info->iommu;
> u8 bus = info->bus, devfn = info->devfn;
> + struct pci_dev *pdev;
> + int ret;
>
> if (!dev_is_pci(dev))
> return domain_context_mapping_one(domain, iommu, bus, devfn);
>
> - return pci_for_each_dma_alias(to_pci_dev(dev),
> - domain_context_mapping_cb, domain);
> + pdev = to_pci_dev(dev);
> + ret = pci_for_each_dma_alias(pdev, domain_context_mapping_cb, domain);
> + if (ret)
> + return ret;
> +
> + if (info->ats_supported && pci_ats_page_aligned(pdev) &&
> + !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> + info->ats_enabled = 1;
> +
> + return 0;
> }
>
> /* Return largest possible superpage level for a given mapping */
> @@ -1843,8 +1825,6 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
> if (ret)
> goto out_block_translation;
>
> - iommu_enable_pci_caps(info);
> -
> ret = cache_tag_assign_domain(domain, dev, IOMMU_NO_PASID);
> if (ret)
> goto out_block_translation;
> @@ -3191,13 +3171,20 @@ static int domain_context_clear_one_cb(struct pci_dev *pdev, u16 alias, void *op
> */
> static void domain_context_clear(struct device_domain_info *info)
> {
> + struct pci_dev *pdev;
> +
> if (!dev_is_pci(info->dev)) {
> domain_context_clear_one(info, info->bus, info->devfn);
> return;
> }
>
> - pci_for_each_dma_alias(to_pci_dev(info->dev),
> - &domain_context_clear_one_cb, info);
> + pdev = to_pci_dev(info->dev);
> + pci_for_each_dma_alias(pdev, &domain_context_clear_one_cb, info);
> +
> + if (info->ats_enabled) {
> + pci_disable_ats(pdev);
> + info->ats_enabled = 0;
> + }
> }
>
> /*
> @@ -3214,7 +3201,6 @@ void device_block_translation(struct device *dev)
> if (info->domain)
> cache_tag_unassign_domain(info->domain, dev, IOMMU_NO_PASID);
>
> - iommu_disable_pci_caps(info);
> if (!dev_is_real_dma_subdevice(dev)) {
> if (sm_supported(iommu))
> intel_pasid_tear_down_entry(iommu, dev,
> @@ -3749,6 +3735,16 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
> !pci_enable_pasid(pdev, info->pasid_supported & ~1))
> info->pasid_enabled = 1;
>
> + if (sm_supported(iommu)) {
> + if (info->ats_supported && pci_ats_page_aligned(pdev)) {
> + ret = pci_enable_ats(pdev, VTD_PAGE_SHIFT);
> + if (ret)
> + pci_info(pdev, "Failed to enable ATS on device\n");
> + else
> + info->ats_enabled = 1;
> + }
> + }
> +
> return &iommu->iommu;
> free_table:
> intel_pasid_free_table(dev);
> @@ -3765,6 +3761,11 @@ static void intel_iommu_release_device(struct device *dev)
> struct device_domain_info *info = dev_iommu_priv_get(dev);
> struct intel_iommu *iommu = info->iommu;
>
> + if (info->ats_enabled) {
> + pci_disable_ats(to_pci_dev(dev));
> + info->ats_enabled = 0;
> + }
> +
> if (info->pasid_enabled) {
> pci_disable_pasid(to_pci_dev(dev));
> info->pasid_enabled = 0;
> @@ -4365,13 +4366,10 @@ static int identity_domain_attach_dev(struct iommu_domain *domain, struct device
> if (dev_is_real_dma_subdevice(dev))
> return 0;
>
> - if (sm_supported(iommu)) {
> + if (sm_supported(iommu))
> ret = intel_pasid_setup_pass_through(iommu, dev, IOMMU_NO_PASID);
> - if (!ret)
> - iommu_enable_pci_caps(info);
> - } else {
> + else
> ret = device_setup_pass_through(dev);
> - }
>
> return ret;
> }
--
Regards,
Yi Liu
> From: Lu Baolu <baolu.lu@linux.intel.com> > Sent: Monday, February 24, 2025 1:16 PM > > Device ATS is currently enabled when a domain is attached to the device > and disabled when the domain is detached. This creates a limitation: > when the IOMMU is operating in scalable mode and IOPF is enabled, the > device's domain cannot be changed. I got what this patch does, but not this description. Can you elaborate about the limitation? > @@ -1556,12 +1528,22 @@ domain_context_mapping(struct dmar_domain > *domain, struct device *dev) > struct device_domain_info *info = dev_iommu_priv_get(dev); > struct intel_iommu *iommu = info->iommu; > u8 bus = info->bus, devfn = info->devfn; > + struct pci_dev *pdev; > + int ret; > > if (!dev_is_pci(dev)) > return domain_context_mapping_one(domain, iommu, bus, > devfn); > > - return pci_for_each_dma_alias(to_pci_dev(dev), > - domain_context_mapping_cb, domain); > + pdev = to_pci_dev(dev); > + ret = pci_for_each_dma_alias(pdev, domain_context_mapping_cb, > domain); > + if (ret) > + return ret; > + > + if (info->ats_supported && pci_ats_page_aligned(pdev) && > + !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) > + info->ats_enabled = 1; > + > + return 0; > } It'd be good to add a note here for why legacy mode still requires dynamic toggle at attach/detach time. It's not obvious w/o knowing the story about legacy + identity. btw the same enabling logic occurs in multiple places. Probably you can still make a helper for that. Otherwise, Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On 2/25/25 15:28, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Monday, February 24, 2025 1:16 PM
>>
>> Device ATS is currently enabled when a domain is attached to the device
>> and disabled when the domain is detached. This creates a limitation:
>> when the IOMMU is operating in scalable mode and IOPF is enabled, the
>> device's domain cannot be changed.
>
> I got what this patch does, but not this description. Can you elaborate
> about the limitation?
Okay, sure.
The previous code enables ATS when a domain is set to a device's RID and
disables it during RID domain switch. So, if a PASID is set with a
domain requiring PRI, ATS should remain enabled until the domain is
removed. During the PASID domain's lifecycle, if the RID's domain
changes, PRI will be disrupted because it depends on ATS, which is
disabled when the blocking domain is set for the device's RID.
>
>> @@ -1556,12 +1528,22 @@ domain_context_mapping(struct dmar_domain
>> *domain, struct device *dev)
>> struct device_domain_info *info = dev_iommu_priv_get(dev);
>> struct intel_iommu *iommu = info->iommu;
>> u8 bus = info->bus, devfn = info->devfn;
>> + struct pci_dev *pdev;
>> + int ret;
>>
>> if (!dev_is_pci(dev))
>> return domain_context_mapping_one(domain, iommu, bus,
>> devfn);
>>
>> - return pci_for_each_dma_alias(to_pci_dev(dev),
>> - domain_context_mapping_cb, domain);
>> + pdev = to_pci_dev(dev);
>> + ret = pci_for_each_dma_alias(pdev, domain_context_mapping_cb,
>> domain);
>> + if (ret)
>> + return ret;
>> +
>> + if (info->ats_supported && pci_ats_page_aligned(pdev) &&
>> + !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
>> + info->ats_enabled = 1;
>> +
>> + return 0;
>> }
>
> It'd be good to add a note here for why legacy mode still requires
> dynamic toggle at attach/detach time. It's not obvious w/o knowing
> the story about legacy + identity.
"legacy + identity" is part of the reason. Additionally, in legacy mode,
the ATS control bit is defined as a translation type and should be set
together with the page table pointer in the context entry. Separating
ATS enablement and the translation page table into different places
would make the code more complex and fragile.
> btw the same enabling logic occurs in multiple places. Probably
> you can still make a helper for that.
Yes, I will make a helper like this,
+static void device_enable_pci_ats(struct pci_dev *pdev)
+{
+ struct device_domain_info *info = dev_iommu_priv_get(&pdev->dev);
+
+ if (!info->ats_supported)
+ return;
+
+ if (!pci_ats_page_aligned(pdev))
+ return;
+
+ if(!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
+ info->ats_enabled = 1;
+}
> Otherwise,
>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Thanks,
baolu
© 2016 - 2025 Red Hat, Inc.