Open
Conversation
Peter noted inconsistent use of “device number” vs “device ID”. I did not find other ambiguous occurrences in transport-msg, so I added a sentence explicitly distinguishing the bus-assigned device number from the virtio device ID returned by GET_DEVICE_INFO. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter noted minor editorial issues (“Implement”/“respond”). Update the Device bullet to “Implements” and “responds”. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter noted inconsistent naming. Update the virtio-msg revision wording and paragraph title to consistently use “transport revision”. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter requested active-voice wording for the reserved header bits requirement. Rephrase to make the bus implementation the subject. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter noted this requirement was already stated in the Initialization Flow driver requirements. Remove the duplicate from the Device Information driver requirements to avoid repetition. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter questioned “reset” on DEVICE_BUS_STATE_REMOVED. Rephrase the intended-usage text to avoid implying a device reset after removal and to focus on driver-side teardown. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter noted the spec implied “one or more” devices. Update the device numbering section to allow zero devices, matching the earlier bus-layer statement and hotplug use cases. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Demi suggested a non-normative note discouraging configuration space for bulk data when a virtqueue can be used. Add a short explanatory note in Device Configuration. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Parav requested symmetric naming for device hotplug events. Rename the READY state to ADDED throughout the EVENT_DEVICE definition and related text; values remain unchanged. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter asked whether transport feature bits are negotiated. Clarify that the bus presents only the common subset and add a bus requirement to advertise only mutually supported bits. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Peter asked whether devices may reset the configuration generation count. Allow reset (including on device reset) and require drivers to avoid assuming monotonic or persistent values. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Rephrase driver and device notification text to focus on which messages are used, avoiding timing-specific requirements for EVENT_AVAIL, EVENT_CONFIG, and EVENT_USED. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Strengthen EVENT_USED requirements to MUST, with explicit exceptions for polling or other agreed-upon completion mechanisms. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Remove the runtime requirement to issue RESET_VQUEUE before reprogramming a queue, since it does not avoid races. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Use le32 for admin_vq_start and admin_vq_count to match max_virtqueues. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
PROPOSAL: Clarify that echoed request parameters in responses are informational and do not change the ordering-based correlation model. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Make device requirements explicit: if features are unsupported, the device MUST clear FEATURES_OK in the status response. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Use le64 for shared memory length and address, and add a reserved padding field aligned with existing message layouts. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Require EVENT_CONFIG to carry the affected offset/length even when data is omitted, and update related guidance. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Clarify that DEVICE_NEEDS_RESET is reported via status updates without repeating EVENT_CONFIG-specific requirements. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Require the bus to discard invalid bus msg_ids and leave transport msg_id validation to the transport endpoints. NOTE: I am unsure whether the explicit "MUST NOT validate transport msg_id" line is necessary; please comment if a softer requirement is preferred. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Remove the redundant sentence in SET_DRIVER_FEATURES device requirements. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Add a non-normative description of bus-level rejection for forbidden requests and how it can be surfaced to the driver. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Rephrase the Message Ordering requirements so the device, not the bus, MUST send exactly one response per request. The bus ordering guarantee remains unchanged. Signed-off-by: Bertrand Marquis <Bertrand.Marquis@arm.com> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Add an explicit note that virtqueue descriptors and buffer data are exchanged via shared memory or other DMA-accessible memory. Transport messages remain control-plane only. Signed-off-by: Bertrand Marquis <Bertrand.Marquis@arm.com> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Clarify that event notifications are independent and may be delivered out of order with respect to other events, while request/response ordering remains unchanged. Signed-off-by: Bertrand Marquis <Bertrand.Marquis@arm.com> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Align virtqueue and shared-memory address wording with mmio/pci by consistently referring to physical addresses throughout the virtio-msg transport text and message definitions. Signed-off-by: Bertrand Marquis <Bertrand.Marquis@arm.com> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
The configuration generation count is now allowed to reset, but the normative text still required unique values for EVENT_CONFIG across the entire device lifetime. Scope the uniqueness requirement to the period since the last generation-count reset to remove the contradiction while preserving the intended sequencing guarantee. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
EVENT_CONFIG is used for both configuration changes and status updates, but the offset/length fields only describe configuration ranges. Define that status-only updates set offset and length to zero, removing the ambiguity while keeping range reporting for configuration updates. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
The Device Notifications section only allows EVENT_USED omission when polling or another completion mechanism is used. Drop the extra "driver explicitly disabled" clause from the message definition to match the normative text and keep the rules consistent. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
The common header section implied the bus must sanitize reserved bits when forwarding transport messages. Rephrase the rule to apply to the message originator and receivers, preserving forward-compatibility requirements without conflicting with bus forwarding behavior. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Mandatory messages such as GET_VQUEUE/SET_VQUEUE are 40-byte payloads plus the 8-byte header. Update the revision table and the normative minimum to 48 bytes so the minimum advertised size can carry all mandatory messages. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
The bus is not responsible for validating device numbers. Drop the "MUST NOT forward" clause from device-number assignment and clarify that routing failures may be dropped or surfaced as a transport error. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
GET_DEVICE_INFO does not report shared memory segments. Update the Device Information description to mention admin virtqueues only and remove the shared-memory reference. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
The revision table shows the protocol-defined maximum message size, while the text recommends advertising no more than 264 bytes. Add a sentence to distinguish the protocol maximum from the recommended advertised maximum. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Update the label path for "Bus Specific Messages" to match the Bus Messages section hierarchy, keeping labels consistent with the document structure. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
…our) Clarify that buses may signal runtime notifications out-of-band but MUST still relay or synthesize the corresponding EVENT_* transport message so the transport always receives events uniformly. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Proposal: extend GET_DEVICE_INFO with a 16-byte device_uuid field so a device can provide a unique identifier when available. If UUID reporting is not supported, or the device has no identifier, the device returns the nil UUID (all-zero value). Drivers interpret the nil UUID as "no UUID provided". Because this increases the fixed GET_DEVICE_INFO response size, raise the transport minimum advertised message size from 48 to 52 bytes and update the revision size table accordingly. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Proposal: update the common virtio-msg header to use `le32 msg_size` and `le64 dev_num`, and move `dev_num` to the end of the header so the new 16-byte layout keeps fields naturally aligned. This updates header semantics and size-related limits accordingly, and propagates 64-bit device numbers into bus message payloads that carry device numbers (GET_DEVICES and EVENT_DEVICE). GET_DEVICES response field order is adjusted to keep 64-bit fields aligned. Related review discussion in mail threads: - Parav Pandit (device-number width and msg_size concerns) - Demi Marie Obenour (larger unique device identifiers) - Peter Hilber (device-number reuse/race concerns) Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Add a bus-level recommendation in Device Number Assignment to prevent immediate reuse of a device number after device removal. This is specified as SHOULD (not MUST) to avoid hard policy requirements while still reducing race conditions with delayed messages associated with the removed device. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
… Hilber) Extend GET_VQUEUE and SET_VQUEUE to carry queue state and update control via a flags field. For GET_VQUEUE, replace the reserved field with flags bit0 to report the queue enabled state (bits[31:1] reserved zero). For SET_VQUEUE, replace reserved0 with flags: - bits[1:0]: queue state operation (disable, enable, keep, reserved) - bits[5:2]: per-field ignore bits for size/desc_addr/driver_addr/device_addr - bits[31:6]: reserved zero Define flags==0 as legacy behavior (disable + apply all queue fields). Add normative rules for valid flag values, atomic apply-or-reject processing, and driver-side verification through GET_VQUEUE when confirmation is required (SET_VQUEUE response has no payload). Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Collaborator
Author
|
I add various patches for some specific proposals/requests in the review that we must discuss:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
order, and the spec should support that for performance.
baseline requirement.
Configuration Generation Optional Feature Discussion
incompatibility with existing driver behavior.
VQ Enabled Flag Discussion
enablement. This is aimed at reducing overhead in the initialization flow.
Device Number Size Discussion
space.
EVENT_CONFIG Data Mandatory Discussion
unnecessary.