Re: [PATCH v5 23/24] iommu/amd: Assign per-vIOMMU translate device ID

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

On 9/22/2026 2:09 PM, Guixin Liu wrote:
>> @@ -93,31 +98,63 @@ int amd_iommufd_viommu_init(struct iommufd_viommu *viommu, struct iommu_domain *
>>   
>>   	data.out_vfmmio_mmap_offset = aviommu->vfmmio_mmap_offset;
>>   
>> -	/* Reset vIOMMU MMIOs to initialize the vIOMMU */
>> +	/* Reset vIOMMU MMIOs before the trans_devid is visible in the pool. */
>>   	ret = amd_iommu_reset_vmmio(iommu, aviommu->gid);
>>   	if (ret)
>> -		goto err_init;
>> +		goto err_trans_devid;
>> +
>> +	mutex_lock(&aviommu->trans_devid_lock);
>> +	ret = amd_iommu_trans_devid_alloc(iommu->pci_seg, aviommu);
>> +	if (ret < 0) {
>> +		mutex_unlock(&aviommu->trans_devid_lock);
>> +		goto err_trans_devid;
>> +	}
>> +	trans_devid_allocated = true;
>> +	aviommu->trans_devid = ret;
>> +
>> +	ret = amd_iommu_set_translate_dte(viommu);
>> +	if (ret)
>> +		goto err_init_unlock;
>> +	trans_dte_set = true;
>> +	amd_iommu_update_vfctrl_mmio_translate_devid(iommu, aviommu->gid,
>> +						     aviommu->trans_devid);
>> +	mutex_unlock(&aviommu->trans_devid_lock);
>>   
>>   	ret = amd_viommu_init_one(iommu, aviommu);
>>   	if (ret)
>>   		goto err_init;
>> +	uninit_on_err = true;
>>   
>>   	ret = iommu_copy_struct_to_user(user_data, &data,
>>   					IOMMU_VIOMMU_TYPE_AMD,
>>   					out_vfmmio_mmap_offset);
>>   	if (ret)
>> -		goto err_init_one;
>> +		goto err_init;
>>   
>>   	viommu->ops = &amd_viommu_ops;
>>   
>> -	spin_lock_irqsave(&pdom->lock, flags);
>> -	list_add(&aviommu->pdom_list, &pdom->viommu_list);
>> -	spin_unlock_irqrestore(&pdom->lock, flags);
>> -
>>   	return 0;
>> -err_init_one:
>> -	amd_viommu_uninit_one(iommu, aviommu);
>>   err_init:
>> +	mutex_lock(&aviommu->trans_devid_lock);
>> +err_init_unlock:
>> +	if (trans_dte_set) {
>> +		spin_lock_irqsave(&pdom->lock, flags);
>> +		if (!list_empty(&aviommu->pdom_list))
>> +			list_del_init(&aviommu->pdom_list);
>> +		spin_unlock_irqrestore(&pdom->lock, flags);
>> +		amd_iommu_update_vfctrl_mmio_translate_devid(iommu, aviommu->gid, 0);
>> +		amd_iommu_clear_translate_dte(iommu, aviommu->trans_dev_data,
>> +					      aviommu->trans_devid);
>> +		amd_iommu_free_dev_data(iommu, aviommu->trans_dev_data);
>> +		aviommu->trans_dev_data = NULL;
>> +	}
>> +	if (trans_devid_allocated)
>> +		amd_iommu_trans_devid_free(iommu->pci_seg, aviommu->trans_devid,
>> +					   aviommu);
>> +	mutex_unlock(&aviommu->trans_devid_lock);
>> +	if (uninit_on_err)
>> +		amd_viommu_uninit_one(iommu, aviommu);
>> +err_trans_devid:
>>   	iommufd_viommu_destroy_mmap(&aviommu->core, aviommu->vfmmio_mmap_offset);
>>   err_mmap:
>>   	amd_iommu_gid_free(iommu, aviommu->gid);
>> @@ -140,6 +177,17 @@ 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);
>> +
>> +	mutex_lock(&aviommu->trans_devid_lock);
>> +	amd_iommu_update_vfctrl_mmio_translate_devid(iommu, aviommu->gid, 0);
>> +	amd_iommu_clear_translate_dte(iommu, aviommu->trans_dev_data,
>> +				      aviommu->trans_devid);
>> +	amd_iommu_free_dev_data(iommu, aviommu->trans_dev_data);
>> +	aviommu->trans_dev_data = NULL;
>> +	amd_iommu_trans_devid_free(iommu->pci_seg, aviommu->trans_devid,
>> +				   aviommu);
>> +	mutex_unlock(&aviommu->trans_devid_lock);
>> +
>>   	amd_viommu_uninit_one(iommu, aviommu);
>>   	amd_iommu_gid_free(iommu, aviommu->gid);
>>   	mutex_destroy(&aviommu->vfctrl_lock);
> Should destroy trans_devid_lock too.

Correct. I'll update in v6.

Thanks,
Suravee