Re: [PATCH v1 2/4] virtio-msg: Add virtio-msg, a message based virtio transport layer
Peter Hilber <[email protected]> Thu, 26 Feb 2026 14:53:37 +0100
| Newsgroups | dev.linux.lists.virtio-comment |
|---|---|
| Message-ID | <rlqvriokmytc7etiayw6chxbti4nhjxy4gkhdf5bkftw42yfgm@lsl7ekuxe3il> |
On Fri, Feb 20, 2026 at 08:23:26AM +0000, Bertrand Marquis wrote: > Hi Peter, > > > On 12 Feb 2026, at 12:16, Peter Hilber > > <[email protected]> wrote: > > > > On Tue, Feb 10, 2026 at 09:39:09AM +0000, Bertrand Marquis wrote: > >> Hi Peter, > >> > > > > Hi Bertrand, > > > > thank you for the fast response! I added replies where I think they are > > relevant. I also added a few new comments. > > > > Best regards, > > > > Peter > > > >>> On 6 Feb 2026, at 17:28, Peter Hilber <[email protected]> wrote: > >>> > >>> On Mon, Jan 26, 2026 at 11:32:28AM -0500, Bill Mills wrote: [...] > >>>> +\busnormative{\paragraph}{Error Handling}{Virtio Transport Options / Virtio Over Messages / Basic Concepts / Error Signaling / Bus} > >>>> +\begin{itemize} > >>>> + \item A bus implementation MAY report a transport-visible failure (for > >>>> + example, after exhausting a bounded retry policy) when it cannot deliver > >>>> + a request or obtain a response. > >>>> + \item A bus implementation MUST treat malformed headers or unsupported > >>>> + \field{msg_id} values as invalid, MUST discard them without generating > >>>> + additional protocol traffic, and MAY log the condition for diagnostics. > > > > The bus does not have to understand transport messages, so does it have > > to recognize all unsupported msg_id values? > > Very true, i will need to fix the wording to state invalid bus msg ids, transport should > discard invalid transport message IDs. > I am also wondering why any "additional protocol traffic" is forbidden for unsupported (bus or transport) requests. Couldn't it be reasonable to indicate an error so that the remote endpoint doesn't need to wait for a response timeout? > > > >>>> + \item A bus implementation MUST NOT generate error responses to event > >>>> + (one-way) messages. > >>>> +\end{itemize} > >>>> + > >>>> +\devicenormative{\paragraph}{Error Handling}{Virtio Transport Options / Virtio Over Messages / Basic Concepts / Error Signaling / Device} > >>>> +\begin{itemize} > >>>> + \item A device receiving a malformed or unsupported transport message MUST > >>>> + discard it without producing further protocol traffic. > >>>> + \item Recovery actions taken in response to an error (such as retries, > >>>> + selective resets, or device removal) MUST follow the normative reset and > >>>> + status semantics defined in > >>>> + \ref{sec:Virtio Transport Options / Virtio Over Messages / Device Operation}. > >>>> +\end{itemize} > >>>> + > >>>> +This specification does not define a dedicated error-reporting message; it only > >>>> +permits implementations to surface failures when silent recovery is not > >>>> +feasible. > >>>> + [...] > >>>> +\msgdef{GET_DEVICE_FEATURES} > >>>> + > >>>> +Drivers retrieve device feature bits in 32-bit blocks via > >>>> +\msgref{GET_DEVICE_FEATURES}; the response echoes the requested block index and > >>>> +returns one or more 32-bit values with the feature bits in that range. > >>> > >>> I do not understand the purpose of echoing request values. Looking at > >>> virtqueue messages, the driver keeps track of its outstanding requests > >>> internally and looks them up upon receiving the response. Doesn't a > >>> virtio-msg transport also keep track internally? What advantage has > >>> echoing? > >> > >> We only echo the index in the response (there is no data in the request). > >> The idea here is to allow asynchronous requests/response and provide > >> an easy way to identify the content of the data in the response. > > > > I would have thought that it is simpler and more robust if > > implementations would refer to the token to identify which request is > > being responded to (instead of possibly taking the response echoes at > > face value). Implementations might even have at most one outstanding > > request at a time. > > We want to be future proof so that this is possible in the future. > If tokens are purely handled by the bus, having all information might be > easier for the transport. > I still feel uneasy, since I think the context of a response would be available locally easily and in a more robust manner (e.g. if the same request is issued repeatedly). The bus preserves the (per-device) request-response order anyway. So couldn't the bus layer point out the association of responses to requests to the transport (as needed, e.g. through a pointer submitted by the transport alongside the request)? Also, omitting the echoes would shrink the response size (with message sizes being a concern AFAIU). > >>>> +\msgdef{SET_DEVICE_STATUS} > >>>> + > >>>> +\msgref{SET_DEVICE_STATUS} writes a new device status value. Drivers use it to > >>>> +progress through the virtio-defined states or to request a reset by writing 0. > >>>> +The device responds with its resulting status, which may differ (for example, > >>>> +if it refuses FEATURES\_OK or sets DEVICE\_NEEDS\_RESET). > >>>> + > >>>> +\begin{lstlisting} > >>>> +struct virtio_msg_set_device_status_req { > >>>> + le32 status; /* desired device status value */ > >>>> +}; > >>>> + > >>>> +struct virtio_msg_set_device_status_resp { > >>>> + le32 status; /* resulting device status */ > >>>> +}; > >>>> +\end{lstlisting} > >>>> + > >>>> +\drivernormative{\paragraph}{SET\_DEVICE\_STATUS}{Virtio Transport Options / Virtio Over Messages / Transport Messages / VIRTIO_MSG_SET_DEVICE_STATUS / Driver} > >>>> +\begin{itemize} > >>>> + \item A driver MUST write 0 via \msgref{SET_DEVICE_STATUS} to request a device > >>>> + reset and MUST re-read the status (e.g., via \msgref{GET_DEVICE_STATUS}) > >>>> + if it needs to confirm acceptance. > > > > SET_DEVICE_STATUS already responds with the resulting device status. > > Why would re-reading the status be necessary? > > We identified some cases in which a reset cannot be handled atomically hence the need to have a way to confirm or wait until reset is finished. I think this should be rephrased to explicitly indicate that the driver must wait until it observes the device status change. As of now, it could be interpreted to mean that the driver just needs to confirm that a status change did not fail by re-reading once. Best regards, Peter