linux-kernel - Re: [PATCH] drm/virtio: implement virtio_gpu

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: <iptz2uxajkl3l62piqu6tq5pembbmqho667otbaj7nneh5vk3r@sxdvm7e57nae>
Date: Tue, 13 May 2025 12:18:44 +0200
From: Gerd Hoffmann <kraxel@...hat.com>
To: Maxime Ripard <mripard@...nel.org>
Cc: dri-devel@...ts.freedesktop.org, "Michael S. Tsirkin" <mst@...hat.com>, 
	Eric Auger <eric.auger@...hat.com>, David Airlie <airlied@...hat.com>, 
	Gurchetan Singh <gurchetansingh@...omium.org>, Chia-I Wu <olvaffe@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	Simona Vetter <simona@...ll.ch>, "open list:VIRTIO GPU DRIVER" <virtualization@...ts.linux.dev>, 
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/virtio: implement virtio_gpu_shutdown

> > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> > index e32e680c7197..71c6ccad4b99 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> > @@ -130,10 +130,10 @@ static void virtio_gpu_remove(struct virtio_device *vdev)
> >  
> >  static void virtio_gpu_shutdown(struct virtio_device *vdev)
> >  {
> > -	/*
> > -	 * drm does its own synchronization on shutdown.
> > -	 * Do nothing here, opt out of device reset.
> > -	 */
> > +	struct drm_device *dev = vdev->priv;
> > +
> > +	/* stop talking to the device */
> > +	drm_dev_unplug(dev);
> 
> I'm not necessarily opposed to using drm_dev_unplug() here, but it's
> still pretty surprising to me. It's typically used in remove, not
> shutdown. The typical helper to use at shutdown is
> drm_atomic_helper_shutdown.
> 
> So if the latter isn't enough or wrong, we should at least document why.

The intention of this is to make sure the driver stops talking to the
device (as the comment already says).

There are checks in place in the virt queue functions which will make
sure the driver will not try place new requests in the queues after
drm_dev_unplug() has been called.  Which why I decided to implement it
that way.

drm_atomic_helper_shutdown() tears down all outputs according to the
documentation.  Which is something different.  I don't think calling
drm_atomic_helper_shutdown() will do what I need here.  Calling it in
addition to drm_dev_unplug() might make sense, not sure.

Suggestions are welcome.

take care,
  Gerd