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

> [ ... 73 lines skipped ... ]
> +int amd_iommu_gid_alloc(struct amd_iommu *iommu)
> +{
> +	int ret = ida_alloc_range(&iommu->gid_ida, 1, VIOMMU_MAX_GID, GFP_KERNEL);
> +
> +	if (ret < 0)
> +		pr_err("%s: Failed to allocate guest ID (devid=%#x)\n",
> +		       __func__, iommu->devid);

Don't print on user triggerable failure paths

> [ ... 19 lines skipped ... ]
> @@ -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;
> +
> +	ret = amd_iommu_gid_alloc(iommu);
> +	if (ret < 0)
> +		goto err_gid;
> +	aviommu->gid = ret;
> +	pr_debug("%s: gid=%#x", __func__, aviommu->gid);
> +
> +	ret = iommu_copy_struct_to_user(user_data, &data,
> +					IOMMU_VIOMMU_TYPE_AMD,
> +					out_vfmmio_mmap_offset);
> +	if (ret)
> +		goto err_init;

These hunks doing the copy struct are probably in the wrong patch since
nothing touches out_vfmmio_mmap_offset.

Otherwise looks OK

Reviewed-by: Jason Gunthorpe <[email protected]>

-- 
Jason