Re: [PATCH RFC v2 1/1] virtio-msg: Add virtio-msg, a message based virtio transport layer

Some minor comments below based on the Trusty implementation of
virtio-msg, and potential future issues for implementers:

> +  \item \textbf{Number of Feature Bits}: Indicates how many bits (organized in
> +    32-bit blocks) the device uses for feature negotiation.
I think v1 required this to be a multiple of 32, and would reject
invalid values (might be misremembering).
IMHO it would be simpler for implementers if this was "number of
feature blocks" instead, like in other places.
Is there a specific reason it is "bits" here?

> +The driver \emph{MAY} issue new \msgref{SET_VQUEUE} for virtqueues that have not
> +yet be setup.  If the VIRTIO_F_RING_RESET feature has been negotiated,
> +individual virtqueues can be reset and then optionally re-configured.
This implies that the device should reject SET_VQUEUE for queues that
have already been setup?
Or could a vqueue be configured multiple times?

> +\msgdef{SET_CONFIG}
> +
> +This message is sent by the virtio-msg transport driver and requires a
> +response from the device.
> +
> +\begin{tabular}{|l|l|l|l|}
> +\hline
> +Type & Offset & Size (bytes) & Content \\
> +\hline \hline
> +Request & 0 & 4 & Configuration generation count \\
> +& 4 & 4 & Configuration offset in bytes \\
> +& 8 & 4 & Number of bytes \\
> +& 12 & ... & Configuration data \\
> +\hline
> +Answer & 0 & 4 & New Configuration generation count \\
Is this a copy of the generation count from the request, or the actual
value from the device?

> +& 4 & 4 & Configuration offset in bytes \\
> +& 8 & 4 &  Number of bytes, or 0 if rejected \\
> +& 12 & ... & Configuration data \\
Is this empty in the "if rejected" case, or a copy of the request data?
My interpretation is "empty", but perhaps this should be specified.

> +\busdef{GET_DEVICES}
> +
> +This message is sent by the virtio-msg driver side bus and requires a response
> +from the device side bus.
> +
> +\begin{tabular}{|l|l|l|l|}
> +\hline
> +Type & Offset & Size (bytes) & Content \\
> +\hline \hline
> +Request & 0 & 2 & Offset \\
> +        & 2 & 2 & Number of device numbers requested \\
> +\hline
> +Answer & 0 & 2 & Offset \\
> +       & 2 & 2 & Number of device numbers in the answer \\
How should a driver handle the case where a misbehaving device sends
back more devices than requested? (in other words, if this value is
greater than the device count in the request)
Or is that not misbehaving?
A driver implementation could ask for e.g. 32 devices at a time and
store the resulting bitmap in a uint32_t.
How should it handle getting back more devices than it has space for?

> +The offset and number of device numbers requested \emph{MUST} be multiples of 8.
> +The next offset \emph{MUST} also be a multiple of 8.
Same question here as for feature bits: why not use bytes (defined as
8 bits, or alternatively octets) instead of bits as the unit for
GET_DEVICES?
Then the device implementation wouldn't need to check for and reject
invalid requests.

Andrei Homescu