linux-kernel - Re: [PATCH RESEND] drm/virtio: Align host mapping request to maximum platform page size

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAF6AEGuGWNj7dcC4DouWckaP9fOKgLyqw8iPBwKxrnr9GYXpPg@mail.gmail.com>
Date: Wed, 12 Feb 2025 13:07:10 -0800
From: Rob Clark <robdclark@...il.com>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc: fnkl.kernel@...il.com, 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>, Sergio Lopez <slp@...hat.com>, dri-devel@...ts.freedesktop.org, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	asahi@...ts.linux.dev, Rob Clark <robdclark@...omium.org>
Subject: Re: [PATCH RESEND] drm/virtio: Align host mapping request to maximum
 platform page size

On Fri, Jan 24, 2025 at 2:52 PM Dmitry Osipenko
<dmitry.osipenko@...labora.com> wrote:
>
> On 1/25/25 01:01, Sasha Finkelstein via B4 Relay wrote:
> > From: Sasha Finkelstein <fnkl.kernel@...il.com>
> >
> > This allows running different page sizes between host and guest on
> > platforms that support mixed page sizes.
> >
> > Signed-off-by: Sasha Finkelstein <fnkl.kernel@...il.com>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_vram.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
> > index 25df81c027837c248a746e41856b5aa7e216b8d5..8a0577c2170ec9c12cad12be57f9a41c14f04660 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_vram.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
> > @@ -138,6 +138,12 @@ bool virtio_gpu_is_vram(struct virtio_gpu_object *bo)
> >       return bo->base.base.funcs == &virtio_gpu_vram_funcs;
> >  }
> >
> > +#if defined(__powerpc64__) || defined(__aarch64__) || defined(__mips__) || defined(__loongarch__)
> > +#define MAX_PAGE_SIZE 65536
>
> #define MAX_PAGE_SIZE SZ_64K
>
> for improved readability
>
> > +#else
> > +#define MAX_PAGE_SIZE PAGE_SIZE
> > +#endif
> > +
> >  static int virtio_gpu_vram_map(struct virtio_gpu_object *bo)
> >  {
> >       int ret;
> > @@ -150,8 +156,8 @@ static int virtio_gpu_vram_map(struct virtio_gpu_object *bo)
> >               return -EINVAL;
> >
> >       spin_lock(&vgdev->host_visible_lock);
> > -     ret = drm_mm_insert_node(&vgdev->host_visible_mm, &vram->vram_node,
> > -                              bo->base.base.size);
> > +     ret = drm_mm_insert_node_generic(&vgdev->host_visible_mm, &vram->vram_node,
> > +                                      bo->base.base.size, MAX_PAGE_SIZE, 0, 0);
>
> This change only reserves extra space in the memory allocator, but
> doesn't change actual size of allocated BO. Instead, you likely need to
> replace all ALIGN(size, PAGE_SIZE) occurrences in the driver code with
> ALIGN(args->size, MAX_PAGE_SIZE)
>
> >       spin_unlock(&vgdev->host_visible_lock);
> >
> >       if (ret)
>
> Note, previously a new virtio-gpu parameter was proposed to expose
> host's page size to guest [1], if you haven't seen it.
>
> [1] https://lore.kernel.org/dri-devel/20240723114914.53677-1-slp@redhat.com/
>
> Aligning GEM's size to 64K indeed could be a good enough immediate
> solution. Don't see any obvious problems with that, other than the
> potential size overhead for a small BOs.
>
> We have been running into cases where a DXVK game allocates enormous
> amounts of small BOs to the point that amount reaches max number of
> mappings on QEMU with amdgpu native context. On the other hand, it
> showed that adding internal sub-allocator to RADV might be a worthwhile
> effort. We won't change alignment on x86 with this patch and on non-x86
> likely the increased size won't be noticeable, hence the proposed change
> might be acceptable.
>
> Curious what Rob Clark thinks about it. Rob, WDYT?

I think userspace needs to know the minimum "gpu" page size (ie. the
host page size) for other reasons, such as sparse binding.  As Sergio
proposes, we should add this to the virtgpu protocol

BR,
-R


>
> --
> Best regards,
> Dmitry
>