Review v1/v0

[bertrand-marquis]

[PDF version of this branch as of 2026-04-23 2:00 UTC ]
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.

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.

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.

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.

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.
• 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, 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.

[bertrand-marquis]

@epilys
I pushed new changes to handle your comments, please review them and tell me if that is ok for you.

[wmamills]

The file contains non-ascii characters again. None of the other files in the spec/project do.
You can find non-ASCII characters with a regular expression of [^\x00-\x7f] (I used VSCode but something similar should work in most editors.

The non-ASCII charaters currently are:

1. smart/paired double quotes. These should be replaces with straight double quotes.
2. em-dash or long dash. When used between numbers this should be replaced with simple dash. When used in paragraphs it COULD be replaced with dash but stylisticly I would prefer commas. I understand they are subtly different but I think the comma is just simpler for the reader. I could be convienced otherwise if there is any existing use of dash in paragraphs in the existing spec

[bertrand-marquis]

I will fix the non ascii issues.
I also did a global review through chatgpt and it came with some findings that i need to fix:

• current generation count wording and handling is breaking the semantics compared to other virtio transport. We should not transmit it as 0 but only make the strict mode reject config set if its value is not the same (accept in non strict mode)
• VIRTIO_F_NOTIF_CONFIG_DATA is PCI only and is not supported in our case (it is PCI only and we have no data field even though the virtqueue id one could be used). This is PCI only so we should just state that it is not supported
• we have some inconsistencies on ownership between bus and transport to fix, in particular for get_device_info. To make this clear we should just provide device num available to the transport which should do the get_device_info and register the devices. This will make the layering cleaner and simpler (and fit with implementation).
• there is some failure handling which is not quite clear, for example we say retry without saying that it should not be retried infinitely but fail after a timeout
• get_vqueue handling for invalid virtqueue ids is not explicit
• we have some terminology that must be removed/reworded (Rationale: for example)
• we have some missing entries in conformance

I will make some patches to handle those and the ascii issue and submit them today or tomorrow

[wmamills]

I made this before I read your message. Feel free to ignore:
wmamills@06fe9f3

[wmamills]

I added a link to the PDF of the current version in the header of this PR. I will update when you do your update Bertrand.

[bertrand-marquis]

Patches that i will push after this point are a bit invasive but are coming out of the experience i made writing a full PoC and out of deep reviews from chatgpt 4.
We might not want all of these but i think the final status is a lot cleaner and clear with a lot of corner cases now handled precisely in the spec.
Please give your view. I am ok to discard them if you think those are to invasive (but keep the fixes for bill finding on ascii).

[wmamills]

@bertrand-marquis you have updated the virtio-msg FFA spec link in the cover letter but I don't see it added to the bibliography as we had discussed

[wmamills]

In the April 23 meeting it was agreed to accept this PR as is and do any additional fix-up needed as additional PRs if needed.