Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit

On Thu, Aug 13, 2026 at 11:13 PM Vijayanand Jitta
<[email protected]> wrote:
>
>
>
> On 8/11/2026 10:34 AM, Daniel Mentz wrote:
> > On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
> > <[email protected]> wrote:
> >> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> >> index 476c0e25631af..23a238de53ed5 100644
> >> --- a/drivers/iommu/io-pgtable-arm.c
> >> +++ b/drivers/iommu/io-pgtable-arm.c
> >> [...]
> >> +static unsigned long arm_lpae_get_cont_sizes(struct io_pgtable_cfg *cfg)

I'm wondering if we need this function at all. I'm trying to
understand that would happen if we advertise page sizes that cannot
possibly be used given the constraints imposed by cfg->ias and
cfg->oas.
arm_lpae_read_and_clear_dirty() appears to check if the end of the
iova range is out-of-bounds (WARN_ON((iova + size - 1) &
~(BIT(cfg->ias) - 1))), but function arm_lpae_map_pages() appears to
not have such a check.

If we need to restrict the "cont sizes", we could consider Will's
suggestion: Add all the sizes to cfg->pgsize_bitmap in
arm_lpae_restrict_pgsizes, and then subsequently clamp it like so

cfg->pgsize_bitmap &= (BIT(cfg->ias) - 1))
cfg->pgsize_bitmap &= (BIT(cfg->oas) - 1))

That would be shorter than the 39-line arm_lpae_get_cont_sizes function.

> >> +/*
> >> + * Install num_entries leaf entries starting at ptep (index map_idx_start
> >> + * within the current table), tagging arm_lpae_num_cont()-sized groups with
> >> + * the contiguous hint where both idx and paddr are aligned to the group
> >> + * size. Entries in a misaligned group are installed without the hint.
> >> + *
> >> + * idx and paddr both advance by block_size per entry, so their alignment
> >> + * relative to the group size is invariant across a run of entries within
> >> + * this call: once a group qualifies (or fails to), every later whole group
> >> + * does too, up to num_entries. This merges each such run into a single
> >> + * arm_lpae_init_pte() call instead of one call per group.
> >> + */
> >
> > Can you provide an example for when this function installs descriptors
> > where the contiguous bit is only set on a subset of them. I would
> > assume that the contiguous bit is either set for all descriptors or
> > none of them.
> >
>
> That assumption doesn't hold in general -- it's only true when the
> map request happens to start and end on a cont_size boundary. For an
> arbitrary map_pages() call it usually doesn't.

I believe you won't see arbitrary map_pages() calls. I understand that
these calls are exclusively coming from __iommu_map_domain_pgtbl()
which uses iommu_pgsize() to determine optimal page sizes.

> Example, 4K granule (num_cont = 16, cont_size = 64K),
> iova = paddr = 0x1000, pgcount = 34:
>
>    - idx 1..15  (off != 0, misaligned prefix):        installed plain
>    - idx 16..31 (off == 0, paddr now 64K-aligned):     installed w/ CONT
>    - idx 32..34 (off == 0, remaining < num_cont):      installed plain

In the example you provided, I expect that you'll receive three
separate calls from __iommu_map_domain_pgtbl:
 * idx 1..15 with pgsize 4KB
 * one call with pgsize 64KB
 * idx 32..34 with pgsize 4KB

If I took your argument further, I could argue that we'd also have to
check if we can put down a block mapping if iova = paddr = 0x0 and
pgcount = 512, but we're not doing that either.

Could you provide the input parameters to the iommu_map() call that
resulted in the parameters you provided i.e. iova = paddr = 0x1000,
pgcount = 34:

>
> One arm_lpae_install_leaf() call, three chunks, CONT set on only the
> middle one.
>
> >> +static int arm_lpae_install_leaf(struct arm_lpae_io_pgtable *data,
> >> +                                unsigned long iova, phys_addr_t paddr,
> >> +                                arm_lpae_iopte prot, int lvl,
> >> +                                int map_idx_start, int num_entries, int num_cont,
> >> +                                arm_lpae_iopte *ptep, size_t *mapped)
> >> +{
> >> +       size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data);
> >> +       size_t cont_size = num_cont * block_size;
> >> +       int done = 0;
> >> +
> >> +       while (done < num_entries) {
> >> +               int idx = map_idx_start + done;
> >> +               int remaining = num_entries - done;
> >> +               int off = idx % num_cont;
> >> +               arm_lpae_iopte pte = prot;
> >> +               int chunk, ret;
> >> +
> >> +               if (off) {
> >> +                       /* Misaligned prefix: advance to the next boundary */
> >> +                       chunk = min_t(int, num_cont - off, remaining);
> >> +               } else if (remaining >= num_cont && IS_ALIGNED(paddr, cont_size)) {
> >> +                       /* Aligned: merge every full group in this run */
> >> +                       chunk = remaining - remaining % num_cont;
> >> +                       pte |= ARM_LPAE_PTE_CONT;
> >> +               } else {
> >> +                       /*
> >> +                        * Aligned idx but paddr doesn't line up with cont_size,
> >> +                        * or too short for a full group. That holds for the
> >> +                        * rest of this call too, so install the remainder
> >> +                        * plain in one go.
> >> +                        */
> >> +                       chunk = remaining;
> >> +               }
> >> +
> >> +               ret = arm_lpae_init_pte(data, iova, paddr, pte, lvl, chunk, ptep);
> >> +               if (ret)
> >> +                       return ret;
> >> +
> >> +               *mapped += chunk * block_size;
> >> +               ptep += chunk;
> >> +               iova += chunk * block_size;
> >> +               paddr += chunk * block_size;
> >> +               done += chunk;
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >>  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,21 +608,44 @@ 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;
> >> +       bool cont_hint_enabled = !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT);
> >> +       int num_entries, max_entries, map_idx_start;
> >> +       int num_cont = cont_hint_enabled ? arm_lpae_num_cont(block_size) : 1;
> >> +       bool use_cont = cont_hint_enabled && num_cont > 1;
> >>
> >>         /* Find our entry at the current level */
> >>         map_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
> >>         ptep += map_idx_start;
> >>
> >> +       /*
> >> +        * Normalize an exact whole-CONT-group request down to the
> >> +        * equivalent block_size/pgcount so it funnels through the same
> >> +        * leaf path below. arm_lpae_install_leaf() independently decides,
> >> +        * per sub-chunk, whether the CONT hint actually applies.
> >> +        */
> >> +       if (use_cont && size == block_size * num_cont) {
> >> +               pgcount *= num_cont;
> >> +               size = block_size;
> >
> > This appears to me as if you're throwing away information about
> > whether this mapping request is suitable for the contiguous bit, and
> > then in arm_lpae_install_leaf(), you're trying to recover that
> > information. Can't you just do "prot |=ARM_LPAE_PTE_CONT" here and
> > then completely avoid the logic in arm_lpae_install_leaf?
> >
>
> That optimization only applies to the exact-whole-group case already
> handled above (size == block_size * num_cont). A single map_pages()
> call can also cover the general case shown above, where a misaligned
> prefix/suffix surrounds one or more aligned groups within the same
> call. Setting prot |= ARM_LPAE_PTE_CONT unconditionally here would
> incorrectly tag those misaligned entries with the hint.

Due to how __iommu_map_domain_pgtbl and iommu_pgsize operate, I don't
expect to see the prefixes and suffixes that you are describing.
Instead, I expect we'll see separate calls to __arm_lpae_map(): One
for the prefix, one for the set of aligned groups and another one for
the suffix.

>
> arm_lpae_install_leaf()'s off/remaining logic is what detects those
> group boundaries per chunk, so I don't think we can drop it in favor
> of always setting prot |= CONT at this call site. The size ==
> block_size * num_cont check here is just a fast path for the common
> whole-group case, avoiding a walk through install_leaf() for something
> the caller has already told us.
>