Skip to content

Commit 526606b

Browse files
hegdevasantjoergroedel
authored andcommitted
iommu/amd: Fix Invalid wait context issue
With commit c4cb231 ("iommu/amd: Add support for enable/disable IOPF") we are hitting below issue. This happens because in IOPF enablement path it holds spin lock with irq disable and then tries to take mutex lock. dmesg: ----- [ 0.938739] ============================= [ 0.938740] [ BUG: Invalid wait context ] [ 0.938742] 6.10.0-rc1+ #1 Not tainted [ 0.938745] ----------------------------- [ 0.938746] swapper/0/1 is trying to lock: [ 0.938748] ffffffff8c9f01d8 (&port_lock_key){....}-{3:3}, at: serial8250_console_write+0x78/0x4a0 [ 0.938767] other info that might help us debug this: [ 0.938768] context-{5:5} [ 0.938769] 7 locks held by swapper/0/1: [ 0.938772] #0: ffff888101a91310 (&group->mutex){+.+.}-{4:4}, at: bus_iommu_probe+0x70/0x160 [ 0.938790] #1: ffff888101d1f1b8 (&domain->lock){....}-{3:3}, at: amd_iommu_attach_device+0xa5/0x700 [ 0.938799] #2: ffff888101cc3d18 (&dev_data->lock){....}-{3:3}, at: amd_iommu_attach_device+0xc5/0x700 [ 0.938806] #3: ffff888100052830 (&iommu->lock){....}-{2:2}, at: amd_iommu_iopf_add_device+0x3f/0xa0 [ 0.938813] #4: ffffffff8945a340 (console_lock){+.+.}-{0:0}, at: _printk+0x48/0x50 [ 0.938822] #5: ffffffff8945a390 (console_srcu){....}-{0:0}, at: console_flush_all+0x58/0x4e0 [ 0.938867] #6: ffffffff82459f80 (console_owner){....}-{0:0}, at: console_flush_all+0x1f0/0x4e0 [ 0.938872] stack backtrace: [ 0.938874] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.10.0-rc1+ #1 [ 0.938877] Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.39 04/16/2019 Fix above issue by re-arranging code in attach device path: - move device PASID/IOPF enablement outside lock in AMD IOMMU driver. This is safe as core layer holds group->mutex lock before calling iommu_ops->attach_dev. Reported-by: Borislav Petkov <[email protected]> Reported-by: Mikhail Gavrilov <[email protected]> Reported-by: Chris Bainbridge <[email protected]> Fixes: c4cb231 ("iommu/amd: Add support for enable/disable IOPF") Tested-by: Borislav Petkov <[email protected]> Tested-by: Chris Bainbridge <[email protected]> Tested-by: Mikhail Gavrilov <[email protected]> Signed-off-by: Vasant Hegde <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]>
1 parent 48dc345 commit 526606b

File tree

2 files changed

+28
-42
lines changed

2 files changed

+28
-42
lines changed

drivers/iommu/amd/iommu.c

+24-24
Original file line numberDiff line numberDiff line change
@@ -2032,7 +2032,6 @@ static int do_attach(struct iommu_dev_data *dev_data,
20322032
struct protection_domain *domain)
20332033
{
20342034
struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
2035-
struct pci_dev *pdev;
20362035
int ret = 0;
20372036

20382037
/* Update data structures */
@@ -2047,30 +2046,13 @@ static int do_attach(struct iommu_dev_data *dev_data,
20472046
domain->dev_iommu[iommu->index] += 1;
20482047
domain->dev_cnt += 1;
20492048

2050-
pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
2049+
/* Setup GCR3 table */
20512050
if (pdom_is_sva_capable(domain)) {
20522051
ret = init_gcr3_table(dev_data, domain);
20532052
if (ret)
20542053
return ret;
2055-
2056-
if (pdev) {
2057-
pdev_enable_caps(pdev);
2058-
2059-
/*
2060-
* Device can continue to function even if IOPF
2061-
* enablement failed. Hence in error path just
2062-
* disable device PRI support.
2063-
*/
2064-
if (amd_iommu_iopf_add_device(iommu, dev_data))
2065-
pdev_disable_cap_pri(pdev);
2066-
}
2067-
} else if (pdev) {
2068-
pdev_enable_cap_ats(pdev);
20692054
}
20702055

2071-
/* Update device table */
2072-
amd_iommu_dev_update_dte(dev_data, true);
2073-
20742056
return ret;
20752057
}
20762058

@@ -2163,17 +2145,18 @@ static void detach_device(struct device *dev)
21632145

21642146
do_detach(dev_data);
21652147

2148+
out:
2149+
spin_unlock(&dev_data->lock);
2150+
2151+
spin_unlock_irqrestore(&domain->lock, flags);
2152+
21662153
/* Remove IOPF handler */
21672154
if (ppr)
21682155
amd_iommu_iopf_remove_device(iommu, dev_data);
21692156

21702157
if (dev_is_pci(dev))
21712158
pdev_disable_caps(to_pci_dev(dev));
21722159

2173-
out:
2174-
spin_unlock(&dev_data->lock);
2175-
2176-
spin_unlock_irqrestore(&domain->lock, flags);
21772160
}
21782161

21792162
static struct iommu_device *amd_iommu_probe_device(struct device *dev)
@@ -2485,6 +2468,7 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
24852468
struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
24862469
struct protection_domain *domain = to_pdomain(dom);
24872470
struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
2471+
struct pci_dev *pdev;
24882472
int ret;
24892473

24902474
/*
@@ -2517,7 +2501,23 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
25172501
}
25182502
#endif
25192503

2520-
iommu_completion_wait(iommu);
2504+
pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
2505+
if (pdev && pdom_is_sva_capable(domain)) {
2506+
pdev_enable_caps(pdev);
2507+
2508+
/*
2509+
* Device can continue to function even if IOPF
2510+
* enablement failed. Hence in error path just
2511+
* disable device PRI support.
2512+
*/
2513+
if (amd_iommu_iopf_add_device(iommu, dev_data))
2514+
pdev_disable_cap_pri(pdev);
2515+
} else if (pdev) {
2516+
pdev_enable_cap_ats(pdev);
2517+
}
2518+
2519+
/* Update device table */
2520+
amd_iommu_dev_update_dte(dev_data, true);
25212521

25222522
return ret;
25232523
}

drivers/iommu/amd/ppr.c

+4-18
Original file line numberDiff line numberDiff line change
@@ -248,40 +248,26 @@ void amd_iommu_page_response(struct device *dev, struct iopf_fault *evt,
248248
int amd_iommu_iopf_add_device(struct amd_iommu *iommu,
249249
struct iommu_dev_data *dev_data)
250250
{
251-
unsigned long flags;
252251
int ret = 0;
253252

254253
if (!dev_data->pri_enabled)
255254
return ret;
256255

257-
raw_spin_lock_irqsave(&iommu->lock, flags);
258-
259-
if (!iommu->iopf_queue) {
260-
ret = -EINVAL;
261-
goto out_unlock;
262-
}
256+
if (!iommu->iopf_queue)
257+
return -EINVAL;
263258

264259
ret = iopf_queue_add_device(iommu->iopf_queue, dev_data->dev);
265260
if (ret)
266-
goto out_unlock;
261+
return ret;
267262

268263
dev_data->ppr = true;
269-
270-
out_unlock:
271-
raw_spin_unlock_irqrestore(&iommu->lock, flags);
272-
return ret;
264+
return 0;
273265
}
274266

275267
/* Its assumed that caller has verified that device was added to iopf queue */
276268
void amd_iommu_iopf_remove_device(struct amd_iommu *iommu,
277269
struct iommu_dev_data *dev_data)
278270
{
279-
unsigned long flags;
280-
281-
raw_spin_lock_irqsave(&iommu->lock, flags);
282-
283271
iopf_queue_remove_device(iommu->iopf_queue, dev_data->dev);
284272
dev_data->ppr = false;
285-
286-
raw_spin_unlock_irqrestore(&iommu->lock, flags);
287273
}

0 commit comments

Comments
 (0)