public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
From: Demi Marie Obenour <demiobenour@gmail.com>
To: Bertrand Marquis <Bertrand.Marquis@arm.com>
Cc: "Bill Mills (bill.mills@linaro.org)" <bill.mills@linaro.org>,
	"virtio-comment@lists.linux.dev" <virtio-comment@lists.linux.dev>,
	"Edgar E . Iglesias" <edgar.iglesias@amd.com>,
	Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Alex Bennee <alex.bennee@linaro.org>,
	Armelle Laine <armellel@google.com>
Subject: Re: [PATCH v1 2/4] virtio-msg: Add virtio-msg, a message based virtio transport layer
Date: Fri, 20 Feb 2026 21:04:50 -0500	[thread overview]
Message-ID: <9647c9bf-3f62-42fe-ad07-7e9dacc613dd@gmail.com> (raw)
In-Reply-To: <EC730EB8-9CEB-4B50-9295-2E1A908E22EF@arm.com>


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

On 2/20/26 03:52, Bertrand Marquis wrote:
> Hi Demi,
> 
>> On 13 Feb 2026, at 20:09, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>
>> 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.
> 
> We have the token meant to be used to reorder messages or make them correspond to a request.
> 
> Now we choosed on the first version to enforce the order to prevent having something that would not
> work in the existing virtio implementation where drivers are not designed to handle this (configuration
> space access is done in order and is not expected to get answers coming back randomly when reading it).
> 
> Now I do agree that this could be a place for performance optimization and it would be possible to (as pointed
> out by Mickael) introduce this as a feature bit so that implementation could support that or not and disable it.
> This could be handled at bus level or directly on the device side.

This was made under the (mistaken) assumption that virtqueue data was
also sent over messages.  An ordering requirement for that _would_
be a performance problem.

I do believe that virtqueue notifications (data available/data
consumed) should be allowed to be received out of order.  There is no
requirement that these be _sent_ in any particular order, so further
reordering by the bus isn't going to make things worse.

>>> +\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.
> 
> At the bus layer there are only requests and responses but at the end on the device
> side you only get requests that you have to answer or event that you send. There is
> no case of request send by the device side which would require an answer from the
> driver side.
> This why we have this here but we could relax it if you think this could be useful.

I think this would be better written as a constraint on devices, rather than as
a constraint on buses.  Right now, it reads as if the bus is responsible for
ensuring messages are replied to.

>>> +\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?
> 
> This is what we meant to say with acceptance/rejection. If something is not accepted
> the FEATURES_OK bit will not be set in the message returned.
> Maybe we need rephrasing to make that clearer ?

I agree.

>>> +\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.
> 
> This is to be handled at the bus level which should return an error to the transport.
> FF-A bus defines an error message so that the sender bus when receiving it back
> for a request can give an error back to the transport.
> 
> Maybe we should describe this model in non normative way in the spec ?
> 
> 
>>
>>> +\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.
> 
> The token is meant for that and as said in other mails, we will investigate the solution to
> introduce a feature bit to enable/disable out-of-order to leave a door open to such optimizations.

See above.

(snip)

>>> +  \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?
> 
> If polling is used, there might be cases where this will never be sent.
> Maybe this should be a MUST with something like "unless polling is used" or something
> like that.

What about this?

+  \item A device MUST 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 such as polling.

>>> +\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?
> 
> Shared memory can be used and we have a message for that.
> 
> To be able to use virtqueues and dma between device and driver memory must be made accessible
> to the other side as it is the case for other transports.

Should these be I/O virtual addresses instead of guest physical addresses?
-- 
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 --]

  reply	other threads:[~2026-02-21  2:05 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-26 16:32 [PATCH v1 0/4] virtio-msg transport layer Bill Mills
2026-01-26 16:32 ` [PATCH v1 1/4] virtio-msg: add new command for bus normative Bill Mills
2026-02-03 19:42   ` Edgar E. Iglesias
2026-01-26 16:32 ` [PATCH v1 2/4] virtio-msg: Add virtio-msg, a message based virtio transport layer Bill Mills
2026-02-06 16:28   ` Peter Hilber
2026-02-10  9:39     ` Bertrand Marquis
2026-02-12 11:16       ` Peter Hilber
2026-02-20  8:23         ` Bertrand Marquis
2026-02-26 13:53           ` Peter Hilber
2026-02-13 19:09   ` Demi Marie Obenour
2026-02-20  8:52     ` Bertrand Marquis
2026-02-21  2:04       ` Demi Marie Obenour [this message]
2026-02-23  7:44         ` Bertrand Marquis
2026-02-24 15:41   ` Demi Marie Obenour
2026-02-24 16:14     ` Bertrand Marquis
2026-02-24 17:36     ` Edgar E. Iglesias
2026-02-24 17:14   ` Demi Marie Obenour
2026-02-24 17:20     ` Bertrand Marquis
2026-02-24 17:46       ` Demi Marie Obenour
2026-02-25  7:26         ` Bertrand Marquis
2026-02-25 12:36   ` Demi Marie Obenour
2026-02-25 12:46     ` Bertrand Marquis
2026-01-26 16:32 ` [PATCH v1 3/4] virtio-msg: link virtio-msg content Bill Mills
2026-02-03 19:43   ` Edgar E. Iglesias
2026-01-26 16:32 ` [PATCH v1 4/4] virtio-msg: add conformance entries in conformance chapter Bill Mills
2026-02-03 19:43   ` Edgar E. Iglesias
2026-01-26 21:47 ` [PATCH v1 0/4] virtio-msg transport layer Demi Marie Obenour
2026-02-03 13:21 ` Michael S. Tsirkin
2026-02-03 19:48   ` Edgar E. Iglesias
2026-02-03 19:55     ` Michael S. Tsirkin
2026-02-04  8:33   ` Bertrand Marquis
2026-02-04 13:50   ` Arnaud POULIQUEN
2026-02-04  3:29 ` Viresh Kumar
2026-02-04  5:34 ` Manivannan Sadhasivam
2026-02-13 13:52 ` Parav Pandit
2026-02-13 19:45   ` Demi Marie Obenour
2026-02-19 17:31     ` Armelle Laine
2026-02-20  8:55     ` Bertrand Marquis
2026-02-19 23:54   ` Michael S. Tsirkin
2026-02-20  6:13     ` Parav Pandit
2026-02-20  9:02       ` Bertrand Marquis
2026-02-25  7:45         ` Manivannan Sadhasivam
2026-02-25  8:03           ` Bertrand Marquis
2026-02-25  8:12             ` Michael S. Tsirkin
2026-02-25 10:06             ` Manivannan Sadhasivam
2026-02-25 10:10               ` Michael S. Tsirkin
2026-02-25 10:14               ` Bertrand Marquis
2026-02-25 10:22               ` Michael S. Tsirkin
2026-02-25 10:53                 ` Manivannan Sadhasivam
2026-02-25 10:24               ` Parav Pandit
2026-02-25 10:35                 ` Bertrand Marquis
2026-02-25 10:52                   ` Michael S. Tsirkin
2026-02-25 10:55                     ` Bertrand Marquis
2026-02-25 10:58                       ` Michael S. Tsirkin
2026-02-25 14:45                   ` Parav Pandit
2026-02-25 14:49                     ` Michael S. Tsirkin
2026-02-25 14:53                       ` Bertrand Marquis
2026-02-25 15:00                         ` Parav Pandit
2026-02-25 15:07                           ` Parav Pandit
2026-02-25 15:12                             ` Bertrand Marquis
2026-02-25 15:15                               ` Michael S. Tsirkin
2026-02-25 15:36                               ` Demi Marie Obenour
2026-02-25 15:40                                 ` Bertrand Marquis
2026-02-25 15:48                                   ` Demi Marie Obenour
2026-02-25 15:51                                     ` Bertrand Marquis
2026-02-25 16:15                                       ` Demi Marie Obenour
2026-02-26  5:40                               ` Manivannan Sadhasivam
2026-02-26  7:05                                 ` Bertrand Marquis
2026-02-25 15:11                           ` Manivannan Sadhasivam
2026-02-25 15:15                             ` Parav Pandit
2026-02-26  5:36                               ` Manivannan Sadhasivam
2026-02-26  5:59                                 ` Parav Pandit
2026-02-26  6:19                                   ` Manivannan Sadhasivam
2026-02-26  7:01                                 ` Bertrand Marquis
2026-02-26  7:28                                   ` Manivannan Sadhasivam
2026-02-26 19:20                                 ` Demi Marie Obenour
2026-02-26 22:08                                   ` Edgar E. Iglesias
2026-02-25 15:23                           ` Demi Marie Obenour
2026-02-25 16:42                         ` Edgar E. Iglesias
2026-02-25 12:53           ` Demi Marie Obenour
2026-02-25 13:09             ` Manivannan Sadhasivam
2026-02-25 13:12               ` Demi Marie Obenour
2026-02-25 13:29                 ` Bertrand Marquis
2026-02-25 15:19                   ` Demi Marie Obenour
2026-02-25 15:27                     ` Bertrand Marquis
2026-02-20 10:03       ` Michael S. Tsirkin
2026-02-25  5:09         ` Parav Pandit
2026-02-25  7:25           ` Michael S. Tsirkin
2026-02-25  9:18             ` Parav Pandit
2026-02-25  9:22               ` Michael S. Tsirkin
2026-02-25  9:35                 ` Bertrand Marquis
2026-02-25  9:54                   ` Michael S. Tsirkin
2026-02-25 10:01                     ` Bertrand Marquis
2026-02-25 10:08                       ` Michael S. Tsirkin
2026-02-20  8:58     ` Bertrand Marquis
2026-02-20  8:40   ` Bertrand Marquis
2026-02-25  4:58     ` Parav Pandit
2026-02-25  7:52       ` Bertrand Marquis
2026-02-25 12:46         ` Demi Marie Obenour
2026-02-25 13:05           ` Bertrand Marquis
2026-02-25 13:09             ` Demi Marie Obenour
2026-02-25 15:17         ` Bertrand Marquis
2026-02-24 17:57 ` Demi Marie Obenour
2026-02-25 15:21   ` Alex Bennée
2026-02-25 15:46     ` Demi Marie Obenour

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=9647c9bf-3f62-42fe-ad07-7e9dacc613dd@gmail.com \
    --to=demiobenour@gmail.com \
    --cc=Bertrand.Marquis@arm.com \
    --cc=alex.bennee@linaro.org \
    --cc=armellel@google.com \
    --cc=arnaud.pouliquen@foss.st.com \
    --cc=bill.mills@linaro.org \
    --cc=edgar.iglesias@amd.com \
    --cc=viresh.kumar@linaro.org \
    --cc=virtio-comment@lists.linux.dev \
    /path/to/YOUR_REPLY

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

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