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

[Suthikulpanit, Suravee]

On 8/10/2026 4:21 PM, Vasant Hegde wrote:
> Suravee,
>
> On 7/27/2026 6:58 PM, Suravee Suthikulpanit wrote:
> > .....
> > 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.

Ok

> >   };
> >   
> >   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 ?

Ok

> > .......
> > 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" ?

Ok

> > +
> > +	ret = amd_iommu_gid_alloc(iommu);
>
> Looks like you missed initializing "iommu->gid_ida" ?


It's being initialized at host IOMMU bring-up, from amd_viommu_init() in 
init.c (early_amd_iommu / per-IOMMU setup).

> > +	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

Ok.

I'll update these in V5.

Thanks,
Suravee