Skip to content

Commit b5bf777

Browse files
jgunthorpewilldeacon
authored andcommitted
iommu/arm-smmu-v3: Do not use GFP_KERNEL under as spinlock
If the SMMU is configured to use a two level CD table then arm_smmu_write_ctx_desc() allocates a CD table leaf internally using GFP_KERNEL. Due to recent changes this is being done under a spinlock to iterate over the device list - thus it will trigger a sleeping while atomic warning: arm_smmu_sva_set_dev_pasid() mutex_lock(&sva_lock); __arm_smmu_sva_bind() arm_smmu_mmu_notifier_get() spin_lock_irqsave() arm_smmu_write_ctx_desc() arm_smmu_get_cd_ptr() arm_smmu_alloc_cd_leaf_table() dmam_alloc_coherent(GFP_KERNEL) This is a 64K high order allocation and really should not be done atomically. At the moment the rework of the SVA to follow the new API is half finished. Recently the CD table memory was moved from the domain to the master, however we have the confusing situation where the SVA code is wrongly using the RID domains device's list to track which CD tables the SVA is installed in. Remove the logic to replicate the CD across all the domain's masters during attach. We know which master and which CD table the PASID should be installed in. Right now SVA only works when dma-iommu.c is in control of the RID translation, which means we have a single iommu_domain shared across the entire group and that iommu_domain is not shared outside the group. Critically this means that the iommu_group->devices list and RID's smmu_domain->devices list describe the same set of masters. For PCI cases the core code also insists on singleton groups so there is only one entry in the smmu_domain->devices list that is equal to the master being passed in to arm_smmu_sva_set_dev_pasid(). Only non-PCI cases may have multi-device groups. However, the core code will repeat the calls to arm_smmu_sva_set_dev_pasid() across the entire iommu_group->devices list. Instead of having arm_smmu_mmu_notifier_get() indirectly loop over all the devices in the group via the RID's smmu_domain, rely on __arm_smmu_sva_bind() to be called for each device in the group and install the repeated CD entry that way. This avoids taking the spinlock to access the devices list and permits the arm_smmu_write_ctx_desc() to use a sleeping allocation. Leave the arm_smmu_mm_release() as a confusing situation, this requires tracking attached masters inside the SVA domain. Removing the loop allows arm_smmu_write_ctx_desc() to be called outside the spinlock and thus is safe to use GFP_KERNEL. Move the clearing of the CD into arm_smmu_sva_remove_dev_pasid() so that arm_smmu_mmu_notifier_get/put() remain paired functions. Fixes: 2450314 ("iommu/arm-smmu-v3: Refactor write_ctx_desc") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/all/4e25d161-0cf8-4050-9aa3-dfa21cd63e56@moroto.mountain/ Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Michael Shavit <mshavit@google.com> Link: https://lore.kernel.org/r/0-v3-11978fc67151+112-smmu_cd_atomic_jgg@nvidia.com Signed-off-by: Will Deacon <will@kernel.org>
1 parent b419c5e commit b5bf777

File tree

1 file changed

+12
-26
lines changed

1 file changed

+12
-26
lines changed

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,8 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
292292
struct mm_struct *mm)
293293
{
294294
int ret;
295-
unsigned long flags;
296295
struct arm_smmu_ctx_desc *cd;
297296
struct arm_smmu_mmu_notifier *smmu_mn;
298-
struct arm_smmu_master *master;
299297

300298
list_for_each_entry(smmu_mn, &smmu_domain->mmu_notifiers, list) {
301299
if (smmu_mn->mn.mm == mm) {
@@ -325,28 +323,9 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
325323
goto err_free_cd;
326324
}
327325

328-
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
329-
list_for_each_entry(master, &smmu_domain->devices, domain_head) {
330-
ret = arm_smmu_write_ctx_desc(master, mm_get_enqcmd_pasid(mm),
331-
cd);
332-
if (ret) {
333-
list_for_each_entry_from_reverse(
334-
master, &smmu_domain->devices, domain_head)
335-
arm_smmu_write_ctx_desc(
336-
master, mm_get_enqcmd_pasid(mm), NULL);
337-
break;
338-
}
339-
}
340-
spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
341-
if (ret)
342-
goto err_put_notifier;
343-
344326
list_add(&smmu_mn->list, &smmu_domain->mmu_notifiers);
345327
return smmu_mn;
346328

347-
err_put_notifier:
348-
/* Frees smmu_mn */
349-
mmu_notifier_put(&smmu_mn->mn);
350329
err_free_cd:
351330
arm_smmu_free_shared_cd(cd);
352331
return ERR_PTR(ret);
@@ -363,9 +342,6 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
363342

364343
list_del(&smmu_mn->list);
365344

366-
arm_smmu_update_ctx_desc_devices(smmu_domain, mm_get_enqcmd_pasid(mm),
367-
NULL);
368-
369345
/*
370346
* If we went through clear(), we've already invalidated, and no
371347
* new TLB entry can have been formed.
@@ -381,7 +357,8 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
381357
arm_smmu_free_shared_cd(cd);
382358
}
383359

384-
static int __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm)
360+
static int __arm_smmu_sva_bind(struct device *dev, ioasid_t pasid,
361+
struct mm_struct *mm)
385362
{
386363
int ret;
387364
struct arm_smmu_bond *bond;
@@ -404,9 +381,15 @@ static int __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm)
404381
goto err_free_bond;
405382
}
406383

384+
ret = arm_smmu_write_ctx_desc(master, pasid, bond->smmu_mn->cd);
385+
if (ret)
386+
goto err_put_notifier;
387+
407388
list_add(&bond->list, &master->bonds);
408389
return 0;
409390

391+
err_put_notifier:
392+
arm_smmu_mmu_notifier_put(bond->smmu_mn);
410393
err_free_bond:
411394
kfree(bond);
412395
return ret;
@@ -568,6 +551,9 @@ void arm_smmu_sva_remove_dev_pasid(struct iommu_domain *domain,
568551
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
569552

570553
mutex_lock(&sva_lock);
554+
555+
arm_smmu_write_ctx_desc(master, id, NULL);
556+
571557
list_for_each_entry(t, &master->bonds, list) {
572558
if (t->mm == mm) {
573559
bond = t;
@@ -590,7 +576,7 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
590576
struct mm_struct *mm = domain->mm;
591577

592578
mutex_lock(&sva_lock);
593-
ret = __arm_smmu_sva_bind(dev, mm);
579+
ret = __arm_smmu_sva_bind(dev, id, mm);
594580
mutex_unlock(&sva_lock);
595581

596582
return ret;

0 commit comments

Comments
 (0)