[PATCH v4 02/16] iommu/arm-smmu-v3: Consolidate the STE generation for abort/bypass

[Mostafa Saleh]

On Wed, Jan 31, 2024 at 10:47:02AM -0400, Jason Gunthorpe wrote:
> On Wed, Jan 31, 2024 at 02:40:24PM +0000, Mostafa Saleh wrote:
> > > +static void arm_smmu_make_bypass_ste(struct arm_smmu_ste *target)
> > > +{
> > > +	memset(target, 0, sizeof(*target));
> > 
> > I see this can be used with the actual STE. Although this is done at init, but
> > briefly making the STE abort from “arm_smmu_make_bypass_ste”, seems a bit
> > fragile to me, in case we use this in the future in different scenarios, it
> > might break the hitless assumption. But no strong opinion though.
> 
> At init time, when that case happens, the STE table hasn't been
> installed in the HW yet. This is why that specific code path has been
> directly manipulating the STE and does not call the normal update path
> with the sync'ing.
> 
> It is perhaps subtle that is why it is a different flow.
> 
> (this is also why I moved the function order as it was a bit obscure
> to see that indeed all this stuff was sequenced right and we were not
> updating a live STE improperly)

I agree, this is not an issue, but it was just confusing when I first read it
as “arm_smmu_make_bypass_ste” would make the STE transiently unavailable, and
I had to go and check the usage. Maybe we can add a comment to clarify how this
function is expected to be used.

Thanks,
Mostafa