From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f172.google.com (mail-qt1-f172.google.com [209.85.160.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DEDA23B19D0 for ; Fri, 19 Jun 2026 19:40:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=209.85.160.172 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781898058; cv=pass; b=MvudrLU+Sb72qSEhooLBcrz0nIJXfr7oIBZMSFpxcH7nfM5AchpU+Cu0YsIE13yaVfnQCyfersYdnjYoiZSSBs6ax4SNcUGwzwFAK7I1h0v5/48+JDc5roBcW6N9do/aZMwNaHvzTjirxUCcHMHxYtz7xOUY5sHX7qRLMR0VX2A= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781898058; c=relaxed/simple; bh=urugoTsq/yWR4pligPKsz1qk3EaXU6x5HOdKG2W4ADM=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=HvuWM/hrdsY/YMHF7Xor4aHREARPJJCVV3H8W0AHlMC45YP9sJa5Pzcz4zlHEGKe3/87I44/7tXsD3p6aEeyauxU74g9WOnaAxRs0n1+MM5W/utRSYelMub/WCy3wyoJHYp6hdDqOea+HQ1SzeBNI5bgCFnHDbqicAsG9tsn+YI= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=IQK913hW; arc=pass smtp.client-ip=209.85.160.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="IQK913hW" Received: by mail-qt1-f172.google.com with SMTP id d75a77b69052e-51765331535so279231cf.1 for ; Fri, 19 Jun 2026 12:40:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1781898055; cv=none; d=google.com; s=arc-20260327; b=jxagkBrYHZjWhKlY0Ov5y5UE5xzTT0G+2PzgW9USmNTJrYANORZiwmoTdw6cxsWbfd +zvFVq1ndI7l2pCA04BEgh3XYDVXJdNjoM5uo5rWdkVpi1PVC1WxHsZbsO8tOgQM1V/3 BMcFzLZrtA0JaoZv2XBymM0MAhU/IQP07DsqkDcOiTwZTEfrOPHqwMLQO6cbW5P90f/a q5RqfGSBdcnIFsyawhoOiKIjE5IyYyV7tKtqeZKYpC8GecMiHLIUuyYH7ZiGLEVyuV7/ pycISHDRhw1CUJPUa8dqS6bEcYka11fq7ch/6HeL2/1J2ZbJqjIuUK3pWA6LAAf7qkKj NCLg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=dJE6cIYV1OOaFfhZSUjqsaXExfjdxdplzfZm/ifnrlk=; fh=0Gxt1AbVZsV1yZAB+kunpxUTjz/2/wYZYMHGOp5enZY=; b=KWV6m8eIlqO8OAcebqcltIE3H+VhI6ona3si+nQFp9Lg51j4ibjMCRnwMNU/9MsoX5 ZkWpZVpJ9rTMfIhKsLnvmnPn3GHlXYGU+jvKrR1Z6EQSl/7YOUKNytMGu4FENLBqYM70 30N/zcm3Fs/1uOCmLAHjh0bCICIuVnhzXgDJm7f+JiKpEMcaqLWfEVWfTzq6rX8d5kW6 Y481zhZvf54cdMrdrg+w4h9RKg8jCxOXLmGrEIvOv8zBGKKxI+eT0jvNbG8r7U2R2fre zbQNzCVd3M7+3eBkW2MiixwaVXcPmBzi4hJ754KN8tS/s49SWrrZmm6Qw1OSLFJ820dI V+/w==; darn=vger.kernel.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1781898055; x=1782502855; darn=vger.kernel.org; h=content-transfer-encoding:content-type:cc:to:subject:message-id :date:from:in-reply-to:references:mime-version:from:to:cc:subject :date:message-id:reply-to:content-type; bh=dJE6cIYV1OOaFfhZSUjqsaXExfjdxdplzfZm/ifnrlk=; b=IQK913hWydOVIL2ZiDRkI07BgJug9U0W5eNe9fXIuiABFv8+au8WiQr4U/pATw8MI7 MkqspHDB9Ap+JBfR68SyRfWkvYIEODcA/zr+gqm81GoBmK2URveoqodNQfo0Iq8ccBv4 mvyZqBp6W6wrdc+NYd1044LF7qw7SQgWbifltlo8Eg34PCAMYMlVLzt/sdxo+WyDsbeh JSRdWZk8itnOzK4fmrQVpEvJcnhxMl2VaXgzzyRSWgwoT+JuFSq/cvlBm6QxEa1L5g/H mhkQoTm8lWsQuNL7bxP3ct2Ju3tzICAwC+b//MPl2mGvYWGviOMcZqQSOEqxTr7Co+QO nG2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781898055; x=1782502855; h=content-transfer-encoding:content-type:cc:to:subject:message-id :date:from:in-reply-to:references:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=dJE6cIYV1OOaFfhZSUjqsaXExfjdxdplzfZm/ifnrlk=; b=huZ03/wcwzmZehV1l5l4Lpt9Yh+fpxrSuocG+MpVybbsAaykh9SjwrihgFSnCZtxnq aH7rmMCpVwM/BLMZ5IySArmHsr/kRYt3lzeTdtJsGYE1BiwloD82WCrgBZrTP6/oTGeE SGmjugLqTvrWWAp3y+n0icPSfvrIr9O1RIXUoaHG5xLy3WcuVUrr0w0DI/UAWGJyIyRq pRQc2/soLkgy0CJt/OWvxeSsAUyn2gjqKzoOUlbVBv+aK8N2cvAwjFCdn4sk3Qx7fmEq 52OlSTq59VwQuhUHkEevBiqJvdhbZQqm5zuOKcIpX7fwrc1NwAv32IKdcNloqZg6GOzj 4cNQ== X-Forwarded-Encrypted: i=1; AFNElJ+IAn7L159JQWLDdmJ32P3m/+Oprhzrl23wk+2DMdGzG/6mrli7Qf3To72JjDWn80sHacowrSKfQImVrhA=@vger.kernel.org X-Gm-Message-State: AOJu0YyOJU1ZNERuvuvDe1gUlWbTK+14En+Q6v1hKkHoXvb7TgKcTNNj q5THKdu+0z5OkJhIe/dE5apj9cDCYqcRgFcUXy8NW9IldXyYuq0bnApVXNE46/jgKJDfWT+oQFt +Zvfk1jgv9Tw7diKHbUSA1bbbcuz6GKlyrt0DQS7t X-Gm-Gg: AfdE7ckXn7sSpwWbCf9BXsuu/ceX44TjCawHv5lsZ7m0/cjO2OjZquhgsmrtuhgVuta F0f/ug27Ib2Sr3sHniOHMQ3H2/UBpmkuFCelEf2h72EFDaASXwCRqh9qSATee0TUj7yU3J5RVuz hVadvx/Mx0NkJkmvkXtn5ZCN32gaz0ZcfyC/yOCuDZl40jqPz0hVQ6eG72e4NHY3Qw8t72wo8Ey lWrJ69IiGrejC4o0v+zgXNHpX3ymqna0VU50/GYz0oyAwPBZ78Z8kKudvAUazRkQdABpA== X-Received: by 2002:a05:622a:11c5:b0:516:ccc0:ee38 with SMTP id d75a77b69052e-519e891bc2bmr8238871cf.9.1781898053871; Fri, 19 Jun 2026 12:40:53 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20260618-iommu_contig_hint-v1-1-4502a59e6388@oss.qualcomm.com> In-Reply-To: <20260618-iommu_contig_hint-v1-1-4502a59e6388@oss.qualcomm.com> From: Daniel Mentz Date: Fri, 19 Jun 2026 12:40:41 -0700 X-Gm-Features: AVVi8CfGMSn6U3ZYxuzVWyR1AQuFNJS8d4-0Q4Dfsm2iT6SBHKVnQMNx3er3cMg Message-ID: Subject: Re: [PATCH] iommu/io-pgtable-arm: Add support for contiguous hint bit To: Vijayanand Jitta Cc: "Joerg Roedel (AMD)" , Will Deacon , Robin Murphy , linux-arm-msm@vger.kernel.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Prakash Gupta Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Jun 18, 2026 at 2:06=E2=80=AFAM Vijayanand Jitta wrote: > Support is gated behind CONFIG_IOMMU_IO_PGTABLE_CONTIG_HINT, which > provides a compile-time opt-out for hardware affected by SMMU errata > related to the contiguous bit. Have you considered making this a runtime option? Compare this with arm_smmu_device_iidr_probe() where the smmuv3 driver disables certain features based on the identified implementation and the errata affecting that implementation. > On the mapping side, __arm_lpae_map() detects when the requested size > matches a contiguous range at the next level, sets the CONT bit on all > PTEs in the group, then recurses with the base block size and an > adjusted pgcount. I would perform this check at the current level not the previous level. See comments below. > > On the unmapping side, the CONT bit is cleared from all PTEs in the > affected contiguous group before any individual entry is invalidated, > following the Break-Before-Make requirement of the architecture. My understanding is that for unmap operations, the following rule applies: The IOVA range targeted by an unmap operation must exactly match the IOVA range of a previous map operation. Partial unmap operations are not allowed. The iopgtable code previously had a function named arm_lpae_split_blk_unmap() which allowed a block mapping to be split up. However, that function has since been removed, which aligns with prohibiting partial unmaps. The other concern I have is a potential race condition: While one thread clears the contiguous bit, another thread could try to unmap the same descriptor. Consider dropping support for partial unmap and just triggering a WARN_ON() if you detect that a contiguous group is partially unmapped. > +static inline int arm_lpae_cont_pmds(unsigned long size) PMD is not a term that is used in this file. I advise against introducing this term. > +static u32 arm_lpae_find_num_cont(struct arm_lpae_io_pgtable *data, int = lvl) > +{ > + if (lvl =3D=3D ARM_LPAE_MAX_LEVELS - 2) > + return arm_lpae_cont_pmds(ARM_LPAE_BLOCK_SIZE(lvl, data))= ; > + else if (lvl =3D=3D ARM_LPAE_MAX_LEVELS - 1) > + return arm_lpae_cont_ptes(ARM_LPAE_BLOCK_SIZE(lvl, data))= ; Consider supporting the contiguous bit at lookup level 1. > static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned lon= g iova, > phys_addr_t paddr, size_t size, size_t pgcount, > arm_lpae_iopte prot, int lvl, arm_lpae_iopte *p= tep, > @@ -463,6 +583,7 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable = *data, unsigned long iova, > size_t tblsz =3D ARM_LPAE_GRANULE(data); > struct io_pgtable_cfg *cfg =3D &data->iop.cfg; > int ret =3D 0, num_entries, max_entries, map_idx_start; > + u32 num_cont =3D 1; > > /* Find our entry at the current level */ > map_idx_start =3D ARM_LPAE_LVL_IDX(iova, lvl, data); > @@ -505,6 +626,24 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable= *data, unsigned long iova, > return -EEXIST; > } > > + if (arm_lpae_pte_is_contiguous_range(data, size, lvl + 1, &num_co= nt)) { I would recommend performing this check at the actual level not at the previous lookup level i.e. not at the (lvl - 1) level. Imagine the following situation: The granule size is 4KB, the initial lookup level is 2, and size is 32MB. I'm wondering if in that case, it'll just keep recursing until it hits (WARN_ON(lvl >=3D ARM_LPAE_MAX_LEVELS - 1)). > +#ifdef CONFIG_IOMMU_IO_PGTABLE_CONTIG_HINT > +static void arm_lpae_cont_clear(struct arm_lpae_io_pgtable *data, > + unsigned long iova, int lvl, > + arm_lpae_iopte *ptep, size_t num_entries) > +{ > + struct io_pgtable_cfg *cfg =3D &data->iop.cfg; > + u32 num_cont =3D arm_lpae_find_num_cont(data, lvl); > + arm_lpae_iopte *cont_ptep; > + arm_lpae_iopte *cont_ptep_start; > + unsigned long cont_iova; > + int offset, itr; > + > + cont_ptep =3D ptep - ARM_LPAE_LVL_IDX(iova, lvl, data); > + cont_iova =3D round_down(iova, > + ARM_LPAE_BLOCK_SIZE(lvl, data) * num_cont)= ; As a result of this round_down() function, you are accessing a descriptor that describes an IOVA outside the range targeted by the iommu_unmap call. Consequently, you might race against another thread accessing the same descriptor. > + cont_ptep +=3D ARM_LPAE_LVL_IDX(cont_iova, lvl, data); > + cont_ptep_start =3D cont_ptep; > + > + /* > + * iova may not be aligned to the contiguous group boundary; incl= ude > + * any leading entries so round_up() covers all overlapping group= s. > + */ > + offset =3D ARM_LPAE_LVL_IDX(iova, lvl, data) - > + ARM_LPAE_LVL_IDX(cont_iova, lvl, data); > + num_entries =3D round_up(offset + num_entries, num_cont); > + > + for (itr =3D 0; itr < num_entries; itr++) { > + WRITE_ONCE(*cont_ptep, READ_ONCE(*cont_ptep) & ~ARM_LPAE_= PTE_CONT); This read-modify-write operation is not safe due to the potential race described above. > + cont_ptep++; > + } > + > + if (!cfg->coherent_walk) > + __arm_lpae_sync_pte(cont_ptep_start, num_entries, cfg); > +} > +#else > +static void arm_lpae_cont_clear(struct arm_lpae_io_pgtable *data, > + unsigned long iova, int lvl, > + arm_lpae_iopte *ptep, size_t num_entries) > +{ > +} > +#endif > + > static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > struct iommu_iotlb_gather *gather, > unsigned long iova, size_t size, size_t pg= count, > @@ -660,7 +841,7 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgt= able *data, > { > arm_lpae_iopte pte; > struct io_pgtable *iop =3D &data->iop; > - int i =3D 0, num_entries, max_entries, unmap_idx_start; > + int i =3D 0, num_cont =3D 1, num_entries, max_entries, unmap_idx_= start; > > /* Something went horribly wrong and we ran out of page table */ > if (WARN_ON(lvl =3D=3D ARM_LPAE_MAX_LEVELS)) > @@ -675,9 +856,15 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pg= table *data, > } > > /* If the size matches this level, we're in the right place */ > - if (size =3D=3D ARM_LPAE_BLOCK_SIZE(lvl, data)) { > + if (size =3D=3D ARM_LPAE_BLOCK_SIZE(lvl, data) || > + (size =3D=3D arm_lpae_find_num_cont(data, lvl) * > + ARM_LPAE_BLOCK_SIZE(lvl, data))) { > + size_t pte_size; > + > max_entries =3D arm_lpae_max_entries(unmap_idx_start, dat= a); > - num_entries =3D min_t(int, pgcount, max_entries); > + num_cont =3D arm_lpae_check_num_cont(data, size, lvl); > + num_entries =3D min_t(int, num_cont * pgcount, max_entrie= s); > + pte_size =3D size / num_cont; > > /* Find and handle non-leaf entries */ > for (i =3D 0; i < num_entries; i++) { > @@ -687,11 +874,27 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_p= gtable *data, > break; > } > > + /* > + * Break-Before-Make: before invalidating any lea= f > + * entry, clear the CONT bit from every entry in = the > + * contiguous group(s) and flush the TLB, as requ= ired > + * by the architecture. arm_lpae_cont_clear() co= vers > + * the full [iova, iova + num_entries * pte_size)= range > + * via round_up(), so subsequent entries read bac= k > + * CONT=3D0 and skip this block. > + */ > + if (pte & ARM_LPAE_PTE_CONT) { > + arm_lpae_cont_clear(data, iova, lvl, ptep= , num_entries); > + io_pgtable_tlb_flush_walk(iop, iova, > + num_entries * p= te_size, > + ARM_LPAE_GRANUL= E(data)); I believe this is inefficient. Consider the case where we unmap 2MB worth of IOVA space mapped by 512 4KB page descriptors with the contiguous bit set. If I'm not mistaken, you're running CMOs (__arm_lpae_sync_pte) twice for every page descriptor. In addition, io_pgtable_tlb_flush_walk() will submit an extra CMD_SYNC and wait for it's completion. Additionally, you perform rounding in arm_lpae_cont_clear(). However, io_pgtable_tlb_flush_walk() is called on the original, potentially unaligned range. Can this lead to under invalidation? Again, my preference would be to drop support for partial unmaps which would also remove the requirement for calling io_pgtable_tlb_flush_walk() here. > + } > + > if (!iopte_leaf(pte, lvl, iop->fmt)) { > __arm_lpae_clear_pte(&ptep[i], &iop->cfg,= 1); > > /* Also flush any partial walks */ > - io_pgtable_tlb_flush_walk(iop, iova + i *= size, size, > + io_pgtable_tlb_flush_walk(iop, iova + i *= pte_size, pte_size, > ARM_LPAE_GRANUL= E(data)); > __arm_lpae_free_pgtable(data, lvl + 1, io= pte_deref(pte, data)); > }