Proposed changes Oct 16: token and response matching, error handling, redundant messages sizes and editorial input from Matias and Peter #25

wmamills · 2025-10-16T14:01:32Z

Redone after discussion at Oct 23 meeting

Dropped the "remove redundant length fields" commit
Added GET/SET_VQUEUE padding
A few fixups/simplifications for response matching

---- Older
Redone after discussion at Oct 16 meeting

Included @bertrand-marquis patches as-is and then made my changes as new commits. I was going to keep that branch for transparency but squash them for the PR. I decided instead to not squash in the PR for better local project transparency. When we do PATCHv1 or RFCv3 all local commits will be squashed anyway.

DROPPED

virtio-msg: add req_id to common header

ADDED:

virtio-msg: 8 byte header with unique identifier
- virtio-msg: revert chucks of msg_id -> msg_op
- virtio-msg: rename msg_uid to token and msg_op back to msg_id
virtio-msg: introduce a message correlation protocol
- virtio-msg: fix up message correlation protocol
virtio-msg: Relax error handling in the bus
- virtio-msg: Fix-up error handling in the bus

As before:

virtio-msg: remove redundant length fields
virtio-msg: editorial changes from Matias
virtio-msg: editorial changes from Peter
- Minor fixups in message header, use new text but use types from Peter

Move the header to be 8 bytes and include a unique identifier that can be use to correlate request to responses. Rename the msg_id field to msg_op as having a msg_id and msg_uid field and using identifiers in 2 contexts would have been unclear. Rework ID to OP and change wording message identifer to message operation to be coherent in the rest of the specification. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

This reverts commit 4c2274c. Signed-off-by: Bill Mills <bill.mills@linaro.org>

Rename these as agreed in the meeting on Oct 17. Signed-off-by: Bill Mills <bill.mills@linaro.org>

Add a sub-section to do a first explanation draft of how the message correlation is to be done using a combination of a message device number and the message unique identifier value. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

* change msg_uid to token and msg_op back to msg_id * More explicitly define tuple * Use "request originator" not "driver" * Restate that Requests have responses and Events don't in intro then drop those qualifications from the rules. Otherwise it sounds like some requests might not have responses and some events might have them * Specify that an implementation does not need to support sending multiple requests in parallel * Use the word "assign" not "allocate" as it is less suggestive of implementation choice Signed-off-by: Bill Mills <bill.mills@linaro.org>

Relax error handling in the virtio-msg bus and introduce wording to allow the virtio-msg bus to report error to the virtio-msg transport if a request could not be transmited or a malformed response was received. This prevents the need to generate dummy responses in the bus and make the specification coherent with the current Linux PoC implementation. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

* change msg_op back to msg_id Signed-off-by: Bill Mills <bill.mills@linaro.org>

bertrand-marquis

Some minor comments.
I am also not really a fan of removing the size fields in specific messages as it makes the computation and text a bit more complex.

transport-msg.tex

bertrand-marquis · 2025-10-20T07:52:57Z

transport-msg.tex

+  \item The request originator assigns a non-zero \field{token} for every
+    request such that the tuple is unique for all inflight requests.
+  \item Event (one-way) messages \textbf{MUST} set \field{token}=0
+  \item A response \textbf{MUST} echo \field{token}, \field{dev_num} (for


even for bus messages, dev_num (which should be 0) should be echoed. SO i suggest remove "(for transport)"

@bertrand-marquis This should be updated in this PR with a new commit.
If you agree please close this part of the convo; if not add comments to that commit

bertrand-marquis · 2025-10-20T07:54:27Z

transport-msg.tex

+    request such that the tuple is unique for all inflight requests.
+  \item Event (one-way) messages \textbf{MUST} set \field{token}=0
+  \item A response \textbf{MUST} echo \field{token}, \field{dev_num} (for
+    transport), and \field{msg_id}.


we might want to remove msg_id here. Correlation should only be based on dev_num and token.
In some cases, the bus might use a different message id for responses.
I have 2 examples in ffa bus:

event poll which returns an event as response to a poll

bus error messages which are returned in case of error to a request

OK, can we then also say that dev_num should match even in the case of bus messages? After all 0 == 0.

@bertrand-marquis same for this one and others

bertrand-marquis · 2025-10-20T07:55:10Z

transport-msg.tex

+    \textbf{SHOULD} result in discarding the response without further protocol
+    action.
+  \item Implementations \textbf{MAY} deliver responses out of order unless a
+    specific message definition mandates ordering.


Do we have this case ?
I am wondering if we should not completely remove the "unless ..." here

There are cases where order has to be enforced but I don't think it is mandated by the "message definition" and I think it is up to the sender to wait for responses before sending new messages.

So, yes, I think we can drop the last sentence

bertrand-marquis · 2025-10-20T07:57:28Z

transport-msg.tex

 \msgref{SET_CONFIG}, providing:
 \begin{itemize}
-  \item \textbf{Offset} and \textbf{Number of bytes},
+  \item \textbf{Offset},


In the paragraph just before we say that the driver supplies "offset and number of bytes". We might want to update that part or specify here that the number of bytes is deducted from the payload size in the header

dropped this commit

bertrand-marquis · 2025-10-20T07:59:23Z

transport-msg.tex

 A feature block is a group of 32 bits.  The feature data \emph{MUST} be a
-multiple of 4 bytes in length.
+multiple of 4 bytes in length. The number of feature blocks is calculated from
+the message length in the common message header.


maybe we would make all that simpler by no enforcing feature blocks of 32bits.
But i am really not a fan of removing those size fields after reading the result

dropped this commit

edgarigl

Not sure if itwas intentional but you may have forgotten to remove the number of blocks from the get_features response (I understand it must be there in the get_features request).

Looking at get_config, the number of bytes is in the request but not in the response.

wmamills · 2025-10-23T11:44:00Z

@edgarigl said "Not sure if itwas intentional but you may have forgotten to remove the number of blocks from the get_features response"

OK if we keep the commit, I will fix that

* drop 'underlying' * add MUST to Optional Bus Messages * s/tables/table/ (Matias suggested s/defines/define/ but there is only one table ATM) (Also change table defines to table lists to avoid double "define") * If the bus adds s/it/its/ * Fix feature {itemize} list * drop verb from {Block Index} * Also fix s/32 bits values/32 bit values/ * drop \emph{chooses} in two places * s/defines/defined/ in Endianness section * add "and the buffer has been added to the used ring of the virtqueue" * s/in band/in-band and s/out of band event/out-of-band event * (Did not add "on", the sentience reads ok without it) * Replace sentence with "it should prevent this feature ..." * s/standardizes/standardized/ * Fix sentence about PING being optional Signed-off-by: Bill Mills <bill.mills@linaro.org>

* add missing ')' * clarify that bus properties are advertised from the bus to the transport layer and change "driver" to bus or transport layer to more clearly show whom is targeted by the requirements * Avoid passive voice and clearly specify which element must conform * s/All encoding of values and fields/All values and fields/ * Use u8 and le16 types for virtio_msg_message * s/must be zero of Bus messages/must be zero for Bus messages/ * use ':' at end of item in list, not ';' * Add missing verb and s/posted/available in Runtime Notifications Signed-off-by: Bill Mills <bill.mills@linaro.org>

Align 8 byte fields to offsets that are multiples of 8. Messages are byte streams not directly addressable C structures. Custom messages may use any field alignment they wish. However, for the current standard messages there is no harm in aligning them. This only effects GET_VQUEUE and SET_VQUEUE. Signed-off-by: Bill Mills <bill.mills@linaro.org>

Simplify the response matching as discussed in the PR and in the Oct 23 meeting. * Bus messages have a dev_num and it is normally 0 so we don't need to say that matching dev_num if only for transport messages. * Drop msg_id from matching. Altough the msg_id would normally match there may be some cases where it would not. The token field should be unique for a given dev_num. * This simplifys the description of the matching tuple as well Signed-off-by: Bill Mills <bill.mills@linaro.org>

bertrand-marquis

We need to discuss the point on correlation responsibility but this is definitely not a reason to block this. Just something to remember.

bertrand-marquis · 2025-10-24T06:32:48Z

transport-msg.tex

+
+Rules:
+\begin{itemize}
+  \item The request originator assigns a non-zero \field{token} for every


I think we have something unclear here that we should define: who is generating the token and handling the correlation ? All models are possible:

transport generating the token and doing correlation

bus generating token and doing correlation

transport generating token and bus doing correlation

bus generating token and transport doing correlation

In my view solutions 3 and 4 do not make sense as the one doing correlation must be in charge of generating the token.

I see some arguments to consider:

a bus that only handles one message a time does not need the token -> in favor of 2

a bus doing the correlation would want to generate itself the token, same for transport -> against 3 and 4

having the token and correlation handled at transport level would allow to reuse some code and simplify the bus work -> in favor of 1

in case of 1, the bus would still have to handle its own correlation for bus messages -> in favor of 2

OS blocking semantics for drivers might be easier to handle in the transport to prevent busses to potentially block where they should not -> in favor of 1

I am in favor of 1, even if the bus would have to handle correlation for its own messages because of the OS blocking semantics arguments but definitely something open for discussion.

Today AMP does the thread resumption in the bus. Resuming the right thread means matching the token. So this is number 2 above. This is required from the nature of the interface between transport and bus: one call with pointer to tx data and (optional) pointer to rx buffer.

If we change to number 1 we would need to change the interface between transport and bus.

Of course this choice can be made per implementation. Linux could do it one way and BSD or RTOS a different way. So I would prefer to keep it out of the virtio spec.

Moved this discussion to Issue #26 so we can find it easily after this PR is closed

bertrand-marquis · 2025-10-25T09:33:59Z

I agree but would be a good idea to mention that this implementation defined and maybe list the 2 models. Bertrand Marquis

________________________________ De : Bill Mills ***@***.***> Envoyé : Friday, October 24, 2025 3:56:47 PM À : Linaro/virtio-msg-spec ***@***.***> Cc : Bertrand Marquis ***@***.***>; Mention ***@***.***> Objet : Re: [Linaro/virtio-msg-spec] Proposed changes Oct 16: token and response matching, error handling, redundant messages sizes and editorial input from Matias and Peter (PR #25) @wmamills commented on this pull request.

________________________________ In transport-msg.tex<#25 (comment)>:

+driver side but bus message requests such as \busref{PING} may originate at

+either driver side or device side. + +This section defines how responses are correlated to requests. An implementation +does not need to support sending multiple requests in parallel but these rules +allow for that possibility. + +The token field in the message header is part of a tuple that is unique during +a request to response interval. +\begin{itemize} + \item Message tuple: ( \field{dev_num}, \field{token} ). +\end{itemize} + +Rules: +\begin{itemize} + \item The request originator assigns a non-zero \field{token} for every Today AMP does the thread resumption in the bus. Resuming the right thread means matching the token. So this is number 2 above. This is required from the nature of the interface between transport and bus: one call with pointer to tx data and (optional) pointer to rx buffer. If we change to number 1 we would need to change the interface between transport and bus. Of course this choice can be made per implementation. Linux could do it one way and BSD or RTOS a different way. So I would prefer to keep it out of the virtio spec. — Reply to this email directly, view it on GitHub<#25 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AMZC6T72GBQE7QOHB6XAMDT3ZIVZ7AVCNFSM6AAAAACJL6X3EGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGNZWHE2TCNRYHE>. You are receiving this because you were mentioned.Message ID: ***@***.***> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

bertrand-marquis and others added 6 commits October 9, 2025 09:48

virtio-msg: revert chucks of msg_id -> msg_op

e36035e

This reverts commit 4c2274c. Signed-off-by: Bill Mills <bill.mills@linaro.org>

virtio-msg: rename msg_uid to token and msg_op back to msg_id

8f3ea12

Rename these as agreed in the meeting on Oct 17. Signed-off-by: Bill Mills <bill.mills@linaro.org>

wmamills force-pushed the wam-review-10-16 branch from 9a04b02 to bf96f3c Compare October 18, 2025 12:55

wmamills changed the title ~~Proposed changes Oct 16: req_id, redundant messages sizes and editorial input from Matias and Peter~~ Proposed changes Oct 16: token and response matching, error handling, redundant messages sizes and editorial input from Matias and Peter Oct 18, 2025

wmamills mentioned this pull request Oct 18, 2025

Virtio msg: Add unique identifier and relax error handing by the bus #24

Closed

wmamills requested review from arnopo, bertrand-marquis and edgarigl October 18, 2025 13:10

virtio-msg: Fix-up error handling in the bus

fca607d

* change msg_op back to msg_id Signed-off-by: Bill Mills <bill.mills@linaro.org>

wmamills force-pushed the wam-review-10-16 branch from bf96f3c to 0454e72 Compare October 18, 2025 13:17

bertrand-marquis reviewed Oct 20, 2025

View reviewed changes

edgarigl reviewed Oct 20, 2025

View reviewed changes

wmamills added 4 commits October 23, 2025 22:06

wmamills force-pushed the wam-review-10-16 branch from 0454e72 to 6bad617 Compare October 24, 2025 02:06

bertrand-marquis approved these changes Oct 24, 2025

View reviewed changes

wmamills mentioned this pull request Nov 2, 2025

Handle remainder of Peter's feedback #27

Open

wmamills merged commit fb00064 into Linaro:virtio-msg-rfc2 Nov 2, 2025
1 check passed

Proposed changes Oct 16: token and response matching, error handling, redundant messages sizes and editorial input from Matias and Peter #25

Proposed changes Oct 16: token and response matching, error handling, redundant messages sizes and editorial input from Matias and Peter #25

Uh oh!

Conversation

wmamills commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bertrand-marquis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edgarigl left a comment

Choose a reason for hiding this comment

Uh oh!

wmamills commented Oct 23, 2025

Uh oh!

bertrand-marquis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wmamills Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bertrand-marquis commented Oct 25, 2025 via email

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wmamills commented Oct 16, 2025 •

edited

Loading

wmamills Nov 2, 2025 •

edited

Loading