linux-kernel - Re: [PATCH] drm/virtio: Support partial maps of GEM objects

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMT+MTT11xg-14KrcqUTx3DxF=1NnYZAMY8RoZFoTesc0zEiVA@mail.gmail.com>
Date: Sun, 19 Jan 2025 17:18:09 +0100
From: Sasha Finkelstein <fnkl.kernel@...il.com>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc: David Airlie <airlied@...hat.com>, Gerd Hoffmann <kraxel@...hat.com>, 
	Gurchetan Singh <gurchetansingh@...omium.org>, Chia-I Wu <olvaffe@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, Simona Vetter <simona@...ll.ch>, dri-devel@...ts.freedesktop.org, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	asahi@...ts.linux.dev
Subject: Re: [PATCH] drm/virtio: Support partial maps of GEM objects

On Sun, 19 Jan 2025 at 12:50, Dmitry Osipenko
<dmitry.osipenko@...labora.com> wrote:
> >       ret = io_remap_pfn_range(vma, vma->vm_start,
> > -                              vram->vram_node.start >> PAGE_SHIFT,
> > +                              (vram->vram_node.start >> PAGE_SHIFT) + vma->vm_pgoff,
> >                                vm_size, vma->vm_page_prot);
> >       return ret;
> >  }
>
> The vma->vm_pgoff is fake in DRM, it's used for looking up DRM GEM
> object based on the vma->vm_pgoff value when mmap is invoked.

If my understanding is correct, vm_pgoff gets "unfaked" by
https://elixir.bootlin.com/linux/v6.12.6/source/drivers/gpu/drm/virtio/virtgpu_vram.c#L48

> vma->vm_pgoff should be treated as zero here. Hence we can map a part of
> GEM, but only from its start. See drm_gem_mmap().

I've had a "v0" (not on ml) of this patch that always treated vma->vm_pgoff as
zero. This broke when anything tried to mmap with a non-zero offset. Adding
vm_pgoff made it work correctly.

> Please correct vma->vm_pgoff in v2.

I need apps to be able to mmap with a non-zero offset for my usecase.
While the correct value may be something else other than what is in
the current patch, 0 is definitely incorrect for at least some workloads.