Re: [PATCH v11 3/7] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Alyssa Ross <hi@alyssa.is>]

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

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Jan 15, 2026 at 01:10:44PM +0100, Alyssa Ross wrote:
>> Albert Esteve <aesteve@redhat.com> writes:
>> 
>> > Add SHMEM_MAP/_UNMAP request to the vhost-user
>> > spec documentation.
>> >
>> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> > Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
>> > ---
>> >  docs/interop/vhost-user.rst | 59 +++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 59 insertions(+)
>> >
>> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
>> > index 17a68a62eb..6c1d66d7d3 100644
>> > --- a/docs/interop/vhost-user.rst
>> > +++ b/docs/interop/vhost-user.rst
>> > @@ -350,6 +350,27 @@ Device state transfer parameters
>> >    In the future, additional phases might be added e.g. to allow
>> >    iterative migration while the device is running.
>> >  
>> > +MMAP request
>> > +^^^^^^^^^^^^
>> > +
>> > ++-------+---------+-----------+------------+-----+-------+
>> > +| shmid | padding | fd_offset | shm_offset | len | flags |
>> > ++-------+---------+-----------+------------+-----+-------+
>> > +
>> > +:shmid: a 8-bit shared memory region identifier
>> > +
>> > +:fd_offset: a 64-bit offset of this area from the start
>> > +            of the supplied file descriptor
>> > +
>> > +:shm_offset: a 64-bit offset from the start of the
>> > +             pointed shared memory region
>> > +
>> > +:len: a 64-bit size of the memory to map
>> > +
>> > +:flags: a 64-bit value:
>> > +  - 0: Pages are mapped read-only
>> > +  - 1: Pages are mapped read-write
>> > +
>> >  C structure
>> >  -----------
>> >  
>> > @@ -375,6 +396,7 @@ In QEMU the vhost-user message is implemented with the following struct:
>> >            VhostUserInflight inflight;
>> >            VhostUserShared object;
>> >            VhostUserTransferDeviceState transfer_state;
>> > +          VhostUserMMap mmap;
>> >        };
>> >    } QEMU_PACKED VhostUserMsg;
>> >  
>> > @@ -1064,6 +1086,7 @@ Protocol features
>> >    #define VHOST_USER_PROTOCOL_F_XEN_MMAP             17
>> >    #define VHOST_USER_PROTOCOL_F_SHARED_OBJECT        18
>> >    #define VHOST_USER_PROTOCOL_F_DEVICE_STATE         19
>> > +  #define VHOST_USER_PROTOCOL_F_SHMEM                20
>> >  
>> >  Front-end message types
>> >  -----------------------
>> > @@ -1872,6 +1895,42 @@ is sent by the front-end.
>> >    when the operation is successful, or non-zero otherwise. Note that if the
>> >    operation fails, no fd is sent to the backend.
>> >  
>> > +``VHOST_USER_BACKEND_SHMEM_MAP``
>> > +  :id: 9
>> > +  :equivalent ioctl: N/A
>> > +  :request payload: fd and ``struct VhostUserMMap``
>> > +  :reply payload: N/A
>> > +
>> > +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
>> > +  successfully negotiated, this message can be submitted by the backends to
>> > +  advertise a new mapping to be made in a given VIRTIO Shared Memory Region.
>> > +  Upon receiving the message, the front-end will mmap the given fd into the
>> > +  VIRTIO Shared Memory Region with the requested ``shmid``.
>> > +  If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and
>> > +  back-end set the ``VHOST_USER_NEED_REPLY`` flag, the front-end
>> > +  must respond with zero when operation is successfully completed,
>> > +  or non-zero otherwise.
>> 
>> Having now tried to implement this, I'm wondering whether replies should
>> be mandatory, even without VHOST_USER_PROTOCOL_F_REPLY_ACK, like they
>> are for some other messages.  Without waiting for a reply, a backend
>> doesn't know when it can tell the driver to start using the mapped
>> memory, so I'm not sure there's ever a case in which a backend would not
>> want to wait for a reply after sending VHOST_USER_BACKEND_SHMEM_MAP,
>> even if it doesn't wait habitually wait for replies for other messages.
>> (crosvm is like this — its backends don't negotiate
>> VHOST_USER_PROTOCOL_F_REPLY_ACK, and their non-standard map/unmap
>> requests had mandatory replies.)
>
> the use-case would be multiple MAP request and a single ack at the end
> to confirm them all.
>
> behaviour of sending ack without VHOST_USER_PROTOCOL_F_REPLY_ACK is
> legacy. let's just stick to the simple rule - if you want
> an ack set VHOST_USER_PROTOCOL_F_REPLY_ACK.

Okay, makes sense to me.  I'll try to fix crosvm's implementation.

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