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

[Vijayanand Jitta]

On 8/11/2026 10:34 AM, Daniel Mentz wrote:
> 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.
> 

For the same reason that you mentioned, I don't see we can reuse,
at this point we only have cfg, no data.
>> +       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?
> 

Sure, will rename it to granule.

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

I don't see a easy way to reuse it, for same reason that you mentioned.
Sure, will rename it to 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?
> 
> [...]
> 

You are right, it's always true. The !cfg->pgsize_bitmap check above
rules out the zero case. Will drop the redundant check and keep just
the arm_lpae_cont_size_fits() check.

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

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

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.

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.

Thanks,
Vijay

>> +       }
>> +
>>         /* 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);