Skip to content

Commit 0b13649

Browse files
hegdevasantjoergroedel
authored andcommitted
iommu/amd: Reorder attach device code
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>
1 parent e843aed commit 0b13649

File tree

1 file changed

+29
-36
lines changed

1 file changed

+29
-36
lines changed

drivers/iommu/amd/iommu.c

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,6 +2090,7 @@ static int attach_device(struct device *dev,
20902090
{
20912091
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
20922092
struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
2093+
struct pci_dev *pdev;
20932094
int ret = 0;
20942095

20952096
mutex_lock(&dev_data->mutex);
@@ -2099,10 +2100,6 @@ static int attach_device(struct device *dev,
20992100
goto out;
21002101
}
21012102

2102-
/* Update data structures */
2103-
dev_data->domain = domain;
2104-
list_add(&dev_data->list, &domain->dev_list);
2105-
21062103
/* Do reference counting */
21072104
ret = pdom_attach_iommu(iommu, domain);
21082105
if (ret)
@@ -2117,6 +2114,28 @@ static int attach_device(struct device *dev,
21172114
}
21182115
}
21192116

2117+
pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
2118+
if (pdev && pdom_is_sva_capable(domain)) {
2119+
pdev_enable_caps(pdev);
2120+
2121+
/*
2122+
* Device can continue to function even if IOPF
2123+
* enablement failed. Hence in error path just
2124+
* disable device PRI support.
2125+
*/
2126+
if (amd_iommu_iopf_add_device(iommu, dev_data))
2127+
pdev_disable_cap_pri(pdev);
2128+
} else if (pdev) {
2129+
pdev_enable_cap_ats(pdev);
2130+
}
2131+
2132+
/* Update data structures */
2133+
dev_data->domain = domain;
2134+
list_add(&dev_data->list, &domain->dev_list);
2135+
2136+
/* Update device table */
2137+
dev_update_dte(dev_data, true);
2138+
21202139
out:
21212140
mutex_unlock(&dev_data->mutex);
21222141

@@ -2131,7 +2150,6 @@ static void detach_device(struct device *dev)
21312150
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
21322151
struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
21332152
struct protection_domain *domain = dev_data->domain;
2134-
bool ppr = dev_data->ppr;
21352153
unsigned long flags;
21362154

21372155
mutex_lock(&dev_data->mutex);
@@ -2145,13 +2163,15 @@ static void detach_device(struct device *dev)
21452163
if (WARN_ON(!dev_data->domain))
21462164
goto out;
21472165

2148-
if (ppr) {
2166+
/* Remove IOPF handler */
2167+
if (dev_data->ppr) {
21492168
iopf_queue_flush_dev(dev);
2150-
2151-
/* Updated here so that it gets reflected in DTE */
2152-
dev_data->ppr = false;
2169+
amd_iommu_iopf_remove_device(iommu, dev_data);
21532170
}
21542171

2172+
if (dev_is_pci(dev))
2173+
pdev_disable_caps(to_pci_dev(dev));
2174+
21552175
/* Clear DTE and flush the entry */
21562176
dev_update_dte(dev_data, false);
21572177

@@ -2173,14 +2193,6 @@ static void detach_device(struct device *dev)
21732193

21742194
out:
21752195
mutex_unlock(&dev_data->mutex);
2176-
2177-
/* Remove IOPF handler */
2178-
if (ppr)
2179-
amd_iommu_iopf_remove_device(iommu, dev_data);
2180-
2181-
if (dev_is_pci(dev))
2182-
pdev_disable_caps(to_pci_dev(dev));
2183-
21842196
}
21852197

21862198
static struct iommu_device *amd_iommu_probe_device(struct device *dev)
@@ -2509,7 +2521,6 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
25092521
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
25102522
struct protection_domain *domain = to_pdomain(dom);
25112523
struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
2512-
struct pci_dev *pdev;
25132524
int ret;
25142525

25152526
/*
@@ -2542,24 +2553,6 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
25422553
}
25432554
#endif
25442555

2545-
pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
2546-
if (pdev && pdom_is_sva_capable(domain)) {
2547-
pdev_enable_caps(pdev);
2548-
2549-
/*
2550-
* Device can continue to function even if IOPF
2551-
* enablement failed. Hence in error path just
2552-
* disable device PRI support.
2553-
*/
2554-
if (amd_iommu_iopf_add_device(iommu, dev_data))
2555-
pdev_disable_cap_pri(pdev);
2556-
} else if (pdev) {
2557-
pdev_enable_cap_ats(pdev);
2558-
}
2559-
2560-
/* Update device table */
2561-
dev_update_dte(dev_data, true);
2562-
25632556
return ret;
25642557
}
25652558

0 commit comments

Comments
 (0)