linux-kernel - Re: [PATCH rc v3 1/4] iommu/arm-smmu-v3: Add ignored bits to fix STE update sequence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251216000952.GA6079@nvidia.com>
Date: Mon, 15 Dec 2025 20:09:52 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Mostafa Saleh <smostafa@...gle.com>
Cc: Nicolin Chen <nicolinc@...dia.com>, will@...nel.org,
	robin.murphy@....com, joro@...tes.org,
	linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, skolothumtho@...dia.com,
	praan@...gle.com, xueshuai@...ux.alibaba.com
Subject: Re: [PATCH rc v3 1/4] iommu/arm-smmu-v3: Add ignored bits to fix STE
 update sequence

On Sun, Dec 14, 2025 at 10:32:35PM +0000, Mostafa Saleh wrote:
> >   * Figure out if we can do a hitless update of entry to become target. Returns a
> >   * bit mask where 1 indicates that qword needs to be set disruptively.
> > @@ -1094,13 +1100,22 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer,
> >  {
> >  	__le64 target_used[NUM_ENTRY_QWORDS] = {};
> >  	__le64 cur_used[NUM_ENTRY_QWORDS] = {};
> > +	__le64 ignored[NUM_ENTRY_QWORDS] = {};
> 
> I think we can avoid extra stack allocation for another STE, if we make
> the function update cur_used directly, but no strong opinion.

It does more than just mask cur_used, it also adjusts ignored:

> > +		/*
> > +		 * Ignored is only used for bits that are used by both entries,
> > +		 * otherwise it is sequenced according to the unused entry.
> > +		 */
> > +		ignored[i] &= target_used[i] & cur_used[i];

Which also explains this:

> I have some mixed feelings about this, having get_used(), then get_ignored()
> with the same bits set seems confusing to me, specially the get_ignored()
> loops back to update cur_used, which is set from get_used()

The same bits are set because of the above - we need to know what the
actual used bits are to decide if we need to rely on the ignored rule
to do the update.
 
> My initial though was just to remove this bit from get_used() + some changes
> to checks setting bits that are not used would be enough, and the semantics
> of get_used() can be something as:
> “Return bits used by the updated translation regime that MUST be observed
> atomically” and in that case we can ignore things as MEV as it doesn’t
> impact the translation.

Aside from the above this would cause problems with the validation
assertions, so it is not a great idea.

> However, this approach makes it a bit explicit which bits are ignored, if we
> keep this logic, I think changing the name of get_ignored() might help, to
> something as "get_allowed_break()" or "get_update_safe()"?

update_safe sounds good to me

Jason