[PATCH v3 04/12] iommu/vt-d: Move scalable mode ATS enablement to probe path

Lu Baolu posted 12 patches 11 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 04/12] iommu/vt-d: Move scalable mode ATS enablement to probe path
Posted by Lu Baolu 11 months, 2 weeks ago
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>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 drivers/iommu/intel/iommu.c | 51 ++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 16dd8f0de76d..7efb32a0ac96 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1172,32 +1172,28 @@ static bool dev_needs_extra_dtlb_flush(struct pci_dev *pdev)
 	return true;
 }
 
-static void iommu_enable_pci_caps(struct device_domain_info *info)
+static void iommu_enable_pci_ats(struct device_domain_info *info)
 {
 	struct pci_dev *pdev;
 
-	if (!dev_is_pci(info->dev))
+	if (!info->ats_supported)
 		return;
 
 	pdev = to_pci_dev(info->dev);
-	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
-	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
+	if (!pci_ats_page_aligned(pdev))
+		return;
+
+	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
 		info->ats_enabled = 1;
 }
 
-static void iommu_disable_pci_caps(struct device_domain_info *info)
+static void iommu_disable_pci_ats(struct device_domain_info *info)
 {
-	struct pci_dev *pdev;
-
-	if (!dev_is_pci(info->dev))
+	if (!info->ats_enabled)
 		return;
 
-	pdev = to_pci_dev(info->dev);
-
-	if (info->ats_enabled) {
-		pci_disable_ats(pdev);
-		info->ats_enabled = 0;
-	}
+	pci_disable_ats(to_pci_dev(info->dev));
+	info->ats_enabled = 0;
 }
 
 static void intel_flush_iotlb_all(struct iommu_domain *domain)
@@ -1556,12 +1552,19 @@ 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;
+	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);
+	ret = pci_for_each_dma_alias(to_pci_dev(dev),
+				     domain_context_mapping_cb, domain);
+	if (ret)
+		return ret;
+
+	iommu_enable_pci_ats(info);
+
+	return 0;
 }
 
 /* Return largest possible superpage level for a given mapping */
@@ -1843,8 +1846,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;
@@ -3198,6 +3199,7 @@ static void domain_context_clear(struct device_domain_info *info)
 
 	pci_for_each_dma_alias(to_pci_dev(info->dev),
 			       &domain_context_clear_one_cb, info);
+	iommu_disable_pci_ats(info);
 }
 
 /*
@@ -3214,7 +3216,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 +3750,9 @@ 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))
+		iommu_enable_pci_ats(info);
+
 	return &iommu->iommu;
 free_table:
 	intel_pasid_free_table(dev);
@@ -3765,6 +3769,8 @@ 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;
 
+	iommu_disable_pci_ats(info);
+
 	if (info->pasid_enabled) {
 		pci_disable_pasid(to_pci_dev(dev));
 		info->pasid_enabled = 0;
@@ -4365,13 +4371,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.0
RE: [PATCH v3 04/12] iommu/vt-d: Move scalable mode ATS enablement to probe path
Posted by Tian, Kevin 11 months, 1 week ago
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, February 28, 2025 5:26 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.

could you extend it with your earlier reply?

https://lore.kernel.org/linux-iommu/6a418974-d06e-46e3-879f-ab4c84a95231@linux.intel.com/

> 
> -static void iommu_enable_pci_caps(struct device_domain_info *info)
> +static void iommu_enable_pci_ats(struct device_domain_info *info)
>  {
>  	struct pci_dev *pdev;
> 
> -	if (!dev_is_pci(info->dev))
> +	if (!info->ats_supported)
>  		return;
> 
>  	pdev = to_pci_dev(info->dev);
> -	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
> -	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> +	if (!pci_ats_page_aligned(pdev))
> +		return;
> +
> +	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
>  		info->ats_enabled = 1;
>  }
> 

still prefer to some comment above as you explained in above
reply. It's not obvious w/o knowing the tricky background.
Re: [PATCH v3 04/12] iommu/vt-d: Move scalable mode ATS enablement to probe path
Posted by Baolu Lu 11 months, 1 week ago
On 2025/3/4 16:15, Tian, Kevin wrote:
>> From: Lu Baolu<baolu.lu@linux.intel.com>
>> Sent: Friday, February 28, 2025 5:26 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.
> could you extend it with your earlier reply?
> 
> https://lore.kernel.org/linux-iommu/6a418974-d06e-46e3-879f- 
> ab4c84a95231@linux.intel.com/
> 
>> -static void iommu_enable_pci_caps(struct device_domain_info *info)
>> +static void iommu_enable_pci_ats(struct device_domain_info *info)
>>   {
>>   	struct pci_dev *pdev;
>>
>> -	if (!dev_is_pci(info->dev))
>> +	if (!info->ats_supported)
>>   		return;
>>
>>   	pdev = to_pci_dev(info->dev);
>> -	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
>> -	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT))
>> +	if (!pci_ats_page_aligned(pdev))
>> +		return;
>> +
>> +	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
>>   		info->ats_enabled = 1;
>>   }
>>
> still prefer to some comment above as you explained in above
> reply. It's not obvious w/o knowing the tricky background.

Okay, sure.