[OASIS Issue Tracker] (VIRTIO-94) VIRTIO Spec feedback #2 From: Thomas Huth

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

     [ https://tools.oasis-open.org/issues/browse/VIRTIO-94?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Tsirkin updated VIRTIO-94:
----------------------------------

    Affects Version/s: virtio 1.0 csprd01
        Fix Version/s: virtio 1.0 csprd02

> VIRTIO Spec feedback  #2 From: Thomas Huth
> ------------------------------------------
>
>                 Key: VIRTIO-94
>                 URL: https://tools.oasis-open.org/issues/browse/VIRTIO-94
>             Project: OASIS Virtual I/O Device (VIRTIO) TC
>          Issue Type: Bug
>    Affects Versions: virtio 1.0 csprd01
>            Reporter: Michael Tsirkin
>             Fix For: virtio 1.0 csprd02
>
>
> Date: Fri, 10 Jan 2014 13:49:49 +0100
> Link to Mail: https://lists.oasis-open.org/archives/virtio-comment/201401/msg00001.html
> Commenter name: 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.



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