Am 2023-01-16 um 06:42 schrieb Christian König:
[SNIP]
When the BO is imported into the same GPU, you get a reference to
the same BO, so the imported BO has the same mmap_offset as the
original BO.
When the BO is imported into a different GPU, it is a new BO with a
new mmap_offset.
That won't work.
I don't think this is incorrect.
No, this is completely incorrect. It mixes up the reverse tracking
of mappings and might crash the system.
I don't understand that. The imported BO is a different BO with a
different mmap offset in a different device file. I don't see how
that messes with the tracking of mappings.
The tracking keeps note which piece of information is accessible
through which address space object and offset. I you suddenly have two
address spaces and offsets pointing to the same piece of information
that won't work any more.
How do you identify a "piece of information". I don't think it's the
physical page. VRAM doesn't even have struct pages. I think it's the BO
that's being tracked. With a dmabuf import you have a second BO aliasing
the same physical pages as the original BO. Then those two BOs are seen
as two distinct "pieces of information" that can each have their own
mapping.
This is the reason why we can't mmap() imported BOs.
I don't see anything preventing that. For userptr BOs, there is this
code in amdgpu_gem_object_mmap:
if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm))
return -EPERM;
I don't see anything like this preventing mmapping of imported dmabuf
BOs. What am I missing?
At some point I really need to make a big presentation about all this
stuff, we had the same discussion multiple times now :)
It's the same reason why you can't mmap() VRAM through the kfd node:
Each file can have only one address space object associated with it.
I remember that. We haven't used KFD to mmap BOs for a long time for
that reason.
See dma_buf_mmap() and vma_set_file() how this is worked around in
DMA-buf.
These are for mmapping memory through the dmabuf fd. I'm not sure that's
a good example. drm_gem_prime_mmap creates a temporary struct file and
struct drm_file that are destroyed immediately after calling
obj->dev->driver->fops->mmap. I think that would break any reverse mapping.
mmapping the memory with that new offset should still work. The
imported BO is created with ttm_bo_type_sg, and AFAICT ttm_bo_vm.c
supports mapping of SG BOs.
Actually it shouldn't. This can go boom really easily.
OK. I don't think we're doing this, but after Xiaogang raised the
question I went looking through the code whether it's theoretically
possible. I didn't find anything in the code that says that mmapping
imported dmabufs would be prohibited or even dangerous. On the
contrary, I found that ttm_bo_vm explicitly supports mmapping SG BOs.
When you have imported a BO the only correct way of to mmap() it is
to do so on the original exporter.
That seems sensible, and this is what we do today. That said, if
mmapping an imported BO is dangerous, I'm missing a mechanism to
protect against this. It could be as simple as setting
AMDGPU_GEM_CREATE_NO_CPU_ACCESS in amdgpu_dma_buf_create_obj.
At least for the GEM mmap() handler this is double checked very early
by looking at obj->import_attach and then either rejecting it or
redirecting the request to the DMA-buf file instead.
Can you point me at where this check is? I see a check for
obj->import_attach in drm_gem_dumb_map_offset. But I can't see how this
function is called in amdgpu. I don't think it is used at all.
We probably need something similar when stuff is mapped through the
KFD node. But I think we don't do that any more for "normal" BOs
anyway, don't we?
Correct, we don't map BOs through the KFD device file. The only mappings
we still use it for are:
* Doorbells on APUs
* Events page on APUs
* MMIO page for HDP flushing
The code for mmapping regular BOs through /dev/kfd was never upstream.
Regards,
Felix
Regards,
Christian.
Regards,
Felix
