linux-kernel - Re: [PATCH v5] drm/virtio: Add drm

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] [day] [month] [year] [list]

Message-ID: <CAHpthZrZ6DjsCQ4baQ80b2vOTdkR=vHDx=10W7DTS4ohxb6=pg@mail.gmail.com>
Date: Tue, 10 Dec 2024 21:24:03 +0900
From: Ryosuke Yasuoka <ryasuoka@...hat.com>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc: airlied@...hat.com, kraxel@...hat.com, maarten.lankhorst@...ux.intel.com, 
	mripard@...nel.org, tzimmermann@...e.de, daniel@...ll.ch, jfalempe@...hat.com, 
	christophe.jaillet@...adoo.fr, virtualization@...ts.linux.dev, 
	spice-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v5] drm/virtio: Add drm_panic support

On Mon, Dec 2, 2024 at 11:30 PM Dmitry Osipenko
<dmitry.osipenko@...labora.com> wrote:
>
> On 11/29/24 15:24, Ryosuke Yasuoka wrote:
> ....
> > +     } else {
> > +             iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> > +     }
> > +
> > +     sb->format = plane->state->fb->format;
> > +     sb->height = plane->state->fb->height;
> > +     sb->width = plane->state->fb->width;
> > +     sb->pitch[0] = plane->state->fb->pitches[0];
> > +     return 0;
> > +}
> ...
> > +static void virtio_panic_flush(struct drm_plane *plane)
> > +{
> > +     struct virtio_gpu_object *bo;
> > +     struct drm_device *dev = plane->dev;
> > +     struct virtio_gpu_device *vgdev = dev->dev_private;
> > +     struct drm_rect rect;
> > +     void *p_vbuf = vgdev->panic_vbuf;
> > +     struct virtio_gpu_vbuffer *vbuf_dumb_bo = p_vbuf;
> > +     struct virtio_gpu_vbuffer *vbuf_resource_flush = p_vbuf + VBUFFER_SIZE;
>
> This p_vbuf + VBUFFER_SIZE looks suspicious. The VBUFFER_SIZE macro
> isn't guarded with parentheses (), hence this vbuf_resource_flush is
> pointing at the CMD part of the vbuf_dumb_bo?

No. I intend vbuf_resource_flush to point to a different vbuf than
vbuf_dumb_bo. I allocate 2 vbufs in virtio_gpu_plane_init() so that
vbuf_dumb_bo points to the first vbuf and vbuf_resource_flush points
to the second one.

> Won't be using kmem_cache_zalloc(vgdev->vbufs, GFP_ATOMIC) to
> dynamically allocate both buffers make everything cleaner?
>
> ...
> > -#define MAX_INLINE_CMD_SIZE   96
> > -#define MAX_INLINE_RESP_SIZE  24
> > -#define VBUFFER_SIZE          (sizeof(struct virtio_gpu_vbuffer) \
> > -                            + MAX_INLINE_CMD_SIZE             \
> > -                            + MAX_INLINE_RESP_SIZE)...
>

Yes, it is feasible.

Thank you for your review.
Ryosuke

> Best regards,
> Dmitry
>