Re: [PATCH v3 16/22] iommu/amd: Introduce IOMMUFD vDevice support for AMD

[Suthikulpanit, Suravee]

On 7/7/2026 9:31 PM, Jason Gunthorpe wrote:
> On Mon, Jun 29, 2026 at 03:35:29PM +0000, Suravee Suthikulpanit wrote:
> > @@ -388,6 +388,11 @@
> >   #define DTE_GPT_LEVEL_SHIFT	54
> >   #define DTE_GPT_LEVEL_MASK	GENMASK_ULL(55, 54)
> >   
> > +/* vIOMMU bit fields */
> > +#define DTE_VIOMMU_EN_SHIFT		15
> > +#define DTE_VIOMMU_GDEVICEID_MASK	GENMASK_ULL(31, 16)
> > +#define DTE_VIOMMU_GUESTID_MASK		GENMASK_ULL(47, 32)
> > +
> >   #define GCR3_VALID		0x01ULL
> >   
> >   /* DTE[128:179] | DTE[184:191] */
> > @@ -895,6 +900,9 @@ struct iommu_dev_data {
> >   	bool defer_attach;
> >   
> >   	struct ratelimit_state rs;        /* Ratelimit IOPF messages */
> > +
> > +	u16 gid;			/* Guest ID */
> > +	u16 gDevId;			/* Guest Device ID */
> >   };
>
> ?
>
> I don't expect this information to ever be stored in a dev_data? why
> is it needed here? You should be able to reach the viommu from any
> places that would need it.
>
> > @@ -227,6 +227,18 @@ static void set_dte_nested(struct amd_iommu *iommu, struct iommu_domain *dom,
> >   
> >   	/* Guest paging mode */
> >   	new->data[2] |= gdte->dte[2] & DTE_GPT_LEVEL_MASK;
> > +
> > +	/* vImuEn */
> > +	new->data[3] |= 1ULL << DTE_VIOMMU_EN_SHIFT;
> > +
> > +	/* GDeviceID */
> > +	new->data[3] |= FIELD_PREP(DTE_VIOMMU_GDEVICEID_MASK,
> > +				   dev_data->gDevId);
> > +
> > +	/* GuestID */
> > +	new->data[3] |= FIELD_PREP(DTE_VIOMMU_GUESTID_MASK,
> > +				   dev_data->gid);
> > +
> >   }
>
> Like here, the iommu_domain should have a path to the viommu if it is
> in fact a viommu based nested domain, and only in that case should the
> DTE gain the gid/gdevid.
>
> Jason

Good point. I will make change to get rid of this in v4.

Thanks,
Suravee