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

["Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>]

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