* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 9:53 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-16 9:53 UTC (permalink / raw)
To: christian.koenig
Cc: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm, Daniel Vetter
On Mon, Sep 14, 2020 at 08:26:47PM +0200, Christian König wrote:
> Am 14.09.20 um 16:06 schrieb Jason Gunthorpe:
> > On Mon, Sep 14, 2020 at 03:30:47PM +0200, Christian König wrote:
> > > Am 14.09.20 um 15:29 schrieb Christian König:
> > > > Hi Andrew,
> > > >
> > > > I'm the new DMA-buf maintainer and Daniel and others came up with
> > > > patches extending the use of the dma_buf_mmap() function.
> > > >
> > > > Now this function is doing something a bit odd by changing the
> > > > vma->vm_file while installing a VMA in the mmap() system call
> > It doesn't look obviously safe as mmap_region() has an interesting mix
> > of file and vma->file
> >
> > Eg it calls mapping_unmap_writable() using both routes
>
> Thanks for the hint, going to take a look at that code tomorrow.
>
> > What about security? Is it OK that some other random file, maybe in
> > another process, is being linked to this mmap?
>
> Good question, I have no idea. That's why I send out this mail.
>
> > > > The background here is that DMA-buf allows device drivers to
> > > > export buffer which are then imported into another device
> > > > driver. The mmap() handler of the importing device driver then
> > > > find that the pgoff belongs to the exporting device and so
> > > > redirects the mmap() call there.
> > So the pgoff is some virtualized thing?
>
> Yes, absolutely.
Maybe notch more context. Conceptually the buffer objects we use to manage
gpu memory are all stand-alone objects, internally refcounted and
everything. And if you export them as a dma-buf, then they are indeed
stand-alone file descriptors like any other.
But within the driver, we generally need thousands of these, and that
tends to bring fd exhaustion problems with it. That's why all the private
buffer objects which aren't shared with other process or other drivers are
handles only valid for a specific fd instance of the drm chardev (each
open gets their own namespace), and only for ioctls done on that chardev.
And for mmap we assign fake (but unique across all open fd on it) offsets
within the overall chardev. Hence all the pgoff mangling and re-mangling.
Now for unmap_mapping_range we'd like it to find all such fake offset
aliases pointing at the one underlying buffer object:
- mmap on the dma-buf fd, at offset 0
- mmap on the drm chardev where the buffer was originally allocated, at some unique offset
- mmap on the drm chardev where the buffer was imported, again at some
(likely) different unique (for that chardev) offset.
So to make unmap_mapping_range work across the entire delegation change
we'd actually need to change the vma->vma_file and pgoff twice:
- once when forwarding from the importing drm chardev to the dma-buf
- once when forwarding from the dma-buf to the exported drm chardev fake
offset, which (mostly for historical reasons) is considered the
canonical fake offset
We can't really do the delegation in userspace because:
- the importer might not have access to the exporters drm chardev, it only
gets the dma-buf. If we'd give it the underlying drm chardev it could do
stuff like issue rendering commands, breaking the access model.
- the dma-buf fd is only used to establish the sharing, once it's imported
everywhere it generally gets closed. Userspace could re-export it and
then call mmap on that, but feels a bit contrived.
- especially on SoC platforms this has already become uapi. It's not a big
problem because the drivers that really need unmap_mapping_range to work
are the big gpu drivers with discrete vram, where mappings need to be
invalidate when moving buffer objects in/out of vram.
Hence why we'd like to be able to forward aliasing mappings and adjust the
file and pgoff, while hopefully everything keeps working. I thought this
would work, but Christian noticed it doesn't really.
Cheers, Daniel
>
> Christian.
>
> >
> > Jason
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
2020-09-16 9:53 ` Daniel Vetter
@ 2020-09-16 10:14 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 10:14 UTC (permalink / raw)
To: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
[-- Attachment #1.1: Type: text/plain, Size: 5124 bytes --]
Am 16.09.20 um 11:53 schrieb Daniel Vetter:
> On Mon, Sep 14, 2020 at 08:26:47PM +0200, Christian König wrote:
>> Am 14.09.20 um 16:06 schrieb Jason Gunthorpe:
>>> On Mon, Sep 14, 2020 at 03:30:47PM +0200, Christian König wrote:
>>>> Am 14.09.20 um 15:29 schrieb Christian König:
>>>>> Hi Andrew,
>>>>>
>>>>> I'm the new DMA-buf maintainer and Daniel and others came up with
>>>>> patches extending the use of the dma_buf_mmap() function.
>>>>>
>>>>> Now this function is doing something a bit odd by changing the
>>>>> vma->vm_file while installing a VMA in the mmap() system call
>>> It doesn't look obviously safe as mmap_region() has an interesting mix
>>> of file and vma->file
>>>
>>> Eg it calls mapping_unmap_writable() using both routes
>> Thanks for the hint, going to take a look at that code tomorrow.
>>
>>> What about security? Is it OK that some other random file, maybe in
>>> another process, is being linked to this mmap?
>> Good question, I have no idea. That's why I send out this mail.
>>
>>>>> The background here is that DMA-buf allows device drivers to
>>>>> export buffer which are then imported into another device
>>>>> driver. The mmap() handler of the importing device driver then
>>>>> find that the pgoff belongs to the exporting device and so
>>>>> redirects the mmap() call there.
>>> So the pgoff is some virtualized thing?
>> Yes, absolutely.
> Maybe notch more context. Conceptually the buffer objects we use to manage
> gpu memory are all stand-alone objects, internally refcounted and
> everything. And if you export them as a dma-buf, then they are indeed
> stand-alone file descriptors like any other.
>
> But within the driver, we generally need thousands of these, and that
> tends to bring fd exhaustion problems with it. That's why all the private
> buffer objects which aren't shared with other process or other drivers are
> handles only valid for a specific fd instance of the drm chardev (each
> open gets their own namespace), and only for ioctls done on that chardev.
> And for mmap we assign fake (but unique across all open fd on it) offsets
> within the overall chardev. Hence all the pgoff mangling and re-mangling.
>
> Now for unmap_mapping_range we'd like it to find all such fake offset
> aliases pointing at the one underlying buffer object:
> - mmap on the dma-buf fd, at offset 0
> - mmap on the drm chardev where the buffer was originally allocated, at some unique offset
> - mmap on the drm chardev where the buffer was imported, again at some
> (likely) different unique (for that chardev) offset.
>
> So to make unmap_mapping_range work across the entire delegation change
> we'd actually need to change the vma->vma_file and pgoff twice:
> - once when forwarding from the importing drm chardev to the dma-buf
> - once when forwarding from the dma-buf to the exported drm chardev fake
> offset, which (mostly for historical reasons) is considered the
> canonical fake offset
>
> We can't really do the delegation in userspace because:
> - the importer might not have access to the exporters drm chardev, it only
> gets the dma-buf. If we'd give it the underlying drm chardev it could do
> stuff like issue rendering commands, breaking the access model.
> - the dma-buf fd is only used to establish the sharing, once it's imported
> everywhere it generally gets closed. Userspace could re-export it and
> then call mmap on that, but feels a bit contrived.
> - especially on SoC platforms this has already become uapi. It's not a big
> problem because the drivers that really need unmap_mapping_range to work
> are the big gpu drivers with discrete vram, where mappings need to be
> invalidate when moving buffer objects in/out of vram.
>
> Hence why we'd like to be able to forward aliasing mappings and adjust the
> file and pgoff, while hopefully everything keeps working. I thought this
> would work, but Christian noticed it doesn't really.
Well to be clear I'm still not sure if that works or not :)
But Jason pointed me to the right piece of code. See this comment in in
mmap_region():
> /* ->mmap() can change vma->vm_file, but must guarantee that
> * vma_link() below can deny write-access if VM_DENYWRITE is set
> * and map writably if VM_SHARED is set. This usually means the
> * new file must not have been exposed to user-space, yet.
> */
> vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>->vm_file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vm_file> = get_file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/get_file>(file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>);
> error = call_mmap
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/call_mmap>(file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>, vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>);
So changing vma->vm_file is allowed at least under certain circumstances.
Only the "file must not have been exposed to user-space, yet" part still
needs double checking. Currently working on that.
Regards,
Christian.
>
> Cheers, Daniel
>
>
>> Christian.
>>
>>> Jason
[-- Attachment #1.2: Type: text/html, Size: 7530 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 10:14 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 10:14 UTC (permalink / raw)
To: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
[-- Attachment #1: Type: text/plain, Size: 5230 bytes --]
Am 16.09.20 um 11:53 schrieb Daniel Vetter:
> On Mon, Sep 14, 2020 at 08:26:47PM +0200, Christian König wrote:
>> Am 14.09.20 um 16:06 schrieb Jason Gunthorpe:
>>> On Mon, Sep 14, 2020 at 03:30:47PM +0200, Christian König wrote:
>>>> Am 14.09.20 um 15:29 schrieb Christian König:
>>>>> Hi Andrew,
>>>>>
>>>>> I'm the new DMA-buf maintainer and Daniel and others came up with
>>>>> patches extending the use of the dma_buf_mmap() function.
>>>>>
>>>>> Now this function is doing something a bit odd by changing the
>>>>> vma->vm_file while installing a VMA in the mmap() system call
>>> It doesn't look obviously safe as mmap_region() has an interesting mix
>>> of file and vma->file
>>>
>>> Eg it calls mapping_unmap_writable() using both routes
>> Thanks for the hint, going to take a look at that code tomorrow.
>>
>>> What about security? Is it OK that some other random file, maybe in
>>> another process, is being linked to this mmap?
>> Good question, I have no idea. That's why I send out this mail.
>>
>>>>> The background here is that DMA-buf allows device drivers to
>>>>> export buffer which are then imported into another device
>>>>> driver. The mmap() handler of the importing device driver then
>>>>> find that the pgoff belongs to the exporting device and so
>>>>> redirects the mmap() call there.
>>> So the pgoff is some virtualized thing?
>> Yes, absolutely.
> Maybe notch more context. Conceptually the buffer objects we use to manage
> gpu memory are all stand-alone objects, internally refcounted and
> everything. And if you export them as a dma-buf, then they are indeed
> stand-alone file descriptors like any other.
>
> But within the driver, we generally need thousands of these, and that
> tends to bring fd exhaustion problems with it. That's why all the private
> buffer objects which aren't shared with other process or other drivers are
> handles only valid for a specific fd instance of the drm chardev (each
> open gets their own namespace), and only for ioctls done on that chardev.
> And for mmap we assign fake (but unique across all open fd on it) offsets
> within the overall chardev. Hence all the pgoff mangling and re-mangling.
>
> Now for unmap_mapping_range we'd like it to find all such fake offset
> aliases pointing at the one underlying buffer object:
> - mmap on the dma-buf fd, at offset 0
> - mmap on the drm chardev where the buffer was originally allocated, at some unique offset
> - mmap on the drm chardev where the buffer was imported, again at some
> (likely) different unique (for that chardev) offset.
>
> So to make unmap_mapping_range work across the entire delegation change
> we'd actually need to change the vma->vma_file and pgoff twice:
> - once when forwarding from the importing drm chardev to the dma-buf
> - once when forwarding from the dma-buf to the exported drm chardev fake
> offset, which (mostly for historical reasons) is considered the
> canonical fake offset
>
> We can't really do the delegation in userspace because:
> - the importer might not have access to the exporters drm chardev, it only
> gets the dma-buf. If we'd give it the underlying drm chardev it could do
> stuff like issue rendering commands, breaking the access model.
> - the dma-buf fd is only used to establish the sharing, once it's imported
> everywhere it generally gets closed. Userspace could re-export it and
> then call mmap on that, but feels a bit contrived.
> - especially on SoC platforms this has already become uapi. It's not a big
> problem because the drivers that really need unmap_mapping_range to work
> are the big gpu drivers with discrete vram, where mappings need to be
> invalidate when moving buffer objects in/out of vram.
>
> Hence why we'd like to be able to forward aliasing mappings and adjust the
> file and pgoff, while hopefully everything keeps working. I thought this
> would work, but Christian noticed it doesn't really.
Well to be clear I'm still not sure if that works or not :)
But Jason pointed me to the right piece of code. See this comment in in
mmap_region():
> /* ->mmap() can change vma->vm_file, but must guarantee that
> * vma_link() below can deny write-access if VM_DENYWRITE is set
> * and map writably if VM_SHARED is set. This usually means the
> * new file must not have been exposed to user-space, yet.
> */
> vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>->vm_file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vm_file> = get_file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/get_file>(file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>);
> error = call_mmap
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/call_mmap>(file
> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>, vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>);
So changing vma->vm_file is allowed at least under certain circumstances.
Only the "file must not have been exposed to user-space, yet" part still
needs double checking. Currently working on that.
Regards,
Christian.
>
> Cheers, Daniel
>
>
>> Christian.
>>
>>> Jason
[-- Attachment #2: Type: text/html, Size: 7692 bytes --]
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
2020-09-16 10:14 ` Christian König
@ 2020-09-16 11:45 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 11:45 UTC (permalink / raw)
To: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
[-- Attachment #1.1: Type: text/plain, Size: 1444 bytes --]
[SNIP]
> But Jason pointed me to the right piece of code. See this comment in
> in mmap_region():
>
>> /* ->mmap() can change vma->vm_file, but must guarantee that
>> * vma_link() below can deny write-access if VM_DENYWRITE is set
>> * and map writably if VM_SHARED is set. This usually means the
>> * new file must not have been exposed to user-space, yet.
>> */
>> vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>->vm_file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vm_file> = get_file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/get_file>(file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>);
>> error = call_mmap
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/call_mmap>(file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>, vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>);
>
> So changing vma->vm_file is allowed at least under certain circumstances.
>
> Only the "file must not have been exposed to user-space, yet" part
> still needs double checking. Currently working on that.
Ok, I think we can guarantee for all DMA-bufs what is required here.
While searching the code I've found that at least vgem_prime_mmap() and
i915_gem_dmabuf_mmap() are doing the same thing of modifying vma->vm_file.
So I'm leaning towards that this works as expected and we should just
document this properly.
Daniel and Jason what do you think?
Christian.
[-- Attachment #1.2: Type: text/html, Size: 2620 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 11:45 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 11:45 UTC (permalink / raw)
To: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]
[SNIP]
> But Jason pointed me to the right piece of code. See this comment in
> in mmap_region():
>
>> /* ->mmap() can change vma->vm_file, but must guarantee that
>> * vma_link() below can deny write-access if VM_DENYWRITE is set
>> * and map writably if VM_SHARED is set. This usually means the
>> * new file must not have been exposed to user-space, yet.
>> */
>> vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>->vm_file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vm_file> = get_file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/get_file>(file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>);
>> error = call_mmap
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/call_mmap>(file
>> <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/file>, vma <https://elixir.bootlin.com/linux/v5.9-rc5/C/ident/vma>);
>
> So changing vma->vm_file is allowed at least under certain circumstances.
>
> Only the "file must not have been exposed to user-space, yet" part
> still needs double checking. Currently working on that.
Ok, I think we can guarantee for all DMA-bufs what is required here.
While searching the code I've found that at least vgem_prime_mmap() and
i915_gem_dmabuf_mmap() are doing the same thing of modifying vma->vm_file.
So I'm leaning towards that this works as expected and we should just
document this properly.
Daniel and Jason what do you think?
Christian.
[-- Attachment #2: Type: text/html, Size: 2620 bytes --]
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
2020-09-16 11:45 ` Christian König
@ 2020-09-16 12:41 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-16 12:41 UTC (permalink / raw)
To: Christian König
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Wed, Sep 16, 2020 at 1:45 PM Christian König
<christian.koenig@amd.com> wrote:
>
> [SNIP]
>
> But Jason pointed me to the right piece of code. See this comment in in mmap_region():
>
> /* ->mmap() can change vma->vm_file, but must guarantee that
> * vma_link() below can deny write-access if VM_DENYWRITE is set
> * and map writably if VM_SHARED is set. This usually means the
> * new file must not have been exposed to user-space, yet.
> */
> vma->vm_file = get_file(file);
> error = call_mmap(file, vma);
>
>
> So changing vma->vm_file is allowed at least under certain circumstances.
>
> Only the "file must not have been exposed to user-space, yet" part still needs double checking. Currently working on that.
>
>
> Ok, I think we can guarantee for all DMA-bufs what is required here.
>
> While searching the code I've found that at least vgem_prime_mmap() and i915_gem_dmabuf_mmap() are doing the same thing of modifying vma->vm_file.
>
> So I'm leaning towards that this works as expected and we should just document this properly.
>
> Daniel and Jason what do you think?
Well I can claim I started this, so I started out with naively
assuming that it Just Works :-)
I think we already have vgem/i915 prime testcases in igt which try to
excercise this mmap forwarding, including provoking pte shoot-downs.
So I think best would be if you could also add a variant for amdgpu,
to make sure this really works and keeps working.
Plus ofc the documentation patch so that core mm folks can officially
ack this as legit.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 12:41 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-16 12:41 UTC (permalink / raw)
To: Christian König
Cc: Jason Gunthorpe, Andrew Morton, Sumit Semwal,
open list:DMA BUFFER SHARING FRAMEWORK, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK,
Linux Kernel Mailing List, Linux MM
On Wed, Sep 16, 2020 at 1:45 PM Christian König
<christian.koenig@amd.com> wrote:
>
> [SNIP]
>
> But Jason pointed me to the right piece of code. See this comment in in mmap_region():
>
> /* ->mmap() can change vma->vm_file, but must guarantee that
> * vma_link() below can deny write-access if VM_DENYWRITE is set
> * and map writably if VM_SHARED is set. This usually means the
> * new file must not have been exposed to user-space, yet.
> */
> vma->vm_file = get_file(file);
> error = call_mmap(file, vma);
>
>
> So changing vma->vm_file is allowed at least under certain circumstances.
>
> Only the "file must not have been exposed to user-space, yet" part still needs double checking. Currently working on that.
>
>
> Ok, I think we can guarantee for all DMA-bufs what is required here.
>
> While searching the code I've found that at least vgem_prime_mmap() and i915_gem_dmabuf_mmap() are doing the same thing of modifying vma->vm_file.
>
> So I'm leaning towards that this works as expected and we should just document this properly.
>
> Daniel and Jason what do you think?
Well I can claim I started this, so I started out with naively
assuming that it Just Works :-)
I think we already have vgem/i915 prime testcases in igt which try to
excercise this mmap forwarding, including provoking pte shoot-downs.
So I think best would be if you could also add a variant for amdgpu,
to make sure this really works and keeps working.
Plus ofc the documentation patch so that core mm folks can officially
ack this as legit.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
2020-09-16 9:53 ` Daniel Vetter
@ 2020-09-16 14:07 ` Jason Gunthorpe
-1 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-16 14:07 UTC (permalink / raw)
To: christian.koenig, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> But within the driver, we generally need thousands of these, and that
> tends to bring fd exhaustion problems with it. That's why all the private
> buffer objects which aren't shared with other process or other drivers are
> handles only valid for a specific fd instance of the drm chardev (each
> open gets their own namespace), and only for ioctls done on that chardev.
> And for mmap we assign fake (but unique across all open fd on it) offsets
> within the overall chardev. Hence all the pgoff mangling and re-mangling.
Are they still unique struct files? Just without a fdno?
> Hence why we'd like to be able to forward aliasing mappings and adjust the
> file and pgoff, while hopefully everything keeps working. I thought this
> would work, but Christian noticed it doesn't really.
It seems reasonable to me that the dma buf should be the owner of the
VMA, otherwise like you say, there is a big mess attaching the custom
vma ops and what not to the proper dma buf.
I don't see anything obviously against this in mmap_region() - why did
Chritian notice it doesn't really work?
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 14:07 ` Jason Gunthorpe
0 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-16 14:07 UTC (permalink / raw)
To: christian.koenig, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> But within the driver, we generally need thousands of these, and that
> tends to bring fd exhaustion problems with it. That's why all the private
> buffer objects which aren't shared with other process or other drivers are
> handles only valid for a specific fd instance of the drm chardev (each
> open gets their own namespace), and only for ioctls done on that chardev.
> And for mmap we assign fake (but unique across all open fd on it) offsets
> within the overall chardev. Hence all the pgoff mangling and re-mangling.
Are they still unique struct files? Just without a fdno?
> Hence why we'd like to be able to forward aliasing mappings and adjust the
> file and pgoff, while hopefully everything keeps working. I thought this
> would work, but Christian noticed it doesn't really.
It seems reasonable to me that the dma buf should be the owner of the
VMA, otherwise like you say, there is a big mess attaching the custom
vma ops and what not to the proper dma buf.
I don't see anything obviously against this in mmap_region() - why did
Chritian notice it doesn't really work?
Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
2020-09-16 14:07 ` Jason Gunthorpe
@ 2020-09-16 14:14 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 14:14 UTC (permalink / raw)
To: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
>
>> But within the driver, we generally need thousands of these, and that
>> tends to bring fd exhaustion problems with it. That's why all the private
>> buffer objects which aren't shared with other process or other drivers are
>> handles only valid for a specific fd instance of the drm chardev (each
>> open gets their own namespace), and only for ioctls done on that chardev.
>> And for mmap we assign fake (but unique across all open fd on it) offsets
>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> Are they still unique struct files? Just without a fdno?
Yes, exactly.
>> Hence why we'd like to be able to forward aliasing mappings and adjust the
>> file and pgoff, while hopefully everything keeps working. I thought this
>> would work, but Christian noticed it doesn't really.
> It seems reasonable to me that the dma buf should be the owner of the
> VMA, otherwise like you say, there is a big mess attaching the custom
> vma ops and what not to the proper dma buf.
>
> I don't see anything obviously against this in mmap_region() - why did
> Chritian notice it doesn't really work?
To clarify I think this might work.
I just had the same "Is that legal?", "What about security?", etc..
questions you raised as well.
It seems like a source of trouble so I thought better ask somebody more
familiar with that.
Christian.
>
> Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 14:14 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 14:14 UTC (permalink / raw)
To: Jason Gunthorpe, akpm, sumit.semwal, linux-media, dri-devel,
linaro-mm-sig, linux-kernel, linux-mm
Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
>
>> But within the driver, we generally need thousands of these, and that
>> tends to bring fd exhaustion problems with it. That's why all the private
>> buffer objects which aren't shared with other process or other drivers are
>> handles only valid for a specific fd instance of the drm chardev (each
>> open gets their own namespace), and only for ioctls done on that chardev.
>> And for mmap we assign fake (but unique across all open fd on it) offsets
>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> Are they still unique struct files? Just without a fdno?
Yes, exactly.
>> Hence why we'd like to be able to forward aliasing mappings and adjust the
>> file and pgoff, while hopefully everything keeps working. I thought this
>> would work, but Christian noticed it doesn't really.
> It seems reasonable to me that the dma buf should be the owner of the
> VMA, otherwise like you say, there is a big mess attaching the custom
> vma ops and what not to the proper dma buf.
>
> I don't see anything obviously against this in mmap_region() - why did
> Chritian notice it doesn't really work?
To clarify I think this might work.
I just had the same "Is that legal?", "What about security?", etc..
questions you raised as well.
It seems like a source of trouble so I thought better ask somebody more
familiar with that.
Christian.
>
> Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
2020-09-16 14:14 ` Christian König
@ 2020-09-16 15:24 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-16 15:24 UTC (permalink / raw)
To: Christian König
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Wed, Sep 16, 2020 at 4:14 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> > On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> >
> >> But within the driver, we generally need thousands of these, and that
> >> tends to bring fd exhaustion problems with it. That's why all the private
> >> buffer objects which aren't shared with other process or other drivers are
> >> handles only valid for a specific fd instance of the drm chardev (each
> >> open gets their own namespace), and only for ioctls done on that chardev.
> >> And for mmap we assign fake (but unique across all open fd on it) offsets
> >> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> > Are they still unique struct files? Just without a fdno?
>
> Yes, exactly.
Not entirely, since dma-buf happened after drm chardev, so for that
historical reason the underlying struct file is shared, since it's the
drm chardev. But since that's per-device we don't have a problem in
practice with different vm_ops, since those are also per-device. But
yeah we could fish out some entirely hidden per-object struct file if
that's required for some mm internal reasons.
-Daniel
> >> Hence why we'd like to be able to forward aliasing mappings and adjust the
> >> file and pgoff, while hopefully everything keeps working. I thought this
> >> would work, but Christian noticed it doesn't really.
> > It seems reasonable to me that the dma buf should be the owner of the
> > VMA, otherwise like you say, there is a big mess attaching the custom
> > vma ops and what not to the proper dma buf.
> >
> > I don't see anything obviously against this in mmap_region() - why did
> > Chritian notice it doesn't really work?
>
> To clarify I think this might work.
>
> I just had the same "Is that legal?", "What about security?", etc..
> questions you raised as well.
>
> It seems like a source of trouble so I thought better ask somebody more
> familiar with that.
>
> Christian.
>
> >
> > Jason
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 15:24 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-16 15:24 UTC (permalink / raw)
To: Christian König
Cc: Jason Gunthorpe, Andrew Morton, Sumit Semwal,
open list:DMA BUFFER SHARING FRAMEWORK, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK,
Linux Kernel Mailing List, Linux MM
On Wed, Sep 16, 2020 at 4:14 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> > On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> >
> >> But within the driver, we generally need thousands of these, and that
> >> tends to bring fd exhaustion problems with it. That's why all the private
> >> buffer objects which aren't shared with other process or other drivers are
> >> handles only valid for a specific fd instance of the drm chardev (each
> >> open gets their own namespace), and only for ioctls done on that chardev.
> >> And for mmap we assign fake (but unique across all open fd on it) offsets
> >> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> > Are they still unique struct files? Just without a fdno?
>
> Yes, exactly.
Not entirely, since dma-buf happened after drm chardev, so for that
historical reason the underlying struct file is shared, since it's the
drm chardev. But since that's per-device we don't have a problem in
practice with different vm_ops, since those are also per-device. But
yeah we could fish out some entirely hidden per-object struct file if
that's required for some mm internal reasons.
-Daniel
> >> Hence why we'd like to be able to forward aliasing mappings and adjust the
> >> file and pgoff, while hopefully everything keeps working. I thought this
> >> would work, but Christian noticed it doesn't really.
> > It seems reasonable to me that the dma buf should be the owner of the
> > VMA, otherwise like you say, there is a big mess attaching the custom
> > vma ops and what not to the proper dma buf.
> >
> > I don't see anything obviously against this in mmap_region() - why did
> > Chritian notice it doesn't really work?
>
> To clarify I think this might work.
>
> I just had the same "Is that legal?", "What about security?", etc..
> questions you raised as well.
>
> It seems like a source of trouble so I thought better ask somebody more
> familiar with that.
>
> Christian.
>
> >
> > Jason
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-16 15:24 ` Daniel Vetter
@ 2020-09-16 15:31 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 15:31 UTC (permalink / raw)
To: Daniel Vetter, Christian König
Cc: Jason Gunthorpe, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 16.09.20 um 17:24 schrieb Daniel Vetter:
> On Wed, Sep 16, 2020 at 4:14 PM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
>>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
>>>
>>>> But within the driver, we generally need thousands of these, and that
>>>> tends to bring fd exhaustion problems with it. That's why all the private
>>>> buffer objects which aren't shared with other process or other drivers are
>>>> handles only valid for a specific fd instance of the drm chardev (each
>>>> open gets their own namespace), and only for ioctls done on that chardev.
>>>> And for mmap we assign fake (but unique across all open fd on it) offsets
>>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
>>> Are they still unique struct files? Just without a fdno?
>> Yes, exactly.
> Not entirely, since dma-buf happened after drm chardev, so for that
> historical reason the underlying struct file is shared, since it's the
> drm chardev. But since that's per-device we don't have a problem in
> practice with different vm_ops, since those are also per-device. But
> yeah we could fish out some entirely hidden per-object struct file if
> that's required for some mm internal reasons.
Hui? Ok that is just the handling in i915, isn't it?
As far as I know we create an unique struct file for each DMA-buf.
Regards,
Christian.
> -Daniel
>
>>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
>>>> file and pgoff, while hopefully everything keeps working. I thought this
>>>> would work, but Christian noticed it doesn't really.
>>> It seems reasonable to me that the dma buf should be the owner of the
>>> VMA, otherwise like you say, there is a big mess attaching the custom
>>> vma ops and what not to the proper dma buf.
>>>
>>> I don't see anything obviously against this in mmap_region() - why did
>>> Chritian notice it doesn't really work?
>> To clarify I think this might work.
>>
>> I just had the same "Is that legal?", "What about security?", etc..
>> questions you raised as well.
>>
>> It seems like a source of trouble so I thought better ask somebody more
>> familiar with that.
>>
>> Christian.
>>
>>> Jason
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-16 15:31 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-16 15:31 UTC (permalink / raw)
To: Daniel Vetter, Christian König
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 16.09.20 um 17:24 schrieb Daniel Vetter:
> On Wed, Sep 16, 2020 at 4:14 PM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
>>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
>>>
>>>> But within the driver, we generally need thousands of these, and that
>>>> tends to bring fd exhaustion problems with it. That's why all the private
>>>> buffer objects which aren't shared with other process or other drivers are
>>>> handles only valid for a specific fd instance of the drm chardev (each
>>>> open gets their own namespace), and only for ioctls done on that chardev.
>>>> And for mmap we assign fake (but unique across all open fd on it) offsets
>>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
>>> Are they still unique struct files? Just without a fdno?
>> Yes, exactly.
> Not entirely, since dma-buf happened after drm chardev, so for that
> historical reason the underlying struct file is shared, since it's the
> drm chardev. But since that's per-device we don't have a problem in
> practice with different vm_ops, since those are also per-device. But
> yeah we could fish out some entirely hidden per-object struct file if
> that's required for some mm internal reasons.
Hui? Ok that is just the handling in i915, isn't it?
As far as I know we create an unique struct file for each DMA-buf.
Regards,
Christian.
> -Daniel
>
>>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
>>>> file and pgoff, while hopefully everything keeps working. I thought this
>>>> would work, but Christian noticed it doesn't really.
>>> It seems reasonable to me that the dma buf should be the owner of the
>>> VMA, otherwise like you say, there is a big mess attaching the custom
>>> vma ops and what not to the proper dma buf.
>>>
>>> I don't see anything obviously against this in mmap_region() - why did
>>> Chritian notice it doesn't really work?
>> To clarify I think this might work.
>>
>> I just had the same "Is that legal?", "What about security?", etc..
>> questions you raised as well.
>>
>> It seems like a source of trouble so I thought better ask somebody more
>> familiar with that.
>>
>> Christian.
>>
>>> Jason
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-16 15:31 ` Christian König
@ 2020-09-17 6:23 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 6:23 UTC (permalink / raw)
To: Christian König
Cc: Jason Gunthorpe, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Wed, Sep 16, 2020 at 5:31 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 16.09.20 um 17:24 schrieb Daniel Vetter:
> > On Wed, Sep 16, 2020 at 4:14 PM Christian König
> > <christian.koenig@amd.com> wrote:
> >> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> >>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> >>>
> >>>> But within the driver, we generally need thousands of these, and that
> >>>> tends to bring fd exhaustion problems with it. That's why all the private
> >>>> buffer objects which aren't shared with other process or other drivers are
> >>>> handles only valid for a specific fd instance of the drm chardev (each
> >>>> open gets their own namespace), and only for ioctls done on that chardev.
> >>>> And for mmap we assign fake (but unique across all open fd on it) offsets
> >>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> >>> Are they still unique struct files? Just without a fdno?
> >> Yes, exactly.
> > Not entirely, since dma-buf happened after drm chardev, so for that
> > historical reason the underlying struct file is shared, since it's the
> > drm chardev. But since that's per-device we don't have a problem in
> > practice with different vm_ops, since those are also per-device. But
> > yeah we could fish out some entirely hidden per-object struct file if
> > that's required for some mm internal reasons.
>
> Hui? Ok that is just the handling in i915, isn't it?
>
> As far as I know we create an unique struct file for each DMA-buf.
Yes dma-buf, but that gets forwarded to the original drm chardev which
originally exported the buffer. It's only there where the forwarding
chain stops. The other thing is that iirc we have a singleton
anon_inode behind all the dma-buf, so they'd share all the same
address_space and so would all alias for unmap_mapping_range (I think
at least).
-Daniel
>
> Regards,
> Christian.
>
>
> > -Daniel
> >
> >>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
> >>>> file and pgoff, while hopefully everything keeps working. I thought this
> >>>> would work, but Christian noticed it doesn't really.
> >>> It seems reasonable to me that the dma buf should be the owner of the
> >>> VMA, otherwise like you say, there is a big mess attaching the custom
> >>> vma ops and what not to the proper dma buf.
> >>>
> >>> I don't see anything obviously against this in mmap_region() - why did
> >>> Chritian notice it doesn't really work?
> >> To clarify I think this might work.
> >>
> >> I just had the same "Is that legal?", "What about security?", etc..
> >> questions you raised as well.
> >>
> >> It seems like a source of trouble so I thought better ask somebody more
> >> familiar with that.
> >>
> >> Christian.
> >>
> >>> Jason
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 6:23 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 6:23 UTC (permalink / raw)
To: Christian König
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Wed, Sep 16, 2020 at 5:31 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 16.09.20 um 17:24 schrieb Daniel Vetter:
> > On Wed, Sep 16, 2020 at 4:14 PM Christian König
> > <christian.koenig@amd.com> wrote:
> >> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> >>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> >>>
> >>>> But within the driver, we generally need thousands of these, and that
> >>>> tends to bring fd exhaustion problems with it. That's why all the private
> >>>> buffer objects which aren't shared with other process or other drivers are
> >>>> handles only valid for a specific fd instance of the drm chardev (each
> >>>> open gets their own namespace), and only for ioctls done on that chardev.
> >>>> And for mmap we assign fake (but unique across all open fd on it) offsets
> >>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> >>> Are they still unique struct files? Just without a fdno?
> >> Yes, exactly.
> > Not entirely, since dma-buf happened after drm chardev, so for that
> > historical reason the underlying struct file is shared, since it's the
> > drm chardev. But since that's per-device we don't have a problem in
> > practice with different vm_ops, since those are also per-device. But
> > yeah we could fish out some entirely hidden per-object struct file if
> > that's required for some mm internal reasons.
>
> Hui? Ok that is just the handling in i915, isn't it?
>
> As far as I know we create an unique struct file for each DMA-buf.
Yes dma-buf, but that gets forwarded to the original drm chardev which
originally exported the buffer. It's only there where the forwarding
chain stops. The other thing is that iirc we have a singleton
anon_inode behind all the dma-buf, so they'd share all the same
address_space and so would all alias for unmap_mapping_range (I think
at least).
-Daniel
>
> Regards,
> Christian.
>
>
> > -Daniel
> >
> >>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
> >>>> file and pgoff, while hopefully everything keeps working. I thought this
> >>>> would work, but Christian noticed it doesn't really.
> >>> It seems reasonable to me that the dma buf should be the owner of the
> >>> VMA, otherwise like you say, there is a big mess attaching the custom
> >>> vma ops and what not to the proper dma buf.
> >>>
> >>> I don't see anything obviously against this in mmap_region() - why did
> >>> Chritian notice it doesn't really work?
> >> To clarify I think this might work.
> >>
> >> I just had the same "Is that legal?", "What about security?", etc..
> >> questions you raised as well.
> >>
> >> It seems like a source of trouble so I thought better ask somebody more
> >> familiar with that.
> >>
> >> Christian.
> >>
> >>> Jason
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 6:23 ` Daniel Vetter
@ 2020-09-17 7:11 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 7:11 UTC (permalink / raw)
To: Daniel Vetter
Cc: Jason Gunthorpe, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 08:23 schrieb Daniel Vetter:
> On Wed, Sep 16, 2020 at 5:31 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 16.09.20 um 17:24 schrieb Daniel Vetter:
>>> On Wed, Sep 16, 2020 at 4:14 PM Christian König
>>> <christian.koenig@amd.com> wrote:
>>>> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
>>>>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
>>>>>
>>>>>> But within the driver, we generally need thousands of these, and that
>>>>>> tends to bring fd exhaustion problems with it. That's why all the private
>>>>>> buffer objects which aren't shared with other process or other drivers are
>>>>>> handles only valid for a specific fd instance of the drm chardev (each
>>>>>> open gets their own namespace), and only for ioctls done on that chardev.
>>>>>> And for mmap we assign fake (but unique across all open fd on it) offsets
>>>>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
>>>>> Are they still unique struct files? Just without a fdno?
>>>> Yes, exactly.
>>> Not entirely, since dma-buf happened after drm chardev, so for that
>>> historical reason the underlying struct file is shared, since it's the
>>> drm chardev. But since that's per-device we don't have a problem in
>>> practice with different vm_ops, since those are also per-device. But
>>> yeah we could fish out some entirely hidden per-object struct file if
>>> that's required for some mm internal reasons.
>> Hui? Ok that is just the handling in i915, isn't it?
>>
>> As far as I know we create an unique struct file for each DMA-buf.
> Yes dma-buf, but that gets forwarded to the original drm chardev which
> originally exported the buffer. It's only there where the forwarding
> chain stops. The other thing is that iirc we have a singleton
> anon_inode behind all the dma-buf, so they'd share all the same
> address_space and so would all alias for unmap_mapping_range (I think
> at least).
Amdgpu works by using the address_space of the drm chardev into the
struct file of DMA-buf instead.
I think that this is cleaner, but only by a little bit :)
Anyway I'm a bit concerned that we have so many different approaches for
the same problem.
Christian.
> -Daniel
>
>> Regards,
>> Christian.
>>
>>
>>> -Daniel
>>>
>>>>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
>>>>>> file and pgoff, while hopefully everything keeps working. I thought this
>>>>>> would work, but Christian noticed it doesn't really.
>>>>> It seems reasonable to me that the dma buf should be the owner of the
>>>>> VMA, otherwise like you say, there is a big mess attaching the custom
>>>>> vma ops and what not to the proper dma buf.
>>>>>
>>>>> I don't see anything obviously against this in mmap_region() - why did
>>>>> Chritian notice it doesn't really work?
>>>> To clarify I think this might work.
>>>>
>>>> I just had the same "Is that legal?", "What about security?", etc..
>>>> questions you raised as well.
>>>>
>>>> It seems like a source of trouble so I thought better ask somebody more
>>>> familiar with that.
>>>>
>>>> Christian.
>>>>
>>>>> Jason
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Cf725d2eb6a5a49bd533f08d85ad23308%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637359206142262941&sdata=qcLsl9R1gP%2FGY39ctsQkIzI99Bn%2F840YS17F4xudrAE%3D&reserved=0
>>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 7:11 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 7:11 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 08:23 schrieb Daniel Vetter:
> On Wed, Sep 16, 2020 at 5:31 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 16.09.20 um 17:24 schrieb Daniel Vetter:
>>> On Wed, Sep 16, 2020 at 4:14 PM Christian König
>>> <christian.koenig@amd.com> wrote:
>>>> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
>>>>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
>>>>>
>>>>>> But within the driver, we generally need thousands of these, and that
>>>>>> tends to bring fd exhaustion problems with it. That's why all the private
>>>>>> buffer objects which aren't shared with other process or other drivers are
>>>>>> handles only valid for a specific fd instance of the drm chardev (each
>>>>>> open gets their own namespace), and only for ioctls done on that chardev.
>>>>>> And for mmap we assign fake (but unique across all open fd on it) offsets
>>>>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
>>>>> Are they still unique struct files? Just without a fdno?
>>>> Yes, exactly.
>>> Not entirely, since dma-buf happened after drm chardev, so for that
>>> historical reason the underlying struct file is shared, since it's the
>>> drm chardev. But since that's per-device we don't have a problem in
>>> practice with different vm_ops, since those are also per-device. But
>>> yeah we could fish out some entirely hidden per-object struct file if
>>> that's required for some mm internal reasons.
>> Hui? Ok that is just the handling in i915, isn't it?
>>
>> As far as I know we create an unique struct file for each DMA-buf.
> Yes dma-buf, but that gets forwarded to the original drm chardev which
> originally exported the buffer. It's only there where the forwarding
> chain stops. The other thing is that iirc we have a singleton
> anon_inode behind all the dma-buf, so they'd share all the same
> address_space and so would all alias for unmap_mapping_range (I think
> at least).
Amdgpu works by using the address_space of the drm chardev into the
struct file of DMA-buf instead.
I think that this is cleaner, but only by a little bit :)
Anyway I'm a bit concerned that we have so many different approaches for
the same problem.
Christian.
> -Daniel
>
>> Regards,
>> Christian.
>>
>>
>>> -Daniel
>>>
>>>>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
>>>>>> file and pgoff, while hopefully everything keeps working. I thought this
>>>>>> would work, but Christian noticed it doesn't really.
>>>>> It seems reasonable to me that the dma buf should be the owner of the
>>>>> VMA, otherwise like you say, there is a big mess attaching the custom
>>>>> vma ops and what not to the proper dma buf.
>>>>>
>>>>> I don't see anything obviously against this in mmap_region() - why did
>>>>> Chritian notice it doesn't really work?
>>>> To clarify I think this might work.
>>>>
>>>> I just had the same "Is that legal?", "What about security?", etc..
>>>> questions you raised as well.
>>>>
>>>> It seems like a source of trouble so I thought better ask somebody more
>>>> familiar with that.
>>>>
>>>> Christian.
>>>>
>>>>> Jason
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Cf725d2eb6a5a49bd533f08d85ad23308%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637359206142262941&sdata=qcLsl9R1gP%2FGY39ctsQkIzI99Bn%2F840YS17F4xudrAE%3D&reserved=0
>>>
>
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 7:11 ` Christian König
@ 2020-09-17 8:09 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 8:09 UTC (permalink / raw)
To: Christian König
Cc: Jason Gunthorpe, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 9:11 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 17.09.20 um 08:23 schrieb Daniel Vetter:
> > On Wed, Sep 16, 2020 at 5:31 PM Christian König
> > <ckoenig.leichtzumerken@gmail.com> wrote:
> >> Am 16.09.20 um 17:24 schrieb Daniel Vetter:
> >>> On Wed, Sep 16, 2020 at 4:14 PM Christian König
> >>> <christian.koenig@amd.com> wrote:
> >>>> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> >>>>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> >>>>>
> >>>>>> But within the driver, we generally need thousands of these, and that
> >>>>>> tends to bring fd exhaustion problems with it. That's why all the private
> >>>>>> buffer objects which aren't shared with other process or other drivers are
> >>>>>> handles only valid for a specific fd instance of the drm chardev (each
> >>>>>> open gets their own namespace), and only for ioctls done on that chardev.
> >>>>>> And for mmap we assign fake (but unique across all open fd on it) offsets
> >>>>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> >>>>> Are they still unique struct files? Just without a fdno?
> >>>> Yes, exactly.
> >>> Not entirely, since dma-buf happened after drm chardev, so for that
> >>> historical reason the underlying struct file is shared, since it's the
> >>> drm chardev. But since that's per-device we don't have a problem in
> >>> practice with different vm_ops, since those are also per-device. But
> >>> yeah we could fish out some entirely hidden per-object struct file if
> >>> that's required for some mm internal reasons.
> >> Hui? Ok that is just the handling in i915, isn't it?
> >>
> >> As far as I know we create an unique struct file for each DMA-buf.
> > Yes dma-buf, but that gets forwarded to the original drm chardev which
> > originally exported the buffer. It's only there where the forwarding
> > chain stops. The other thing is that iirc we have a singleton
> > anon_inode behind all the dma-buf, so they'd share all the same
> > address_space and so would all alias for unmap_mapping_range (I think
> > at least).
>
> Amdgpu works by using the address_space of the drm chardev into the
> struct file of DMA-buf instead.
>
> I think that this is cleaner, but only by a little bit :)
Yeah, but it doesn't work when forwarding from the drm chardev to the
dma-buf on the importer side, since you'd need a ton of different
address spaces. And you still rely on the core code picking up your
pgoff mangling, which feels about as risky to me as the vma file
pointer wrangling - if it's not consistently applied the reverse map
is toast and unmap_mapping_range doesn't work correctly for our needs.
> Anyway I'm a bit concerned that we have so many different approaches for
> the same problem.
Yeah, I think if we can standardize this then that would be really good.
-Daniel
>
> Christian.
>
> > -Daniel
> >
> >> Regards,
> >> Christian.
> >>
> >>
> >>> -Daniel
> >>>
> >>>>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
> >>>>>> file and pgoff, while hopefully everything keeps working. I thought this
> >>>>>> would work, but Christian noticed it doesn't really.
> >>>>> It seems reasonable to me that the dma buf should be the owner of the
> >>>>> VMA, otherwise like you say, there is a big mess attaching the custom
> >>>>> vma ops and what not to the proper dma buf.
> >>>>>
> >>>>> I don't see anything obviously against this in mmap_region() - why did
> >>>>> Chritian notice it doesn't really work?
> >>>> To clarify I think this might work.
> >>>>
> >>>> I just had the same "Is that legal?", "What about security?", etc..
> >>>> questions you raised as well.
> >>>>
> >>>> It seems like a source of trouble so I thought better ask somebody more
> >>>> familiar with that.
> >>>>
> >>>> Christian.
> >>>>
> >>>>> Jason
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@lists.freedesktop.org
> >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Cf725d2eb6a5a49bd533f08d85ad23308%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637359206142262941&sdata=qcLsl9R1gP%2FGY39ctsQkIzI99Bn%2F840YS17F4xudrAE%3D&reserved=0
> >>>
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 8:09 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 8:09 UTC (permalink / raw)
To: Christian König
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 9:11 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 17.09.20 um 08:23 schrieb Daniel Vetter:
> > On Wed, Sep 16, 2020 at 5:31 PM Christian König
> > <ckoenig.leichtzumerken@gmail.com> wrote:
> >> Am 16.09.20 um 17:24 schrieb Daniel Vetter:
> >>> On Wed, Sep 16, 2020 at 4:14 PM Christian König
> >>> <christian.koenig@amd.com> wrote:
> >>>> Am 16.09.20 um 16:07 schrieb Jason Gunthorpe:
> >>>>> On Wed, Sep 16, 2020 at 11:53:59AM +0200, Daniel Vetter wrote:
> >>>>>
> >>>>>> But within the driver, we generally need thousands of these, and that
> >>>>>> tends to bring fd exhaustion problems with it. That's why all the private
> >>>>>> buffer objects which aren't shared with other process or other drivers are
> >>>>>> handles only valid for a specific fd instance of the drm chardev (each
> >>>>>> open gets their own namespace), and only for ioctls done on that chardev.
> >>>>>> And for mmap we assign fake (but unique across all open fd on it) offsets
> >>>>>> within the overall chardev. Hence all the pgoff mangling and re-mangling.
> >>>>> Are they still unique struct files? Just without a fdno?
> >>>> Yes, exactly.
> >>> Not entirely, since dma-buf happened after drm chardev, so for that
> >>> historical reason the underlying struct file is shared, since it's the
> >>> drm chardev. But since that's per-device we don't have a problem in
> >>> practice with different vm_ops, since those are also per-device. But
> >>> yeah we could fish out some entirely hidden per-object struct file if
> >>> that's required for some mm internal reasons.
> >> Hui? Ok that is just the handling in i915, isn't it?
> >>
> >> As far as I know we create an unique struct file for each DMA-buf.
> > Yes dma-buf, but that gets forwarded to the original drm chardev which
> > originally exported the buffer. It's only there where the forwarding
> > chain stops. The other thing is that iirc we have a singleton
> > anon_inode behind all the dma-buf, so they'd share all the same
> > address_space and so would all alias for unmap_mapping_range (I think
> > at least).
>
> Amdgpu works by using the address_space of the drm chardev into the
> struct file of DMA-buf instead.
>
> I think that this is cleaner, but only by a little bit :)
Yeah, but it doesn't work when forwarding from the drm chardev to the
dma-buf on the importer side, since you'd need a ton of different
address spaces. And you still rely on the core code picking up your
pgoff mangling, which feels about as risky to me as the vma file
pointer wrangling - if it's not consistently applied the reverse map
is toast and unmap_mapping_range doesn't work correctly for our needs.
> Anyway I'm a bit concerned that we have so many different approaches for
> the same problem.
Yeah, I think if we can standardize this then that would be really good.
-Daniel
>
> Christian.
>
> > -Daniel
> >
> >> Regards,
> >> Christian.
> >>
> >>
> >>> -Daniel
> >>>
> >>>>>> Hence why we'd like to be able to forward aliasing mappings and adjust the
> >>>>>> file and pgoff, while hopefully everything keeps working. I thought this
> >>>>>> would work, but Christian noticed it doesn't really.
> >>>>> It seems reasonable to me that the dma buf should be the owner of the
> >>>>> VMA, otherwise like you say, there is a big mess attaching the custom
> >>>>> vma ops and what not to the proper dma buf.
> >>>>>
> >>>>> I don't see anything obviously against this in mmap_region() - why did
> >>>>> Chritian notice it doesn't really work?
> >>>> To clarify I think this might work.
> >>>>
> >>>> I just had the same "Is that legal?", "What about security?", etc..
> >>>> questions you raised as well.
> >>>>
> >>>> It seems like a source of trouble so I thought better ask somebody more
> >>>> familiar with that.
> >>>>
> >>>> Christian.
> >>>>
> >>>>> Jason
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@lists.freedesktop.org
> >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Cf725d2eb6a5a49bd533f08d85ad23308%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637359206142262941&sdata=qcLsl9R1gP%2FGY39ctsQkIzI99Bn%2F840YS17F4xudrAE%3D&reserved=0
> >>>
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 8:09 ` Daniel Vetter
@ 2020-09-17 11:31 ` Jason Gunthorpe
-1 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 11:31 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, Christian König,
open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> Yeah, but it doesn't work when forwarding from the drm chardev to the
> dma-buf on the importer side, since you'd need a ton of different
> address spaces. And you still rely on the core code picking up your
> pgoff mangling, which feels about as risky to me as the vma file
> pointer wrangling - if it's not consistently applied the reverse map
> is toast and unmap_mapping_range doesn't work correctly for our needs.
I would think the pgoff has to be translated at the same time the
vm->vm_file is changed?
The owner of the dma_buf should have one virtual address space and FD,
all its dma bufs should be linked to it, and all pgoffs translated to
that space.
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 11:31 ` Jason Gunthorpe
0 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 11:31 UTC (permalink / raw)
To: Daniel Vetter
Cc: Christian König, Linux MM, Linux Kernel Mailing List,
dri-devel, moderated list:DMA BUFFER SHARING FRAMEWORK,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> Yeah, but it doesn't work when forwarding from the drm chardev to the
> dma-buf on the importer side, since you'd need a ton of different
> address spaces. And you still rely on the core code picking up your
> pgoff mangling, which feels about as risky to me as the vma file
> pointer wrangling - if it's not consistently applied the reverse map
> is toast and unmap_mapping_range doesn't work correctly for our needs.
I would think the pgoff has to be translated at the same time the
vm->vm_file is changed?
The owner of the dma_buf should have one virtual address space and FD,
all its dma bufs should be linked to it, and all pgoffs translated to
that space.
Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 11:31 ` Jason Gunthorpe
@ 2020-09-17 12:03 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 12:03 UTC (permalink / raw)
To: Jason Gunthorpe, Daniel Vetter
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, Christian König,
open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>
>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>> dma-buf on the importer side, since you'd need a ton of different
>> address spaces. And you still rely on the core code picking up your
>> pgoff mangling, which feels about as risky to me as the vma file
>> pointer wrangling - if it's not consistently applied the reverse map
>> is toast and unmap_mapping_range doesn't work correctly for our needs.
> I would think the pgoff has to be translated at the same time the
> vm->vm_file is changed?
>
> The owner of the dma_buf should have one virtual address space and FD,
> all its dma bufs should be linked to it, and all pgoffs translated to
> that space.
Yeah, that is exactly like amdgpu is doing it.
Going to document that somehow when I'm done with TTM cleanups.
Christian.
>
> Jason
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 12:03 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 12:03 UTC (permalink / raw)
To: Jason Gunthorpe, Daniel Vetter
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, Christian König,
open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>
>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>> dma-buf on the importer side, since you'd need a ton of different
>> address spaces. And you still rely on the core code picking up your
>> pgoff mangling, which feels about as risky to me as the vma file
>> pointer wrangling - if it's not consistently applied the reverse map
>> is toast and unmap_mapping_range doesn't work correctly for our needs.
> I would think the pgoff has to be translated at the same time the
> vm->vm_file is changed?
>
> The owner of the dma_buf should have one virtual address space and FD,
> all its dma bufs should be linked to it, and all pgoffs translated to
> that space.
Yeah, that is exactly like amdgpu is doing it.
Going to document that somehow when I'm done with TTM cleanups.
Christian.
>
> Jason
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 12:03 ` Christian König
@ 2020-09-17 12:18 ` Jason Gunthorpe
-1 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 12:18 UTC (permalink / raw)
To: christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> >
> > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > dma-buf on the importer side, since you'd need a ton of different
> > > address spaces. And you still rely on the core code picking up your
> > > pgoff mangling, which feels about as risky to me as the vma file
> > > pointer wrangling - if it's not consistently applied the reverse map
> > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > I would think the pgoff has to be translated at the same time the
> > vm->vm_file is changed?
> >
> > The owner of the dma_buf should have one virtual address space and FD,
> > all its dma bufs should be linked to it, and all pgoffs translated to
> > that space.
>
> Yeah, that is exactly like amdgpu is doing it.
>
> Going to document that somehow when I'm done with TTM cleanups.
BTW, while people are looking at this, is there a way to go from a VMA
to a dma_buf that owns it?
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 12:18 ` Jason Gunthorpe
0 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 12:18 UTC (permalink / raw)
To: christian.koenig
Cc: Daniel Vetter, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> >
> > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > dma-buf on the importer side, since you'd need a ton of different
> > > address spaces. And you still rely on the core code picking up your
> > > pgoff mangling, which feels about as risky to me as the vma file
> > > pointer wrangling - if it's not consistently applied the reverse map
> > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > I would think the pgoff has to be translated at the same time the
> > vm->vm_file is changed?
> >
> > The owner of the dma_buf should have one virtual address space and FD,
> > all its dma bufs should be linked to it, and all pgoffs translated to
> > that space.
>
> Yeah, that is exactly like amdgpu is doing it.
>
> Going to document that somehow when I'm done with TTM cleanups.
BTW, while people are looking at this, is there a way to go from a VMA
to a dma_buf that owns it?
Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 12:18 ` Jason Gunthorpe
@ 2020-09-17 12:24 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 12:24 UTC (permalink / raw)
To: Jason Gunthorpe, christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
>> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
>>> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>>>
>>>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>>>> dma-buf on the importer side, since you'd need a ton of different
>>>> address spaces. And you still rely on the core code picking up your
>>>> pgoff mangling, which feels about as risky to me as the vma file
>>>> pointer wrangling - if it's not consistently applied the reverse map
>>>> is toast and unmap_mapping_range doesn't work correctly for our needs.
>>> I would think the pgoff has to be translated at the same time the
>>> vm->vm_file is changed?
>>>
>>> The owner of the dma_buf should have one virtual address space and FD,
>>> all its dma bufs should be linked to it, and all pgoffs translated to
>>> that space.
>> Yeah, that is exactly like amdgpu is doing it.
>>
>> Going to document that somehow when I'm done with TTM cleanups.
> BTW, while people are looking at this, is there a way to go from a VMA
> to a dma_buf that owns it?
Only a driver specific one.
For TTM drivers vma->vm_private_data points to the buffer object. Not
sure about the drivers using GEM only.
Why are you asking?
Regards,
Christian.
>
> Jason
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 12:24 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 12:24 UTC (permalink / raw)
To: Jason Gunthorpe, christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Daniel Vetter, Andrew Morton,
open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
>> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
>>> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>>>
>>>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>>>> dma-buf on the importer side, since you'd need a ton of different
>>>> address spaces. And you still rely on the core code picking up your
>>>> pgoff mangling, which feels about as risky to me as the vma file
>>>> pointer wrangling - if it's not consistently applied the reverse map
>>>> is toast and unmap_mapping_range doesn't work correctly for our needs.
>>> I would think the pgoff has to be translated at the same time the
>>> vm->vm_file is changed?
>>>
>>> The owner of the dma_buf should have one virtual address space and FD,
>>> all its dma bufs should be linked to it, and all pgoffs translated to
>>> that space.
>> Yeah, that is exactly like amdgpu is doing it.
>>
>> Going to document that somehow when I'm done with TTM cleanups.
> BTW, while people are looking at this, is there a way to go from a VMA
> to a dma_buf that owns it?
Only a driver specific one.
For TTM drivers vma->vm_private_data points to the buffer object. Not
sure about the drivers using GEM only.
Why are you asking?
Regards,
Christian.
>
> Jason
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 12:24 ` Christian König
@ 2020-09-17 12:26 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 12:26 UTC (permalink / raw)
To: christian.koenig
Cc: Linux MM, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Jason Gunthorpe,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > >
> > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > address spaces. And you still rely on the core code picking up your
> > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > I would think the pgoff has to be translated at the same time the
> > > > vm->vm_file is changed?
> > > >
> > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > that space.
> > > Yeah, that is exactly like amdgpu is doing it.
> > >
> > > Going to document that somehow when I'm done with TTM cleanups.
> > BTW, while people are looking at this, is there a way to go from a VMA
> > to a dma_buf that owns it?
>
> Only a driver specific one.
>
> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> about the drivers using GEM only.
For gem drivers (which includes the ones using vram helpers, which uses
ttm internally) that points at the drm_gem_object pointer. I guess that
might be something we can unify a bit on the ttm mmap paths of the
remaining driver, now that there's a drm_gem_object embedded anyway.
-Daniel
>
> Why are you asking?
>
> Regards,
> Christian.
>
> >
> > Jason
> > _______________________________________________
> > Linaro-mm-sig mailing list
> > Linaro-mm-sig@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 12:26 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 12:26 UTC (permalink / raw)
To: christian.koenig
Cc: Jason Gunthorpe, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Daniel Vetter, Andrew Morton,
open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > >
> > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > address spaces. And you still rely on the core code picking up your
> > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > I would think the pgoff has to be translated at the same time the
> > > > vm->vm_file is changed?
> > > >
> > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > that space.
> > > Yeah, that is exactly like amdgpu is doing it.
> > >
> > > Going to document that somehow when I'm done with TTM cleanups.
> > BTW, while people are looking at this, is there a way to go from a VMA
> > to a dma_buf that owns it?
>
> Only a driver specific one.
>
> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> about the drivers using GEM only.
For gem drivers (which includes the ones using vram helpers, which uses
ttm internally) that points at the drm_gem_object pointer. I guess that
might be something we can unify a bit on the ttm mmap paths of the
remaining driver, now that there's a drm_gem_object embedded anyway.
-Daniel
>
> Why are you asking?
>
> Regards,
> Christian.
>
> >
> > Jason
> > _______________________________________________
> > Linaro-mm-sig mailing list
> > Linaro-mm-sig@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 12:24 ` Christian König
@ 2020-09-17 14:35 ` Jason Gunthorpe
-1 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 14:35 UTC (permalink / raw)
To: christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > >
> > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > address spaces. And you still rely on the core code picking up your
> > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > I would think the pgoff has to be translated at the same time the
> > > > vm->vm_file is changed?
> > > >
> > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > that space.
> > > Yeah, that is exactly like amdgpu is doing it.
> > >
> > > Going to document that somehow when I'm done with TTM cleanups.
> > BTW, while people are looking at this, is there a way to go from a VMA
> > to a dma_buf that owns it?
>
> Only a driver specific one.
Sounds OK
> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> about the drivers using GEM only.
Why are drivers in control of the vma? I would think dma_buf should be
the vma owner. IIRC module lifetime correctness essentially hings on
the module owner of the struct file
> Why are you asking?
I'm thinking about using find_vma on something that is not
get_user_pages()'able to go to the underlying object, in this case dma
buf.
So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
memory it represents
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 14:35 ` Jason Gunthorpe
0 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 14:35 UTC (permalink / raw)
To: christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Daniel Vetter, Andrew Morton,
open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > >
> > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > address spaces. And you still rely on the core code picking up your
> > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > I would think the pgoff has to be translated at the same time the
> > > > vm->vm_file is changed?
> > > >
> > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > that space.
> > > Yeah, that is exactly like amdgpu is doing it.
> > >
> > > Going to document that somehow when I'm done with TTM cleanups.
> > BTW, while people are looking at this, is there a way to go from a VMA
> > to a dma_buf that owns it?
>
> Only a driver specific one.
Sounds OK
> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> about the drivers using GEM only.
Why are drivers in control of the vma? I would think dma_buf should be
the vma owner. IIRC module lifetime correctness essentially hings on
the module owner of the struct file
> Why are you asking?
I'm thinking about using find_vma on something that is not
get_user_pages()'able to go to the underlying object, in this case dma
buf.
So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
memory it represents
Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 14:35 ` Jason Gunthorpe
@ 2020-09-17 14:54 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 14:54 UTC (permalink / raw)
To: Jason Gunthorpe, christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
> On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
>> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
>>> On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
>>>> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
>>>>> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>>>>>
>>>>>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>>>>>> dma-buf on the importer side, since you'd need a ton of different
>>>>>> address spaces. And you still rely on the core code picking up your
>>>>>> pgoff mangling, which feels about as risky to me as the vma file
>>>>>> pointer wrangling - if it's not consistently applied the reverse map
>>>>>> is toast and unmap_mapping_range doesn't work correctly for our needs.
>>>>> I would think the pgoff has to be translated at the same time the
>>>>> vm->vm_file is changed?
>>>>>
>>>>> The owner of the dma_buf should have one virtual address space and FD,
>>>>> all its dma bufs should be linked to it, and all pgoffs translated to
>>>>> that space.
>>>> Yeah, that is exactly like amdgpu is doing it.
>>>>
>>>> Going to document that somehow when I'm done with TTM cleanups.
>>> BTW, while people are looking at this, is there a way to go from a VMA
>>> to a dma_buf that owns it?
>> Only a driver specific one.
> Sounds OK
>
>> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
>> about the drivers using GEM only.
> Why are drivers in control of the vma? I would think dma_buf should be
> the vma owner. IIRC module lifetime correctness essentially hings on
> the module owner of the struct file
Because the page fault handling is completely driver specific.
We could install some DMA-buf vmops, but that would just be another
layer of redirection.
>> Why are you asking?
> I'm thinking about using find_vma on something that is not
> get_user_pages()'able to go to the underlying object, in this case dma
> buf.
>
> So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
> memory it represents
Ah, yes we are already doing this in amdgpu as well. But only for
DMA-bufs or more generally buffers which are mmaped by this driver instance.
Some applications are braindead enough to mmap() a buffer and then give
us back the CPU pointer and request to make it a handle (userptr) again.
That is clearly forbidden by OpenGL, OpenCL and Vulkan, but they use it
anyway.
Christian.
>
> Jason
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 14:54 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 14:54 UTC (permalink / raw)
To: Jason Gunthorpe, christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Daniel Vetter, Andrew Morton,
open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
> On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
>> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
>>> On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
>>>> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
>>>>> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>>>>>
>>>>>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>>>>>> dma-buf on the importer side, since you'd need a ton of different
>>>>>> address spaces. And you still rely on the core code picking up your
>>>>>> pgoff mangling, which feels about as risky to me as the vma file
>>>>>> pointer wrangling - if it's not consistently applied the reverse map
>>>>>> is toast and unmap_mapping_range doesn't work correctly for our needs.
>>>>> I would think the pgoff has to be translated at the same time the
>>>>> vm->vm_file is changed?
>>>>>
>>>>> The owner of the dma_buf should have one virtual address space and FD,
>>>>> all its dma bufs should be linked to it, and all pgoffs translated to
>>>>> that space.
>>>> Yeah, that is exactly like amdgpu is doing it.
>>>>
>>>> Going to document that somehow when I'm done with TTM cleanups.
>>> BTW, while people are looking at this, is there a way to go from a VMA
>>> to a dma_buf that owns it?
>> Only a driver specific one.
> Sounds OK
>
>> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
>> about the drivers using GEM only.
> Why are drivers in control of the vma? I would think dma_buf should be
> the vma owner. IIRC module lifetime correctness essentially hings on
> the module owner of the struct file
Because the page fault handling is completely driver specific.
We could install some DMA-buf vmops, but that would just be another
layer of redirection.
>> Why are you asking?
> I'm thinking about using find_vma on something that is not
> get_user_pages()'able to go to the underlying object, in this case dma
> buf.
>
> So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
> memory it represents
Ah, yes we are already doing this in amdgpu as well. But only for
DMA-bufs or more generally buffers which are mmaped by this driver instance.
Some applications are braindead enough to mmap() a buffer and then give
us back the CPU pointer and request to make it a handle (userptr) again.
That is clearly forbidden by OpenGL, OpenCL and Vulkan, but they use it
anyway.
Christian.
>
> Jason
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 14:54 ` Christian König
@ 2020-09-17 15:24 ` Jason Gunthorpe
-1 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 15:24 UTC (permalink / raw)
To: christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 04:54:44PM +0200, Christian König wrote:
> Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> > > Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > > > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > > > >
> > > > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > > > address spaces. And you still rely on the core code picking up your
> > > > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > > > I would think the pgoff has to be translated at the same time the
> > > > > > vm->vm_file is changed?
> > > > > >
> > > > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > > > that space.
> > > > > Yeah, that is exactly like amdgpu is doing it.
> > > > >
> > > > > Going to document that somehow when I'm done with TTM cleanups.
> > > > BTW, while people are looking at this, is there a way to go from a VMA
> > > > to a dma_buf that owns it?
> > > Only a driver specific one.
> > Sounds OK
> >
> > > For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> > > about the drivers using GEM only.
> > Why are drivers in control of the vma? I would think dma_buf should be
> > the vma owner. IIRC module lifetime correctness essentially hings on
> > the module owner of the struct file
>
> Because the page fault handling is completely driver specific.
>
> We could install some DMA-buf vmops, but that would just be another layer of
> redirection.
If it is already taking a page fault I'm not sure the extra function
call indirection is going to be a big deal. Having a uniform VMA
sounds saner than every driver custom rolling something.
When I unwound a similar mess in RDMA all the custom VMA stuff in the
drivers turned out to be generally buggy, at least.
Is vma->vm_file->private_data universally a dma_buf pointer at least?
> > So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
> > memory it represents
>
> Ah, yes we are already doing this in amdgpu as well. But only for DMA-bufs
> or more generally buffers which are mmaped by this driver instance.
So there is no general dma_buf service? That is a real bummer
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 15:24 ` Jason Gunthorpe
0 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 15:24 UTC (permalink / raw)
To: christian.koenig
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Daniel Vetter, Andrew Morton,
open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 04:54:44PM +0200, Christian König wrote:
> Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
> > On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> > > Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > > > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > > > >
> > > > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > > > address spaces. And you still rely on the core code picking up your
> > > > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > > > I would think the pgoff has to be translated at the same time the
> > > > > > vm->vm_file is changed?
> > > > > >
> > > > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > > > that space.
> > > > > Yeah, that is exactly like amdgpu is doing it.
> > > > >
> > > > > Going to document that somehow when I'm done with TTM cleanups.
> > > > BTW, while people are looking at this, is there a way to go from a VMA
> > > > to a dma_buf that owns it?
> > > Only a driver specific one.
> > Sounds OK
> >
> > > For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> > > about the drivers using GEM only.
> > Why are drivers in control of the vma? I would think dma_buf should be
> > the vma owner. IIRC module lifetime correctness essentially hings on
> > the module owner of the struct file
>
> Because the page fault handling is completely driver specific.
>
> We could install some DMA-buf vmops, but that would just be another layer of
> redirection.
If it is already taking a page fault I'm not sure the extra function
call indirection is going to be a big deal. Having a uniform VMA
sounds saner than every driver custom rolling something.
When I unwound a similar mess in RDMA all the custom VMA stuff in the
drivers turned out to be generally buggy, at least.
Is vma->vm_file->private_data universally a dma_buf pointer at least?
> > So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
> > memory it represents
>
> Ah, yes we are already doing this in amdgpu as well. But only for DMA-bufs
> or more generally buffers which are mmaped by this driver instance.
So there is no general dma_buf service? That is a real bummer
Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 15:24 ` Jason Gunthorpe
@ 2020-09-17 15:37 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 15:37 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, Christian König,
open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 5:24 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Sep 17, 2020 at 04:54:44PM +0200, Christian König wrote:
> > Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
> > > On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> > > > Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > > > > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > > > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > > > > >
> > > > > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > > > > address spaces. And you still rely on the core code picking up your
> > > > > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > > > > I would think the pgoff has to be translated at the same time the
> > > > > > > vm->vm_file is changed?
> > > > > > >
> > > > > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > > > > that space.
> > > > > > Yeah, that is exactly like amdgpu is doing it.
> > > > > >
> > > > > > Going to document that somehow when I'm done with TTM cleanups.
> > > > > BTW, while people are looking at this, is there a way to go from a VMA
> > > > > to a dma_buf that owns it?
> > > > Only a driver specific one.
> > > Sounds OK
> > >
> > > > For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> > > > about the drivers using GEM only.
> > > Why are drivers in control of the vma? I would think dma_buf should be
> > > the vma owner. IIRC module lifetime correctness essentially hings on
> > > the module owner of the struct file
> >
> > Because the page fault handling is completely driver specific.
> >
> > We could install some DMA-buf vmops, but that would just be another layer of
> > redirection.
Uh geez I didn't know amdgpu was doing that :-/
Since this is on, I guess the inverse of trying to convert a userptr
into a dma-buf is properly rejected?
> If it is already taking a page fault I'm not sure the extra function
> call indirection is going to be a big deal. Having a uniform VMA
> sounds saner than every driver custom rolling something.
>
> When I unwound a similar mess in RDMA all the custom VMA stuff in the
> drivers turned out to be generally buggy, at least.
>
> Is vma->vm_file->private_data universally a dma_buf pointer at least?
Nope. I think if you want this without some large scale rewrite of a
lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
would get the job done.
> > > So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
> > > memory it represents
> >
> > Ah, yes we are already doing this in amdgpu as well. But only for DMA-bufs
> > or more generally buffers which are mmaped by this driver instance.
>
> So there is no general dma_buf service? That is a real bummer
Mostly historical reasons and "it's complicated". One problem is that
dma-buf isn't a powerful enough interface that drivers could use it
for all their native objects, e.g. userptr doesn't pass through it,
and clever cache flushing tricks aren't allowed and a bunch of other
things. So there's some serious roadblocks before we could have a
common allocator (or set of allocators) behind dma-buf.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 15:37 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 15:37 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Christian König, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 5:24 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Sep 17, 2020 at 04:54:44PM +0200, Christian König wrote:
> > Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
> > > On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
> > > > Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
> > > > > On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
> > > > > > Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
> > > > > > > On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
> > > > > > >
> > > > > > > > Yeah, but it doesn't work when forwarding from the drm chardev to the
> > > > > > > > dma-buf on the importer side, since you'd need a ton of different
> > > > > > > > address spaces. And you still rely on the core code picking up your
> > > > > > > > pgoff mangling, which feels about as risky to me as the vma file
> > > > > > > > pointer wrangling - if it's not consistently applied the reverse map
> > > > > > > > is toast and unmap_mapping_range doesn't work correctly for our needs.
> > > > > > > I would think the pgoff has to be translated at the same time the
> > > > > > > vm->vm_file is changed?
> > > > > > >
> > > > > > > The owner of the dma_buf should have one virtual address space and FD,
> > > > > > > all its dma bufs should be linked to it, and all pgoffs translated to
> > > > > > > that space.
> > > > > > Yeah, that is exactly like amdgpu is doing it.
> > > > > >
> > > > > > Going to document that somehow when I'm done with TTM cleanups.
> > > > > BTW, while people are looking at this, is there a way to go from a VMA
> > > > > to a dma_buf that owns it?
> > > > Only a driver specific one.
> > > Sounds OK
> > >
> > > > For TTM drivers vma->vm_private_data points to the buffer object. Not sure
> > > > about the drivers using GEM only.
> > > Why are drivers in control of the vma? I would think dma_buf should be
> > > the vma owner. IIRC module lifetime correctness essentially hings on
> > > the module owner of the struct file
> >
> > Because the page fault handling is completely driver specific.
> >
> > We could install some DMA-buf vmops, but that would just be another layer of
> > redirection.
Uh geez I didn't know amdgpu was doing that :-/
Since this is on, I guess the inverse of trying to convert a userptr
into a dma-buf is properly rejected?
> If it is already taking a page fault I'm not sure the extra function
> call indirection is going to be a big deal. Having a uniform VMA
> sounds saner than every driver custom rolling something.
>
> When I unwound a similar mess in RDMA all the custom VMA stuff in the
> drivers turned out to be generally buggy, at least.
>
> Is vma->vm_file->private_data universally a dma_buf pointer at least?
Nope. I think if you want this without some large scale rewrite of a
lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
would get the job done.
> > > So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
> > > memory it represents
> >
> > Ah, yes we are already doing this in amdgpu as well. But only for DMA-bufs
> > or more generally buffers which are mmaped by this driver instance.
>
> So there is no general dma_buf service? That is a real bummer
Mostly historical reasons and "it's complicated". One problem is that
dma-buf isn't a powerful enough interface that drivers could use it
for all their native objects, e.g. userptr doesn't pass through it,
and clever cache flushing tricks aren't allowed and a bunch of other
things. So there's some serious roadblocks before we could have a
common allocator (or set of allocators) behind dma-buf.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 15:37 ` Daniel Vetter
@ 2020-09-17 16:06 ` Christian König
-1 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 16:06 UTC (permalink / raw)
To: Daniel Vetter, Jason Gunthorpe
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 17:37 schrieb Daniel Vetter:
> On Thu, Sep 17, 2020 at 5:24 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>> On Thu, Sep 17, 2020 at 04:54:44PM +0200, Christian König wrote:
>>> Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
>>>> On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
>>>>> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
>>>>>> On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
>>>>>>> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
>>>>>>>> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>>>>>>>>
>>>>>>>>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>>>>>>>>> dma-buf on the importer side, since you'd need a ton of different
>>>>>>>>> address spaces. And you still rely on the core code picking up your
>>>>>>>>> pgoff mangling, which feels about as risky to me as the vma file
>>>>>>>>> pointer wrangling - if it's not consistently applied the reverse map
>>>>>>>>> is toast and unmap_mapping_range doesn't work correctly for our needs.
>>>>>>>> I would think the pgoff has to be translated at the same time the
>>>>>>>> vm->vm_file is changed?
>>>>>>>>
>>>>>>>> The owner of the dma_buf should have one virtual address space and FD,
>>>>>>>> all its dma bufs should be linked to it, and all pgoffs translated to
>>>>>>>> that space.
>>>>>>> Yeah, that is exactly like amdgpu is doing it.
>>>>>>>
>>>>>>> Going to document that somehow when I'm done with TTM cleanups.
>>>>>> BTW, while people are looking at this, is there a way to go from a VMA
>>>>>> to a dma_buf that owns it?
>>>>> Only a driver specific one.
>>>> Sounds OK
>>>>
>>>>> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
>>>>> about the drivers using GEM only.
>>>> Why are drivers in control of the vma? I would think dma_buf should be
>>>> the vma owner. IIRC module lifetime correctness essentially hings on
>>>> the module owner of the struct file
>>> Because the page fault handling is completely driver specific.
>>>
>>> We could install some DMA-buf vmops, but that would just be another layer of
>>> redirection.
> Uh geez I didn't know amdgpu was doing that :-/
>
> Since this is on, I guess the inverse of trying to convert a userptr
> into a dma-buf is properly rejected?
My fault, I wasn't specific enough in my description :)
Amdgpu is NOT doing this with mmaped DMA-bufs, but rather with it's own
mmaped BOs.
In other words when userspace call the userptr IOCTL and we get an error
because we can't make an userptr from some random device memory we
instead check all CPU mappings if the application was brain dead enough
to provide us our own pointer back.
IIRC this is even done in userspace and not the kernel. But we talked
about doing it in the kernel with the private_data as well.
>
>> If it is already taking a page fault I'm not sure the extra function
>> call indirection is going to be a big deal. Having a uniform VMA
>> sounds saner than every driver custom rolling something.
>>
>> When I unwound a similar mess in RDMA all the custom VMA stuff in the
>> drivers turned out to be generally buggy, at least.
>>
>> Is vma->vm_file->private_data universally a dma_buf pointer at least?
> Nope. I think if you want this without some large scale rewrite of a
> lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
> would get the job done.
Yeah, agree that sounds like the simplest approach.
Regards,
Christian.
>
>>>> So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
>>>> memory it represents
>>> Ah, yes we are already doing this in amdgpu as well. But only for DMA-bufs
>>> or more generally buffers which are mmaped by this driver instance.
>> So there is no general dma_buf service? That is a real bummer
> Mostly historical reasons and "it's complicated". One problem is that
> dma-buf isn't a powerful enough interface that drivers could use it
> for all their native objects, e.g. userptr doesn't pass through it,
> and clever cache flushing tricks aren't allowed and a bunch of other
> things. So there's some serious roadblocks before we could have a
> common allocator (or set of allocators) behind dma-buf.
> -Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 16:06 ` Christian König
0 siblings, 0 replies; 70+ messages in thread
From: Christian König @ 2020-09-17 16:06 UTC (permalink / raw)
To: Daniel Vetter, Jason Gunthorpe
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
Am 17.09.20 um 17:37 schrieb Daniel Vetter:
> On Thu, Sep 17, 2020 at 5:24 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>> On Thu, Sep 17, 2020 at 04:54:44PM +0200, Christian König wrote:
>>> Am 17.09.20 um 16:35 schrieb Jason Gunthorpe:
>>>> On Thu, Sep 17, 2020 at 02:24:29PM +0200, Christian König wrote:
>>>>> Am 17.09.20 um 14:18 schrieb Jason Gunthorpe:
>>>>>> On Thu, Sep 17, 2020 at 02:03:48PM +0200, Christian König wrote:
>>>>>>> Am 17.09.20 um 13:31 schrieb Jason Gunthorpe:
>>>>>>>> On Thu, Sep 17, 2020 at 10:09:12AM +0200, Daniel Vetter wrote:
>>>>>>>>
>>>>>>>>> Yeah, but it doesn't work when forwarding from the drm chardev to the
>>>>>>>>> dma-buf on the importer side, since you'd need a ton of different
>>>>>>>>> address spaces. And you still rely on the core code picking up your
>>>>>>>>> pgoff mangling, which feels about as risky to me as the vma file
>>>>>>>>> pointer wrangling - if it's not consistently applied the reverse map
>>>>>>>>> is toast and unmap_mapping_range doesn't work correctly for our needs.
>>>>>>>> I would think the pgoff has to be translated at the same time the
>>>>>>>> vm->vm_file is changed?
>>>>>>>>
>>>>>>>> The owner of the dma_buf should have one virtual address space and FD,
>>>>>>>> all its dma bufs should be linked to it, and all pgoffs translated to
>>>>>>>> that space.
>>>>>>> Yeah, that is exactly like amdgpu is doing it.
>>>>>>>
>>>>>>> Going to document that somehow when I'm done with TTM cleanups.
>>>>>> BTW, while people are looking at this, is there a way to go from a VMA
>>>>>> to a dma_buf that owns it?
>>>>> Only a driver specific one.
>>>> Sounds OK
>>>>
>>>>> For TTM drivers vma->vm_private_data points to the buffer object. Not sure
>>>>> about the drivers using GEM only.
>>>> Why are drivers in control of the vma? I would think dma_buf should be
>>>> the vma owner. IIRC module lifetime correctness essentially hings on
>>>> the module owner of the struct file
>>> Because the page fault handling is completely driver specific.
>>>
>>> We could install some DMA-buf vmops, but that would just be another layer of
>>> redirection.
> Uh geez I didn't know amdgpu was doing that :-/
>
> Since this is on, I guess the inverse of trying to convert a userptr
> into a dma-buf is properly rejected?
My fault, I wasn't specific enough in my description :)
Amdgpu is NOT doing this with mmaped DMA-bufs, but rather with it's own
mmaped BOs.
In other words when userspace call the userptr IOCTL and we get an error
because we can't make an userptr from some random device memory we
instead check all CPU mappings if the application was brain dead enough
to provide us our own pointer back.
IIRC this is even done in userspace and not the kernel. But we talked
about doing it in the kernel with the private_data as well.
>
>> If it is already taking a page fault I'm not sure the extra function
>> call indirection is going to be a big deal. Having a uniform VMA
>> sounds saner than every driver custom rolling something.
>>
>> When I unwound a similar mess in RDMA all the custom VMA stuff in the
>> drivers turned out to be generally buggy, at least.
>>
>> Is vma->vm_file->private_data universally a dma_buf pointer at least?
> Nope. I think if you want this without some large scale rewrite of a
> lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
> would get the job done.
Yeah, agree that sounds like the simplest approach.
Regards,
Christian.
>
>>>> So, user VA -> find_vma -> dma_buf object -> dma_buf operations on the
>>>> memory it represents
>>> Ah, yes we are already doing this in amdgpu as well. But only for DMA-bufs
>>> or more generally buffers which are mmaped by this driver instance.
>> So there is no general dma_buf service? That is a real bummer
> Mostly historical reasons and "it's complicated". One problem is that
> dma-buf isn't a powerful enough interface that drivers could use it
> for all their native objects, e.g. userptr doesn't pass through it,
> and clever cache flushing tricks aren't allowed and a bunch of other
> things. So there's some serious roadblocks before we could have a
> common allocator (or set of allocators) behind dma-buf.
> -Daniel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 16:06 ` Christian König
@ 2020-09-17 16:39 ` Jason Gunthorpe
-1 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 16:39 UTC (permalink / raw)
To: Christian König
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 06:06:14PM +0200, Christian König wrote:
> > > If it is already taking a page fault I'm not sure the extra function
> > > call indirection is going to be a big deal. Having a uniform VMA
> > > sounds saner than every driver custom rolling something.
> > >
> > > When I unwound a similar mess in RDMA all the custom VMA stuff in the
> > > drivers turned out to be generally buggy, at least.
> > >
> > > Is vma->vm_file->private_data universally a dma_buf pointer at least?
> > Nope. I think if you want this without some large scale rewrite of a
> > lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
> > would get the job done.
>
> Yeah, agree that sounds like the simplest approach.
I don't think that will fly, it is clearly only papering over a mess
inside DRM/dma buf :\
Jason
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 16:39 ` Jason Gunthorpe
0 siblings, 0 replies; 70+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 16:39 UTC (permalink / raw)
To: Christian König
Cc: Daniel Vetter, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 06:06:14PM +0200, Christian König wrote:
> > > If it is already taking a page fault I'm not sure the extra function
> > > call indirection is going to be a big deal. Having a uniform VMA
> > > sounds saner than every driver custom rolling something.
> > >
> > > When I unwound a similar mess in RDMA all the custom VMA stuff in the
> > > drivers turned out to be generally buggy, at least.
> > >
> > > Is vma->vm_file->private_data universally a dma_buf pointer at least?
> > Nope. I think if you want this without some large scale rewrite of a
> > lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
> > would get the job done.
>
> Yeah, agree that sounds like the simplest approach.
I don't think that will fly, it is clearly only papering over a mess
inside DRM/dma buf :\
Jason
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
2020-09-17 16:39 ` Jason Gunthorpe
@ 2020-09-17 17:23 ` Daniel Vetter
-1 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 17:23 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, Christian König,
open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 6:39 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Sep 17, 2020 at 06:06:14PM +0200, Christian König wrote:
> > > > If it is already taking a page fault I'm not sure the extra function
> > > > call indirection is going to be a big deal. Having a uniform VMA
> > > > sounds saner than every driver custom rolling something.
> > > >
> > > > When I unwound a similar mess in RDMA all the custom VMA stuff in the
> > > > drivers turned out to be generally buggy, at least.
> > > >
> > > > Is vma->vm_file->private_data universally a dma_buf pointer at least?
> > > Nope. I think if you want this without some large scale rewrite of a
> > > lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
> > > would get the job done.
> >
> > Yeah, agree that sounds like the simplest approach.
>
> I don't think that will fly, it is clearly only papering over a mess
> inside DRM/dma buf :\
dma-buf started out as something to paper over the disjoint mess of
allocators, since it was pretty clear to anyone involved we're not
going to unify them anytime soon, if ever. So the mess pretty much is
bound to stay.
I think most reasonable thing would be to throw a common vmops in
there somehow, but even that means some large scale surgery across
every driver/subsystem involved in dma-buf. It wouldn't unify
anything, all it would give you is a constant vma->vm_ops to do some
kind of upcasting. And that would still only give you a slightly less
opaque pointer with a callback to upcast to a dma-buf in some
driver/subsystem specific way.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Linaro-mm-sig] Changing vma->vm_file in dma_buf_mmap()
@ 2020-09-17 17:23 ` Daniel Vetter
0 siblings, 0 replies; 70+ messages in thread
From: Daniel Vetter @ 2020-09-17 17:23 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Christian König, Linux Kernel Mailing List, dri-devel,
moderated list:DMA BUFFER SHARING FRAMEWORK, Linux MM,
Andrew Morton, open list:DMA BUFFER SHARING FRAMEWORK
On Thu, Sep 17, 2020 at 6:39 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Sep 17, 2020 at 06:06:14PM +0200, Christian König wrote:
> > > > If it is already taking a page fault I'm not sure the extra function
> > > > call indirection is going to be a big deal. Having a uniform VMA
> > > > sounds saner than every driver custom rolling something.
> > > >
> > > > When I unwound a similar mess in RDMA all the custom VMA stuff in the
> > > > drivers turned out to be generally buggy, at least.
> > > >
> > > > Is vma->vm_file->private_data universally a dma_buf pointer at least?
> > > Nope. I think if you want this without some large scale rewrite of a
> > > lot of code we'd need a vmops->get_dmabuf or similar. Not pretty, but
> > > would get the job done.
> >
> > Yeah, agree that sounds like the simplest approach.
>
> I don't think that will fly, it is clearly only papering over a mess
> inside DRM/dma buf :\
dma-buf started out as something to paper over the disjoint mess of
allocators, since it was pretty clear to anyone involved we're not
going to unify them anytime soon, if ever. So the mess pretty much is
bound to stay.
I think most reasonable thing would be to throw a common vmops in
there somehow, but even that means some large scale surgery across
every driver/subsystem involved in dma-buf. It wouldn't unify
anything, all it would give you is a constant vma->vm_ops to do some
kind of upcasting. And that would still only give you a slightly less
opaque pointer with a callback to upcast to a dma-buf in some
driver/subsystem specific way.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 70+ messages in thread