Re: [PATCH v2 13/26] iommu/amd: Add helper functions to manage DevID / DomID mapping tables

[Weinan Liu <wnliu@google.com>]

On Wed, May 27, 2026 at 10:19 PM Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> wrote:

> diff --git a/drivers/iommu/amd/iommufd.c b/drivers/iommu/amd/iommufd.c
> index 42307ae71b24..efa9e1f49550 100644
> --- a/drivers/iommu/amd/iommufd.c
> +++ b/drivers/iommu/amd/iommufd.c
> @@ -83,6 +83,10 @@ int amd_iommufd_viommu_init(struct iommufd_viommu *viommu, struct iommu_domain *
>         /* Reset vIOMMU MMIOs to initialize the vIOMMU */
>         iommu_reset_vmmio(iommu, aviommu->gid);
>
> +       ret = amd_viommu_init_one(iommu, aviommu);
> +       if (ret)
> +               goto err_init;
> +
>         ret = iommu_copy_struct_to_user(user_data, &data,
>                                         IOMMU_VIOMMU_TYPE_AMD,
>                                         reserved);
> @@ -120,6 +124,7 @@ static void amd_iommufd_viommu_destroy(struct iommufd_viommu *viommu)
>         if (aviommu->vfmmio_mmap_offset)
>                 iommufd_viommu_destroy_mmap(&aviommu->core, aviommu->vfmmio_mmap_offset);
>         amd_iommu_gid_free(iommu, aviommu->gid);
> +       amd_viommu_uninit_one(iommu, aviommu);
>  }
>
The finalization order should be the reverse of the initialization order.

If amd_iommu_gid_free() is called before amd_viommu_uninit_one(), the gid could be reallocated to a new
vIOMMU instance before the cleanup is complete.
Please consider moving amd_viommu_uninit_one() before the GID free call.


> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> index 6dcb02b12a28..3636093732ce 100644
> --- a/drivers/iommu/amd/viommu.c
> +++ b/drivers/iommu/amd/viommu.c
> +
[...]
> +void amd_viommu_uninit_one(struct amd_iommu *iommu, struct amd_iommu_viommu *aviommu)
> +{
> +       pr_debug("%s: gid=%u\n", __func__, aviommu->gid);
> +
> +       free_private_vm_region(iommu, &aviommu->devid_table,
> +                              VIOMMU_DEVID_MAPPING_BASE,
> +                              VIOMMU_DEVID_MAPPING_ENTRY_SIZE,
> +                              aviommu->gid);
> +       free_private_vm_region(iommu, &aviommu->domid_table,
> +                              VIOMMU_DOMID_MAPPING_BASE,
> +                              VIOMMU_DOMID_MAPPING_ENTRY_SIZE,
> +                              aviommu->gid);
> +}
> +