Re: [PATCH v4 07/13] iommu: Add iommu_at[de]tach_device_shared() for multi-device groups

[Lu Baolu]

On 12/22/21 2:46 AM, Jason Gunthorpe wrote:
> > It's worth taking a step back and realising that overall, this is really
> > just a more generalised and finer-grained extension of what 426a273834ea
> > already did for non-group-aware code, so it makes little sense*not*  to
> > integrate it into the existing interfaces.
> This is taking 426a to it's logical conclusion and*removing*  the
> group API from the drivers entirely. This is desirable because drivers
> cannot do anything sane with the group.
>
> The drivers have struct devices, and so we provide APIs that work in
> terms of struct devices to cover both driver use cases today, and do
> so more safely than what is already implemented.
>
> Do not mix up VFIO with the driver interface, these are different
> things. It is better VFIO stay on its own and not complicate the
> driver world.

Per Joerg's previous comments:

https://lore.kernel.org/linux-iommu/20211119150612.jhsvsbzisvux2lga@xxxxxxxxxx/

The commit 426a273834ea came only in order to disallow attaching a
single device within a group to a different iommu_domain. So it's
reasonable to improve the existing iommu_attach/detach_device() to cover
all cases. How about below code? Did I miss anything?

int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
{
        struct iommu_group *group;
        int ret = 0;

        group = iommu_group_get(dev);
        if (!group)
                return -ENODEV;

        mutex_lock(&group->mutex);
        if (group->attach_cnt) {
                if (group->domain != domain) {
                        ret = -EBUSY;
                        goto unlock_out;
                }
        } else {
                ret = __iommu_attach_group(domain, group);
                if (ret)
                        goto unlock_out;
        }

        group->attach_cnt++;
unlock_out:
        mutex_unlock(&group->mutex);
        iommu_group_put(group);

        return ret;
}
EXPORT_SYMBOL_GPL(iommu_attach_device);

void iommu_detach_device_shared(struct iommu_domain *domain, struct 
device *dev)
{
        struct iommu_group *group;

        group = iommu_group_get(dev);
        if (WARN_ON(!group))
                return;

        mutex_lock(&group->mutex);
        if (WARN_ON(!group->attach_cnt || group->domain != domain)
                goto unlock_out;

        if (--group->attach_cnt == 0)
                __iommu_detach_group(domain, group);

unlock_out:
        mutex_unlock(&group->mutex);
        iommu_group_put(group);
}
EXPORT_SYMBOL_GPL(iommu_detach_device);

Best regards,
baolu