Re: [PATCH v1 2/4] virtio-msg: Add virtio-msg, a message based virtio transport layer

[Demi Marie Obenour <demiobenour@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 12672 bytes --]

On 1/26/26 11:32, Bill Mills wrote:
> Add a new transport layer that is based on messages.
> 
> This transport layer still uses virtqueues as the other transport layers do
> but implements transport layer operations by sending and receiving messages
> instead of the "MMR" reads and writes used in virtio-mmio and virtio-pci.
> 
> This transport is useful when the device and driver are both implemented in
> software but the trap and emulate operations of virtio-mmio and virtio-pci
> can not be used.

(snip)

> +\subsubsection{Message Ordering}
> +\label{sec:Virtio Transport Options / Virtio Over Messages / Basic Concepts / Ordering}
> +
> +Transport messages fall into two classes: requests (which expect responses) and
> +events (which are one-way). Drivers and devices rely on the bus to preserve the
> +relative ordering of request/response pairs for each device number; they do not
> +interpret the \field{token} field directly.

I expect that this requires all messages for a given device to be
processed sequentially, which is not going to be fast.  At the very
least, messages on different virtqueues should be able to be processed
out of order.

I would have some sort of stream identifier that is made visible to
the bus layer.  This would map to a QUIC stream, a single ring buffer,
or something else that ensures in-order delivery.  Messages with
different stream identifiers may be processed out of order.

> +\busnormative{\paragraph}{Message Ordering}{Virtio Transport Options / Virtio Over Messages / Basic Concepts / Ordering / Bus}
> +\begin{itemize}
> +  \item For each device number, a bus implementation MUST deliver responses to
> +        the driver in the same order that it forwarded the corresponding
> +        requests to the device.
> +  \item A bus implementation MUST ensure that every request forwarded to a
> +        device results in exactly one response delivered to the driver (unless
> +        the request is defined as an event).
> +\end{itemize}
What is the reason for the device/driver distinction here?
Intuitively, I expect both requests and responses to just be messages
at the bus layer.

(snip)

> +\devicenormative{\paragraph}{Feature Negotiation}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Device Features / Device}
> +\begin{itemize}
> +  \item When handling \msgref{GET_DEVICE_FEATURES}, a device MUST return zero
> +        for any requested bits that fall outside the number of feature bits it
> +        implements.
> +  \item After receiving \msgref{SET_DRIVER_FEATURES}, a device MUST update its
> +        internal feature mask to match the acknowledged set and MUST reflect
> +        acceptance or rejection by leaving the FEATURES\_OK bit set or clearing
> +        it in the status returned by \msgref{SET_DEVICE_STATUS}.
> +\end{itemize}

What should the device do if it doesn't support the feature?

> +\subsubsection{Device Configuration}
> +\label{sec:Virtio Transport Options / Virtio Over Messages / Device Initialization / Device Configuration}
> +
> +Drivers use \msgref{GET_CONFIG} to read portions of the configuration space by
> +supplying an offset and length; the device returns the requested data plus the
> +current configuration generation count. Writing is performed via
> +\msgref{SET_CONFIG}, which carries the same offset/length along with the
> +driver's notion of the generation count and the new data.

I think this should come with a (non-normative) note that the use of
configuration space is discouraged if the data can instead be sent
over a virtqueue.  For MMIO and PCI transports, the configuration
space is slightly more convenient as it is accessed synchronously,
but for the message transport, reads are also asynchronous unless
the driver spins or sleeps until getting a reply.

> +\drivernormative{\paragraph}{Device Configuration}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Device Configuration / Driver}
> +\begin{itemize}
> +  \item A driver MUST ensure that the offset and length in each
> +        \msgref{GET_CONFIG} or \msgref{SET_CONFIG} request stay within the
> +        configuration size reported by \msgref{GET_DEVICE_INFO}.
> +  \item A driver MUST include its most recently observed configuration
> +        generation count in a \msgref{SET_CONFIG} request and SHOULD re-read the
> +        configuration (via \msgref{GET_CONFIG}) if the request is rejected for a
> +        generation mismatch.
> +\end{itemize}
> +
> +\devicenormative{\paragraph}{Device Configuration}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Device Configuration / Device}
> +\begin{itemize}
> +  \item A device MUST reject a \msgref{SET_CONFIG} request whose generation
> +        count does not match its current value and MUST indicate the rejection
> +        in the response.
> +  \item A device MUST return the current configuration generation count
> +        alongside any data returned via \msgref{GET_CONFIG}.
> +\end{itemize}
> +
> +\subsubsection{Virtqueue Configuration}
> +\label{sec:Virtio Transport Options / Virtio Over Messages / Device Initialization / Virtqueue Configuration}
> +
> +Drivers query virtqueue parameters with \msgref{GET_VQUEUE}, configure them via
> +\msgref{SET_VQUEUE}, and optionally reset them using \msgref{RESET_VQUEUE} (if
> +VIRTIO\_F\_RING\_RESET is negotiated). Each queue is typically configured by
> +reading its maximum size, provisioning descriptor/available/used buffers, and
> +then calling \msgref{SET_VQUEUE} with the chosen size and guest-physical
> +addresses.
> +
> +\drivernormative{\paragraph}{Virtqueue Configuration}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Virtqueue Configuration / Driver}
> +\begin{itemize}
> +  \item A driver MUST use \msgref{GET_VQUEUE} to determine the maximum queue
> +        size and confirm that a queue is inactive before programming it.
> +  \item A driver MUST ensure the queue size provided in \msgref{SET_VQUEUE} does
> +        not exceed the maximum reported by the device and MUST supply valid
> +        descriptor/driver/device addresses before enabling the queue.
> +  \item If VIRTIO\_F\_RING\_RESET has been negotiated and a queue requires
> +        reinitialization, the driver SHOULD use \msgref{RESET_VQUEUE} before
> +        reprogramming it.
> +\end{itemize}

Why SHOULD and not MUST?

> +\devicenormative{\paragraph}{Virtqueue Configuration}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Virtqueue Configuration / Device}
> +\begin{itemize}
> +  \item A device MUST report accurate maximum queue sizes in \msgref{GET_VQUEUE}
> +        and MUST persist the parameters supplied via \msgref{SET_VQUEUE} (size,
> +        descriptor, driver, and device addresses).
> +  \item When \msgref{RESET_VQUEUE} is issued (and VIRTIO\_F\_RING\_RESET is
> +        negotiated), the device MUST quiesce the queue, release any resources
> +        associated with it, and allow the driver to reconfigure it.
> +\end{itemize}

What should a device do if it gets a request that a driver is
forbidden from making?  Untested corner-cases tend to be a good
source of security vulnerabilities, so defining behavior in all cases
seems better.

> +\subsubsection{Status Information}
> +\label{sec:Virtio Transport Options / Virtio Over Messages / Device Initialization / Status Information}
> +
> +Drivers query the device status via \msgref{GET_DEVICE_STATUS} to observe
> +progress or detect errors, and they drive the Virtio status transitions via
> +\msgref{SET_DEVICE_STATUS}. Writing zero to the status field resets the device,
> +invalidating any configuration or virtqueue state.
> +
> +\drivernormative{\paragraph}{Status Handling}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Status Information / Driver}
> +\begin{itemize}
> +  \item A driver SHOULD read the device status via \msgref{GET_DEVICE_STATUS}
> +        when diagnosing errors or determining whether the device is ready to
> +        move to the next initialization phase.
> +  \item A driver MUST use \msgref{SET_DEVICE_STATUS} to drive the device through
> +        the virtio-defined status states and MUST write 0 to request a device
> +        reset when needed.
> +\end{itemize}
> +
> +\devicenormative{\paragraph}{Status Handling}{Virtio Transport Options / Virtio Over Messages / Device Initialization / Status Information / Device}
> +\begin{itemize}
> +  \item Upon receiving a \msgref{SET_DEVICE_STATUS} write of 0, a device MUST
> +        reset its internal state, invalidate existing configuration and
> +        virtqueue settings, and present the status field as 0.
> +  \item A device MUST report its current status accurately via
> +        \msgref{GET_DEVICE_STATUS}, including whether the FEATURES\_OK bit has
> +        been accepted or cleared.
> +\end{itemize}

This is fine if all messages are processed in-order, but that is very
bad for performance (see above).  As soon as out-of-order message
handling becomes possible, a race condition will arise: replies and
notifications from before the reset can arrive after the reset.

I thknk solving this requires either having a generation or stream
number in each message, or delegating reset to the bus layer.

(snip)

> +\subsubsection{Device Notifications}
> +\label{sec:Virtio Transport Options / Virtio Over Messages / Device Operation / Device Notifications}
> +
> +\msgref{EVENT_CONFIG} and \msgref{EVENT_USED} provide asynchronous notifications
> +from the device (or device-side bus) to the driver. The bus may forward these
> +messages in-band or synthesize them based on other signals such as interrupts
> +or polling.
> +
> +\devicenormative{\paragraph}{Device Notifications}{Virtio Transport Options / Virtio Over Messages / Device Operation / Device Notifications / Device}
> +\begin{itemize}
> +  \item A device (or device-side bus) MUST send \msgref{EVENT_CONFIG} whenever
> +        it makes a configuration change or status update that becomes visible to
> +        the driver. The message MUST include the new configuration generation
> +        count and MAY include the updated configuration data.

I would change the second line to a MUST, to avoid a round trip.

> +  \item If the configuration data is omitted from \msgref{EVENT_CONFIG}, the
> +        device SHOULD include the relevant offsets/lengths so the driver can
> +        re-fetch the data via \msgref{GET_CONFIG}.

This makes this clause unnecessary.

> +  \item A device SHOULD send \msgref{EVENT_USED} to inform the driver when
> +        buffers on a virtqueue have been consumed, unless the device relies on
> +        an alternative, agreed-upon completion mechanism.

Why SHOULD and not MUST?

> +\end{itemize}

(snip)

> +\msgdef{GET_VQUEUE}
> +
> +\msgref{GET_VQUEUE} returns information about a specific virtqueue, including
> +its maximum size, current size, and, if already configured, the descriptor,
> +driver, and device area addresses.
> +
> +\begin{lstlisting}
> +struct virtio_msg_get_vqueue_req {
> +        le32 index; /* virtqueue index */
> +};
> +
> +struct virtio_msg_get_vqueue_resp {
> +        le32 index;        /* echoed virtqueue index */
> +        le32 max_size;     /* maximum queue size */
> +        le32 cur_size;     /* current size (0 if unconfigured) */
> +        le32 reserved;     /* must be zero */
> +        le64 desc_addr;    /* descriptor area address */
> +        le64 driver_addr;  /* driver area address */
> +        le64 device_addr;  /* device area address */
> +};
> +\end{lstlisting}
> +
> +\devicenormative{\paragraph}{GET\_VQUEUE}{Virtio Transport Options / Virtio Over Messages / Transport Messages / VIRTIO_MSG_GET_VQUEUE / Device}
> +\begin{itemize}
> +  \item A device MUST report accurate maxima and current queue sizes for each
> +        virtqueue and MUST return zero as the current size if the queue has not
> +        yet been configured.
> +\end{itemize}
> +
> +\msgdef{SET_VQUEUE}
> +
> +\msgref{SET_VQUEUE} programs a virtqueue's size and buffer addresses. The driver
> +selects a queue index, supplies the desired size (not exceeding the maximum
> +reported via \msgref{GET_VQUEUE}), and provides guest-physical addresses for the
> +descriptor, driver, and device areas.

Is the intention to still require shared memory?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]