Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
@ 2026-08-04  6:14 Vijayanand Jitta
  2026-08-11  5:04 ` Daniel Mentz
  2026-08-14 21:57 ` Daniel Mentz
  0 siblings, 2 replies; 8+ messages in thread
From: Vijayanand Jitta @ 2026-08-04  6:14 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel (AMD)
  Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel,
	Prakash Gupta, Vijayanand Jitta

From: Prakash Gupta <prakash.gupta@oss.qualcomm.com>

Add support for the contiguous hint (CONT) bit in ARM LPAE page tables.
When a set of consecutive PTEs map a naturally-aligned contiguous block
of memory, the CONT bit can be set on all entries in the group to allow
the hardware to combine them into a single TLB entry, improving TLB
utilization.

The contiguous hint sizes per granule are:

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

Contiguous hint sizes are advertised in pgsize_bitmap so that IOMMU API
users can align allocations to these sizes and benefit from the TLB
optimization automatically.

Partial unmaps of a contiguous group are rejected, ensuring the full
group is always invalidated as a unit. The
IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT quirk allows SMMU drivers to disable
contiguous hint support at runtime for hardware with
implementation-specific errata.

Suggested-by: Robin Murphy <robin.murphy@arm.com>
Co-developed-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Signed-off-by: Prakash Gupta <prakash.gupta@oss.qualcomm.com>
---
Changes in v4:
- Merge each run of consecutive aligned (or consecutive non-aligned)
  num_cont windows within arm_lpae_install_leaf() into a single
  arm_lpae_init_pte() call instead of one call per window. idx and
  paddr both advance by block_size per entry, so once a window
  qualifies (or fails to qualify) for the CONT hint, every later whole
  window in the same call does too - collapsing the common fully-
  aligned or fully-unaligned case back down to one call, matching v2's
  call count without reintroducing v2's alignment gaps.
- Fix min_t(int, pgcount, max_entries) truncating pgcount (size_t) to a
  signed int in both __arm_lpae_map() and __arm_lpae_unmap(), which
  could go negative and spin the caller's while (pgcount) loop forever
  for a large enough single request - especially reachable now that
  pgcount is scaled by num_cont (up to 128) for whole-CONT-group
  requests. Compare in size_t via min_t(size_t, ...) instead; the
  result remains bounded by max_entries before being stored back into
  the int num_entries. Reported by the Sashiko AI review bot on v3.
- Stop short of the offending entry instead of returning 0 outright
  when __arm_lpae_unmap() detects a misaligned CONT group mid-loop.
  Earlier entries in the same call may already have had non-leaf
  sub-tables torn down and freed, so returning 0 both under-reports
  the actual unmap progress to the caller and skips the bulk
  clear/gather for those already-freed entries. Breaking out of the
  loop at the current index lets the existing post-loop clear/gather
  path handle entries [0, i) correctly and report i * size unmapped.
  Reported by the Sashiko AI review bot on v3.
- Link to v3: https://lore.kernel.org/all/20260722-iommu_contig_hint-v3-1-10923a683441@oss.qualcomm.com/

Changes in v3:
- Collapse arm_lpae_cont_ptes()/arm_lpae_cont_blks()/
  arm_lpae_cont_pte_size()/arm_lpae_cont_blk_size()/
  arm_lpae_find_num_cont() into a single arm_lpae_num_cont(size_t size)
  helper, since leaf/block/L1-block sizes never overlap across granules.
- Fix arm_lpae_pte_is_contiguous_range() never checking that iova/paddr
  are aligned to the contiguous group size, by removing it entirely -
  __arm_lpae_map()'s new arm_lpae_install_leaf() helper scans for
  aligned sub-chunks and independently verifies paddr alignment for
  each one before applying the CONT hint.
- Fold the CONT case into __arm_lpae_map()'s existing size == block_size
  leaf path instead of duplicating arm_lpae_init_pte() in a separate
  branch. A request whose size exactly matches a whole CONT group is
  normalized down to block_size/scaled pgcount so it reaches that path;
  arm_lpae_install_leaf() then walks the resulting range in chunks,
  tagging only the sub-chunks that are both index-aligned and
  paddr-aligned to the group size, since a single map_pages() call at
  the plain block_size can still contain such an aligned group midway
  through a larger, otherwise ungrouped range.
- Replace the unmap-side WARN_ON_ONCE(!IS_ALIGNED(iova, size)) with a
  check on the actual ARM_LPAE_PTE_CONT bit of the PTEs being cleared.
  The previous check incorrectly warned on any unmap whose size
  happened to numerically match a CONT group size, even when the
  underlying PTEs were never CONT-tagged (e.g. because the original
  iommu_map() wasn't group-aligned), and even though iommu_unmap() can
  legitimately assemble such a size from multiple independent prior
  iommu_map() calls.
- Close a leak where ARM_MALI_LPAE would gain CONT-sized entries in its
  pgsize_bitmap despite the format having no CONT bit, by having
  arm_mali_lpae_alloc_pgtable() set IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT
  internally rather than special-casing the format in the shared
  arm_lpae_restrict_pgsizes()/arm_lpae_get_cont_sizes() path.
- Gate each contiguous-hint size in arm_lpae_get_cont_sizes() on whether
  a single group actually fits within the configured IAS/OAS, so e.g. a
  16G level-1 CONT group is never advertised for an IAS too small to
  address it.
- Link to v2: https://patch.msgid.link/20260721-iommu_contig_hint-v2-1-90c731a41163@oss.qualcomm.com

Changes in v2:
- Extend contiguous hint support to level-1 (1G) blocks for the 4K granule,
  adding a CONT L1 (16G) grouping alongside the existing CONT PTE/CONT Block
  sizes.
- Replace the compile-time CONFIG_IOMMU_IO_PGTABLE_CONTIG_HINT Kconfig option
  with a runtime quirk, IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT, so SMMU drivers
  can opt out per page table instance instead of at build time.
- Simplify __arm_lpae_map() to program the CONT-sized block directly via
  arm_lpae_init_pte() instead of recursing into the next level with an
  adjusted pgcount.
- Reject unmaps that are not aligned to the contiguous group size with
  WARN_ON_ONCE(), instead of clearing the CONT bit on a partial group before
  invalidation.
- Link to v1: https://patch.msgid.link/20260618-iommu_contig_hint-v1-1-4502a59e6388@oss.qualcomm.com

To: Will Deacon <will@kernel.org>
To: Robin Murphy <robin.murphy@arm.com>
To: "Joerg Roedel (AMD)" <joro@8bytes.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: iommu@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
---
 drivers/iommu/io-pgtable-arm.c | 242 +++++++++++++++++++++++++++++++++++++++--
 include/linux/io-pgtable.h     |   3 +
 2 files changed, 235 insertions(+), 10 deletions(-)

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
@@ -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     |          |         |
+ *------------------------------------------------------------------
+ */
+
 /* Stage-1 PTE */
 #define ARM_LPAE_PTE_AP_UNPRIV		(((arm_lpae_iopte)1) << 6)
 #define ARM_LPAE_PTE_AP_RDONLY_BIT	7
@@ -453,6 +468,137 @@ static arm_lpae_iopte arm_lpae_install_table(arm_lpae_iopte *table,
 	return old;
 }
 
+static int arm_lpae_num_cont(size_t size)
+{
+	switch (size) {
+	case SZ_4K:
+	case SZ_2M:
+	case SZ_1G:
+		return 16;
+	case SZ_64K:
+	case SZ_32M:
+	case SZ_512M:
+		return 32;
+	case SZ_16K:
+		return 128;
+	default:
+		return 1;
+	}
+}
+
+/*
+ * A group is only usable if its span fits within both the configured
+ * ias and oas. Otherwise no aligned iova/paddr pair for a full group
+ * can exist, and advertising the size would let callers pick a
+ * mapping that unconditionally fails.
+ */
+static bool arm_lpae_cont_size_fits(struct io_pgtable_cfg *cfg, unsigned long size)
+{
+	int size_bits = ilog2(size);
+
+	return size_bits <= cfg->ias && size_bits <= cfg->oas;
+}
+
+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));
+	pg_size = 1UL << pg_shift;
+	blk_size = pg_size << bits_per_level;
+	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) &&
+	    arm_lpae_cont_size_fits(cfg, cont_leaf_size))
+		cont_sizes |= cont_leaf_size;
+
+	if (cfg->pgsize_bitmap & blk_size) {
+		cont_blk_size = arm_lpae_num_cont(blk_size) * blk_size;
+		if (arm_lpae_cont_size_fits(cfg, cont_blk_size))
+			cont_sizes |= cont_blk_size;
+	}
+
+	/*
+	 * l1_blk_size is only set in pgsize_bitmap if level-1 blocks are
+	 * supported for this granule (not 16K/64K, per
+	 * arm_lpae_restrict_pgsizes()), so no extra gating is needed here.
+	 */
+	if (cfg->pgsize_bitmap & l1_blk_size) {
+		cont_l1_blk_size = arm_lpae_num_cont(l1_blk_size) * l1_blk_size;
+		if (arm_lpae_cont_size_fits(cfg, cont_l1_blk_size))
+			cont_sizes |= cont_l1_blk_size;
+	}
+
+	return cont_sizes;
+}
+
+/*
+ * 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.
+ */
+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;
+	}
+
 	/* 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);
 	}
 
 	/* We can't allocate tables at the final level */
@@ -660,6 +829,8 @@ 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);
 	int i = 0, num_entries, max_entries, unmap_idx_start;
 
 	/* Something went horribly wrong and we ran out of page table */
@@ -674,10 +845,22 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
 		return 0;
 	}
 
+	/*
+	 * Normalize an exact whole-CONT-group request down to the
+	 * equivalent block_size/pgcount, mirroring __arm_lpae_map().
+	 */
+	if (!(data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT) &&
+	    num_cont > 1 && size == block_size * num_cont) {
+		pgcount *= num_cont;
+		size = block_size;
+	}
+
 	/* If the size matches this level, we're in the right place */
-	if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) {
+	if (size == block_size) {
+		size_t cont_size = num_cont * 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);
 
 		/* Find and handle non-leaf entries */
 		for (i = 0; i < num_entries; i++) {
@@ -687,6 +870,40 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
 				break;
 			}
 
+			/*
+			 * A real CONT group must always be invalidated as a
+			 * unit, so reject an unmap that splits one. Check the
+			 * PTE's own CONT bit rather than the caller's size,
+			 * since a legitimate unmap can span multiple prior
+			 * iommu_map() calls and its size alone doesn't say how
+			 * the underlying PTEs were grouped. Only the first and
+			 * last entries can straddle a group boundary; an
+			 * interior CONT-tagged entry's group is necessarily
+			 * fully covered by this unmap, since groups can't
+			 * overlap without also covering everything between
+			 * them.
+			 */
+			if (pte & ARM_LPAE_PTE_CONT) {
+				bool ok = true;
+
+				if (i == 0)
+					ok = ok && IS_ALIGNED(iova, cont_size);
+				if (i == num_entries - 1)
+					ok = ok && IS_ALIGNED(iova + (i + 1) * block_size,
+							      cont_size);
+
+				/*
+				 * Stop short of this entry instead of returning
+				 * 0: entries before i may already have had
+				 * non-leaf sub-tables torn down above, so the
+				 * caller needs the real unmapped count, and the
+				 * loop exit below still clears/gathers entries
+				 * [0, i) correctly.
+				 */
+				if (WARN_ON_ONCE(!ok))
+					break;
+			}
+
 			if (!iopte_leaf(pte, lvl, iop->fmt)) {
 				__arm_lpae_clear_pte(&ptep[i], &iop->cfg, 1);
 
@@ -943,6 +1160,7 @@ static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
 	}
 
 	cfg->pgsize_bitmap &= page_sizes;
+	cfg->pgsize_bitmap |= arm_lpae_get_cont_sizes(cfg);
 	cfg->ias = min(cfg->ias, max_addr_bits);
 	cfg->oas = min(cfg->oas, max_addr_bits);
 }
@@ -1001,7 +1219,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie)
 			    IO_PGTABLE_QUIRK_ARM_TTBR1 |
 			    IO_PGTABLE_QUIRK_ARM_OUTER_WBWA |
 			    IO_PGTABLE_QUIRK_ARM_HD |
-			    IO_PGTABLE_QUIRK_NO_WARN))
+			    IO_PGTABLE_QUIRK_NO_WARN |
+			    IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT))
 		return NULL;
 
 	data = arm_lpae_alloc_pgtable(cfg);
@@ -1103,7 +1322,8 @@ arm_64_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, void *cookie)
 	typeof(&cfg->arm_lpae_s2_cfg.vtcr) vtcr = &cfg->arm_lpae_s2_cfg.vtcr;
 
 	if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_S2FWB |
-			    IO_PGTABLE_QUIRK_NO_WARN))
+			    IO_PGTABLE_QUIRK_NO_WARN |
+			    IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT))
 		return NULL;
 
 	data = arm_lpae_alloc_pgtable(cfg);
@@ -1224,6 +1444,8 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
 		return NULL;
 
 	cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G);
+	/* Mali LPAE has no CONT bit - never advertise CONT page sizes */
+	cfg->quirks |= IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT;
 
 	data = arm_lpae_alloc_pgtable(cfg);
 	if (!data)
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index e19872e37e067..7b2097aaffb09 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -86,6 +86,8 @@ struct io_pgtable_cfg {
 	 *
 	 * IO_PGTABLE_QUIRK_ARM_HD: Enables dirty tracking in stage 1 pagetable.
 	 * IO_PGTABLE_QUIRK_ARM_S2FWB: Use the FWB format for the MemAttrs bits
+	 * IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT: Disable use of the contiguous
+	 *	hint for hardware affected by implementation-specific errata.
 	 *
 	 * IO_PGTABLE_QUIRK_NO_WARN: Do not WARN_ON() on conflicting
 	 *	mappings, but silently return -EEXISTS.  Normally an attempt
@@ -103,6 +105,7 @@ struct io_pgtable_cfg {
 	#define IO_PGTABLE_QUIRK_ARM_HD			BIT(7)
 	#define IO_PGTABLE_QUIRK_ARM_S2FWB		BIT(8)
 	#define IO_PGTABLE_QUIRK_NO_WARN		BIT(9)
+	#define IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT	BIT(10)
 	unsigned long			quirks;
 	unsigned long			pgsize_bitmap;
 	unsigned int			ias;

---
base-commit: 9a4cdc958dd79fc6c3b20b51a10debec6ca09fec
change-id: 20260618-iommu_contig_hint-71ae491fbb52

Best regards,
--  
Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-04  6:14 [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit Vijayanand Jitta
@ 2026-08-11  5:04 ` Daniel Mentz
  2026-08-14  6:12   ` Vijayanand Jitta
  2026-08-14 21:57 ` Daniel Mentz
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Mentz @ 2026-08-11  5:04 UTC (permalink / raw)
  To: Vijayanand Jitta, Prakash Gupta
  Cc: Will Deacon, Robin Murphy, Joerg Roedel (AMD), linux-arm-msm,
	linux-arm-kernel, iommu, linux-kernel

On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
<vijayanand.jitta@oss.qualcomm.com> 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);


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-11  5:04 ` Daniel Mentz
@ 2026-08-14  6:12   ` Vijayanand Jitta
  2026-08-14 20:45     ` Daniel Mentz
  0 siblings, 1 reply; 8+ messages in thread
From: Vijayanand Jitta @ 2026-08-14  6:12 UTC (permalink / raw)
  To: Daniel Mentz, Prakash Gupta
  Cc: Will Deacon, Robin Murphy, Joerg Roedel (AMD), linux-arm-msm,
	linux-arm-kernel, iommu, linux-kernel



On 8/11/2026 10:34 AM, Daniel Mentz wrote:
> On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
> <vijayanand.jitta@oss.qualcomm.com> 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);



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-14  6:12   ` Vijayanand Jitta
@ 2026-08-14 20:45     ` Daniel Mentz
  2026-08-27  5:54       ` Vijayanand Jitta
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Mentz @ 2026-08-14 20:45 UTC (permalink / raw)
  To: Vijayanand Jitta
  Cc: Prakash Gupta, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
	linux-arm-msm, linux-arm-kernel, iommu, linux-kernel

On Thu, Aug 13, 2026 at 11:13 PM Vijayanand Jitta
<vijayanand.jitta@oss.qualcomm.com> wrote:
>
>
>
> On 8/11/2026 10:34 AM, Daniel Mentz wrote:
> > On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
> > <vijayanand.jitta@oss.qualcomm.com> 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.
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-04  6:14 [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit Vijayanand Jitta
  2026-08-11  5:04 ` Daniel Mentz
@ 2026-08-14 21:57 ` Daniel Mentz
  2026-08-27  8:24   ` Vijayanand Jitta
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Mentz @ 2026-08-14 21:57 UTC (permalink / raw)
  To: Vijayanand Jitta
  Cc: Will Deacon, Robin Murphy, Joerg Roedel (AMD), linux-arm-msm,
	linux-arm-kernel, iommu, linux-kernel, Prakash Gupta

On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
<vijayanand.jitta@oss.qualcomm.com> wrote:
> +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));
> +       pg_size = 1UL << pg_shift;
> +       blk_size = pg_size << bits_per_level;
> +       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) &&
> +           arm_lpae_cont_size_fits(cfg, cont_leaf_size))
> +               cont_sizes |= cont_leaf_size;
> +
> +       if (cfg->pgsize_bitmap & blk_size) {
> +               cont_blk_size = arm_lpae_num_cont(blk_size) * blk_size;
> +               if (arm_lpae_cont_size_fits(cfg, cont_blk_size))
> +                       cont_sizes |= cont_blk_size;
> +       }
> +
> +       /*
> +        * l1_blk_size is only set in pgsize_bitmap if level-1 blocks are
> +        * supported for this granule (not 16K/64K, per
> +        * arm_lpae_restrict_pgsizes()), so no extra gating is needed here.
> +        */
> +       if (cfg->pgsize_bitmap & l1_blk_size) {
> +               cont_l1_blk_size = arm_lpae_num_cont(l1_blk_size) * l1_blk_size;

Our AI model is saying that this might overflow cont_l1_blk_size on 32
bit platforms i.e. 16 * 1G doesn't fit into a 32 bit type. It says
that cont_l1_blk_size will be truncated to 0, and
arm_lpae_cont_size_fits() then calls ilog2(0) which is undefined.

> +               if (arm_lpae_cont_size_fits(cfg, cont_l1_blk_size))
> +                       cont_sizes |= cont_l1_blk_size;
> +       }
> +
> +       return cont_sizes;
> +}
[...]
> @@ -660,6 +829,8 @@ 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);
>         int i = 0, num_entries, max_entries, unmap_idx_start;
>
>         /* Something went horribly wrong and we ran out of page table */
> @@ -674,10 +845,22 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>                 return 0;
>         }
>
> +       /*
> +        * Normalize an exact whole-CONT-group request down to the
> +        * equivalent block_size/pgcount, mirroring __arm_lpae_map().
> +        */
> +       if (!(data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT) &&
> +           num_cont > 1 && size == block_size * num_cont) {
> +               pgcount *= num_cont;
> +               size = block_size;
> +       }
> +
>         /* If the size matches this level, we're in the right place */
> -       if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) {
> +       if (size == block_size) {
> +               size_t cont_size = num_cont * 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);
>
>                 /* Find and handle non-leaf entries */

This comment is no longer accurate. The handling now extends beyond
non-leaf entries.

>                 for (i = 0; i < num_entries; i++) {
> @@ -687,6 +870,40 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>                                 break;
>                         }
>
> +                       /*
> +                        * A real CONT group must always be invalidated as a
> +                        * unit, so reject an unmap that splits one. Check the
> +                        * PTE's own CONT bit rather than the caller's size,
> +                        * since a legitimate unmap can span multiple prior
> +                        * iommu_map() calls and its size alone doesn't say how
> +                        * the underlying PTEs were grouped. Only the first and
> +                        * last entries can straddle a group boundary; an
> +                        * interior CONT-tagged entry's group is necessarily
> +                        * fully covered by this unmap, since groups can't
> +                        * overlap without also covering everything between
> +                        * them.
> +                        */
> +                       if (pte & ARM_LPAE_PTE_CONT) {
> +                               bool ok = true;
> +
> +                               if (i == 0)
> +                                       ok = ok && IS_ALIGNED(iova, cont_size);
> +                               if (i == num_entries - 1)
> +                                       ok = ok && IS_ALIGNED(iova + (i + 1) * block_size,
> +                                                             cont_size);
> +
> +                               /*
> +                                * Stop short of this entry instead of returning
> +                                * 0: entries before i may already have had
> +                                * non-leaf sub-tables torn down above, so the
> +                                * caller needs the real unmapped count, and the
> +                                * loop exit below still clears/gathers entries
> +                                * [0, i) correctly.
> +                                */
> +                               if (WARN_ON_ONCE(!ok))

Consider aligning with the following WARN_ONCE in the same function:

WARN_ONCE(true, "Unmap of a partial large IOPTE is not allowed");

> +                                       break;

I think this behavior is inconsistent: when a problem is detected at
the beginning of the unmap range, you return without modifying the
table, whereas if it's detected at the end, the code proceeds with
unmapping and leaves the table misconfigured. Could these checks be
performed before entering the loop?

> +                       }
> +
>                         if (!iopte_leaf(pte, lvl, iop->fmt)) {
>                                 __arm_lpae_clear_pte(&ptep[i], &iop->cfg, 1);
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-14 20:45     ` Daniel Mentz
@ 2026-08-27  5:54       ` Vijayanand Jitta
  0 siblings, 0 replies; 8+ messages in thread
From: Vijayanand Jitta @ 2026-08-27  5:54 UTC (permalink / raw)
  To: Daniel Mentz
  Cc: Prakash Gupta, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
	linux-arm-msm, linux-arm-kernel, iommu, linux-kernel



On 8/15/2026 2:15 AM, Daniel Mentz wrote:
> On Thu, Aug 13, 2026 at 11:13 PM Vijayanand Jitta
> <vijayanand.jitta@oss.qualcomm.com> wrote:
>>
>>
>>
>> On 8/11/2026 10:34 AM, Daniel Mentz wrote:
>>> On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
>>> <vijayanand.jitta@oss.qualcomm.com> 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.
> 

Agree , I think both arm_lpae_get_cont_sizes and arm_lpae_cont_size_fits can
be removed.

Instead I'll add something like below to arm_lpae_restrict_pgsizes as suggested.

+ if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT)) {
+        unsigned long sizes = cfg->pgsize_bitmap;
+
+        while (sizes) {
+                unsigned long size = BIT(__ffs(sizes));
+
+                cfg->pgsize_bitmap |= arm_lpae_num_cont(size) * size;
+                sizes &= ~size;
+        }
+  }

  cfg->ias = min(cfg->ias, max_addr_bits);
  cfg->oas = min(cfg->oas, max_addr_bits);

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

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

You're right -- for the iommu_map()/__iommu_map_domain_pgtbl() path, iommu_pgsize()
already splits the request at the boundaries you describe before install_leaf() ever
sees it, so install_leaf() doesn't need to handle a mixed prefix/CONT-group/suffix
chunk for that caller.

That said, install_leaf() is shared by other callers that reach it through
ops->map_pages() directly, without going through iommu_pgsize(). panthor_vm_map_pages()
(drivers/gpu/drm/panthor/panthor_mmu.c) is one -- it allocates its io_pgtable_ops via
alloc_io_pgtable_ops(ARM_64_LPAE_S1, ...), same as any other LPAE consumer, but does its
own chunking with a local get_pgsize() that only ever returns SZ_4K or SZ_2M, with no
notion of the 64K/32M CONT boundaries. That can hand install_leaf() exactly the mixed
iova=paddr=0x1000, pgcount=34 shape in a single call (panfrost's map loop uses the same
get_pgsize() and hits the same case). So the prefix/aligned-group/suffix handling in
install_leaf() is still needed for that path.


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

Agreed, Replied in above comment.

Thanks,
Vijay>>
>> 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.
>>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-14 21:57 ` Daniel Mentz
@ 2026-08-27  8:24   ` Vijayanand Jitta
  2026-08-27  8:38     ` Vijayanand Jitta
  0 siblings, 1 reply; 8+ messages in thread
From: Vijayanand Jitta @ 2026-08-27  8:24 UTC (permalink / raw)
  To: Daniel Mentz
  Cc: Will Deacon, Robin Murphy, Joerg Roedel (AMD), linux-arm-msm,
	linux-arm-kernel, iommu, linux-kernel, Prakash Gupta



On 8/15/2026 3:27 AM, Daniel Mentz wrote:
> On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
> <vijayanand.jitta@oss.qualcomm.com> wrote:
>> +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));
>> +       pg_size = 1UL << pg_shift;
>> +       blk_size = pg_size << bits_per_level;
>> +       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) &&
>> +           arm_lpae_cont_size_fits(cfg, cont_leaf_size))
>> +               cont_sizes |= cont_leaf_size;
>> +
>> +       if (cfg->pgsize_bitmap & blk_size) {
>> +               cont_blk_size = arm_lpae_num_cont(blk_size) * blk_size;
>> +               if (arm_lpae_cont_size_fits(cfg, cont_blk_size))
>> +                       cont_sizes |= cont_blk_size;
>> +       }
>> +
>> +       /*
>> +        * l1_blk_size is only set in pgsize_bitmap if level-1 blocks are
>> +        * supported for this granule (not 16K/64K, per
>> +        * arm_lpae_restrict_pgsizes()), so no extra gating is needed here.
>> +        */
>> +       if (cfg->pgsize_bitmap & l1_blk_size) {
>> +               cont_l1_blk_size = arm_lpae_num_cont(l1_blk_size) * l1_blk_size;
> 
> Our AI model is saying that this might overflow cont_l1_blk_size on 32
> bit platforms i.e. 16 * 1G doesn't fit into a 32 bit type. It says
> that cont_l1_blk_size will be truncated to 0, and
> arm_lpae_cont_size_fits() then calls ilog2(0) which is undefined.
> 

Ack. With arm_lpae_cont_size_fits removed this won't be an issue anymore.

>> +               if (arm_lpae_cont_size_fits(cfg, cont_l1_blk_size))
>> +                       cont_sizes |= cont_l1_blk_size;
>> +       }
>> +
>> +       return cont_sizes;
>> +}
> [...]
>> @@ -660,6 +829,8 @@ 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);
>>         int i = 0, num_entries, max_entries, unmap_idx_start;
>>
>>         /* Something went horribly wrong and we ran out of page table */
>> @@ -674,10 +845,22 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>                 return 0;
>>         }
>>
>> +       /*
>> +        * Normalize an exact whole-CONT-group request down to the
>> +        * equivalent block_size/pgcount, mirroring __arm_lpae_map().
>> +        */
>> +       if (!(data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT) &&
>> +           num_cont > 1 && size == block_size * num_cont) {
>> +               pgcount *= num_cont;
>> +               size = block_size;
>> +       }
>> +
>>         /* If the size matches this level, we're in the right place */
>> -       if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) {
>> +       if (size == block_size) {
>> +               size_t cont_size = num_cont * 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);
>>
>>                 /* Find and handle non-leaf entries */
> 
> This comment is no longer accurate. The handling now extends beyond
> non-leaf entries.
> 

Agreed, that comment is stale -- the loop now also validates CONT-group
alignment on leaf entries (rejecting an unmap that would split a tagged
group) before falling through to the non-leaf teardown. Will update it to
something like:

/* Validate leaf entries and handle non-leaf entries */


>>                 for (i = 0; i < num_entries; i++) {
>> @@ -687,6 +870,40 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>                                 break;
>>                         }
>>
>> +                       /*
>> +                        * A real CONT group must always be invalidated as a
>> +                        * unit, so reject an unmap that splits one. Check the
>> +                        * PTE's own CONT bit rather than the caller's size,
>> +                        * since a legitimate unmap can span multiple prior
>> +                        * iommu_map() calls and its size alone doesn't say how
>> +                        * the underlying PTEs were grouped. Only the first and
>> +                        * last entries can straddle a group boundary; an
>> +                        * interior CONT-tagged entry's group is necessarily
>> +                        * fully covered by this unmap, since groups can't
>> +                        * overlap without also covering everything between
>> +                        * them.
>> +                        */
>> +                       if (pte & ARM_LPAE_PTE_CONT) {
>> +                               bool ok = true;
>> +
>> +                               if (i == 0)
>> +                                       ok = ok && IS_ALIGNED(iova, cont_size);
>> +                               if (i == num_entries - 1)
>> +                                       ok = ok && IS_ALIGNED(iova + (i + 1) * block_size,
>> +                                                             cont_size);
>> +
>> +                               /*
>> +                                * Stop short of this entry instead of returning
>> +                                * 0: entries before i may already have had
>> +                                * non-leaf sub-tables torn down above, so the
>> +                                * caller needs the real unmapped count, and the
>> +                                * loop exit below still clears/gathers entries
>> +                                * [0, i) correctly.
>> +                                */
>> +                               if (WARN_ON_ONCE(!ok))
> 
> Consider aligning with the following WARN_ONCE in the same function:
> 
> WARN_ONCE(true, "Unmap of a partial large IOPTE is not allowed");
> 

Ack.

>> +                                       break;
> 
> I think this behavior is inconsistent: when a problem is detected at
> the beginning of the unmap range, you return without modifying the
> table, whereas if it's detected at the end, the code proceeds with
> unmapping and leaves the table misconfigured. Could these checks be
> performed before entering the loop?
> 

Agreed, Will move both checks before the loop so a rejected unmap is always a
full no-op, regardless of whether the violation is at the start or end of
the range.

Thanks,
Vijay

>> +                       }
>> +
>>                         if (!iopte_leaf(pte, lvl, iop->fmt)) {
>>                                 __arm_lpae_clear_pte(&ptep[i], &iop->cfg, 1);
>>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit
  2026-08-27  8:24   ` Vijayanand Jitta
@ 2026-08-27  8:38     ` Vijayanand Jitta
  0 siblings, 0 replies; 8+ messages in thread
From: Vijayanand Jitta @ 2026-08-27  8:38 UTC (permalink / raw)
  To: Daniel Mentz
  Cc: Will Deacon, Robin Murphy, Joerg Roedel (AMD), linux-arm-msm,
	linux-arm-kernel, iommu, linux-kernel, Prakash Gupta



On 8/27/2026 1:54 PM, Vijayanand Jitta wrote:
> 
> 
> On 8/15/2026 3:27 AM, Daniel Mentz wrote:
>> On Mon, Aug 3, 2026 at 11:19 PM Vijayanand Jitta
>> <vijayanand.jitta@oss.qualcomm.com> wrote:
>>> +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));
>>> +       pg_size = 1UL << pg_shift;
>>> +       blk_size = pg_size << bits_per_level;
>>> +       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) &&
>>> +           arm_lpae_cont_size_fits(cfg, cont_leaf_size))
>>> +               cont_sizes |= cont_leaf_size;
>>> +
>>> +       if (cfg->pgsize_bitmap & blk_size) {
>>> +               cont_blk_size = arm_lpae_num_cont(blk_size) * blk_size;
>>> +               if (arm_lpae_cont_size_fits(cfg, cont_blk_size))
>>> +                       cont_sizes |= cont_blk_size;
>>> +       }
>>> +
>>> +       /*
>>> +        * l1_blk_size is only set in pgsize_bitmap if level-1 blocks are
>>> +        * supported for this granule (not 16K/64K, per
>>> +        * arm_lpae_restrict_pgsizes()), so no extra gating is needed here.
>>> +        */
>>> +       if (cfg->pgsize_bitmap & l1_blk_size) {
>>> +               cont_l1_blk_size = arm_lpae_num_cont(l1_blk_size) * l1_blk_size;
>>
>> Our AI model is saying that this might overflow cont_l1_blk_size on 32
>> bit platforms i.e. 16 * 1G doesn't fit into a 32 bit type. It says
>> that cont_l1_blk_size will be truncated to 0, and
>> arm_lpae_cont_size_fits() then calls ilog2(0) which is undefined.
>>
> 
> Ack. With arm_lpae_cont_size_fits removed this won't be an issue anymore.
> 
>>> +               if (arm_lpae_cont_size_fits(cfg, cont_l1_blk_size))
>>> +                       cont_sizes |= cont_l1_blk_size;
>>> +       }
>>> +
>>> +       return cont_sizes;
>>> +}
>> [...]
>>> @@ -660,6 +829,8 @@ 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);
>>>         int i = 0, num_entries, max_entries, unmap_idx_start;
>>>
>>>         /* Something went horribly wrong and we ran out of page table */
>>> @@ -674,10 +845,22 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>>                 return 0;
>>>         }
>>>
>>> +       /*
>>> +        * Normalize an exact whole-CONT-group request down to the
>>> +        * equivalent block_size/pgcount, mirroring __arm_lpae_map().
>>> +        */
>>> +       if (!(data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NO_CONT_HINT) &&
>>> +           num_cont > 1 && size == block_size * num_cont) {
>>> +               pgcount *= num_cont;
>>> +               size = block_size;
>>> +       }
>>> +
>>>         /* If the size matches this level, we're in the right place */
>>> -       if (size == ARM_LPAE_BLOCK_SIZE(lvl, data)) {
>>> +       if (size == block_size) {
>>> +               size_t cont_size = num_cont * 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);
>>>
>>>                 /* Find and handle non-leaf entries */
>>
>> This comment is no longer accurate. The handling now extends beyond
>> non-leaf entries.
>>
> 
> Agreed, that comment is stale -- the loop now also validates CONT-group
> alignment on leaf entries (rejecting an unmap that would split a tagged
> group) before falling through to the non-leaf teardown. Will update it to
> something like:
> 
> /* Validate leaf entries and handle non-leaf entries */
> 
> 

You can ignore the above comment, after moving the checks to outside the loop
the earlier comment would stay accurate for the loop.

Thanks,
Vijay
>>>                 for (i = 0; i < num_entries; i++) {
>>> @@ -687,6 +870,40 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>>>                                 break;
>>>                         }
>>>
>>> +                       /*
>>> +                        * A real CONT group must always be invalidated as a
>>> +                        * unit, so reject an unmap that splits one. Check the
>>> +                        * PTE's own CONT bit rather than the caller's size,
>>> +                        * since a legitimate unmap can span multiple prior
>>> +                        * iommu_map() calls and its size alone doesn't say how
>>> +                        * the underlying PTEs were grouped. Only the first and
>>> +                        * last entries can straddle a group boundary; an
>>> +                        * interior CONT-tagged entry's group is necessarily
>>> +                        * fully covered by this unmap, since groups can't
>>> +                        * overlap without also covering everything between
>>> +                        * them.
>>> +                        */
>>> +                       if (pte & ARM_LPAE_PTE_CONT) {
>>> +                               bool ok = true;
>>> +
>>> +                               if (i == 0)
>>> +                                       ok = ok && IS_ALIGNED(iova, cont_size);
>>> +                               if (i == num_entries - 1)
>>> +                                       ok = ok && IS_ALIGNED(iova + (i + 1) * block_size,
>>> +                                                             cont_size);
>>> +
>>> +                               /*
>>> +                                * Stop short of this entry instead of returning
>>> +                                * 0: entries before i may already have had
>>> +                                * non-leaf sub-tables torn down above, so the
>>> +                                * caller needs the real unmapped count, and the
>>> +                                * loop exit below still clears/gathers entries
>>> +                                * [0, i) correctly.
>>> +                                */
>>> +                               if (WARN_ON_ONCE(!ok))
>>
>> Consider aligning with the following WARN_ONCE in the same function:
>>
>> WARN_ONCE(true, "Unmap of a partial large IOPTE is not allowed");
>>
> 
> Ack.
> 
>>> +                                       break;
>>
>> I think this behavior is inconsistent: when a problem is detected at
>> the beginning of the unmap range, you return without modifying the
>> table, whereas if it's detected at the end, the code proceeds with
>> unmapping and leaves the table misconfigured. Could these checks be
>> performed before entering the loop?
>>
> 
> Agreed, Will move both checks before the loop so a rejected unmap is always a
> full no-op, regardless of whether the violation is at the start or end of
> the range.
> 
> Thanks,
> Vijay
> 
>>> +                       }
>>> +
>>>                         if (!iopte_leaf(pte, lvl, iop->fmt)) {
>>>                                 __arm_lpae_clear_pte(&ptep[i], &iop->cfg, 1);
>>>
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-27  8:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  6:14 [PATCH v4] iommu/io-pgtable-arm: Add support for contiguous hint bit Vijayanand Jitta
2026-08-11  5:04 ` Daniel Mentz
2026-08-14  6:12   ` Vijayanand Jitta
2026-08-14 20:45     ` Daniel Mentz
2026-08-27  5:54       ` Vijayanand Jitta
2026-08-14 21:57 ` Daniel Mentz
2026-08-27  8:24   ` Vijayanand Jitta
2026-08-27  8:38     ` Vijayanand Jitta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox