summaryrefslogtreecommitdiff
path: root/linux-core/mga_warp.c
AgeCommit message (Expand)Author
2006-12-01Track linux-core symlinks in git.Michel Dänzer
23' href='#n23'>23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150
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