[virtio-comment] Re: [PATCH v2 3/3] shared memory: Define mmio registers

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

On Fri, Feb 22, 2019 at 11:54:33AM +0000, Dr. David Alan Gilbert (git) wrote:
> diff --git a/content.tex b/content.tex
> index 9838208..c3070bd 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -1726,6 +1726,28 @@ All register values are organized as Little Endian.
>      selected by writing to \field{QueueSel}.
>    }
>    \hline 
> +  \mmioreg{SHMId}{Shared memory id}{0x0ac}{W}{%
> +    Writing to this register selects the shared memory region \ref{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
> +    following operations on \field{SHMLenLow}, \field{SHMLenHigh},
> +    \field{SHMBaseLow} and \field{SHMBaseHigh} apply to.
> +  }
> +  \hline 
> +  \mmiodreg{SHMLenLow}{SHMLenHigh}{Shared memory region 64 bit long length}{0x0b0}{0x0xb4}{R}{%

QueueDescLow/High explicitly documents the low and high 32 bits
relationship of these two registers.  Please do the same of
SHMLenLow/High and SHMBaseLow/High for clarity and consistency.  That
way readers know the purpose of Low and High.

> +    These registers return the length of the shared memory

Adding "in bytes" makes the units explicit.

> +    region, as defined by the device for the region selected by
> +    the \field{SHMId} register.  Reading from a non-existent
> +    region (i.e. where the ID written to \field{SHMId} is unused)
> +    results in a length of -1.

-1 is used to indicate the absence of a region, does 0 have a meaning?

> @@ -1761,6 +1783,9 @@ driver seeing an inconsistent configuration state.
>  
>  The device MUST NOT access virtual queue contents when \field{QueueReady} is zero (0x0).
>  
> +The device MUST NOT respond to SHM Base writes for none-existent

s/none/non/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]