Re: [PATCH 15/24] iommu/amd: Add helper functions to manage DevID / DomID mapping tables

["guanghuifeng@linux.alibaba.com" <guanghuifeng@linux.alibaba.com>]

在 2026/7/27 21:29, Suravee Suthikulpanit 写道:
> Introduce amd_viommu_init_one() and amd_viommu_uninit_one().
> These functions are called during IOMMUFD vIOMMU initialize and destroy.
> Currently, it manages the IPA mapping for Device ID and Domain ID mapping
> tables.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   drivers/iommu/amd/amd_iommu_types.h |  3 ++
>   drivers/iommu/amd/amd_viommu.h      | 13 ++++++++
>   drivers/iommu/amd/iommu.c           |  1 +
>   drivers/iommu/amd/iommufd.c         |  5 +++
>   drivers/iommu/amd/viommu.c          | 52 +++++++++++++++++++++++++++++
>   5 files changed, 74 insertions(+)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 340929ae776a..03346258e2dc 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -549,6 +549,9 @@ struct amd_iommu_viommu {
>   	 */
>   	struct xarray gdomid_array;
>   
> +	u64 *devid_table;
> +	u64 *domid_table;
> +
>   	/* Offset for mmap() of guest VF MMIO; set after iommufd_viommu_alloc_mmap(). */
>   	unsigned long vfmmio_mmap_offset;
>   };
> diff --git a/drivers/iommu/amd/amd_viommu.h b/drivers/iommu/amd/amd_viommu.h
> index 447692b9101c..8b57717c22a6 100644
> --- a/drivers/iommu/amd/amd_viommu.h
> +++ b/drivers/iommu/amd/amd_viommu.h
> @@ -14,6 +14,10 @@ void __init amd_viommu_uninit(struct amd_iommu *iommu);
>   
>   u64 amd_viommu_get_vfmmio_addr(struct amd_iommu *iommu, u16 gid);
>   
> +int amd_viommu_init_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu);
> +
> +void amd_viommu_uninit_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu);
> +
>   #else
>   
>   static inline int amd_viommu_init(struct amd_iommu *iommu)
> @@ -30,6 +34,15 @@ static inline u64 amd_viommu_get_vfmmio_addr(struct amd_iommu *iommu, u16 gid)
>   	return 0;
>   }
>   
> +static inline int amd_viommu_init_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline void amd_viommu_uninit_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu)
> +{
> +}
> +
>   #endif /* CONFIG_AMD_IOMMU_IOMMUFD */
>   
>   #endif /* AMD_VIOMMU_H */
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 79698eec21dc..6895629cc8a5 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -43,6 +43,7 @@
>   #include <linux/generic_pt/iommu.h>
>   
>   #include "amd_iommu.h"
> +#include "amd_viommu.h"
>   #include "iommufd.h"
>   #include "../irq_remapping.h"
>   #include "../iommu-pages.h"
> diff --git a/drivers/iommu/amd/iommufd.c b/drivers/iommu/amd/iommufd.c
> index 81e0244348a2..bfc4b0ec22a9 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,
>   					out_vfmmio_mmap_offset);
> @@ -118,6 +122,7 @@ static void amd_iommufd_viommu_destroy(struct iommufd_viommu *viommu)
>   	spin_unlock_irqrestore(&pdom->lock, flags);
>   	xa_destroy(&aviommu->gdomid_array);
>   	iommufd_viommu_destroy_mmap(&aviommu->core, aviommu->vfmmio_mmap_offset);
> +	amd_viommu_uninit_one(iommu, aviommu);
>   	amd_iommu_gid_free(iommu, aviommu->gid);
>   }
>   
> diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c
> index 2c1a267826dc..91d0dd3ac912 100644
> --- a/drivers/iommu/amd/viommu.c
> +++ b/drivers/iommu/amd/viommu.c
> @@ -26,6 +26,20 @@
>   #include "amd_viommu.h"
>   #include "../iommu-pages.h"
>   
> +/*
> + * Guest Device ID Mapping Table
> + */
> +#define VIOMMU_MAX_GDEVID	0xFFFF
> +#define VIOMMU_DEVID_MAPPING_BASE	0x1000000000ULL
> +#define VIOMMU_DEVID_MAPPING_ENTRY_SIZE	(1 << 20)
> +
> +/*
> + * Guest Domain ID Mapping Table
> + */
> +#define VIOMMU_MAX_GDOMID	0xFFFF
> +#define VIOMMU_DOMID_MAPPING_BASE	0x2000000000ULL
> +#define VIOMMU_DOMID_MAPPING_ENTRY_SIZE	(1 << 19)
> +
>   LIST_HEAD(viommu_devid_map);
>   
>   static int viommu_init_pci_vsc(struct amd_iommu *iommu)
> @@ -405,3 +419,41 @@ static void __maybe_unused free_private_vm_region(struct amd_iommu *iommu, u64 *
>   
>   	*entry = NULL;
>   }
> +
> +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);
> +}
> +
> +int amd_viommu_init_one(struct amd_iommu *iommu, struct amd_iommu_viommu *viommu)
> +{
> +	int ret;
> +
> +	ret = alloc_private_vm_region(iommu, &viommu->devid_table,
> +				      VIOMMU_DEVID_MAPPING_BASE,
> +				      VIOMMU_DEVID_MAPPING_ENTRY_SIZE,
> +				      viommu->gid);
> +	if (ret)
> +		goto err_out;
> +
> +	ret = alloc_private_vm_region(iommu, &viommu->domid_table,
> +				      VIOMMU_DOMID_MAPPING_BASE,
> +				      VIOMMU_DOMID_MAPPING_ENTRY_SIZE,
> +				      viommu->gid);
> +	if (ret)
> +		goto err_out;
> +
> +	return 0;
> +err_out:
> +	amd_viommu_uninit_one(iommu, viommu);
> +	return -ENOMEM;
> +}

The err_out label overwrites the real error with -ENOMEM.
alloc_private_vm_region() (via viommu_priv_alloc_map()) can also
return -EINVAL when the pdom is not set up, and that information
gets lost on the way to iommufd. Please just "return ret;" here.