Re: [PATCH 05/24] iommu/amd: Allocate Guest IDs for IOMMUFD vIOMMU instances

[Vasant Hegde]

Suravee,

On 7/27/2026 6:58 PM, Suravee Suthikulpanit wrote:
> Hardware vIOMMU uses a 16-bit Guest ID (GID) per guest IOMMU to
> index driver and hardware state. Allocate one GID per IOMMUFD vIOMMU
> from a per-amd_iommu IDA (unique within that IOMMU; a VM behind
> multiple IOMMUs may hold more than one GID).
> 
> Add amd_iommu_gid_alloc() and amd_iommu_gid_free(), store the ID in
> amd_iommu_viommu::gid, and call them from amd_iommufd_viommu_init() and
> destroy after copying struct iommu_viommu_amd to userspace.
> 
> ida_init() for gid_ida is done when vIOMMU MMIO is mapped (next patch).
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
>  drivers/iommu/amd/amd_iommu.h       |  4 ++++
>  drivers/iommu/amd/amd_iommu_types.h |  8 ++++++++
>  drivers/iommu/amd/iommu.c           | 19 +++++++++++++++++
>  drivers/iommu/amd/iommufd.c         | 32 +++++++++++++++++++++++++++++
>  4 files changed, 63 insertions(+)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 17fc0b5b3fa8..9f2a1a8a6d3c 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -228,4 +228,8 @@ amd_iommu_make_clear_dte(struct iommu_dev_data *dev_data, struct dev_table_entry
>  struct iommu_domain *
>  amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
>  			      const struct iommu_user_data *user_data);
> +
> +/* Guest ID for vIOMMU */
> +int amd_iommu_gid_alloc(struct amd_iommu *iommu);
> +void amd_iommu_gid_free(struct amd_iommu *iommu, int gid);
>  #endif /* AMD_IOMMU_H */
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index b5327bf6814b..00f964d5b149 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -21,6 +21,7 @@
>  #include <linux/iommufd.h>
>  #include <linux/irqreturn.h>
>  #include <linux/generic_pt/iommu.h>
> +#include <linux/idr.h>
>  
>  #include <uapi/linux/iommufd.h>
>  
> @@ -413,6 +414,9 @@
>  
>  #define MAX_DOMAIN_ID 65536
>  
> +/* For vIOMMU, the GID is 16-bit. */
> +#define VIOMMU_MAX_GID		0xFFFF
> +
>  /* Timeout stuff */
>  #define LOOP_TIMEOUT		100000
>  #define MMIO_STATUS_TIMEOUT	2000000
> @@ -509,6 +513,7 @@ struct amd_iommu_viommu {
>  	struct iommufd_viommu core;
>  	struct protection_domain *parent; /* nest parent domain for this viommu */
>  	struct list_head pdom_list;	  /* For protection_domain->viommu_list */
> +	u16 gid;			  /* Guest ID for the vIOMMU */
>  
>  	/*
>  	 * Per-vIOMMU guest domain ID to host domain ID mapping.
> @@ -768,6 +773,9 @@ struct amd_iommu {
>  	/* IOPF support */
>  	struct iopf_queue *iopf_queue;
>  	unsigned char iopfq_name[32];
> +
> +	struct ida gid_ida;		 /* guest IDs for this IOMMU */
> +	bool gid_ida_inited;

Redundant variable? May be remove gid_ida_inited.

>  };
>  
>  static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 50f26c8123f3..73fba8be40d1 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -252,6 +252,25 @@ static inline bool pdom_is_sva_capable(struct protection_domain *pdom)
>  	return pdom_is_v2_pgtbl_mode(pdom) || pdom_is_in_pt_mode(pdom);
>  }
>  
> +int amd_iommu_gid_alloc(struct amd_iommu *iommu)
> +{
> +	int ret = ida_alloc_range(&iommu->gid_ida, 1, VIOMMU_MAX_GID, GFP_KERNEL);


We know that top half is reserved for secure vIOMMU and we have to construct
that differently. May be restrict max GID to 0x7FFF ?


> +
> +	if (ret < 0)
> +		pr_err("%s: Failed to allocate guest ID (devid=%#x)\n",
> +		       __func__, iommu->devid);
> +	else
> +		pr_debug("%s: iommu devid=%#x, gid=%u\n", __func__, iommu->devid, ret);
> +
> +	return ret;
> +}
> +
> +void amd_iommu_gid_free(struct amd_iommu *iommu, int gid)
> +{
> +	pr_debug("%s: iommu devid=%#x, gid=%u\n", __func__, iommu->devid, gid);
> +	ida_free(&iommu->gid_ida, gid);
> +}
> +
>  static inline int get_acpihid_device_id(struct device *dev,
>  					struct acpihid_map_entry **entry)
>  {
> diff --git a/drivers/iommu/amd/iommufd.c b/drivers/iommu/amd/iommufd.c
> index eee29c26169a..ec3c14e18c7b 100644
> --- a/drivers/iommu/amd/iommufd.c
> +++ b/drivers/iommu/amd/iommufd.c
> @@ -43,13 +43,37 @@ size_t amd_iommufd_get_viommu_size(struct device *dev, enum iommu_viommu_type vi
>  int amd_iommufd_viommu_init(struct iommufd_viommu *viommu, struct iommu_domain *parent,
>  			    const struct iommu_user_data *user_data)
>  {
> +	int ret;
>  	unsigned long flags;
> +	struct iommu_viommu_amd data = {};
>  	struct protection_domain *pdom = to_pdomain(parent);
>  	struct amd_iommu_viommu *aviommu = container_of(viommu, struct amd_iommu_viommu, core);
> +	struct amd_iommu *iommu = container_of(viommu->iommu_dev, struct amd_iommu, iommu);
>  
>  	xa_init_flags(&aviommu->gdomid_array, XA_FLAGS_ALLOC1);
>  	aviommu->parent = pdom;
>  
> +	if (!user_data)
> +		return -EINVAL;
> +
> +	ret = iommu_copy_struct_from_user(&data, user_data,
> +					  IOMMU_VIOMMU_TYPE_AMD,
> +					  out_vfmmio_mmap_offset);
> +	if (ret)
> +		return ret;

May be move this before "xa_init_flags" ?

> +
> +	ret = amd_iommu_gid_alloc(iommu);

Looks like you missed initializing "iommu->gid_ida" ?

> +	if (ret < 0)
> +		goto err_gid;
> +	aviommu->gid = ret;
> +	pr_debug("%s: gid=%#x", __func__, aviommu->gid);

Redundat as you already have pr_debug inside amd_iommu_gid_alloc()?

-Vasant