[PATCH v5] iommu/io-pgtable-arm: Add support for contiguous hint bit
Daniel Mentz
danielmentz at google.com
Thu Sep 24 13:36:12 PDT 2026
On Mon, Sep 21, 2026 at 4:44 AM Vijayanand Jitta
<vijayanand.jitta at oss.qualcomm.com> wrote:
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 476c0e25631af..01d98959c514f 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -86,6 +86,21 @@
> /* Software bit for solving coherency races */
> #define ARM_LPAE_PTE_SW_SYNC (((arm_lpae_iopte)1) << 55)
>
> +/* PTE Contiguous Bit */
> +#define ARM_LPAE_PTE_CONT (((arm_lpae_iopte)1) << 52)
> +
> +/*
> + * Contiguous hint group sizes per granule:
> + *
> + *------------------------------------------------------------------
> + *| Page Size | CONT PTE | Block | CONT Block | L1 Block | CONT L1 |
> + *------------------------------------------------------------------
> + *| 4K | 64K | 2M | 32M | 1G | 16G |
> + *| 16K | 2M | 32M | 1G | | |
> + *| 64K | 2M | 512M | 16G | | |
> + *------------------------------------------------------------------
> + */
I find this comment redundant. People can find this information in the
Arm architecture specification.
> +static int arm_lpae_num_cont(size_t size)
> +{
> + switch (size) {
> + case SZ_4K:
> + case SZ_2M:
> + case SZ_1G:
> + return 16;
I'm thinking that if you use something like
return BITS_PER_TYPE(size_t) >= 64 ? 16 : 1
i.e. return 16 only on 64 bit platforms, then you can avoid those
overflow checks in various places. Same for the SZ_512M cases.
> + case SZ_64K:
> + case SZ_32M:
> + case SZ_512M:
> + return 32;
> + case SZ_16K:
> + return 128;
> + default:
> + return 1;
> + }
> +}
> +
> static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
> phys_addr_t paddr, size_t size, size_t pgcount,
> arm_lpae_iopte prot, int lvl, arm_lpae_iopte *ptep,
> @@ -462,20 +495,41 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,
> size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data);
> size_t tblsz = ARM_LPAE_GRANULE(data);
> struct io_pgtable_cfg *cfg = &data->iop.cfg;
> - int ret = 0, num_entries, max_entries, map_idx_start;
> + int num_cont = arm_lpae_num_cont(block_size);
> + size_t cont_size = 0, entries_per_map;
> + int num_entries, max_entries, map_idx_start;
> + bool cont = false;
> +
> + if (num_cont > 1 && block_size <= SIZE_MAX / num_cont)
> + cont_size = num_cont * block_size;
>
> /* Find our entry at the current level */
> map_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
> ptep += map_idx_start;
>
> /* If we can install a leaf entry at this level, then do so */
> - if (size == block_size) {
> + if (size == block_size ||
> + (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT) &&
I'm thinking that the check for IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT is
redundant. If this quirk is active, none of the contiguous sizes were
advertised, so no one should call this function with any of the
contiguous sizes.
> + size == cont_size)) {
> + int ret;
> +
> + cont = size == cont_size;
> + if (cont && (!IS_ALIGNED(iova, size) || !IS_ALIGNED(paddr, size)))
These alignment checks are also redundant. We can rely on the caller
to pass properly aligned values. It's also inconsistent, because it
verifies alignment only for contiguous sizes.
> + return -EINVAL;
> +
> + entries_per_map = size / block_size;
Can't you just do
pgcount *= num_cont;
Wouldn't that be easier?
> max_entries = arm_lpae_max_entries(map_idx_start, data);
> - num_entries = min_t(int, pgcount, max_entries);
> - ret = arm_lpae_init_pte(data, iova, paddr, prot, lvl, num_entries, ptep);
> + num_entries = min_t(size_t, pgcount,
> + max_entries / entries_per_map) * entries_per_map;
> + if (!num_entries)
> + return -EINVAL;
I believe this check is also redundant. Can we remove it?
> + if (cont)
> + prot |= ARM_LPAE_PTE_CONT;
> +
> + ret = arm_lpae_init_pte(data, iova, paddr, prot, lvl,
> + num_entries, ptep);
> if (!ret)
> - *mapped += num_entries * size;
> -
> + *mapped += num_entries * block_size;
> return ret;
> }
>
> @@ -660,12 +714,18 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
> {
> arm_lpae_iopte pte;
> struct io_pgtable *iop = &data->iop;
> + size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data);
> + int num_cont = arm_lpae_num_cont(block_size);
> + size_t cont_size = 0, entries_per_map;
> int i = 0, num_entries, max_entries, unmap_idx_start;
>
> /* Something went horribly wrong and we ran out of page table */
> if (WARN_ON(lvl == ARM_LPAE_MAX_LEVELS))
> return 0;
>
> + if (num_cont > 1 && block_size <= SIZE_MAX / num_cont)
> + cont_size = num_cont * block_size;
> +
> unmap_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
> ptep += unmap_idx_start;
> pte = READ_ONCE(*ptep);
> @@ -675,9 +735,27 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
> }
>
> /* If the size matches this level, we're in the right place */
> - if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) {
> + if (size == block_size ||
> + (!(data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT) &&
Checking for IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT is redundant. Can we remove it?
> + size == cont_size)) {
> + entries_per_map = size / block_size;
> max_entries = arm_lpae_max_entries(unmap_idx_start, data);
> - num_entries = min_t(int, pgcount, max_entries);
> + num_entries = min_t(size_t, pgcount,
> + max_entries / entries_per_map) * entries_per_map;
> + if (!num_entries)
> + return 0;
> +
> + /*
> + * A CONT group must be invalidated as a unit. Reject a request that
> + * starts or ends inside a tagged group before changing any PTEs.
> + */
> + if ((READ_ONCE(*ptep) & ARM_LPAE_PTE_CONT &&
> + !IS_ALIGNED(iova, cont_size)) ||
> + (READ_ONCE(ptep[num_entries - 1]) & ARM_LPAE_PTE_CONT &&
> + !IS_ALIGNED(iova + num_entries * block_size, cont_size))) {
> + WARN_ONCE(true, "Unmap of a partial CONT IOPTE group is not allowed");
> + return 0;
> + }
>
> /* Find and handle non-leaf entries */
> for (i = 0; i < num_entries; i++) {
> @@ -691,7 +769,8 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
> __arm_lpae_clear_pte(&ptep[i], &iop->cfg, 1);
>
> /* Also flush any partial walks */
> - io_pgtable_tlb_flush_walk(iop, iova + i * size, size,
> + io_pgtable_tlb_flush_walk(iop,
> + iova + i * block_size, block_size,
> ARM_LPAE_GRANULE(data));
> __arm_lpae_free_pgtable(data, lvl + 1, iopte_deref(pte, data));
> }
> @@ -702,9 +781,10 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>
> if (gather && !iommu_iotlb_gather_queued(gather))
> for (int j = 0; j < i; j++)
> - io_pgtable_tlb_add_page(iop, gather, iova + j * size, size);
> + io_pgtable_tlb_add_page(iop, gather,
> + iova + j * block_size, block_size);
>
> - return i * size;
> + return i * block_size;
> } else if (iopte_leaf(pte, lvl, iop->fmt)) {
> WARN_ONCE(true, "Unmap of a partial large IOPTE is not allowed");
> return 0;
> @@ -943,8 +1023,23 @@ static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
I want to re-iterate what I wrote earlier: I think we should change
this function's name. This is what our AI model has to say:
Originally, arm_lpae_restrict_pgsizes() performed a purely monotonic reduction:
1. Identified the translation granule (e.g. matching CPU PAGE_SIZE).
2. Performed a bitwise-AND (cfg->pgsize_bitmap &= page_sizes) to
discard non-granule sizes.
3. Clamped ias and oas.
With this commit, it now:
1. Restricts to the base granule sizes (&= page_sizes).
2. Expands the bitmap with synthesized contiguous sizes (|= num_cont * size).
3. Restricts again against ias and oas (&= GENMASK_ULL(...)).
Calling a function ..._restrict_... when it actively synthesizes and
injects new page sizes violates the principle of least astonishment.
> }
>
> cfg->pgsize_bitmap &= page_sizes;
> + if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT)) {
> + unsigned long sizes = cfg->pgsize_bitmap;
> +
> + while (sizes) {
> + unsigned long size = BIT(__ffs(sizes));
> + int num_cont = arm_lpae_num_cont(size);
> +
> + if (size <= ULONG_MAX / num_cont)
> + cfg->pgsize_bitmap |= num_cont * size;
> + sizes &= ~size;
> + }
> + }
> +
> cfg->ias = min(cfg->ias, max_addr_bits);
> cfg->oas = min(cfg->oas, max_addr_bits);
> + cfg->pgsize_bitmap &= GENMASK_ULL(cfg->ias, 0);
> + cfg->pgsize_bitmap &= GENMASK_ULL(cfg->oas, 0);
> }
Gemini added the following. I have to admit, though, that I'm not
familiar enough with dirty bit tracking to determine if this is a real
concern.
Interaction with Hardware Dirty Tracking (IO_PGTABLE_QUIRK_ARM_HD)
When Hardware Dirty Tracking (ARM_LPAE_PTE_DBM) is enabled on Stage-1 tables:
* In visit_dirty() / arm_lpae_read_and_clear_dirty(), dirty bits are
queried and cleared on a page-by-page granularity
(iopte_set_writeable_clean(ptep)).
* If a single 4KB page within a 64KB CONT group is marked clean while
neighboring pages remain marked dirty/writeable, the descriptors in
that group will differ in access permissions (AP[2]).
* According to Arm ARM D8.3.1, all descriptors in a contiguous block
must share identical permissions and attributes. If dirty tracking is
active on a domain, consider whether IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT
should be set or whether CONT sizes should be suppressed when
IO_PGTABLE_QUIRK_ARM_HD is active.
More information about the linux-arm-kernel
mailing list