Skip to content

Commit c424b18

Browse files
LuBaolugregkh
authored andcommitted
iommu/vt-d: Fix lockdep splat in intel_pasid_get_entry()
[ Upstream commit 803766c ] The pasid_lock is used to synchronize different threads from modifying a same pasid directory entry at the same time. It causes below lockdep splat. [ 83.296538] ======================================================== [ 83.296538] WARNING: possible irq lock inversion dependency detected [ 83.296539] 5.12.0-rc3+ #25 Tainted: G W [ 83.296539] -------------------------------------------------------- [ 83.296540] bash/780 just changed the state of lock: [ 83.296540] ffffffff82b29c98 (device_domain_lock){..-.}-{2:2}, at: iommu_flush_dev_iotlb.part.0+0x32/0x110 [ 83.296547] but this lock took another, SOFTIRQ-unsafe lock in the past: [ 83.296547] (pasid_lock){+.+.}-{2:2} [ 83.296548] and interrupts could create inverse lock ordering between them. [ 83.296549] other info that might help us debug this: [ 83.296549] Chain exists of: device_domain_lock --> &iommu->lock --> pasid_lock [ 83.296551] Possible interrupt unsafe locking scenario: [ 83.296551] CPU0 CPU1 [ 83.296552] ---- ---- [ 83.296552] lock(pasid_lock); [ 83.296553] local_irq_disable(); [ 83.296553] lock(device_domain_lock); [ 83.296554] lock(&iommu->lock); [ 83.296554] <Interrupt> [ 83.296554] lock(device_domain_lock); [ 83.296555] *** DEADLOCK *** Fix it by replacing the pasid_lock with an atomic exchange operation. Reported-and-tested-by: Dave Jiang <[email protected]> Signed-off-by: Lu Baolu <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]> Stable-dep-of: 194b334 ("iommu/vt-d: Fix PASID directory pointer coherency") Signed-off-by: Sasha Levin <[email protected]>
1 parent aeab1f1 commit c424b18

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

drivers/iommu/intel/pasid.c

+13-8
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
/*
2525
* Intel IOMMU system wide PASID name space:
2626
*/
27-
static DEFINE_SPINLOCK(pasid_lock);
2827
u32 intel_pasid_max_id = PASID_MAX;
2928

3029
int vcmd_alloc_pasid(struct intel_iommu *iommu, u32 *pasid)
@@ -259,19 +258,25 @@ struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid)
259258
dir_index = pasid >> PASID_PDE_SHIFT;
260259
index = pasid & PASID_PTE_MASK;
261260

262-
spin_lock(&pasid_lock);
261+
retry:
263262
entries = get_pasid_table_from_pde(&dir[dir_index]);
264263
if (!entries) {
265264
entries = alloc_pgtable_page(info->iommu->node);
266-
if (!entries) {
267-
spin_unlock(&pasid_lock);
265+
if (!entries)
268266
return NULL;
269-
}
270267

271-
WRITE_ONCE(dir[dir_index].val,
272-
(u64)virt_to_phys(entries) | PASID_PTE_PRESENT);
268+
/*
269+
* The pasid directory table entry won't be freed after
270+
* allocation. No worry about the race with free and
271+
* clear. However, this entry might be populated by others
272+
* while we are preparing it. Use theirs with a retry.
273+
*/
274+
if (cmpxchg64(&dir[dir_index].val, 0ULL,
275+
(u64)virt_to_phys(entries) | PASID_PTE_PRESENT)) {
276+
free_pgtable_page(entries);
277+
goto retry;
278+
}
273279
}
274-
spin_unlock(&pasid_lock);
275280

276281
return &entries[index];
277282
}

0 commit comments

Comments
 (0)