Re: [PATCH v3 08/10] iommu/arm-smmu-v3: Allocate INV_TYPE_S2_VMID_VSMMU in arm_vsmmu_init

From: Jonathan Cameron

Date: Thu Mar 12 2026 - 13:11:47 EST


On Mon, 23 Feb 2026 12:27:44 -0800
Nicolin Chen <nicolinc@xxxxxxxxxx> wrote:

> VMID owned by a vSMMU should be allocated in the viommu_init callback for
> - a straightforward lifecycle for a VMID used by a vSMMU
> - HW like tegra241-cmdqv needs to setup VINTF with the VMID
>
> Allocate/free a VMID in arm_vsmmu_init/destroy(). This decouples the VMID
> owned by vSMMU from the VMID living in the S2 parent domain (s2_cfg.vmid).
>
> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
Hi Nicolin,

Not a proper review as I'd need to do a bunch of catch up on how
this stuff all works. So just one query inline.

> ---

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> index a77c60321203c..dc638c38515e4 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> @@ -406,7 +406,20 @@ int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu,
> return ret;
> }
>
> +void arm_vsmmu_destroy(struct iommufd_viommu *viommu)
> +{
> + struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core);
> +
> + guard(mutex)(&arm_smmu_asid_lock);
> + /*
> + * arm_smmu_iotlb_tag_free() must have flushed the IOTLB with the VMID,
> + * but it did not free the VMID to align its lifecycle with the vSMMU.
> + */
> + ida_free(&vsmmu->smmu->vmid_map, vsmmu->vmid);

I'm being slow today, but why do you need the lock?
The ida itself doesn't need it according to the docs.
(it's using the xarray lock underneath)

Likewise of the ida_alloc_range()


> +}
> +
> static const struct iommufd_viommu_ops arm_vsmmu_ops = {
> + .destroy = arm_vsmmu_destroy,
> .alloc_domain_nested = arm_vsmmu_alloc_domain_nested,
> .cache_invalidate = arm_vsmmu_cache_invalidate,
> };
> @@ -456,14 +469,21 @@ int arm_vsmmu_init(struct iommufd_viommu *viommu,
> struct arm_smmu_device *smmu =
> container_of(viommu->iommu_dev, struct arm_smmu_device, iommu);
> struct arm_smmu_domain *s2_parent = to_smmu_domain(parent_domain);
> + int id;
>
> if (s2_parent->smmu != smmu)
> return -EINVAL;
>
> + mutex_lock(&arm_smmu_asid_lock);
> + id = ida_alloc_range(&smmu->vmid_map, 1, (1 << smmu->vmid_bits) - 1,
> + GFP_KERNEL);
> + mutex_unlock(&arm_smmu_asid_lock);