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@xxxxxxx> 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@xxxxxxxx>
>
> 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@xxxxxxxx>
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@xxxxxxx/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@xxxxxxx>
> > > Fixes: 375cca1cfeb5 ("drm/vgem: Implement mmap as GEM object function")
> > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > Cc: Christian König <christian.koenig@xxxxxxx>
> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > Cc: "Christian König" <christian.koenig@xxxxxxx>
> > > Cc: Melissa Wen <melissa.srw@xxxxxxxxx>
> > > Cc: Jason Gunthorpe <jgg@xxxxxxxx>
> > > Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> > > Cc: Lee Jones <lee.jones@xxxxxxxxxx>
> > > ---
> > > 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
