Skip to content

Commit a349ffc

Browse files
LuBaolujoergroedel
authored andcommitted
iommu/vt-d: Fix recursive lock issue in iommu_flush_dev_iotlb()
The per domain spinlock is acquired in iommu_flush_dev_iotlb(), which is possbile to be called in the interrupt context. For example, the drm-intel's CI system got completely blocked with below error: WARNING: inconsistent lock state 6.0.0-rc1-CI_DRM_11990-g6590d43d39b9+ #1 Not tainted -------------------------------- inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. swapper/6/0 [HC0[0]:SC1[1]:HE1:SE0] takes: ffff88810440d678 (&domain->lock){+.?.}-{2:2}, at: iommu_flush_dev_iotlb.part.61+0x23/0x80 {SOFTIRQ-ON-W} state was registered at: lock_acquire+0xd3/0x310 _raw_spin_lock+0x2a/0x40 domain_update_iommu_cap+0x20b/0x2c0 intel_iommu_attach_device+0x5bd/0x860 __iommu_attach_device+0x18/0xe0 bus_iommu_probe+0x1f3/0x2d0 bus_set_iommu+0x82/0xd0 intel_iommu_init+0xe45/0x102a pci_iommu_init+0x9/0x31 do_one_initcall+0x53/0x2f0 kernel_init_freeable+0x18f/0x1e1 kernel_init+0x11/0x120 ret_from_fork+0x1f/0x30 irq event stamp: 162354 hardirqs last enabled at (162354): [<ffffffff81b59274>] _raw_spin_unlock_irqrestore+0x54/0x70 hardirqs last disabled at (162353): [<ffffffff81b5901b>] _raw_spin_lock_irqsave+0x4b/0x50 softirqs last enabled at (162338): [<ffffffff81e00323>] __do_softirq+0x323/0x48e softirqs last disabled at (162349): [<ffffffff810c1588>] irq_exit_rcu+0xb8/0xe0 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&domain->lock); <Interrupt> lock(&domain->lock); *** DEADLOCK *** 1 lock held by swapper/6/0: This coverts the spin_lock/unlock() into the irq save/restore varieties to fix the recursive locking issues. Fixes: ffd5869 ("iommu/vt-d: Replace spin_lock_irqsave() with spin_lock()") Signed-off-by: Lu Baolu <[email protected]> Acked-by: Lucas De Marchi <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]>
1 parent 53fc7ad commit a349ffc

File tree

1 file changed

+23
-16
lines changed

1 file changed

+23
-16
lines changed

drivers/iommu/intel/iommu.c

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -515,8 +515,9 @@ static int domain_update_device_node(struct dmar_domain *domain)
515515
{
516516
struct device_domain_info *info;
517517
int nid = NUMA_NO_NODE;
518+
unsigned long flags;
518519

519-
spin_lock(&domain->lock);
520+
spin_lock_irqsave(&domain->lock, flags);
520521
list_for_each_entry(info, &domain->devices, link) {
521522
/*
522523
* There could possibly be multiple device numa nodes as devices
@@ -528,7 +529,7 @@ static int domain_update_device_node(struct dmar_domain *domain)
528529
if (nid != NUMA_NO_NODE)
529530
break;
530531
}
531-
spin_unlock(&domain->lock);
532+
spin_unlock_irqrestore(&domain->lock, flags);
532533

533534
return nid;
534535
}
@@ -1362,19 +1363,20 @@ iommu_support_dev_iotlb(struct dmar_domain *domain, struct intel_iommu *iommu,
13621363
u8 bus, u8 devfn)
13631364
{
13641365
struct device_domain_info *info;
1366+
unsigned long flags;
13651367

13661368
if (!iommu->qi)
13671369
return NULL;
13681370

1369-
spin_lock(&domain->lock);
1371+
spin_lock_irqsave(&domain->lock, flags);
13701372
list_for_each_entry(info, &domain->devices, link) {
13711373
if (info->iommu == iommu && info->bus == bus &&
13721374
info->devfn == devfn) {
1373-
spin_unlock(&domain->lock);
1375+
spin_unlock_irqrestore(&domain->lock, flags);
13741376
return info->ats_supported ? info : NULL;
13751377
}
13761378
}
1377-
spin_unlock(&domain->lock);
1379+
spin_unlock_irqrestore(&domain->lock, flags);
13781380

13791381
return NULL;
13801382
}
@@ -1383,16 +1385,17 @@ static void domain_update_iotlb(struct dmar_domain *domain)
13831385
{
13841386
struct device_domain_info *info;
13851387
bool has_iotlb_device = false;
1388+
unsigned long flags;
13861389

1387-
spin_lock(&domain->lock);
1390+
spin_lock_irqsave(&domain->lock, flags);
13881391
list_for_each_entry(info, &domain->devices, link) {
13891392
if (info->ats_enabled) {
13901393
has_iotlb_device = true;
13911394
break;
13921395
}
13931396
}
13941397
domain->has_iotlb_device = has_iotlb_device;
1395-
spin_unlock(&domain->lock);
1398+
spin_unlock_irqrestore(&domain->lock, flags);
13961399
}
13971400

13981401
static void iommu_enable_dev_iotlb(struct device_domain_info *info)
@@ -1484,14 +1487,15 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
14841487
u64 addr, unsigned mask)
14851488
{
14861489
struct device_domain_info *info;
1490+
unsigned long flags;
14871491

14881492
if (!domain->has_iotlb_device)
14891493
return;
14901494

1491-
spin_lock(&domain->lock);
1495+
spin_lock_irqsave(&domain->lock, flags);
14921496
list_for_each_entry(info, &domain->devices, link)
14931497
__iommu_flush_dev_iotlb(info, addr, mask);
1494-
spin_unlock(&domain->lock);
1498+
spin_unlock_irqrestore(&domain->lock, flags);
14951499
}
14961500

14971501
static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
@@ -2453,6 +2457,7 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
24532457
{
24542458
struct device_domain_info *info = dev_iommu_priv_get(dev);
24552459
struct intel_iommu *iommu;
2460+
unsigned long flags;
24562461
u8 bus, devfn;
24572462
int ret;
24582463

@@ -2464,9 +2469,9 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
24642469
if (ret)
24652470
return ret;
24662471
info->domain = domain;
2467-
spin_lock(&domain->lock);
2472+
spin_lock_irqsave(&domain->lock, flags);
24682473
list_add(&info->link, &domain->devices);
2469-
spin_unlock(&domain->lock);
2474+
spin_unlock_irqrestore(&domain->lock, flags);
24702475

24712476
/* PASID table is mandatory for a PCI device in scalable mode. */
24722477
if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
@@ -4090,6 +4095,7 @@ static void dmar_remove_one_dev_info(struct device *dev)
40904095
struct device_domain_info *info = dev_iommu_priv_get(dev);
40914096
struct dmar_domain *domain = info->domain;
40924097
struct intel_iommu *iommu = info->iommu;
4098+
unsigned long flags;
40934099

40944100
if (!dev_is_real_dma_subdevice(info->dev)) {
40954101
if (dev_is_pci(info->dev) && sm_supported(iommu))
@@ -4101,9 +4107,9 @@ static void dmar_remove_one_dev_info(struct device *dev)
41014107
intel_pasid_free_table(info->dev);
41024108
}
41034109

4104-
spin_lock(&domain->lock);
4110+
spin_lock_irqsave(&domain->lock, flags);
41054111
list_del(&info->link);
4106-
spin_unlock(&domain->lock);
4112+
spin_unlock_irqrestore(&domain->lock, flags);
41074113

41084114
domain_detach_iommu(domain, iommu);
41094115
info->domain = NULL;
@@ -4422,19 +4428,20 @@ static void domain_set_force_snooping(struct dmar_domain *domain)
44224428
static bool intel_iommu_enforce_cache_coherency(struct iommu_domain *domain)
44234429
{
44244430
struct dmar_domain *dmar_domain = to_dmar_domain(domain);
4431+
unsigned long flags;
44254432

44264433
if (dmar_domain->force_snooping)
44274434
return true;
44284435

4429-
spin_lock(&dmar_domain->lock);
4436+
spin_lock_irqsave(&dmar_domain->lock, flags);
44304437
if (!domain_support_force_snooping(dmar_domain)) {
4431-
spin_unlock(&dmar_domain->lock);
4438+
spin_unlock_irqrestore(&dmar_domain->lock, flags);
44324439
return false;
44334440
}
44344441

44354442
domain_set_force_snooping(dmar_domain);
44364443
dmar_domain->force_snooping = true;
4437-
spin_unlock(&dmar_domain->lock);
4444+
spin_unlock_irqrestore(&dmar_domain->lock, flags);
44384445

44394446
return true;
44404447
}

0 commit comments

Comments
 (0)