Re: [PATCH v2 22/26] iommufd: Add hw_queue_init and split queue alloc paths

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

On 5/29/2026 7:14 AM, Nicolin Chen wrote:
> On Thu, May 28, 2026 at 05:17:34AM +0000, Suravee Suthikulpanit wrote:
>> Splitting helpers and dispatching from the ioctl keeps
>> one uAPI while making the contract explicit.
> 
> Why split? Those two new ioctl functions look quite redundant..
> 
> I imagined that the original ioctl function just needed:
> [...]
> -	struct iommufd_access *access;
> +	struct iommufd_access *access = NULL;
> [...]
>   	if (!viommu->ops || !viommu->ops->get_hw_queue_size ||
> -	    !viommu->ops->hw_queue_init_phys) {
> +	    (!viommu->ops->hw_queue_init_phys && !viommu->ops->hw_queue_init)) {
> 		rc = -EOPNOTSUPP;
> 		goto out_put_viommu;
> 	}
> [...]
> -	access = iommufd_hw_queue_alloc_phys(cmd, viommu, &base_pa);
> -	if (IS_ERR(access)) {
> -		rc = PTR_ERR(access);
> -		goto out_put_viommu;
> -	}
> +	if (viommu->ops->hw_queue_init_phys) {
> +		access = iommufd_hw_queue_alloc_phys(cmd, viommu, &base_pa);
> +		if (IS_ERR(access)) {
> +			rc = PTR_ERR(access);
> +			goto out_put_viommu;
> +		}
> +	}
> [...]
> 	rc = viommu->ops->hw_queue_init_phys(hw_queue, cmd->index, base_pa);
> + 	if (viommu->ops->hw_queue_init_phys)
> +		rc = viommu->ops->hw_queue_init_phys(hw_queue, cmd->index,
> +						     base_pa);
> +	else
> +		rc = viommu->ops->hw_queue_init(hw_queue, cmd->index);
> 
> and then it should work?

Good point. I'll clean this up and send out new version.

Thanks,
Suravee