[PATCH v5 04/17] iommu/arm-smmu-v3: Move the STE generation for S1 and S2 domains into functions

[Will Deacon]

On Fri, Feb 16, 2024 at 01:12:17PM -0400, Jason Gunthorpe wrote:
> On Tue, Feb 06, 2024 at 11:12:41AM -0400, Jason Gunthorpe wrote:
> > +static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
> > +					struct arm_smmu_master *master,
> > +					struct arm_smmu_domain *smmu_domain)
> > +{
> > +	struct arm_smmu_s2_cfg *s2_cfg = &smmu_domain->s2_cfg;
> > +
> > +	memset(target, 0, sizeof(*target));
> > +	target->data[0] = cpu_to_le64(
> > +		STRTAB_STE_0_V |
> > +		FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S2_TRANS));
> > +
> > +	target->data[1] = cpu_to_le64(
> > +		FIELD_PREP(STRTAB_STE_1_EATS,
> > +			   master->ats_enabled ? STRTAB_STE_1_EATS_TRANS : 0) |
> > +		FIELD_PREP(STRTAB_STE_1_SHCFG,
> > +			   STRTAB_STE_1_SHCFG_NON_SHARABLE));
> 
> Just so we are on the same page.. The above NON_SHARABLE is a mistake
> here since v1.
> 
> It is hard to follow arm_smmu_write_strtab_ent() so we all missed that
> the S2 ends up re-using the qword[1] that was installed by the
> bypass/abort STE that has to be in place prior to installing the S2.

Ah! I thought you were inheriting the existing behaviour, but yeah, it's
a straight-up bug which I think just makes life a little more difficult
than it needs to be. If we can keep SHCFG as "use incoming" in all
configurations, then I do think we can move to a per-qword rather than a
per-field approach, as mentioned in the other part of the thread. I'll
try to make some time next week to play with it.

Will