Re: [PATCH v2 1/1] iommu/arm-smmu-v3: Fix error case of range command
From: Robin Murphy
Date: Wed Sep 06 2023 - 09:00:04 EST
On 2023-09-06 06:05, Easwar Hariharan wrote:
On 8/25/23 01:12, zhurui wrote:
On 2023/8/19 0:21, Will Deacon wrote:
On Fri, Aug 18, 2023 at 05:19:31PM +0100, Robin Murphy wrote:
On 2023-08-09 14:48, Robin Murphy wrote:
[...]
Does the patch below work for you?
Any comments on this? Just noticed this commit on a local dev branch
and
realised I'd totally forgotten about it already. I'm pretty
confident it
ought to be right, but then it *was* also me who missed the original
bug to
begin with... ;)
I'm happy to take it if zhurui can confirm that it fixes their issue...
Will (had also forgotten about this)
----->8-----
Subject: [PATCH] iommu/arm-smmu-v3: Avoid constructing invalid range
commands
Although io-pgtable's non-leaf invalidations are always for full
tables,
I missed that SVA also uses non-leaf invalidations, while being at the
mercy of whatever range the MMU notifier throws at it. This means it
definitely wants the previous TTL fix as well, since it also doesn't
know exactly which leaf level(s) may need invalidating, but it can
also
give us less-aligned ranges wherein certain corners may lead to
building
an invalid command where TTL, Num and Scale are all 0. It should be
fine
to handle this by over-invalidating an extra page, since falling
back to
a non-range command opens up a whole can of errata-flavoured worms.
Fixes: 6833b8f2e199 ("iommu/arm-smmu-v3: Set TTL invalidation hint
better")
Reported-by: Rui Zhu <zhurui3@xxxxxxxxxx>
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
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 9b0dc3505601..6ccbae9b93a1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1895,18 +1895,23 @@ static void __arm_smmu_tlb_inv_range(struct
arm_smmu_cmdq_ent *cmd,
/* Get the leaf page size */
tg = __ffs(smmu_domain->domain.pgsize_bitmap);
+ num_pages = size >> tg;
+
/* Convert page size of 12,14,16 (log2) to 1,2,3 */
cmd->tlbi.tg = (tg - 10) / 2;
/*
- * Determine what level the granule is at. For non-leaf,
io-pgtable
- * assumes .tlb_flush_walk can invalidate multiple levels
at once,
- * so ignore the nominal last-level granule and leave TTL=0.
+ * Determine what level the granule is at. For non-leaf, both
+ * io-pgtable and SVA pass a nominal last-level granule
because
+ * they don't know what level(s) actually apply, so ignore
that
+ * and leave TTL=0. However for various errata reasons we
still
+ * want to use a range command, so avoid the SVA corner case
+ * where both scale and num could be 0 as well.
*/
if (cmd->tlbi.leaf)
cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3));
-
- num_pages = size >> tg;
+ else if ((num_pages & CMDQ_TLBI_RANGE_NUM_MAX) == 1)
+ num_pages++;
}
cmds.num = 0;
Hi, Will and Robin,
Sorry for taking so long to reply you. We have some problems with our
machine these days. It's
solved just today. I give a test with Robin's patch for our testcase,
everything is ok. I think
the problem has been solved.
Thanks,
ZhuRui.
Hi Robin,
Could you please send out this patch since ZhuRui has confirmed it fixes
their issue and CC it to stable for v5.15+? Or if Will is willing to
pick it up off this thread, I can do the backport to stable.
I can resend after -rc1 if Will would prefer that. It's tagged as a fix
so should hopefully get picked for stable automatically once it hits
mainline.
Thanks,
Robin.