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

Jason Gunthorpe
2024-04-22T13:10:11Z

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