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/20241024134411.GA6956@xxxxxxxxxx/ > > > > 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 <jgg@xxxxxxxxxx> > > --- > > 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) {
