linux-kernel - Re: [PATCH v1 2/9] iommu/arm-smmu-v3: Add alloc_id/free_id functions to arm_smmu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aUW78ZV2D5aM75yN@Asurada-Nvidia>
Date: Fri, 19 Dec 2025 12:56:17 -0800
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <will@...nel.org>, <robin.murphy@....com>, <joro@...tes.org>,
	<jpb@...nel.org>, <praan@...gle.com>, <miko.lenczewski@....com>,
	<linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <patches@...ts.linux.dev>
Subject: Re: [PATCH v1 2/9] iommu/arm-smmu-v3: Add alloc_id/free_id functions
 to arm_smmu_invs

On Fri, Dec 19, 2025 at 01:05:51PM -0400, Jason Gunthorpe wrote:
> On Thu, Dec 18, 2025 at 12:26:48PM -0800, Nicolin Chen wrote:
> > @@ -720,6 +723,9 @@ struct arm_smmu_invs {
> >  	rwlock_t rwlock;
> >  	bool has_ats;
> >  	struct rcu_head rcu;
> > +	struct arm_smmu_domain *smmu_domain;
> > +	int (*alloc_id)(struct arm_smmu_inv *inv, void *data);
> > +	void (*free_id)(struct arm_smmu_inv *inv, bool flush);
> >  	struct arm_smmu_inv inv[] __counted_by(max_invs);
> >  };
> 
> I'm not keen on this at all..
> 
> On the allocation side everything should be stored inside the
> invalidation list, we don't need function callbacks, just get the
> information from the list in the first place.
[...]
> @@ -3289,8 +3422,18 @@ static int arm_smmu_attach_prepare_invs(struct arm_smmu_attach_state *state,
>  		invst->old_invs = rcu_dereference_protected(
>  			new_smmu_domain->invs,
>  			lockdep_is_held(&arm_smmu_asid_lock));
> -		build_invs = arm_smmu_master_build_invs(
> -			master, state->ats_enabled, ssid, new_smmu_domain);
> +
> +		/* Re-use or allocate an IOTLB cache tag */
> +		ret = arm_smmu_get_tag(new_smmu_domain, master,
> +				       to_vsmmu(new_domain), &invst->tag,
> +				       false);
> +		if (ret)
> +			return ret;
> +
> +		build_invs = arm_smmu_master_build_invs(master,
> +							state->ats_enabled,
> +							ssid, new_smmu_domain,
> +							&invst->tag);
>  		if (!build_invs)
>  			return -EINVAL;

I had this when I was drafting the series in the beginning, but
later changed to the callback functions :-/

Again, my intention is to avoid iterating the array since merge
and unref functions are already doing that in their first loops.

That being said, if this get-before-build is the preferred way,
let's do that.

Thanks
Nicolin