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

Hi Andrei,

> On 6 Mar 2026, at 09:07, Bertrand Marquis <[email protected]> wrote:
>
> Hi Andrei,
>
>> On 6 Mar 2026, at 02:33, Andrei Homescu <[email protected]> wrote:
>>
>> 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?
>
> Yes definitely and i think this was already pointed out by someone.
>
> It does make sense to clear this up and have this work on 32bit blocks and
> have a number of blocks in the header.
>
> I think at some point we had discussion on the implementation impact as giving
> a number of blocks here but a size in the header will require to do a conversion
> where having this saying number of bits could make implementation a bit simpler.
>
> But looking at this more deeply i think the multiply by 4 and round up saying that
> unused/invalid feature bits have to be 0 is making the wording and logic simpler.
>
> Unless someone has arguments for having here number of bits, i will make this
> coherent and use 32bit blocks 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?
>
> We had several discussions on the subject and there will be an enable added in set_vqueue.
> So the idea will be that as along as a queue is not enabled you can reconfigure it.
> Once the queue is enabled, you have to reset it to re-configure it.
>
>>
>>> +\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?
>
> This is the actual value from the device after the config changes have
> been done.
>
>>
>>> +& 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.
>
> I would say this is empty right now.
> It could optimize things and prevent a GET_CONFIG if we return the data
> but as the data modified might be outside of the area touched by SET_CONFIG
> I think it would make things complex to say that you can get an error and a different
> area of data that was modified (and device does not know what is the cache status
> on the other side).
>
> So I would say empty unless someone has an other idea.
>
>>
>>> +\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)
>
> In theory there is nothing wrong in the device deciding to use the full payload
> it can transfer. This is not really expected but I do not see a valid argument
> to reject that.
>
>> Or is that not misbehaving?
>
> I would say it is a bit weird but still compliant to the spec.

In fact i am having second thoughts on this one.
The request explicitly came with an offset and number of devices so the
answer must honor the request.

So I think we must:
- consider a device answering with something bigger than the request as
misbehaving (non spec compliant)
- allow small answer if there are no devices in part of the area requested
to prevent sending big zero data when possible.

Cheers
Bertrand

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.