Hi All,
This email comes from the github PR but I am going to respond here.
On 2/23/26 4:45 AM, bertrand-marquis wrote:
> Hi Everyone,
>
> This pull request include a number of fixes or changes to handle the review
> comment we got on virtio mailing list.
>
> Some points are not handled because they need a decision between us first
> (following summary was generated by chatgpt):
>
> Ordering / Out‑of‑Order Feature Discussion
>
> * Parav Pandit proposes relaxing the strict request/response ordering
> because it is too constraining for high‑performance data paths. He
> suggests that the bus may need to deliver responses out of
> order, and the spec should support that for performance.
> * Demi Marie Obenour raises reset and out‑of‑order race risks if ordering
> is relaxed. She suggests a generation/stream number or bus‑level reset
> semantics to avoid stale or mis‑matched responses.
> * Michael S. Tsirkin recommends keeping v1 simple and introducing
> performance features via optional feature bits. This aligns with making
> relaxed ordering a negotiated capability rather than a
> baseline requirement.
>
I don't think transport level operations (excepting Virtqueue notifications
) have a big impact on performance. Demi seemed to understand our POV once
she saw the option for events to go out of band.
At least part of the time Parav was looking at virtio-msg NOT as a
transport replacement (using existing virtqueue's in shared memory) and
instead was looking at it as full virtio over a message channel (like
virtio over tcp). Michael seemed to agree that was out of scope for our
work and would need to be its own thing. However it is not clear if or
when Parav has changed his thinking.
> Configuration Generation Optional Feature Discussion
>
> * Peter Hilber suggests making the SET_CONFIG generation count optional
> or dropping it, because Linux drivers assume config writes are not
> rejected and do not retry. His rationale is avoiding
> incompatibility with existing driver behavior.
TL;DR include the generation counter in the message but eliminate the
"rejection" of the SET_CONFIG and make it the devices job to coalesce the
new value into the config space.
virtio-msg has a race condition in config write that virtio-pci and
virtio-mmio does not have due to its trap and emulate nature. In
virtio-msg a write can race with a device side change to the same word(s).
I agree with Peter that we should not expect the individual Linux drivers
to respond to this. My initial thought was to do this in the driver side
transport but Peter made we realize this is dumb. The transport will not
have any extra info. It is better to push this race resolution back to the
device. Giving the device the generation count allows the device to know
if the driver was modifying the old value or the new value.
I fully expect many device side implementations to ignore the generation
count and just to take the new value but providing the generation count
allows it to be more sophisticated if it wants to be.
>
> VQ Enabled Flag Discussion
>
> * Peter Hilber suggests adding an explicit “queue enabled” indication in
> SET_VQUEUE (and also reporting it in GET_VQUEUE) so drivers can avoid
> an extra GET_VQUEUE round‑trip just to confirm
> enablement. This is aimed at reducing overhead in the initialization flow.
>
Ack
> Device Number Size Discussion
>
> * Parav Pandit argues the 16‑bit device number is too small; he suggests
> 32‑bit to cover >64k devices and PCI BDF domain usage.
> * Demi Marie Obenour argues device identifiers should be 64‑bit or even
> 128‑bit to ensure long‑term uniqueness.
> * Peter Hilber highlights that reuse of device numbers after removal can
> cause races, and suggests adding a reuse‑limiting policy. He ties this
> to the broader question of expanding the device number
> space.
>
32 or 64 is fine. 128 seems overkill yes? One PCI bus has a limit of 2^64
devices.
> EVENT_CONFIG Data Mandatory Discussion
>
> * Demi Marie Obenour proposes making EVENT_CONFIG always include
> configuration data (MUST), to avoid a round trip. If data is mandatory,
> she notes that offset/length‑only signaling becomes
> unnecessary.
NAK. This will prevent simple out of band notification for the EVENT_AVAIL.
Demi seemed to want OOB notifications so this seems in contradiction.
> * There is a noted contradiction with earlier guidance (Peter Hilber) and
> the current spec change ( I already updated the spec to require offset/
> length even when data is omitted,
Even this will prevent OOB. The existing mechanisum allow this to be
negociated. Any bus that uses OOB can NAK these feature bits to prevent use.
> keeping data optional as I think this
> makes sense and i am not quite sure we can mandate the data without
> breaking compatibility with other transports or existing devices). This
> needs a clear decision.
>
> ---------------------------------------------------------------------------
>
>
> You can view, comment on, or merge this pull request online at:
>
> https://github.com/Linaro/virtio-msg-spec/pull/31 <https://github.com/
> Linaro/virtio-msg-spec/pull/31>
>
>
> Commit Summary
>
> * 52ffe64 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/52ffe641810d26860fbdbb45e7437c8df9360f6a> virtio-msg: clarify
> device number vs device ID (Peter Hilber)
> * 3a70c3a <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/3a70c3ad6a277cf08931a4efb86b3a41fd29eb7b> virtio-msg: fix minor
> grammar in device topology (Peter Hilber)
> * d677fc8 <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> d677fc8c1717320416460d911143fa243b8f4eb8> virtio-msg: use “transport
> revision” consistently (Peter Hilber)
> * c0020f1 <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> c0020f12b985b25c66e3afcf348c1b7aaa81fb78> virtio-msg: use active voice
> for reserved header bits (Peter Hilber)
> * 7eafee2 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/7eafee2d67d847862f6ff7a9a2899fb5b081b217> virtio-msg: drop
> duplicate GET_DEVICE_INFO requirement (Peter Hilber)
> * 623334c <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/623334cf0ae1c2289d23c3b47bdd7f2cfa801ed7> virtio-msg: clarify
> removal handling (Peter Hilber)
> * 4c75e07 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/4c75e07c29f4aa2dbdccfcde8c89d7d9953f28f6> virtio-msg: allow
> zero devices per bus instance (Peter Hilber)
> * 2bbad99 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/2bbad994805f216755ba6abfe3a1b844e41f30aa> virtio-msg: add note
> on config vs virtqueue data (Demi Marie Obenour)
> * 094111b <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/094111b9d9e019c9cc4e50d31bf1c27ea5c8d3dc> virtio-msg: rename
> READY to ADDED in bus device events (Parav Pandit)
> * 1e1e817 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/1e1e81778404d596f1216385a8c8945cf6dbdbdc> virtio-msg: clarify
> transport feature negotiation (Peter Hilber)
> * f50fe3c <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> f50fe3c985bd6061449f7ab50b45a045bbe15aa7> virtio-msg: clarify
> generation count reset semantics (Peter Hilber)
> * f462169 <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> f462169090b0907b59b14ed0b6ee952c67934df1> virtio-msg: clarify
> notification wording (Peter Hilber)
> * 2d8c995 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/2d8c9952035cf72331de5b22ea13aacd9b2c5a29> virtio-msg: require
> EVENT_USED unless polling (Peter Hilber)
> * 4b8c483 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/4b8c483c79709a509a5961309435a941a8e17110> virtio-msg: drop
> RESET_VQUEUE-before-reprogramming (Peter Hilber)
> * b783fc5 <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> b783fc534d8973167770515d12a5fd22703ba331> virtio-msg: align admin vq
> field widths (Peter Hilber)
> * 818142e <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/818142ed4c8612645a46ed211a0a172267684d35> virtio-msg: proposal:
> clarify echoed fields (Peter Hilber)
> * 6ba8c70 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/6ba8c708335f9068c0045f74714f9258b8faf488> virtio-msg: require
> clearing FEATURES_OK (Peter Hilber)
> * 21aeb49 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/21aeb4982be32a80ea2a4852cef9dae8f552786d> virtio-msg: widen
> GET_SHM fields (Peter Hilber)
> * 3e007fd <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/3e007fdbc75eadb931c2cb4eaf4954255f51e419> virtio-msg: clarify
> EVENT_CONFIG ranges (Peter Hilber)
> * f109559 <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> f1095590057baff76976d371a1ecd95357ca3f35> virtio-msg: streamline
> DEVICE_NEEDS_RESET text (Peter Hilber)
> * 835ef71 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/835ef718e432d7b8df08cf0e25f59f89a0a0bdc6> virtio-msg: scope
> msg_id validation (Peter Hilber)
> * 07b0d08 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/07b0d08d12c9038d59f07a627dbf408ec7424435> virtio-msg: drop
> FEATURES_OK sentence (Peter Hilber)
> * ba844e9 <https://github.com/Linaro/virtio-msg-spec/pull/31/commits/
> ba844e93dd1a685fae0d0c04eb216784962a91e1> virtio-msg: note bus-level
> errors (Demi Marie Obenour)
> * 3c685b6 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/3c685b6d5facca7ff01cce29869893c077153209> virtio-msg: reframe
> ordering responses (Demi Marie Obenour)
> * 8a529c0 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/8a529c0a6bd261ef0bdda92b1715bfb5e5e8e034> virtio-msg: clarify
> virtqueue data path (Demi Marie Obenour)
> * 0df8574 <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/0df8574759c0685e802a99c34ef09e0d7cef3af2> virtio-msg: allow
> out-of-order events (Demi Marie Obenour)
> * 773970f <https://github.com/Linaro/virtio-msg-spec/pull/31/
> commits/773970f939facdba7ba7f2ff70838d0133e6cd58> virtio-msg: use
> physical address terminology (Demi Marie Obenour)
>
>
> File Changes
>
> (1 file <https://github.com/Linaro/virtio-msg-spec/pull/31/files>)
>
> * *M* transport-msg.tex <https://github.com/Linaro/virtio-msg-spec/
> pull/31/files#diff-
> ae43224efc56fefe20a4d5de6f7cfb5ccd94950be1246b6ece5274c6c994e167> (232)
>
>
> Patch Links:
>
> * https://github.com/Linaro/virtio-msg-spec/pull/31.patch <https://
> github.com/Linaro/virtio-msg-spec/pull/31.patch>
> * https://github.com/Linaro/virtio-msg-spec/pull/31.diff <https://
> github.com/Linaro/virtio-msg-spec/pull/31.diff>
>
> —
> Reply to this email directly, view it on GitHub <https://github.com/Linaro/
> virtio-msg-spec/pull/31>, or unsubscribe <https://github.com/notifications/
> unsubscribe-auth/
> AIBXWC5SZFXJ2MVGMCE7AYL4NLD5PAVCNFSM6AAAAACV4PIWDWVHI2DSMVQWIX3LMV43ASLTON2WKOZTHE3TOMRUG42TEMI>.
> You are receiving this because your review was requested.Message ID:
> <Linaro/virtio-msg-spec/pull/31(a)github.com>
>
Hi Everyone,
I pushed a first pull request to try to handle some of the comments we got on virtio mailing list:
https://github.com/Linaro/virtio-msg-spec/pull/31
In my comment i list some of the points for which we will need to have a discussion on thursday to decide how and if we handle them.
Please check and add any comments you might have.
Cheers
Bertrand
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.
