Feedback for chapter 4

[Thomas Huth <thuth@linux.vnet.ibm.com>]

Here's my feedback for Virtio draft 01, chapter 4:

Page 20 /  PCI Device Layout:

- "To configure the device, use I/O and/or memory regions and/or PCI configuration
  space of the PCI device."
  => That sounds a little bit sparse/confusing. Maybe rather something like:
  "To configure the device, it is possible to use the PCI configuration space
  and/or to access the configuration data via an I/O and/or MMIO base-address
  register."

Page 21:

- The "device_feature_select" and "driver_feature_select" paragraphs are lacking
  some punctuation marks inbetween.

Page 23 / Virtio Device Configuration Layout Detection:

- "This structure can optionally followed by extra data"
  => "This structure can optionally be followed by extra data"

Page 27 / MMIO Device Discovery:

- The device tree snippet is obviously an example. That's ok, but I think the
  spec should explicitely say so (and maybe add some generic words about the
  required properties before the example, too).

Chapter 4.3.2.*:

- In this chapter, the C-structs are marked with "__attribute__ ((packed));"
  which is just a GNU-C extension, as far as I know. In the other chapters,
  the structs are not marked with this string. So for consistency, I'd remove
  them here, too (and maybe state somewhere at the beginning of the spec
  that structs are considered to be without compiler padding inbetween)

Page 33:

- Some typos:
  neccessarily => necessarily
  issueing => issuing

Page 34 / Virtqueue Layout:

- "...  with padded added ..."
  => "... with padding added ..."

Page 34 / Handling Device Features:

- The text says "Feature bits are in little-endian byte order", but the
  "struct virtio_feature_desc" is described with "be32 features" ...
  that's confusing -- are the feature bits now little or big endian?

Page 36:

- "Bit numbers start at the left"
  => I'd make this sentence more explicit, e.g.:
  "Bit numbers start at the left, i.e. the most significant bit in the
  first byte is assigned the bit number 0."

Page 36 / Notification via Adapter I/O Interrupts:

- "The guest-provided summary indicator is also set."
  => What value is set in the summary indicator byte? 0x01? 0x80? 0xff?
  It maybe does not matter, since any non-zero value could be used, but
  it might help to avoid confusion if you specify the exact value here.

Page 37 / Early printk for Virtio Consoles

- Is this early print really part of virtio-ccw? If yes, I think you
  should also describe the register usage here.


 Regards,
  Thomas