Skip to contentSkip to conversation

iommu/amd: Invalidate cache before removing device from domain list

PATCH Applied Vasant Hegde 3 participants 3 messages Jun 20, 2024 – Jun 25, 2024

Subsystems (2)
lore
Share & tools
lore

Vasant Hegde

author maintainer
#0 lore raw

PATCH 1/1 iommu/amd: Invalidate cache before removing device from domain list

Commit 87a6f1f22c97 ("iommu/amd: Introduce per-device domain ID to fix potential TLB aliasing issue") introduced per device domain ID when domain is configured with v2 page table. And in invalidation path, it uses per device structure (dev_data->gcr3_info.domid) to get the domain ID.

In detach_device() path, current code tries to invalidate IOMMU cache after removing dev_data from domain device list. This means when domain is configured with v2 page table, amd_iommu_domain_flush_all() will not be able to invalidate cache as device is already removed from domain device list.

This is causing change domain tests (changing domain type from identity to DMA) to fail with IO_PAGE_FAULT issue.

Hence invalidate cache and update DTE before updating data structures.

Fixes 87a6f1f22c97 ("iommu/amd: Introduce per-device domain ID to fix potential TLB aliasing issue")
---
 drivers/iommu/amd/iommu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/iommu/amd/iommu.c +6 -6
index c2703599bb16..b19e8c0f48fa 100644
@@ -2061,6 +2061,12 @@ static void do_detach(struct iommu_dev_data *dev_data)
 	struct protection_domain *domain = dev_data->domain;
 	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
 
+	/* Clear DTE and flush the entry */
+	amd_iommu_dev_update_dte(dev_data, false);
+
+	/* Flush IOTLB and wait for the flushes to finish */
+	amd_iommu_domain_flush_all(domain);
+
 	/* Clear GCR3 table */
 	if (pdom_is_sva_capable(domain))
 		destroy_gcr3_table(dev_data, domain);
@@ -2069,12 +2075,6 @@ static void do_detach(struct iommu_dev_data *dev_data)
 	dev_data->domain = NULL;
 	list_del(&dev_data->list);
 
-	/* Clear DTE and flush the entry */
-	amd_iommu_dev_update_dte(dev_data, false);
-
-	/* Flush IOTLB and wait for the flushes to finish */
-	amd_iommu_domain_flush_all(domain);
-
 	/* decrease reference counters - needs to happen after the flushes */
 	domain->dev_iommu[iommu->index] -= 1;
 	domain->dev_cnt                 -= 1;
Signature · 1 line
2.31.1
maintainer Applied
#1 lore raw
Quoted 2 lines · Vasant Hegde
 drivers/iommu/amd/iommu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Applied, thanks.

Reviewed-by on v1 1/1
#2 lore raw
In reply to Vasant Hegde
Quoted 16 lines · Vasant Hegde

Commit 87a6f1f22c97 ("iommu/amd: Introduce per-device domain ID to fix potential TLB aliasing issue") introduced per device domain ID when domain is configured with v2 page table. And in invalidation path, it uses per device structure (dev_data->gcr3_info.domid) to get the domain ID.

In detach_device() path, current code tries to invalidate IOMMU cache after removing dev_data from domain device list. This means when domain is configured with v2 page table, amd_iommu_domain_flush_all() will not be able to invalidate cache as device is already removed from domain device list.

This is causing change domain tests (changing domain type from identity to DMA) to fail with IO_PAGE_FAULT issue.

Hence invalidate cache and update DTE before updating data structures.

Quoted 7 lines
Fixes 87a6f1f22c97 ("iommu/amd: Introduce per-device domain ID to fix potential TLB aliasing issue")
Quoted 37 lines
---
 drivers/iommu/amd/iommu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/iommu/amd/iommu.c +6 -6
index c2703599bb16..b19e8c0f48fa 100644
@@ -2061,6 +2061,12 @@ static void do_detach(struct iommu_dev_data *dev_data)
 	struct protection_domain *domain = dev_data->domain;
 	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
 
+	/* Clear DTE and flush the entry */
+	amd_iommu_dev_update_dte(dev_data, false);
+
+	/* Flush IOTLB and wait for the flushes to finish */
+	amd_iommu_domain_flush_all(domain);
+
 	/* Clear GCR3 table */
 	if (pdom_is_sva_capable(domain))
 		destroy_gcr3_table(dev_data, domain);
@@ -2069,12 +2075,6 @@ static void do_detach(struct iommu_dev_data *dev_data)
 	dev_data->domain = NULL;
 	list_del(&dev_data->list);
 
-	/* Clear DTE and flush the entry */
-	amd_iommu_dev_update_dte(dev_data, false);
-
-	/* Flush IOTLB and wait for the flushes to finish */
-	amd_iommu_domain_flush_all(domain);
-
 	/* decrease reference counters - needs to happen after the flushes */
 	domain->dev_iommu[iommu->index] -= 1;
 	domain->dev_cnt                 -= 1;
Signature · 1 line
2.31.1