[PATCH] VIRTIO-101: ARM's feedback for MMIO chapter

[Pawel Moll <pawel.moll@arm.com>]

* Typos and language mistakes in 4.2.1, 4.2.2 and 4.2.2.2.
* Extra clarifications for QueueNum, QueueReady, ConfigGeneration
  and InterruptACK.
* Decapitalised MUSTs in 4.2.4 Legacy Interface.

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 content.tex | 62 +++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/content.tex b/content.tex
index f2a9e79..2f09419 100644
--- a/content.tex
+++ b/content.tex
@@ -1877,7 +1877,7 @@ following sections.
 
 \subsection{MMIO Device Discovery}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO Device Discovery}
 
-Unlike PCI, MMIO provides no generic device discovery.  For each
+Unlike PCI, MMIO provides no generic device discovery mechanism.  For each
 device, the guest OS will need to know the location of the registers
 and interrupt(s) used.  The suggested binding for systems using
 flattened device trees is shown in this example:
@@ -1893,7 +1893,7 @@ virtio_block@1e000 {
 
 \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}
 
-MMIO virtio devices provides a set of memory mapped control
+MMIO virtio devices provide a set of memory mapped control
 registers followed by a device-specific configuration space,
 described in the table~\ref{tab:Virtio Trasport Options / Virtio Over MMIO / MMIO Device Register Layout}.
 
@@ -1946,7 +1946,7 @@ All register values are organized as Little Endian.
   \hline 
   \mmioreg{DeviceFeatures}{Flags representing features the device supports}{0x010}{R}{%
     Reading from this register returns 32 consecutive flag bits,
-    first bit depending on the last value written to
+    the least significant bit depending on the last value written to
     \field{DeviceFeaturesSel}. Access to this register returns
     bits $\field{DeviceFeaturesSel}*32$ to $(\field{DeviceFeaturesSel}*32)+31$, eg.
     feature bits 0 to 31 if \field{DeviceFeaturesSel} is set to 0 and
@@ -1960,7 +1960,7 @@ All register values are organized as Little Endian.
   }
   \hline 
   \mmioreg{DriverFeatures}{Flags representing device features understood and activated by the driver}{0x020}{W}{%
-    Writing to this register sets 32 consecutive flag bits, first
+    Writing to this register sets 32 consecutive flag bits, the least significant
     bit depending on the last value written to \field{DriverFeaturesSel}.
      Access to this register sets bits $\field{DriverFeaturesSel}*32$
     to $(\field{DriverFeaturesSel}*32)+31$, eg. feature bits 0 to 31 if
@@ -1989,16 +1989,16 @@ All register values are organized as Little Endian.
   }
   \hline 
   \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
-    Queue size is the number of elements in the queue, therefore size
-    of the Descriptor Table and both Available and Used rings.
+    Queue size is the number of elements in the queue, therefore in each
+    of the Descriptor Table, the Available Ring and the Used Ring.
     Writing to this register notifies the device what size of the
     queue the driver will use. This applies to the queue selected by
     writing to \field{QueueSel}.
   }
   \hline 
   \mmioreg{QueueReady}{Virtual queue ready bit}{0x044}{RW}{%
-    Writing one (0x1) to this register notifies the device that the
-    virtual queue is ready to be used. Reading from this register
+    Writing one (0x1) to this register notifies the device that it may
+    execute requests from this virtual queue. Reading from this register
     returns the last value written to it. Both read and write
     accesses apply to the queue selected by writing to \field{QueueSel}.
   }
@@ -2022,8 +2022,9 @@ All register values are organized as Little Endian.
   }
   \hline 
   \mmioreg{InterruptACK}{Interrupt acknowledge}{0x064}{W}{%
-    Writing to this register notifies the device that the interrupt
-    has been handled, as per values for {InterruptStatus}.
+    Writing a value with bits set as defined in \field{InterruptStatus}
+    to this register notifies the device that events causing
+    the interrupt have been handled.
   }
   \hline 
   \mmioreg{Status}{Device status}{0x070}{RW}{%
@@ -2057,7 +2058,10 @@ All register values are organized as Little Endian.
   }
   \hline 
   \mmioreg{ConfigGeneration}{Configuration atomicity value}{0x0fc}{R}{
-    Changes every time the configuration noticeably changes (see \ref {sec:Basic Facilities of a Virtio Device / Device Configuration Space}.
+    Reading from this register returns an arbitrary value.  Subsequent read returns the same
+    value if no part of the device-specific configuration has changed since the previous read
+    or a different value otherwise.
+    See also \ref {sec:Basic Facilities of a Virtio Device / Device Configuration Space}.
   }
   \hline 
   \mmioreg{Config}{Configuration space}{0x100+}{RW}{
@@ -2076,22 +2080,28 @@ The device MUST return value 0x2 in \field{Version}.
 
 The device MUST present each event by setting the corresponding bit in \field{InterruptStatus} from the
 moment it takes place, until the driver acknowledges the interrupt
-by writing a corresponding bit mask to the InterruptACK register.  Bits which
+by writing a corresponding bit mask to the \field{InterruptACK} register.  Bits which
 do not represent events which took place MUST be zero.
 
 Upon reset, the device MUST clear all bits in \field{InterruptStatus} and ready bits in the
 \field{QueueReady} register for all queues in the device.
 
-The device MUST change \field{ConfigGeneration} if there is any risk of a
-device seeing an inconsistent configuration state, but it MAY only change the value
+The device MUST change value returned in \field{ConfigGeneration} if there is any risk of a
+driver seeing an inconsistent configuration state, but it MAY only change the value
 after a configuration read operation.
 
 \drivernormative{\subsubsection}{MMIO Device Register Layout}{Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}
 The driver MUST NOT access memory locations not described in the
-table (or, in case of the configuration space, described in the device specification),
+table \ref{tab:Virtio Trasport Options / Virtio Over MMIO / MMIO Device Register Layout}
+(or, in case of the configuration space, described in the device specification),
 MUST NOT write to the read-only registers (direction R) and
 MUST NOT read from the write-only registers (direction W).
 
+The driver MUST only use 32 bit wide and aligned reads and writes to access the control registers
+described in table \ref{tab:Virtio Trasport Options / Virtio Over MMIO / MMIO Device Register Layout}.
+The driver MAY use other accesses for the device-specific configuration space,
+depending on the device type.
+
 The driver MUST ignore a device with \field{MagicValue} which is not 0x74726976,
 although it MAY report an error.
 
@@ -2110,7 +2120,7 @@ or equal to the value presented by the device in \field{QueueNumMax}.
 
 When \field{QueueReady} is not zero, the driver MUST NOT access
 \field{QueueNum}, \field{QueueDescLow}, \field{QueueDescHigh},
-\field{QueueAvailLow}, \field{QueueAvailHigh}, \field{QueueUsedLow}, \field{QueueUsedHigh}
+\field{QueueAvailLow}, \field{QueueAvailHigh}, \field{QueueUsedLow}, \field{QueueUsedHigh}.
 
 To stop using the queue the driver MUST write zero (0x0) to this
 \field{QueueReady} and MUST read the value back to ensure
@@ -2118,8 +2128,8 @@ synchronization.
 
 The driver MUST ignore undefined bits in \field{InterruptStatus}.
 
-The MUST write the events it handled into \field{InterruptACK} when
-it finishes handling an interrupt.
+The driver MUST write a value with a bit mask describing events it handled into \field{InterruptACK} when
+it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
 \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}
 
@@ -2216,7 +2226,7 @@ nor behaviour:
   \endlastfoot
   \mmioreg{MagicValue}{Magic value}{0x000}{R}{}
   \hline
-  \mmioreg{Version}{Device version number}{0x004}{R}{Legacy device MUST return value 0x1.}
+  \mmioreg{Version}{Device version number}{0x004}{R}{Legacy device must return value 0x1.}
   \hline
   \mmioreg{DeviceID}{Virtio Subsystem Device ID}{0x008}{R}{}
   \hline
@@ -2231,9 +2241,9 @@ nor behaviour:
   \mmioreg{GuestFeaturesSel}{Activated (guest) features word selection}{0x024}{W}{}
   \hline 
   \mmioreg{GuestPageSize}{Guest page size}{0x028}{W}{%
-    The driver MUST write the guest page size in bytes to the
+    The driver must write the guest page size in bytes to the
     register during initialization, before any queues are used.
-    This value MUST be a power of 2 and is used by the device to
+    This value must be a power of 2 and is used by the device to
     calculate the Guest address of the first queue page
     (see QueuePFN).
   }
@@ -2264,7 +2274,7 @@ nor behaviour:
   \hline
   \mmioreg{QueueAlign}{Used Ring alignment in the virtual queue}{0x03c}{W}{%
     Writing to this register notifies the device about alignment
-    boundary of the Used Ring in bytes. This value MUST be a power
+    boundary of the Used Ring in bytes. This value must be a power
     of 2 and applies to the queue selected by writing to \field{QueueSel}.
   }
   \hline
@@ -2274,7 +2284,7 @@ nor behaviour:
     is the index number of a page starting with the queue
     Descriptor Table. Value zero (0x0) means physical address zero
     (0x00000000) and is illegal. When the driver stops using the
-    queue it MUST write zero (0x0) to this register.
+    queue it must write zero (0x0) to this register.
     Reading from this register returns the currently used page
     number of the queue, therefore a value other than zero (0x0)
     means that the queue is in use.
@@ -2293,7 +2303,7 @@ nor behaviour:
     flags.
     Writing non-zero values to this register sets the status flags,
     indicating the OS/driver progress. Writing zero (0x0) to this
-    register triggers a device reset. The device MUST
+    register triggers a device reset. The device must
     set \field{QueuePFN} to zero (0x0) for all queues in the device.
     Also see \ref{sec:General Initialization And Device Operation / Device Initialization}~\nameref{sec:General Initialization And Device Operation / Device Initialization}.
   }
@@ -2303,7 +2313,7 @@ nor behaviour:
 \end{longtable}
 
 The virtual queue page size is defined by writing to \field{GuestPageSize},
-as written by the guest. The driver MUST do this before the
+as written by the guest. The driver must do this before the
 virtual queues are configured.
 
 The virtual queue layout follows
@@ -2324,7 +2334,7 @@ The virtual queue is configured as follows:
 
 \item Allocate and zero the queue pages in contiguous virtual
    memory, aligning the Used Ring to an optimal boundary (usually
-   page size). The driver MUST choose a queue size smaller than or
+   page size). The driver must choose a queue size smaller than or
    equal to \field{QueueNumMax}.
 
 \item Notify the device about the queue size by writing the size to
-- 
1.9.1