On Mon, Apr 05, 2021 at 12:11:09PM -0700, Isaac J. Manjarres wrote:
> Implement the unmap_pages() callback for the ARM LPAE io-pgtable
> format.
>
> Signed-off-by: Isaac J. Manjarres <isa...@codeaurora.org>
> Suggested-by: Will Deacon <w...@kernel.org>
> ---
> drivers/iommu/io-pgtable-arm.c | 124 +++++++++++++++++++++++++++------
> 1 file changed, 104 insertions(+), 20 deletions(-)
[...]
> +static size_t arm_lpae_unmap_pages(struct io_pgtable_ops *ops, unsigned long
> iova,
> + size_t pgsize, size_t pgcount,
> + struct iommu_iotlb_gather *gather)
> +{
> + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> + struct io_pgtable_cfg *cfg = &data->iop.cfg;
> + arm_lpae_iopte *ptep = data->pgd;
> + long iaext = (s64)iova >> cfg->ias;
> + size_t unmapped = 0, unmapped_page;
> + int last_lvl;
> + size_t table_size, pages, tbl_offset, max_entries;
> +
> + if (WARN_ON(!pgsize || (pgsize & cfg->pgsize_bitmap) != pgsize ||
> !pgcount))
> + return 0;
> +
> + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
> + iaext = ~iaext;
> + if (WARN_ON(iaext))
> + return 0;
> +
> + /*
> + * Calculating the page table size here helps avoid situations where
> + * a page range that is being unmapped may be mapped at the same level
> + * but not mapped by the same tables. Allowing such a scenario to
> + * occur can complicate the logic in arm_lpae_split_blk_unmap().
> + */
> + last_lvl = ARM_LPAE_BLOCK_SIZE_LVL(pgsize, data);
> +
> + if (last_lvl == data->start_level)
> + table_size = ARM_LPAE_PGD_SIZE(data);
> + else
> + table_size = ARM_LPAE_GRANULE(data);
> +
> + max_entries = table_size / sizeof(*ptep);
> +
> + while (pgcount) {
> + tbl_offset = ARM_LPAE_LVL_IDX(iova, last_lvl, data);
> + pages = min_t(size_t, pgcount, max_entries - tbl_offset);
> + unmapped_page = __arm_lpae_unmap(data, gather, iova, pgsize,
> + pages, data->start_level,
> + ptep);
> + if (!unmapped_page)
> + break;
> +
> + unmapped += unmapped_page;
> + iova += unmapped_page;
> + pgcount -= pages;
> + }
Robin has some comments on the first version of this patch, and I don't think
you
addressed them:
https://lore.kernel.org/r/b93fa0b1-e2a4-1aad-8b88-4d0dfecdf...@arm.com
I'm inclined to agree that iterating here doesn't make a lot of sense --
we've already come back out of the page-table walk, so I think we should
just return to the caller (who is well prepared to handle a partial unmap).
Same for the map side of things.
If we get numbers showing that this is causing a performance issue, then
we should rework the page-table code to handle this at the lower level
(because I doubt the loop that you have is really worse than returning to
the caller anyway).
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
