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

[Daniel Mentz]

On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
<vijayanand.jitta@xxxxxxxxxxxxxxxx> 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)
> +{
> +       unsigned long pg_size, blk_size, l1_blk_size, cont_sizes = 0;
> +       unsigned long cont_leaf_size, cont_blk_size, cont_l1_blk_size;
> +       int pg_shift, bits_per_level;
> +
> +       if (!cfg->pgsize_bitmap || (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT))
> +               return 0;
> +
> +       pg_shift = __ffs(cfg->pgsize_bitmap);
> +       bits_per_level = pg_shift - ilog2(sizeof(arm_lpae_iopte));

bits_per_level is also calculated in arm_lpae_alloc_pgtable(). I'm
wondering if we can somehow re-use that value, although, I do
understand that data->bits_per_level is only populated later.

> +       pg_size = 1UL << pg_shift;

In arm_lpae_restrict_pgsizes(), they call the same value "granule".
Can we align with that and call it granule instead of pg_size?

> +       blk_size = pg_size << bits_per_level;

I'm wondering if we can re-use the macro ARM_LPAE_BLOCK_SIZE. I do
acknowledge, though, that this macro doesn't work in this context,
because (d)->bits_per_level is still not populated.
Also, for consistency, you might want to call this l2_blk_size.

> +       l1_blk_size = blk_size << bits_per_level;
> +
> +       cont_leaf_size = arm_lpae_num_cont(pg_size) * pg_size;
> +       if ((cfg->pgsize_bitmap & pg_size) &&

Is (cfg->pgsize_bitmap & pg_size) ever false?

[...]

> +/*
> + * 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.

> +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?

> +       }
> +
>         /* If we can install a leaf entry at this level, then do so */
>         if (size == block_size) {
> +               int ret;
> +
>                 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);
> -               if (!ret)
> -                       *mapped += num_entries * size;
> +               num_entries = min_t(size_t, pgcount, max_entries);
>
> -               return ret;
> +               if (!use_cont) {
> +                       ret = arm_lpae_init_pte(data, iova, paddr, prot, lvl,
> +                                               num_entries, ptep);
> +                       if (!ret)
> +                               *mapped += num_entries * size;
> +                       return ret;
> +               }
> +
> +               return arm_lpae_install_leaf(data, iova, paddr, prot, lvl,
> +                                            map_idx_start, num_entries,
> +                                            num_cont, ptep, mapped);