Using a global device_domain_lock spinlock to protect per-domain device tracking lists is an inefficient way, especially considering this lock is also needed in the hot paths.
On the other hand, in the iommu_unmap() path, the driver needs to iterate over the device tracking list and flush the caches on the devices through qi_submit_sync(), where unfortunately cpu_relax() is used. In order to avoid holding a spinlock lock when cpu_relax() is called, this also covert the spinlock into a mutex one. This works as the device tracking lists are not touched in any interrupt contexts. Signed-off-by: Lu Baolu <baolu...@linux.intel.com> --- drivers/iommu/intel/iommu.h | 1 + drivers/iommu/intel/iommu.c | 45 +++++++++++++++---------------------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h index 6724703d573b..9e572ddffc08 100644 --- a/drivers/iommu/intel/iommu.h +++ b/drivers/iommu/intel/iommu.h @@ -541,6 +541,7 @@ struct dmar_domain { u8 force_snooping : 1; /* Create IOPTEs with snoop control */ u8 set_pte_snp:1; + struct mutex mutex; /* Protect device tracking lists */ struct list_head devices; /* all devices' list */ struct dma_pte *pgd; /* virtual address */ diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index f8aa8649dc6f..1815a9d73426 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -310,7 +310,6 @@ static int iommu_skip_te_disable; #define IDENTMAP_GFX 2 #define IDENTMAP_AZALIA 4 -static DEFINE_SPINLOCK(device_domain_lock); const struct iommu_ops intel_iommu_ops; static bool translation_pre_enabled(struct intel_iommu *iommu) @@ -534,9 +533,8 @@ static int domain_update_device_node(struct dmar_domain *domain) { struct device_domain_info *info; int nid = NUMA_NO_NODE; - unsigned long flags; - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&domain->mutex); list_for_each_entry(info, &domain->devices, link) { /* * There could possibly be multiple device numa nodes as devices @@ -548,7 +546,7 @@ static int domain_update_device_node(struct dmar_domain *domain) if (nid != NUMA_NO_NODE) break; } - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&domain->mutex); return nid; } @@ -1375,12 +1373,11 @@ iommu_support_dev_iotlb(struct dmar_domain *domain, struct intel_iommu *iommu, u8 bus, u8 devfn) { struct device_domain_info *info = NULL, *tmp; - unsigned long flags; if (!iommu->qi) return NULL; - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&domain->mutex); list_for_each_entry(tmp, &domain->devices, link) { if (tmp->iommu == iommu && tmp->bus == bus && tmp->devfn == devfn) { @@ -1389,7 +1386,7 @@ iommu_support_dev_iotlb(struct dmar_domain *domain, struct intel_iommu *iommu, break; } } - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&domain->mutex); return info; } @@ -1398,9 +1395,8 @@ static void domain_update_iotlb(struct dmar_domain *domain) { struct device_domain_info *info; bool has_iotlb_device = false; - unsigned long flags; - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&domain->mutex); list_for_each_entry(info, &domain->devices, link) { if (info->ats_enabled) { has_iotlb_device = true; @@ -1408,7 +1404,7 @@ static void domain_update_iotlb(struct dmar_domain *domain) } } domain->has_iotlb_device = has_iotlb_device; - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&domain->mutex); } static void iommu_enable_dev_iotlb(struct device_domain_info *info) @@ -1499,17 +1495,15 @@ static void __iommu_flush_dev_iotlb(struct device_domain_info *info, static void iommu_flush_dev_iotlb(struct dmar_domain *domain, u64 addr, unsigned mask) { - unsigned long flags; struct device_domain_info *info; if (!domain->has_iotlb_device) return; - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&domain->mutex); list_for_each_entry(info, &domain->devices, link) __iommu_flush_dev_iotlb(info, addr, mask); - - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&domain->mutex); } static void iommu_flush_iotlb_psi(struct intel_iommu *iommu, @@ -1761,6 +1755,7 @@ static struct dmar_domain *alloc_domain(unsigned int type) domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL; domain->has_iotlb_device = false; INIT_LIST_HEAD(&domain->devices); + mutex_init(&domain->mutex); return domain; } @@ -2434,7 +2429,6 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev) { struct device_domain_info *info = dev_iommu_priv_get(dev); struct intel_iommu *iommu; - unsigned long flags; u8 bus, devfn; int ret; @@ -2446,9 +2440,9 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev) if (ret) return ret; - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&domain->mutex); list_add(&info->link, &domain->devices); - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&domain->mutex); info->domain = domain; /* PASID table is mandatory for a PCI device in scalable mode. */ @@ -4126,7 +4120,6 @@ static void dmar_remove_one_dev_info(struct device *dev) struct device_domain_info *info = dev_iommu_priv_get(dev); struct dmar_domain *domain = info->domain; struct intel_iommu *iommu = info->iommu; - unsigned long flags; if (!dev_is_real_dma_subdevice(info->dev)) { if (dev_is_pci(info->dev) && sm_supported(iommu)) @@ -4138,9 +4131,9 @@ static void dmar_remove_one_dev_info(struct device *dev) intel_pasid_free_table(info->dev); } - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&domain->mutex); list_del(&info->link); - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&domain->mutex); domain_detach_iommu(domain, iommu); } @@ -4424,7 +4417,7 @@ static bool domain_support_force_snooping(struct dmar_domain *domain) struct device_domain_info *info; bool support = true; - assert_spin_locked(&device_domain_lock); + lockdep_assert_held(&domain->mutex); list_for_each_entry(info, &domain->devices, link) { if (!ecap_sc_support(info->iommu->ecap)) { support = false; @@ -4439,8 +4432,7 @@ static void domain_set_force_snooping(struct dmar_domain *domain) { struct device_domain_info *info; - assert_spin_locked(&device_domain_lock); - + lockdep_assert_held(&domain->mutex); /* * Second level page table supports per-PTE snoop control. The * iommu_map() interface will handle this by setting SNP bit. @@ -4458,20 +4450,19 @@ static void domain_set_force_snooping(struct dmar_domain *domain) static bool intel_iommu_enforce_cache_coherency(struct iommu_domain *domain) { struct dmar_domain *dmar_domain = to_dmar_domain(domain); - unsigned long flags; if (dmar_domain->force_snooping) return true; - spin_lock_irqsave(&device_domain_lock, flags); + mutex_lock(&dmar_domain->mutex); if (!domain_support_force_snooping(dmar_domain)) { - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&dmar_domain->mutex); return false; } domain_set_force_snooping(dmar_domain); dmar_domain->force_snooping = true; - spin_unlock_irqrestore(&device_domain_lock, flags); + mutex_unlock(&dmar_domain->mutex); return true; } -- 2.25.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu