All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Brian Daniels <briandaniels@google.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	adelva@google.com, aesteve@redhat.com, changyeon@google.com,
	daniel.almeida@collabora.com, eperezma@redhat.com,
	gnurou@gmail.com, gurchetansingh@google.com, hverkuil@xs4all.nl,
	jasowang@redhat.com, linux-kernel@vger.kernel.org,
	linux-media@vger.kernel.org, mst@redhat.com,
	nicolas.dufresne@collabora.com, virtualization@lists.linux.dev,
	xuanzhuo@linux.alibaba.com
Subject: Re: [PATCH v5 5/5] media: virtio: Add USERPTR memory type support
Date: Mon, 10 Aug 2026 14:21:43 +0300	[thread overview]
Message-ID: <20260810112143.GA2967445@killaraus.ideasonboard.com> (raw)
In-Reply-To: <20260723183219.737296-6-briandaniels@google.com>

On Thu, Jul 23, 2026 at 02:32:19PM -0400, Brian Daniels wrote:
> From: Alexandre Courbot <gnurou@gmail.com>
> 
> This patch adds support for the USERPTR memory type to the virtio-media
> driver.
> 
> It adds the allow_userptr module parameter, implements the userptr
> mapping logic in the scatterlist builder, and enables USERPTR in
> reqbufs if allowed.

USERPTR is deprecated, it shouldn't be used in new drivers or in new
userspace code.

> Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> Assisted-by: Antigravity:gemini-3.5-flash
> Co-developed-by: Brian Daniels <briandaniels@google.com>
> Signed-off-by: Brian Daniels <briandaniels@google.com>
> ---
>  drivers/media/virtio/scatterlist_builder.c | 64 ++++++++++++++++++++++
>  drivers/media/virtio/scatterlist_builder.h |  3 +
>  drivers/media/virtio/virtio_media_driver.c | 42 +++++++++-----
>  drivers/media/virtio/virtio_media_ioctls.c | 11 +++-
>  4 files changed, 104 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/virtio/scatterlist_builder.c b/drivers/media/virtio/scatterlist_builder.c
> index 97925b277..85c6a36b4 100644
> --- a/drivers/media/virtio/scatterlist_builder.c
> +++ b/drivers/media/virtio/scatterlist_builder.c
> @@ -349,14 +349,30 @@ static int scatterlist_builder_add_userptr(struct scatterlist_builder *builder,
>  int scatterlist_builder_add_buffer(struct scatterlist_builder *builder,
>  				   struct v4l2_buffer *b)
>  {
> +	int i;
>  	int ret;
>  
> +	/* Fixup: plane length must be zero if userptr is NULL */
> +	if (!V4L2_TYPE_IS_MULTIPLANAR(b->type) &&
> +	    b->memory == V4L2_MEMORY_USERPTR && b->m.userptr == 0)
> +		b->length = 0;
> +
>  	/* v4l2_buffer */
>  	ret = scatterlist_builder_add_data(builder, b, sizeof(*b));
>  	if (ret)
>  		return ret;
>  
>  	if (V4L2_TYPE_IS_MULTIPLANAR(b->type) && b->length > 0) {
> +		/* Fixup: plane length must be zero if userptr is NULL */
> +		if (b->memory == V4L2_MEMORY_USERPTR) {
> +			for (i = 0; i < b->length; i++) {
> +				struct v4l2_plane *plane = &b->m.planes[i];
> +
> +				if (plane->m.userptr == 0)
> +					plane->length = 0;
> +			}
> +		}
> +
>  		/* Array of v4l2_planes */
>  		ret = scatterlist_builder_add_data(builder, b->m.planes,
>  						   sizeof(struct v4l2_plane) *
> @@ -368,6 +384,54 @@ int scatterlist_builder_add_buffer(struct scatterlist_builder *builder,
>  	return 0;
>  }
>  
> +/**
> + * scatterlist_builder_add_buffer_userptr() - Add the payload of a ``USERPTR``
> + *                                            &struct v4l2_buffer to the
> + *                                            descriptor chain.
> + * @builder: builder to use.
> + * @b: &struct v4l2_buffer whose ``USERPTR`` payload we want to add.
> + *
> + * Add an array of &struct virtio_media_sg_entry pointing to a ``USERPTR``
> + * buffer's contents. Does nothing if the buffer is not of type ``USERPTR``.
> + * This is split out of scatterlist_builder_add_buffer() because we only want
> + * to add these to the device-readable part of the descriptor chain.
> + */
> +int scatterlist_builder_add_buffer_userptr(struct scatterlist_builder *builder,
> +					   struct v4l2_buffer *b)
> +{
> +	int i;
> +	int ret;
> +
> +	if (b->memory != V4L2_MEMORY_USERPTR)
> +		return 0;
> +
> +	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> +		for (i = 0; i < b->length; i++) {
> +			struct v4l2_plane *plane = &b->m.planes[i];
> +
> +			if (b->memory == V4L2_MEMORY_USERPTR &&
> +			    plane->length > 0) {
> +				unsigned long uptr = plane->m.userptr;
> +				unsigned long len = plane->length;
> +
> +				ret =
> +				scatterlist_builder_add_userptr(builder,
> +								uptr,
> +								len);
> +				if (ret)
> +					return ret;
> +			}
> +		}
> +	} else if (b->length > 0) {
> +		ret = scatterlist_builder_add_userptr(builder, b->m.userptr,
> +						      b->length);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * scatterlist_builder_retrieve_buffer() - Retrieve a &struct v4l2_buffer
>   *                                         written by the device on the shadow
> diff --git a/drivers/media/virtio/scatterlist_builder.h b/drivers/media/virtio/scatterlist_builder.h
> index 47bfd7ae0..53d964a48 100644
> --- a/drivers/media/virtio/scatterlist_builder.h
> +++ b/drivers/media/virtio/scatterlist_builder.h
> @@ -90,6 +90,9 @@ int scatterlist_builder_add_ioctl_resp(struct scatterlist_builder *builder,
>  int scatterlist_builder_add_buffer(struct scatterlist_builder *builder,
>  				   struct v4l2_buffer *buffer);
>  
> +int scatterlist_builder_add_buffer_userptr(struct scatterlist_builder *builder,
> +					   struct v4l2_buffer *b);
> +
>  int scatterlist_builder_retrieve_buffer(struct scatterlist_builder *builder,
>  					size_t sg_index,
>  					struct v4l2_buffer *buffer,
> diff --git a/drivers/media/virtio/virtio_media_driver.c b/drivers/media/virtio/virtio_media_driver.c
> index c431c3eb2..b6f79593d 100644
> --- a/drivers/media/virtio/virtio_media_driver.c
> +++ b/drivers/media/virtio/virtio_media_driver.c
> @@ -7,26 +7,29 @@
>   */
>  
>  #include <linux/bits.h>
> +#include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/dev_printk.h>
> +#include <linux/mm.h>
>  #include <linux/mutex.h>
> +#include <linux/scatterlist.h>
>  #include <linux/types.h>
> +#include <linux/videodev2.h>
> +#include <linux/vmalloc.h>
> +#include <linux/wait.h>
> +#include <linux/workqueue.h>
>  #include <linux/module.h>
> +#include <linux/moduleparam.h>
>  #include <linux/virtio.h>
>  #include <linux/virtio_config.h>
>  #include <linux/virtio_ids.h>
> -#include <linux/slab.h>
> -#include <linux/scatterlist.h>
> -#include <linux/vmalloc.h>
> -#include <linux/workqueue.h>
> -#include <linux/dma-mapping.h>
> -#include <linux/poll.h>
> -#include <linux/mm.h>
>  
> +#include <media/frame_vector.h>
>  #include <media/v4l2-dev.h>
> -#include <media/v4l2-device.h>
> -#include <media/v4l2-fh.h>
>  #include <media/v4l2-event.h>
> +#include <media/videobuf2-memops.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
>  
>  #include "uapi/linux/virtio_media.h"
>  #include "session.h"
> @@ -40,6 +43,15 @@
>  /* Bit mask for the VIRTIO_MEDIA_MMAP_FLAG_RW flag */
>  #define VIRTIO_MEDIA_MMAP_FLAG_RW_MASK BIT(VIRTIO_MEDIA_MMAP_FLAG_RW)
>  
> +/*
> + * Whether USERPTR buffers are allowed.
> + *
> + * This is disabled by default as USERPTR buffers are dangerous, but the option
> + * is left to enable them if desired.
> + */
> +bool virtio_media_allow_userptr;
> +module_param_named(allow_userptr, virtio_media_allow_userptr, bool, 0660);
> +
>  /**
>   * virtio_media_session_alloc() - Allocate a new session.
>   * @vv: virtio-media device the session belongs to.
> @@ -849,15 +861,11 @@ static int virtio_media_probe(struct virtio_device *virtio_dev)
>  			      VIRTIO_MEDIA_SHM_MMAP);
>  
>  	vd = &vv->video_dev;
> +
>  	vd->v4l2_dev = &vv->v4l2_dev;
>  	vd->vfl_type = VFL_TYPE_VIDEO;
>  	vd->ioctl_ops = &virtio_media_ioctl_ops;
>  	vd->fops = &virtio_media_fops;
> -	vd->release = video_device_release_empty;
> -	strscpy(vd->name, "virtio-media", sizeof(vd->name));
> -
> -	video_set_drvdata(vd, vv);
> -
>  	vd->device_caps = virtio_cread32(virtio_dev, 0);
>  	if (vd->device_caps & (V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE))
>  		vd->vfl_dir = VFL_DIR_M2M;
> @@ -866,6 +874,10 @@ static int virtio_media_probe(struct virtio_device *virtio_dev)
>  		vd->vfl_dir = VFL_DIR_TX;
>  	else
>  		vd->vfl_dir = VFL_DIR_RX;
> +	vd->release = video_device_release_empty;
> +	strscpy(vd->name, "virtio-media", sizeof(vd->name));
> +
> +	video_set_drvdata(vd, vv);
>  
>  	ret = video_register_device(vd, virtio_cread32(virtio_dev, 4), 0);
>  	if (ret)
> @@ -890,6 +902,7 @@ static int virtio_media_probe(struct virtio_device *virtio_dev)
>  	virtio_dev->config->del_vqs(virtio_dev);
>  err_find_vqs:
>  	v4l2_device_unregister(&vv->v4l2_dev);
> +
>  	return ret;
>  }
>  
> @@ -900,6 +913,7 @@ static void virtio_media_remove(struct virtio_device *virtio_dev)
>  
>  	cancel_work_sync(&vv->eventq_work);
>  	virtio_reset_device(virtio_dev);
> +
>  	v4l2_device_unregister(&vv->v4l2_dev);
>  	virtio_dev->config->del_vqs(virtio_dev);
>  	video_unregister_device(&vv->video_dev);
> diff --git a/drivers/media/virtio/virtio_media_ioctls.c b/drivers/media/virtio/virtio_media_ioctls.c
> index f0b82b5ec..88465f239 100644
> --- a/drivers/media/virtio/virtio_media_ioctls.c
> +++ b/drivers/media/virtio/virtio_media_ioctls.c
> @@ -273,6 +273,12 @@ static int virtio_media_send_buffer_ioctl(struct v4l2_fh *fh, u32 ioctl,
>  		return ret;
>  
>  	end_buf_sg = builder.cur_sg;
> +
> +	/* Payload of SHARED_PAGES buffers, if relevant */
> +	ret = scatterlist_builder_add_buffer_userptr(&builder, b);
> +	if (ret < 0)
> +		return ret;
> +
>  	num_cmd_sgs = builder.cur_sg;
>  
>  	/* Response descriptor */
> @@ -719,7 +725,7 @@ static int virtio_media_reqbufs(struct file *file, void *fh,
>  	if (b->type > VIRTIO_MEDIA_LAST_QUEUE)
>  		return -EINVAL;
>  
> -	if (b->memory == V4L2_MEMORY_USERPTR)
> +	if (b->memory == V4L2_MEMORY_USERPTR && !virtio_media_allow_userptr)
>  		return -EINVAL;
>  
>  	ret = virtio_media_send_wr_ioctl(vfh, VIDIOC_REQBUFS, b, sizeof(*b),
> @@ -752,7 +758,8 @@ static int virtio_media_reqbufs(struct file *file, void *fh,
>  	if (V4L2_TYPE_IS_MULTIPLANAR(b->type))
>  		session->uses_mplane = true;
>  
> -	b->capabilities &= ~V4L2_BUF_CAP_SUPPORTS_USERPTR;
> +	if (!virtio_media_allow_userptr)
> +		b->capabilities &= ~V4L2_BUF_CAP_SUPPORTS_USERPTR;
>  
>  	/* We do not support DMABUF yet. */
>  	b->capabilities &= ~V4L2_BUF_CAP_SUPPORTS_DMABUF;

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2026-08-10 11:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23 18:32 [PATCH v5 0/5] media: add virtio-media driver Brian Daniels
2026-07-23 18:32 ` [PATCH v5 1/5] media: virtio: Add skeleton " Brian Daniels
2026-07-23 18:32 ` [PATCH v5 2/5] media: virtio: Add session management Brian Daniels
2026-07-23 18:32 ` [PATCH v5 3/5] media: virtio: Add scatterlist builder Brian Daniels
2026-07-23 18:32 ` [PATCH v5 4/5] media: virtio: Add ioctl operations and driver logic Brian Daniels
2026-07-23 18:32 ` [PATCH v5 5/5] media: virtio: Add USERPTR memory type support Brian Daniels
2026-08-10 11:21   ` Laurent Pinchart [this message]
2026-08-11 16:27     ` Brian Daniels
2026-08-04 12:29 ` [PATCH v5 0/5] media: add virtio-media driver Albert Esteve
2026-08-04 16:08   ` Brian Daniels
     [not found]   ` <CAD4i_GR0Njq9T38VKJ1c39GW5=sFVvMCv=kSEjFpZ+XhWnqUZA@mail.gmail.com>
2026-08-05  7:14     ` Albert Esteve
2026-08-04 18:10 ` Brian Daniels
2026-08-04 20:47 ` Michael S. Tsirkin
2026-08-05  7:17   ` Albert Esteve
2026-08-05 16:27     ` Brian Daniels

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260810112143.GA2967445@killaraus.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=adelva@google.com \
    --cc=aesteve@redhat.com \
    --cc=briandaniels@google.com \
    --cc=changyeon@google.com \
    --cc=daniel.almeida@collabora.com \
    --cc=eperezma@redhat.com \
    --cc=gnurou@gmail.com \
    --cc=gurchetansingh@google.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mst@redhat.com \
    --cc=nicolas.dufresne@collabora.com \
    --cc=virtualization@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.