Re: [PATCH v2 1/1] iommu/arm-smmu-v3: Fix error case of range command

On 2023-09-06 06:05, Easwar Hariharan wrote:

On 8/25/23 01:12, zhurui wrote:

quoted

On 2023/8/19 0:21, Will Deacon wrote:

quoted

On Fri, Aug 18, 2023 at 05:19:31PM +0100, Robin Murphy wrote:

quoted

On 2023-08-09 14:48, Robin Murphy wrote:
[...]

quoted

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)

quoted

quoted

----->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 <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
   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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel