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

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

[Albert Esteve]

New attempt of including virtio-media
device specification.

v2->v3:
- Updated device ID to 49
- Renamed virtio memory types so that
  they are differentiated from V4L2
  memory types
- Memory types description slightly
  rewritten, explicitely excluding
  USERPTR support for guest userspace

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 at:

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).
It also includes a FFmpeg-based video encoder
device. Steps to reproduce are also detailed[3].

Recently, virtio-media got a proposal to reserve
device ID 49, which was finally approved for
inclusion in v1.4.

There is some overlap with virtio-video in regards
to which devices it can handle. However,
they take different approaches, making them
the preferable choice for different scenarios.
Moreover, 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 is justified the desire to include
the specification in the next release despite
the aforementioned overlap.

Full PDF: https://drive.google.com/file/d/1zhmteCqkZsn-oQz5Gle2uG8HvDy1vGSw/view?usp=sharing
PDF with the media section only: https://drive.google.com/file/d/19A8RH-3kiOkod0xkHkuxCtoq0-PtuaQ4/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        | 583 ++++++++++++++++++++++
 device-types/media/device-conformance.tex |  11 +
 device-types/media/driver-conformance.tex |   9 +
 5 files changed, 613 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.47.0

[PATCH v3 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        | 583 ++++++++++++++++++++++
 device-types/media/device-conformance.tex |  11 +
 device-types/media/driver-conformance.tex |   9 +
 5 files changed, 613 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..793e9b1
--- /dev/null
+++ b/device-types/media/description.tex
@@ -0,0 +1,583 @@
+\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}
+
+49
+
+\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.
+
+However, the driver shall only advertise support for \textit{MMAP} and
+\textit{DMABUF} to the guest userspace. \textit{USERPTR} is merely employed to
+discern \textit{GUEST_PAGES} buffers, similar to how \textit{DMABUF} is used
+to signify \textit{VIRTIO_OBJECT} buffers in the 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{GUEST_PAGES}
+
+In virtio-media, \textit{GUEST_PAGES} buffers are provisioned by the driver,
+and use guest physical addresses. 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{VIRTIO_OBJECT}
+
+In virtio-media, \textit{VIRTIO_OBJECT} 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.47.0

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

[Daniel Verkamp]

On Thu, Oct 17, 2024 at 12:04 AM Albert Esteve <aesteve@redhat.com> 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.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
[...]
> diff --git a/device-types/media/description.tex b/device-types/media/description.tex
> new file mode 100644
> index 0000000..793e9b1
> --- /dev/null
> +++ b/device-types/media/description.tex
> @@ -0,0 +1,583 @@
> +\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}.

I would omit the "The device assumes" part; this spec is defining the
protocol used by both the device and driver.

It would also be nice to be a bit more explicit about the meaning of
"64-bit"; presumably this means something like "struct layout matches
the Linux psABI for <some 64-bit architecture>", although that's not
great either IMO. Punting to the Linux docs rather than defining the
structs in the virtio spec feels iffy to me (though this is also the
approach taken by fs with fuse); hopefully none of the V4L2 structs
use features like bitfields that could cause layout differences (and
may not match the bit-field definition in the virtio spec), and
hopefully Linux never introduces any new structs that don't make sense
in virtio-media.

Also, though this is more of an implementation-specific detail, I
wonder how this will be handled by the Linux virtio-media driver; will
it have code to convert between the 64-bit/little-endian layout used
by virtio-media and the native V4L2 structs? Or is it assumed that the
driver will never be used on a 32-bit or big-endian platform and
structs can just be naively passed through without conversion?

> +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}.

This is probably redundant, since it just repeats the "definition of
V4L2 structures and ioctls" link earlier in this same section.

[...]
> +\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}.

nit: This link is pointing to v4.9 docs rather than "latest" like the
others. (Maybe using a specific version would be preferable to avoid
link rot, but all the links should match.)

[...]
> +\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.

nit: "thesession" is missing a space.

> +\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 should be more specific about the ioctl encoding, which is not
consistent across architectures (_IOC_SIZEBITS can differ).

> +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.

As discussed above, "64-bit representation" is pretty vague, and this
section should probably also describe the endianness requirement.

> +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:

nit: "reponse" -> "response"

> +\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.

How is the driver supposed to know "[i]f the device has not written
the payload"? It would probably be clearer to say the driver may not
assume anything about the state of the payload when an error is
returned.

[...]

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

[Matias Ezequiel Vara Larsen]

Hello Albert, 

I gave a first reading. I left some minor comments below. I could not
cover the whole patch yet.

On Thu, Oct 17, 2024 at 09:04:23AM +0200, 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.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  conformance.tex                           |  13 +-
>  content.tex                               |   1 +
>  device-types/media/description.tex        | 583 ++++++++++++++++++++++
>  device-types/media/device-conformance.tex |  11 +
>  device-types/media/driver-conformance.tex |   9 +
>  5 files changed, 613 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..793e9b1
> --- /dev/null
> +++ b/device-types/media/description.tex
> @@ -0,0 +1,583 @@
> +\section{Media Device}\label{sec:Device Types / Media Device}
> +
> +The virtio media device follow the same model (and structures) as V4L2. It

s/follow/follows

> +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}
> +
> +49
> +
> +\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.
> +

I think this is true for any device. I wonder what is specific for
virtio-media to put it here. 

> +\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.
> +

This is also true for any device. The mechanism to provide buffers
through the available ring to be consumed and then enqueued in the used
ring is not device specific. Is there any reason that this is different
for virtio-media?

> +\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}.

I think you should not start a sentence with a verb. I think you should
add the person. Something like `It corresponds with the ...`. I see
other cases below.

> +\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}
> +
I think other devices remove `the driver` and write it as sequence by
starting with the verb like:

`- reads the field{}
 - sets up the \field{}
 ...
` 
Also, the last sentence talks about `session` but it was not introduced
yet, what is a session?

> +\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}.

Maybe I am missing something but where and how is stored the returned
error? I mean, it is stored in the device-writable area but what is the
structure?

> +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}
> +

OK, I guess this is the structure that contains the response.

> +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.
> +
I think this paragraph can be rewritten. For example, what do you mean
with `multiplexed`? I think I got the idea but I would elaborate it a
bit. Also, I think you can rewrite the last sentence to something like:
`Before start operating, the driver must open a session`

> +\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.

I would rewrite it as:

`... identifies the current session, which is used for other commands
like ioctls.`

But I do not have a strong opinion about it.

> +\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.
> +
Why do you use `ask` here? Could it be `tells` instead? But I do not
have a strong opinion about it. Also, I think with `active` you mean the
session identified by `session_id`. I think I would not use `active`.

> +\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.

I would rewrite it as:

`\item[\field{session_id}] identifies the session 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).

I think you can remove some of the `thats`:

`This means that its ioctl code is \textit{4}, its payload is a
\textit{struct v4l2_format}, and 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. 
> +

I think you can remove `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}.
> +

I would rewrite it as:

`When a request is not supported, the device MUST return \textit{ENOTTY},
which corresponds with the response for unknown ioctls.`

Matias

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

[Parav Pandit]

> From: Albert Esteve <aesteve@redhat.com>
> Sent: Thursday, October 17, 2024 12:34 PM
> Subject: [PATCH v3 1/1] virtio-media: Add virtio media device specification
> 
> 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.
If it is v4l2, any reason to still call it virtio-media? How about virtio-v4l2 to describe what it is.

[..]
> +/* Header for all virtio commands from the driver to the device on the
> +commandq. */ struct virtio_media_cmd_header {
> +	u32 cmd;
> +	u32 __padding;
> +};
Please fix u32 to le32 here and all other places.

> +
> +/* 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}
> +
I didn't read the spec fully but please explore admin commands as they offer resource (session) create/destroy infrastructure already.
And v4l2 will get simplified a lot by reusing existing virtio admin framework.


> +\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}
> +
This maps to modify or it can be new command.

> +\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/vi
> +deodev.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/bu
> +ffer.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}
> +
Can you please screen it through, and make sure that in this UAPI there is no u16/32/64 data type.
And all are leX or beX, to not assume any endianness between driver and device?


> +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/bu
> +ffer.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.
> +
> +However, the driver shall only advertise support for \textit{MMAP} and
> +\textit{DMABUF} to the guest userspace. \textit{USERPTR} is merely
> +employed to discern \textit{GUEST_PAGES} buffers, similar to how
> +\textit{DMABUF} is used to signify \textit{VIRTIO_OBJECT} buffers in the
> 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{GUEST_PAGES}
> +
Please change the notion from guest/host to device/driver respectively.

> +In virtio-media, \textit{GUEST_PAGES} buffers are provisioned by the
> +driver, and use guest physical addresses. 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{VIRTIO_OBJECT}
> +
> +In virtio-media, \textit{VIRTIO_OBJECT} 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;
> +};
Mostly you wont need padding once you move to admin commands, but wherever applicable, please convert into reserved field in the name.
So, it can be used in future. Converting reserved to meaningful is doable compared to padding constant bytes.

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

[Albert Esteve]

On Fri, Oct 18, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Albert Esteve <aesteve@redhat.com>
> > Sent: Thursday, October 17, 2024 12:34 PM
> > Subject: [PATCH v3 1/1] virtio-media: Add virtio media device specification
> >
> > 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.
> If it is v4l2, any reason to still call it virtio-media? How about virtio-v4l2 to describe what it is.

Originally it was virtio-v4l2, but it was changed along the way.
I think that the main reason is to avoid the possible confusion
that the virtio device requires v4l2 in the host and/or the guest,
when in fact just leverages v4l2 to define the communication
between device and driver. This strategy allows the virtio device
to keep up with API updates without having to explicitely
adapt the specification often.

I think the name virtio-media makes sense and describes the
device better. But acourbot@chromium.org could explain it
(and keep me honest with what I said above) with more detail,
as he is the owner and creator.

>
> [..]
> > +/* Header for all virtio commands from the driver to the device on the
> > +commandq. */ struct virtio_media_cmd_header {
> > +     u32 cmd;
> > +     u32 __padding;
> > +};
> Please fix u32 to le32 here and all other places.
>
> > +
> > +/* 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}
> > +
> I didn't read the spec fully but please explore admin commands as they offer resource (session) create/destroy infrastructure already.
> And v4l2 will get simplified a lot by reusing existing virtio admin framework.

Thanks, I'll check.

>
>
> > +\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}
> > +
> This maps to modify or it can be new command.

What do you mean?

>
> > +\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/vi
> > +deodev.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/bu
> > +ffer.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}
> > +
> Can you please screen it through, and make sure that in this UAPI there is no u16/32/64 data type.
> And all are leX or beX, to not assume any endianness between driver and device?

Sure, makes sense.

>
>
> > +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/bu
> > +ffer.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.
> > +
> > +However, the driver shall only advertise support for \textit{MMAP} and
> > +\textit{DMABUF} to the guest userspace. \textit{USERPTR} is merely
> > +employed to discern \textit{GUEST_PAGES} buffers, similar to how
> > +\textit{DMABUF} is used to signify \textit{VIRTIO_OBJECT} buffers in the
> > 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{GUEST_PAGES}
> > +
> Please change the notion from guest/host to device/driver respectively.

Ok. I'll change it to either SHARED_PAGES or DRIVER_PAGES,
to signify better that this memory is privisioned by the driver.

>
> > +In virtio-media, \textit{GUEST_PAGES} buffers are provisioned by the
> > +driver, and use guest physical addresses. 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{VIRTIO_OBJECT}
> > +
> > +In virtio-media, \textit{VIRTIO_OBJECT} 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;
> > +};
> Mostly you wont need padding once you move to admin commands, but wherever applicable, please convert into reserved field in the name.
> So, it can be used in future. Converting reserved to meaningful is doable compared to padding constant bytes.
>

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

[Albert Esteve]

On Thu, Oct 17, 2024 at 10:03 PM Daniel Verkamp <dverkamp@chromium.org> wrote:
>
> On Thu, Oct 17, 2024 at 12:04 AM Albert Esteve <aesteve@redhat.com> 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.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> [...]
> > diff --git a/device-types/media/description.tex b/device-types/media/description.tex
> > new file mode 100644
> > index 0000000..793e9b1
> > --- /dev/null
> > +++ b/device-types/media/description.tex
> > @@ -0,0 +1,583 @@
> > +\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}.
>
> I would omit the "The device assumes" part; this spec is defining the
> protocol used by both the device and driver.

Will do.
>
> It would also be nice to be a bit more explicit about the meaning of
> "64-bit"; presumably this means something like "struct layout matches
> the Linux psABI for <some 64-bit architecture>", although that's not
> great either IMO. Punting to the Linux docs rather than defining the
> structs in the virtio spec feels iffy to me (though this is also the
> approach taken by fs with fuse); hopefully none of the V4L2 structs
> use features like bitfields that could cause layout differences (and
> may not match the bit-field definition in the virtio spec), and
> hopefully Linux never introduces any new structs that don't make sense
> in virtio-media.

I see your point. But it has its advantages to do it this way.

I feel one of the points for taking this approach to begin with was
to actually leverage v4l2 specs to be able to keep up with API updates.
V4L2 API change relatively quickly (faster cadence that virtio), to
keep up with new devices and their needs.

At any point, if Linux introduces some struct that does not make
sense or breaks compatibility with virtio-media, we will be forced
to differ and explicitly set it in the virtio specification. But I think
this occurrence will be more scarce, if ever, compared to all the
support for new device that we obtain for free by just pointing
to V4L2 structs.

>
> Also, though this is more of an implementation-specific detail, I
> wonder how this will be handled by the Linux virtio-media driver; will
> it have code to convert between the 64-bit/little-endian layout used
> by virtio-media and the native V4L2 structs? Or is it assumed that the
> driver will never be used on a 32-bit or big-endian platform and
> structs can just be naively passed through without conversion?
>
> > +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}.
>
> This is probably redundant, since it just repeats the "definition of
> V4L2 structures and ioctls" link earlier in this same section.
>
> [...]
> > +\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}.
>
> nit: This link is pointing to v4.9 docs rather than "latest" like the
> others. (Maybe using a specific version would be preferable to avoid
> link rot, but all the links should match.)

True, thanks for noticing. I will fix it.
Fixed version links may rot too. I think latest should be fine.
But you are right, they all should match.

>
> [...]
> > +\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.
>
> nit: "thesession" is missing a space.
>
> > +\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 should be more specific about the ioctl encoding, which is not
> consistent across architectures (_IOC_SIZEBITS can differ).
>
> > +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.
>
> As discussed above, "64-bit representation" is pretty vague, and this
> section should probably also describe the endianness requirement.
>
> > +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:
>
> nit: "reponse" -> "response"
>
> > +\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.
>
> How is the driver supposed to know "[i]f the device has not written
> the payload"? It would probably be clearer to say the driver may not
> assume anything about the state of the payload when an error is
> returned.
>
> [...]
>

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

[Albert Esteve]

On Fri, Oct 18, 2024 at 10:52 AM Matias Ezequiel Vara Larsen
<mvaralar@redhat.com> wrote:
>
> Hello Albert,
>
> I gave a first reading. I left some minor comments below. I could not
> cover the whole patch yet.
>
> On Thu, Oct 17, 2024 at 09:04:23AM +0200, 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.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >  conformance.tex                           |  13 +-
> >  content.tex                               |   1 +
> >  device-types/media/description.tex        | 583 ++++++++++++++++++++++
> >  device-types/media/device-conformance.tex |  11 +
> >  device-types/media/driver-conformance.tex |   9 +
> >  5 files changed, 613 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..793e9b1
> > --- /dev/null
> > +++ b/device-types/media/description.tex
> > @@ -0,0 +1,583 @@
> > +\section{Media Device}\label{sec:Device Types / Media Device}
> > +
> > +The virtio media device follow the same model (and structures) as V4L2. It
>
> s/follow/follows
>
> > +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}
> > +
> > +49
> > +
> > +\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.
> > +
>
> I think this is true for any device. I wonder what is specific for
> virtio-media to put it here.
>
> > +\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.
> > +
>
> This is also true for any device. The mechanism to provide buffers
> through the available ring to be consumed and then enqueued in the used
> ring is not device specific. Is there any reason that this is different
> for virtio-media?

Not really. I think you are right and this can be skipped. Shorten the
spec is good :)

>
> > +\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}.
>
> I think you should not start a sentence with a verb. I think you should
> add the person. Something like `It corresponds with the ...`. I see
> other cases below.
>
> > +\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}
> > +
> I think other devices remove `the driver` and write it as sequence by
> starting with the verb like:
>
> `- reads the field{}
>  - sets up the \field{}
>  ...
> `
> Also, the last sentence talks about `session` but it was not introduced
> yet, what is a session?
>
> > +\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}.
>
> Maybe I am missing something but where and how is stored the returned
> error? I mean, it is stored in the device-writable area but what is the
> structure?
>
> > +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}
> > +
>
> OK, I guess this is the structure that contains the response.
>
> > +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.
> > +
> I think this paragraph can be rewritten. For example, what do you mean
> with `multiplexed`? I think I got the idea but I would elaborate it a
> bit. Also, I think you can rewrite the last sentence to something like:
> `Before start operating, the driver must open a session`
>
> > +\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.
>
> I would rewrite it as:
>
> `... identifies the current session, which is used for other commands
> like ioctls.`
>
> But I do not have a strong opinion about it.
>
> > +\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.
> > +
> Why do you use `ask` here? Could it be `tells` instead? But I do not
> have a strong opinion about it. Also, I think with `active` you mean the
> session identified by `session_id`. I think I would not use `active`.
>
> > +\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.
>
> I would rewrite it as:
>
> `\item[\field{session_id}] identifies the session 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).
>
> I think you can remove some of the `thats`:
>
> `This means that its ioctl code is \textit{4}, its payload is a
> \textit{struct v4l2_format}, and 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.
> > +
>
> I think you can remove `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}.
> > +
>
> I would rewrite it as:
>
> `When a request is not supported, the device MUST return \textit{ENOTTY},
> which corresponds with the response for unknown ioctls.`
>
> Matias

I am ok with most/all the rewrites you suggested. Thanks for taking a look!

BR,
Albert.

>

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

[Albert Esteve]

On Mon, Oct 21, 2024 at 9:53 AM Alexandre Courbot <acourbot@chromium.org> wrote:
>
> Hi Albert, thanks for this new revision!
>
> On Thu, Oct 17, 2024 at 4:04 PM Albert Esteve <aesteve@redhat.com> 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.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >  conformance.tex                           |  13 +-
> >  content.tex                               |   1 +
> >  device-types/media/description.tex        | 583 ++++++++++++++++++++++
> >  device-types/media/device-conformance.tex |  11 +
> >  device-types/media/driver-conformance.tex |   9 +
> >  5 files changed, 613 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..793e9b1
> > --- /dev/null
> > +++ b/device-types/media/description.tex
> > @@ -0,0 +1,583 @@
> > +\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}
> > +
> > +49
> > +
> > +\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;
>
> This field is now
>
>   u32 offset;
>
> u32 is the type actually returned by V4L2_QUERYBUF for the offset
> field, so using a u64 here made no sense. Furthermore, I realized that
> the offset is an identifier more than an actual starting address entry
> in a range that must cover the whole buffer's length, so there is no
> risk of running out of space even with a u32.

Sounds good, thanks for pointing this out.
Parav commented to change all fields to leX (le32, le64, etc.), so
that the endianess
is not assumed through the spec. I think that makes sense, it was also something
I was pondering when writing the specs to make it consistent with all other
structures.

However, it would require changing all structures in the virtio-media
driver and device code, and handle endianess translation where
needed. I hope that is ok?

>
> > +};
> > +\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;
>
> This has been renamed to
>
>   u64 guest_addr;
>
> to avoid confusion with virtio_media_cmd_mmap's "offset" field.
>
> > +    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;
>
> Similarly to above, this has been renamed to
>
>   u64 guest_addr;

In general I am being asked to remove guest from the spec and use
driver instead.
So I will change it for driver_addr instead.

BR,
Albert.

>
> Cheers,
> Alex.
>

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

[Albert Esteve]

On Wed, Oct 23, 2024 at 7:25 AM Alexandre Courbot <acourbot@google.com> wrote:
>
> On Mon, Oct 21, 2024 at 10:42 PM Albert Esteve <aesteve@redhat.com> wrote:
> >
> > On Mon, Oct 21, 2024 at 9:53 AM Alexandre Courbot <acourbot@chromium.org> wrote:
> > >
> > > Hi Albert, thanks for this new revision!
> > >
> > > On Thu, Oct 17, 2024 at 4:04 PM Albert Esteve <aesteve@redhat.com> 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.
> > > >
> > > > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > > > ---
> > > >  conformance.tex                           |  13 +-
> > > >  content.tex                               |   1 +
> > > >  device-types/media/description.tex        | 583 ++++++++++++++++++++++
> > > >  device-types/media/device-conformance.tex |  11 +
> > > >  device-types/media/driver-conformance.tex |   9 +
> > > >  5 files changed, 613 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..793e9b1
> > > > --- /dev/null
> > > > +++ b/device-types/media/description.tex
> > > > @@ -0,0 +1,583 @@
> > > > +\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}
> > > > +
> > > > +49
> > > > +
> > > > +\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;
> > >
> > > This field is now
> > >
> > >   u32 offset;
> > >
> > > u32 is the type actually returned by V4L2_QUERYBUF for the offset
> > > field, so using a u64 here made no sense. Furthermore, I realized that
> > > the offset is an identifier more than an actual starting address entry
> > > in a range that must cover the whole buffer's length, so there is no
> > > risk of running out of space even with a u32.
> >
> > Sounds good, thanks for pointing this out.
> > Parav commented to change all fields to leX (le32, le64, etc.), so
> > that the endianess
> > is not assumed through the spec. I think that makes sense, it was also something
> > I was pondering when writing the specs to make it consistent with all other
> > structures.
> >
> > However, it would require changing all structures in the virtio-media
> > driver and device code, and handle endianess translation where
> > needed. I hope that is ok?
>
> I guess that's a necessity anyway, unless we disable build for
> big-endian platforms. For the driver we can probably keep the
> structures as they are, as long as we make sure to fix their
> endianness when pushing them on the virtqueue.

Makes sense. Thanks for confirming!

BR,
Albert.

>
> >
> > >
> > > > +};
> > > > +\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;
> > >
> > > This has been renamed to
> > >
> > >   u64 guest_addr;
> > >
> > > to avoid confusion with virtio_media_cmd_mmap's "offset" field.
> > >
> > > > +    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;
> > >
> > > Similarly to above, this has been renamed to
> > >
> > >   u64 guest_addr;
> >
> > In general I am being asked to remove guest from the spec and use
> > driver instead.
> > So I will change it for driver_addr instead.
> >
> > BR,
> > Albert.
> >
> > >
> > > Cheers,
> > > Alex.
> > >
> >
>