> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, March 8, 2023 8:36 AM
>
> @@ -379,52 +388,57 @@ static int
> iommufd_device_auto_get_domain(struct iommufd_device *idev,
>
> if (!iommufd_lock_obj(&hwpt->obj))
> continue;
> - rc = iommufd_device_do_attach(idev, hwpt);
> - iommufd_put_object(&hwpt->obj);
> -
> - /*
> - * -EINVAL means the domain is incompatible with the device.
> - * Other error codes should propagate to userspace as failure.
> - * Success means the domain is attached.
> - */
> - if (rc == -EINVAL)
> - continue;
> + destroy_hwpt = (*do_attach)(idev, hwpt);
> *pt_id = hwpt->obj.id;
only when succeed?
> + iommufd_put_object(&hwpt->obj);
> + if (IS_ERR(destroy_hwpt)) {
> + /*
> + * -EINVAL means the domain is incompatible with
> the
> + * device. Other error codes should propagate to
> + * userspace as failure. Success means the domain is
> + * attached.
> + */
> + if (PTR_ERR(destroy_hwpt) == -EINVAL)
> + continue;
> + goto out_unlock;
> + }
> goto out_unlock;
two goto's can be merged, if you still want to keep pt_id assignment
in original place.
> }
>
> - hwpt = iommufd_hw_pagetable_alloc(idev->ictx, ioas, idev, true);
> + hwpt = iommufd_hw_pagetable_alloc(idev->ictx, ioas, idev,
> + immediate_attach);
> if (IS_ERR(hwpt)) {
> - rc = PTR_ERR(hwpt);
> + destroy_hwpt = ERR_CAST(hwpt);
> goto out_unlock;
> }
> +
> + if (!immediate_attach) {
> + destroy_hwpt = (*do_attach)(idev, hwpt);
> + if (IS_ERR(destroy_hwpt))
> + goto out_abort;
> + } else {
> + destroy_hwpt = NULL;
> + }
> +
Above is a bit confusing.
On one hand we have immediate_attach for drivers which must
complete attach before we can add the domain to iopt. From
this angle it should always be set when calling
iommufd_hw_pagetable_alloc() no matter it's attach or replace.
On the other hand we assume *replace* doesn't work with
driver which requires immediate_attach so it's done outside of
iommufd_hw_pagetable_alloc().
I'm unsure any better way of handling this transition phase, but
at least some comment would be useful in this part.
