[PATCH v2 0/1] virtio-media: Add device specification

[PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

Hi,

This a formal attempt of including virtio-media
device specification.

Virtio-media came from a discussion on virtio-dev
mailing list, which lead to presenting virtio-v4l2[1]
specification as an alternative to virtio-video.

Later, virtio-v4l2 was renamed to virtio-media[2]
and published through:

https://github.com/chromeos/virtio-media

The repository above includes a virtio-media driver able
to pass v4l2-compliance when proxying the vivid/vicodec
virtual devices or an actual UVC camera using the
V4L2 vhost device (available in the repository).
Steps to reproduce are also detailed[3].

There is some overlap with virtio-video in regards
to which devices it can handle. However,
as virtio-media will likely be the virtualization
solution for ChromeOS (already landed into the chromeos
organization) and possibly other Google projects for
media devices, it would be desirable to include the
specification in the next virtio release despite
the aforementioned overlap.

The device ID in this document differs from
the ID in the virtio-media project repository.
And it will probably need some discussion on which
would be the correct definitive ID.

Full PDF: https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
PDF with the media section only: https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing

[1] https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
[2] https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
[3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md

Albert Esteve (1):
  virtio-media: Add virtio media device specification

 conformance.tex                           |  13 +-
 content.tex                               |   1 +
 device-types/media/description.tex        | 578 ++++++++++++++++++++++
 device-types/media/device-conformance.tex |  11 +
 device-types/media/driver-conformance.tex |   9 +
 5 files changed, 608 insertions(+), 4 deletions(-)
 create mode 100644 device-types/media/description.tex
 create mode 100644 device-types/media/device-conformance.tex
 create mode 100644 device-types/media/driver-conformance.tex

-- 
2.44.0

[PATCH v2 1/1] virtio-media: Add virtio media device specification

[Albert Esteve]

Virtio-media is an encapsulation of the V4L2 UAPI into
virtio, able to virtualize any video device supported
by V4L2

Note that virtio-media does not require the use of a
V4L2 device driver or of Linux on the host or
guest side - V4L2 is only used as a host-guest protocol,
and both sides are free to convert it from/to any
model that they wish to use.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 conformance.tex                           |  13 +-
 content.tex                               |   1 +
 device-types/media/description.tex        | 578 ++++++++++++++++++++++
 device-types/media/device-conformance.tex |  11 +
 device-types/media/driver-conformance.tex |   9 +
 5 files changed, 608 insertions(+), 4 deletions(-)
 create mode 100644 device-types/media/description.tex
 create mode 100644 device-types/media/device-conformance.tex
 create mode 100644 device-types/media/driver-conformance.tex

diff --git a/conformance.tex b/conformance.tex
index dc00e84..c369da1 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -32,8 +32,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
-\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
-\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
+\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
+\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
+\ref{sec:Conformance / Driver Conformance / Media Driver Conformance}.
+
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -59,8 +61,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
 \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
 \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
-\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
-\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
+\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
+\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
+\ref{sec:Conformance / Device Conformance / Media Device Conformance}.
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -152,6 +155,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \input{device-types/scmi/driver-conformance.tex}
 \input{device-types/gpio/driver-conformance.tex}
 \input{device-types/pmem/driver-conformance.tex}
+\input{device-types/media/driver-conformance.tex}
 
 \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
 
@@ -238,6 +242,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \input{device-types/scmi/device-conformance.tex}
 \input{device-types/gpio/device-conformance.tex}
 \input{device-types/pmem/device-conformance.tex}
+\input{device-types/media/device-conformance.tex}
 
 \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
 A conformant implementation MUST be either transitional or
diff --git a/content.tex b/content.tex
index 0a62dce..59925ae 100644
--- a/content.tex
+++ b/content.tex
@@ -767,6 +767,7 @@ \chapter{Device Types}\label{sec:Device Types}
 \input{device-types/scmi/description.tex}
 \input{device-types/gpio/description.tex}
 \input{device-types/pmem/description.tex}
+\input{device-types/media/description.tex}
 
 \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
diff --git a/device-types/media/description.tex b/device-types/media/description.tex
new file mode 100644
index 0000000..887eacf
--- /dev/null
+++ b/device-types/media/description.tex
@@ -0,0 +1,578 @@
+\section{Media Device}\label{sec:Device Types / Media Device}
+
+The virtio media device follow the same model (and structures) as V4L2. It
+can be used to virtualize cameras, codec devices, or any other device
+supported by V4L2. The device assumes 64-bit little-endian V4L2 structures
+are exchanged. The complete definition of V4L2 structures and ioctls can
+be found under the
+\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
+
+V4L2 is a UAPI that allows a less privileged entity (user-space) to use video
+hardware exposed by a more privileged entity (the kernel). Virtio-media is an
+encapsulation of this API into virtio, turning it into a virtualization API
+for all classes of video devices supported by V4L2, where the device plays the
+role of the kernel and the driver the role of user-space.
+
+The device is therefore responsible for presenting a virtual device that behaves
+like an actual V4L2 device, which the driver can control.
+
+Note that virtio-media does not require the use of a V4L2 device driver or of
+Linux on any side - V4L2 is only used as a transport protocol,
+and both sides are free to convert it from/to any model that they wish to use.
+
+This section relies on definitions from
+\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
+
+\subsection{Device ID}\label{sec:Device Types / Media Device / Device ID}
+
+42
+
+\subsection{Virtqueues}\label{sec:Device Types / Media Device / Virtqueues}
+
+\begin{description}
+\item[0] commandq - used for driver commands and device responses to these
+commands.
+\item[1] eventq - used for events sent by the device to the driver.
+\end{description}
+
+\devicenormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
+
+The device MUST return the descriptor chains it receives on the commandq as
+soon as possible, and must never hold them for indefinite periods of time.
+
+\drivernormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
+
+The driver MUST re-queue the descriptor chains returned by the device on the
+eventq as soon as possible, and must never hold them for indefinite periods
+of time.
+
+\subsection{Feature Bits}\label{sec:Device Types / Media Device / Feature Bits}
+
+None
+
+\subsection{Device Configuration Layout}\label{sec:Device Types / Media Device / Device Configuration Layout}
+
+The video device configuration space uses the following layout:
+
+\begin{lstlisting}
+struct virtio_media_config {
+    le32 device_caps;
+    le32 device_type;
+    u8 card[32];
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{device_caps}] (driver-read-only) flags representing the device
+capabilities as used in
+\href{https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-querycap.html#c.v4l2_capability}{struct v4l2_capabilities}.
+Corresponds with the \field{device_caps} field in the \textit{struct video_device}.
+\item[\field{device_type}] (driver-read-only) informs the driver of the type
+of the video device. Corresponds with the \field{vfl_devnode_type} field of the device.
+\item[\field{card}] (driver-read-only) name of the device, a NUL-terminated
+UTF-8 string. Corresponds with the \field{card} field of the \textit{struct v4l2_capability}.
+If all the characters of the field are used, it does not need to be NUL-terminated.
+\end{description}
+
+\subsection{Device Initialization}
+
+\begin{enumerate}
+\item The driver reads the \field{device_caps} and \field{device_type} fields
+from the configuration layout to identify the device.
+\item The driver sets up the \field{commandq} and \field{eventq}.
+\item The driver may open a session to use the device and send V4L2 ioctls in
+order to receive more information about the device, such as supported
+formats or controls.
+\end{enumerate}
+
+\subsection{Device Operation}\label{sec:Device Types / Media Device / Device Operation}
+
+Commands are queued on the command queue by the driver for the device to
+process. The errors returned by each command are standard
+\href{https://www.kernel.org/doc/html/latest/userspace-api/media/gen-errors.html}{Linux kernel error codes}.
+For instance, a command that contains invalid options will return \textit{EINVAL}.
+
+Events are sent on the event queue by the device for the driver to handle.
+
+\subsubsection{Command Virtqueue}
+
+\paragraph{Device Operation: Command headers}
+
+\begin{lstlisting}
+#define VIRTIO_MEDIA_CMD_OPEN 1
+#define VIRTIO_MEDIA_CMD_CLOSE 2
+#define VIRTIO_MEDIA_CMD_IOCTL 3
+#define VIRTIO_MEDIA_CMD_MMAP 4
+#define VIRTIO_MEDIA_CMD_MUNMAP 5
+
+/* Header for all virtio commands from the driver to the device on the commandq. */
+struct virtio_media_cmd_header {
+	u32 cmd;
+	u32 __padding;
+};
+
+/* Header for all virtio responses from the device to the driver on the commandq. */
+struct virtio_media_resp_header {
+	u32 status;
+	u32 __padding;
+};
+\end{lstlisting}
+
+A command consists of a command header \textit{virtio_media_cmd_header}
+containing the following device-readable field:
+
+\begin{description}
+\item[\field{cmd}] specifies a device request type (VIRTIO_MEDIA_CMD_*).
+\end{description}
+
+A response consists of a response header \textit{virtio_media_resp_header}
+containing the following device-writable field:
+
+\begin{description}
+\item[\field{status}] indicates a device request status.
+\end{description}
+
+The status field can take 0 if the command was successful, or one of the
+standard Linux error codes if it was not.
+
+\drivernormative{\paragraph}{Device Operation: Command Virtqueue: Sessions}{Device Types / Media Device / Device Operation / Command Virtqueue}
+
+Sessions are how the device is multiplexed, allowing several distinct works to
+take place simultaneously. The driver needs to open a session before it can
+perform any useful operation on the device.
+
+\paragraph{Device Operation: Open device}
+
+\textbf{VIRTIO_MEDIA_CMD_OPEN} Command for creating a new session.
+
+This is the equivalent of calling \textit{open} on a V4L2 device node.
+The driver uses \textit{virtio_media_cmd_open} to send an open request.
+
+\begin{lstlisting}
+struct virtio_media_cmd_open {
+    struct virtio_media_cmd_header hdr;
+};
+\end{lstlisting}
+
+The device responds to \textit{VIRTIO_MEDIA_CMD_OPEN} with \textit{virtio_media_resp_open}.
+
+\begin{lstlisting}
+struct virtio_media_resp_open {
+    struct virtio_media_resp_header hdr;
+    u32 session_id;
+    u32 __padding;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{session_id}] specifies an identifier for the current session. The
+identifier can be used to perform other commands on the session, notably ioctls.
+\end{description}
+
+\devicenormative{\subparagraph}{Device Operation: Open device}{Device Types / Media Device / Device Operation / Open device}
+
+Upon success, the device MUST set a \field{session_id} in \textit{virtio_media_resp_open}
+to an integer that is NOT used by any other open session.
+
+\paragraph{Device Operation: Close device}
+
+\textbf{VIRTIO_MEDIA_CMD_CLOSE} Command for closing an active session.
+
+This is the equivalent of calling \textit{close} on a previously opened V4L2
+device node. All resources associated with this session will be freed.
+
+This command does not require a response from the device.
+
+\begin{lstlisting}
+struct virtio_media_cmd_close {
+    struct virtio_media_cmd_header hdr;
+    u32 session_id;
+    u32 __padding;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{session_id}] specifies an identifier for the session to close.
+\end{description}
+
+\drivernormative{\subparagraph}{Device Operation: Close device}{Device Types / Media Device / Device Operation / Close device}
+
+The session ID SHALL NOT be used again after queueing this command.
+
+\paragraph{Device Operation: V4L2 ioctls}
+
+\textbf{VIRTIO_MEDIA_CMD_IOCTL} Command for executing an ioctl on an open
+session.
+
+This command asks the device to run one of the `VIDIOC_*` ioctls on the active
+session.
+
+\begin{lstlisting}
+struct virtio_media_cmd_ioctl {
+    struct virtio_media_cmd_header hdr;
+    u32 session_id;
+    u32 code;
+    /* Followed by the relevant ioctl payload as defined in the macro */
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{session_id}] specifies an identifier of thesession to run the ioctl on.
+\item[\field{code}] specifies the code of the \field{VIDIOC_*} ioctl to run.
+\end{description}
+
+The code is extracted from the
+\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/videodev.html}{videodev2.h},
+header file. The file defines the ioctl's codes, type of payload, and
+direction. The code consists of the second argument of the \field{_IO*} macro.
+
+For example, the \textit{VIDIOC_G_FMT} is defined as follows:
+
+\begin{lstlisting}
+#define VIDIOC_G_FMT _IOWR('V',  4, struct v4l2_format)
+\end{lstlisting}
+
+This means that its ioctl code is \textit{4}, that its payload is a
+\textit{struct v4l2_format}, and that its direction is \textit{WR} (i.e., the
+payload is written by both the driver and the device).
+See Section \ref{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
+for more information about the direction of ioctls.
+
+The payload layout is always a 64-bit representation of the corresponding
+V4L2 structure.
+
+The device responds to \textit{VIRTIO_MEDIA_CMD_IOCTL} with \textit{virtio_media_resp_ioctl}.
+
+\begin{lstlisting}
+struct virtio_media_resp_ioctl {
+    struct virtio_media_resp_header hdr;
+    /* Followed by the ioctl payload as defined in the macro */
+};
+\end{lstlisting}
+
+\subparagraph{Ioctls payload}\label{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
+
+Each ioctl has a payload, which is defined by the third argument of the
+\field{_IO*} macro defining it. 
+
+The payload of an ioctl in the descriptor chain follows the command structure,
+the reponse structure, or both depending on the direction:
+
+\begin{itemize}
+\item \textbf{_IOR} is read-only for the driver, meaning the payload
+follows the response in the device-writable section of the descriptor chain.
+\item \textbf{_IOW} is read-only for the device, meaning the payload
+follows the command in the driver-writable section of the descriptor chain.
+\item \textbf{_IOWR} is writable by both the device and driver,
+meaning the payload must follow both the command in the driver-writable section
+of the descriptor chain, and the response in the device-writable section.
+\end{itemize}
+
+A common optimization for \textit{WR} ioctls is to provide the payload using
+descriptors that both point to the same buffer. This mimics the behavior of
+V4L2 ioctls where the data is only passed once and used as both input and
+output by the kernel.
+
+\devicenormative{\subparagraph}{Device Operation: V4L2 ioctls}{Device Types / Media Device / Device Operation / V4L2 ioctls}
+
+In case of success of a device-writable ioctl, the device MUST always write the
+payload in the device-writable part of the descriptor chain.
+
+In case of failure of a device-writable ioctl, the device is free to write the
+payload in the device-writable part of the descriptor chain or not. Some errors
+may still result in the payload being updated, and in this case the device is
+expected to write the updated payload. If the device has not written the
+payload after an error, the driver MUST assume that the payload has not been
+modified.
+
+\subparagraph{Handling of pointers in ioctl payload}
+
+A few structures used as ioctl payloads contain pointers to further
+data needed for the ioctl. There are notably:
+
+\begin{itemize}
+\item The \field{planes} pointer of
+\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer}{struct v4l2_buffer},
+which size is determined by the length member.
+\item The \field{controls} pointer of \textit{struct v4l2_ext_controls}, which
+size is determined by the count member.
+\end{itemize}
+
+If the size of the pointed area is determined to be non-zero, then the main
+payload is immediately followed by the pointed data in their order of
+appearance in the structure, and the pointer value itself is ignored by the
+device, which must also return the value initially passed by the driver.
+
+\subparagraph{Handling of pointers to userspace memory}
+\label{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
+
+A few pointers are special in that they point to userspace memory in the
+original V4L2 specification. They are:
+
+\begin{itemize}
+\item The \field{m.userptr} member of \textit{struct v4l2_buffer} and
+\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-plane}{struct v4l2_plane}
+(technically an unsigned long, but designated a userspace address).
+\item The \field{ptr} member of \textit{struct v4l2_ext_ctrl}.
+\end{itemize}
+
+These pointers can cover large areas of scattered memory, which has the
+potential to require more descriptors than the virtio queue can provide. For
+these particular pointers only, a list of \textit{struct virtio_media_sg_entry}
+that covers the needed amount of memory for the pointer is used instead of
+using descriptors to map the pointed memory directly.
+
+\begin{lstlisting}
+struct virtio_media_sg_entry {
+	u64 start;
+	u32 len;
+	u32 __padding;
+};
+\end{lstlisting}
+
+For each such pointer to read, the device reads as many SG entries as needed
+to cover the length of the pointed buffer, as described by its parent
+structure (\field{length} member of \textit{struct v4l2_buffer} or
+\textit{struct v4l2_plane} for buffer memory, and \field{size} member of
+\textit{struct v4l2_ext_control} for control data).
+
+Since the device never needs to modify the list of SG entries, it is only
+provided by the driver in the device-readable section of the descriptor chain,
+and not repeated in the device-writable section, even for WR ioctls.
+
+\subparagraph{Unsupported ioctls}
+
+A few ioctls are replaced by other, more suitable mechanisms.
+
+\begin{itemize}
+\item \textit{VIDIOC_QUERYCAP} is replaced by reading the configuration area
+(see \ref{sec:Device Types / Media Device / Device Configuration Layout}).
+\item \textit{VIDIOC_DQBUF} is replaced by a dedicated event
+(see \ref{sec:Device Types / Media Device / Device Operation / Dequeue buffer}).
+\item \textit{VIDIOC_DQEVENT} is replaced by a dedicated event
+(see \ref{sec:Device Types / Media Device / Device Operation / Emit an event}).
+\item \textit{VIDIOC_G_JPEGCOMP} and \textit{VIDIOC_S_JPEGCOMP} are deprecated
+and replaced by the controls of the JPEG class.
+\item \textit{VIDIOC_LOG_STATUS} is a driver-only operation and shall not be
+implemented by the device.
+\end{itemize}
+
+\devicenormative{\subparagraph}{Device Operation: Unsupported ioctls}{Device Types / Media Device / Device Operation / Unsupported ioctls}
+
+If being requested an unsupported ioctl, the device MUST return the same
+error response as it would for an unknown ioctl, i.e. \textit{ENOTTY}.
+
+\paragraph{Device Operation: Mapping a MMAP buffer}
+
+\textbf{VIRTIO_MEDIA_CMD_MMAP} Command for mapping a MMAP buffer into the
+driver's address space.
+
+Shared memory region ID 0 is used to map MMAP buffers with
+the \textit{VIRTIO_MEDIA_CMD_MMAP} command.
+
+\begin{lstlisting}
+#define VIRTIO_MEDIA_MMAP_FLAG_RW (1 << 0)
+
+struct virtio_media_cmd_mmap {
+	struct virtio_media_cmd_header hdr;
+	u32 session_id;
+	u32 flags;
+	u64 offset;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{flags}] is the set of flags for the mapping. \field{VIRTIO_MEDIA_MMAP_FLAG_RW}
+can be set if a read-write mapping is desired. Without this flag the mapping
+will be read-only.
+\item[\field{offset}] corresponds to the \field{mem_offset} field of the
+\textit{union v4l2_plane} for the plane to map. This field can be obtained
+using the \textit{VIDIOC_QUERYBUF} ioctl.
+\end{description}
+
+The device responds to \textit{VIRTIO_MEDIA_CMD_MMAP} with \textit{virtio_media_resp_mmap}.
+
+\begin{lstlisting}
+struct virtio_media_resp_mmap {
+    struct virtio_media_resp_header hdr;
+    u64 offset;
+    u64 len;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{offset}] offset into SHM region ID 0 of the start of the mapping.
+\item[\field{len}] length of the mapping as indicated by the \textit{struct v4l2_plane}
+the buffer belongs to.
+\end{description}
+
+\paragraph{Device Operation: Unmapping a MMAP buffer}
+
+\textbf{VIRTIO_MEDIA_CMD_MUNMAP} Unmap a MMAP buffer previously mapped using \field{VIRTIO_MEDIA_CMD_MMAP}.
+
+\begin{lstlisting}
+struct virtio_media_cmd_munmap {
+    struct virtio_media_cmd_header hdr;
+    u64 offset;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{offset}] offset into SHM region ID 0 previously returned by 
+\textit{VIRTIO_MEDIA_CMD_MMAP} at which the buffer has been previously mapped.
+\end{description}
+
+The device responds to \textit{VIRTIO_MEDIA_CMD_MUNMAP} with \textit{virtio_media_resp_munmap}.
+
+\begin{lstlisting}
+struct virtio_media_resp_munmap {
+    struct virtio_media_resp_header hdr;
+};
+\end{lstlisting}
+
+\devicenormative{\subparagraph}{Device Operation: Unmapping a MMAP buffer}{Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
+
+The device MUST keep mappings performed using \textit{VIRTIO_MEDIA_CMD_MMAP}
+valid until \textit{VIRTIO_MEDIA_CMD_MUNMAP} is called, even if the buffers or
+session they belong to are released or closed by the driver.
+
+\paragraph{Device Operation: Memory Types}
+
+The semantics of the three V4L2 memory types (\textit{MMAP}, \textit{USERPTR}
+and \textit{DMABUF}) can easily be mapped to both driver and device context.
+
+\subparagraph{MMAP}
+
+In virtio-media, \textit{MMAP} buffers are provisioned by the device, just like
+they are by the kernel in regular V4L2. Similarly to how userspace can map a
+\textit{MMAP} buffer into its address space using mmap and munmap, the
+virtio-media driver can map device buffers into the driver space by queueing the
+\textit{struct virtio_media_cmd_mmap} and \textit{struct virtio_media_cmd_munmap}
+commands to the commandq.
+
+\subparagraph{USERPTR}
+
+In virtio-media, \textit{USERPTR} buffers are provisioned by the driver, just
+like they are by userspace in regular V4L2. Instances of \textit{struct v4l2_buffer}
+and \textit{struct v4l2_plane} of this type are followed by a list of
+\textit{struct virtio_media_sg_entry}. For more information, see
+\ref{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
+
+The device must not alter the pointer values provided by the driver, i.e.
+\field{the m.userptr} member of \textit{struct v4l2_buffer} and
+\textit{struct v4l2_plane} must be returned to the driver with the same value
+as it was provided.
+
+\subparagraph{DMABUF}
+
+In virtio-media, \textit{DMABUF} buffers are provisioned by a virtio object,
+just like they are by a \textit{DMABUF} in regular V4L2. Virtio objects are
+16-bytes UUIDs and do not fit in the placeholders for file descriptors, so
+they follow their embedding data structure as needed and the device must
+leave the V4L2 structure placeholder unchanged.
+
+Contrary to \textit{USERPTR} buffers, virtio objects UUIDs need to be added in
+both the device-readable and device-writable section of the descriptor chain.
+
+Device-allocated buffers with the \textit{V4L2_MEMORY_MMAP} memory type can also
+be exported as virtio objects for use with another virtio device using the
+\textit{VIDIOC_EXPBUF} ioctl. The fd placefolder of \textit{v4l2_exportbuffer}
+means that space for the UUID needs to be reserved right after that structure
+
+\subsubsection{Event Virtqueue}
+
+Events are a way for the device to inform the driver about asynchronous events
+that it should know about. In virtio-media, they are used as a replacement for
+the \textit{VIDIOC_DQBUF} and \textit{VIDIOC_DQEVENT} ioctls and the polling
+mechanism, which would be impractical to implement on top of virtio.
+
+\paragraph{Device Operation: Event header}
+
+\begin{lstlisting}
+#define VIRTIO_MEDIA_EVT_ERROR 0
+#define VIRTIO_MEDIA_EVT_DQBUF 1
+#define VIRTIO_MEDIA_EVT_EVENT 2
+
+/* Header for events queued by the device for the driver on the eventq. */
+struct virtio_media_event_header {
+    u32 event;
+    u32 session_id;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{event}] one of \field{VIRTIO_MEDIA_EVT_*}.
+\item[\field{session_id}] ID of the session the event applies to.
+\end{description}
+
+\paragraph{Device Operation: Device-side error}
+
+\textbf{VIRTIO_MEDIA_EVT_ERROR} Upon receiving this event, the session
+mentioned in the header is considered corrupted and automatically closed by
+the device.
+
+\begin{lstlisting}
+struct virtio_media_event_error {
+    struct virtio_media_event_header hdr;
+    u32 errno;
+    u32 __padding;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{errno}] error code describing the kind of error that occurred.
+\end{description}
+
+\paragraph{Device Operation: Dequeue buffer}
+\label{sec:Device Types / Media Device / Device Operation / Dequeue buffer}
+
+\textbf{VIRTIO_MEDIA_EVT_DQBUF} Signals that a buffer is not being used anymore
+by the device and is returned to the driver.
+
+A \textit{struct virtio_media_event_dqbuf} event is queued on the eventq by the
+device every time a buffer previously queued using the \textit{VIDIOC_QBUF}
+ioctl is done being processed and can be used by the driver again. This is like
+an implicit \textit{VIDIOC_DQBUF} ioctl.
+
+\begin{lstlisting}
+struct virtio_media_event_dqbuf {
+    struct virtio_media_event_header hdr;
+    struct v4l2_buffer buffer;
+    struct v4l2_plane planes[VIDEO_MAX_PLANES];
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{buffer}] \textit{struct v4l2_buffer} describing the buffer that has been dequeued.
+\item[\field{planes}] array of \textit{struct v4l2_plane} containing the plane information for multi-planar buffers.
+\end{description}
+
+Pointer values in the \textit{struct v4l2_buffer} and \textit{struct v4l2_plane}
+are meaningless and must be ignored by the driver. It is recommended that the
+device sets them to NULL in order to avoid leaking potential device addresses.
+
+Note that in the case of a \field{USERPTR} buffer, the \textit{struct v4l2_buffer}
+used as event payload is not followed by the buffer memory: since that memory
+is the same that the driver submitted with the \textit{VIDIOC_QBUF}, it would
+be redundant to have it here.
+
+\paragraph{Device Operation: Emit an event}
+\label{sec:Device Types / Media Device / Device Operation / Emit an event}
+
+\textbf{VIRTIO_MEDIA_EVT_EVENT} Signals that a V4L2 event has been emitted for a session.
+
+A \textit{struct virtio_media_event_event} event is queued on the eventq by the
+device every time an event the driver previously subscribed to using the
+\textit{VIDIOC_SUBSCRIBE_EVENT} ioctl has been signaled. This is like an
+implicit \textit{VIDIOC_DQEVENT} ioctl.
+
+\begin{lstlisting}
+struct virtio_media_event_event {
+    struct virtio_media_event_header hdr;
+    struct v4l2_event event;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{event}] \textit{struct v4l2_event} describing the event that occurred.
+\end{description}
diff --git a/device-types/media/device-conformance.tex b/device-types/media/device-conformance.tex
new file mode 100644
index 0000000..3338822
--- /dev/null
+++ b/device-types/media/device-conformance.tex
@@ -0,0 +1,11 @@
+\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Device Conformance / Media Device Conformance}
+
+A Media device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / Media Device / Virtqueues}
+\item \ref{devicenormative:Device Types / Media Device / Device Operation / Open device}
+\item \ref{devicenormative:Device Types / Media Device / Device Operation / V4L2 ioctls}
+\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unsupported ioctls}
+\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
+\end{itemize}
\ No newline at end of file
diff --git a/device-types/media/driver-conformance.tex b/device-types/media/driver-conformance.tex
new file mode 100644
index 0000000..058b812
--- /dev/null
+++ b/device-types/media/driver-conformance.tex
@@ -0,0 +1,9 @@
+\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Driver Conformance / Media Driver Conformance}
+
+A Media device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / Media Device / Virtqueues}
+\item \ref{drivernormative:Device Types / Media Device / Device Operation / Command Virtqueue}
+\item \ref{drivernormative:Device Types / Media Device / Device Operation / Close device}
+\end{itemize}
\ No newline at end of file
-- 
2.44.0

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

[-- Attachment #1: Type: text/plain, Size: 2796 bytes --]

Sorry, forgot the changeset. I will collect this for the next version.

v1->v2:
- Remove naming host/guest in the text
- Explicitly specify endian-ness of the device
- Change address by offset in the MMAP operation
- Specify SHM region for MMAP operation


On Fri, Jun 7, 2024 at 10:00 AM Albert Esteve <aesteve@redhat.com> wrote:

> Hi,
>
> This a formal attempt of including virtio-media
> device specification.
>
> Virtio-media came from a discussion on virtio-dev
> mailing list, which lead to presenting virtio-v4l2[1]
> specification as an alternative to virtio-video.
>
> Later, virtio-v4l2 was renamed to virtio-media[2]
> and published through:
>
> https://github.com/chromeos/virtio-media
>
> The repository above includes a virtio-media driver able
> to pass v4l2-compliance when proxying the vivid/vicodec
> virtual devices or an actual UVC camera using the
> V4L2 vhost device (available in the repository).
> Steps to reproduce are also detailed[3].
>
> There is some overlap with virtio-video in regards
> to which devices it can handle. However,
> as virtio-media will likely be the virtualization
> solution for ChromeOS (already landed into the chromeos
> organization) and possibly other Google projects for
> media devices, it would be desirable to include the
> specification in the next virtio release despite
> the aforementioned overlap.
>
> The device ID in this document differs from
> the ID in the virtio-media project repository.
> And it will probably need some discussion on which
> would be the correct definitive ID.
>
> Full PDF:
> https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
> PDF with the media section only:
> https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing
>
> [1]
> https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
> [2]
> https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
> [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
>
> Albert Esteve (1):
>   virtio-media: Add virtio media device specification
>
>  conformance.tex                           |  13 +-
>  content.tex                               |   1 +
>  device-types/media/description.tex        | 578 ++++++++++++++++++++++
>  device-types/media/device-conformance.tex |  11 +
>  device-types/media/driver-conformance.tex |   9 +
>  5 files changed, 608 insertions(+), 4 deletions(-)
>  create mode 100644 device-types/media/description.tex
>  create mode 100644 device-types/media/device-conformance.tex
>  create mode 100644 device-types/media/driver-conformance.tex
>
> --
> 2.44.0
>
>

[-- Attachment #2: Type: text/html, Size: 4563 bytes --]

Re: [PATCH v2 1/1] virtio-media: Add virtio media device specification

[Hans Verkuil]

Hi Albert,


On 07/06/2024 10:00, Albert Esteve wrote:
> Virtio-media is an encapsulation of the V4L2 UAPI into
> virtio, able to virtualize any video device supported
> by V4L2
> 
> Note that virtio-media does not require the use of a
> V4L2 device driver or of Linux on the host or
> guest side - V4L2 is only used as a host-guest protocol,
> and both sides are free to convert it from/to any
> model that they wish to use.

I just have two notes, one minor, one more substantial:

> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  conformance.tex                           |  13 +-
>  content.tex                               |   1 +
>  device-types/media/description.tex        | 578 ++++++++++++++++++++++
>  device-types/media/device-conformance.tex |  11 +
>  device-types/media/driver-conformance.tex |   9 +
>  5 files changed, 608 insertions(+), 4 deletions(-)
>  create mode 100644 device-types/media/description.tex
>  create mode 100644 device-types/media/device-conformance.tex
>  create mode 100644 device-types/media/driver-conformance.tex
> 
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84..c369da1 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -32,8 +32,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
> +\ref{sec:Conformance / Driver Conformance / Media Driver Conformance}.
> +
>  
>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>    \end{itemize}
> @@ -59,8 +61,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
>  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
>  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
> +\ref{sec:Conformance / Device Conformance / Media Device Conformance}.
>  
>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>    \end{itemize}
> @@ -152,6 +155,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \input{device-types/scmi/driver-conformance.tex}
>  \input{device-types/gpio/driver-conformance.tex}
>  \input{device-types/pmem/driver-conformance.tex}
> +\input{device-types/media/driver-conformance.tex}
>  
>  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>  
> @@ -238,6 +242,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \input{device-types/scmi/device-conformance.tex}
>  \input{device-types/gpio/device-conformance.tex}
>  \input{device-types/pmem/device-conformance.tex}
> +\input{device-types/media/device-conformance.tex}
>  
>  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
>  A conformant implementation MUST be either transitional or
> diff --git a/content.tex b/content.tex
> index 0a62dce..59925ae 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -767,6 +767,7 @@ \chapter{Device Types}\label{sec:Device Types}
>  \input{device-types/scmi/description.tex}
>  \input{device-types/gpio/description.tex}
>  \input{device-types/pmem/description.tex}
> +\input{device-types/media/description.tex}
>  
>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  
> diff --git a/device-types/media/description.tex b/device-types/media/description.tex
> new file mode 100644
> index 0000000..887eacf
> --- /dev/null
> +++ b/device-types/media/description.tex
> @@ -0,0 +1,578 @@
> +\section{Media Device}\label{sec:Device Types / Media Device}
> +
> +The virtio media device follow the same model (and structures) as V4L2. It
> +can be used to virtualize cameras, codec devices, or any other device
> +supported by V4L2. The device assumes 64-bit little-endian V4L2 structures
> +are exchanged. The complete definition of V4L2 structures and ioctls can
> +be found under the
> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
> +
> +V4L2 is a UAPI that allows a less privileged entity (user-space) to use video
> +hardware exposed by a more privileged entity (the kernel). Virtio-media is an
> +encapsulation of this API into virtio, turning it into a virtualization API
> +for all classes of video devices supported by V4L2, where the device plays the
> +role of the kernel and the driver the role of user-space.
> +
> +The device is therefore responsible for presenting a virtual device that behaves
> +like an actual V4L2 device, which the driver can control.
> +
> +Note that virtio-media does not require the use of a V4L2 device driver or of
> +Linux on any side - V4L2 is only used as a transport protocol,
> +and both sides are free to convert it from/to any model that they wish to use.
> +
> +This section relies on definitions from
> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
> +
> +\subsection{Device ID}\label{sec:Device Types / Media Device / Device ID}
> +
> +42
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Media Device / Virtqueues}
> +
> +\begin{description}
> +\item[0] commandq - used for driver commands and device responses to these
> +commands.
> +\item[1] eventq - used for events sent by the device to the driver.
> +\end{description}
> +
> +\devicenormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
> +
> +The device MUST return the descriptor chains it receives on the commandq as
> +soon as possible, and must never hold them for indefinite periods of time.
> +
> +\drivernormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
> +
> +The driver MUST re-queue the descriptor chains returned by the device on the
> +eventq as soon as possible, and must never hold them for indefinite periods
> +of time.
> +
> +\subsection{Feature Bits}\label{sec:Device Types / Media Device / Feature Bits}
> +
> +None
> +
> +\subsection{Device Configuration Layout}\label{sec:Device Types / Media Device / Device Configuration Layout}
> +
> +The video device configuration space uses the following layout:
> +
> +\begin{lstlisting}
> +struct virtio_media_config {
> +    le32 device_caps;
> +    le32 device_type;
> +    u8 card[32];

Use char instead of u8. It's always been a pain that struct v4l2_capability
used u8 instead of char for the character arrays. I never understood why, and
if you are making a new struct I would recommend using char.

Up to you, though.

> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{device_caps}] (driver-read-only) flags representing the device
> +capabilities as used in
> +\href{https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-querycap.html#c.v4l2_capability}{struct v4l2_capabilities}.
> +Corresponds with the \field{device_caps} field in the \textit{struct video_device}.
> +\item[\field{device_type}] (driver-read-only) informs the driver of the type
> +of the video device. Corresponds with the \field{vfl_devnode_type} field of the device.
> +\item[\field{card}] (driver-read-only) name of the device, a NUL-terminated
> +UTF-8 string. Corresponds with the \field{card} field of the \textit{struct v4l2_capability}.
> +If all the characters of the field are used, it does not need to be NUL-terminated.
> +\end{description}
> +
> +\subsection{Device Initialization}
> +
> +\begin{enumerate}
> +\item The driver reads the \field{device_caps} and \field{device_type} fields
> +from the configuration layout to identify the device.
> +\item The driver sets up the \field{commandq} and \field{eventq}.
> +\item The driver may open a session to use the device and send V4L2 ioctls in
> +order to receive more information about the device, such as supported
> +formats or controls.
> +\end{enumerate}
> +
> +\subsection{Device Operation}\label{sec:Device Types / Media Device / Device Operation}
> +
> +Commands are queued on the command queue by the driver for the device to
> +process. The errors returned by each command are standard
> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/gen-errors.html}{Linux kernel error codes}.
> +For instance, a command that contains invalid options will return \textit{EINVAL}.
> +
> +Events are sent on the event queue by the device for the driver to handle.
> +
> +\subsubsection{Command Virtqueue}
> +
> +\paragraph{Device Operation: Command headers}
> +
> +\begin{lstlisting}
> +#define VIRTIO_MEDIA_CMD_OPEN 1
> +#define VIRTIO_MEDIA_CMD_CLOSE 2
> +#define VIRTIO_MEDIA_CMD_IOCTL 3
> +#define VIRTIO_MEDIA_CMD_MMAP 4
> +#define VIRTIO_MEDIA_CMD_MUNMAP 5
> +
> +/* Header for all virtio commands from the driver to the device on the commandq. */
> +struct virtio_media_cmd_header {
> +	u32 cmd;
> +	u32 __padding;
> +};
> +
> +/* Header for all virtio responses from the device to the driver on the commandq. */
> +struct virtio_media_resp_header {
> +	u32 status;
> +	u32 __padding;
> +};
> +\end{lstlisting}
> +
> +A command consists of a command header \textit{virtio_media_cmd_header}
> +containing the following device-readable field:
> +
> +\begin{description}
> +\item[\field{cmd}] specifies a device request type (VIRTIO_MEDIA_CMD_*).
> +\end{description}
> +
> +A response consists of a response header \textit{virtio_media_resp_header}
> +containing the following device-writable field:
> +
> +\begin{description}
> +\item[\field{status}] indicates a device request status.
> +\end{description}
> +
> +The status field can take 0 if the command was successful, or one of the
> +standard Linux error codes if it was not.
> +
> +\drivernormative{\paragraph}{Device Operation: Command Virtqueue: Sessions}{Device Types / Media Device / Device Operation / Command Virtqueue}
> +
> +Sessions are how the device is multiplexed, allowing several distinct works to
> +take place simultaneously. The driver needs to open a session before it can
> +perform any useful operation on the device.
> +
> +\paragraph{Device Operation: Open device}
> +
> +\textbf{VIRTIO_MEDIA_CMD_OPEN} Command for creating a new session.
> +
> +This is the equivalent of calling \textit{open} on a V4L2 device node.
> +The driver uses \textit{virtio_media_cmd_open} to send an open request.
> +
> +\begin{lstlisting}
> +struct virtio_media_cmd_open {
> +    struct virtio_media_cmd_header hdr;
> +};
> +\end{lstlisting}
> +
> +The device responds to \textit{VIRTIO_MEDIA_CMD_OPEN} with \textit{virtio_media_resp_open}.
> +
> +\begin{lstlisting}
> +struct virtio_media_resp_open {
> +    struct virtio_media_resp_header hdr;
> +    u32 session_id;
> +    u32 __padding;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{session_id}] specifies an identifier for the current session. The
> +identifier can be used to perform other commands on the session, notably ioctls.
> +\end{description}
> +
> +\devicenormative{\subparagraph}{Device Operation: Open device}{Device Types / Media Device / Device Operation / Open device}
> +
> +Upon success, the device MUST set a \field{session_id} in \textit{virtio_media_resp_open}
> +to an integer that is NOT used by any other open session.
> +
> +\paragraph{Device Operation: Close device}
> +
> +\textbf{VIRTIO_MEDIA_CMD_CLOSE} Command for closing an active session.
> +
> +This is the equivalent of calling \textit{close} on a previously opened V4L2
> +device node. All resources associated with this session will be freed.
> +
> +This command does not require a response from the device.
> +
> +\begin{lstlisting}
> +struct virtio_media_cmd_close {
> +    struct virtio_media_cmd_header hdr;
> +    u32 session_id;
> +    u32 __padding;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{session_id}] specifies an identifier for the session to close.
> +\end{description}
> +
> +\drivernormative{\subparagraph}{Device Operation: Close device}{Device Types / Media Device / Device Operation / Close device}
> +
> +The session ID SHALL NOT be used again after queueing this command.
> +
> +\paragraph{Device Operation: V4L2 ioctls}
> +
> +\textbf{VIRTIO_MEDIA_CMD_IOCTL} Command for executing an ioctl on an open
> +session.
> +
> +This command asks the device to run one of the `VIDIOC_*` ioctls on the active
> +session.
> +
> +\begin{lstlisting}
> +struct virtio_media_cmd_ioctl {
> +    struct virtio_media_cmd_header hdr;
> +    u32 session_id;
> +    u32 code;
> +    /* Followed by the relevant ioctl payload as defined in the macro */
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{session_id}] specifies an identifier of thesession to run the ioctl on.
> +\item[\field{code}] specifies the code of the \field{VIDIOC_*} ioctl to run.
> +\end{description}
> +
> +The code is extracted from the
> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/videodev.html}{videodev2.h},
> +header file. The file defines the ioctl's codes, type of payload, and
> +direction. The code consists of the second argument of the \field{_IO*} macro.
> +
> +For example, the \textit{VIDIOC_G_FMT} is defined as follows:
> +
> +\begin{lstlisting}
> +#define VIDIOC_G_FMT _IOWR('V',  4, struct v4l2_format)
> +\end{lstlisting}
> +
> +This means that its ioctl code is \textit{4}, that its payload is a
> +\textit{struct v4l2_format}, and that its direction is \textit{WR} (i.e., the
> +payload is written by both the driver and the device).
> +See Section \ref{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
> +for more information about the direction of ioctls.
> +
> +The payload layout is always a 64-bit representation of the corresponding
> +V4L2 structure.
> +
> +The device responds to \textit{VIRTIO_MEDIA_CMD_IOCTL} with \textit{virtio_media_resp_ioctl}.
> +
> +\begin{lstlisting}
> +struct virtio_media_resp_ioctl {
> +    struct virtio_media_resp_header hdr;
> +    /* Followed by the ioctl payload as defined in the macro */
> +};
> +\end{lstlisting}
> +
> +\subparagraph{Ioctls payload}\label{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
> +
> +Each ioctl has a payload, which is defined by the third argument of the
> +\field{_IO*} macro defining it. 
> +
> +The payload of an ioctl in the descriptor chain follows the command structure,
> +the reponse structure, or both depending on the direction:
> +
> +\begin{itemize}
> +\item \textbf{_IOR} is read-only for the driver, meaning the payload
> +follows the response in the device-writable section of the descriptor chain.
> +\item \textbf{_IOW} is read-only for the device, meaning the payload
> +follows the command in the driver-writable section of the descriptor chain.
> +\item \textbf{_IOWR} is writable by both the device and driver,
> +meaning the payload must follow both the command in the driver-writable section
> +of the descriptor chain, and the response in the device-writable section.
> +\end{itemize}
> +
> +A common optimization for \textit{WR} ioctls is to provide the payload using
> +descriptors that both point to the same buffer. This mimics the behavior of
> +V4L2 ioctls where the data is only passed once and used as both input and
> +output by the kernel.
> +
> +\devicenormative{\subparagraph}{Device Operation: V4L2 ioctls}{Device Types / Media Device / Device Operation / V4L2 ioctls}
> +
> +In case of success of a device-writable ioctl, the device MUST always write the
> +payload in the device-writable part of the descriptor chain.
> +
> +In case of failure of a device-writable ioctl, the device is free to write the
> +payload in the device-writable part of the descriptor chain or not. Some errors
> +may still result in the payload being updated, and in this case the device is
> +expected to write the updated payload. If the device has not written the
> +payload after an error, the driver MUST assume that the payload has not been
> +modified.
> +
> +\subparagraph{Handling of pointers in ioctl payload}
> +
> +A few structures used as ioctl payloads contain pointers to further
> +data needed for the ioctl. There are notably:
> +
> +\begin{itemize}
> +\item The \field{planes} pointer of
> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer}{struct v4l2_buffer},
> +which size is determined by the length member.
> +\item The \field{controls} pointer of \textit{struct v4l2_ext_controls}, which
> +size is determined by the count member.
> +\end{itemize}
> +
> +If the size of the pointed area is determined to be non-zero, then the main
> +payload is immediately followed by the pointed data in their order of
> +appearance in the structure, and the pointer value itself is ignored by the
> +device, which must also return the value initially passed by the driver.
> +
> +\subparagraph{Handling of pointers to userspace memory}
> +\label{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
> +
> +A few pointers are special in that they point to userspace memory in the
> +original V4L2 specification. They are:
> +
> +\begin{itemize}
> +\item The \field{m.userptr} member of \textit{struct v4l2_buffer} and
> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-plane}{struct v4l2_plane}
> +(technically an unsigned long, but designated a userspace address).
> +\item The \field{ptr} member of \textit{struct v4l2_ext_ctrl}.
> +\end{itemize}
> +
> +These pointers can cover large areas of scattered memory, which has the

Actually, only m.userptr can cover a large area. The other two are limited.
See my comment about USERPTR support below.

> +potential to require more descriptors than the virtio queue can provide. For
> +these particular pointers only, a list of \textit{struct virtio_media_sg_entry}
> +that covers the needed amount of memory for the pointer is used instead of
> +using descriptors to map the pointed memory directly.
> +
> +\begin{lstlisting}
> +struct virtio_media_sg_entry {
> +	u64 start;
> +	u32 len;
> +	u32 __padding;
> +};
> +\end{lstlisting}
> +
> +For each such pointer to read, the device reads as many SG entries as needed
> +to cover the length of the pointed buffer, as described by its parent
> +structure (\field{length} member of \textit{struct v4l2_buffer} or
> +\textit{struct v4l2_plane} for buffer memory, and \field{size} member of
> +\textit{struct v4l2_ext_control} for control data).
> +
> +Since the device never needs to modify the list of SG entries, it is only
> +provided by the driver in the device-readable section of the descriptor chain,
> +and not repeated in the device-writable section, even for WR ioctls.
> +
> +\subparagraph{Unsupported ioctls}
> +
> +A few ioctls are replaced by other, more suitable mechanisms.
> +
> +\begin{itemize}
> +\item \textit{VIDIOC_QUERYCAP} is replaced by reading the configuration area
> +(see \ref{sec:Device Types / Media Device / Device Configuration Layout}).
> +\item \textit{VIDIOC_DQBUF} is replaced by a dedicated event
> +(see \ref{sec:Device Types / Media Device / Device Operation / Dequeue buffer}).
> +\item \textit{VIDIOC_DQEVENT} is replaced by a dedicated event
> +(see \ref{sec:Device Types / Media Device / Device Operation / Emit an event}).
> +\item \textit{VIDIOC_G_JPEGCOMP} and \textit{VIDIOC_S_JPEGCOMP} are deprecated
> +and replaced by the controls of the JPEG class.
> +\item \textit{VIDIOC_LOG_STATUS} is a driver-only operation and shall not be
> +implemented by the device.
> +\end{itemize}
> +
> +\devicenormative{\subparagraph}{Device Operation: Unsupported ioctls}{Device Types / Media Device / Device Operation / Unsupported ioctls}
> +
> +If being requested an unsupported ioctl, the device MUST return the same
> +error response as it would for an unknown ioctl, i.e. \textit{ENOTTY}.
> +
> +\paragraph{Device Operation: Mapping a MMAP buffer}
> +
> +\textbf{VIRTIO_MEDIA_CMD_MMAP} Command for mapping a MMAP buffer into the
> +driver's address space.
> +
> +Shared memory region ID 0 is used to map MMAP buffers with
> +the \textit{VIRTIO_MEDIA_CMD_MMAP} command.
> +
> +\begin{lstlisting}
> +#define VIRTIO_MEDIA_MMAP_FLAG_RW (1 << 0)
> +
> +struct virtio_media_cmd_mmap {
> +	struct virtio_media_cmd_header hdr;
> +	u32 session_id;
> +	u32 flags;
> +	u64 offset;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{flags}] is the set of flags for the mapping. \field{VIRTIO_MEDIA_MMAP_FLAG_RW}
> +can be set if a read-write mapping is desired. Without this flag the mapping
> +will be read-only.
> +\item[\field{offset}] corresponds to the \field{mem_offset} field of the
> +\textit{union v4l2_plane} for the plane to map. This field can be obtained
> +using the \textit{VIDIOC_QUERYBUF} ioctl.
> +\end{description}
> +
> +The device responds to \textit{VIRTIO_MEDIA_CMD_MMAP} with \textit{virtio_media_resp_mmap}.
> +
> +\begin{lstlisting}
> +struct virtio_media_resp_mmap {
> +    struct virtio_media_resp_header hdr;
> +    u64 offset;
> +    u64 len;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{offset}] offset into SHM region ID 0 of the start of the mapping.
> +\item[\field{len}] length of the mapping as indicated by the \textit{struct v4l2_plane}
> +the buffer belongs to.
> +\end{description}
> +
> +\paragraph{Device Operation: Unmapping a MMAP buffer}
> +
> +\textbf{VIRTIO_MEDIA_CMD_MUNMAP} Unmap a MMAP buffer previously mapped using \field{VIRTIO_MEDIA_CMD_MMAP}.
> +
> +\begin{lstlisting}
> +struct virtio_media_cmd_munmap {
> +    struct virtio_media_cmd_header hdr;
> +    u64 offset;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{offset}] offset into SHM region ID 0 previously returned by 
> +\textit{VIRTIO_MEDIA_CMD_MMAP} at which the buffer has been previously mapped.
> +\end{description}
> +
> +The device responds to \textit{VIRTIO_MEDIA_CMD_MUNMAP} with \textit{virtio_media_resp_munmap}.
> +
> +\begin{lstlisting}
> +struct virtio_media_resp_munmap {
> +    struct virtio_media_resp_header hdr;
> +};
> +\end{lstlisting}
> +
> +\devicenormative{\subparagraph}{Device Operation: Unmapping a MMAP buffer}{Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
> +
> +The device MUST keep mappings performed using \textit{VIRTIO_MEDIA_CMD_MMAP}
> +valid until \textit{VIRTIO_MEDIA_CMD_MUNMAP} is called, even if the buffers or
> +session they belong to are released or closed by the driver.
> +
> +\paragraph{Device Operation: Memory Types}
> +
> +The semantics of the three V4L2 memory types (\textit{MMAP}, \textit{USERPTR}
> +and \textit{DMABUF}) can easily be mapped to both driver and device context.
> +
> +\subparagraph{MMAP}
> +
> +In virtio-media, \textit{MMAP} buffers are provisioned by the device, just like
> +they are by the kernel in regular V4L2. Similarly to how userspace can map a
> +\textit{MMAP} buffer into its address space using mmap and munmap, the
> +virtio-media driver can map device buffers into the driver space by queueing the
> +\textit{struct virtio_media_cmd_mmap} and \textit{struct virtio_media_cmd_munmap}
> +commands to the commandq.
> +
> +\subparagraph{USERPTR}
> +
> +In virtio-media, \textit{USERPTR} buffers are provisioned by the driver, just
> +like they are by userspace in regular V4L2. Instances of \textit{struct v4l2_buffer}
> +and \textit{struct v4l2_plane} of this type are followed by a list of
> +\textit{struct virtio_media_sg_entry}. For more information, see
> +\ref{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
> +
> +The device must not alter the pointer values provided by the driver, i.e.
> +\field{the m.userptr} member of \textit{struct v4l2_buffer} and
> +\textit{struct v4l2_plane} must be returned to the driver with the same value
> +as it was provided.

I very, very strongly recommend that you drop USERPTR support for virtio.
MMAP and DMABUF are sufficient.

It's a pain to support, and we discourage it for new drivers.

If memory serves, there is at least one chromeos driver that is stuck to
USERPTR support for some reason (I can't remember the details), and that is
(I think) the only reason USERPTR support is part of virtio. But that is
really a chromeos issue that they need to fix, IMHO.

Regards,

	Hans

> +
> +\subparagraph{DMABUF}
> +
> +In virtio-media, \textit{DMABUF} buffers are provisioned by a virtio object,
> +just like they are by a \textit{DMABUF} in regular V4L2. Virtio objects are
> +16-bytes UUIDs and do not fit in the placeholders for file descriptors, so
> +they follow their embedding data structure as needed and the device must
> +leave the V4L2 structure placeholder unchanged.
> +
> +Contrary to \textit{USERPTR} buffers, virtio objects UUIDs need to be added in
> +both the device-readable and device-writable section of the descriptor chain.
> +
> +Device-allocated buffers with the \textit{V4L2_MEMORY_MMAP} memory type can also
> +be exported as virtio objects for use with another virtio device using the
> +\textit{VIDIOC_EXPBUF} ioctl. The fd placefolder of \textit{v4l2_exportbuffer}
> +means that space for the UUID needs to be reserved right after that structure
> +
> +\subsubsection{Event Virtqueue}
> +
> +Events are a way for the device to inform the driver about asynchronous events
> +that it should know about. In virtio-media, they are used as a replacement for
> +the \textit{VIDIOC_DQBUF} and \textit{VIDIOC_DQEVENT} ioctls and the polling
> +mechanism, which would be impractical to implement on top of virtio.
> +
> +\paragraph{Device Operation: Event header}
> +
> +\begin{lstlisting}
> +#define VIRTIO_MEDIA_EVT_ERROR 0
> +#define VIRTIO_MEDIA_EVT_DQBUF 1
> +#define VIRTIO_MEDIA_EVT_EVENT 2
> +
> +/* Header for events queued by the device for the driver on the eventq. */
> +struct virtio_media_event_header {
> +    u32 event;
> +    u32 session_id;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{event}] one of \field{VIRTIO_MEDIA_EVT_*}.
> +\item[\field{session_id}] ID of the session the event applies to.
> +\end{description}
> +
> +\paragraph{Device Operation: Device-side error}
> +
> +\textbf{VIRTIO_MEDIA_EVT_ERROR} Upon receiving this event, the session
> +mentioned in the header is considered corrupted and automatically closed by
> +the device.
> +
> +\begin{lstlisting}
> +struct virtio_media_event_error {
> +    struct virtio_media_event_header hdr;
> +    u32 errno;
> +    u32 __padding;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{errno}] error code describing the kind of error that occurred.
> +\end{description}
> +
> +\paragraph{Device Operation: Dequeue buffer}
> +\label{sec:Device Types / Media Device / Device Operation / Dequeue buffer}
> +
> +\textbf{VIRTIO_MEDIA_EVT_DQBUF} Signals that a buffer is not being used anymore
> +by the device and is returned to the driver.
> +
> +A \textit{struct virtio_media_event_dqbuf} event is queued on the eventq by the
> +device every time a buffer previously queued using the \textit{VIDIOC_QBUF}
> +ioctl is done being processed and can be used by the driver again. This is like
> +an implicit \textit{VIDIOC_DQBUF} ioctl.
> +
> +\begin{lstlisting}
> +struct virtio_media_event_dqbuf {
> +    struct virtio_media_event_header hdr;
> +    struct v4l2_buffer buffer;
> +    struct v4l2_plane planes[VIDEO_MAX_PLANES];
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{buffer}] \textit{struct v4l2_buffer} describing the buffer that has been dequeued.
> +\item[\field{planes}] array of \textit{struct v4l2_plane} containing the plane information for multi-planar buffers.
> +\end{description}
> +
> +Pointer values in the \textit{struct v4l2_buffer} and \textit{struct v4l2_plane}
> +are meaningless and must be ignored by the driver. It is recommended that the
> +device sets them to NULL in order to avoid leaking potential device addresses.
> +
> +Note that in the case of a \field{USERPTR} buffer, the \textit{struct v4l2_buffer}
> +used as event payload is not followed by the buffer memory: since that memory
> +is the same that the driver submitted with the \textit{VIDIOC_QBUF}, it would
> +be redundant to have it here.
> +
> +\paragraph{Device Operation: Emit an event}
> +\label{sec:Device Types / Media Device / Device Operation / Emit an event}
> +
> +\textbf{VIRTIO_MEDIA_EVT_EVENT} Signals that a V4L2 event has been emitted for a session.
> +
> +A \textit{struct virtio_media_event_event} event is queued on the eventq by the
> +device every time an event the driver previously subscribed to using the
> +\textit{VIDIOC_SUBSCRIBE_EVENT} ioctl has been signaled. This is like an
> +implicit \textit{VIDIOC_DQEVENT} ioctl.
> +
> +\begin{lstlisting}
> +struct virtio_media_event_event {
> +    struct virtio_media_event_header hdr;
> +    struct v4l2_event event;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{event}] \textit{struct v4l2_event} describing the event that occurred.
> +\end{description}
> diff --git a/device-types/media/device-conformance.tex b/device-types/media/device-conformance.tex
> new file mode 100644
> index 0000000..3338822
> --- /dev/null
> +++ b/device-types/media/device-conformance.tex
> @@ -0,0 +1,11 @@
> +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Device Conformance / Media Device Conformance}
> +
> +A Media device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / Media Device / Virtqueues}
> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Open device}
> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / V4L2 ioctls}
> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unsupported ioctls}
> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
> +\end{itemize}
> \ No newline at end of file
> diff --git a/device-types/media/driver-conformance.tex b/device-types/media/driver-conformance.tex
> new file mode 100644
> index 0000000..058b812
> --- /dev/null
> +++ b/device-types/media/driver-conformance.tex
> @@ -0,0 +1,9 @@
> +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Driver Conformance / Media Driver Conformance}
> +
> +A Media device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{drivernormative:Device Types / Media Device / Virtqueues}
> +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Command Virtqueue}
> +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Close device}
> +\end{itemize}
> \ No newline at end of file

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Michael S. Tsirkin]

On Fri, Jun 07, 2024 at 10:00:44AM +0200, Albert Esteve wrote:
> Hi,
> 
> This a formal attempt of including virtio-media
> device specification.

This was sent to a wrong mailing list.
Please repost to the correct list.

Albert, when you subscribed to virtio-dev you should
have received a message that read, in part:

When to use this list:
- questions and change proposals for Virtio drivers and devices
  implementing the specification.

When not to use this list:
- questions and change proposals for the Virtio specification,
  including the specification of basic functionality, transports and
  devices (please use the virtio-comment mailing list
  [mailto:virtio-comment@lists.linux.dev] for this).

To do:
- send email preferably in text format (best for archiving)

Not to do:
- copy both virtio-dev and virtio-comment mailing lists (instead, pick
  one)
- send full copies of the virtio spec (in any format)



So what made you send a specification change proposal to
virtio-dev, and also copy the defunct, non-functional
list at virtio-dev@lists.oasis-open.org ?


Was the message unclear? If yes I'd like to improve it.


-- 
MST

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On Fri, Jun 7, 2024 at 11:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Fri, Jun 07, 2024 at 10:00:44AM +0200, Albert Esteve wrote:
> > Hi,
> >
> > This a formal attempt of including virtio-media
> > device specification.
>
> This was sent to a wrong mailing list.
> Please repost to the correct list.
>
> Albert, when you subscribed to virtio-dev you should
> have received a message that read, in part:
>
> When to use this list:
> - questions and change proposals for Virtio drivers and devices
>   implementing the specification.
>
> When not to use this list:
> - questions and change proposals for the Virtio specification,
>   including the specification of basic functionality, transports and
>   devices (please use the virtio-comment mailing list
>   [mailto:virtio-comment@lists.linux.dev] for this).
>
> To do:
> - send email preferably in text format (best for archiving)
>
> Not to do:
> - copy both virtio-dev and virtio-comment mailing lists (instead, pick
>   one)
> - send full copies of the virtio spec (in any format)
>
>
>
> So what made you send a specification change proposal to
> virtio-dev, and also copy the defunct, non-functional
> list at virtio-dev@lists.oasis-open.org ?
>
>
> Was the message unclear? If yes I'd like to improve it.
>

My bad.
The instructions were clear, and I set up the mail to be sent to the
correct virtio-dev mailing list address. But I had the configuration
of the project set to automatically add both of the old addresses to the
patch. I only noticed after sending, so the damage was already
made.

I will fix it for the next drop.


>
>
> --
> MST
>
>

[-- Attachment #2: Type: text/html, Size: 2544 bytes --]

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Michael S. Tsirkin]

On Fri, Jun 07, 2024 at 11:48:36AM +0200, Albert Esteve wrote:
> 
> 
> On Fri, Jun 7, 2024 at 11:41 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Fri, Jun 07, 2024 at 10:00:44AM +0200, Albert Esteve wrote:
>     > Hi,
>     >
>     > This a formal attempt of including virtio-media
>     > device specification.
> 
>     This was sent to a wrong mailing list.
>     Please repost to the correct list.
> 
>     Albert, when you subscribed to virtio-dev you should
>     have received a message that read, in part:
> 
>     When to use this list:
>     - questions and change proposals for Virtio drivers and devices
>       implementing the specification.
> 
>     When not to use this list:
>     - questions and change proposals for the Virtio specification,
>       including the specification of basic functionality, transports and
>       devices (please use the virtio-comment mailing list
>       [mailto:virtio-comment@lists.linux.dev] for this).
> 
>     To do:
>     - send email preferably in text format (best for archiving)
> 
>     Not to do:
>     - copy both virtio-dev and virtio-comment mailing lists (instead, pick
>       one)
>     - send full copies of the virtio spec (in any format)
> 
> 
> 
>     So what made you send a specification change proposal to
>     virtio-dev, and also copy the defunct, non-functional
>     list at virtio-dev@lists.oasis-open.org ?
> 
> 
>     Was the message unclear? If yes I'd like to improve it.
> 
> 
> My bad.
> The instructions were clear, and I set up the mail to be sent to the
> correct virtio-dev mailing list address.


Hmm no, it's actually virtio-comment@lists.linux.dev for change
proposals for the Virtio specification which is what you have here.


> But I had the configuration
> of the project set to automatically add both of the old addresses to the
> patch. I only noticed after sending, so the damage was already
> made. 
> 
> I will fix it for the next drop.
>  
> 
> 
> 
>     --
>     MST
> 
> 

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

[-- Attachment #1: Type: text/plain, Size: 2304 bytes --]

On Fri, Jun 7, 2024 at 12:22 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Fri, Jun 07, 2024 at 11:48:36AM +0200, Albert Esteve wrote:
> >
> >
> > On Fri, Jun 7, 2024 at 11:41 AM Michael S. Tsirkin <mst@redhat.com>
> wrote:
> >
> >     On Fri, Jun 07, 2024 at 10:00:44AM +0200, Albert Esteve wrote:
> >     > Hi,
> >     >
> >     > This a formal attempt of including virtio-media
> >     > device specification.
> >
> >     This was sent to a wrong mailing list.
> >     Please repost to the correct list.
> >
> >     Albert, when you subscribed to virtio-dev you should
> >     have received a message that read, in part:
> >
> >     When to use this list:
> >     - questions and change proposals for Virtio drivers and devices
> >       implementing the specification.
> >
> >     When not to use this list:
> >     - questions and change proposals for the Virtio specification,
> >       including the specification of basic functionality, transports and
> >       devices (please use the virtio-comment mailing list
> >       [mailto:virtio-comment@lists.linux.dev] for this).
> >
> >     To do:
> >     - send email preferably in text format (best for archiving)
> >
> >     Not to do:
> >     - copy both virtio-dev and virtio-comment mailing lists (instead,
> pick
> >       one)
> >     - send full copies of the virtio spec (in any format)
> >
> >
> >
> >     So what made you send a specification change proposal to
> >     virtio-dev, and also copy the defunct, non-functional
> >     list at virtio-dev@lists.oasis-open.org ?
> >
> >
> >     Was the message unclear? If yes I'd like to improve it.
> >
> >
> > My bad.
> > The instructions were clear, and I set up the mail to be sent to the
> > correct virtio-dev mailing list address.
>
>
> Hmm no, it's actually virtio-comment@lists.linux.dev for change
> proposals for the Virtio specification which is what you have here.
>
>
Ok, then I'll repost to virtio-comment.


>
> > But I had the configuration
> > of the project set to automatically add both of the old addresses to the
> > patch. I only noticed after sending, so the damage was already
> > made.
> >
> > I will fix it for the next drop.
> >
> >
> >
> >
> >     --
> >     MST
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3567 bytes --]

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Alexander Gordeev]

Hi Albert,

On 07.06.24 10:00, Albert Esteve wrote:
> Hi,
> 
> This a formal attempt of including virtio-media
> device specification.
> 
> Virtio-media came from a discussion on virtio-dev
> mailing list, which lead to presenting virtio-v4l2[1]
> specification as an alternative to virtio-video.
> 
> Later, virtio-v4l2 was renamed to virtio-media[2]
> and published through:
> 
> https://github.com/chromeos/virtio-media
> 
> The repository above includes a virtio-media driver able
> to pass v4l2-compliance when proxying the vivid/vicodec
> virtual devices or an actual UVC camera using the
> V4L2 vhost device (available in the repository).
> Steps to reproduce are also detailed[3].
> 
> There is some overlap with virtio-video in regards
> to which devices it can handle. However,
> as virtio-media will likely be the virtualization
> solution for ChromeOS (already landed into the chromeos
> organization) and possibly other Google projects for
> media devices, it would be desirable to include the
> specification in the next virtio release despite
> the aforementioned overlap.
> 
> The device ID in this document differs from
> the ID in the virtio-media project repository.
> And it will probably need some discussion on which
> would be the correct definitive ID.
> 
> Full PDF: https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
> PDF with the media section only: https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing
> 
> [1] https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
> [2] https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
> [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
> 
> Albert Esteve (1):
>    virtio-media: Add virtio media device specification
> 
>   conformance.tex                           |  13 +-
>   content.tex                               |   1 +
>   device-types/media/description.tex        | 578 ++++++++++++++++++++++
>   device-types/media/device-conformance.tex |  11 +
>   device-types/media/driver-conformance.tex |   9 +
>   5 files changed, 608 insertions(+), 4 deletions(-)
>   create mode 100644 device-types/media/description.tex
>   create mode 100644 device-types/media/device-conformance.tex
>   create mode 100644 device-types/media/driver-conformance.tex

I'd like to add some general considerations:

1. virtio-media device capability discovery is very chatty

V4L2 requires potentially hundreds of system calls to get the full view 
of the device capabilities. This is inherited by virtio-media.
AFAIU V4L2 developers agree there is room for enhancement here, see [1], 
[2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were 
added for similar reasons.

 From the point of view of virtualization developers like my colleagues 
and me hundreds of hypervisor "exits" are excessive and costly. It can 
noticeably increase boot times, which is something we fight against in 
automotive. AFAIU other virtio developers agree with this, see [3].

In contrast virtio-video has been doing this in one command from day 
one. Yes, the data was incomplete. That's why in draft v9 I added TLV as 
a way to store all possible capabilities. I hope the linux-media folks 
could take a look on it. Maybe this is something, that can be adopted in 
V4L2?

2. virtio-media has a hard dependency on V4L2

There are certainly some "patches" on top of V4L2 in virtio-media, like 
the representation of the extended controls (which actually looks 
similar to the representation of the controls in virtio-video) or 
VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your 
own ioctl ids in virtio-media?

AFAIK the linux-media maintainers have been overloaded for years, see 
[4]. Would they be happy to deal with the additional requirements? Would 
virtio community like to have a dependency here?

2.1. an example: format modifiers

There is a patchset aiming at unifying V4L2 pixel formats and extending 
them with DRM format modifiers. It is now at version 7 submitted in 
2023, see [5]. The first version was submitted in 2019, see [6]. Not 
merged yet AFAIK.

In virtio-video I just added them in draft v9.

I'm absolutely not trying to criticize here. I just try to highlight 
that there is a lot of legacy and the process is painful. Right now we 
have an opportunity to design a new API according to the current state 
of the art of the stateful codecs.

3. uncertainty with cameras

AFAIK there is still no agreement about how cameras should be 
virtualized, see [3], [7], [8], [9]. virtio-media provides support for 
cameras in a specific way, which might not be desirable.

4. (minor) is it possible/hard to implement the device in hardware/on a 
micro controller?

This is something I thought about recently, there might be a use-case 
for it in the future. One of the concerns is that dynamic memory 
allocations are IMHO inevitable in virtio-media or virtio-video up to 
draft v8. I think multiple virtqueues in virtio-video draft v9 would 
help here. Not sure yet...

There are also other minor concerns, that are probably tolerable.

[1] 
https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
[2] Page 6 in 
https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
[3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
[4] 
https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
[5] 
https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
[6] 
https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
[7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
[8] 
https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
[9] 
https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/

-- 
Alexander Gordeev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin
www.opensynergy.com

[PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

Repost to fix the mailing list to which this is
sent to. Sorry for the noise.

Check previous thread here so that no responses
are lost:
https://lore.kernel.org/virtio-dev/b2abecb4-03a2-49ce-bfc3-2d95eb7a6956@opensynergy.com/T/#t

This an attempt of including virtio-media
device specification.

v1->v2:
- Remove naming host/guest in the text
- Explicitly specify endian-ness of the device
- Change address by offset in the MMAP operation
- Specify SHM region for MMAP operation

Virtio-media came from a discussion on virtio-dev
mailing list, which lead to presenting virtio-v4l2[1]
specification as an alternative to virtio-video.

Later, virtio-v4l2 was renamed to virtio-media[2]
and published through:

https://github.com/chromeos/virtio-media

The repository above includes a virtio-media driver able
to pass v4l2-compliance when proxying the vivid/vicodec
virtual devices or an actual UVC camera using the
V4L2 vhost device (available in the repository).
Steps to reproduce are also detailed[3].

There is some overlap with virtio-video in regards
to which devices it can handle. However,
as virtio-media will likely be the virtualization
solution for ChromeOS (already landed into the chromeos
organization) and possibly other Google projects for
media devices, it would be desirable to include the
specification in the next virtio release despite
the aforementioned overlap.

The device ID in this document differs from
the ID in the virtio-media project repository.
And it will probably need some discussion on which
would be the correct definitive ID.

Full PDF: https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
PDF with the media section only: https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing

[1] https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
[2] https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
[3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md

Albert Esteve (1):
  virtio-media: Add virtio media device specification

 conformance.tex                           |  13 +-
 content.tex                               |   1 +
 device-types/media/description.tex        | 578 ++++++++++++++++++++++
 device-types/media/device-conformance.tex |  11 +
 device-types/media/driver-conformance.tex |   9 +
 5 files changed, 608 insertions(+), 4 deletions(-)
 create mode 100644 device-types/media/description.tex
 create mode 100644 device-types/media/device-conformance.tex
 create mode 100644 device-types/media/driver-conformance.tex

-- 
2.44.0

Re: [PATCH v2 1/1] virtio-media: Add virtio media device specification

[Alexandre Courbot]

Hi Hans,

On Fri, Jun 7, 2024 at 5:43 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Albert,
>
>
> On 07/06/2024 10:00, Albert Esteve wrote:
> > Virtio-media is an encapsulation of the V4L2 UAPI into
> > virtio, able to virtualize any video device supported
> > by V4L2
> >
> > Note that virtio-media does not require the use of a
> > V4L2 device driver or of Linux on the host or
> > guest side - V4L2 is only used as a host-guest protocol,
> > and both sides are free to convert it from/to any
> > model that they wish to use.
>
> I just have two notes, one minor, one more substantial:
>
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >  conformance.tex                           |  13 +-
> >  content.tex                               |   1 +
> >  device-types/media/description.tex        | 578 ++++++++++++++++++++++
> >  device-types/media/device-conformance.tex |  11 +
> >  device-types/media/driver-conformance.tex |   9 +
> >  5 files changed, 608 insertions(+), 4 deletions(-)
> >  create mode 100644 device-types/media/description.tex
> >  create mode 100644 device-types/media/device-conformance.tex
> >  create mode 100644 device-types/media/driver-conformance.tex
> >
> > diff --git a/conformance.tex b/conformance.tex
> > index dc00e84..c369da1 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -32,8 +32,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
> >  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
> >  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
> > -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
> > -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
> > +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
> > +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
> > +\ref{sec:Conformance / Driver Conformance / Media Driver Conformance}.
> > +
> >
> >      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
> >    \end{itemize}
> > @@ -59,8 +61,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
> >  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
> >  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
> > -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
> > -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
> > +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
> > +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
> > +\ref{sec:Conformance / Device Conformance / Media Device Conformance}.
> >
> >      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
> >    \end{itemize}
> > @@ -152,6 +155,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \input{device-types/scmi/driver-conformance.tex}
> >  \input{device-types/gpio/driver-conformance.tex}
> >  \input{device-types/pmem/driver-conformance.tex}
> > +\input{device-types/media/driver-conformance.tex}
> >
> >  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
> >
> > @@ -238,6 +242,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \input{device-types/scmi/device-conformance.tex}
> >  \input{device-types/gpio/device-conformance.tex}
> >  \input{device-types/pmem/device-conformance.tex}
> > +\input{device-types/media/device-conformance.tex}
> >
> >  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
> >  A conformant implementation MUST be either transitional or
> > diff --git a/content.tex b/content.tex
> > index 0a62dce..59925ae 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -767,6 +767,7 @@ \chapter{Device Types}\label{sec:Device Types}
> >  \input{device-types/scmi/description.tex}
> >  \input{device-types/gpio/description.tex}
> >  \input{device-types/pmem/description.tex}
> > +\input{device-types/media/description.tex}
> >
> >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >
> > diff --git a/device-types/media/description.tex b/device-types/media/description.tex
> > new file mode 100644
> > index 0000000..887eacf
> > --- /dev/null
> > +++ b/device-types/media/description.tex
> > @@ -0,0 +1,578 @@
> > +\section{Media Device}\label{sec:Device Types / Media Device}
> > +
> > +The virtio media device follow the same model (and structures) as V4L2. It
> > +can be used to virtualize cameras, codec devices, or any other device
> > +supported by V4L2. The device assumes 64-bit little-endian V4L2 structures
> > +are exchanged. The complete definition of V4L2 structures and ioctls can
> > +be found under the
> > +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
> > +
> > +V4L2 is a UAPI that allows a less privileged entity (user-space) to use video
> > +hardware exposed by a more privileged entity (the kernel). Virtio-media is an
> > +encapsulation of this API into virtio, turning it into a virtualization API
> > +for all classes of video devices supported by V4L2, where the device plays the
> > +role of the kernel and the driver the role of user-space.
> > +
> > +The device is therefore responsible for presenting a virtual device that behaves
> > +like an actual V4L2 device, which the driver can control.
> > +
> > +Note that virtio-media does not require the use of a V4L2 device driver or of
> > +Linux on any side - V4L2 is only used as a transport protocol,
> > +and both sides are free to convert it from/to any model that they wish to use.
> > +
> > +This section relies on definitions from
> > +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
> > +
> > +\subsection{Device ID}\label{sec:Device Types / Media Device / Device ID}
> > +
> > +42
> > +
> > +\subsection{Virtqueues}\label{sec:Device Types / Media Device / Virtqueues}
> > +
> > +\begin{description}
> > +\item[0] commandq - used for driver commands and device responses to these
> > +commands.
> > +\item[1] eventq - used for events sent by the device to the driver.
> > +\end{description}
> > +
> > +\devicenormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
> > +
> > +The device MUST return the descriptor chains it receives on the commandq as
> > +soon as possible, and must never hold them for indefinite periods of time.
> > +
> > +\drivernormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
> > +
> > +The driver MUST re-queue the descriptor chains returned by the device on the
> > +eventq as soon as possible, and must never hold them for indefinite periods
> > +of time.
> > +
> > +\subsection{Feature Bits}\label{sec:Device Types / Media Device / Feature Bits}
> > +
> > +None
> > +
> > +\subsection{Device Configuration Layout}\label{sec:Device Types / Media Device / Device Configuration Layout}
> > +
> > +The video device configuration space uses the following layout:
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_config {
> > +    le32 device_caps;
> > +    le32 device_type;
> > +    u8 card[32];
>
> Use char instead of u8. It's always been a pain that struct v4l2_capability
> used u8 instead of char for the character arrays. I never understood why, and
> if you are making a new struct I would recommend using char.
>
> Up to you, though.
>
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{device_caps}] (driver-read-only) flags representing the device
> > +capabilities as used in
> > +\href{https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-querycap.html#c.v4l2_capability}{struct v4l2_capabilities}.
> > +Corresponds with the \field{device_caps} field in the \textit{struct video_device}.
> > +\item[\field{device_type}] (driver-read-only) informs the driver of the type
> > +of the video device. Corresponds with the \field{vfl_devnode_type} field of the device.
> > +\item[\field{card}] (driver-read-only) name of the device, a NUL-terminated
> > +UTF-8 string. Corresponds with the \field{card} field of the \textit{struct v4l2_capability}.
> > +If all the characters of the field are used, it does not need to be NUL-terminated.
> > +\end{description}
> > +
> > +\subsection{Device Initialization}
> > +
> > +\begin{enumerate}
> > +\item The driver reads the \field{device_caps} and \field{device_type} fields
> > +from the configuration layout to identify the device.
> > +\item The driver sets up the \field{commandq} and \field{eventq}.
> > +\item The driver may open a session to use the device and send V4L2 ioctls in
> > +order to receive more information about the device, such as supported
> > +formats or controls.
> > +\end{enumerate}
> > +
> > +\subsection{Device Operation}\label{sec:Device Types / Media Device / Device Operation}
> > +
> > +Commands are queued on the command queue by the driver for the device to
> > +process. The errors returned by each command are standard
> > +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/gen-errors.html}{Linux kernel error codes}.
> > +For instance, a command that contains invalid options will return \textit{EINVAL}.
> > +
> > +Events are sent on the event queue by the device for the driver to handle.
> > +
> > +\subsubsection{Command Virtqueue}
> > +
> > +\paragraph{Device Operation: Command headers}
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_MEDIA_CMD_OPEN 1
> > +#define VIRTIO_MEDIA_CMD_CLOSE 2
> > +#define VIRTIO_MEDIA_CMD_IOCTL 3
> > +#define VIRTIO_MEDIA_CMD_MMAP 4
> > +#define VIRTIO_MEDIA_CMD_MUNMAP 5
> > +
> > +/* Header for all virtio commands from the driver to the device on the commandq. */
> > +struct virtio_media_cmd_header {
> > +     u32 cmd;
> > +     u32 __padding;
> > +};
> > +
> > +/* Header for all virtio responses from the device to the driver on the commandq. */
> > +struct virtio_media_resp_header {
> > +     u32 status;
> > +     u32 __padding;
> > +};
> > +\end{lstlisting}
> > +
> > +A command consists of a command header \textit{virtio_media_cmd_header}
> > +containing the following device-readable field:
> > +
> > +\begin{description}
> > +\item[\field{cmd}] specifies a device request type (VIRTIO_MEDIA_CMD_*).
> > +\end{description}
> > +
> > +A response consists of a response header \textit{virtio_media_resp_header}
> > +containing the following device-writable field:
> > +
> > +\begin{description}
> > +\item[\field{status}] indicates a device request status.
> > +\end{description}
> > +
> > +The status field can take 0 if the command was successful, or one of the
> > +standard Linux error codes if it was not.
> > +
> > +\drivernormative{\paragraph}{Device Operation: Command Virtqueue: Sessions}{Device Types / Media Device / Device Operation / Command Virtqueue}
> > +
> > +Sessions are how the device is multiplexed, allowing several distinct works to
> > +take place simultaneously. The driver needs to open a session before it can
> > +perform any useful operation on the device.
> > +
> > +\paragraph{Device Operation: Open device}
> > +
> > +\textbf{VIRTIO_MEDIA_CMD_OPEN} Command for creating a new session.
> > +
> > +This is the equivalent of calling \textit{open} on a V4L2 device node.
> > +The driver uses \textit{virtio_media_cmd_open} to send an open request.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_cmd_open {
> > +    struct virtio_media_cmd_header hdr;
> > +};
> > +\end{lstlisting}
> > +
> > +The device responds to \textit{VIRTIO_MEDIA_CMD_OPEN} with \textit{virtio_media_resp_open}.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_resp_open {
> > +    struct virtio_media_resp_header hdr;
> > +    u32 session_id;
> > +    u32 __padding;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{session_id}] specifies an identifier for the current session. The
> > +identifier can be used to perform other commands on the session, notably ioctls.
> > +\end{description}
> > +
> > +\devicenormative{\subparagraph}{Device Operation: Open device}{Device Types / Media Device / Device Operation / Open device}
> > +
> > +Upon success, the device MUST set a \field{session_id} in \textit{virtio_media_resp_open}
> > +to an integer that is NOT used by any other open session.
> > +
> > +\paragraph{Device Operation: Close device}
> > +
> > +\textbf{VIRTIO_MEDIA_CMD_CLOSE} Command for closing an active session.
> > +
> > +This is the equivalent of calling \textit{close} on a previously opened V4L2
> > +device node. All resources associated with this session will be freed.
> > +
> > +This command does not require a response from the device.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_cmd_close {
> > +    struct virtio_media_cmd_header hdr;
> > +    u32 session_id;
> > +    u32 __padding;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{session_id}] specifies an identifier for the session to close.
> > +\end{description}
> > +
> > +\drivernormative{\subparagraph}{Device Operation: Close device}{Device Types / Media Device / Device Operation / Close device}
> > +
> > +The session ID SHALL NOT be used again after queueing this command.
> > +
> > +\paragraph{Device Operation: V4L2 ioctls}
> > +
> > +\textbf{VIRTIO_MEDIA_CMD_IOCTL} Command for executing an ioctl on an open
> > +session.
> > +
> > +This command asks the device to run one of the `VIDIOC_*` ioctls on the active
> > +session.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_cmd_ioctl {
> > +    struct virtio_media_cmd_header hdr;
> > +    u32 session_id;
> > +    u32 code;
> > +    /* Followed by the relevant ioctl payload as defined in the macro */
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{session_id}] specifies an identifier of thesession to run the ioctl on.
> > +\item[\field{code}] specifies the code of the \field{VIDIOC_*} ioctl to run.
> > +\end{description}
> > +
> > +The code is extracted from the
> > +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/videodev.html}{videodev2.h},
> > +header file. The file defines the ioctl's codes, type of payload, and
> > +direction. The code consists of the second argument of the \field{_IO*} macro.
> > +
> > +For example, the \textit{VIDIOC_G_FMT} is defined as follows:
> > +
> > +\begin{lstlisting}
> > +#define VIDIOC_G_FMT _IOWR('V',  4, struct v4l2_format)
> > +\end{lstlisting}
> > +
> > +This means that its ioctl code is \textit{4}, that its payload is a
> > +\textit{struct v4l2_format}, and that its direction is \textit{WR} (i.e., the
> > +payload is written by both the driver and the device).
> > +See Section \ref{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
> > +for more information about the direction of ioctls.
> > +
> > +The payload layout is always a 64-bit representation of the corresponding
> > +V4L2 structure.
> > +
> > +The device responds to \textit{VIRTIO_MEDIA_CMD_IOCTL} with \textit{virtio_media_resp_ioctl}.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_resp_ioctl {
> > +    struct virtio_media_resp_header hdr;
> > +    /* Followed by the ioctl payload as defined in the macro */
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Ioctls payload}\label{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
> > +
> > +Each ioctl has a payload, which is defined by the third argument of the
> > +\field{_IO*} macro defining it.
> > +
> > +The payload of an ioctl in the descriptor chain follows the command structure,
> > +the reponse structure, or both depending on the direction:
> > +
> > +\begin{itemize}
> > +\item \textbf{_IOR} is read-only for the driver, meaning the payload
> > +follows the response in the device-writable section of the descriptor chain.
> > +\item \textbf{_IOW} is read-only for the device, meaning the payload
> > +follows the command in the driver-writable section of the descriptor chain.
> > +\item \textbf{_IOWR} is writable by both the device and driver,
> > +meaning the payload must follow both the command in the driver-writable section
> > +of the descriptor chain, and the response in the device-writable section.
> > +\end{itemize}
> > +
> > +A common optimization for \textit{WR} ioctls is to provide the payload using
> > +descriptors that both point to the same buffer. This mimics the behavior of
> > +V4L2 ioctls where the data is only passed once and used as both input and
> > +output by the kernel.
> > +
> > +\devicenormative{\subparagraph}{Device Operation: V4L2 ioctls}{Device Types / Media Device / Device Operation / V4L2 ioctls}
> > +
> > +In case of success of a device-writable ioctl, the device MUST always write the
> > +payload in the device-writable part of the descriptor chain.
> > +
> > +In case of failure of a device-writable ioctl, the device is free to write the
> > +payload in the device-writable part of the descriptor chain or not. Some errors
> > +may still result in the payload being updated, and in this case the device is
> > +expected to write the updated payload. If the device has not written the
> > +payload after an error, the driver MUST assume that the payload has not been
> > +modified.
> > +
> > +\subparagraph{Handling of pointers in ioctl payload}
> > +
> > +A few structures used as ioctl payloads contain pointers to further
> > +data needed for the ioctl. There are notably:
> > +
> > +\begin{itemize}
> > +\item The \field{planes} pointer of
> > +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer}{struct v4l2_buffer},
> > +which size is determined by the length member.
> > +\item The \field{controls} pointer of \textit{struct v4l2_ext_controls}, which
> > +size is determined by the count member.
> > +\end{itemize}
> > +
> > +If the size of the pointed area is determined to be non-zero, then the main
> > +payload is immediately followed by the pointed data in their order of
> > +appearance in the structure, and the pointer value itself is ignored by the
> > +device, which must also return the value initially passed by the driver.
> > +
> > +\subparagraph{Handling of pointers to userspace memory}
> > +\label{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
> > +
> > +A few pointers are special in that they point to userspace memory in the
> > +original V4L2 specification. They are:
> > +
> > +\begin{itemize}
> > +\item The \field{m.userptr} member of \textit{struct v4l2_buffer} and
> > +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-plane}{struct v4l2_plane}
> > +(technically an unsigned long, but designated a userspace address).
> > +\item The \field{ptr} member of \textit{struct v4l2_ext_ctrl}.
> > +\end{itemize}
> > +
> > +These pointers can cover large areas of scattered memory, which has the
>
> Actually, only m.userptr can cover a large area. The other two are limited.
> See my comment about USERPTR support below.
>
> > +potential to require more descriptors than the virtio queue can provide. For
> > +these particular pointers only, a list of \textit{struct virtio_media_sg_entry}
> > +that covers the needed amount of memory for the pointer is used instead of
> > +using descriptors to map the pointed memory directly.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_sg_entry {
> > +     u64 start;
> > +     u32 len;
> > +     u32 __padding;
> > +};
> > +\end{lstlisting}
> > +
> > +For each such pointer to read, the device reads as many SG entries as needed
> > +to cover the length of the pointed buffer, as described by its parent
> > +structure (\field{length} member of \textit{struct v4l2_buffer} or
> > +\textit{struct v4l2_plane} for buffer memory, and \field{size} member of
> > +\textit{struct v4l2_ext_control} for control data).
> > +
> > +Since the device never needs to modify the list of SG entries, it is only
> > +provided by the driver in the device-readable section of the descriptor chain,
> > +and not repeated in the device-writable section, even for WR ioctls.
> > +
> > +\subparagraph{Unsupported ioctls}
> > +
> > +A few ioctls are replaced by other, more suitable mechanisms.
> > +
> > +\begin{itemize}
> > +\item \textit{VIDIOC_QUERYCAP} is replaced by reading the configuration area
> > +(see \ref{sec:Device Types / Media Device / Device Configuration Layout}).
> > +\item \textit{VIDIOC_DQBUF} is replaced by a dedicated event
> > +(see \ref{sec:Device Types / Media Device / Device Operation / Dequeue buffer}).
> > +\item \textit{VIDIOC_DQEVENT} is replaced by a dedicated event
> > +(see \ref{sec:Device Types / Media Device / Device Operation / Emit an event}).
> > +\item \textit{VIDIOC_G_JPEGCOMP} and \textit{VIDIOC_S_JPEGCOMP} are deprecated
> > +and replaced by the controls of the JPEG class.
> > +\item \textit{VIDIOC_LOG_STATUS} is a driver-only operation and shall not be
> > +implemented by the device.
> > +\end{itemize}
> > +
> > +\devicenormative{\subparagraph}{Device Operation: Unsupported ioctls}{Device Types / Media Device / Device Operation / Unsupported ioctls}
> > +
> > +If being requested an unsupported ioctl, the device MUST return the same
> > +error response as it would for an unknown ioctl, i.e. \textit{ENOTTY}.
> > +
> > +\paragraph{Device Operation: Mapping a MMAP buffer}
> > +
> > +\textbf{VIRTIO_MEDIA_CMD_MMAP} Command for mapping a MMAP buffer into the
> > +driver's address space.
> > +
> > +Shared memory region ID 0 is used to map MMAP buffers with
> > +the \textit{VIRTIO_MEDIA_CMD_MMAP} command.
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_MEDIA_MMAP_FLAG_RW (1 << 0)
> > +
> > +struct virtio_media_cmd_mmap {
> > +     struct virtio_media_cmd_header hdr;
> > +     u32 session_id;
> > +     u32 flags;
> > +     u64 offset;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{flags}] is the set of flags for the mapping. \field{VIRTIO_MEDIA_MMAP_FLAG_RW}
> > +can be set if a read-write mapping is desired. Without this flag the mapping
> > +will be read-only.
> > +\item[\field{offset}] corresponds to the \field{mem_offset} field of the
> > +\textit{union v4l2_plane} for the plane to map. This field can be obtained
> > +using the \textit{VIDIOC_QUERYBUF} ioctl.
> > +\end{description}
> > +
> > +The device responds to \textit{VIRTIO_MEDIA_CMD_MMAP} with \textit{virtio_media_resp_mmap}.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_resp_mmap {
> > +    struct virtio_media_resp_header hdr;
> > +    u64 offset;
> > +    u64 len;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{offset}] offset into SHM region ID 0 of the start of the mapping.
> > +\item[\field{len}] length of the mapping as indicated by the \textit{struct v4l2_plane}
> > +the buffer belongs to.
> > +\end{description}
> > +
> > +\paragraph{Device Operation: Unmapping a MMAP buffer}
> > +
> > +\textbf{VIRTIO_MEDIA_CMD_MUNMAP} Unmap a MMAP buffer previously mapped using \field{VIRTIO_MEDIA_CMD_MMAP}.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_cmd_munmap {
> > +    struct virtio_media_cmd_header hdr;
> > +    u64 offset;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{offset}] offset into SHM region ID 0 previously returned by
> > +\textit{VIRTIO_MEDIA_CMD_MMAP} at which the buffer has been previously mapped.
> > +\end{description}
> > +
> > +The device responds to \textit{VIRTIO_MEDIA_CMD_MUNMAP} with \textit{virtio_media_resp_munmap}.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_resp_munmap {
> > +    struct virtio_media_resp_header hdr;
> > +};
> > +\end{lstlisting}
> > +
> > +\devicenormative{\subparagraph}{Device Operation: Unmapping a MMAP buffer}{Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
> > +
> > +The device MUST keep mappings performed using \textit{VIRTIO_MEDIA_CMD_MMAP}
> > +valid until \textit{VIRTIO_MEDIA_CMD_MUNMAP} is called, even if the buffers or
> > +session they belong to are released or closed by the driver.
> > +
> > +\paragraph{Device Operation: Memory Types}
> > +
> > +The semantics of the three V4L2 memory types (\textit{MMAP}, \textit{USERPTR}
> > +and \textit{DMABUF}) can easily be mapped to both driver and device context.
> > +
> > +\subparagraph{MMAP}
> > +
> > +In virtio-media, \textit{MMAP} buffers are provisioned by the device, just like
> > +they are by the kernel in regular V4L2. Similarly to how userspace can map a
> > +\textit{MMAP} buffer into its address space using mmap and munmap, the
> > +virtio-media driver can map device buffers into the driver space by queueing the
> > +\textit{struct virtio_media_cmd_mmap} and \textit{struct virtio_media_cmd_munmap}
> > +commands to the commandq.
> > +
> > +\subparagraph{USERPTR}
> > +
> > +In virtio-media, \textit{USERPTR} buffers are provisioned by the driver, just
> > +like they are by userspace in regular V4L2. Instances of \textit{struct v4l2_buffer}
> > +and \textit{struct v4l2_plane} of this type are followed by a list of
> > +\textit{struct virtio_media_sg_entry}. For more information, see
> > +\ref{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
> > +
> > +The device must not alter the pointer values provided by the driver, i.e.
> > +\field{the m.userptr} member of \textit{struct v4l2_buffer} and
> > +\textit{struct v4l2_plane} must be returned to the driver with the same value
> > +as it was provided.
>
> I very, very strongly recommend that you drop USERPTR support for virtio.
> MMAP and DMABUF are sufficient.
>
> It's a pain to support, and we discourage it for new drivers.
>
> If memory serves, there is at least one chromeos driver that is stuck to
> USERPTR support for some reason (I can't remember the details), and that is
> (I think) the only reason USERPTR support is part of virtio. But that is
> really a chromeos issue that they need to fix, IMHO.

I am wondering if the concerns about USERPTR should apply to the
host/guest context.

In virtio-media, the USERPTR memory type just means that the buffer
memory has been allocated by the guest and the addresses passed are
guest physical addresses. It doesn't necessarily mean that the memory
has been allocated with the USERPTR type by the guest user-space.

For instance, say the guest user-space allocated memory using memfd,
and is passing the FDs to the guest driver as DMABUF buffers. From the
guest kernel point of view, these buffers will still resolve into
guest physical memory, so the USERPTR memory type will be used by
guest/host communication to indicate that fact. I don't know of any
formal way to resolve guest DMABUFs into proper virtio resources
unfortunately.

Without the ability to communicate buffers as guest physical memory,
the only kinds of DMABUFs supported by the guest driver would be those
coming from another virtio driver, like virtio-gpu. Granted, in
practice that's probably where these will come from anyway, but I
think it would be nice to be able to support memfd, at least for
testing purposes.

But to answer what I think your concern is, if you prefer that the
guest driver does not advertise support for USERPTR memory type to the
guest userspace, we can certainly enforce that (either absolutely or
through an option).

Cheers,
Alex.

>
> Regards,
>
>         Hans
>
> > +
> > +\subparagraph{DMABUF}
> > +
> > +In virtio-media, \textit{DMABUF} buffers are provisioned by a virtio object,
> > +just like they are by a \textit{DMABUF} in regular V4L2. Virtio objects are
> > +16-bytes UUIDs and do not fit in the placeholders for file descriptors, so
> > +they follow their embedding data structure as needed and the device must
> > +leave the V4L2 structure placeholder unchanged.
> > +
> > +Contrary to \textit{USERPTR} buffers, virtio objects UUIDs need to be added in
> > +both the device-readable and device-writable section of the descriptor chain.
> > +
> > +Device-allocated buffers with the \textit{V4L2_MEMORY_MMAP} memory type can also
> > +be exported as virtio objects for use with another virtio device using the
> > +\textit{VIDIOC_EXPBUF} ioctl. The fd placefolder of \textit{v4l2_exportbuffer}
> > +means that space for the UUID needs to be reserved right after that structure
> > +
> > +\subsubsection{Event Virtqueue}
> > +
> > +Events are a way for the device to inform the driver about asynchronous events
> > +that it should know about. In virtio-media, they are used as a replacement for
> > +the \textit{VIDIOC_DQBUF} and \textit{VIDIOC_DQEVENT} ioctls and the polling
> > +mechanism, which would be impractical to implement on top of virtio.
> > +
> > +\paragraph{Device Operation: Event header}
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_MEDIA_EVT_ERROR 0
> > +#define VIRTIO_MEDIA_EVT_DQBUF 1
> > +#define VIRTIO_MEDIA_EVT_EVENT 2
> > +
> > +/* Header for events queued by the device for the driver on the eventq. */
> > +struct virtio_media_event_header {
> > +    u32 event;
> > +    u32 session_id;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{event}] one of \field{VIRTIO_MEDIA_EVT_*}.
> > +\item[\field{session_id}] ID of the session the event applies to.
> > +\end{description}
> > +
> > +\paragraph{Device Operation: Device-side error}
> > +
> > +\textbf{VIRTIO_MEDIA_EVT_ERROR} Upon receiving this event, the session
> > +mentioned in the header is considered corrupted and automatically closed by
> > +the device.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_event_error {
> > +    struct virtio_media_event_header hdr;
> > +    u32 errno;
> > +    u32 __padding;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{errno}] error code describing the kind of error that occurred.
> > +\end{description}
> > +
> > +\paragraph{Device Operation: Dequeue buffer}
> > +\label{sec:Device Types / Media Device / Device Operation / Dequeue buffer}
> > +
> > +\textbf{VIRTIO_MEDIA_EVT_DQBUF} Signals that a buffer is not being used anymore
> > +by the device and is returned to the driver.
> > +
> > +A \textit{struct virtio_media_event_dqbuf} event is queued on the eventq by the
> > +device every time a buffer previously queued using the \textit{VIDIOC_QBUF}
> > +ioctl is done being processed and can be used by the driver again. This is like
> > +an implicit \textit{VIDIOC_DQBUF} ioctl.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_event_dqbuf {
> > +    struct virtio_media_event_header hdr;
> > +    struct v4l2_buffer buffer;
> > +    struct v4l2_plane planes[VIDEO_MAX_PLANES];
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{buffer}] \textit{struct v4l2_buffer} describing the buffer that has been dequeued.
> > +\item[\field{planes}] array of \textit{struct v4l2_plane} containing the plane information for multi-planar buffers.
> > +\end{description}
> > +
> > +Pointer values in the \textit{struct v4l2_buffer} and \textit{struct v4l2_plane}
> > +are meaningless and must be ignored by the driver. It is recommended that the
> > +device sets them to NULL in order to avoid leaking potential device addresses.
> > +
> > +Note that in the case of a \field{USERPTR} buffer, the \textit{struct v4l2_buffer}
> > +used as event payload is not followed by the buffer memory: since that memory
> > +is the same that the driver submitted with the \textit{VIDIOC_QBUF}, it would
> > +be redundant to have it here.
> > +
> > +\paragraph{Device Operation: Emit an event}
> > +\label{sec:Device Types / Media Device / Device Operation / Emit an event}
> > +
> > +\textbf{VIRTIO_MEDIA_EVT_EVENT} Signals that a V4L2 event has been emitted for a session.
> > +
> > +A \textit{struct virtio_media_event_event} event is queued on the eventq by the
> > +device every time an event the driver previously subscribed to using the
> > +\textit{VIDIOC_SUBSCRIBE_EVENT} ioctl has been signaled. This is like an
> > +implicit \textit{VIDIOC_DQEVENT} ioctl.
> > +
> > +\begin{lstlisting}
> > +struct virtio_media_event_event {
> > +    struct virtio_media_event_header hdr;
> > +    struct v4l2_event event;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{event}] \textit{struct v4l2_event} describing the event that occurred.
> > +\end{description}
> > diff --git a/device-types/media/device-conformance.tex b/device-types/media/device-conformance.tex
> > new file mode 100644
> > index 0000000..3338822
> > --- /dev/null
> > +++ b/device-types/media/device-conformance.tex
> > @@ -0,0 +1,11 @@
> > +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Device Conformance / Media Device Conformance}
> > +
> > +A Media device MUST conform to the following normative statements:
> > +
> > +\begin{itemize}
> > +\item \ref{devicenormative:Device Types / Media Device / Virtqueues}
> > +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Open device}
> > +\item \ref{devicenormative:Device Types / Media Device / Device Operation / V4L2 ioctls}
> > +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unsupported ioctls}
> > +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
> > +\end{itemize}
> > \ No newline at end of file
> > diff --git a/device-types/media/driver-conformance.tex b/device-types/media/driver-conformance.tex
> > new file mode 100644
> > index 0000000..058b812
> > --- /dev/null
> > +++ b/device-types/media/driver-conformance.tex
> > @@ -0,0 +1,9 @@
> > +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Driver Conformance / Media Driver Conformance}
> > +
> > +A Media device MUST conform to the following normative statements:
> > +
> > +\begin{itemize}
> > +\item \ref{drivernormative:Device Types / Media Device / Virtqueues}
> > +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Command Virtqueue}
> > +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Close device}
> > +\end{itemize}
> > \ No newline at end of file
>

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Alexandre Courbot]

On Fri, Jun 7, 2024 at 10:29 PM Alexander Gordeev
<alexander.gordeev@opensynergy.com> wrote:
>
> Hi Albert,
>
> On 07.06.24 10:00, Albert Esteve wrote:
> > Hi,
> >
> > This a formal attempt of including virtio-media
> > device specification.
> >
> > Virtio-media came from a discussion on virtio-dev
> > mailing list, which lead to presenting virtio-v4l2[1]
> > specification as an alternative to virtio-video.
> >
> > Later, virtio-v4l2 was renamed to virtio-media[2]
> > and published through:
> >
> > https://github.com/chromeos/virtio-media
> >
> > The repository above includes a virtio-media driver able
> > to pass v4l2-compliance when proxying the vivid/vicodec
> > virtual devices or an actual UVC camera using the
> > V4L2 vhost device (available in the repository).
> > Steps to reproduce are also detailed[3].
> >
> > There is some overlap with virtio-video in regards
> > to which devices it can handle. However,
> > as virtio-media will likely be the virtualization
> > solution for ChromeOS (already landed into the chromeos
> > organization) and possibly other Google projects for
> > media devices, it would be desirable to include the
> > specification in the next virtio release despite
> > the aforementioned overlap.
> >
> > The device ID in this document differs from
> > the ID in the virtio-media project repository.
> > And it will probably need some discussion on which
> > would be the correct definitive ID.
> >
> > Full PDF: https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
> > PDF with the media section only: https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing
> >
> > [1] https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
> > [2] https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
> > [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
> >
> > Albert Esteve (1):
> >    virtio-media: Add virtio media device specification
> >
> >   conformance.tex                           |  13 +-
> >   content.tex                               |   1 +
> >   device-types/media/description.tex        | 578 ++++++++++++++++++++++
> >   device-types/media/device-conformance.tex |  11 +
> >   device-types/media/driver-conformance.tex |   9 +
> >   5 files changed, 608 insertions(+), 4 deletions(-)
> >   create mode 100644 device-types/media/description.tex
> >   create mode 100644 device-types/media/device-conformance.tex
> >   create mode 100644 device-types/media/driver-conformance.tex
>
> I'd like to add some general considerations:
>
> 1. virtio-media device capability discovery is very chatty
>
> V4L2 requires potentially hundreds of system calls to get the full view
> of the device capabilities. This is inherited by virtio-media.
> AFAIU V4L2 developers agree there is room for enhancement here, see [1],
> [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
> added for similar reasons.
>
>  From the point of view of virtualization developers like my colleagues
> and me hundreds of hypervisor "exits" are excessive and costly. It can
> noticeably increase boot times, which is something we fight against in
> automotive. AFAIU other virtio developers agree with this, see [3].

There is a simple way to mitigate this: allow an arbitrary number of
commands to be submitted by the driver with each command buffer. The
host processes the commands sequentially and stops at the first error,
if any.

That way, when enumerating formats, the guest can place a bunch of
VIDIOC_ENUM_FMT ioctls covering an arbitrary range 0..n in its command
buffer, and get all these formats in one vmexit instead of n. In the
case of a video decoder supporting 5 input formats, all the formats
are enumerated in 5 exits.

Having this option could also enable other optimizations, like the
guest being able to queue all its CAPTURE buffers in a single vmexit,
although I doubt this would result in a noticeable performance boost.

But at the very least this should address concerns about noisy
enumeration. I haven't noticed any slowness due to format enumeration
with the current scheme, but the option is trivial to enable.

>
> In contrast virtio-video has been doing this in one command from day
> one. Yes, the data was incomplete. That's why in draft v9 I added TLV as
> a way to store all possible capabilities. I hope the linux-media folks
> could take a look on it. Maybe this is something, that can be adopted in
> V4L2?
>
> 2. virtio-media has a hard dependency on V4L2
>
> There are certainly some "patches" on top of V4L2 in virtio-media, like
> the representation of the extended controls (which actually looks
> similar to the representation of the controls in virtio-video) or
> VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your
> own ioctl ids in virtio-media?

virtio-media doesn't add any ioctl ID and the extended controls use
the same structure as their V4L2 counterparts.

>
> AFAIK the linux-media maintainers have been overloaded for years, see
> [4]. Would they be happy to deal with the additional requirements? Would
> virtio community like to have a dependency here?

The burden of having a couple new events in virtio-media seems lighter
to me on the V4L2 maintainers than the one of having a whole new video
protocol to maintain.

>
> 2.1. an example: format modifiers
>
> There is a patchset aiming at unifying V4L2 pixel formats and extending
> them with DRM format modifiers. It is now at version 7 submitted in
> 2023, see [5]. The first version was submitted in 2019, see [6]. Not
> merged yet AFAIK.
>
> In virtio-video I just added them in draft v9.
>
> I'm absolutely not trying to criticize here. I just try to highlight
> that there is a lot of legacy and the process is painful. Right now we
> have an opportunity to design a new API according to the current state
> of the art of the stateful codecs.
>
> 3. uncertainty with cameras
>
> AFAIK there is still no agreement about how cameras should be
> virtualized, see [3], [7], [8], [9]. virtio-media provides support for
> cameras in a specific way, which might not be desirable.

There is definitely an agreement on how cameras should work on V4L2 ;
and thanks to that you can operate a camera using virtio-media,
*today*.

I'm sure the virtio-camera specification, driver, and host devices
will be very impressive once they are released ; it's just that I
would enjoy a solution, however imperfect, within my life span.

>
> 4. (minor) is it possible/hard to implement the device in hardware/on a
> micro controller?
>
> This is something I thought about recently, there might be a use-case
> for it in the future. One of the concerns is that dynamic memory
> allocations are IMHO inevitable in virtio-media or virtio-video up to
> draft v8. I think multiple virtqueues in virtio-video draft v9 would
> help here. Not sure yet...

V4L2's MMAP buffer type has that covered.

>
> There are also other minor concerns, that are probably tolerable.
>
> [1]
> https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
> [2] Page 6 in
> https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
> [3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
> [4]
> https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
> [5]
> https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
> [6]
> https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
> [7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
> [8]
> https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
> [9]
> https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/
>
> --
> Alexander Gordeev
> Senior Software Engineer
>
> OpenSynergy GmbH
> Rotherstr. 20, 10245 Berlin
> www.opensynergy.com

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

[-- Attachment #1: Type: text/plain, Size: 5300 bytes --]

Hi Alex, allow me to recover your message to repond.

On Fri, Jun 7, 2024 at 3:29 PM Alexander Gordeev <
alexander.gordeev@opensynergy.com> wrote:

> Hi Albert,
>
>

>
> I'd like to add some general considerations:
>
> 1. virtio-media device capability discovery is very chatty
>
> V4L2 requires potentially hundreds of system calls to get the full view
> of the device capabilities. This is inherited by virtio-media.
> AFAIU V4L2 developers agree there is room for enhancement here, see [1],
> [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
> added for similar reasons.
>
>  From the point of view of virtualization developers like my colleagues
> and me hundreds of hypervisor "exits" are excessive and costly. It can
> noticeably increase boot times, which is something we fight against in
> automotive. AFAIU other virtio developers agree with this, see [3].


> In contrast virtio-video has been doing this in one command from day
> one. Yes, the data was incomplete. That's why in draft v9 I added TLV as
> a way to store all possible capabilities. I hope the linux-media folks
> could take a look on it. Maybe this is something, that can be adopted in
> V4L2?


> 2. virtio-media has a hard dependency on V4L2
>
> There are certainly some "patches" on top of V4L2 in virtio-media, like
> the representation of the extended controls (which actually looks
> similar to the representation of the controls in virtio-video) or
> VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your
> own ioctl ids in virtio-media?
>
> AFAIK the linux-media maintainers have been overloaded for years, see
> [4]. Would they be happy to deal with the additional requirements? Would
> virtio community like to have a dependency here?
>
> 2.1. an example: format modifiers
>
> There is a patchset aiming at unifying V4L2 pixel formats and extending
> them with DRM format modifiers. It is now at version 7 submitted in
> 2023, see [5]. The first version was submitted in 2019, see [6]. Not
> merged yet AFAIK.
>
> In virtio-video I just added them in draft v9.
>
> I'm absolutely not trying to criticize here. I just try to highlight
> that there is a lot of legacy and the process is painful. Right now we
> have an opportunity to design a new API according to the current state
> of the art of the stateful codecs.
>
> 3. uncertainty with cameras
>
> AFAIK there is still no agreement about how cameras should be
> virtualized, see [3], [7], [8], [9]. virtio-media provides support for
> cameras in a specific way, which might not be desirable.
>
> 4. (minor) is it possible/hard to implement the device in hardware/on a
> micro controller?
>
> This is something I thought about recently, there might be a use-case
> for it in the future. One of the concerns is that dynamic memory
> allocations are IMHO inevitable in virtio-media or virtio-video up to
> draft v8. I think multiple virtqueues in virtio-video draft v9 would
> help here. Not sure yet...
>
> There are also other minor concerns, that are probably tolerable.
>
> [1]
>
> https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
> [2] Page 6 in
>
> https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
> [3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
> [4]
>
> https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
> [5]
>
> https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
> [6]
>
> https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
> [7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
> [8]
>
> https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
> [9]
>
> https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/
>
>
As I mentioned in the previous drop of the specs, I prefer not to
re-open the debate video vs media. Most of the points raised here
were already discussed previously, and yet it led nowhere as there
was not real agreement.

Personally, I think there are big advantages relying on a well established
API
and lessons learned from v4l2, and using them for virtualizing a video
device. That allowed to produce code for the driver and a reusable device,
already available in the spec source repository for anyone to try:
https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md

Also, new devices with new requisites appear, and the V4L2 API is forced to
evolve fast. The virtio-media approach makes the spec less volatile, as it
lets v4l2 do the heavy lifting for adapting.

But really, this is mainly to avoid having virtio-media as a downstream-only
device. It also opens the spec to feedback from experts, and makes it
more visible. There were already changes from v1 to v2 following the
feedback, and these changes are also already reflected in the available
driver and device implementations.

BR,
Albert.


> --
> Alexander Gordeev
> Senior Software Engineer
>
> OpenSynergy GmbH
> Rotherstr. 20, 10245 Berlin
> www.opensynergy.com
>
>
>

[-- Attachment #2: Type: text/html, Size: 7970 bytes --]

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

[-- Attachment #1: Type: text/plain, Size: 5816 bytes --]

On Mon, Jun 10, 2024 at 11:09 AM Albert Esteve <aesteve@redhat.com> wrote:

> Hi Alex, allow me to recover your message to repond.
>
> On Fri, Jun 7, 2024 at 3:29 PM Alexander Gordeev <
> alexander.gordeev@opensynergy.com> wrote:
>
>> Hi Albert,
>>
>>
>
>>
>> I'd like to add some general considerations:
>>
>> 1. virtio-media device capability discovery is very chatty
>>
>> V4L2 requires potentially hundreds of system calls to get the full view
>> of the device capabilities. This is inherited by virtio-media.
>> AFAIU V4L2 developers agree there is room for enhancement here, see [1],
>> [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
>> added for similar reasons.
>>
>>  From the point of view of virtualization developers like my colleagues
>> and me hundreds of hypervisor "exits" are excessive and costly. It can
>> noticeably increase boot times, which is something we fight against in
>> automotive. AFAIU other virtio developers agree with this, see [3].
>
>
>> In contrast virtio-video has been doing this in one command from day
>> one. Yes, the data was incomplete. That's why in draft v9 I added TLV as
>> a way to store all possible capabilities. I hope the linux-media folks
>> could take a look on it. Maybe this is something, that can be adopted in
>> V4L2?
>
>
>> 2. virtio-media has a hard dependency on V4L2
>>
>> There are certainly some "patches" on top of V4L2 in virtio-media, like
>> the representation of the extended controls (which actually looks
>> similar to the representation of the controls in virtio-video) or
>> VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your
>> own ioctl ids in virtio-media?
>>
>> AFAIK the linux-media maintainers have been overloaded for years, see
>> [4]. Would they be happy to deal with the additional requirements? Would
>> virtio community like to have a dependency here?
>>
>> 2.1. an example: format modifiers
>>
>> There is a patchset aiming at unifying V4L2 pixel formats and extending
>> them with DRM format modifiers. It is now at version 7 submitted in
>> 2023, see [5]. The first version was submitted in 2019, see [6]. Not
>> merged yet AFAIK.
>>
>> In virtio-video I just added them in draft v9.
>>
>> I'm absolutely not trying to criticize here. I just try to highlight
>> that there is a lot of legacy and the process is painful. Right now we
>> have an opportunity to design a new API according to the current state
>> of the art of the stateful codecs.
>>
>> 3. uncertainty with cameras
>>
>> AFAIK there is still no agreement about how cameras should be
>> virtualized, see [3], [7], [8], [9]. virtio-media provides support for
>> cameras in a specific way, which might not be desirable.
>>
>> 4. (minor) is it possible/hard to implement the device in hardware/on a
>> micro controller?
>>
>> This is something I thought about recently, there might be a use-case
>> for it in the future. One of the concerns is that dynamic memory
>> allocations are IMHO inevitable in virtio-media or virtio-video up to
>> draft v8. I think multiple virtqueues in virtio-video draft v9 would
>> help here. Not sure yet...
>>
>> There are also other minor concerns, that are probably tolerable.
>>
>> [1]
>>
>> https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
>> [2] Page 6 in
>>
>> https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
>> [3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
>> [4]
>>
>> https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
>> [5]
>>
>> https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
>> [6]
>>
>> https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
>> [7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
>> [8]
>>
>> https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
>> [9]
>>
>> https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/
>>
>>
> As I mentioned in the previous drop of the specs, I prefer not to
> re-open the debate video vs media. Most of the points raised here
> were already discussed previously, and yet it led nowhere as there
> was not real agreement.
>
> Personally, I think there are big advantages relying on a well established
> API
> and lessons learned from v4l2, and using them for virtualizing a video
> device. That allowed to produce code for the driver and a reusable device,
> already available in the spec source repository for anyone to try:
> https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
>
> Also, new devices with new requisites appear, and the V4L2 API is forced to
> evolve fast. The virtio-media approach makes the spec less volatile, as it
> lets v4l2 do the heavy lifting for adapting.
>
> But really, this is mainly to avoid having virtio-media as a
> downstream-only
> device. It also opens the spec to feedback from experts, and makes it
> more visible. There were already changes from v1 to v2 following the
> feedback, and these changes are also already reflected in the available
> driver and device implementations.
>
> BR,
> Albert.
>

Just saw Alex Courbot's point-per-point response in the previous thread.
I'll link it here, as I'd prefer to move the discussion:
https://lore.kernel.org/all/CAPBb6MW36YaZFaWgksePQ=fMC7QffJD6ZpHN-iSzhLYp9Ho0sg@mail.gmail.com/

Again, sorry for the confusion.


>
>
>> --
>> Alexander Gordeev
>> Senior Software Engineer
>>
>> OpenSynergy GmbH
>> Rotherstr. 20, 10245 Berlin
>> www.opensynergy.com
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 9062 bytes --]

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Alexander Gordeev]

Hi Albert, Alexandre,

On 10.06.24 11:24, Albert Esteve wrote:
> Just saw Alex Courbot's point-per-point response in the previous thread.
> I'll link it here, as I'd prefer to move the discussion:
> https://lore.kernel.org/all/CAPBb6MW36YaZFaWgksePQ=fMC7QffJD6ZpHN-iSzhLYp9Ho0sg@mail.gmail.com/
> 
> Again, sorry for the confusion.

I think I'll quote Alexandre's email and respond to him below to have it 
in the new place.

Replying to Alexandre:

On 08.06.24 03:50, Alexandre Courbot wrote:
> On Fri, Jun 7, 2024 at 10:29 PM Alexander Gordeev
> <alexander.gordeev@opensynergy.com> wrote:
>>
>> Hi Albert,
>>
>> On 07.06.24 10:00, Albert Esteve wrote:
>>> Hi,
>>>
>>> This a formal attempt of including virtio-media
>>> device specification.

The thing is we already had a long email discussion. Unfortunately it 
didn't help to reach a consensus. You asked Cornelia to organize a 
meeting. So she did, there were a bunch of virtio and V4L2 experts 
invited by her. We both had an opportunity to bring our own experts to 
the discussion and to present our points of view. I presented the same 
arguments listed below in this email.

I clearly remember, that all the experts told you, that you're going in 
a wrong direction, if your goal is standardization. I don't know what 
can I add on top of this. I think it is time to take the opinions of the 
experts and these arguments seriously, and stop pushing.

Has anything changed since June 2023? I don't think so. I promised to 
continue the virtio-video development, I provided two drafts. I think, 
the last one addresses all previous comments and I'm myself satisfied 
with it. I'm also working on the driver update, I've published some 
intermediate results. You promised to make a downstream specification 
and implement it, you did it. Great, everything went according to the 
made decisions so far.

You know, virtio-video is not a toy project too. AFAIK you can literally 
go and buy a mass market car with virtio-video on board since 2021. So 
our device and the driver are in production for years. Yes, the driver 
is based on draft v3/v4, there are problems, it needs updating. It is a 
work in progress right now. I hope to bring updates within a few months.

>>> Virtio-media came from a discussion on virtio-dev
>>> mailing list, which lead to presenting virtio-v4l2[1]
>>> specification as an alternative to virtio-video.
>>>
>>> Later, virtio-v4l2 was renamed to virtio-media[2]
>>> and published through:
>>>
>>> https://github.com/chromeos/virtio-media
>>>
>>> The repository above includes a virtio-media driver able
>>> to pass v4l2-compliance when proxying the vivid/vicodec
>>> virtual devices or an actual UVC camera using the
>>> V4L2 vhost device (available in the repository).
>>> Steps to reproduce are also detailed[3].
>>>
>>> There is some overlap with virtio-video in regards
>>> to which devices it can handle. However,
>>> as virtio-media will likely be the virtualization
>>> solution for ChromeOS (already landed into the chromeos
>>> organization) and possibly other Google projects for
>>> media devices, it would be desirable to include the
>>> specification in the next virtio release despite
>>> the aforementioned overlap.
>>>
>>> The device ID in this document differs from
>>> the ID in the virtio-media project repository.
>>> And it will probably need some discussion on which
>>> would be the correct definitive ID.
>>>
>>> Full PDF: https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
>>> PDF with the media section only: https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing
>>>
>>> [1] https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
>>> [2] https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
>>> [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
>>>
>>> Albert Esteve (1):
>>>     virtio-media: Add virtio media device specification
>>>
>>>    conformance.tex                           |  13 +-
>>>    content.tex                               |   1 +
>>>    device-types/media/description.tex        | 578 ++++++++++++++++++++++ >>>    device-types/media/device-conformance.tex |  11 +
>>>    device-types/media/driver-conformance.tex |   9 +
>>>    5 files changed, 608 insertions(+), 4 deletions(-)
>>>    create mode 100644 device-types/media/description.tex
>>>    create mode 100644 device-types/media/device-conformance.tex
>>>    create mode 100644 device-types/media/driver-conformance.tex
>>
>> I'd like to add some general considerations:
>>
>> 1. virtio-media device capability discovery is very chatty
>>
>> V4L2 requires potentially hundreds of system calls to get the full view
>> of the device capabilities. This is inherited by virtio-media.
>> AFAIU V4L2 developers agree there is room for enhancement here, see [1],
>> [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
>> added for similar reasons.
>>
>>   From the point of view of virtualization developers like my colleagues
>> and me hundreds of hypervisor "exits" are excessive and costly. It can
>> noticeably increase boot times, which is something we fight against in
>> automotive. AFAIU other virtio developers agree with this, see [3].
>
> There is a simple way to mitigate this: allow an arbitrary number of
> commands to be submitted by the driver with each command buffer. The
> host processes the commands sequentially and stops at the first error,
> if any.
>
> That way, when enumerating formats, the guest can place a bunch of
> VIDIOC_ENUM_FMT ioctls covering an arbitrary range 0..n in its command
> buffer, and get all these formats in one vmexit instead of n. In the
> case of a video decoder supporting 5 input formats, all the formats
> are enumerated in 5 exits.

I don't understand how could you end up with a fixed amount of vmexits. 
When I try to count with the batching as you wrote, the results depend 
very much on the amounts of supported coded and raw formats. I can 
easily have 50+ vmexits on our hardware.
And of course it only gets worse with the encoder, because you have a 
lot of controls to query.

> Having this option could also enable other optimizations, like the
> guest being able to queue all its CAPTURE buffers in a single vmexit,
> although I doubt this would result in a noticeable performance boost.

Queuing of many independent buffers is already supported by virtqueues. 
You can simply queue a bunch of commands, but trigger an interrupt only 
once.
The problem with the device capabilities is that in many cases you have 
to get the results of the previous command (batch) before you can submit 
a new one.

> But at the very least this should address concerns about noisy
> enumeration. I haven't noticed any slowness due to format enumeration
> with the current scheme, but the option is trivial to enable.

Well, I discussed the hack you mentioned and other ways to squeeze V4L2 
in virtio with some very experienced people in my team. Everybody says 
this is simply not the way how it is done in virtio. I think we both 
heard the same thing in the meeting a year ago.

>> In contrast virtio-video has been doing this in one command from day
>> one. Yes, the data was incomplete. That's why in draft v9 I added TLV as
>> a way to store all possible capabilities. I hope the linux-media folks
>> could take a look on it. Maybe this is something, that can be adopted in
>> V4L2?
>>
>> 2. virtio-media has a hard dependency on V4L2
>>
>> There are certainly some "patches" on top of V4L2 in virtio-media, like
>> the representation of the extended controls (which actually looks
>> similar to the representation of the controls in virtio-video) or
>> VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your
>> own ioctl ids in virtio-media?
>
> virtio-media doesn't add any ioctl ID and the extended controls use
> the same structure as their V4L2 counterparts.

Have you considered adding an ioctl ID to get all the device 
capabilities as a single blob?

>> AFAIK the linux-media maintainers have been overloaded for years, see
>> [4]. Would they be happy to deal with the additional requirements? Would
>> virtio community like to have a dependency here?
>
> The burden of having a couple new events in virtio-media seems lighter
> to me on the V4L2 maintainers than the one of having a whole new video
> protocol to maintain.

You've always wrote that virtio-video is the same thing as V4L2, but now 
it is a whole new video protocol all of a sudden.
In fact virtio-video allows to use the controls from V4L2, for example. 
Also the general flow is aligned IMHO.

I meant the potential new requirements for the V4L2 UAPI. I think it is 
a much more sensitive topic compared to only one of the many drivers. It 
is true, that the burden is going to increase in any way. But I wouldn't 
try to speculate on this, I'm not a V4L2 maintainer.

>> 2.1. an example: format modifiers
>>
>> There is a patchset aiming at unifying V4L2 pixel formats and extending
>> them with DRM format modifiers. It is now at version 7 submitted in
>> 2023, see [5]. The first version was submitted in 2019, see [6]. Not
>> merged yet AFAIK.
>>
>> In virtio-video I just added them in draft v9.
>>
>> I'm absolutely not trying to criticize here. I just try to highlight
>> that there is a lot of legacy and the process is painful. Right now we
>> have an opportunity to design a new API according to the current state
>> of the art of the stateful codecs.
>>
>> 3. uncertainty with cameras
>>
>> AFAIK there is still no agreement about how cameras should be
>> virtualized, see [3], [7], [8], [9]. virtio-media provides support for
>> cameras in a specific way, which might not be desirable.
>
> There is definitely an agreement on how cameras should work on V4L2 ;
> and thanks to that you can operate a camera using virtio-media,
> *today*.
>
> I'm sure the virtio-camera specification, driver, and host devices
> will be very impressive once they are released ; it's just that I
> would enjoy a solution, however imperfect, within my life span.

Well, you have the solution now. Nobody stops you from using it.

>> 4. (minor) is it possible/hard to implement the device in hardware/on a
>> micro controller?
>>
>> This is something I thought about recently, there might be a use-case
>> for it in the future. One of the concerns is that dynamic memory
>> allocations are IMHO inevitable in virtio-media or virtio-video up to
>> draft v8. I think multiple virtqueues in virtio-video draft v9 would
>> help here. Not sure yet...
>
> V4L2's MMAP buffer type has that covered.
>
>>
>> There are also other minor concerns, that are probably tolerable.
>>
>> [1] https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
>> [2] Page 6 in https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
>> [3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
>> [4] https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
>> [5] https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
>> [6] https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
>> [7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
>> [8] https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
>> [9] https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/
>> 


-- 
Alexander Gordeev
Senior Software Engineer

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Alexander Gordeev]

Hi Albert,

On 10.06.24 11:09, Albert Esteve wrote:
> Hi Alex, allow me to recover your message to repond.
> 
> On Fri, Jun 7, 2024 at 3:29 PM Alexander Gordeev 
> <alexander.gordeev@opensynergy.com 
> <mailto:alexander.gordeev@opensynergy.com>> wrote:
> 
>     Hi Albert,
> 
> 
>     I'd like to add some general considerations:
> 
>     1. virtio-media device capability discovery is very chatty
> 
>     V4L2 requires potentially hundreds of system calls to get the full view
>     of the device capabilities. This is inherited by virtio-media.
>     AFAIU V4L2 developers agree there is room for enhancement here, see [1],
>     [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
>     added for similar reasons.
> 
>       From the point of view of virtualization developers like my colleagues
>     and me hundreds of hypervisor "exits" are excessive and costly. It can
>     noticeably increase boot times, which is something we fight against in
>     automotive. AFAIU other virtio developers agree with this, see [3]. 
> 
> 
>     In contrast virtio-video has been doing this in one command from day
>     one. Yes, the data was incomplete. That's why in draft v9 I added TLV as
>     a way to store all possible capabilities. I hope the linux-media folks
>     could take a look on it. Maybe this is something, that can be adopted in
>     V4L2?
> 
> 
>     2. virtio-media has a hard dependency on V4L2
> 
>     There are certainly some "patches" on top of V4L2 in virtio-media, like
>     the representation of the extended controls (which actually looks
>     similar to the representation of the controls in virtio-video) or
>     VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your
>     own ioctl ids in virtio-media?
> 
>     AFAIK the linux-media maintainers have been overloaded for years, see
>     [4]. Would they be happy to deal with the additional requirements? Would
>     virtio community like to have a dependency here?
> 
>     2.1. an example: format modifiers
> 
>     There is a patchset aiming at unifying V4L2 pixel formats and extending
>     them with DRM format modifiers. It is now at version 7 submitted in
>     2023, see [5]. The first version was submitted in 2019, see [6]. Not
>     merged yet AFAIK.
> 
>     In virtio-video I just added them in draft v9.
> 
>     I'm absolutely not trying to criticize here. I just try to highlight
>     that there is a lot of legacy and the process is painful. Right now we
>     have an opportunity to design a new API according to the current state
>     of the art of the stateful codecs.
> 
>     3. uncertainty with cameras
> 
>     AFAIK there is still no agreement about how cameras should be
>     virtualized, see [3], [7], [8], [9]. virtio-media provides support for
>     cameras in a specific way, which might not be desirable.
> 
>     4. (minor) is it possible/hard to implement the device in hardware/on a
>     micro controller?
> 
>     This is something I thought about recently, there might be a use-case
>     for it in the future. One of the concerns is that dynamic memory
>     allocations are IMHO inevitable in virtio-media or virtio-video up to
>     draft v8. I think multiple virtqueues in virtio-video draft v9 would
>     help here. Not sure yet...
> 
>     There are also other minor concerns, that are probably tolerable.
> 
>     [1]
>     https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
>     [2] Page 6 in
>     https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
>     [3]
>     https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
>     [4]
>     https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
>     [5]
>     https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
>     [6]
>     https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
>     [7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
>     [8]
>     https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
>     [9]
>     https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/
> 
> 
> As I mentioned in the previous drop of the specs, I prefer not to
> re-open the debate video vs media. Most of the points raised here
> were already discussed previously, and yet it led nowhere as there
> was not real agreement.

Sorry, I don't agree, that it led nowhere. Now we have a more advanced 
specification that is aiming for standardization waiting for review and 
a new version of the driver with all the patches made by two teams 
merged together. And also (from my PoV) a compatibility solution ready 
for use. I think we achieved much more during the last year compared to 
two years before that.

> Personally, I think there are big advantages relying on a well 
> established API
> and lessons learned from v4l2, and using them for virtualizing a video
> device. That allowed to produce code for the driver and a reusable device,
> already available in the spec source repository for anyone to try:
> https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md

I also think there are big advantages relying on V4L2 for most things. 
That's what I tried to do in virtio-video too. The driver is also 
available, but it needs updating. I'm actively working on this. 
Unfortunately I can't provide a device implementation at the moment. 
Maybe it is time to discuss this again internally, but likely the answer 
would still be no. Still I'd be happy to work closely with you or anyone 
else, who is willing to implement an open source device.

> Also, new devices with new requisites appear, and the V4L2 API is forced to
> evolve fast. The virtio-media approach makes the spec less volatile, as it
> lets v4l2 do the heavy lifting for adapting.
> 
> But really, this is mainly to avoid having virtio-media as a downstream-only
> device. It also opens the spec to feedback from experts, and makes it
> more visible. There were already changes from v1 to v2 following the
> feedback, and these changes are also already reflected in the available
> driver and device implementations.

IMO virtio-media shouldn't be considered ready without discussing 
(again) and addressing the points above.

-- 
Alexander Gordeev
Senior Software Engineer

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Albert Esteve]

[-- Attachment #1: Type: text/plain, Size: 12934 bytes --]

Hi Alexander,

On Tue, Jun 11, 2024 at 12:10 PM Alexander Gordeev <
alexander.gordeev@opensynergy.com> wrote:

> Hi Albert, Alexandre,
>
> On 10.06.24 11:24, Albert Esteve wrote:
> > Just saw Alex Courbot's point-per-point response in the previous thread.
> > I'll link it here, as I'd prefer to move the discussion:
> >
> https://lore.kernel.org/all/CAPBb6MW36YaZFaWgksePQ=fMC7QffJD6ZpHN-iSzhLYp9Ho0sg@mail.gmail.com/
> >
> > Again, sorry for the confusion.
>
> I think I'll quote Alexandre's email and respond to him below to have it
> in the new place.
>
> Replying to Alexandre:
>
> On 08.06.24 03:50, Alexandre Courbot wrote:
> > On Fri, Jun 7, 2024 at 10:29 PM Alexander Gordeev
> > <alexander.gordeev@opensynergy.com> wrote:
> >>
> >> Hi Albert,
> >>
> >> On 07.06.24 10:00, Albert Esteve wrote:
> >>> Hi,
> >>>
> >>> This a formal attempt of including virtio-media
> >>> device specification.
>
> The thing is we already had a long email discussion. Unfortunately it
> didn't help to reach a consensus. You asked Cornelia to organize a
> meeting. So she did, there were a bunch of virtio and V4L2 experts
> invited by her. We both had an opportunity to bring our own experts to
> the discussion and to present our points of view. I presented the same
> arguments listed below in this email.
>
> I clearly remember, that all the experts told you, that you're going in
> a wrong direction, if your goal is standardization. I don't know what
> can I add on top of this. I think it is time to take the opinions of the
> experts and these arguments seriously, and stop pushing.
>
> Has anything changed since June 2023? I don't think so. I promised to
> continue the virtio-video development, I provided two drafts. I think,
> the last one addresses all previous comments and I'm myself satisfied
> with it. I'm also working on the driver update, I've published some
> intermediate results. You promised to make a downstream specification
> and implement it, you did it. Great, everything went according to the
> made decisions so far.
>
> You know, virtio-video is not a toy project too. AFAIK you can literally
> go and buy a mass market car with virtio-video on board since 2021. So
> our device and the driver are in production for years. Yes, the driver
> is based on draft v3/v4, there are problems, it needs updating. It is a
> work in progress right now. I hope to bring updates within a few months.
>

It is me who wanted to push for standardization. And I already gave
the reasoning. It is not my intention to torpedo virtio-video efforts.
I wish they did not split. But I now hope they can co-exist
in the VIRTIO standard, each covering specific usecases, and yeah,
with some overlapping.

Let's keep the discussion technical, to try to improve the specs :)


>
> >>> Virtio-media came from a discussion on virtio-dev
> >>> mailing list, which lead to presenting virtio-v4l2[1]
> >>> specification as an alternative to virtio-video.
> >>>
> >>> Later, virtio-v4l2 was renamed to virtio-media[2]
> >>> and published through:
> >>>
> >>> https://github.com/chromeos/virtio-media
> >>>
> >>> The repository above includes a virtio-media driver able
> >>> to pass v4l2-compliance when proxying the vivid/vicodec
> >>> virtual devices or an actual UVC camera using the
> >>> V4L2 vhost device (available in the repository).
> >>> Steps to reproduce are also detailed[3].
> >>>
> >>> There is some overlap with virtio-video in regards
> >>> to which devices it can handle. However,
> >>> as virtio-media will likely be the virtualization
> >>> solution for ChromeOS (already landed into the chromeos
> >>> organization) and possibly other Google projects for
> >>> media devices, it would be desirable to include the
> >>> specification in the next virtio release despite
> >>> the aforementioned overlap.
> >>>
> >>> The device ID in this document differs from
> >>> the ID in the virtio-media project repository.
> >>> And it will probably need some discussion on which
> >>> would be the correct definitive ID.
> >>>
> >>> Full PDF:
> https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
> >>> PDF with the media section only:
> https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing
> >>>
> >>> [1]
> https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
> >>> [2]
> https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html
> >>> [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
> >>>
> >>> Albert Esteve (1):
> >>>     virtio-media: Add virtio media device specification
> >>>
> >>>    conformance.tex                           |  13 +-
> >>>    content.tex                               |   1 +
> >>>    device-types/media/description.tex        | 578
> ++++++++++++++++++++++ >>>    device-types/media/device-conformance.tex |
> 11 +
> >>>    device-types/media/driver-conformance.tex |   9 +
> >>>    5 files changed, 608 insertions(+), 4 deletions(-)
> >>>    create mode 100644 device-types/media/description.tex
> >>>    create mode 100644 device-types/media/device-conformance.tex
> >>>    create mode 100644 device-types/media/driver-conformance.tex
> >>
> >> I'd like to add some general considerations:
> >>
> >> 1. virtio-media device capability discovery is very chatty
> >>
> >> V4L2 requires potentially hundreds of system calls to get the full view
> >> of the device capabilities. This is inherited by virtio-media.
> >> AFAIU V4L2 developers agree there is room for enhancement here, see [1],
> >> [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
> >> added for similar reasons.
> >>
> >>   From the point of view of virtualization developers like my colleagues
> >> and me hundreds of hypervisor "exits" are excessive and costly. It can
> >> noticeably increase boot times, which is something we fight against in
> >> automotive. AFAIU other virtio developers agree with this, see [3].
> >
> > There is a simple way to mitigate this: allow an arbitrary number of
> > commands to be submitted by the driver with each command buffer. The
> > host processes the commands sequentially and stops at the first error,
> > if any.
> >
> > That way, when enumerating formats, the guest can place a bunch of
> > VIDIOC_ENUM_FMT ioctls covering an arbitrary range 0..n in its command
> > buffer, and get all these formats in one vmexit instead of n. In the
> > case of a video decoder supporting 5 input formats, all the formats
> > are enumerated in 5 exits.
>
> I don't understand how could you end up with a fixed amount of vmexits.
> When I try to count with the batching as you wrote, the results depend
> very much on the amounts of supported coded and raw formats. I can
> easily have 50+ vmexits on our hardware.
> And of course it only gets worse with the encoder, because you have a
> lot of controls to query.
>
> > Having this option could also enable other optimizations, like the
> > guest being able to queue all its CAPTURE buffers in a single vmexit,
> > although I doubt this would result in a noticeable performance boost.
>
> Queuing of many independent buffers is already supported by virtqueues.
> You can simply queue a bunch of commands, but trigger an interrupt only
> once.
> The problem with the device capabilities is that in many cases you have
> to get the results of the previous command (batch) before you can submit
> a new one.
>
> > But at the very least this should address concerns about noisy
> > enumeration. I haven't noticed any slowness due to format enumeration
> > with the current scheme, but the option is trivial to enable.
>
> Well, I discussed the hack you mentioned and other ways to squeeze V4L2
> in virtio with some very experienced people in my team. Everybody says
> this is simply not the way how it is done in virtio. I think we both
> heard the same thing in the meeting a year ago.
>
> >> In contrast virtio-video has been doing this in one command from day
> >> one. Yes, the data was incomplete. That's why in draft v9 I added TLV as
> >> a way to store all possible capabilities. I hope the linux-media folks
> >> could take a look on it. Maybe this is something, that can be adopted in
> >> V4L2?
> >>
> >> 2. virtio-media has a hard dependency on V4L2
> >>
> >> There are certainly some "patches" on top of V4L2 in virtio-media, like
> >> the representation of the extended controls (which actually looks
> >> similar to the representation of the controls in virtio-video) or
> >> VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your
> >> own ioctl ids in virtio-media?
> >
> > virtio-media doesn't add any ioctl ID and the extended controls use
> > the same structure as their V4L2 counterparts.
>
> Have you considered adding an ioctl ID to get all the device
> capabilities as a single blob?
>
> >> AFAIK the linux-media maintainers have been overloaded for years, see
> >> [4]. Would they be happy to deal with the additional requirements? Would
> >> virtio community like to have a dependency here?
> >
> > The burden of having a couple new events in virtio-media seems lighter
> > to me on the V4L2 maintainers than the one of having a whole new video
> > protocol to maintain.
>
> You've always wrote that virtio-video is the same thing as V4L2, but now
> it is a whole new video protocol all of a sudden.
> In fact virtio-video allows to use the controls from V4L2, for example.
> Also the general flow is aligned IMHO.
>
> I meant the potential new requirements for the V4L2 UAPI. I think it is
> a much more sensitive topic compared to only one of the many drivers. It
> is true, that the burden is going to increase in any way. But I wouldn't
> try to speculate on this, I'm not a V4L2 maintainer.
>
> >> 2.1. an example: format modifiers
> >>
> >> There is a patchset aiming at unifying V4L2 pixel formats and extending
> >> them with DRM format modifiers. It is now at version 7 submitted in
> >> 2023, see [5]. The first version was submitted in 2019, see [6]. Not
> >> merged yet AFAIK.
> >>
> >> In virtio-video I just added them in draft v9.
> >>
> >> I'm absolutely not trying to criticize here. I just try to highlight
> >> that there is a lot of legacy and the process is painful. Right now we
> >> have an opportunity to design a new API according to the current state
> >> of the art of the stateful codecs.
> >>
> >> 3. uncertainty with cameras
> >>
> >> AFAIK there is still no agreement about how cameras should be
> >> virtualized, see [3], [7], [8], [9]. virtio-media provides support for
> >> cameras in a specific way, which might not be desirable.
> >
> > There is definitely an agreement on how cameras should work on V4L2 ;
> > and thanks to that you can operate a camera using virtio-media,
> > *today*.
> >
> > I'm sure the virtio-camera specification, driver, and host devices
> > will be very impressive once they are released ; it's just that I
> > would enjoy a solution, however imperfect, within my life span.
>
> Well, you have the solution now. Nobody stops you from using it.
>
> >> 4. (minor) is it possible/hard to implement the device in hardware/on a
> >> micro controller?
> >>
> >> This is something I thought about recently, there might be a use-case
> >> for it in the future. One of the concerns is that dynamic memory
> >> allocations are IMHO inevitable in virtio-media or virtio-video up to
> >> draft v8. I think multiple virtqueues in virtio-video draft v9 would
> >> help here. Not sure yet...
> >
> > V4L2's MMAP buffer type has that covered.
> >
> >>
> >> There are also other minor concerns, that are probably tolerable.
> >>
> >> [1]
> https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/
> >> [2] Page 6 in
> https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
> >> [3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
> >> [4]
> https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/
> >> [5]
> https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/
> >> [6]
> https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/
> >> [7] https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
> >> [8]
> https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/
> >> [9]
> https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/
> >>
>
>
> --
> Alexander Gordeev
> Senior Software Engineer
>
>

[-- Attachment #2: Type: text/html, Size: 17553 bytes --]

Re: [PATCH v2 0/1] virtio-media: Add device specification

[Alexander Gordeev]

Hi Albert,

On 11.06.24 14:24, Albert Esteve wrote:
> Hi Alexander,
> 
> On Tue, Jun 11, 2024 at 12:10 PM Alexander Gordeev 
> <alexander.gordeev@opensynergy.com 
> <mailto:alexander.gordeev@opensynergy.com>> wrote:
> 
>     Hi Albert, Alexandre,
> 
>     On 10.06.24 11:24, Albert Esteve wrote:
>      > Just saw Alex Courbot's point-per-point response in the previous
>     thread.
>      > I'll link it here, as I'd prefer to move the discussion:
>      >
>     https://lore.kernel.org/all/CAPBb6MW36YaZFaWgksePQ=fMC7QffJD6ZpHN-iSzhLYp9Ho0sg@mail.gmail.com/ <https://lore.kernel.org/all/CAPBb6MW36YaZFaWgksePQ=fMC7QffJD6ZpHN-iSzhLYp9Ho0sg@mail.gmail.com/>
>      >
>      > Again, sorry for the confusion.
> 
>     I think I'll quote Alexandre's email and respond to him below to
>     have it
>     in the new place.
> 
>     Replying to Alexandre:
> 
>     On 08.06.24 03:50, Alexandre Courbot wrote:
>      > On Fri, Jun 7, 2024 at 10:29 PM Alexander Gordeev
>      > <alexander.gordeev@opensynergy.com
>     <mailto:alexander.gordeev@opensynergy.com>> wrote:
>      >>
>      >> Hi Albert,
>      >>
>      >> On 07.06.24 10:00, Albert Esteve wrote:
>      >>> Hi,
>      >>>
>      >>> This a formal attempt of including virtio-media
>      >>> device specification.
> 
>     The thing is we already had a long email discussion. Unfortunately it
>     didn't help to reach a consensus. You asked Cornelia to organize a
>     meeting. So she did, there were a bunch of virtio and V4L2 experts
>     invited by her. We both had an opportunity to bring our own experts to
>     the discussion and to present our points of view. I presented the same
>     arguments listed below in this email.
> 
>     I clearly remember, that all the experts told you, that you're going in
>     a wrong direction, if your goal is standardization. I don't know what
>     can I add on top of this. I think it is time to take the opinions of
>     the
>     experts and these arguments seriously, and stop pushing.
> 
>     Has anything changed since June 2023? I don't think so. I promised to
>     continue the virtio-video development, I provided two drafts. I think,
>     the last one addresses all previous comments and I'm myself satisfied
>     with it. I'm also working on the driver update, I've published some
>     intermediate results. You promised to make a downstream specification
>     and implement it, you did it. Great, everything went according to the
>     made decisions so far.
> 
>     You know, virtio-video is not a toy project too. AFAIK you can
>     literally
>     go and buy a mass market car with virtio-video on board since 2021. So
>     our device and the driver are in production for years. Yes, the driver
>     is based on draft v3/v4, there are problems, it needs updating. It is a
>     work in progress right now. I hope to bring updates within a few months.
> 
> 
> It is me who wanted to push for standardization. And I already gave
> the reasoning. It is not my intention to torpedo virtio-video efforts.
> I wish they did not split. But I now hope they can co-exist
> in the VIRTIO standard, each covering specific usecases, and yeah,
> with some overlapping.

Thanks for the clarification. Personally, I'm not against merging the 
device specs even if there is some overlap. There are clearly use cases 
for both right now. It is embarrassing that we spent so much time 
discussing them as if it had to be one or the other, and even made a 
decision, only to go back to the beginning again.

OK, let's imagine they are both going to be in the spec:

1. I assume the presence of virtio-media must in no way exclude the 
development of virtio-camera, right?
2. It would be great if we could agree on guidelines for users to 
reasonably select one of the overlapping specs.
3. Both specs should be merged together as part of a single common patch 
set to make things fair.
I think this list is more or less fine, but please let me think more 
about other possible points. IMHO first this should be outlined as a 
detailed plan and then the plan should be voted upon, assuming there are 
no objections now. If this works, it would be really really exciting. 
What do you think?

> Let's keep the discussion technical, to try to improve the specs :)

I'm not sure it is possible without people equally actively reviewing 
virtio-video. :) I'm definitely in favor of the technical discussion 
too, of course.

>      >>> Virtio-media came from a discussion on virtio-dev
>      >>> mailing list, which lead to presenting virtio-v4l2[1]
>      >>> specification as an alternative to virtio-video.
>      >>>
>      >>> Later, virtio-v4l2 was renamed to virtio-media[2]
>      >>> and published through:
>      >>>
>      >>> https://github.com/chromeos/virtio-media
>     <https://github.com/chromeos/virtio-media>
>      >>>
>      >>> The repository above includes a virtio-media driver able
>      >>> to pass v4l2-compliance when proxying the vivid/vicodec
>      >>> virtual devices or an actual UVC camera using the
>      >>> V4L2 vhost device (available in the repository).
>      >>> Steps to reproduce are also detailed[3].
>      >>>
>      >>> There is some overlap with virtio-video in regards
>      >>> to which devices it can handle. However,
>      >>> as virtio-media will likely be the virtualization
>      >>> solution for ChromeOS (already landed into the chromeos
>      >>> organization) and possibly other Google projects for
>      >>> media devices, it would be desirable to include the
>      >>> specification in the next virtio release despite
>      >>> the aforementioned overlap.
>      >>>
>      >>> The device ID in this document differs from
>      >>> the ID in the virtio-media project repository.
>      >>> And it will probably need some discussion on which
>      >>> would be the correct definitive ID.
>      >>>
>      >>> Full PDF:
>     https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing <https://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdrive.google.com%2ffile%2fd%2f1pNCFP06VoV8Zuyx0aDVQ7HAT4xp%2dpZ0a%2fview%3fusp%3dsharing&umid=27cf87f3-325b-433c-ad31-9a67a735f346&auth=53c7c7de28b92dfd96e93d9dd61a23e634d2fbec-323bfac486c2d18aa0491476b4ae06765f5da7e5>
>      >>> PDF with the media section only:
>     https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing <https://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdrive.google.com%2ffile%2fd%2f1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T%2fview%3fusp%3dsharing&umid=27cf87f3-325b-433c-ad31-9a67a735f346&auth=53c7c7de28b92dfd96e93d9dd61a23e634d2fbec-0e87247aa6bb7ae56721dccc96b49eff785f0151>
>      >>>
>      >>> [1]
>     https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0 <https://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fmail.google.com%2fmail%2fu%2f0%3fui%3d2%26ik%3d73ebd65ebd%26attid%3d0.1%26permmsgid%3dmsg%2df%3a1767388565327924962%26th%3d1887068940754ee2%26view%3datt%26disp%3dinline%26realattid%3df%5flibalimc0&umid=27cf87f3-325b-433c-ad31-9a67a735f346&auth=53c7c7de28b92dfd96e93d9dd61a23e634d2fbec-eb99848afb29b5bb784fc4feea4af75f88b34acd>
>      >>> [2]
>     https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg12665.html <https://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fwww.mail%2darchive.com%2fvirtio%2ddev%40lists.oasis%2dopen.org%2fmsg12665.html&umid=27cf87f3-325b-433c-ad31-9a67a735f346&auth=53c7c7de28b92dfd96e93d9dd61a23e634d2fbec-0ae7cf33c4a9b38d4121fcdc95b8df11495ccc2e>
>      >>> [3]
>     https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
>     <https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md>
>      >>>
>      >>> Albert Esteve (1):
>      >>>     virtio-media: Add virtio media device specification
>      >>>
>      >>>    conformance.tex                           |  13 +-
>      >>>    content.tex                               |   1 +
>      >>>    device-types/media/description.tex        | 578
>     ++++++++++++++++++++++ >>>   
>     device-types/media/device-conformance.tex |  11 +
>      >>>    device-types/media/driver-conformance.tex |   9 +
>      >>>    5 files changed, 608 insertions(+), 4 deletions(-)
>      >>>    create mode 100644 device-types/media/description.tex
>      >>>    create mode 100644 device-types/media/device-conformance.tex
>      >>>    create mode 100644 device-types/media/driver-conformance.tex
>      >>
>      >> I'd like to add some general considerations:
>      >>
>      >> 1. virtio-media device capability discovery is very chatty
>      >>
>      >> V4L2 requires potentially hundreds of system calls to get the
>     full view
>      >> of the device capabilities. This is inherited by virtio-media.
>      >> AFAIU V4L2 developers agree there is room for enhancement here,
>     see [1],
>      >> [2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were
>      >> added for similar reasons.
>      >>
>      >>   From the point of view of virtualization developers like my
>     colleagues
>      >> and me hundreds of hypervisor "exits" are excessive and costly.
>     It can
>      >> noticeably increase boot times, which is something we fight
>     against in
>      >> automotive. AFAIU other virtio developers agree with this, see [3].
>      >
>      > There is a simple way to mitigate this: allow an arbitrary number of
>      > commands to be submitted by the driver with each command buffer. The
>      > host processes the commands sequentially and stops at the first
>     error,
>      > if any.
>      >
>      > That way, when enumerating formats, the guest can place a bunch of
>      > VIDIOC_ENUM_FMT ioctls covering an arbitrary range 0..n in its
>     command
>      > buffer, and get all these formats in one vmexit instead of n. In the
>      > case of a video decoder supporting 5 input formats, all the formats
>      > are enumerated in 5 exits.
> 
>     I don't understand how could you end up with a fixed amount of vmexits.
>     When I try to count with the batching as you wrote, the results depend
>     very much on the amounts of supported coded and raw formats. I can
>     easily have 50+ vmexits on our hardware.
>     And of course it only gets worse with the encoder, because you have a
>     lot of controls to query.
> 
>      > Having this option could also enable other optimizations, like the
>      > guest being able to queue all its CAPTURE buffers in a single vmexit,
>      > although I doubt this would result in a noticeable performance boost.
> 
>     Queuing of many independent buffers is already supported by virtqueues.
>     You can simply queue a bunch of commands, but trigger an interrupt only
>     once.
>     The problem with the device capabilities is that in many cases you have
>     to get the results of the previous command (batch) before you can
>     submit
>     a new one.
> 
>      > But at the very least this should address concerns about noisy
>      > enumeration. I haven't noticed any slowness due to format enumeration
>      > with the current scheme, but the option is trivial to enable.
> 
>     Well, I discussed the hack you mentioned and other ways to squeeze V4L2
>     in virtio with some very experienced people in my team. Everybody says
>     this is simply not the way how it is done in virtio. I think we both
>     heard the same thing in the meeting a year ago.
> 
>      >> In contrast virtio-video has been doing this in one command from day
>      >> one. Yes, the data was incomplete. That's why in draft v9 I
>     added TLV as
>      >> a way to store all possible capabilities. I hope the linux-media
>     folks
>      >> could take a look on it. Maybe this is something, that can be
>     adopted in
>      >> V4L2?
>      >>
>      >> 2. virtio-media has a hard dependency on V4L2
>      >>
>      >> There are certainly some "patches" on top of V4L2 in
>     virtio-media, like
>      >> the representation of the extended controls (which actually looks
>      >> similar to the representation of the controls in virtio-video) or
>      >> VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add
>     your
>      >> own ioctl ids in virtio-media?
>      >
>      > virtio-media doesn't add any ioctl ID and the extended controls use
>      > the same structure as their V4L2 counterparts.
> 
>     Have you considered adding an ioctl ID to get all the device
>     capabilities as a single blob?
> 
>      >> AFAIK the linux-media maintainers have been overloaded for
>     years, see
>      >> [4]. Would they be happy to deal with the additional
>     requirements? Would
>      >> virtio community like to have a dependency here?
>      >
>      > The burden of having a couple new events in virtio-media seems
>     lighter
>      > to me on the V4L2 maintainers than the one of having a whole new
>     video
>      > protocol to maintain.
> 
>     You've always wrote that virtio-video is the same thing as V4L2, but
>     now
>     it is a whole new video protocol all of a sudden.
>     In fact virtio-video allows to use the controls from V4L2, for example.
>     Also the general flow is aligned IMHO.
> 
>     I meant the potential new requirements for the V4L2 UAPI. I think it is
>     a much more sensitive topic compared to only one of the many
>     drivers. It
>     is true, that the burden is going to increase in any way. But I
>     wouldn't
>     try to speculate on this, I'm not a V4L2 maintainer.
> 
>      >> 2.1. an example: format modifiers
>      >>
>      >> There is a patchset aiming at unifying V4L2 pixel formats and
>     extending
>      >> them with DRM format modifiers. It is now at version 7 submitted in
>      >> 2023, see [5]. The first version was submitted in 2019, see [6]. Not
>      >> merged yet AFAIK.
>      >>
>      >> In virtio-video I just added them in draft v9.
>      >>
>      >> I'm absolutely not trying to criticize here. I just try to highlight
>      >> that there is a lot of legacy and the process is painful. Right
>     now we
>      >> have an opportunity to design a new API according to the current
>     state
>      >> of the art of the stateful codecs.
>      >>
>      >> 3. uncertainty with cameras
>      >>
>      >> AFAIK there is still no agreement about how cameras should be
>      >> virtualized, see [3], [7], [8], [9]. virtio-media provides
>     support for
>      >> cameras in a specific way, which might not be desirable.
>      >
>      > There is definitely an agreement on how cameras should work on V4L2 ;
>      > and thanks to that you can operate a camera using virtio-media,
>      > *today*.
>      >
>      > I'm sure the virtio-camera specification, driver, and host devices
>      > will be very impressive once they are released ; it's just that I
>      > would enjoy a solution, however imperfect, within my life span.
> 
>     Well, you have the solution now. Nobody stops you from using it.
> 
>      >> 4. (minor) is it possible/hard to implement the device in
>     hardware/on a
>      >> micro controller?
>      >>
>      >> This is something I thought about recently, there might be a
>     use-case
>      >> for it in the future. One of the concerns is that dynamic memory
>      >> allocations are IMHO inevitable in virtio-media or virtio-video
>     up to
>      >> draft v8. I think multiple virtqueues in virtio-video draft v9 would
>      >> help here. Not sure yet...
>      >
>      > V4L2's MMAP buffer type has that covered.
>      >
>      >>
>      >> There are also other minor concerns, that are probably tolerable.
>      >>
>      >> [1]
>     https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/ <https://lore.kernel.org/linux-media/20230922100303.GF19112@pendragon.ideasonboard.com/>
>      >> [2] Page 6 in
>     https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf <https://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fhverkuil.home.xs4all.nl%2fmediasummit2023%2dpdfs%2fHsia%2dJun%2520Li%2520%2d%2520V4L2%2520M2M%2520EXT%2520API%2520enhancement.pdf&umid=27cf87f3-325b-433c-ad31-9a67a735f346&auth=53c7c7de28b92dfd96e93d9dd61a23e634d2fbec-226bace75039ec8e39915f0f47af3bbfefe2c492>
>      >> [3]
>     https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
>     <https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/>
>      >> [4]
>     https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/ <https://lore.kernel.org/linux-media/4b6b1355-9baa-ff1e-e1c0-89dfdc83ad04@xs4all.nl/>
>      >> [5]
>     https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/ <https://lore.kernel.org/linux-media/20230206043308.28365-2-ayaka@soulik.info/>
>      >> [6]
>     https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/ <https://lore.kernel.org/linux-media/20190319145243.25047-1-boris.brezillon@collabora.com/>
>      >> [7]
>     https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/
>     <https://lore.kernel.org/virtio-dev/87354dtp30.fsf@linaro.org/>
>      >> [8]
>     https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/ <https://lore.kernel.org/linux-media/00f53c06-e66d-aa46-ca4f-c3baab6cf455@xs4all.nl/>
>      >> [9]
>     https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/ <https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/>
>      >>
> 
> 
>     -- 
>     Alexander Gordeev
>     Senior Software Engineer
> 

-- 
Alexander Gordeev
Senior Software Engineer

Re: [PATCH v2 1/1] virtio-media: Add virtio media device specification

[Hans Verkuil]

On 08/06/2024 03:24, Alexandre Courbot wrote:
> Hi Hans,
> 
> On Fri, Jun 7, 2024 at 5:43 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> Hi Albert,
>>
>>
>> On 07/06/2024 10:00, Albert Esteve wrote:
>>> Virtio-media is an encapsulation of the V4L2 UAPI into
>>> virtio, able to virtualize any video device supported
>>> by V4L2
>>>
>>> Note that virtio-media does not require the use of a
>>> V4L2 device driver or of Linux on the host or
>>> guest side - V4L2 is only used as a host-guest protocol,
>>> and both sides are free to convert it from/to any
>>> model that they wish to use.
>>
>> I just have two notes, one minor, one more substantial:
>>
>>>
>>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
>>> ---
>>>  conformance.tex                           |  13 +-
>>>  content.tex                               |   1 +
>>>  device-types/media/description.tex        | 578 ++++++++++++++++++++++
>>>  device-types/media/device-conformance.tex |  11 +
>>>  device-types/media/driver-conformance.tex |   9 +
>>>  5 files changed, 608 insertions(+), 4 deletions(-)
>>>  create mode 100644 device-types/media/description.tex
>>>  create mode 100644 device-types/media/device-conformance.tex
>>>  create mode 100644 device-types/media/driver-conformance.tex
>>>
>>> diff --git a/conformance.tex b/conformance.tex
>>> index dc00e84..c369da1 100644
>>> --- a/conformance.tex
>>> +++ b/conformance.tex
>>> @@ -32,8 +32,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
>>>  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
>>>  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
>>> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
>>> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
>>> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
>>> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
>>> +\ref{sec:Conformance / Driver Conformance / Media Driver Conformance}.
>>> +
>>>
>>>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>>    \end{itemize}
>>> @@ -59,8 +61,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
>>>  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
>>>  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
>>> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
>>> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
>>> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
>>> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
>>> +\ref{sec:Conformance / Device Conformance / Media Device Conformance}.
>>>
>>>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>>    \end{itemize}
>>> @@ -152,6 +155,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>  \input{device-types/scmi/driver-conformance.tex}
>>>  \input{device-types/gpio/driver-conformance.tex}
>>>  \input{device-types/pmem/driver-conformance.tex}
>>> +\input{device-types/media/driver-conformance.tex}
>>>
>>>  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>>>
>>> @@ -238,6 +242,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>  \input{device-types/scmi/device-conformance.tex}
>>>  \input{device-types/gpio/device-conformance.tex}
>>>  \input{device-types/pmem/device-conformance.tex}
>>> +\input{device-types/media/device-conformance.tex}
>>>
>>>  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
>>>  A conformant implementation MUST be either transitional or
>>> diff --git a/content.tex b/content.tex
>>> index 0a62dce..59925ae 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -767,6 +767,7 @@ \chapter{Device Types}\label{sec:Device Types}
>>>  \input{device-types/scmi/description.tex}
>>>  \input{device-types/gpio/description.tex}
>>>  \input{device-types/pmem/description.tex}
>>> +\input{device-types/media/description.tex}
>>>
>>>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>>>
>>> diff --git a/device-types/media/description.tex b/device-types/media/description.tex
>>> new file mode 100644
>>> index 0000000..887eacf
>>> --- /dev/null
>>> +++ b/device-types/media/description.tex
>>> @@ -0,0 +1,578 @@
>>> +\section{Media Device}\label{sec:Device Types / Media Device}
>>> +
>>> +The virtio media device follow the same model (and structures) as V4L2. It
>>> +can be used to virtualize cameras, codec devices, or any other device
>>> +supported by V4L2. The device assumes 64-bit little-endian V4L2 structures
>>> +are exchanged. The complete definition of V4L2 structures and ioctls can
>>> +be found under the
>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
>>> +
>>> +V4L2 is a UAPI that allows a less privileged entity (user-space) to use video
>>> +hardware exposed by a more privileged entity (the kernel). Virtio-media is an
>>> +encapsulation of this API into virtio, turning it into a virtualization API
>>> +for all classes of video devices supported by V4L2, where the device plays the
>>> +role of the kernel and the driver the role of user-space.
>>> +
>>> +The device is therefore responsible for presenting a virtual device that behaves
>>> +like an actual V4L2 device, which the driver can control.
>>> +
>>> +Note that virtio-media does not require the use of a V4L2 device driver or of
>>> +Linux on any side - V4L2 is only used as a transport protocol,
>>> +and both sides are free to convert it from/to any model that they wish to use.
>>> +
>>> +This section relies on definitions from
>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
>>> +
>>> +\subsection{Device ID}\label{sec:Device Types / Media Device / Device ID}
>>> +
>>> +42
>>> +
>>> +\subsection{Virtqueues}\label{sec:Device Types / Media Device / Virtqueues}
>>> +
>>> +\begin{description}
>>> +\item[0] commandq - used for driver commands and device responses to these
>>> +commands.
>>> +\item[1] eventq - used for events sent by the device to the driver.
>>> +\end{description}
>>> +
>>> +\devicenormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
>>> +
>>> +The device MUST return the descriptor chains it receives on the commandq as
>>> +soon as possible, and must never hold them for indefinite periods of time.
>>> +
>>> +\drivernormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
>>> +
>>> +The driver MUST re-queue the descriptor chains returned by the device on the
>>> +eventq as soon as possible, and must never hold them for indefinite periods
>>> +of time.
>>> +
>>> +\subsection{Feature Bits}\label{sec:Device Types / Media Device / Feature Bits}
>>> +
>>> +None
>>> +
>>> +\subsection{Device Configuration Layout}\label{sec:Device Types / Media Device / Device Configuration Layout}
>>> +
>>> +The video device configuration space uses the following layout:
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_config {
>>> +    le32 device_caps;
>>> +    le32 device_type;
>>> +    u8 card[32];
>>
>> Use char instead of u8. It's always been a pain that struct v4l2_capability
>> used u8 instead of char for the character arrays. I never understood why, and
>> if you are making a new struct I would recommend using char.
>>
>> Up to you, though.
>>
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{device_caps}] (driver-read-only) flags representing the device
>>> +capabilities as used in
>>> +\href{https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-querycap.html#c.v4l2_capability}{struct v4l2_capabilities}.
>>> +Corresponds with the \field{device_caps} field in the \textit{struct video_device}.
>>> +\item[\field{device_type}] (driver-read-only) informs the driver of the type
>>> +of the video device. Corresponds with the \field{vfl_devnode_type} field of the device.
>>> +\item[\field{card}] (driver-read-only) name of the device, a NUL-terminated
>>> +UTF-8 string. Corresponds with the \field{card} field of the \textit{struct v4l2_capability}.
>>> +If all the characters of the field are used, it does not need to be NUL-terminated.
>>> +\end{description}
>>> +
>>> +\subsection{Device Initialization}
>>> +
>>> +\begin{enumerate}
>>> +\item The driver reads the \field{device_caps} and \field{device_type} fields
>>> +from the configuration layout to identify the device.
>>> +\item The driver sets up the \field{commandq} and \field{eventq}.
>>> +\item The driver may open a session to use the device and send V4L2 ioctls in
>>> +order to receive more information about the device, such as supported
>>> +formats or controls.
>>> +\end{enumerate}
>>> +
>>> +\subsection{Device Operation}\label{sec:Device Types / Media Device / Device Operation}
>>> +
>>> +Commands are queued on the command queue by the driver for the device to
>>> +process. The errors returned by each command are standard
>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/gen-errors.html}{Linux kernel error codes}.
>>> +For instance, a command that contains invalid options will return \textit{EINVAL}.
>>> +
>>> +Events are sent on the event queue by the device for the driver to handle.
>>> +
>>> +\subsubsection{Command Virtqueue}
>>> +
>>> +\paragraph{Device Operation: Command headers}
>>> +
>>> +\begin{lstlisting}
>>> +#define VIRTIO_MEDIA_CMD_OPEN 1
>>> +#define VIRTIO_MEDIA_CMD_CLOSE 2
>>> +#define VIRTIO_MEDIA_CMD_IOCTL 3
>>> +#define VIRTIO_MEDIA_CMD_MMAP 4
>>> +#define VIRTIO_MEDIA_CMD_MUNMAP 5
>>> +
>>> +/* Header for all virtio commands from the driver to the device on the commandq. */
>>> +struct virtio_media_cmd_header {
>>> +     u32 cmd;
>>> +     u32 __padding;
>>> +};
>>> +
>>> +/* Header for all virtio responses from the device to the driver on the commandq. */
>>> +struct virtio_media_resp_header {
>>> +     u32 status;
>>> +     u32 __padding;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +A command consists of a command header \textit{virtio_media_cmd_header}
>>> +containing the following device-readable field:
>>> +
>>> +\begin{description}
>>> +\item[\field{cmd}] specifies a device request type (VIRTIO_MEDIA_CMD_*).
>>> +\end{description}
>>> +
>>> +A response consists of a response header \textit{virtio_media_resp_header}
>>> +containing the following device-writable field:
>>> +
>>> +\begin{description}
>>> +\item[\field{status}] indicates a device request status.
>>> +\end{description}
>>> +
>>> +The status field can take 0 if the command was successful, or one of the
>>> +standard Linux error codes if it was not.
>>> +
>>> +\drivernormative{\paragraph}{Device Operation: Command Virtqueue: Sessions}{Device Types / Media Device / Device Operation / Command Virtqueue}
>>> +
>>> +Sessions are how the device is multiplexed, allowing several distinct works to
>>> +take place simultaneously. The driver needs to open a session before it can
>>> +perform any useful operation on the device.
>>> +
>>> +\paragraph{Device Operation: Open device}
>>> +
>>> +\textbf{VIRTIO_MEDIA_CMD_OPEN} Command for creating a new session.
>>> +
>>> +This is the equivalent of calling \textit{open} on a V4L2 device node.
>>> +The driver uses \textit{virtio_media_cmd_open} to send an open request.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_cmd_open {
>>> +    struct virtio_media_cmd_header hdr;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_OPEN} with \textit{virtio_media_resp_open}.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_resp_open {
>>> +    struct virtio_media_resp_header hdr;
>>> +    u32 session_id;
>>> +    u32 __padding;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{session_id}] specifies an identifier for the current session. The
>>> +identifier can be used to perform other commands on the session, notably ioctls.
>>> +\end{description}
>>> +
>>> +\devicenormative{\subparagraph}{Device Operation: Open device}{Device Types / Media Device / Device Operation / Open device}
>>> +
>>> +Upon success, the device MUST set a \field{session_id} in \textit{virtio_media_resp_open}
>>> +to an integer that is NOT used by any other open session.
>>> +
>>> +\paragraph{Device Operation: Close device}
>>> +
>>> +\textbf{VIRTIO_MEDIA_CMD_CLOSE} Command for closing an active session.
>>> +
>>> +This is the equivalent of calling \textit{close} on a previously opened V4L2
>>> +device node. All resources associated with this session will be freed.
>>> +
>>> +This command does not require a response from the device.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_cmd_close {
>>> +    struct virtio_media_cmd_header hdr;
>>> +    u32 session_id;
>>> +    u32 __padding;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{session_id}] specifies an identifier for the session to close.
>>> +\end{description}
>>> +
>>> +\drivernormative{\subparagraph}{Device Operation: Close device}{Device Types / Media Device / Device Operation / Close device}
>>> +
>>> +The session ID SHALL NOT be used again after queueing this command.
>>> +
>>> +\paragraph{Device Operation: V4L2 ioctls}
>>> +
>>> +\textbf{VIRTIO_MEDIA_CMD_IOCTL} Command for executing an ioctl on an open
>>> +session.
>>> +
>>> +This command asks the device to run one of the `VIDIOC_*` ioctls on the active
>>> +session.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_cmd_ioctl {
>>> +    struct virtio_media_cmd_header hdr;
>>> +    u32 session_id;
>>> +    u32 code;
>>> +    /* Followed by the relevant ioctl payload as defined in the macro */
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{session_id}] specifies an identifier of thesession to run the ioctl on.
>>> +\item[\field{code}] specifies the code of the \field{VIDIOC_*} ioctl to run.
>>> +\end{description}
>>> +
>>> +The code is extracted from the
>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/videodev.html}{videodev2.h},
>>> +header file. The file defines the ioctl's codes, type of payload, and
>>> +direction. The code consists of the second argument of the \field{_IO*} macro.
>>> +
>>> +For example, the \textit{VIDIOC_G_FMT} is defined as follows:
>>> +
>>> +\begin{lstlisting}
>>> +#define VIDIOC_G_FMT _IOWR('V',  4, struct v4l2_format)
>>> +\end{lstlisting}
>>> +
>>> +This means that its ioctl code is \textit{4}, that its payload is a
>>> +\textit{struct v4l2_format}, and that its direction is \textit{WR} (i.e., the
>>> +payload is written by both the driver and the device).
>>> +See Section \ref{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
>>> +for more information about the direction of ioctls.
>>> +
>>> +The payload layout is always a 64-bit representation of the corresponding
>>> +V4L2 structure.
>>> +
>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_IOCTL} with \textit{virtio_media_resp_ioctl}.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_resp_ioctl {
>>> +    struct virtio_media_resp_header hdr;
>>> +    /* Followed by the ioctl payload as defined in the macro */
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\subparagraph{Ioctls payload}\label{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
>>> +
>>> +Each ioctl has a payload, which is defined by the third argument of the
>>> +\field{_IO*} macro defining it.
>>> +
>>> +The payload of an ioctl in the descriptor chain follows the command structure,
>>> +the reponse structure, or both depending on the direction:
>>> +
>>> +\begin{itemize}
>>> +\item \textbf{_IOR} is read-only for the driver, meaning the payload
>>> +follows the response in the device-writable section of the descriptor chain.
>>> +\item \textbf{_IOW} is read-only for the device, meaning the payload
>>> +follows the command in the driver-writable section of the descriptor chain.
>>> +\item \textbf{_IOWR} is writable by both the device and driver,
>>> +meaning the payload must follow both the command in the driver-writable section
>>> +of the descriptor chain, and the response in the device-writable section.
>>> +\end{itemize}
>>> +
>>> +A common optimization for \textit{WR} ioctls is to provide the payload using
>>> +descriptors that both point to the same buffer. This mimics the behavior of
>>> +V4L2 ioctls where the data is only passed once and used as both input and
>>> +output by the kernel.
>>> +
>>> +\devicenormative{\subparagraph}{Device Operation: V4L2 ioctls}{Device Types / Media Device / Device Operation / V4L2 ioctls}
>>> +
>>> +In case of success of a device-writable ioctl, the device MUST always write the
>>> +payload in the device-writable part of the descriptor chain.
>>> +
>>> +In case of failure of a device-writable ioctl, the device is free to write the
>>> +payload in the device-writable part of the descriptor chain or not. Some errors
>>> +may still result in the payload being updated, and in this case the device is
>>> +expected to write the updated payload. If the device has not written the
>>> +payload after an error, the driver MUST assume that the payload has not been
>>> +modified.
>>> +
>>> +\subparagraph{Handling of pointers in ioctl payload}
>>> +
>>> +A few structures used as ioctl payloads contain pointers to further
>>> +data needed for the ioctl. There are notably:
>>> +
>>> +\begin{itemize}
>>> +\item The \field{planes} pointer of
>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer}{struct v4l2_buffer},
>>> +which size is determined by the length member.
>>> +\item The \field{controls} pointer of \textit{struct v4l2_ext_controls}, which
>>> +size is determined by the count member.
>>> +\end{itemize}
>>> +
>>> +If the size of the pointed area is determined to be non-zero, then the main
>>> +payload is immediately followed by the pointed data in their order of
>>> +appearance in the structure, and the pointer value itself is ignored by the
>>> +device, which must also return the value initially passed by the driver.
>>> +
>>> +\subparagraph{Handling of pointers to userspace memory}
>>> +\label{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
>>> +
>>> +A few pointers are special in that they point to userspace memory in the
>>> +original V4L2 specification. They are:
>>> +
>>> +\begin{itemize}
>>> +\item The \field{m.userptr} member of \textit{struct v4l2_buffer} and
>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-plane}{struct v4l2_plane}
>>> +(technically an unsigned long, but designated a userspace address).
>>> +\item The \field{ptr} member of \textit{struct v4l2_ext_ctrl}.
>>> +\end{itemize}
>>> +
>>> +These pointers can cover large areas of scattered memory, which has the
>>
>> Actually, only m.userptr can cover a large area. The other two are limited.
>> See my comment about USERPTR support below.
>>
>>> +potential to require more descriptors than the virtio queue can provide. For
>>> +these particular pointers only, a list of \textit{struct virtio_media_sg_entry}
>>> +that covers the needed amount of memory for the pointer is used instead of
>>> +using descriptors to map the pointed memory directly.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_sg_entry {
>>> +     u64 start;
>>> +     u32 len;
>>> +     u32 __padding;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +For each such pointer to read, the device reads as many SG entries as needed
>>> +to cover the length of the pointed buffer, as described by its parent
>>> +structure (\field{length} member of \textit{struct v4l2_buffer} or
>>> +\textit{struct v4l2_plane} for buffer memory, and \field{size} member of
>>> +\textit{struct v4l2_ext_control} for control data).
>>> +
>>> +Since the device never needs to modify the list of SG entries, it is only
>>> +provided by the driver in the device-readable section of the descriptor chain,
>>> +and not repeated in the device-writable section, even for WR ioctls.
>>> +
>>> +\subparagraph{Unsupported ioctls}
>>> +
>>> +A few ioctls are replaced by other, more suitable mechanisms.
>>> +
>>> +\begin{itemize}
>>> +\item \textit{VIDIOC_QUERYCAP} is replaced by reading the configuration area
>>> +(see \ref{sec:Device Types / Media Device / Device Configuration Layout}).
>>> +\item \textit{VIDIOC_DQBUF} is replaced by a dedicated event
>>> +(see \ref{sec:Device Types / Media Device / Device Operation / Dequeue buffer}).
>>> +\item \textit{VIDIOC_DQEVENT} is replaced by a dedicated event
>>> +(see \ref{sec:Device Types / Media Device / Device Operation / Emit an event}).
>>> +\item \textit{VIDIOC_G_JPEGCOMP} and \textit{VIDIOC_S_JPEGCOMP} are deprecated
>>> +and replaced by the controls of the JPEG class.
>>> +\item \textit{VIDIOC_LOG_STATUS} is a driver-only operation and shall not be
>>> +implemented by the device.
>>> +\end{itemize}
>>> +
>>> +\devicenormative{\subparagraph}{Device Operation: Unsupported ioctls}{Device Types / Media Device / Device Operation / Unsupported ioctls}
>>> +
>>> +If being requested an unsupported ioctl, the device MUST return the same
>>> +error response as it would for an unknown ioctl, i.e. \textit{ENOTTY}.
>>> +
>>> +\paragraph{Device Operation: Mapping a MMAP buffer}
>>> +
>>> +\textbf{VIRTIO_MEDIA_CMD_MMAP} Command for mapping a MMAP buffer into the
>>> +driver's address space.
>>> +
>>> +Shared memory region ID 0 is used to map MMAP buffers with
>>> +the \textit{VIRTIO_MEDIA_CMD_MMAP} command.
>>> +
>>> +\begin{lstlisting}
>>> +#define VIRTIO_MEDIA_MMAP_FLAG_RW (1 << 0)
>>> +
>>> +struct virtio_media_cmd_mmap {
>>> +     struct virtio_media_cmd_header hdr;
>>> +     u32 session_id;
>>> +     u32 flags;
>>> +     u64 offset;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{flags}] is the set of flags for the mapping. \field{VIRTIO_MEDIA_MMAP_FLAG_RW}
>>> +can be set if a read-write mapping is desired. Without this flag the mapping
>>> +will be read-only.
>>> +\item[\field{offset}] corresponds to the \field{mem_offset} field of the
>>> +\textit{union v4l2_plane} for the plane to map. This field can be obtained
>>> +using the \textit{VIDIOC_QUERYBUF} ioctl.
>>> +\end{description}
>>> +
>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_MMAP} with \textit{virtio_media_resp_mmap}.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_resp_mmap {
>>> +    struct virtio_media_resp_header hdr;
>>> +    u64 offset;
>>> +    u64 len;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{offset}] offset into SHM region ID 0 of the start of the mapping.
>>> +\item[\field{len}] length of the mapping as indicated by the \textit{struct v4l2_plane}
>>> +the buffer belongs to.
>>> +\end{description}
>>> +
>>> +\paragraph{Device Operation: Unmapping a MMAP buffer}
>>> +
>>> +\textbf{VIRTIO_MEDIA_CMD_MUNMAP} Unmap a MMAP buffer previously mapped using \field{VIRTIO_MEDIA_CMD_MMAP}.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_cmd_munmap {
>>> +    struct virtio_media_cmd_header hdr;
>>> +    u64 offset;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{offset}] offset into SHM region ID 0 previously returned by
>>> +\textit{VIRTIO_MEDIA_CMD_MMAP} at which the buffer has been previously mapped.
>>> +\end{description}
>>> +
>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_MUNMAP} with \textit{virtio_media_resp_munmap}.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_resp_munmap {
>>> +    struct virtio_media_resp_header hdr;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\devicenormative{\subparagraph}{Device Operation: Unmapping a MMAP buffer}{Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
>>> +
>>> +The device MUST keep mappings performed using \textit{VIRTIO_MEDIA_CMD_MMAP}
>>> +valid until \textit{VIRTIO_MEDIA_CMD_MUNMAP} is called, even if the buffers or
>>> +session they belong to are released or closed by the driver.
>>> +
>>> +\paragraph{Device Operation: Memory Types}
>>> +
>>> +The semantics of the three V4L2 memory types (\textit{MMAP}, \textit{USERPTR}
>>> +and \textit{DMABUF}) can easily be mapped to both driver and device context.
>>> +
>>> +\subparagraph{MMAP}
>>> +
>>> +In virtio-media, \textit{MMAP} buffers are provisioned by the device, just like
>>> +they are by the kernel in regular V4L2. Similarly to how userspace can map a
>>> +\textit{MMAP} buffer into its address space using mmap and munmap, the
>>> +virtio-media driver can map device buffers into the driver space by queueing the
>>> +\textit{struct virtio_media_cmd_mmap} and \textit{struct virtio_media_cmd_munmap}
>>> +commands to the commandq.
>>> +
>>> +\subparagraph{USERPTR}
>>> +
>>> +In virtio-media, \textit{USERPTR} buffers are provisioned by the driver, just
>>> +like they are by userspace in regular V4L2. Instances of \textit{struct v4l2_buffer}
>>> +and \textit{struct v4l2_plane} of this type are followed by a list of
>>> +\textit{struct virtio_media_sg_entry}. For more information, see
>>> +\ref{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
>>> +
>>> +The device must not alter the pointer values provided by the driver, i.e.
>>> +\field{the m.userptr} member of \textit{struct v4l2_buffer} and
>>> +\textit{struct v4l2_plane} must be returned to the driver with the same value
>>> +as it was provided.
>>
>> I very, very strongly recommend that you drop USERPTR support for virtio.
>> MMAP and DMABUF are sufficient.
>>
>> It's a pain to support, and we discourage it for new drivers.
>>
>> If memory serves, there is at least one chromeos driver that is stuck to
>> USERPTR support for some reason (I can't remember the details), and that is
>> (I think) the only reason USERPTR support is part of virtio. But that is
>> really a chromeos issue that they need to fix, IMHO.
> 
> I am wondering if the concerns about USERPTR should apply to the
> host/guest context.
> 
> In virtio-media, the USERPTR memory type just means that the buffer
> memory has been allocated by the guest and the addresses passed are
> guest physical addresses. It doesn't necessarily mean that the memory
> has been allocated with the USERPTR type by the guest user-space.

Ah, OK. But then USERPTR is a really confusing name for people with a
V4L2 background.

Regardless, this needs to be explained better.

> For instance, say the guest user-space allocated memory using memfd,
> and is passing the FDs to the guest driver as DMABUF buffers. From the
> guest kernel point of view, these buffers will still resolve into
> guest physical memory, so the USERPTR memory type will be used by
> guest/host communication to indicate that fact. I don't know of any
> formal way to resolve guest DMABUFs into proper virtio resources
> unfortunately.
> 
> Without the ability to communicate buffers as guest physical memory,
> the only kinds of DMABUFs supported by the guest driver would be those
> coming from another virtio driver, like virtio-gpu. Granted, in
> practice that's probably where these will come from anyway, but I
> think it would be nice to be able to support memfd, at least for
> testing purposes.
> 
> But to answer what I think your concern is, if you prefer that the
> guest driver does not advertise support for USERPTR memory type to the
> guest userspace, we can certainly enforce that (either absolutely or
> through an option).

I think I would like that.

Regards,

	Hans

> 
> Cheers,
> Alex.
> 
>>
>> Regards,
>>
>>         Hans
>>
>>> +
>>> +\subparagraph{DMABUF}
>>> +
>>> +In virtio-media, \textit{DMABUF} buffers are provisioned by a virtio object,
>>> +just like they are by a \textit{DMABUF} in regular V4L2. Virtio objects are
>>> +16-bytes UUIDs and do not fit in the placeholders for file descriptors, so
>>> +they follow their embedding data structure as needed and the device must
>>> +leave the V4L2 structure placeholder unchanged.
>>> +
>>> +Contrary to \textit{USERPTR} buffers, virtio objects UUIDs need to be added in
>>> +both the device-readable and device-writable section of the descriptor chain.
>>> +
>>> +Device-allocated buffers with the \textit{V4L2_MEMORY_MMAP} memory type can also
>>> +be exported as virtio objects for use with another virtio device using the
>>> +\textit{VIDIOC_EXPBUF} ioctl. The fd placefolder of \textit{v4l2_exportbuffer}
>>> +means that space for the UUID needs to be reserved right after that structure
>>> +
>>> +\subsubsection{Event Virtqueue}
>>> +
>>> +Events are a way for the device to inform the driver about asynchronous events
>>> +that it should know about. In virtio-media, they are used as a replacement for
>>> +the \textit{VIDIOC_DQBUF} and \textit{VIDIOC_DQEVENT} ioctls and the polling
>>> +mechanism, which would be impractical to implement on top of virtio.
>>> +
>>> +\paragraph{Device Operation: Event header}
>>> +
>>> +\begin{lstlisting}
>>> +#define VIRTIO_MEDIA_EVT_ERROR 0
>>> +#define VIRTIO_MEDIA_EVT_DQBUF 1
>>> +#define VIRTIO_MEDIA_EVT_EVENT 2
>>> +
>>> +/* Header for events queued by the device for the driver on the eventq. */
>>> +struct virtio_media_event_header {
>>> +    u32 event;
>>> +    u32 session_id;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{event}] one of \field{VIRTIO_MEDIA_EVT_*}.
>>> +\item[\field{session_id}] ID of the session the event applies to.
>>> +\end{description}
>>> +
>>> +\paragraph{Device Operation: Device-side error}
>>> +
>>> +\textbf{VIRTIO_MEDIA_EVT_ERROR} Upon receiving this event, the session
>>> +mentioned in the header is considered corrupted and automatically closed by
>>> +the device.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_event_error {
>>> +    struct virtio_media_event_header hdr;
>>> +    u32 errno;
>>> +    u32 __padding;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{errno}] error code describing the kind of error that occurred.
>>> +\end{description}
>>> +
>>> +\paragraph{Device Operation: Dequeue buffer}
>>> +\label{sec:Device Types / Media Device / Device Operation / Dequeue buffer}
>>> +
>>> +\textbf{VIRTIO_MEDIA_EVT_DQBUF} Signals that a buffer is not being used anymore
>>> +by the device and is returned to the driver.
>>> +
>>> +A \textit{struct virtio_media_event_dqbuf} event is queued on the eventq by the
>>> +device every time a buffer previously queued using the \textit{VIDIOC_QBUF}
>>> +ioctl is done being processed and can be used by the driver again. This is like
>>> +an implicit \textit{VIDIOC_DQBUF} ioctl.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_event_dqbuf {
>>> +    struct virtio_media_event_header hdr;
>>> +    struct v4l2_buffer buffer;
>>> +    struct v4l2_plane planes[VIDEO_MAX_PLANES];
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{buffer}] \textit{struct v4l2_buffer} describing the buffer that has been dequeued.
>>> +\item[\field{planes}] array of \textit{struct v4l2_plane} containing the plane information for multi-planar buffers.
>>> +\end{description}
>>> +
>>> +Pointer values in the \textit{struct v4l2_buffer} and \textit{struct v4l2_plane}
>>> +are meaningless and must be ignored by the driver. It is recommended that the
>>> +device sets them to NULL in order to avoid leaking potential device addresses.
>>> +
>>> +Note that in the case of a \field{USERPTR} buffer, the \textit{struct v4l2_buffer}
>>> +used as event payload is not followed by the buffer memory: since that memory
>>> +is the same that the driver submitted with the \textit{VIDIOC_QBUF}, it would
>>> +be redundant to have it here.
>>> +
>>> +\paragraph{Device Operation: Emit an event}
>>> +\label{sec:Device Types / Media Device / Device Operation / Emit an event}
>>> +
>>> +\textbf{VIRTIO_MEDIA_EVT_EVENT} Signals that a V4L2 event has been emitted for a session.
>>> +
>>> +A \textit{struct virtio_media_event_event} event is queued on the eventq by the
>>> +device every time an event the driver previously subscribed to using the
>>> +\textit{VIDIOC_SUBSCRIBE_EVENT} ioctl has been signaled. This is like an
>>> +implicit \textit{VIDIOC_DQEVENT} ioctl.
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_media_event_event {
>>> +    struct virtio_media_event_header hdr;
>>> +    struct v4l2_event event;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +\begin{description}
>>> +\item[\field{event}] \textit{struct v4l2_event} describing the event that occurred.
>>> +\end{description}
>>> diff --git a/device-types/media/device-conformance.tex b/device-types/media/device-conformance.tex
>>> new file mode 100644
>>> index 0000000..3338822
>>> --- /dev/null
>>> +++ b/device-types/media/device-conformance.tex
>>> @@ -0,0 +1,11 @@
>>> +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Device Conformance / Media Device Conformance}
>>> +
>>> +A Media device MUST conform to the following normative statements:
>>> +
>>> +\begin{itemize}
>>> +\item \ref{devicenormative:Device Types / Media Device / Virtqueues}
>>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Open device}
>>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / V4L2 ioctls}
>>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unsupported ioctls}
>>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
>>> +\end{itemize}
>>> \ No newline at end of file
>>> diff --git a/device-types/media/driver-conformance.tex b/device-types/media/driver-conformance.tex
>>> new file mode 100644
>>> index 0000000..058b812
>>> --- /dev/null
>>> +++ b/device-types/media/driver-conformance.tex
>>> @@ -0,0 +1,9 @@
>>> +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Driver Conformance / Media Driver Conformance}
>>> +
>>> +A Media device MUST conform to the following normative statements:
>>> +
>>> +\begin{itemize}
>>> +\item \ref{drivernormative:Device Types / Media Device / Virtqueues}
>>> +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Command Virtqueue}
>>> +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Close device}
>>> +\end{itemize}
>>> \ No newline at end of file
>>

Re: [PATCH v2 1/1] virtio-media: Add virtio media device specification

[Alexandre Courbot]

On Thu, Jun 20, 2024 at 9:45 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 08/06/2024 03:24, Alexandre Courbot wrote:
> > Hi Hans,
> >
> > On Fri, Jun 7, 2024 at 5:43 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> Hi Albert,
> >>
> >>
> >> On 07/06/2024 10:00, Albert Esteve wrote:
> >>> Virtio-media is an encapsulation of the V4L2 UAPI into
> >>> virtio, able to virtualize any video device supported
> >>> by V4L2
> >>>
> >>> Note that virtio-media does not require the use of a
> >>> V4L2 device driver or of Linux on the host or
> >>> guest side - V4L2 is only used as a host-guest protocol,
> >>> and both sides are free to convert it from/to any
> >>> model that they wish to use.
> >>
> >> I just have two notes, one minor, one more substantial:
> >>
> >>>
> >>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> >>> ---
> >>>  conformance.tex                           |  13 +-
> >>>  content.tex                               |   1 +
> >>>  device-types/media/description.tex        | 578 ++++++++++++++++++++++
> >>>  device-types/media/device-conformance.tex |  11 +
> >>>  device-types/media/driver-conformance.tex |   9 +
> >>>  5 files changed, 608 insertions(+), 4 deletions(-)
> >>>  create mode 100644 device-types/media/description.tex
> >>>  create mode 100644 device-types/media/device-conformance.tex
> >>>  create mode 100644 device-types/media/driver-conformance.tex
> >>>
> >>> diff --git a/conformance.tex b/conformance.tex
> >>> index dc00e84..c369da1 100644
> >>> --- a/conformance.tex
> >>> +++ b/conformance.tex
> >>> @@ -32,8 +32,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >>>  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
> >>>  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
> >>>  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
> >>> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
> >>> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
> >>> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
> >>> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
> >>> +\ref{sec:Conformance / Driver Conformance / Media Driver Conformance}.
> >>> +
> >>>
> >>>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
> >>>    \end{itemize}
> >>> @@ -59,8 +61,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >>>  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
> >>>  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
> >>>  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
> >>> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
> >>> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
> >>> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
> >>> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
> >>> +\ref{sec:Conformance / Device Conformance / Media Device Conformance}.
> >>>
> >>>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
> >>>    \end{itemize}
> >>> @@ -152,6 +155,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >>>  \input{device-types/scmi/driver-conformance.tex}
> >>>  \input{device-types/gpio/driver-conformance.tex}
> >>>  \input{device-types/pmem/driver-conformance.tex}
> >>> +\input{device-types/media/driver-conformance.tex}
> >>>
> >>>  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
> >>>
> >>> @@ -238,6 +242,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >>>  \input{device-types/scmi/device-conformance.tex}
> >>>  \input{device-types/gpio/device-conformance.tex}
> >>>  \input{device-types/pmem/device-conformance.tex}
> >>> +\input{device-types/media/device-conformance.tex}
> >>>
> >>>  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
> >>>  A conformant implementation MUST be either transitional or
> >>> diff --git a/content.tex b/content.tex
> >>> index 0a62dce..59925ae 100644
> >>> --- a/content.tex
> >>> +++ b/content.tex
> >>> @@ -767,6 +767,7 @@ \chapter{Device Types}\label{sec:Device Types}
> >>>  \input{device-types/scmi/description.tex}
> >>>  \input{device-types/gpio/description.tex}
> >>>  \input{device-types/pmem/description.tex}
> >>> +\input{device-types/media/description.tex}
> >>>
> >>>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >>>
> >>> diff --git a/device-types/media/description.tex b/device-types/media/description.tex
> >>> new file mode 100644
> >>> index 0000000..887eacf
> >>> --- /dev/null
> >>> +++ b/device-types/media/description.tex
> >>> @@ -0,0 +1,578 @@
> >>> +\section{Media Device}\label{sec:Device Types / Media Device}
> >>> +
> >>> +The virtio media device follow the same model (and structures) as V4L2. It
> >>> +can be used to virtualize cameras, codec devices, or any other device
> >>> +supported by V4L2. The device assumes 64-bit little-endian V4L2 structures
> >>> +are exchanged. The complete definition of V4L2 structures and ioctls can
> >>> +be found under the
> >>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
> >>> +
> >>> +V4L2 is a UAPI that allows a less privileged entity (user-space) to use video
> >>> +hardware exposed by a more privileged entity (the kernel). Virtio-media is an
> >>> +encapsulation of this API into virtio, turning it into a virtualization API
> >>> +for all classes of video devices supported by V4L2, where the device plays the
> >>> +role of the kernel and the driver the role of user-space.
> >>> +
> >>> +The device is therefore responsible for presenting a virtual device that behaves
> >>> +like an actual V4L2 device, which the driver can control.
> >>> +
> >>> +Note that virtio-media does not require the use of a V4L2 device driver or of
> >>> +Linux on any side - V4L2 is only used as a transport protocol,
> >>> +and both sides are free to convert it from/to any model that they wish to use.
> >>> +
> >>> +This section relies on definitions from
> >>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
> >>> +
> >>> +\subsection{Device ID}\label{sec:Device Types / Media Device / Device ID}
> >>> +
> >>> +42
> >>> +
> >>> +\subsection{Virtqueues}\label{sec:Device Types / Media Device / Virtqueues}
> >>> +
> >>> +\begin{description}
> >>> +\item[0] commandq - used for driver commands and device responses to these
> >>> +commands.
> >>> +\item[1] eventq - used for events sent by the device to the driver.
> >>> +\end{description}
> >>> +
> >>> +\devicenormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
> >>> +
> >>> +The device MUST return the descriptor chains it receives on the commandq as
> >>> +soon as possible, and must never hold them for indefinite periods of time.
> >>> +
> >>> +\drivernormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
> >>> +
> >>> +The driver MUST re-queue the descriptor chains returned by the device on the
> >>> +eventq as soon as possible, and must never hold them for indefinite periods
> >>> +of time.
> >>> +
> >>> +\subsection{Feature Bits}\label{sec:Device Types / Media Device / Feature Bits}
> >>> +
> >>> +None
> >>> +
> >>> +\subsection{Device Configuration Layout}\label{sec:Device Types / Media Device / Device Configuration Layout}
> >>> +
> >>> +The video device configuration space uses the following layout:
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_config {
> >>> +    le32 device_caps;
> >>> +    le32 device_type;
> >>> +    u8 card[32];
> >>
> >> Use char instead of u8. It's always been a pain that struct v4l2_capability
> >> used u8 instead of char for the character arrays. I never understood why, and
> >> if you are making a new struct I would recommend using char.
> >>
> >> Up to you, though.
> >>
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{device_caps}] (driver-read-only) flags representing the device
> >>> +capabilities as used in
> >>> +\href{https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-querycap.html#c.v4l2_capability}{struct v4l2_capabilities}.
> >>> +Corresponds with the \field{device_caps} field in the \textit{struct video_device}.
> >>> +\item[\field{device_type}] (driver-read-only) informs the driver of the type
> >>> +of the video device. Corresponds with the \field{vfl_devnode_type} field of the device.
> >>> +\item[\field{card}] (driver-read-only) name of the device, a NUL-terminated
> >>> +UTF-8 string. Corresponds with the \field{card} field of the \textit{struct v4l2_capability}.
> >>> +If all the characters of the field are used, it does not need to be NUL-terminated.
> >>> +\end{description}
> >>> +
> >>> +\subsection{Device Initialization}
> >>> +
> >>> +\begin{enumerate}
> >>> +\item The driver reads the \field{device_caps} and \field{device_type} fields
> >>> +from the configuration layout to identify the device.
> >>> +\item The driver sets up the \field{commandq} and \field{eventq}.
> >>> +\item The driver may open a session to use the device and send V4L2 ioctls in
> >>> +order to receive more information about the device, such as supported
> >>> +formats or controls.
> >>> +\end{enumerate}
> >>> +
> >>> +\subsection{Device Operation}\label{sec:Device Types / Media Device / Device Operation}
> >>> +
> >>> +Commands are queued on the command queue by the driver for the device to
> >>> +process. The errors returned by each command are standard
> >>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/gen-errors.html}{Linux kernel error codes}.
> >>> +For instance, a command that contains invalid options will return \textit{EINVAL}.
> >>> +
> >>> +Events are sent on the event queue by the device for the driver to handle.
> >>> +
> >>> +\subsubsection{Command Virtqueue}
> >>> +
> >>> +\paragraph{Device Operation: Command headers}
> >>> +
> >>> +\begin{lstlisting}
> >>> +#define VIRTIO_MEDIA_CMD_OPEN 1
> >>> +#define VIRTIO_MEDIA_CMD_CLOSE 2
> >>> +#define VIRTIO_MEDIA_CMD_IOCTL 3
> >>> +#define VIRTIO_MEDIA_CMD_MMAP 4
> >>> +#define VIRTIO_MEDIA_CMD_MUNMAP 5
> >>> +
> >>> +/* Header for all virtio commands from the driver to the device on the commandq. */
> >>> +struct virtio_media_cmd_header {
> >>> +     u32 cmd;
> >>> +     u32 __padding;
> >>> +};
> >>> +
> >>> +/* Header for all virtio responses from the device to the driver on the commandq. */
> >>> +struct virtio_media_resp_header {
> >>> +     u32 status;
> >>> +     u32 __padding;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +A command consists of a command header \textit{virtio_media_cmd_header}
> >>> +containing the following device-readable field:
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{cmd}] specifies a device request type (VIRTIO_MEDIA_CMD_*).
> >>> +\end{description}
> >>> +
> >>> +A response consists of a response header \textit{virtio_media_resp_header}
> >>> +containing the following device-writable field:
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{status}] indicates a device request status.
> >>> +\end{description}
> >>> +
> >>> +The status field can take 0 if the command was successful, or one of the
> >>> +standard Linux error codes if it was not.
> >>> +
> >>> +\drivernormative{\paragraph}{Device Operation: Command Virtqueue: Sessions}{Device Types / Media Device / Device Operation / Command Virtqueue}
> >>> +
> >>> +Sessions are how the device is multiplexed, allowing several distinct works to
> >>> +take place simultaneously. The driver needs to open a session before it can
> >>> +perform any useful operation on the device.
> >>> +
> >>> +\paragraph{Device Operation: Open device}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_CMD_OPEN} Command for creating a new session.
> >>> +
> >>> +This is the equivalent of calling \textit{open} on a V4L2 device node.
> >>> +The driver uses \textit{virtio_media_cmd_open} to send an open request.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_cmd_open {
> >>> +    struct virtio_media_cmd_header hdr;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +The device responds to \textit{VIRTIO_MEDIA_CMD_OPEN} with \textit{virtio_media_resp_open}.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_resp_open {
> >>> +    struct virtio_media_resp_header hdr;
> >>> +    u32 session_id;
> >>> +    u32 __padding;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{session_id}] specifies an identifier for the current session. The
> >>> +identifier can be used to perform other commands on the session, notably ioctls.
> >>> +\end{description}
> >>> +
> >>> +\devicenormative{\subparagraph}{Device Operation: Open device}{Device Types / Media Device / Device Operation / Open device}
> >>> +
> >>> +Upon success, the device MUST set a \field{session_id} in \textit{virtio_media_resp_open}
> >>> +to an integer that is NOT used by any other open session.
> >>> +
> >>> +\paragraph{Device Operation: Close device}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_CMD_CLOSE} Command for closing an active session.
> >>> +
> >>> +This is the equivalent of calling \textit{close} on a previously opened V4L2
> >>> +device node. All resources associated with this session will be freed.
> >>> +
> >>> +This command does not require a response from the device.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_cmd_close {
> >>> +    struct virtio_media_cmd_header hdr;
> >>> +    u32 session_id;
> >>> +    u32 __padding;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{session_id}] specifies an identifier for the session to close.
> >>> +\end{description}
> >>> +
> >>> +\drivernormative{\subparagraph}{Device Operation: Close device}{Device Types / Media Device / Device Operation / Close device}
> >>> +
> >>> +The session ID SHALL NOT be used again after queueing this command.
> >>> +
> >>> +\paragraph{Device Operation: V4L2 ioctls}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_CMD_IOCTL} Command for executing an ioctl on an open
> >>> +session.
> >>> +
> >>> +This command asks the device to run one of the `VIDIOC_*` ioctls on the active
> >>> +session.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_cmd_ioctl {
> >>> +    struct virtio_media_cmd_header hdr;
> >>> +    u32 session_id;
> >>> +    u32 code;
> >>> +    /* Followed by the relevant ioctl payload as defined in the macro */
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{session_id}] specifies an identifier of thesession to run the ioctl on.
> >>> +\item[\field{code}] specifies the code of the \field{VIDIOC_*} ioctl to run.
> >>> +\end{description}
> >>> +
> >>> +The code is extracted from the
> >>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/videodev.html}{videodev2.h},
> >>> +header file. The file defines the ioctl's codes, type of payload, and
> >>> +direction. The code consists of the second argument of the \field{_IO*} macro.
> >>> +
> >>> +For example, the \textit{VIDIOC_G_FMT} is defined as follows:
> >>> +
> >>> +\begin{lstlisting}
> >>> +#define VIDIOC_G_FMT _IOWR('V',  4, struct v4l2_format)
> >>> +\end{lstlisting}
> >>> +
> >>> +This means that its ioctl code is \textit{4}, that its payload is a
> >>> +\textit{struct v4l2_format}, and that its direction is \textit{WR} (i.e., the
> >>> +payload is written by both the driver and the device).
> >>> +See Section \ref{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
> >>> +for more information about the direction of ioctls.
> >>> +
> >>> +The payload layout is always a 64-bit representation of the corresponding
> >>> +V4L2 structure.
> >>> +
> >>> +The device responds to \textit{VIRTIO_MEDIA_CMD_IOCTL} with \textit{virtio_media_resp_ioctl}.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_resp_ioctl {
> >>> +    struct virtio_media_resp_header hdr;
> >>> +    /* Followed by the ioctl payload as defined in the macro */
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\subparagraph{Ioctls payload}\label{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
> >>> +
> >>> +Each ioctl has a payload, which is defined by the third argument of the
> >>> +\field{_IO*} macro defining it.
> >>> +
> >>> +The payload of an ioctl in the descriptor chain follows the command structure,
> >>> +the reponse structure, or both depending on the direction:
> >>> +
> >>> +\begin{itemize}
> >>> +\item \textbf{_IOR} is read-only for the driver, meaning the payload
> >>> +follows the response in the device-writable section of the descriptor chain.
> >>> +\item \textbf{_IOW} is read-only for the device, meaning the payload
> >>> +follows the command in the driver-writable section of the descriptor chain.
> >>> +\item \textbf{_IOWR} is writable by both the device and driver,
> >>> +meaning the payload must follow both the command in the driver-writable section
> >>> +of the descriptor chain, and the response in the device-writable section.
> >>> +\end{itemize}
> >>> +
> >>> +A common optimization for \textit{WR} ioctls is to provide the payload using
> >>> +descriptors that both point to the same buffer. This mimics the behavior of
> >>> +V4L2 ioctls where the data is only passed once and used as both input and
> >>> +output by the kernel.
> >>> +
> >>> +\devicenormative{\subparagraph}{Device Operation: V4L2 ioctls}{Device Types / Media Device / Device Operation / V4L2 ioctls}
> >>> +
> >>> +In case of success of a device-writable ioctl, the device MUST always write the
> >>> +payload in the device-writable part of the descriptor chain.
> >>> +
> >>> +In case of failure of a device-writable ioctl, the device is free to write the
> >>> +payload in the device-writable part of the descriptor chain or not. Some errors
> >>> +may still result in the payload being updated, and in this case the device is
> >>> +expected to write the updated payload. If the device has not written the
> >>> +payload after an error, the driver MUST assume that the payload has not been
> >>> +modified.
> >>> +
> >>> +\subparagraph{Handling of pointers in ioctl payload}
> >>> +
> >>> +A few structures used as ioctl payloads contain pointers to further
> >>> +data needed for the ioctl. There are notably:
> >>> +
> >>> +\begin{itemize}
> >>> +\item The \field{planes} pointer of
> >>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer}{struct v4l2_buffer},
> >>> +which size is determined by the length member.
> >>> +\item The \field{controls} pointer of \textit{struct v4l2_ext_controls}, which
> >>> +size is determined by the count member.
> >>> +\end{itemize}
> >>> +
> >>> +If the size of the pointed area is determined to be non-zero, then the main
> >>> +payload is immediately followed by the pointed data in their order of
> >>> +appearance in the structure, and the pointer value itself is ignored by the
> >>> +device, which must also return the value initially passed by the driver.
> >>> +
> >>> +\subparagraph{Handling of pointers to userspace memory}
> >>> +\label{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
> >>> +
> >>> +A few pointers are special in that they point to userspace memory in the
> >>> +original V4L2 specification. They are:
> >>> +
> >>> +\begin{itemize}
> >>> +\item The \field{m.userptr} member of \textit{struct v4l2_buffer} and
> >>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-plane}{struct v4l2_plane}
> >>> +(technically an unsigned long, but designated a userspace address).
> >>> +\item The \field{ptr} member of \textit{struct v4l2_ext_ctrl}.
> >>> +\end{itemize}
> >>> +
> >>> +These pointers can cover large areas of scattered memory, which has the
> >>
> >> Actually, only m.userptr can cover a large area. The other two are limited.
> >> See my comment about USERPTR support below.
> >>
> >>> +potential to require more descriptors than the virtio queue can provide. For
> >>> +these particular pointers only, a list of \textit{struct virtio_media_sg_entry}
> >>> +that covers the needed amount of memory for the pointer is used instead of
> >>> +using descriptors to map the pointed memory directly.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_sg_entry {
> >>> +     u64 start;
> >>> +     u32 len;
> >>> +     u32 __padding;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +For each such pointer to read, the device reads as many SG entries as needed
> >>> +to cover the length of the pointed buffer, as described by its parent
> >>> +structure (\field{length} member of \textit{struct v4l2_buffer} or
> >>> +\textit{struct v4l2_plane} for buffer memory, and \field{size} member of
> >>> +\textit{struct v4l2_ext_control} for control data).
> >>> +
> >>> +Since the device never needs to modify the list of SG entries, it is only
> >>> +provided by the driver in the device-readable section of the descriptor chain,
> >>> +and not repeated in the device-writable section, even for WR ioctls.
> >>> +
> >>> +\subparagraph{Unsupported ioctls}
> >>> +
> >>> +A few ioctls are replaced by other, more suitable mechanisms.
> >>> +
> >>> +\begin{itemize}
> >>> +\item \textit{VIDIOC_QUERYCAP} is replaced by reading the configuration area
> >>> +(see \ref{sec:Device Types / Media Device / Device Configuration Layout}).
> >>> +\item \textit{VIDIOC_DQBUF} is replaced by a dedicated event
> >>> +(see \ref{sec:Device Types / Media Device / Device Operation / Dequeue buffer}).
> >>> +\item \textit{VIDIOC_DQEVENT} is replaced by a dedicated event
> >>> +(see \ref{sec:Device Types / Media Device / Device Operation / Emit an event}).
> >>> +\item \textit{VIDIOC_G_JPEGCOMP} and \textit{VIDIOC_S_JPEGCOMP} are deprecated
> >>> +and replaced by the controls of the JPEG class.
> >>> +\item \textit{VIDIOC_LOG_STATUS} is a driver-only operation and shall not be
> >>> +implemented by the device.
> >>> +\end{itemize}
> >>> +
> >>> +\devicenormative{\subparagraph}{Device Operation: Unsupported ioctls}{Device Types / Media Device / Device Operation / Unsupported ioctls}
> >>> +
> >>> +If being requested an unsupported ioctl, the device MUST return the same
> >>> +error response as it would for an unknown ioctl, i.e. \textit{ENOTTY}.
> >>> +
> >>> +\paragraph{Device Operation: Mapping a MMAP buffer}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_CMD_MMAP} Command for mapping a MMAP buffer into the
> >>> +driver's address space.
> >>> +
> >>> +Shared memory region ID 0 is used to map MMAP buffers with
> >>> +the \textit{VIRTIO_MEDIA_CMD_MMAP} command.
> >>> +
> >>> +\begin{lstlisting}
> >>> +#define VIRTIO_MEDIA_MMAP_FLAG_RW (1 << 0)
> >>> +
> >>> +struct virtio_media_cmd_mmap {
> >>> +     struct virtio_media_cmd_header hdr;
> >>> +     u32 session_id;
> >>> +     u32 flags;
> >>> +     u64 offset;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{flags}] is the set of flags for the mapping. \field{VIRTIO_MEDIA_MMAP_FLAG_RW}
> >>> +can be set if a read-write mapping is desired. Without this flag the mapping
> >>> +will be read-only.
> >>> +\item[\field{offset}] corresponds to the \field{mem_offset} field of the
> >>> +\textit{union v4l2_plane} for the plane to map. This field can be obtained
> >>> +using the \textit{VIDIOC_QUERYBUF} ioctl.
> >>> +\end{description}
> >>> +
> >>> +The device responds to \textit{VIRTIO_MEDIA_CMD_MMAP} with \textit{virtio_media_resp_mmap}.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_resp_mmap {
> >>> +    struct virtio_media_resp_header hdr;
> >>> +    u64 offset;
> >>> +    u64 len;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{offset}] offset into SHM region ID 0 of the start of the mapping.
> >>> +\item[\field{len}] length of the mapping as indicated by the \textit{struct v4l2_plane}
> >>> +the buffer belongs to.
> >>> +\end{description}
> >>> +
> >>> +\paragraph{Device Operation: Unmapping a MMAP buffer}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_CMD_MUNMAP} Unmap a MMAP buffer previously mapped using \field{VIRTIO_MEDIA_CMD_MMAP}.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_cmd_munmap {
> >>> +    struct virtio_media_cmd_header hdr;
> >>> +    u64 offset;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{offset}] offset into SHM region ID 0 previously returned by
> >>> +\textit{VIRTIO_MEDIA_CMD_MMAP} at which the buffer has been previously mapped.
> >>> +\end{description}
> >>> +
> >>> +The device responds to \textit{VIRTIO_MEDIA_CMD_MUNMAP} with \textit{virtio_media_resp_munmap}.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_resp_munmap {
> >>> +    struct virtio_media_resp_header hdr;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\devicenormative{\subparagraph}{Device Operation: Unmapping a MMAP buffer}{Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
> >>> +
> >>> +The device MUST keep mappings performed using \textit{VIRTIO_MEDIA_CMD_MMAP}
> >>> +valid until \textit{VIRTIO_MEDIA_CMD_MUNMAP} is called, even if the buffers or
> >>> +session they belong to are released or closed by the driver.
> >>> +
> >>> +\paragraph{Device Operation: Memory Types}
> >>> +
> >>> +The semantics of the three V4L2 memory types (\textit{MMAP}, \textit{USERPTR}
> >>> +and \textit{DMABUF}) can easily be mapped to both driver and device context.
> >>> +
> >>> +\subparagraph{MMAP}
> >>> +
> >>> +In virtio-media, \textit{MMAP} buffers are provisioned by the device, just like
> >>> +they are by the kernel in regular V4L2. Similarly to how userspace can map a
> >>> +\textit{MMAP} buffer into its address space using mmap and munmap, the
> >>> +virtio-media driver can map device buffers into the driver space by queueing the
> >>> +\textit{struct virtio_media_cmd_mmap} and \textit{struct virtio_media_cmd_munmap}
> >>> +commands to the commandq.
> >>> +
> >>> +\subparagraph{USERPTR}
> >>> +
> >>> +In virtio-media, \textit{USERPTR} buffers are provisioned by the driver, just
> >>> +like they are by userspace in regular V4L2. Instances of \textit{struct v4l2_buffer}
> >>> +and \textit{struct v4l2_plane} of this type are followed by a list of
> >>> +\textit{struct virtio_media_sg_entry}. For more information, see
> >>> +\ref{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
> >>> +
> >>> +The device must not alter the pointer values provided by the driver, i.e.
> >>> +\field{the m.userptr} member of \textit{struct v4l2_buffer} and
> >>> +\textit{struct v4l2_plane} must be returned to the driver with the same value
> >>> +as it was provided.
> >>
> >> I very, very strongly recommend that you drop USERPTR support for virtio.
> >> MMAP and DMABUF are sufficient.
> >>
> >> It's a pain to support, and we discourage it for new drivers.
> >>
> >> If memory serves, there is at least one chromeos driver that is stuck to
> >> USERPTR support for some reason (I can't remember the details), and that is
> >> (I think) the only reason USERPTR support is part of virtio. But that is
> >> really a chromeos issue that they need to fix, IMHO.
> >
> > I am wondering if the concerns about USERPTR should apply to the
> > host/guest context.
> >
> > In virtio-media, the USERPTR memory type just means that the buffer
> > memory has been allocated by the guest and the addresses passed are
> > guest physical addresses. It doesn't necessarily mean that the memory
> > has been allocated with the USERPTR type by the guest user-space.
>
> Ah, OK. But then USERPTR is a really confusing name for people with a
> V4L2 background.
>
> Regardless, this needs to be explained better.

We can certainly do that and use a different name, as Albert suggested
we do for DMABUF. I agree it can be confusing,

>
> > For instance, say the guest user-space allocated memory using memfd,
> > and is passing the FDs to the guest driver as DMABUF buffers. From the
> > guest kernel point of view, these buffers will still resolve into
> > guest physical memory, so the USERPTR memory type will be used by
> > guest/host communication to indicate that fact. I don't know of any
> > formal way to resolve guest DMABUFs into proper virtio resources
> > unfortunately.
> >
> > Without the ability to communicate buffers as guest physical memory,
> > the only kinds of DMABUFs supported by the guest driver would be those
> > coming from another virtio driver, like virtio-gpu. Granted, in
> > practice that's probably where these will come from anyway, but I
> > think it would be nice to be able to support memfd, at least for
> > testing purposes.
> >
> > But to answer what I think your concern is, if you prefer that the
> > guest driver does not advertise support for USERPTR memory type to the
> > guest userspace, we can certainly enforce that (either absolutely or
> > through an option).
>
> I think I would like that.

Acknowledged - memfd + DMABUF can cover the same use-cases anyway.
Just for my erudition, is the reason for eschewing USERPTR documented
somewhere?

Thanks,
Alex.

>
> Regards,
>
>         Hans
>
> >
> > Cheers,
> > Alex.
> >
> >>
> >> Regards,
> >>
> >>         Hans
> >>
> >>> +
> >>> +\subparagraph{DMABUF}
> >>> +
> >>> +In virtio-media, \textit{DMABUF} buffers are provisioned by a virtio object,
> >>> +just like they are by a \textit{DMABUF} in regular V4L2. Virtio objects are
> >>> +16-bytes UUIDs and do not fit in the placeholders for file descriptors, so
> >>> +they follow their embedding data structure as needed and the device must
> >>> +leave the V4L2 structure placeholder unchanged.
> >>> +
> >>> +Contrary to \textit{USERPTR} buffers, virtio objects UUIDs need to be added in
> >>> +both the device-readable and device-writable section of the descriptor chain.
> >>> +
> >>> +Device-allocated buffers with the \textit{V4L2_MEMORY_MMAP} memory type can also
> >>> +be exported as virtio objects for use with another virtio device using the
> >>> +\textit{VIDIOC_EXPBUF} ioctl. The fd placefolder of \textit{v4l2_exportbuffer}
> >>> +means that space for the UUID needs to be reserved right after that structure
> >>> +
> >>> +\subsubsection{Event Virtqueue}
> >>> +
> >>> +Events are a way for the device to inform the driver about asynchronous events
> >>> +that it should know about. In virtio-media, they are used as a replacement for
> >>> +the \textit{VIDIOC_DQBUF} and \textit{VIDIOC_DQEVENT} ioctls and the polling
> >>> +mechanism, which would be impractical to implement on top of virtio.
> >>> +
> >>> +\paragraph{Device Operation: Event header}
> >>> +
> >>> +\begin{lstlisting}
> >>> +#define VIRTIO_MEDIA_EVT_ERROR 0
> >>> +#define VIRTIO_MEDIA_EVT_DQBUF 1
> >>> +#define VIRTIO_MEDIA_EVT_EVENT 2
> >>> +
> >>> +/* Header for events queued by the device for the driver on the eventq. */
> >>> +struct virtio_media_event_header {
> >>> +    u32 event;
> >>> +    u32 session_id;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{event}] one of \field{VIRTIO_MEDIA_EVT_*}.
> >>> +\item[\field{session_id}] ID of the session the event applies to.
> >>> +\end{description}
> >>> +
> >>> +\paragraph{Device Operation: Device-side error}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_EVT_ERROR} Upon receiving this event, the session
> >>> +mentioned in the header is considered corrupted and automatically closed by
> >>> +the device.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_event_error {
> >>> +    struct virtio_media_event_header hdr;
> >>> +    u32 errno;
> >>> +    u32 __padding;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{errno}] error code describing the kind of error that occurred.
> >>> +\end{description}
> >>> +
> >>> +\paragraph{Device Operation: Dequeue buffer}
> >>> +\label{sec:Device Types / Media Device / Device Operation / Dequeue buffer}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_EVT_DQBUF} Signals that a buffer is not being used anymore
> >>> +by the device and is returned to the driver.
> >>> +
> >>> +A \textit{struct virtio_media_event_dqbuf} event is queued on the eventq by the
> >>> +device every time a buffer previously queued using the \textit{VIDIOC_QBUF}
> >>> +ioctl is done being processed and can be used by the driver again. This is like
> >>> +an implicit \textit{VIDIOC_DQBUF} ioctl.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_event_dqbuf {
> >>> +    struct virtio_media_event_header hdr;
> >>> +    struct v4l2_buffer buffer;
> >>> +    struct v4l2_plane planes[VIDEO_MAX_PLANES];
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{buffer}] \textit{struct v4l2_buffer} describing the buffer that has been dequeued.
> >>> +\item[\field{planes}] array of \textit{struct v4l2_plane} containing the plane information for multi-planar buffers.
> >>> +\end{description}
> >>> +
> >>> +Pointer values in the \textit{struct v4l2_buffer} and \textit{struct v4l2_plane}
> >>> +are meaningless and must be ignored by the driver. It is recommended that the
> >>> +device sets them to NULL in order to avoid leaking potential device addresses.
> >>> +
> >>> +Note that in the case of a \field{USERPTR} buffer, the \textit{struct v4l2_buffer}
> >>> +used as event payload is not followed by the buffer memory: since that memory
> >>> +is the same that the driver submitted with the \textit{VIDIOC_QBUF}, it would
> >>> +be redundant to have it here.
> >>> +
> >>> +\paragraph{Device Operation: Emit an event}
> >>> +\label{sec:Device Types / Media Device / Device Operation / Emit an event}
> >>> +
> >>> +\textbf{VIRTIO_MEDIA_EVT_EVENT} Signals that a V4L2 event has been emitted for a session.
> >>> +
> >>> +A \textit{struct virtio_media_event_event} event is queued on the eventq by the
> >>> +device every time an event the driver previously subscribed to using the
> >>> +\textit{VIDIOC_SUBSCRIBE_EVENT} ioctl has been signaled. This is like an
> >>> +implicit \textit{VIDIOC_DQEVENT} ioctl.
> >>> +
> >>> +\begin{lstlisting}
> >>> +struct virtio_media_event_event {
> >>> +    struct virtio_media_event_header hdr;
> >>> +    struct v4l2_event event;
> >>> +};
> >>> +\end{lstlisting}
> >>> +
> >>> +\begin{description}
> >>> +\item[\field{event}] \textit{struct v4l2_event} describing the event that occurred.
> >>> +\end{description}
> >>> diff --git a/device-types/media/device-conformance.tex b/device-types/media/device-conformance.tex
> >>> new file mode 100644
> >>> index 0000000..3338822
> >>> --- /dev/null
> >>> +++ b/device-types/media/device-conformance.tex
> >>> @@ -0,0 +1,11 @@
> >>> +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Device Conformance / Media Device Conformance}
> >>> +
> >>> +A Media device MUST conform to the following normative statements:
> >>> +
> >>> +\begin{itemize}
> >>> +\item \ref{devicenormative:Device Types / Media Device / Virtqueues}
> >>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Open device}
> >>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / V4L2 ioctls}
> >>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unsupported ioctls}
> >>> +\item \ref{devicenormative:Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
> >>> +\end{itemize}
> >>> \ No newline at end of file
> >>> diff --git a/device-types/media/driver-conformance.tex b/device-types/media/driver-conformance.tex
> >>> new file mode 100644
> >>> index 0000000..058b812
> >>> --- /dev/null
> >>> +++ b/device-types/media/driver-conformance.tex
> >>> @@ -0,0 +1,9 @@
> >>> +\conformance{\subsection}{Media Device Conformance}\label{sec:Conformance / Driver Conformance / Media Driver Conformance}
> >>> +
> >>> +A Media device MUST conform to the following normative statements:
> >>> +
> >>> +\begin{itemize}
> >>> +\item \ref{drivernormative:Device Types / Media Device / Virtqueues}
> >>> +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Command Virtqueue}
> >>> +\item \ref{drivernormative:Device Types / Media Device / Device Operation / Close device}
> >>> +\end{itemize}
> >>> \ No newline at end of file
> >>
>

Re: [PATCH v2 1/1] virtio-media: Add virtio media device specification

[Hans Verkuil]

On 20/06/2024 14:53, Alexandre Courbot wrote:
> On Thu, Jun 20, 2024 at 9:45 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 08/06/2024 03:24, Alexandre Courbot wrote:
>>> Hi Hans,
>>>
>>> On Fri, Jun 7, 2024 at 5:43 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>>>
>>>> Hi Albert,
>>>>
>>>>
>>>> On 07/06/2024 10:00, Albert Esteve wrote:
>>>>> Virtio-media is an encapsulation of the V4L2 UAPI into
>>>>> virtio, able to virtualize any video device supported
>>>>> by V4L2
>>>>>
>>>>> Note that virtio-media does not require the use of a
>>>>> V4L2 device driver or of Linux on the host or
>>>>> guest side - V4L2 is only used as a host-guest protocol,
>>>>> and both sides are free to convert it from/to any
>>>>> model that they wish to use.
>>>>
>>>> I just have two notes, one minor, one more substantial:
>>>>
>>>>>
>>>>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
>>>>> ---
>>>>>  conformance.tex                           |  13 +-
>>>>>  content.tex                               |   1 +
>>>>>  device-types/media/description.tex        | 578 ++++++++++++++++++++++
>>>>>  device-types/media/device-conformance.tex |  11 +
>>>>>  device-types/media/driver-conformance.tex |   9 +
>>>>>  5 files changed, 608 insertions(+), 4 deletions(-)
>>>>>  create mode 100644 device-types/media/description.tex
>>>>>  create mode 100644 device-types/media/device-conformance.tex
>>>>>  create mode 100644 device-types/media/driver-conformance.tex
>>>>>
>>>>> diff --git a/conformance.tex b/conformance.tex
>>>>> index dc00e84..c369da1 100644
>>>>> --- a/conformance.tex
>>>>> +++ b/conformance.tex
>>>>> @@ -32,8 +32,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>>  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
>>>>>  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
>>>>>  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
>>>>> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
>>>>> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
>>>>> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
>>>>> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
>>>>> +\ref{sec:Conformance / Driver Conformance / Media Driver Conformance}.
>>>>> +
>>>>>
>>>>>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>>>>    \end{itemize}
>>>>> @@ -59,8 +61,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>>  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
>>>>>  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
>>>>>  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
>>>>> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
>>>>> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
>>>>> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
>>>>> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
>>>>> +\ref{sec:Conformance / Device Conformance / Media Device Conformance}.
>>>>>
>>>>>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>>>>    \end{itemize}
>>>>> @@ -152,6 +155,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>>  \input{device-types/scmi/driver-conformance.tex}
>>>>>  \input{device-types/gpio/driver-conformance.tex}
>>>>>  \input{device-types/pmem/driver-conformance.tex}
>>>>> +\input{device-types/media/driver-conformance.tex}
>>>>>
>>>>>  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>>>>>
>>>>> @@ -238,6 +242,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>>  \input{device-types/scmi/device-conformance.tex}
>>>>>  \input{device-types/gpio/device-conformance.tex}
>>>>>  \input{device-types/pmem/device-conformance.tex}
>>>>> +\input{device-types/media/device-conformance.tex}
>>>>>
>>>>>  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
>>>>>  A conformant implementation MUST be either transitional or
>>>>> diff --git a/content.tex b/content.tex
>>>>> index 0a62dce..59925ae 100644
>>>>> --- a/content.tex
>>>>> +++ b/content.tex
>>>>> @@ -767,6 +767,7 @@ \chapter{Device Types}\label{sec:Device Types}
>>>>>  \input{device-types/scmi/description.tex}
>>>>>  \input{device-types/gpio/description.tex}
>>>>>  \input{device-types/pmem/description.tex}
>>>>> +\input{device-types/media/description.tex}
>>>>>
>>>>>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>>>>>
>>>>> diff --git a/device-types/media/description.tex b/device-types/media/description.tex
>>>>> new file mode 100644
>>>>> index 0000000..887eacf
>>>>> --- /dev/null
>>>>> +++ b/device-types/media/description.tex
>>>>> @@ -0,0 +1,578 @@
>>>>> +\section{Media Device}\label{sec:Device Types / Media Device}
>>>>> +
>>>>> +The virtio media device follow the same model (and structures) as V4L2. It
>>>>> +can be used to virtualize cameras, codec devices, or any other device
>>>>> +supported by V4L2. The device assumes 64-bit little-endian V4L2 structures
>>>>> +are exchanged. The complete definition of V4L2 structures and ioctls can
>>>>> +be found under the
>>>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
>>>>> +
>>>>> +V4L2 is a UAPI that allows a less privileged entity (user-space) to use video
>>>>> +hardware exposed by a more privileged entity (the kernel). Virtio-media is an
>>>>> +encapsulation of this API into virtio, turning it into a virtualization API
>>>>> +for all classes of video devices supported by V4L2, where the device plays the
>>>>> +role of the kernel and the driver the role of user-space.
>>>>> +
>>>>> +The device is therefore responsible for presenting a virtual device that behaves
>>>>> +like an actual V4L2 device, which the driver can control.
>>>>> +
>>>>> +Note that virtio-media does not require the use of a V4L2 device driver or of
>>>>> +Linux on any side - V4L2 is only used as a transport protocol,
>>>>> +and both sides are free to convert it from/to any model that they wish to use.
>>>>> +
>>>>> +This section relies on definitions from
>>>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/index.html}{V4L2 UAPI documentation}.
>>>>> +
>>>>> +\subsection{Device ID}\label{sec:Device Types / Media Device / Device ID}
>>>>> +
>>>>> +42
>>>>> +
>>>>> +\subsection{Virtqueues}\label{sec:Device Types / Media Device / Virtqueues}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[0] commandq - used for driver commands and device responses to these
>>>>> +commands.
>>>>> +\item[1] eventq - used for events sent by the device to the driver.
>>>>> +\end{description}
>>>>> +
>>>>> +\devicenormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
>>>>> +
>>>>> +The device MUST return the descriptor chains it receives on the commandq as
>>>>> +soon as possible, and must never hold them for indefinite periods of time.
>>>>> +
>>>>> +\drivernormative{\subsubsection}{Virtqueues}{Device Types / Media Device / Virtqueues}
>>>>> +
>>>>> +The driver MUST re-queue the descriptor chains returned by the device on the
>>>>> +eventq as soon as possible, and must never hold them for indefinite periods
>>>>> +of time.
>>>>> +
>>>>> +\subsection{Feature Bits}\label{sec:Device Types / Media Device / Feature Bits}
>>>>> +
>>>>> +None
>>>>> +
>>>>> +\subsection{Device Configuration Layout}\label{sec:Device Types / Media Device / Device Configuration Layout}
>>>>> +
>>>>> +The video device configuration space uses the following layout:
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_config {
>>>>> +    le32 device_caps;
>>>>> +    le32 device_type;
>>>>> +    u8 card[32];
>>>>
>>>> Use char instead of u8. It's always been a pain that struct v4l2_capability
>>>> used u8 instead of char for the character arrays. I never understood why, and
>>>> if you are making a new struct I would recommend using char.
>>>>
>>>> Up to you, though.
>>>>
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{device_caps}] (driver-read-only) flags representing the device
>>>>> +capabilities as used in
>>>>> +\href{https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-querycap.html#c.v4l2_capability}{struct v4l2_capabilities}.
>>>>> +Corresponds with the \field{device_caps} field in the \textit{struct video_device}.
>>>>> +\item[\field{device_type}] (driver-read-only) informs the driver of the type
>>>>> +of the video device. Corresponds with the \field{vfl_devnode_type} field of the device.
>>>>> +\item[\field{card}] (driver-read-only) name of the device, a NUL-terminated
>>>>> +UTF-8 string. Corresponds with the \field{card} field of the \textit{struct v4l2_capability}.
>>>>> +If all the characters of the field are used, it does not need to be NUL-terminated.
>>>>> +\end{description}
>>>>> +
>>>>> +\subsection{Device Initialization}
>>>>> +
>>>>> +\begin{enumerate}
>>>>> +\item The driver reads the \field{device_caps} and \field{device_type} fields
>>>>> +from the configuration layout to identify the device.
>>>>> +\item The driver sets up the \field{commandq} and \field{eventq}.
>>>>> +\item The driver may open a session to use the device and send V4L2 ioctls in
>>>>> +order to receive more information about the device, such as supported
>>>>> +formats or controls.
>>>>> +\end{enumerate}
>>>>> +
>>>>> +\subsection{Device Operation}\label{sec:Device Types / Media Device / Device Operation}
>>>>> +
>>>>> +Commands are queued on the command queue by the driver for the device to
>>>>> +process. The errors returned by each command are standard
>>>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/gen-errors.html}{Linux kernel error codes}.
>>>>> +For instance, a command that contains invalid options will return \textit{EINVAL}.
>>>>> +
>>>>> +Events are sent on the event queue by the device for the driver to handle.
>>>>> +
>>>>> +\subsubsection{Command Virtqueue}
>>>>> +
>>>>> +\paragraph{Device Operation: Command headers}
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +#define VIRTIO_MEDIA_CMD_OPEN 1
>>>>> +#define VIRTIO_MEDIA_CMD_CLOSE 2
>>>>> +#define VIRTIO_MEDIA_CMD_IOCTL 3
>>>>> +#define VIRTIO_MEDIA_CMD_MMAP 4
>>>>> +#define VIRTIO_MEDIA_CMD_MUNMAP 5
>>>>> +
>>>>> +/* Header for all virtio commands from the driver to the device on the commandq. */
>>>>> +struct virtio_media_cmd_header {
>>>>> +     u32 cmd;
>>>>> +     u32 __padding;
>>>>> +};
>>>>> +
>>>>> +/* Header for all virtio responses from the device to the driver on the commandq. */
>>>>> +struct virtio_media_resp_header {
>>>>> +     u32 status;
>>>>> +     u32 __padding;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +A command consists of a command header \textit{virtio_media_cmd_header}
>>>>> +containing the following device-readable field:
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{cmd}] specifies a device request type (VIRTIO_MEDIA_CMD_*).
>>>>> +\end{description}
>>>>> +
>>>>> +A response consists of a response header \textit{virtio_media_resp_header}
>>>>> +containing the following device-writable field:
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{status}] indicates a device request status.
>>>>> +\end{description}
>>>>> +
>>>>> +The status field can take 0 if the command was successful, or one of the
>>>>> +standard Linux error codes if it was not.
>>>>> +
>>>>> +\drivernormative{\paragraph}{Device Operation: Command Virtqueue: Sessions}{Device Types / Media Device / Device Operation / Command Virtqueue}
>>>>> +
>>>>> +Sessions are how the device is multiplexed, allowing several distinct works to
>>>>> +take place simultaneously. The driver needs to open a session before it can
>>>>> +perform any useful operation on the device.
>>>>> +
>>>>> +\paragraph{Device Operation: Open device}
>>>>> +
>>>>> +\textbf{VIRTIO_MEDIA_CMD_OPEN} Command for creating a new session.
>>>>> +
>>>>> +This is the equivalent of calling \textit{open} on a V4L2 device node.
>>>>> +The driver uses \textit{virtio_media_cmd_open} to send an open request.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_cmd_open {
>>>>> +    struct virtio_media_cmd_header hdr;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_OPEN} with \textit{virtio_media_resp_open}.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_resp_open {
>>>>> +    struct virtio_media_resp_header hdr;
>>>>> +    u32 session_id;
>>>>> +    u32 __padding;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{session_id}] specifies an identifier for the current session. The
>>>>> +identifier can be used to perform other commands on the session, notably ioctls.
>>>>> +\end{description}
>>>>> +
>>>>> +\devicenormative{\subparagraph}{Device Operation: Open device}{Device Types / Media Device / Device Operation / Open device}
>>>>> +
>>>>> +Upon success, the device MUST set a \field{session_id} in \textit{virtio_media_resp_open}
>>>>> +to an integer that is NOT used by any other open session.
>>>>> +
>>>>> +\paragraph{Device Operation: Close device}
>>>>> +
>>>>> +\textbf{VIRTIO_MEDIA_CMD_CLOSE} Command for closing an active session.
>>>>> +
>>>>> +This is the equivalent of calling \textit{close} on a previously opened V4L2
>>>>> +device node. All resources associated with this session will be freed.
>>>>> +
>>>>> +This command does not require a response from the device.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_cmd_close {
>>>>> +    struct virtio_media_cmd_header hdr;
>>>>> +    u32 session_id;
>>>>> +    u32 __padding;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{session_id}] specifies an identifier for the session to close.
>>>>> +\end{description}
>>>>> +
>>>>> +\drivernormative{\subparagraph}{Device Operation: Close device}{Device Types / Media Device / Device Operation / Close device}
>>>>> +
>>>>> +The session ID SHALL NOT be used again after queueing this command.
>>>>> +
>>>>> +\paragraph{Device Operation: V4L2 ioctls}
>>>>> +
>>>>> +\textbf{VIRTIO_MEDIA_CMD_IOCTL} Command for executing an ioctl on an open
>>>>> +session.
>>>>> +
>>>>> +This command asks the device to run one of the `VIDIOC_*` ioctls on the active
>>>>> +session.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_cmd_ioctl {
>>>>> +    struct virtio_media_cmd_header hdr;
>>>>> +    u32 session_id;
>>>>> +    u32 code;
>>>>> +    /* Followed by the relevant ioctl payload as defined in the macro */
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{session_id}] specifies an identifier of thesession to run the ioctl on.
>>>>> +\item[\field{code}] specifies the code of the \field{VIDIOC_*} ioctl to run.
>>>>> +\end{description}
>>>>> +
>>>>> +The code is extracted from the
>>>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/videodev.html}{videodev2.h},
>>>>> +header file. The file defines the ioctl's codes, type of payload, and
>>>>> +direction. The code consists of the second argument of the \field{_IO*} macro.
>>>>> +
>>>>> +For example, the \textit{VIDIOC_G_FMT} is defined as follows:
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +#define VIDIOC_G_FMT _IOWR('V',  4, struct v4l2_format)
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +This means that its ioctl code is \textit{4}, that its payload is a
>>>>> +\textit{struct v4l2_format}, and that its direction is \textit{WR} (i.e., the
>>>>> +payload is written by both the driver and the device).
>>>>> +See Section \ref{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
>>>>> +for more information about the direction of ioctls.
>>>>> +
>>>>> +The payload layout is always a 64-bit representation of the corresponding
>>>>> +V4L2 structure.
>>>>> +
>>>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_IOCTL} with \textit{virtio_media_resp_ioctl}.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_resp_ioctl {
>>>>> +    struct virtio_media_resp_header hdr;
>>>>> +    /* Followed by the ioctl payload as defined in the macro */
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\subparagraph{Ioctls payload}\label{sec:Device Types / Media Device / V4L2 ioctls / Ioctls payload}
>>>>> +
>>>>> +Each ioctl has a payload, which is defined by the third argument of the
>>>>> +\field{_IO*} macro defining it.
>>>>> +
>>>>> +The payload of an ioctl in the descriptor chain follows the command structure,
>>>>> +the reponse structure, or both depending on the direction:
>>>>> +
>>>>> +\begin{itemize}
>>>>> +\item \textbf{_IOR} is read-only for the driver, meaning the payload
>>>>> +follows the response in the device-writable section of the descriptor chain.
>>>>> +\item \textbf{_IOW} is read-only for the device, meaning the payload
>>>>> +follows the command in the driver-writable section of the descriptor chain.
>>>>> +\item \textbf{_IOWR} is writable by both the device and driver,
>>>>> +meaning the payload must follow both the command in the driver-writable section
>>>>> +of the descriptor chain, and the response in the device-writable section.
>>>>> +\end{itemize}
>>>>> +
>>>>> +A common optimization for \textit{WR} ioctls is to provide the payload using
>>>>> +descriptors that both point to the same buffer. This mimics the behavior of
>>>>> +V4L2 ioctls where the data is only passed once and used as both input and
>>>>> +output by the kernel.
>>>>> +
>>>>> +\devicenormative{\subparagraph}{Device Operation: V4L2 ioctls}{Device Types / Media Device / Device Operation / V4L2 ioctls}
>>>>> +
>>>>> +In case of success of a device-writable ioctl, the device MUST always write the
>>>>> +payload in the device-writable part of the descriptor chain.
>>>>> +
>>>>> +In case of failure of a device-writable ioctl, the device is free to write the
>>>>> +payload in the device-writable part of the descriptor chain or not. Some errors
>>>>> +may still result in the payload being updated, and in this case the device is
>>>>> +expected to write the updated payload. If the device has not written the
>>>>> +payload after an error, the driver MUST assume that the payload has not been
>>>>> +modified.
>>>>> +
>>>>> +\subparagraph{Handling of pointers in ioctl payload}
>>>>> +
>>>>> +A few structures used as ioctl payloads contain pointers to further
>>>>> +data needed for the ioctl. There are notably:
>>>>> +
>>>>> +\begin{itemize}
>>>>> +\item The \field{planes} pointer of
>>>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer}{struct v4l2_buffer},
>>>>> +which size is determined by the length member.
>>>>> +\item The \field{controls} pointer of \textit{struct v4l2_ext_controls}, which
>>>>> +size is determined by the count member.
>>>>> +\end{itemize}
>>>>> +
>>>>> +If the size of the pointed area is determined to be non-zero, then the main
>>>>> +payload is immediately followed by the pointed data in their order of
>>>>> +appearance in the structure, and the pointer value itself is ignored by the
>>>>> +device, which must also return the value initially passed by the driver.
>>>>> +
>>>>> +\subparagraph{Handling of pointers to userspace memory}
>>>>> +\label{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
>>>>> +
>>>>> +A few pointers are special in that they point to userspace memory in the
>>>>> +original V4L2 specification. They are:
>>>>> +
>>>>> +\begin{itemize}
>>>>> +\item The \field{m.userptr} member of \textit{struct v4l2_buffer} and
>>>>> +\href{https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-plane}{struct v4l2_plane}
>>>>> +(technically an unsigned long, but designated a userspace address).
>>>>> +\item The \field{ptr} member of \textit{struct v4l2_ext_ctrl}.
>>>>> +\end{itemize}
>>>>> +
>>>>> +These pointers can cover large areas of scattered memory, which has the
>>>>
>>>> Actually, only m.userptr can cover a large area. The other two are limited.
>>>> See my comment about USERPTR support below.
>>>>
>>>>> +potential to require more descriptors than the virtio queue can provide. For
>>>>> +these particular pointers only, a list of \textit{struct virtio_media_sg_entry}
>>>>> +that covers the needed amount of memory for the pointer is used instead of
>>>>> +using descriptors to map the pointed memory directly.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_sg_entry {
>>>>> +     u64 start;
>>>>> +     u32 len;
>>>>> +     u32 __padding;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +For each such pointer to read, the device reads as many SG entries as needed
>>>>> +to cover the length of the pointed buffer, as described by its parent
>>>>> +structure (\field{length} member of \textit{struct v4l2_buffer} or
>>>>> +\textit{struct v4l2_plane} for buffer memory, and \field{size} member of
>>>>> +\textit{struct v4l2_ext_control} for control data).
>>>>> +
>>>>> +Since the device never needs to modify the list of SG entries, it is only
>>>>> +provided by the driver in the device-readable section of the descriptor chain,
>>>>> +and not repeated in the device-writable section, even for WR ioctls.
>>>>> +
>>>>> +\subparagraph{Unsupported ioctls}
>>>>> +
>>>>> +A few ioctls are replaced by other, more suitable mechanisms.
>>>>> +
>>>>> +\begin{itemize}
>>>>> +\item \textit{VIDIOC_QUERYCAP} is replaced by reading the configuration area
>>>>> +(see \ref{sec:Device Types / Media Device / Device Configuration Layout}).
>>>>> +\item \textit{VIDIOC_DQBUF} is replaced by a dedicated event
>>>>> +(see \ref{sec:Device Types / Media Device / Device Operation / Dequeue buffer}).
>>>>> +\item \textit{VIDIOC_DQEVENT} is replaced by a dedicated event
>>>>> +(see \ref{sec:Device Types / Media Device / Device Operation / Emit an event}).
>>>>> +\item \textit{VIDIOC_G_JPEGCOMP} and \textit{VIDIOC_S_JPEGCOMP} are deprecated
>>>>> +and replaced by the controls of the JPEG class.
>>>>> +\item \textit{VIDIOC_LOG_STATUS} is a driver-only operation and shall not be
>>>>> +implemented by the device.
>>>>> +\end{itemize}
>>>>> +
>>>>> +\devicenormative{\subparagraph}{Device Operation: Unsupported ioctls}{Device Types / Media Device / Device Operation / Unsupported ioctls}
>>>>> +
>>>>> +If being requested an unsupported ioctl, the device MUST return the same
>>>>> +error response as it would for an unknown ioctl, i.e. \textit{ENOTTY}.
>>>>> +
>>>>> +\paragraph{Device Operation: Mapping a MMAP buffer}
>>>>> +
>>>>> +\textbf{VIRTIO_MEDIA_CMD_MMAP} Command for mapping a MMAP buffer into the
>>>>> +driver's address space.
>>>>> +
>>>>> +Shared memory region ID 0 is used to map MMAP buffers with
>>>>> +the \textit{VIRTIO_MEDIA_CMD_MMAP} command.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +#define VIRTIO_MEDIA_MMAP_FLAG_RW (1 << 0)
>>>>> +
>>>>> +struct virtio_media_cmd_mmap {
>>>>> +     struct virtio_media_cmd_header hdr;
>>>>> +     u32 session_id;
>>>>> +     u32 flags;
>>>>> +     u64 offset;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{flags}] is the set of flags for the mapping. \field{VIRTIO_MEDIA_MMAP_FLAG_RW}
>>>>> +can be set if a read-write mapping is desired. Without this flag the mapping
>>>>> +will be read-only.
>>>>> +\item[\field{offset}] corresponds to the \field{mem_offset} field of the
>>>>> +\textit{union v4l2_plane} for the plane to map. This field can be obtained
>>>>> +using the \textit{VIDIOC_QUERYBUF} ioctl.
>>>>> +\end{description}
>>>>> +
>>>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_MMAP} with \textit{virtio_media_resp_mmap}.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_resp_mmap {
>>>>> +    struct virtio_media_resp_header hdr;
>>>>> +    u64 offset;
>>>>> +    u64 len;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{offset}] offset into SHM region ID 0 of the start of the mapping.
>>>>> +\item[\field{len}] length of the mapping as indicated by the \textit{struct v4l2_plane}
>>>>> +the buffer belongs to.
>>>>> +\end{description}
>>>>> +
>>>>> +\paragraph{Device Operation: Unmapping a MMAP buffer}
>>>>> +
>>>>> +\textbf{VIRTIO_MEDIA_CMD_MUNMAP} Unmap a MMAP buffer previously mapped using \field{VIRTIO_MEDIA_CMD_MMAP}.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_cmd_munmap {
>>>>> +    struct virtio_media_cmd_header hdr;
>>>>> +    u64 offset;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\begin{description}
>>>>> +\item[\field{offset}] offset into SHM region ID 0 previously returned by
>>>>> +\textit{VIRTIO_MEDIA_CMD_MMAP} at which the buffer has been previously mapped.
>>>>> +\end{description}
>>>>> +
>>>>> +The device responds to \textit{VIRTIO_MEDIA_CMD_MUNMAP} with \textit{virtio_media_resp_munmap}.
>>>>> +
>>>>> +\begin{lstlisting}
>>>>> +struct virtio_media_resp_munmap {
>>>>> +    struct virtio_media_resp_header hdr;
>>>>> +};
>>>>> +\end{lstlisting}
>>>>> +
>>>>> +\devicenormative{\subparagraph}{Device Operation: Unmapping a MMAP buffer}{Device Types / Media Device / Device Operation / Unmapping a MMAP buffer}
>>>>> +
>>>>> +The device MUST keep mappings performed using \textit{VIRTIO_MEDIA_CMD_MMAP}
>>>>> +valid until \textit{VIRTIO_MEDIA_CMD_MUNMAP} is called, even if the buffers or
>>>>> +session they belong to are released or closed by the driver.
>>>>> +
>>>>> +\paragraph{Device Operation: Memory Types}
>>>>> +
>>>>> +The semantics of the three V4L2 memory types (\textit{MMAP}, \textit{USERPTR}
>>>>> +and \textit{DMABUF}) can easily be mapped to both driver and device context.
>>>>> +
>>>>> +\subparagraph{MMAP}
>>>>> +
>>>>> +In virtio-media, \textit{MMAP} buffers are provisioned by the device, just like
>>>>> +they are by the kernel in regular V4L2. Similarly to how userspace can map a
>>>>> +\textit{MMAP} buffer into its address space using mmap and munmap, the
>>>>> +virtio-media driver can map device buffers into the driver space by queueing the
>>>>> +\textit{struct virtio_media_cmd_mmap} and \textit{struct virtio_media_cmd_munmap}
>>>>> +commands to the commandq.
>>>>> +
>>>>> +\subparagraph{USERPTR}
>>>>> +
>>>>> +In virtio-media, \textit{USERPTR} buffers are provisioned by the driver, just
>>>>> +like they are by userspace in regular V4L2. Instances of \textit{struct v4l2_buffer}
>>>>> +and \textit{struct v4l2_plane} of this type are followed by a list of
>>>>> +\textit{struct virtio_media_sg_entry}. For more information, see
>>>>> +\ref{sec:Device Types / Media Device / V4L2 ioctls / Userspace memory}
>>>>> +
>>>>> +The device must not alter the pointer values provided by the driver, i.e.
>>>>> +\field{the m.userptr} member of \textit{struct v4l2_buffer} and
>>>>> +\textit{struct v4l2_plane} must be returned to the driver with the same value
>>>>> +as it was provided.
>>>>
>>>> I very, very strongly recommend that you drop USERPTR support for virtio.
>>>> MMAP and DMABUF are sufficient.
>>>>
>>>> It's a pain to support, and we discourage it for new drivers.
>>>>
>>>> If memory serves, there is at least one chromeos driver that is stuck to
>>>> USERPTR support for some reason (I can't remember the details), and that is
>>>> (I think) the only reason USERPTR support is part of virtio. But that is
>>>> really a chromeos issue that they need to fix, IMHO.
>>>
>>> I am wondering if the concerns about USERPTR should apply to the
>>> host/guest context.
>>>
>>> In virtio-media, the USERPTR memory type just means that the buffer
>>> memory has been allocated by the guest and the addresses passed are
>>> guest physical addresses. It doesn't necessarily mean that the memory
>>> has been allocated with the USERPTR type by the guest user-space.
>>
>> Ah, OK. But then USERPTR is a really confusing name for people with a
>> V4L2 background.
>>
>> Regardless, this needs to be explained better.
> 
> We can certainly do that and use a different name, as Albert suggested
> we do for DMABUF. I agree it can be confusing,
> 
>>
>>> For instance, say the guest user-space allocated memory using memfd,
>>> and is passing the FDs to the guest driver as DMABUF buffers. From the
>>> guest kernel point of view, these buffers will still resolve into
>>> guest physical memory, so the USERPTR memory type will be used by
>>> guest/host communication to indicate that fact. I don't know of any
>>> formal way to resolve guest DMABUFs into proper virtio resources
>>> unfortunately.
>>>
>>> Without the ability to communicate buffers as guest physical memory,
>>> the only kinds of DMABUFs supported by the guest driver would be those
>>> coming from another virtio driver, like virtio-gpu. Granted, in
>>> practice that's probably where these will come from anyway, but I
>>> think it would be nice to be able to support memfd, at least for
>>> testing purposes.
>>>
>>> But to answer what I think your concern is, if you prefer that the
>>> guest driver does not advertise support for USERPTR memory type to the
>>> guest userspace, we can certainly enforce that (either absolutely or
>>> through an option).
>>
>> I think I would like that.
> 
> Acknowledged - memfd + DMABUF can cover the same use-cases anyway.
> Just for my erudition, is the reason for eschewing USERPTR documented
> somewhere?

No. Basically DMABUF is a much better and cleaner approach. USERPTR is old
(it predates DMABUF) and is fairly complex to support. It is basically there
for legacy drivers, and new drivers should just enable MMAP and DMABUF only.

Regards,

	Hans