[PATCH] drm/vgem: Restore mmap functionality

[Daniel Vetter]

On Mon, Jul 12, 2021 at 09:09:24AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 09.07.21 um 16:43 schrieb Daniel Vetter:
> > On Fri, Jul 9, 2021 at 1:47 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
> > > 
> > > Commit 375cca1cfeb5 ("drm/vgem: Implement mmap as GEM object function")
> > > accidentally removed the actual mmap functionality from vgem. Restore
> > > the original implementation and VMA flags.
> > 
> > Ah yes that explains things.
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> Apparently, this fix fails a number of other tests. [1] Can we revert the
> original patch for now? I'd like to have time to investigate.

Uh yes something funny is going on here. I've also beent trying to debug
my conversion of vgem to shmem helpers, and been hitting lots of strange
bugs (but the ones I debugged thus far were all real issues, and resulted
in shmem helper fixes).

So yeah, on the revert:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Please include discussions and links to your patch here and CI results in
the commit message, so that we have a better starting point for the next
attempt.
-Daniel

> 
> Best regards
> Thomas
> 
> 
> [1] https://lore.kernel.org/intel-gfx/20210709154256.12005-1-tzimmermann@suse.de/T/#maa12be2a6d4b6a4ed8cb05e98f00b8955638c518
> 
> > > 
> > > Fixes access to unmapped memory:
> > > 
> > > [  106.591744] BUG: KASAN: vmalloc-out-of-bounds in do_fault+0x38/0x480
> > > [  106.598154] Read of size 8 at addr ffffffffc10c44a8 by task vgem_basic/1514
> > > [  106.605173]
> > > [  106.606678] CPU: 1 PID: 1514 Comm: vgem_basic Tainted: G            E     5.13.0-1-default+ #990
> > > [  106.615535] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24 10/24/2018
> > > [  106.622818] Call Trace:
> > > [  106.625289]  dump_stack+0xa5/0xdc
> > > [  106.628642]  print_address_description.constprop.0+0x18/0x100
> > > [  106.634439]  ? do_fault+0x38/0x480
> > > [  106.637872]  kasan_report.cold+0x7c/0xd8
> > > [  106.641834]  ? do_fault+0x38/0x480
> > > [  106.645274]  do_fault+0x38/0x480
> > > [  106.648535]  __handle_mm_fault+0x935/0xb00
> > > [  106.652676]  ? vm_iomap_memory+0xe0/0xe0
> > > [  106.656634]  ? __lock_release+0x12f/0x4e0
> > > [  106.660696]  ? count_memcg_event_mm.part.0+0xb9/0x190
> > > [  106.665799]  handle_mm_fault+0xd0/0x370
> > > [  106.669675]  do_user_addr_fault+0x2a0/0x8c0
> > > [  106.673908]  exc_page_fault+0x64/0xf0
> > > [  106.677604]  ? asm_exc_page_fault+0x8/0x30
> > > [  106.681739]  asm_exc_page_fault+0x1e/0x30
> > > [  106.685782] RIP: 0033:0x402e12
> > > ...
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> > > Fixes: 375cca1cfeb5 ("drm/vgem: Implement mmap as GEM object function")
> > > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > > Cc: Christian König <christian.koenig at amd.com>
> > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > Cc: "Christian König" <christian.koenig at amd.com>
> > > Cc: Melissa Wen <melissa.srw at gmail.com>
> > > Cc: Jason Gunthorpe <jgg at ziepe.ca>
> > > Cc: Gerd Hoffmann <kraxel at redhat.com>
> > > Cc: Lee Jones <lee.jones at linaro.org>
> > > ---
> > >   drivers/gpu/drm/vgem/vgem_drv.c | 11 ++++++++++-
> > >   1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> > > index df634aa52638..f50fd10c4fad 100644
> > > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > > @@ -364,8 +364,17 @@ static void vgem_prime_vunmap(struct drm_gem_object *obj, struct dma_buf_map *ma
> > > 
> > >   static int vgem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > >   {
> > > +       int ret;
> > > +
> > > +       if (!obj->filp)
> > > +               return -ENODEV;
> > > +
> > > +       ret = call_mmap(obj->filp, vma);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > >          vma_set_file(vma, obj->filp);
> > > -       vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> > > +       vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> > >          vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> > >          vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> > > 
> > > --
> > > 2.32.0
> > > 
> > 
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch