Commit 9cd4f143 authored by Lu Baolu's avatar Lu Baolu Committed by Joerg Roedel

iommu/vt-d: Fix possible recursive locking in intel_iommu_init()

The global rwsem dmar_global_lock was introduced by commit 3a5670e8
("iommu/vt-d: Introduce a rwsem to protect global data structures"). It
is used to protect DMAR related global data from DMAR hotplug operations.

The dmar_global_lock used in the intel_iommu_init() might cause recursive
locking issue, for example, intel_iommu_get_resv_regions() is taking the
dmar_global_lock from within a section where intel_iommu_init() already
holds it via probe_acpi_namespace_devices().

Using dmar_global_lock in intel_iommu_init() could be relaxed since it is
unlikely that any IO board must be hot added before the IOMMU subsystem is
initialized. This eliminates the possible recursive locking issue by moving
down DMAR hotplug support after the IOMMU is initialized and removing the
uses of dmar_global_lock in intel_iommu_init().

Fixes: d5692d4a ("iommu/vt-d: Fix suspicious RCU usage in probe_acpi_namespace_devices()")
Reported-by: default avatarRobin Murphy <robin.murphy@arm.com>
Signed-off-by: default avatarLu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: default avatarKevin Tian <kevin.tian@intel.com>
Link: https://lore.kernel.org/r/894db0ccae854b35c73814485569b634237b5538.1657034828.git.robin.murphy@arm.com
Link: https://lore.kernel.org/r/20220718235325.3952426-1-baolu.lu@linux.intel.comSigned-off-by: default avatarJoerg Roedel <jroedel@suse.de>
parent 91c98fe7
...@@ -2349,6 +2349,13 @@ static int dmar_device_hotplug(acpi_handle handle, bool insert) ...@@ -2349,6 +2349,13 @@ static int dmar_device_hotplug(acpi_handle handle, bool insert)
if (!dmar_in_use()) if (!dmar_in_use())
return 0; return 0;
/*
* It's unlikely that any I/O board is hot added before the IOMMU
* subsystem is initialized.
*/
if (IS_ENABLED(CONFIG_INTEL_IOMMU) && !intel_iommu_enabled)
return -EOPNOTSUPP;
if (dmar_detect_dsm(handle, DMAR_DSM_FUNC_DRHD)) { if (dmar_detect_dsm(handle, DMAR_DSM_FUNC_DRHD)) {
tmp = handle; tmp = handle;
} else { } else {
......
...@@ -3019,13 +3019,7 @@ static int __init init_dmars(void) ...@@ -3019,13 +3019,7 @@ static int __init init_dmars(void)
#ifdef CONFIG_INTEL_IOMMU_SVM #ifdef CONFIG_INTEL_IOMMU_SVM
if (pasid_supported(iommu) && ecap_prs(iommu->ecap)) { if (pasid_supported(iommu) && ecap_prs(iommu->ecap)) {
/*
* Call dmar_alloc_hwirq() with dmar_global_lock held,
* could cause possible lock race condition.
*/
up_write(&dmar_global_lock);
ret = intel_svm_enable_prq(iommu); ret = intel_svm_enable_prq(iommu);
down_write(&dmar_global_lock);
if (ret) if (ret)
goto free_iommu; goto free_iommu;
} }
...@@ -3938,7 +3932,6 @@ int __init intel_iommu_init(void) ...@@ -3938,7 +3932,6 @@ int __init intel_iommu_init(void)
force_on = (!intel_iommu_tboot_noforce && tboot_force_iommu()) || force_on = (!intel_iommu_tboot_noforce && tboot_force_iommu()) ||
platform_optin_force_iommu(); platform_optin_force_iommu();
down_write(&dmar_global_lock);
if (dmar_table_init()) { if (dmar_table_init()) {
if (force_on) if (force_on)
panic("tboot: Failed to initialize DMAR table\n"); panic("tboot: Failed to initialize DMAR table\n");
...@@ -3951,16 +3944,6 @@ int __init intel_iommu_init(void) ...@@ -3951,16 +3944,6 @@ int __init intel_iommu_init(void)
goto out_free_dmar; goto out_free_dmar;
} }
up_write(&dmar_global_lock);
/*
* The bus notifier takes the dmar_global_lock, so lockdep will
* complain later when we register it under the lock.
*/
dmar_register_bus_notifier();
down_write(&dmar_global_lock);
if (!no_iommu) if (!no_iommu)
intel_iommu_debugfs_init(); intel_iommu_debugfs_init();
...@@ -4005,11 +3988,9 @@ int __init intel_iommu_init(void) ...@@ -4005,11 +3988,9 @@ int __init intel_iommu_init(void)
pr_err("Initialization failed\n"); pr_err("Initialization failed\n");
goto out_free_dmar; goto out_free_dmar;
} }
up_write(&dmar_global_lock);
init_iommu_pm_ops(); init_iommu_pm_ops();
down_read(&dmar_global_lock);
for_each_active_iommu(iommu, drhd) { for_each_active_iommu(iommu, drhd) {
/* /*
* The flush queue implementation does not perform * The flush queue implementation does not perform
...@@ -4027,13 +4008,11 @@ int __init intel_iommu_init(void) ...@@ -4027,13 +4008,11 @@ int __init intel_iommu_init(void)
"%s", iommu->name); "%s", iommu->name);
iommu_device_register(&iommu->iommu, &intel_iommu_ops, NULL); iommu_device_register(&iommu->iommu, &intel_iommu_ops, NULL);
} }
up_read(&dmar_global_lock);
bus_set_iommu(&pci_bus_type, &intel_iommu_ops); bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
if (si_domain && !hw_pass_through) if (si_domain && !hw_pass_through)
register_memory_notifier(&intel_iommu_memory_nb); register_memory_notifier(&intel_iommu_memory_nb);
down_read(&dmar_global_lock);
if (probe_acpi_namespace_devices()) if (probe_acpi_namespace_devices())
pr_warn("ACPI name space devices didn't probe correctly\n"); pr_warn("ACPI name space devices didn't probe correctly\n");
...@@ -4044,17 +4023,15 @@ int __init intel_iommu_init(void) ...@@ -4044,17 +4023,15 @@ int __init intel_iommu_init(void)
iommu_disable_protect_mem_regions(iommu); iommu_disable_protect_mem_regions(iommu);
} }
up_read(&dmar_global_lock);
pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
intel_iommu_enabled = 1; intel_iommu_enabled = 1;
dmar_register_bus_notifier();
pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
return 0; return 0;
out_free_dmar: out_free_dmar:
intel_iommu_free_dmars(); intel_iommu_free_dmars();
up_write(&dmar_global_lock);
return ret; return ret;
} }
......
...@@ -65,6 +65,7 @@ struct dmar_pci_notify_info { ...@@ -65,6 +65,7 @@ struct dmar_pci_notify_info {
extern struct rw_semaphore dmar_global_lock; extern struct rw_semaphore dmar_global_lock;
extern struct list_head dmar_drhd_units; extern struct list_head dmar_drhd_units;
extern int intel_iommu_enabled;
#define for_each_drhd_unit(drhd) \ #define for_each_drhd_unit(drhd) \
list_for_each_entry_rcu(drhd, &dmar_drhd_units, list, \ list_for_each_entry_rcu(drhd, &dmar_drhd_units, list, \
...@@ -88,7 +89,8 @@ extern struct list_head dmar_drhd_units; ...@@ -88,7 +89,8 @@ extern struct list_head dmar_drhd_units;
static inline bool dmar_rcu_check(void) static inline bool dmar_rcu_check(void)
{ {
return rwsem_is_locked(&dmar_global_lock) || return rwsem_is_locked(&dmar_global_lock) ||
system_state == SYSTEM_BOOTING; system_state == SYSTEM_BOOTING ||
(IS_ENABLED(CONFIG_INTEL_IOMMU) && !intel_iommu_enabled);
} }
#define dmar_rcu_dereference(p) rcu_dereference_check((p), dmar_rcu_check()) #define dmar_rcu_dereference(p) rcu_dereference_check((p), dmar_rcu_check())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment