All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Jonathan Corbet <corbet@lwn.net>,
	iommu@lists.linux.dev, "Joerg Roedel (AMD)" <joro@8bytes.org>,
	Jean-Philippe Brucker <jpb@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Shuah Khan <skhan@linuxfoundation.org>,
	David Matlack <dmatlack@google.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	patches@lists.linux.dev, Pranjal Shrivastava <peaan@google.com>,
	Pranjal Shrivastava <praan@google.com>,
	Samiullah Khawaja <skhawaja@google.com>,
	Mostafa Saleh <smostafa@google.com>,
	Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Subject: Re: [PATCH v8 8/9] iommu/arm-smmu-v3: Change how the tlbi describes the invalidation
Date: Sun, 4 Oct 2026 14:00:12 -0300	[thread overview]
Message-ID: <20261004170012.GC4064@nvidia.com> (raw)
In-Reply-To: <asJYBfMe_qf5Nbn-@willie-the-truck>

On Sun, Oct 04, 2026 at 02:43:33PM +0100, Will Deacon wrote:
> > +	/*
> > +	 * Assumes the page table is formed properly and does not trigger the
> > +	 * 16k TTL=1 condition for leaf-only unless DS is enabled.
> > +	 *
> > +	 * ARM level -1 never has a leaf so something has gone wrong. ARM Level
> > +	 * 0 cannot be hinted because ttl=0 means no-hint.
> > +	 */
> 
> nit: This isn't strictly true for the non-range invalidation operations, so
> I think it would be handy to have a big note saying that this function only
> works for range invalidation or even stick "range" in the function name
> somewhere.

I figured "ttl" was enough since ttl is only used as part of the range
invalidation command. I can change it to range ttl
 
> > @@ -2917,12 +2998,17 @@ static void arm_smmu_tlb_inv_walk(unsigned long iova, size_t size,
> >  				  size_t granule, void *cookie)
> >  {
> >  	struct arm_smmu_domain *smmu_domain = cookie;
> > +	u8 tgsz_lg2 = smmu_domain->tgsz_lg2;
> >  	struct arm_smmu_tlbi tlbi = {
> >  		.tgsz_lg2 = smmu_domain->tgsz_lg2,
> > -		.iova = iova,
> > -		.size = size,
> > -		.iopte_size = 1 << smmu_domain->tgsz_lg2,
> > +		.start = iova,
> > +		.last = iova + size - 1,
> >  	};
> > +	u8 table_levels =
> > +		BIT(arm_smmu_pt_lg2sz_to_level(tgsz_lg2, ilog2(size)));
> > +
> > +	tlbi.table_levels_bitmap = table_levels;
> > +	tlbi.leaf_levels_bitmap = table_levels - 1;
> 
> I'm having a tough time understanding this part. Specifically, I'm trying
> to work out what happens if we end up doing a non-leaf invalidation at the
> PGD (1GiB) level with leaves at the PTE (4KiB) level. In that case, don't
> we need to have table_levels describing both PGD and PMD levels so that
> the TTL describes the 4k leaf entries?

The comment you clipped off tried to explain it:

/*
 * Called by io-pgtable-arm.c for each single table level it wants to remove.
 * size is the size of the table level and granule is the tg in bytes. This must
 * clear the walk cache and any leaves within the range.
 */

But, I had AI double check this statement "each single table level"
and it agrees with you and gave the same example. It does look like I
understood it wrong, I thought the segmantion logic in the core code
would save things but it doesn't work quite like that.

This still works right as-is because the leaf_levels_bitmap disables TTL
and forces single to step at 4k, so it isn't a functional bug, but the
comment is wrong and the logic should be more like:

tlbi.table_levels_bitmap = (table_levels | (table_levels - 1)) & ~BIT(0);

Jason

  reply	other threads:[~2026-10-04 17:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-03  1:23 [PATCH v8 0/9] Organize the SMMUv3 invalidation flow so iommupt can use it Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 1/9] iommu/arm-smmu-v3: Handle ARM erratum for CONT under invalidation with SVA Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 2/9] iommu/arm-smmu-v3: Pass the parameters for the invalidation in a struct Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 3/9] iommu/arm-smmu-v3: Move pgsize out of arm_smmu_inv Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 4/9] iommu/arm-smmu-v3: Optimize range invalidation for latency Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 5/9] iommu/arm-smmu-v3: Keep track in arm_smmu_invs if range invalidation is used Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 6/9] iommu/arm-smmu-v3: Precompute the invalidation commands Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 7/9] iommu/arm-smmu-v3: Populate the tlbi at the top of the call chain Jason Gunthorpe
2026-10-03  1:23 ` [PATCH v8 8/9] iommu/arm-smmu-v3: Change how the tlbi describes the invalidation Jason Gunthorpe
2026-10-04 13:43   ` Will Deacon
2026-10-04 17:00     ` Jason Gunthorpe [this message]
2026-10-03  1:23 ` [PATCH v8 9/9] iommu/arm-smmu-v3: Support the DS expansion of range invalidation SCALE Jason Gunthorpe
2026-10-04 16:50 ` [PATCH v8 0/9] Organize the SMMUv3 invalidation flow so iommupt can use it Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261004170012.GC4064@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=dmatlack@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=jean-philippe@linaro.org \
    --cc=joro@8bytes.org \
    --cc=jpb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nicolinc@nvidia.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=patches@lists.linux.dev \
    --cc=peaan@google.com \
    --cc=praan@google.com \
    --cc=rdunlap@infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=skhan@linuxfoundation.org \
    --cc=skhawaja@google.com \
    --cc=smostafa@google.com \
    --cc=vijayanand.jitta@oss.qualcomm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.