Re: [PATCH] iommu/arm-smmu-v3: Add a threshold to avoid potential soft lockup
From: zhangzekun (A)
Date: Thu Jan 18 2024 - 06:21:27 EST
在 2024/1/15 23:31, Jason Gunthorpe 写道:
On Mon, Jan 15, 2024 at 07:40:40PM +0800, Zhang Zekun wrote:
The commit d5afb4b47e13 ("iommu/arm-smmu-v3: Fix soft lockup triggered
by arm_smmu_mm_invalidate_range") has fix a soft lockup problem when
running SVA case, but code paths from iommu_unmap and dma APIs still
remain unfixed which could also cause potential soft lockup.
When cmdq is quite busy and don't have much space for batch submitting
cmds, and size passed to __arm_smmu_tlb_inv_range() is large (1G in this
case), the following softlockup is triggered.
WARN: soft lockup - CPU#71 stuck for 12s! [qemu-kvm:1303]
...
Call trace:
dump_backtrace+0x0/0x200
show_stack+0x20/0x30
dump_stack+0xf0/0x138
watchdog_print_info+0x48/0x54
watchdog_process_before_softlockup+0x9c/0xa0
watchdog_timer_fn+0x1ac/0x2f0
__run_hrtimer+0x98/0x2b4
__hrtimer_run_queues+0xc0/0x13c
hrtimer_interrupt+0x150/0x3e4
arch_timer_handler_phys+0x3c/0x50
handle_percpu_devid_irq+0x90/0x1f4
__handle_domain_irq+0x84/0xfc
gic_handle_irq+0x88/0x2b0
el1_irq+0xb8/0x140
arm_smmu_cmdq_issue_cmdlist+0x184/0x5f4
__arm_smmu_tlb_inv_range+0x114/0x22c
arm_smmu_tlb_inv_walk+0x88/0x120
__arm_lpae_unmap+0x188/0x2c0
__arm_lpae_unmap+0x104/0x2c0
arm_lpae_unmap+0x68/0x80
arm_smmu_unmap+0x24/0x40
__iommu_unmap+0xd8/0x210
iommu_unmap+0x44/0x9c
...
What is the rest of the call chain? How did you get into such a large
invalidation?
we are doing some test with qemu virtual machines. The testing platform
has 96 cores and 960GB memories. It will enable 64 virtual machines and
every virtual machine will cost 8G memories. When batching closing these
64 virtaul machine concurrently, we will get soft lockup.
WARN: soft lockup - CPU#18 stuck for 11s! [worker:3219]
...
Call trace:
dump_backtrace+0x0/0x200
show_stack+0x20/0x30
dump_stack+0xf0/0x138
watchdog_print_info+0x48/0x54
watchdog_process_before_softlockup+0x9c/0xa0
watchdog_timer_fn+0x1ac/0x2f0
__run_hrtimer+0x98/0x2b4
__hrtimer_run_queues+0xc0/0x13c
hrtimer_interrupt+0x150/0x3e4
arch_timer_handler_phys+0x3c/0x50
handle_percpu_devid_irq+0x90/0x1f4
__handle_domain_irq+0x84/0xfc
gic_handle_irq+0x88/0x2b0
el1_irq+0xb8/0x140
ktime_get+0x3c/0xac
arm_smmu_cmdq_poll_until_not_full+0xb0/0x1a0
arm_smmu_cmdq_issue_cmdlist+0x194/0x5f4
__arm_smmu_tlb_inv_range+0x114/0x22c
arm_smmu_tlb_inv_walk+0x88/0x120
__arm_lpae_unmap+0x188/0x2c0
__arm_lpae_unmap+0x104/0x2c0
arm_lpae_unmap+0x68/0x80
arm_smmu_unmap+0x24/0x40
__iommu_unmap+0xd8/0x210
iommu_unmap_fast+0x18/0x24
unmap_unpin_fast+0x7c/0x140 [vfio_iommu_type1]
vfio_unmap_unpin+0x14c/0x3ac [vfio_iommu_type1]
vfio_remove_dma+0x38/0x124 [vfio_iommu_type1]
vfio_iommu_type1_detach_group+0x4b8/0x4e0 [vfio_iommu_type1]
__vfio_group_unset_container+0x58/0x18c [vfio]
vfio_group_try_dissolve_container+0x80/0x94 [vfio]
vfio_group_put_external_user+0x20/0x54 [vfio]
kvm_vfio_destroy+0xa8/0x12c
kvm_destroy_vm+0x20c/0x300
kvm_put_kvm+0x74/0xa0
kvm_vcpu_release+0x20/0x30
__fput+0xc4/0x270
____fput+0x18/0x24
task_work_run+0xd0/0x1a0
do_exit+0x1d8/0x480
do_group_exit+0x40/0x130
get_signal+0x1f8/0x744
do_signal+0x98/0x204
do_notify_resume+0x15c/0x1e0
work_pending+0xc/0xa4
@@ -228,7 +219,7 @@ static void arm_smmu_mm_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
*/
size = end - start;
if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_RANGE_INV)) {
- if (size >= CMDQ_MAX_TLBI_OPS * PAGE_SIZE)
+ if (size >= CMDQ_MAX_TLBI_OPS(PAGE_SIZE) * PAGE_SIZE)
size = 0;
} else {
if (size == ULONG_MAX)
I would like to see the SVA code rely on the common invalidation
infrastructure, this should be pushed down into the invalidation logic
not be in the SVA code. With some adjustments we can get common
functions for all this and push the decision making into the actual
function.
Thanks for your patch, the patch described below make sense to me. After
spliting TLB invalidate operations according to domain stage,
arm_smmu_tlb_inv_*_s1() can be used both in SVA codes and original smmu
codes, and can implement arm_smmu_tlb_inv_all_s1() just using
arm_smmu_tlb_inv_range_s1() by passing a special size. But the current
smmu-v3 driver has not move "asid" into "struct smmu_domain", we still
need to pass the exact asid from SVA.
Something like this, on top of my "part 3" branch
https://github.com/jgunthorpe/linux/commits/smmuv3_newapi/
split into more patches:
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 5ab976f6b108dd..b30cce97b01d06 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -30,7 +30,6 @@ static int arm_smmu_realloc_s1_domain_asid(struct arm_smmu_device *smmu,
struct arm_smmu_domain *smmu_domain)
{
struct arm_smmu_master_domain *master_domain;
- u32 old_asid = smmu_domain->asid;
struct arm_smmu_cd target_cd;
unsigned long flags;
int ret;
@@ -68,9 +67,6 @@ static int arm_smmu_realloc_s1_domain_asid(struct arm_smmu_device *smmu,
&target_cd);
}
spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
-
- /* Clean the ASID we are about to assign to a new translation */
- arm_smmu_tlb_inv_asid(smmu, old_asid);
return 0;
}
@@ -175,24 +171,12 @@ static void arm_smmu_mm_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
* range. So do a simple translation here by calculating size correctly.
*/
size = end - start;
- if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_RANGE_INV)) {
- if (size >= CMDQ_MAX_TLBI_OPS * PAGE_SIZE)
- size = 0;
- } else {
- if (size == ULONG_MAX)
- size = 0;
- }
+ if (size == ULONG_MAX)
+ size = 0;
- if (!smmu_domain->btm_invalidation) {
- ioasid_t asid = READ_ONCE(smmu_domain->asid);
-
- if (!size)
- arm_smmu_tlb_inv_asid(smmu_domain->smmu, asid);
- else
- arm_smmu_tlb_inv_range_asid(start, size, asid,
- PAGE_SIZE, false,
- smmu_domain);
- }
+ if (!smmu_domain->btm_invalidation)
+ arm_smmu_tlb_inv_range_s1(start, size, PAGE_SIZE, false,
+ smmu_domain);
arm_smmu_atc_inv_domain(smmu_domain, start, size);
}
@@ -228,7 +212,7 @@ static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
}
spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
- arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_domain->asid);
+ arm_smmu_tlb_inv_all_s1(smmu_domain);
arm_smmu_atc_inv_domain(smmu_domain, 0, 0);
}
@@ -506,6 +490,8 @@ static int arm_smmu_share_asid(struct arm_smmu_device *smmu,
ret = arm_smmu_realloc_s1_domain_asid(smmu, old_s1_domain);
if (ret)
goto out_restore_s1;
+ /* Clean the ASID since it was just recovered */
+ arm_smmu_tlb_inv_all_s1(smmu_domain);
}
smmu_domain->btm_invalidation = true;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index b07e0692eb78ea..79c85791b0c3d5 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -90,6 +90,7 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu);
static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_device *smmu);
static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
+static void arm_smmu_tlb_inv_all_s2(struct arm_smmu_domain *smmu_domain);
static void parse_driver_options(struct arm_smmu_device *smmu)
{
@@ -965,17 +966,6 @@ static int arm_smmu_page_response(struct device *dev,
}
/* Context descriptor manipulation functions */
-void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
-{
- struct arm_smmu_cmdq_ent cmd = {
- .opcode = smmu->features & ARM_SMMU_FEAT_E2H ?
- CMDQ_OP_TLBI_EL2_ASID : CMDQ_OP_TLBI_NH_ASID,
- .tlbi.asid = asid,
- };
-
- arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
-}
-
#define NUM_ENTRY_QWORDS \
(max(sizeof(struct arm_smmu_ste), sizeof(struct arm_smmu_cd)) / \
sizeof(u64))
@@ -2106,8 +2096,6 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
static void arm_smmu_tlb_inv_context(void *cookie)
{
struct arm_smmu_domain *smmu_domain = cookie;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_cmdq_ent cmd;
/*
* NOTE: when io-pgtable is in non-strict mode, we may get here with
@@ -2116,13 +2104,10 @@ static void arm_smmu_tlb_inv_context(void *cookie)
* insertion to guarantee those are observed before the TLBI. Do be
* careful, 007.
*/
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- arm_smmu_tlb_inv_asid(smmu, READ_ONCE(smmu_domain->asid));
- } else {
- cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
- cmd.tlbi.vmid = smmu_domain->vmid;
- arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
- }
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
+ arm_smmu_tlb_inv_all_s1(smmu_domain);
+ else
+ arm_smmu_tlb_inv_all_s2(smmu_domain);
arm_smmu_atc_inv_domain(smmu_domain, 0, 0);
}
@@ -2197,47 +2182,69 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
arm_smmu_cmdq_batch_submit(smmu, &cmds);
}
-static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
- size_t granule, bool leaf,
- struct arm_smmu_domain *smmu_domain)
+static bool arm_smmu_inv_range_too_big(struct arm_smmu_device *smmu,
+ size_t size, size_t granule)
{
- struct arm_smmu_cmdq_ent cmd = {
- .tlbi = {
- .leaf = leaf,
- },
- };
+ unsigned max_ops;
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- cmd.opcode = smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H ?
- CMDQ_OP_TLBI_EL2_VA : CMDQ_OP_TLBI_NH_VA;
- cmd.tlbi.asid = smmu_domain->asid;
- } else {
- cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
- cmd.tlbi.vmid = smmu_domain->vmid;
- }
- __arm_smmu_tlb_inv_range(&cmd, iova, size, granule, smmu_domain);
+ /* 0 size means invalidate all */
+ if (!size || size == SIZE_MAX)
+ return true;
- /*
- * Unfortunately, this can't be leaf-only since we may have
- * zapped an entire table.
- */
- arm_smmu_atc_inv_domain(smmu_domain, iova, size);
+ if (smmu->features & ARM_SMMU_FEAT_RANGE_INV)
+ return false;
+
+ max_ops = 1 << (ilog2(granule) - 3);
+ return size >= max_ops * granule;
}
-void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,
- size_t granule, bool leaf,
- struct arm_smmu_domain *smmu_domain)
+void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size, size_t granule,
+ bool leaf, struct arm_smmu_domain *smmu_domain)
{
struct arm_smmu_cmdq_ent cmd = {
.opcode = smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H ?
CMDQ_OP_TLBI_EL2_VA : CMDQ_OP_TLBI_NH_VA,
.tlbi = {
- .asid = asid,
+ .asid = READ_ONCE(smmu_domain->asid),
.leaf = leaf,
},
};
- __arm_smmu_tlb_inv_range(&cmd, iova, size, granule, smmu_domain);
+ if (arm_smmu_inv_range_too_big(smmu_domain->smmu, size, granule)) {
+ cmd.opcode = smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H ?
+ CMDQ_OP_TLBI_EL2_ASID :
+ CMDQ_OP_TLBI_NH_ASID,
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu_domain->smmu, &cmd);
+ } else{
+ __arm_smmu_tlb_inv_range(&cmd, iova, size, granule,
+ smmu_domain);
+ }
+}
+
+static void arm_smmu_tlb_inv_range_s2(unsigned long iova, size_t size,
+ size_t granule, bool leaf,
+ struct arm_smmu_domain *smmu_domain)
+{
+ struct arm_smmu_cmdq_ent cmd = {
+ .opcode = CMDQ_OP_TLBI_S2_IPA,
+ .tlbi = {
+ .vmid = smmu_domain->vmid,
+ .leaf = leaf,
+ },
+ };
+
+ if (arm_smmu_inv_range_too_big(smmu_domain->smmu, size, granule)) {
+ cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
+ arm_smmu_cmdq_issue_cmd_with_sync(smmu_domain->smmu, &cmd);
+ } else {
+ __arm_smmu_tlb_inv_range(&cmd, iova, size, granule,
+ smmu_domain);
+ }
+}
+
+static void arm_smmu_tlb_inv_all_s2(struct arm_smmu_domain *smmu_domain)
+{
+ arm_smmu_tlb_inv_range_s2(0, 0, PAGE_SIZE, false, smmu_domain);
}
static void arm_smmu_tlb_inv_page_nosync(struct iommu_iotlb_gather *gather,
@@ -2253,7 +2260,15 @@ static void arm_smmu_tlb_inv_page_nosync(struct iommu_iotlb_gather *gather,
static void arm_smmu_tlb_inv_walk(unsigned long iova, size_t size,
size_t granule, void *cookie)
{
- arm_smmu_tlb_inv_range_domain(iova, size, granule, false, cookie);
+ struct arm_smmu_domain *smmu_domain = cookie;
+
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
+ arm_smmu_tlb_inv_range_s1(iova, size, granule, false,
+ smmu_domain);
+ else
+ arm_smmu_tlb_inv_range_s2(iova, size, granule, false,
+ smmu_domain);
+ arm_smmu_atc_inv_domain(smmu_domain, iova, size);
}
static const struct iommu_flush_ops arm_smmu_flush_ops = {
@@ -2379,7 +2394,7 @@ void arm_smmu_domain_free_id(struct arm_smmu_domain *smmu_domain)
if ((smmu_domain->stage == ARM_SMMU_DOMAIN_S1 ||
smmu_domain->domain.type == IOMMU_DOMAIN_SVA) &&
smmu_domain->asid) {
- arm_smmu_tlb_inv_asid(smmu, smmu_domain->asid);
+ arm_smmu_tlb_inv_all_s1(smmu_domain);
/* Prevent SVA from touching the CD while we're freeing it */
mutex_lock(&smmu->asid_lock);
@@ -3183,13 +3198,23 @@ static void arm_smmu_iotlb_sync(struct iommu_domain *domain,
struct iommu_iotlb_gather *gather)
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ size_t size = gather->end - gather->start + 1;
if (!gather->pgsize)
return;
- arm_smmu_tlb_inv_range_domain(gather->start,
- gather->end - gather->start + 1,
- gather->pgsize, true, smmu_domain);
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
+ arm_smmu_tlb_inv_range_s1(gather->start, size, gather->pgsize,
+ true, smmu_domain);
+ else
+ arm_smmu_tlb_inv_range_s2(gather->start, size, gather->pgsize,
+ true, smmu_domain);
+
+ /*
+ * Unfortunately, this can't be leaf-only since we may have
+ * zapped an entire table.
+ */
+ arm_smmu_atc_inv_domain(smmu_domain, gather->start, size);
}
static phys_addr_t
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index d65c716d4fc04f..9f5537ef97caff 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -834,10 +834,13 @@ void arm_smmu_remove_pasid(struct arm_smmu_master *master,
int arm_smmu_domain_alloc_id(struct arm_smmu_device *smmu,
struct arm_smmu_domain *smmu_domain);
void arm_smmu_domain_free_id(struct arm_smmu_domain *smmu_domain);
-void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid);
-void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,
- size_t granule, bool leaf,
- struct arm_smmu_domain *smmu_domain);
+void arm_smmu_tlb_inv_range_s1(unsigned long iova, size_t size, size_t granule,
+ bool leaf, struct arm_smmu_domain *smmu_domain);
+static inline void arm_smmu_tlb_inv_all_s1(struct arm_smmu_domain *smmu_domain)
+{
+ arm_smmu_tlb_inv_range_s1(0, 0, PAGE_SIZE, false, smmu_domain);
+}
+
int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
unsigned long iova, size_t size);