mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 01:55:51 +02:00
iommu/amd: Fix IRQ unsafe locking in gdom allocation
Lockdep complains:
[ 259.410489] =====================================================
[ 259.417287] WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
[ 259.424667] 7.0.0-g51db1d8d2113 #54 Not tainted
[ 259.429718] -----------------------------------------------------
[ 259.436516] qemu-system-x86/10143 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[ 259.444670] ff3b2b1c60305170 (&xa->xa_lock#25){+.+.}-{3:3}, at: __domain_flush_pages+0x17c/0x4b0
[ 259.454485]
and this task is already holding:
[ 259.460991] ff3b2b1c98504cc0 (&domain->lock){-.-.}-{3:3}, at: amd_iommu_iotlb_sync+0x25/0x60
[ 259.470408] which would create a new lock dependency:
[ 259.476041] (&domain->lock){-.-.}-{3:3} -> (&xa->xa_lock#25){+.+.}-{3:3}
[ 259.483615]
but this new dependency connects a HARDIRQ-irq-safe lock:
[ 259.492447] (&domain->lock){-.-.}-{3:3}
[ 259.492449]
... which became HARDIRQ-irq-safe at:
[ 259.503705] lock_acquire+0xb6/0x2e0
[ 259.507790] _raw_spin_lock_irqsave+0x3e/0x60
[ 259.512748] amd_iommu_flush_iotlb_all+0x20/0x50
[ 259.517996] iommu_dma_free_iova.isra.0+0x1b8/0x1e0
[ 259.523534] __iommu_dma_unmap+0xc2/0x140
[ 259.528100] iommu_dma_unmap_phys+0x55/0xc0
[ 259.532863] dma_unmap_phys+0x274/0x2e0
[ 259.537238] dma_unmap_page_attrs+0x17/0x30
[ 259.542000] nvme_unmap_data+0x13e/0x280
[ 259.546473] nvme_pci_complete_batch+0x45/0x70
[ 259.551524] nvme_irq+0x83/0x90
[ 259.555123] __handle_irq_event_percpu+0x92/0x360
[ 259.560466] handle_irq_event+0x39/0x80
[ 259.564841] handle_edge_irq+0xb2/0x1a0
[ 259.569214] __common_interrupt+0x4e/0x130
[ 259.573882] common_interrupt+0x88/0xa0
[ 259.578256] asm_common_interrupt+0x27/0x40
[ 259.583019] cpuidle_enter_state+0x119/0x5d0
[ 259.587877] cpuidle_enter+0x2e/0x50
[ 259.591962] do_idle+0x153/0x2c0
[ 259.595657] cpu_startup_entry+0x29/0x30
[ 259.600128] start_secondary+0x118/0x150
[ 259.604601] common_startup_64+0x13e/0x141
[ 259.609266]
to a HARDIRQ-irq-unsafe lock:
[ 259.615384] (&xa->xa_lock#25){+.+.}-{3:3}
[ 259.615386]
... which became HARDIRQ-irq-unsafe at:
[ 259.627039] ...
[ 259.627039] lock_acquire+0xb6/0x2e0
[ 259.633071] _raw_spin_lock+0x2f/0x50
[ 259.637250] amd_iommu_alloc_domain_nested+0x140/0x3c0
[ 259.643078] iommufd_hwpt_alloc+0x272/0x800 [iommufd]
[ 259.648813] iommufd_fops_ioctl+0x14e/0x200 [iommufd]
[ 259.654547] __x64_sys_ioctl+0x9d/0xf0
...
Since amd_iommu_domain_flush_pages() necessarily holds domain->lock to do the
flush, switch the allocation side in gdom_info_load_or_alloc_locked() to
HARDIRQ-safe allocation. The IOMMU_DESTROY->free path has the same issue,
so switch that path to HARDIRQ-safe locking as well.
Fixes: 757d2b1fdf ("iommu/amd: Introduce gDomID-to-hDomID Mapping and handle parent domain invalidation")
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Ankit Soni <Ankit.Soni@amd.com>
Signed-off-by: Will Deacon <will@kernel.org>
This commit is contained in:
parent
8cdeaa50ea
commit
0db3a430d9
|
|
@ -59,7 +59,9 @@ static int validate_gdte_nested(struct iommu_hwpt_amd_guest *gdte)
|
|||
return 0;
|
||||
}
|
||||
|
||||
static void *gdom_info_load_or_alloc_locked(struct xarray *xa, unsigned long index)
|
||||
static void *gdom_info_load_or_alloc_locked(struct xarray *xa,
|
||||
unsigned long index,
|
||||
unsigned long *flags)
|
||||
{
|
||||
struct guest_domain_mapping_info *elm, *res;
|
||||
|
||||
|
|
@ -67,13 +69,13 @@ static void *gdom_info_load_or_alloc_locked(struct xarray *xa, unsigned long ind
|
|||
if (elm)
|
||||
return elm;
|
||||
|
||||
xa_unlock(xa);
|
||||
xa_unlock_irqrestore(xa, *flags);
|
||||
elm = kzalloc_obj(struct guest_domain_mapping_info);
|
||||
xa_lock(xa);
|
||||
xa_lock_irqsave(xa, *flags);
|
||||
if (!elm)
|
||||
return ERR_PTR(-ENOMEM);
|
||||
|
||||
res = __xa_cmpxchg(xa, index, NULL, elm, GFP_KERNEL);
|
||||
res = __xa_cmpxchg(xa, index, NULL, elm, GFP_ATOMIC);
|
||||
if (xa_is_err(res))
|
||||
res = ERR_PTR(xa_err(res));
|
||||
|
||||
|
|
@ -95,6 +97,7 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
|
|||
const struct iommu_user_data *user_data)
|
||||
{
|
||||
int ret;
|
||||
unsigned long irqflags;
|
||||
struct nested_domain *ndom;
|
||||
struct guest_domain_mapping_info *gdom_info;
|
||||
struct amd_iommu_viommu *aviommu = container_of(viommu, struct amd_iommu_viommu, core);
|
||||
|
|
@ -136,11 +139,12 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
|
|||
* keep track of the gDomID mapping. When the S2 is changed, the INVALIDATE_IOMMU_PAGES
|
||||
* command must be issued for each hDomID in the xarray.
|
||||
*/
|
||||
xa_lock(&aviommu->gdomid_array);
|
||||
xa_lock_irqsave(&aviommu->gdomid_array, irqflags);
|
||||
|
||||
gdom_info = gdom_info_load_or_alloc_locked(&aviommu->gdomid_array, ndom->gdom_id);
|
||||
gdom_info = gdom_info_load_or_alloc_locked(&aviommu->gdomid_array,
|
||||
ndom->gdom_id, &irqflags);
|
||||
if (IS_ERR(gdom_info)) {
|
||||
xa_unlock(&aviommu->gdomid_array);
|
||||
xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
|
||||
ret = PTR_ERR(gdom_info);
|
||||
goto out_err;
|
||||
}
|
||||
|
|
@ -148,7 +152,7 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
|
|||
/* Check if gDomID exist */
|
||||
if (refcount_inc_not_zero(&gdom_info->users)) {
|
||||
ndom->gdom_info = gdom_info;
|
||||
xa_unlock(&aviommu->gdomid_array);
|
||||
xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
|
||||
|
||||
pr_debug("%s: Found gdom_id=%#x, hdom_id=%#x\n",
|
||||
__func__, ndom->gdom_id, gdom_info->hdom_id);
|
||||
|
|
@ -161,7 +165,7 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
|
|||
if (gdom_info->hdom_id <= 0) {
|
||||
__xa_cmpxchg(&aviommu->gdomid_array,
|
||||
ndom->gdom_id, gdom_info, NULL, GFP_ATOMIC);
|
||||
xa_unlock(&aviommu->gdomid_array);
|
||||
xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
|
||||
ret = -ENOSPC;
|
||||
goto out_err_gdom_info;
|
||||
}
|
||||
|
|
@ -169,7 +173,7 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
|
|||
ndom->gdom_info = gdom_info;
|
||||
refcount_set(&gdom_info->users, 1);
|
||||
|
||||
xa_unlock(&aviommu->gdomid_array);
|
||||
xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
|
||||
|
||||
pr_debug("%s: Allocate gdom_id=%#x, hdom_id=%#x\n",
|
||||
__func__, ndom->gdom_id, gdom_info->hdom_id);
|
||||
|
|
@ -257,14 +261,15 @@ static int nested_attach_device(struct iommu_domain *dom, struct device *dev,
|
|||
|
||||
static void nested_domain_free(struct iommu_domain *dom)
|
||||
{
|
||||
unsigned long irqflags;
|
||||
struct guest_domain_mapping_info *curr;
|
||||
struct nested_domain *ndom = to_ndomain(dom);
|
||||
struct amd_iommu_viommu *aviommu = ndom->viommu;
|
||||
|
||||
xa_lock(&aviommu->gdomid_array);
|
||||
xa_lock_irqsave(&aviommu->gdomid_array, irqflags);
|
||||
|
||||
if (!refcount_dec_and_test(&ndom->gdom_info->users)) {
|
||||
xa_unlock(&aviommu->gdomid_array);
|
||||
xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -275,7 +280,7 @@ static void nested_domain_free(struct iommu_domain *dom)
|
|||
curr = __xa_cmpxchg(&aviommu->gdomid_array, ndom->gdom_id,
|
||||
ndom->gdom_info, NULL, GFP_ATOMIC);
|
||||
|
||||
xa_unlock(&aviommu->gdomid_array);
|
||||
xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
|
||||
if (WARN_ON(!curr || xa_err(curr)))
|
||||
return;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user