Re: [PATCH v7 1/9] iommu/arm-smmu-v3: Add an ops indirection to the STE code

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Apr 19, 2024 at 09:02:32PM +0000, Mostafa Saleh wrote:
> >  	} else if (used_qword_diff) {
> >  		/*
> >  		 * At least two qwords need their inuse bits to be changed. This
> >  		 * requires a breaking update, zero the V bit, write all qwords
> >  		 * but 0, then set qword 0
> >  		 */
> > -		unused_update.data[0] = entry->data[0] &
> > -					cpu_to_le64(~STRTAB_STE_0_V);
> > -		entry_set(smmu, sid, entry, &unused_update, 0, 1);
> > -		entry_set(smmu, sid, entry, target, 1, num_entry_qwords - 1);
> > -		entry_set(smmu, sid, entry, target, 0, 1);
> > +		unused_update[0] = entry[0] & (~writer->ops->v_bit);
> 
> arm_smmu_write_entry() assumes that v_bit is in entry[0] and that “1” means valid
> (which is true for both STE and CD) so why do we care about it, if we break the
> STE/CD anyway, why not just do:
> 
> 	unused_update[0] = 0;
> 	entry_set(writer, entry, unused_update, 0, 1);
> 	entry_set(writer, entry, target, 1, NUM_ENTRY_QWORDS - 1)
> 	entry_set(writer, entry, target, 0, 1);
> 
> That makes the code simpler by avoiding having the v_bit in
> arm_smmu_entry_writer_ops.

Sure, done

Jason