[PATCH 0/3] Document SHMEM vhost-user requests

[PATCH 0/3] Document SHMEM vhost-user requests

[Albert Esteve]

As a continuation of the
"Add SHMEM_MAP/UNMAP requests" patch [1],
I wanted to split vhost-user spec
parts into a separate patch, so that
it could be reviewed and integrated
separately. Having the specs upstreamed
would help others to add support for
these new vhost-user requests on
their systems.

This is such documentation-only patch.

[1] - https://lore.kernel.org/all/20240628145710.1516121-1-aesteve@redhat.com/T/

Albert Esteve (3):
  vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec
  vhost_user.rst: Align VhostUserMsg excerpt members
  vhost_user.rst: Add GET_SHMEM_CONFIG message

 docs/interop/vhost-user.rst | 72 +++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

-- 
2.45.2

[PATCH 1/3] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Albert Esteve]

Add SHMEM_MAP/_UNMAP request to the vhost-user
spec documentation.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d8419fd2f1..d680fea4a3 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -369,6 +369,7 @@ In QEMU the vhost-user message is implemented with the following struct:
           VhostUserConfig config;
           VhostUserVringArea area;
           VhostUserInflight inflight;
+          VhostUserMMap mmap;
       };
   } QEMU_PACKED VhostUserMsg;
 
@@ -1859,6 +1860,36 @@ 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
+
+  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``. A reply is generated indicating whether mapping
+  succeeded.
+
+  Mapping over an already existing map is not allowed and request shall fail.
+  Therefore, the memory range in the request must correspond with a valid,
+  free region of the VIRTIO Shared Memory Region. Also, note that mappings
+  consume resources and that the request can fail when there are no resources
+  available.
+
+``VHOST_USER_BACKEND_SHMEM_UNMAP``
+  :id: 10
+  :equivalent ioctl: N/A
+  :request payload: ``struct VhostUserMMap``
+  :reply payload: N/A
+
+  This message can be submitted by the backends so that the front-end un-mmap
+  a given range (``shm_offset``, ``len``) in the VIRTIO Shared Memory Region
+  with the requested ``shmid``. Note that the given range shall correspond to
+  the entirety of a valid mapped region.
+  A reply is generated indicating whether unmapping succeeded.
+
 .. _reply_ack:
 
 VHOST_USER_PROTOCOL_F_REPLY_ACK
-- 
2.45.2

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

[Stefan Hajnoczi]

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

On Thu, Sep 12, 2024 at 04:44:30PM +0200, Albert Esteve wrote:
> Add SHMEM_MAP/_UNMAP request to the vhost-user
> spec documentation.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d8419fd2f1..d680fea4a3 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -369,6 +369,7 @@ In QEMU the vhost-user message is implemented with the following struct:
>            VhostUserConfig config;
>            VhostUserVringArea area;
>            VhostUserInflight inflight;
> +          VhostUserMMap mmap;
>        };
>    } QEMU_PACKED VhostUserMsg;
>  
> @@ -1859,6 +1860,36 @@ 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``

Where is struct VhostUserMMap defined? Please keep patches
self-contained and in a logical order so they can be reviewed linearly.

Otherwise:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

> +  :reply payload: N/A
> +
> +  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``. A reply is generated indicating whether mapping
> +  succeeded.
> +
> +  Mapping over an already existing map is not allowed and request shall fail.
> +  Therefore, the memory range in the request must correspond with a valid,
> +  free region of the VIRTIO Shared Memory Region. Also, note that mappings
> +  consume resources and that the request can fail when there are no resources
> +  available.
> +
> +``VHOST_USER_BACKEND_SHMEM_UNMAP``
> +  :id: 10
> +  :equivalent ioctl: N/A
> +  :request payload: ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  This message can be submitted by the backends so that the front-end un-mmap
> +  a given range (``shm_offset``, ``len``) in the VIRTIO Shared Memory Region
> +  with the requested ``shmid``. Note that the given range shall correspond to
> +  the entirety of a valid mapped region.
> +  A reply is generated indicating whether unmapping succeeded.
> +
>  .. _reply_ack:
>  
>  VHOST_USER_PROTOCOL_F_REPLY_ACK
> -- 
> 2.45.2
> 

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

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

[Stefan Hajnoczi]

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

On Thu, Sep 12, 2024 at 04:44:30PM +0200, Albert Esteve wrote:
> Add SHMEM_MAP/_UNMAP request to the vhost-user
> spec documentation.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d8419fd2f1..d680fea4a3 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -369,6 +369,7 @@ In QEMU the vhost-user message is implemented with the following struct:
>            VhostUserConfig config;
>            VhostUserVringArea area;
>            VhostUserInflight inflight;
> +          VhostUserMMap mmap;
>        };
>    } QEMU_PACKED VhostUserMsg;
>  
> @@ -1859,6 +1860,36 @@ 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
> +
> +  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``. A reply is generated indicating whether mapping
> +  succeeded.
> +
> +  Mapping over an already existing map is not allowed and request shall fail.
> +  Therefore, the memory range in the request must correspond with a valid,
> +  free region of the VIRTIO Shared Memory Region. Also, note that mappings
> +  consume resources and that the request can fail when there are no resources
> +  available.
> +
> +``VHOST_USER_BACKEND_SHMEM_UNMAP``
> +  :id: 10
> +  :equivalent ioctl: N/A
> +  :request payload: ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  This message can be submitted by the backends so that the front-end un-mmap
> +  a given range (``shm_offset``, ``len``) in the VIRTIO Shared Memory Region
> +  with the requested ``shmid``. Note that the given range shall correspond to
> +  the entirety of a valid mapped region.
> +  A reply is generated indicating whether unmapping succeeded.

There is no mention of VHOST_USER_PROTOCOL_F_SHMEM. I think it needs to
be negotiated in order to use VHOST_USER_BACKEND_SHMEM_MAP and
VHOST_USER_BACKEND_SHMEM_UNMAP.

> +
>  .. _reply_ack:
>  
>  VHOST_USER_PROTOCOL_F_REPLY_ACK
> -- 
> 2.45.2
> 

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

[PATCH 2/3] vhost_user.rst: Align VhostUserMsg excerpt members

[Albert Esteve]

Add missing VhostUserShared and VhostUserTransferDeviceState
members to the VhostUserMsg excerpt in the vhost-user
spec documentation.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d680fea4a3..ec898d2440 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -369,6 +369,8 @@ In QEMU the vhost-user message is implemented with the following struct:
           VhostUserConfig config;
           VhostUserVringArea area;
           VhostUserInflight inflight;
+          VhostUserShared object;
+          VhostUserTransferDeviceState transfer_state;
           VhostUserMMap mmap;
       };
   } QEMU_PACKED VhostUserMsg;
-- 
2.45.2

[PATCH 3/3] vhost_user.rst: Add GET_SHMEM_CONFIG message

[Albert Esteve]

Add GET_SHMEM_CONFIG vhost-user frontend
message to the spec documentation.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst | 39 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index ec898d2440..fb7b27ce94 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -348,6 +348,19 @@ Device state transfer parameters
   In the future, additional phases might be added e.g. to allow
   iterative migration while the device is running.
 
+VIRTIO Shared Memory Region configuration
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
++-------------+---------+------------+----+--------------+
+| num regions | padding | mem size 0 | .. | mem size 255 |
++-------------+---------+------------+----+--------------+
+
+:num regions: a 32-bit number of regions
+
+:padding: 32-bit
+
+:mem size: 64-bit size of VIRTIO Shared Memory Region
+
 C structure
 -----------
 
@@ -372,6 +385,7 @@ In QEMU the vhost-user message is implemented with the following struct:
           VhostUserShared object;
           VhostUserTransferDeviceState transfer_state;
           VhostUserMMap mmap;
+          VhostUserShMemConfig shmem;
       };
   } QEMU_PACKED VhostUserMsg;
 
@@ -1054,6 +1068,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
 -----------------------
@@ -1728,6 +1743,30 @@ Front-end message types
   Using this function requires prior negotiation of the
   ``VHOST_USER_PROTOCOL_F_DEVICE_STATE`` feature.
 
+``VHOST_USER_GET_SHMEM_CONFIG``
+  :id: 44
+  :equivalent ioctl: N/A
+  :request payload: N/A
+  :reply payload: ``struct VhostUserShMemConfig``
+
+  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
+  successfully negotiated, this message can be submitted by the front-end
+  to gather the VIRTIO Shared Memory Region configuration. Back-end will
+  respond with the number of VIRTIO Shared Memory Regions it requires, and
+  each shared memory region size in an array. The shared memory IDs are
+  represented by the index of the array. The information returned shall
+  comply with the following rules:
+
+  * The shared information will remain valid and unchanged for the entire
+    lifetime of the connection.
+
+  * The Shared Memory Region size must be a multiple of the page size
+    supported by mmap(2).
+
+  * The size may be 0 if the region is unused. This can happen when the
+    device does not support an optional feature but does support a feature
+    that uses a higher shmid.
+
 Back-end message types
 ----------------------
 
-- 
2.45.2

Re: [PATCH 3/3] vhost_user.rst: Add GET_SHMEM_CONFIG message

[Alyssa Ross]

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

Albert Esteve <aesteve@redhat.com> writes:

> Add GET_SHMEM_CONFIG vhost-user frontend
> message to the spec documentation.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst | 39 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Reviewed-by: Alyssa Ross <hi@alyssa.is>

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

Re: [PATCH 3/3] vhost_user.rst: Add GET_SHMEM_CONFIG message

[Stefan Hajnoczi]

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

On Thu, Sep 12, 2024 at 04:44:32PM +0200, Albert Esteve wrote:
> Add GET_SHMEM_CONFIG vhost-user frontend
> message to the spec documentation.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst | 39 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index ec898d2440..fb7b27ce94 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -348,6 +348,19 @@ Device state transfer parameters
>    In the future, additional phases might be added e.g. to allow
>    iterative migration while the device is running.
>  
> +VIRTIO Shared Memory Region configuration
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> ++-------------+---------+------------+----+--------------+
> +| num regions | padding | mem size 0 | .. | mem size 255 |
> ++-------------+---------+------------+----+--------------+
> +
> +:num regions: a 32-bit number of regions

What is the purpose of this field? Does it indicate the number of mem
size elements (i.e. the array can be truncated if there are fewer than
256 elements) or the number of non-zero length elements?

> +
> +:padding: 32-bit
> +
> +:mem size: 64-bit size of VIRTIO Shared Memory Region
> +
>  C structure
>  -----------
>  
> @@ -372,6 +385,7 @@ In QEMU the vhost-user message is implemented with the following struct:
>            VhostUserShared object;
>            VhostUserTransferDeviceState transfer_state;
>            VhostUserMMap mmap;
> +          VhostUserShMemConfig shmem;
>        };
>    } QEMU_PACKED VhostUserMsg;
>  
> @@ -1054,6 +1068,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
>  -----------------------
> @@ -1728,6 +1743,30 @@ Front-end message types
>    Using this function requires prior negotiation of the
>    ``VHOST_USER_PROTOCOL_F_DEVICE_STATE`` feature.
>  
> +``VHOST_USER_GET_SHMEM_CONFIG``
> +  :id: 44
> +  :equivalent ioctl: N/A
> +  :request payload: N/A
> +  :reply payload: ``struct VhostUserShMemConfig``
> +
> +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
> +  successfully negotiated, this message can be submitted by the front-end
> +  to gather the VIRTIO Shared Memory Region configuration. Back-end will

"Back-end will" -> "The back-end will"

> +  respond with the number of VIRTIO Shared Memory Regions it requires, and
> +  each shared memory region size in an array. The shared memory IDs are
> +  represented by the index of the array. The information returned shall

"index of the array" -> either "array index" or "index of the array
element".

> +  comply with the following rules:
> +
> +  * The shared information will remain valid and unchanged for the entire
> +    lifetime of the connection.
> +
> +  * The Shared Memory Region size must be a multiple of the page size
> +    supported by mmap(2).
> +
> +  * The size may be 0 if the region is unused. This can happen when the
> +    device does not support an optional feature but does support a feature
> +    that uses a higher shmid.
> +
>  Back-end message types
>  ----------------------
>  
> -- 
> 2.45.2
> 

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

Re: [PATCH 0/3] Document SHMEM vhost-user requests

[Albert Esteve]

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

Link to the implementation:
https://lore.kernel.org/all/20240912145335.129447-1-aesteve@redhat.com/T/#t


On Thu, Sep 12, 2024 at 4:44 PM Albert Esteve <aesteve@redhat.com> wrote:

> As a continuation of the
> "Add SHMEM_MAP/UNMAP requests" patch [1],
> I wanted to split vhost-user spec
> parts into a separate patch, so that
> it could be reviewed and integrated
> separately. Having the specs upstreamed
> would help others to add support for
> these new vhost-user requests on
> their systems.
>
> This is such documentation-only patch.
>
> [1] -
> https://lore.kernel.org/all/20240628145710.1516121-1-aesteve@redhat.com/T/
>
> Albert Esteve (3):
>   vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec
>   vhost_user.rst: Align VhostUserMsg excerpt members
>   vhost_user.rst: Add GET_SHMEM_CONFIG message
>
>  docs/interop/vhost-user.rst | 72 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>
> --
> 2.45.2
>
>

[-- Attachment #2: Type: text/html, Size: 1625 bytes --]

Re: [PATCH 0/3] Document SHMEM vhost-user requests

[Stefan Hajnoczi]

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

On Thu, Sep 12, 2024 at 04:44:29PM +0200, Albert Esteve wrote:
> As a continuation of the
> "Add SHMEM_MAP/UNMAP requests" patch [1],
> I wanted to split vhost-user spec
> parts into a separate patch, so that
> it could be reviewed and integrated
> separately. Having the specs upstreamed
> would help others to add support for
> these new vhost-user requests on
> their systems.
> 
> This is such documentation-only patch.

The spec changes do not mention that exposing VIRTIO Shared Memory
Regions can lead to problems when other vhost-user devices attempt to
access those memory addresses. Please either introduce MEM_READ/WRITE
and explain its use or document that front-ends must forward mappings to
back-ends so that VIRTIO Shared Memory Regions can be accessed.

> 
> [1] - https://lore.kernel.org/all/20240628145710.1516121-1-aesteve@redhat.com/T/
> 
> Albert Esteve (3):
>   vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec
>   vhost_user.rst: Align VhostUserMsg excerpt members
>   vhost_user.rst: Add GET_SHMEM_CONFIG message
> 
>  docs/interop/vhost-user.rst | 72 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> -- 
> 2.45.2
> 

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