On 9/28/21 10:07 PM, Jason Gunthorpe wrote:
On Tue, Sep 28, 2021 at 09:35:05PM +0800, Lu Baolu wrote:
Another issue is, when putting a device into user-dma mode, all devices
belonging to the same iommu group shouldn't be bound with a kernel-dma
driver. Kevin's prototype checks this by READ_ONCE(dev->driver). This is
not lock safe as discussed below,

https://lore.kernel.org/linux-iommu/20210927130935.gz964...@nvidia.com/

Any guidance on this?

Something like this?


int iommu_set_device_dma_owner(struct device *dev, enum device_dma_owner mode,
                               struct file *user_owner)
{
        struct iommu_group *group = group_from_dev(dev);

        spin_lock(&iommu_group->dma_owner_lock);
        switch (mode) {
                case DMA_OWNER_KERNEL:
                        if (iommu_group->dma_users[DMA_OWNER_USERSPACE])
                                return -EBUSY;
                        break;
                case DMA_OWNER_SHARED:
                        break;
                case DMA_OWNER_USERSPACE:
                        if (iommu_group->dma_users[DMA_OWNER_KERNEL])
                                return -EBUSY;
                        if (iommu_group->dma_owner_file != user_owner) {
                                if (iommu_group->dma_users[DMA_OWNER_USERSPACE])
                                        return -EPERM;
                                get_file(user_owner);
                                iommu_group->dma_owner_file = user_owner;
                        }
                        break;
                default:
                        spin_unlock(&iommu_group->dma_owner_lock);
                        return -EINVAL;
        }
        iommu_group->dma_users[mode]++;
        spin_unlock(&iommu_group->dma_owner_lock);
        return 0;
}

int iommu_release_device_dma_owner(struct device *dev,
                                   enum device_dma_owner mode)
{
        struct iommu_group *group = group_from_dev(dev);

        spin_lock(&iommu_group->dma_owner_lock);
        if (WARN_ON(!iommu_group->dma_users[mode]))
                goto err_unlock;
        if (!iommu_group->dma_users[mode]--) {
                if (mode == DMA_OWNER_USERSPACE) {
                        fput(iommu_group->dma_owner_file);
                        iommu_group->dma_owner_file = NULL;
                }
        }
err_unlock:
        spin_unlock(&iommu_group->dma_owner_lock);
}


Where, the driver core does before probe:

    iommu_set_device_dma_owner(dev, DMA_OWNER_KERNEL, NULL)

pci_stub/etc does in their probe func:

    iommu_set_device_dma_owner(dev, DMA_OWNER_SHARED, NULL)

And vfio/iommfd does when a struct vfio_device FD is attached:

    iommu_set_device_dma_owner(dev, DMA_OWNER_USERSPACE, group_file/iommu_file)

Really good design. It also helps alleviating some pains elsewhere in
the iommu core.

Just a nit comment, we also need DMA_OWNER_NONE which will be set when
the driver core unbinds the driver from the device.


Jason


Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to