On 11/17/21 9:35 PM, Jason Gunthorpe wrote:
On Wed, Nov 17, 2021 at 01:22:19PM +0800, Lu Baolu wrote:
Hi Jason,
On 11/16/21 9:46 PM, Jason Gunthorpe wrote:
On Tue, Nov 16, 2021 at 09:57:30AM +0800, Lu Baolu wrote:
Hi Christoph,
On 11/15/21 9:14 PM, Christoph Hellwig wrote:
On Mon, Nov 15, 2021 at 10:05:42AM +0800, Lu Baolu wrote:
+enum iommu_dma_owner {
+ DMA_OWNER_NONE,
+ DMA_OWNER_KERNEL,
+ DMA_OWNER_USER,
+};
+
+ enum iommu_dma_owner dma_owner;
+ refcount_t owner_cnt;
+ struct file *owner_user_file;
I'd just overload the ownership into owner_user_file,
NULL -> no owner
(struct file *)1UL) -> kernel
real pointer -> user
Which could simplify a lot of the code dealing with the owner.
Yeah! Sounds reasonable. I will make this in the next version.
It would be good to figure out how to make iommu_attach_device()
enforce no other driver binding as a kernel user without a file *, as
Robin pointed to, before optimizing this.
This fixes an existing bug where iommu_attach_device() only checks the
group size and is vunerable to a hot plug increasing the group size
after it returns. That check should be replaced by this series's logic
instead.
As my my understanding, the essence of this problem is that only the
user owner of the iommu_group could attach an UNMANAGED domain to it.
If I understand it right, how about introducing a new interface to
allocate a user managed domain and storing the user file pointer in it.
For iommu_attach_device() the semantic is simple non-sharing, so there
is no need for the file * at all, it can just be NULL.
The file * being NULL means the device is only owned by the kernel
driver. Perhaps we can check this pointer in iommu_attach_device() to
avoid using it for user domain attachment.
Does above help here?
No, iommu_attach_device() is kernel only and should not interact with
userspace.
The existing iommu_attach_device() allows only for singleton group. As
we have added group ownership attribute, we can enforce this interface
only for kernel domain usage.
I'm also going to see if I can learn what Tegra is doing with
iommu_attach_group()
Okay! Thank you!
Jason
Best regards,
baolu
