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

[Jason Gunthorpe <jgg@nvidia.com>]

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