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 <[email protected]> 
> > 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 <[email protected]>
> 
> 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 <[email protected]>

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/[email protected]/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 <[email protected]>
> > > Fixes: 375cca1cfeb5 ("drm/vgem: Implement mmap as GEM object function")
> > > Cc: Thomas Zimmermann <[email protected]>
> > > Cc: Christian König <[email protected]>
> > > Cc: Daniel Vetter <[email protected]>
> > > Cc: "Christian König" <[email protected]>
> > > Cc: Melissa Wen <[email protected]>
> > > Cc: Jason Gunthorpe <[email protected]>
> > > Cc: Gerd Hoffmann <[email protected]>
> > > Cc: Lee Jones <[email protected]>
> > > ---
> > >   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

Reply via email to