Re: [PATCH v5 5/7] iommu/arm-smmu-v3: Populate smmu_domain->invs when attaching masters

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Nov 24, 2025 at 09:43:18PM +0000, Will Deacon wrote:
> > +	switch (smmu_domain->stage) {
> > +	case ARM_SMMU_DOMAIN_SVA:
> > +	case ARM_SMMU_DOMAIN_S1:
> > +		*cur = (struct arm_smmu_inv){
> > +			.smmu = master->smmu,
> > +			.type = INV_TYPE_S1_ASID,
> > +			.id = smmu_domain->cd.asid,
> > +			.size_opcode = e2h ? CMDQ_OP_TLBI_EL2_VA :
> > +					     CMDQ_OP_TLBI_NH_VA,
> > +			.nsize_opcode = e2h ? CMDQ_OP_TLBI_EL2_ASID :
> > +					      CMDQ_OP_TLBI_NH_ASID
> > +		};
> > +		break;
> > +	case ARM_SMMU_DOMAIN_S2:
> > +		*cur = (struct arm_smmu_inv){
> > +			.smmu = master->smmu,
> > +			.type = INV_TYPE_S2_VMID,
> > +			.id = smmu_domain->s2_cfg.vmid,
> > +			.size_opcode = CMDQ_OP_TLBI_S2_IPA,
> > +			.nsize_opcode = CMDQ_OP_TLBI_S12_VMALL,
> > +		};
> > +		break;
> 
> Having a helper to add an invalidation command would make this a little
> more compact and you could also check against the size of the array.

Yeah but it makes all the parameters positional instead of nicely
named..

> > +/* Must be installed before arm_smmu_install_ste_for_dev() */
> > +static void
> > +arm_smmu_install_new_domain_invs(struct arm_smmu_attach_state *state)
> > +{
> > +	struct arm_smmu_inv_state *invst = &state->new_domain_invst;
> > +
> > +	if (!invst->invs_ptr)
> > +		return;
> > +
> > +	rcu_assign_pointer(*invst->invs_ptr, invst->new_invs);
> > +	/*
> > +	 * We are committed to updating the STE. Ensure the invalidation array
> > +	 * is visable to concurrent map/unmap threads, and acquire any racying
> > +	 * IOPTE updates.
> > +	 */
> > +	smp_mb();
> 
> Sorry, but the comment hasn't really helped me here. We're ordering the
> publishing of the invalidation array above before ... what?

"concurrent map/unmap threads" means other threads calling
arm_smmu_iotlb_sync().. I think the matching comment describing this
and introducing the pair'd smp_mb() is in a later patch.

Either the HW observes the latest page table and needs no invalidation
or arm_smmu_iotlb_sync observes the invalidation list and issues
invalidation.

The smb_mb() is intended to make one of the two statements true prior
to storing the STE.

Nicolin maybe this barrier line should be moved to be added in the
patch that add's the pair'd barrier and description?

Jason