OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Feedback for the first three chapters

 Hello everybody,

here's my feedback for the first three chapters of the Virtio draft 01.

Disclaimer: I am not a native english speaker, so some of my comments
might be wrong, but then you could also take this as an indication that
the related part of the spec might be difficult to understand for
non-english speakers.

General remarks: 

- The first three chapters sometimes uses the pronoun "we" in sentences.
  I think this should be avoided, since it is not always clear who is
  meant with this pronoun: The reader? The driver? The device?

- Some of the generic sections still use the term "PCI" though they
  should not.

I tried to mention the related spots below, but I'd like to suggest to
scan again the whole document for "we" and "PCI" to be sure to get
everything right.

Page 8 / Introduction:

- "Extensible: Virtio PCI devices contain feature bits ..."
  => Remove the "PCI" in above sentence.

Page 10 / Configuration Space:

- "... nor or reads from multiple fields"
  => that's difficult to parse, is this sentence right?

Page 14 / The Virtqueue Available Ring

- "The available ring refers to what descriptor chains the driver is
  offering the device"
  => Somewhat hard to read, maybe better something like this:
  "The available ring refers to the descriptor chains that the driver
  is offering to the device" ?

- "The "idx" field indicates where we would put the next descriptor
  entry in the ring"
  => "The "idx" field indicates where the driver would put the next
  descriptor entry in the ring"

Page 16 / Device Initialization:

- "2. Set the ACKNOWLEDGE status bit: we have noticed the device."
  => "2. The guest OS sets the ACKNOWLEDGE status bit to indicate
  that it has noticed the device."

- "3. Set the DRIVER status bit: we know how to drive the device."
  => "3. The driver sets the DRIVER status bit to indicate that
  it knows how to drive the device"

Page 18 / Notifying the device:

- "... we go ahead and write to the PCI configuration space."
  => "... the driver can go ahead and write to the configuration space."

- "The avail_event field wraps naturally at 65536 as well, iving the
  following algorithm ..."
  => What does "iving" mean? I did not find that in my dictionary.

Page 19:

- "It can then process used ring entries finally enabling interrupts ..."
  => This sentence is hard to parse ... is there missing something
  before "finally"?


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]