Skip to content

Commit 3a04446

Browse files
hegdevasantgregkh
authored andcommitted
iommu/amd: Reorder attach device code
[ Upstream commit 0b13649 ] Ideally in attach device path, it should take dev_data lock before making changes to device data including IOPF enablement. So far dev_data was using spinlock and it was hitting lock order issue when it tries to enable IOPF. Hence Commit 526606b ("iommu/amd: Fix Invalid wait context issue") moved IOPF enablement outside dev_data->lock. Previous patch converted dev_data lock to mutex. Now its safe to call amd_iommu_iopf_add_device() with dev_data->mutex. Hence move back PCI device capability enablement (ATS, PRI, PASID) and IOPF enablement code inside the lock. Also in attach_device(), update 'dev_data->domain' at the end so that error handling becomes simple. Signed-off-by: Vasant Hegde <vasant.hegde@amd.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Link: https://lore.kernel.org/r/20241030063556.6104-11-vasant.hegde@amd.com Signed-off-by: Joerg Roedel <jroedel@suse.de> Stable-dep-of: 4a552f7 ("iommu/amd: Put list_add/del(dev_data) back under the domain->lock") Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 3c3abbc commit 3a04446

1 file changed

Lines changed: 29 additions & 36 deletions

File tree

drivers/iommu/amd/iommu.c

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,6 +2291,7 @@ static int attach_device(struct device *dev,
22912291
{
22922292
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
22932293
struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
2294+
struct pci_dev *pdev;
22942295
int ret = 0;
22952296

22962297
mutex_lock(&dev_data->mutex);
@@ -2300,10 +2301,6 @@ static int attach_device(struct device *dev,
23002301
goto out;
23012302
}
23022303

2303-
/* Update data structures */
2304-
dev_data->domain = domain;
2305-
list_add(&dev_data->list, &domain->dev_list);
2306-
23072304
/* Do reference counting */
23082305
ret = pdom_attach_iommu(iommu, domain);
23092306
if (ret)
@@ -2318,6 +2315,28 @@ static int attach_device(struct device *dev,
23182315
}
23192316
}
23202317

2318+
pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
2319+
if (pdev && pdom_is_sva_capable(domain)) {
2320+
pdev_enable_caps(pdev);
2321+
2322+
/*
2323+
* Device can continue to function even if IOPF
2324+
* enablement failed. Hence in error path just
2325+
* disable device PRI support.
2326+
*/
2327+
if (amd_iommu_iopf_add_device(iommu, dev_data))
2328+
pdev_disable_cap_pri(pdev);
2329+
} else if (pdev) {
2330+
pdev_enable_cap_ats(pdev);
2331+
}
2332+
2333+
/* Update data structures */
2334+
dev_data->domain = domain;
2335+
list_add(&dev_data->list, &domain->dev_list);
2336+
2337+
/* Update device table */
2338+
dev_update_dte(dev_data, true);
2339+
23212340
out:
23222341
mutex_unlock(&dev_data->mutex);
23232342

@@ -2332,7 +2351,6 @@ static void detach_device(struct device *dev)
23322351
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
23332352
struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
23342353
struct protection_domain *domain = dev_data->domain;
2335-
bool ppr = dev_data->ppr;
23362354
unsigned long flags;
23372355

23382356
mutex_lock(&dev_data->mutex);
@@ -2346,13 +2364,15 @@ static void detach_device(struct device *dev)
23462364
if (WARN_ON(!dev_data->domain))
23472365
goto out;
23482366

2349-
if (ppr) {
2367+
/* Remove IOPF handler */
2368+
if (dev_data->ppr) {
23502369
iopf_queue_flush_dev(dev);
2351-
2352-
/* Updated here so that it gets reflected in DTE */
2353-
dev_data->ppr = false;
2370+
amd_iommu_iopf_remove_device(iommu, dev_data);
23542371
}
23552372

2373+
if (dev_is_pci(dev))
2374+
pdev_disable_caps(to_pci_dev(dev));
2375+
23562376
/* Clear DTE and flush the entry */
23572377
dev_update_dte(dev_data, false);
23582378

@@ -2374,14 +2394,6 @@ static void detach_device(struct device *dev)
23742394

23752395
out:
23762396
mutex_unlock(&dev_data->mutex);
2377-
2378-
/* Remove IOPF handler */
2379-
if (ppr)
2380-
amd_iommu_iopf_remove_device(iommu, dev_data);
2381-
2382-
if (dev_is_pci(dev))
2383-
pdev_disable_caps(to_pci_dev(dev));
2384-
23852397
}
23862398

23872399
static struct iommu_device *amd_iommu_probe_device(struct device *dev)
@@ -2670,7 +2682,6 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
26702682
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
26712683
struct protection_domain *domain = to_pdomain(dom);
26722684
struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
2673-
struct pci_dev *pdev;
26742685
int ret;
26752686

26762687
/*
@@ -2703,24 +2714,6 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
27032714
}
27042715
#endif
27052716

2706-
pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
2707-
if (pdev && pdom_is_sva_capable(domain)) {
2708-
pdev_enable_caps(pdev);
2709-
2710-
/*
2711-
* Device can continue to function even if IOPF
2712-
* enablement failed. Hence in error path just
2713-
* disable device PRI support.
2714-
*/
2715-
if (amd_iommu_iopf_add_device(iommu, dev_data))
2716-
pdev_disable_cap_pri(pdev);
2717-
} else if (pdev) {
2718-
pdev_enable_cap_ats(pdev);
2719-
}
2720-
2721-
/* Update device table */
2722-
dev_update_dte(dev_data, true);
2723-
27242717
return ret;
27252718
}
27262719

0 commit comments

Comments
 (0)