[PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap

[Gerd Hoffmann]

  Hi,

> > ... but after looking again I think we are all green here.  Given that
> > only self-import works we'll only see vram gem objects in the mmap code
> > path, which should have everything set up correctly.  Same goes for qxl.
> >
> > All other ttm drivers still use the old mmap code path, so all green
> > there too I think.  Also I somehow doubt dma-buf mmap vs. drm mmap ends
> > up using different f_mapping, ttm code has a WARN_ON in ttm_bo_vm_open()
> > which would fire should that be the case.
> >
> > Do imported dma-bufs hit the drm mmap code path in the first place?
> > Wouldn't mmap be handled by the exporting driver?
> 
> drm_gem_dmabuf_mmap -> obj->funcs->mmap -> ttm_bo_mmap_obj
> 
> And I'm not seeing anyone adjusting vm_file->f_mapping anywhere here at all.

[ some more code browsing ]

Ok, I see.  dma-bufs get their own file, their own anon inode and
thereby their own address space.  So that it used when mmaping the
dma-buf.

drm filehandle's get the shared address space instead, drm_open() sets
it.

So, yes, I see the problem.  It's not new though, as far I can see the
old dma-buf mmap code path doesn't adjust f_mapping anywhere either ...

> Note to hit this you need userspace to
> - handle2fd on a buffer to create a dma-buf fd
> - call mmap directly on that dma-buf fd

Hmm, seems for handle2fd I need a dummy gem_prime_get_sg_table function
wired up even when not actually exporting/importing anything.  So I
think neither qxl nor any of the vram drivers allow to trigger that (and
no other ttm driver uses the new ttm mmap code yet).

So, $subject patch should not make things worse in ttm land.

When hacking the bochs driver to have export callbacks (without
supporting actual exports) handle2fd + mmap() callback works fine.
Didn't verify yet I actually get the correct pages mapped.  But maybe
mmap() isn't the problem when the correct address space is important for
unmap only.

Is there a good test case?

cheers,
  Gerd