[OASIS Issue Tracker] (VIRTIO-88) VIRTIO Spec feedback #8 From: Arun Subbarao

[OASIS Issues Tracker <workgroup_mailer@lists.oasis-open.org>]

Michael Tsirkin created VIRTIO-88:
-------------------------------------

             Summary: VIRTIO Spec feedback  #8 From: Arun Subbarao
                 Key: VIRTIO-88
                 URL: https://tools.oasis-open.org/issues/browse/VIRTIO-88
             Project: OASIS Virtual I/O Device (VIRTIO) TC
          Issue Type: Bug
            Reporter: Michael Tsirkin


https://lists.oasis-open.org/archives/virtio-comment/201401/msg00058.html

Commenter name: Arun Subbarao <asubbarao@lnxw.com>

1) 3.2.1.4 Notifying The Device:
"We" is confusing and ambiguous. Please use either "device" or "driver" instead of "we" everywhere.

2) 3.2.2 Receiving Used Buffers From The Device:
Is it intentional that the if statement body doesn't disable the interrupts again and the loop continues running with interrupts enabled?

3) 4.1.2 PCI Device Layout
It would be nice to know if there is any guidance on the standard PCI register implementation, such as the PCI Command and Status registers. Even if there is none, the document should mention it, rather than leave the developer guessing if something would break, e.g. if they don't implement the Interrupt Status bit in the Status register.

4) 4.1.2.1 Common configuration structure layout:
I'm confused. Is this the "virtio header" mentioned later in the text? If yes, please use a single term throughout the document to avoid confusion. If not, then what is a "virtio header"? Searching for that term doesn't find a definition.

5) Any alignment requirements for these three fields? Whether or not there are any, please state so explicitly.

6) 4.1.2.2 ISR status structure layout
It would be helpful if this section provided the meaning of each bit in the register.

7) 4.1.3.1.1 Virtio Device Configuration Layout Detection
The double "and" on the first line makes the first two sentences ambiguous. Suggest to rephrase as: "Virtio Device Configuration Layout includes the following structures: virtio configuration header, Notification, ISR Status, device configuration."

8) By the way, this is the only occurrence of the word combination "virtio configuration header" in the document. Makes trying to find what it is impossible.

9) (offset) Any alignment requirements? E.g. is it OK to align structures at an odd offset? Whether there are or aren't any alignment requirements, please state so explicitly.

10) 4.1.3.1.2 Queue Vector Configuration
Some of the information from section 8.4 needs to be moved to here, for example that the device may have an MSI-X table size other than 2048. Otherwise, this reads as though the MSI-X table must always have 2048 entries.

11) Please explicitly describe the device behavior when writing a vector value beyond the MSI-X table size. 

12) 4.1.3.4 Notification of Device Configuration Changes
Please use "PCI configuration space" and "device configuration state" consistently, without abbreviation. For example, from the first sentence it looks like "device configuration state" can be changed, but the first bullet claims it's "configuration space". So, which one? Does "configuration space" mean "PCI configuration space" or is it a synonym for "device configuration state"? Because those are two different things; the driver needs to know what exactly to rescan.

13) Another thing not entirely clear: does the driver need to do anything if it read the "virtio header" (whatever it is) from a BAR rather than PCI config space?




--
This message was sent by Atlassian JIRA
(v6.1.1#6155)