> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, March 22, 2023 3:15 AM
>
> /*
> - * FIXME: Hack around missing a device-centric iommu api, only
> attach to
> - * the group once for the first device that is in the group.
> + * Only attach to the group once for the first device that is in the
> + * group. All the other devices will follow this attachment. The user
> + * should attach every device individually to as the per-device
> reserved
"individually to the hwpt"
>
> - mutex_lock(&hwpt->devices_lock);
> + mutex_lock(&idev->igroup->lock);
>
> /*
> * immediate_attach exists only to accommodate iommu drivers that
> cannot
> * directly allocate a domain. These drivers do not finish creating the
> * domain until attach is completed. Thus we must have this call
> * sequence. Once those drivers are fixed this should be removed.
> + *
> + * Note we hold the igroup->lock here which prevents any other
> thread
> + * from observing igroup->hwpt until we finish setting it up.
> */
> if (immediate_attach) {
> rc = iommufd_hw_pagetable_attach(hwpt, idev);
I thought about whether holding igroup->lock is necessary here.
The caller should avoid racing attach/detach/replace on the same device.
Then the only possible race would come from other devices in the group.
But if above attach call succeeds it implies that all other devices must
haven't been attached yet then the only allowed operation is to attach
them to the same ioas as the calling device does (replace cannot happen
w/o attach first). Then it's already protected by ioas->mutex in
iommufd_device_auto_get_domain().
If no oversight then we can directly put the lock in
iommufd_hw_pagetable_attach/detach() which can also simplify a bit on
its callers in device.c.
