On Mon, Nov 04, 2024 at 07:53:46PM +0000, Robin Murphy wrote:
> On 2024-11-04 5:41 pm, Jason Gunthorpe wrote:
> > A minority of page table implementations (arm_lpae, armv7) are unique in
> > how they handle partial unmap of large IOPTEs.
> >
> > Other implementations will unmap the large IOPTE and return it's
> > length. For example if a 2M IOPTE is present and the first 4K is requested
> > to be unmapped then unmap will remove the whole 2M and report 2M as the
> > result.
> >
> > armv7 instead will break up contiguous entries and replace an entry with a
> > whole table so it can unmap the requested 4k.
> >
> > This seems copied from the arm_lpae implementation, which was analyzed
> > here:
> >
> > https://lore.kernel.org/all/[email protected]/
> >
> > Bring consistency to the implementations and remove this unused
> > functionality.
> >
> > There are no uses outside iommu, this effects the ARM_V7S drivers
> > msm_iommu, mtk_iommu, and arm-smmmu.
> >
> > Signed-off-by: Jason Gunthorpe <[email protected]>
> > ---
> > drivers/iommu/io-pgtable-arm-v7s.c | 125 +----------------------------
> > 1 file changed, 4 insertions(+), 121 deletions(-)
>
> Yikes, I'd forgotten quite how much horribleness was devoted to this,
> despite it being the "simpler" non-recursive one...
Yes, it is the contiguous page support that makes it so complex..
> However, there are also "partial unmap" cases in both sets of selftests, so
> I think there's still a bit more to remove yet :)
Sneaky, I got it thanks
Runs OK now:
arm-v7s io-pgtable: self test ok
arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
Jason
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -819,7 +819,7 @@ static int __init arm_v7s_do_selftests(void)
.quirks = IO_PGTABLE_QUIRK_ARM_NS,
.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
};
- unsigned int iova, size, iova_start;
+ unsigned int iova, size;
unsigned int i, loopnr = 0;
size_t mapped;
@@ -871,25 +871,6 @@ static int __init arm_v7s_do_selftests(void)
loopnr++;
}
- /* Partial unmap */
- i = 1;
- size = 1UL << __ffs(cfg.pgsize_bitmap);
- while (i < loopnr) {
- iova_start = i * SZ_16M;
- if (ops->unmap_pages(ops, iova_start + size, size, 1, NULL) !=
size)
- return __FAIL(ops);
-
- /* Remap of partial unmap */
- if (ops->map_pages(ops, iova_start + size, size, size, 1,
- IOMMU_READ, GFP_KERNEL, &mapped))
- return __FAIL(ops);
-
- if (ops->iova_to_phys(ops, iova_start + size + 42)
- != (size + 42))
- return __FAIL(ops);
- i++;
- }
-
/* Full unmap */
iova = 0;
for_each_set_bit(i, &cfg.pgsize_bitmap, BITS_PER_LONG) {
