summaryrefslogtreecommitdiff
path: root/feedback/1.txt
diff options
context:
space:
mode:
Diffstat (limited to 'feedback/1.txt')
-rw-r--r--feedback/1.txt150
1 files changed, 0 insertions, 150 deletions
diff --git a/feedback/1.txt b/feedback/1.txt
deleted file mode 100644
index 65645c8..0000000
--- a/feedback/1.txt
+++ /dev/null
@@ -1,150 +0,0 @@
-Document: virtio-v1.0-csprd01
-Number: 1
-Date: Fri, 10 Jan 2014 11:01:44 +0100
-Link to Mail: https://lists.oasis-open.org/archives/virtio-comment/201401/msg00000.html
-Commenter name: Thomas Huth <thuth@linux.vnet.ibm.com>
-Decision: Agreed at meeting 2014-01-28.
-
-- 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"?
-
-Proposal:
-
-diff --git a/content.tex b/content.tex
-index 803615d..8850c1a 100644
---- a/content.tex
-+++ b/content.tex
-@@ -145,7 +145,7 @@ Interface' in the section title.
-
- Configuration space is generally used for rarely-changing or
- initialization-time parameters. Drivers MUST NOT assume reads from
--fields greater than 32 bits wide are atomic, nor or reads from
-+fields greater than 32 bits wide are atomic, nor reads from
- multiple fields.
-
- Each transport provides a generation count for the configuration
-@@ -418,11 +418,11 @@ the device MUST ignore the write-only flag (flags\&VRING_DESC_F_WRITE) in the de
- };
- \end{lstlisting}
-
--The available ring refers to what descriptor chains the driver is offering the
-+The driver uses the available ring to offer buffers to the
- device: each ring entry refers to the head of a descriptor chain. It is only
- written by the driver and read by the device.
-
--The “idx” field indicates where we would put the next descriptor
-+The “idx” field indicates where the driver would put the next descriptor
- entry in the ring (modulo the queue size). This starts at 0, and increases.
-
- If the VIRTIO_RING_F_INDIRECT_DESC feature bit is not negotiated, the
-@@ -515,9 +515,9 @@ The driver MUST follow this sequence to initialize a device:
- \begin{enumerate}
- \item Reset the device.
-
--\item Set the ACKNOWLEDGE status bit: we have noticed the device.
-+\item Set the ACKNOWLEDGE status bit: the guest OS has notice the device.
-
--\item Set the DRIVER status bit: we know how to drive the device.
-+\item Set the DRIVER status bit: the guest OS knows how to drive the device.
-
- \item Read device feature bits, and write the subset of feature bits
- understood by the OS and driver to the device.
-@@ -686,7 +686,7 @@ we use a memory barrier here before reading the flags or the
- avail_event field.
-
- If the VIRTIO_F_RING_EVENT_IDX feature is not negotiated, and if the
--VRING_USED_F_NOTIFY flag is not set, we go ahead and notify the
-+VRING_USED_F_NOTIFY flag is not set, the driver SHOULD notify the
- device.
-
- If the VIRTIO_F_RING_EVENT_IDX feature is negotiated, we read the
-@@ -694,7 +694,7 @@ avail_event field in the available ring structure. If the
- available index crossed_the avail_event field value since the
- last notification, we go ahead and write to the PCI configuration
- space. The avail_event field wraps naturally at 65536 as well,
--iving the following algorithm for calculating whether a device needs
-+giving the following algorithm for calculating whether a device needs
- notification:
-
- \begin{lstlisting}
-@@ -705,7 +705,7 @@ notification:
-
- Once the device has used a buffer (read from or written to it, or
- parts of both, depending on the nature of the virtqueue and the
--device), it sends an interrupt, following an algorithm very
-+device), it SHOULD send an interrupt, following an algorithm very
- similar to the algorithm used for the driver to send the device a
- buffer:
-
-@@ -732,11 +732,11 @@ buffer:
- \end{enumerate}
- \end{enumerate}
-
--For each ring, the driver should then disable interrupts by writing
-+For each ring, the driver MAY then disable interrupts by writing
- VRING_AVAIL_F_NO_INTERRUPT flag in avail structure, if required.
--It can then process used ring entries finally enabling interrupts
--by clearing the VRING_AVAIL_F_NO_INTERRUPT flag or updating the
--EVENT_IDX field in the available structure. The driver should then
-+Once it has processed the ring entries, it SHOULD re-enable
-+interrupts by clearing the VRING_AVAIL_F_NO_INTERRUPT flag or updating the
-+EVENT_IDX field in the available structure. The driver SHOULD then
- execute a memory barrier, and then recheck the ring empty
- condition. This is necessary to handle the case where after the
- last check and before enabling interrupts, an interrupt has been