[PATCH 04/19] iommu/arm-smmu-v3: Make STE programming independent of the callers

[Michael Shavit]

On Thu, Oct 12, 2023 at 8:16 PM Jason Gunthorpe <jgg at nvidia.com> wrote:
>
> On Thu, Oct 12, 2023 at 04:10:50PM +0800, Michael Shavit wrote:
>
> > This sounds pretty complicated....Is this complexity really required
> > here?
>
> It is first needed before 'iommu/arm-smmu-v3: Do not change the STE
> twice during arm_smmu_attach_dev()' which is a couple of patches
> further on.
>
> Then it keeps getting relied on.
>
> I don't think there is any simple answer here, the HW has this complex
> requirement. The current code is also complex - arguably more complex
> because of how leaky/fragile it is. There is even a couple of pages of
> text in the spec describing how to do this, and it doesn't discuss the
> hitless cases!
>
> At the end this is only 32 lines and it replaces both
> arm_smmu_write_ctx_desc() and arm_smmu_write_strtab_ent().
>
> FWIW, I found the most difficult part the used bit calculation, not
> the update algorithm. Difficult because it is hard to read and find in
> the spec when things are INGORED, but it is a "straightforward" job of
> finding INGORED cases and making the used bits 0.

The update algorithm is the part I'm finding much harder to read and
review :) . arm_smmu_write_entry_step in particular is hard to read
through; on top of which there's some subtle dependencies between loop
iterations that weren't obvious to grok:
* Relying on the used_bits to be recomputed after the first iteration
where V=0 was set to 0 so that more bits can now be set.
* The STE having to be synced between iterations to prevent broken STE
reads by the SMMU (there's a comment somewhere else in arm-smmu-v3.c
that would fit nicely here instead). But the caller is responsible for
calling this between iterations for some reason (supposedly to support
CD entries as well in the next series)