[RFC PATCH v2 2/5] vhost_user: Add frontend command for shmem config

Albert Esteve posted 5 patches 2 months, 2 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>
There is a newer version of this series
[RFC PATCH v2 2/5] vhost_user: Add frontend command for shmem config
Posted by Albert Esteve 2 months, 2 weeks ago
The frontend can use this command to retrieve
VIRTIO Shared Memory Regions configuration from
the backend. The response contains the number of
shared memory regions, their size, and shmid.

This is useful when the frontend is unaware of
specific backend type and configuration,
for example, in the `vhost-user-device` case.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst       | 31 +++++++++++++++++++++++
 hw/virtio/vhost-user.c            | 42 +++++++++++++++++++++++++++++++
 include/hw/virtio/vhost-backend.h |  6 +++++
 include/hw/virtio/vhost-user.h    |  1 +
 4 files changed, 80 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d52ba719d5..51f01d1d84 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 7 |
++-------------+---------+------------+----+------------+
+
+:num regions: a 32-bit number of regions
+
+:padding: 32-bit
+
+:mem size: 64-bit size of VIRTIO Shared Memory Region
+
 C structure
 -----------
 
@@ -369,6 +382,10 @@ 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;
+          VhostUserShMemConfig shmem;
       };
   } QEMU_PACKED VhostUserMsg;
 
@@ -1051,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
 -----------------------
@@ -1725,6 +1743,19 @@ 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.
+
 Back-end message types
 ----------------------
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 7ee8a472c6..57406dc8b4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -104,6 +104,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_SHARED_OBJECT = 41,
     VHOST_USER_SET_DEVICE_STATE_FD = 42,
     VHOST_USER_CHECK_DEVICE_STATE = 43,
+    VHOST_USER_GET_SHMEM_CONFIG = 44,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -138,6 +139,12 @@ typedef struct VhostUserMemRegMsg {
     VhostUserMemoryRegion region;
 } VhostUserMemRegMsg;
 
+typedef struct VhostUserShMemConfig {
+    uint32_t nregions;
+    uint32_t padding;
+    uint64_t memory_sizes[VHOST_MEMORY_BASELINE_NREGIONS];
+} VhostUserShMemConfig;
+
 typedef struct VhostUserLog {
     uint64_t mmap_size;
     uint64_t mmap_offset;
@@ -245,6 +252,7 @@ typedef union {
         VhostUserShared object;
         VhostUserTransferDeviceState transfer_state;
         VhostUserMMap mmap;
+        VhostUserShMemConfig shmem;
 } VhostUserPayload;
 
 typedef struct VhostUserMsg {
@@ -3136,6 +3144,39 @@ static int vhost_user_check_device_state(struct vhost_dev *dev, Error **errp)
     return 0;
 }
 
+static int vhost_user_get_shmem_config(struct vhost_dev *dev,
+                                       int *nregions,
+                                       uint64_t *memory_sizes,
+                                       Error **errp)
+{
+    int ret;
+    VhostUserMsg msg = {
+        .hdr.request = VHOST_USER_GET_SHMEM_CONFIG,
+        .hdr.flags = VHOST_USER_VERSION,
+    };
+
+    if (!virtio_has_feature(dev->protocol_features,
+                            VHOST_USER_PROTOCOL_F_SHMEM)) {
+        return 0;
+    }
+
+    ret = vhost_user_write(dev, &msg, NULL, 0);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = vhost_user_read(dev, &msg);
+    if (ret < 0) {
+        return ret;
+    }
+
+    *nregions = msg.payload.shmem.nregions;
+    memcpy(memory_sizes,
+           &msg.payload.shmem.memory_sizes,
+           sizeof(uint64_t) * VHOST_MEMORY_BASELINE_NREGIONS);
+    return 0;
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_backend_init,
@@ -3174,4 +3215,5 @@ const VhostOps user_ops = {
         .vhost_supports_device_state = vhost_user_supports_device_state,
         .vhost_set_device_state_fd = vhost_user_set_device_state_fd,
         .vhost_check_device_state = vhost_user_check_device_state,
+        .vhost_get_shmem_config = vhost_user_get_shmem_config,
 };
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 70c2e8ffee..f9c2955420 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -159,6 +159,11 @@ typedef int (*vhost_set_device_state_fd_op)(struct vhost_dev *dev,
                                             int *reply_fd,
                                             Error **errp);
 typedef int (*vhost_check_device_state_op)(struct vhost_dev *dev, Error **errp);
+typedef int (*vhost_get_shmem_config_op)(struct vhost_dev *dev,
+                                         int *nregions,
+                                         uint64_t *memory_sizes,
+                                         Error **errp);
+
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -214,6 +219,7 @@ typedef struct VhostOps {
     vhost_supports_device_state_op vhost_supports_device_state;
     vhost_set_device_state_fd_op vhost_set_device_state_fd;
     vhost_check_device_state_op vhost_check_device_state;
+    vhost_get_shmem_config_op vhost_get_shmem_config;
 } VhostOps;
 
 int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index d7c09ffd34..e1b587a908 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -32,6 +32,7 @@ enum VhostUserProtocolFeature {
     /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
     VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
     VHOST_USER_PROTOCOL_F_DEVICE_STATE = 19,
+    VHOST_USER_PROTOCOL_F_SHMEM = 20,
     VHOST_USER_PROTOCOL_F_MAX
 };
 
-- 
2.45.2
Re: [RFC PATCH v2 2/5] vhost_user: Add frontend command for shmem config
Posted by Stefan Hajnoczi 2 months, 1 week ago
On Fri, Jun 28, 2024 at 04:57:07PM +0200, Albert Esteve wrote:
> The frontend can use this command to retrieve
> VIRTIO Shared Memory Regions configuration from
> the backend. The response contains the number of
> shared memory regions, their size, and shmid.
> 
> This is useful when the frontend is unaware of
> specific backend type and configuration,
> for example, in the `vhost-user-device` case.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst       | 31 +++++++++++++++++++++++
>  hw/virtio/vhost-user.c            | 42 +++++++++++++++++++++++++++++++
>  include/hw/virtio/vhost-backend.h |  6 +++++
>  include/hw/virtio/vhost-user.h    |  1 +
>  4 files changed, 80 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d52ba719d5..51f01d1d84 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 7 |
> ++-------------+---------+------------+----+------------+
> +
> +:num regions: a 32-bit number of regions
> +
> +:padding: 32-bit
> +
> +:mem size: 64-bit size of VIRTIO Shared Memory Region
> +
>  C structure
>  -----------
>  
> @@ -369,6 +382,10 @@ 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;
> +          VhostUserShMemConfig shmem;
>        };
>    } QEMU_PACKED VhostUserMsg;
>  
> @@ -1051,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
>  -----------------------
> @@ -1725,6 +1743,19 @@ 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.

Please add:
- 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.
Re: [RFC PATCH v2 2/5] vhost_user: Add frontend command for shmem config
Posted by Stefan Hajnoczi 2 months, 1 week ago
On Fri, Jun 28, 2024 at 04:57:07PM +0200, Albert Esteve wrote:
> The frontend can use this command to retrieve
> VIRTIO Shared Memory Regions configuration from
> the backend. The response contains the number of
> shared memory regions, their size, and shmid.
> 
> This is useful when the frontend is unaware of
> specific backend type and configuration,
> for example, in the `vhost-user-device` case.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst       | 31 +++++++++++++++++++++++
>  hw/virtio/vhost-user.c            | 42 +++++++++++++++++++++++++++++++
>  include/hw/virtio/vhost-backend.h |  6 +++++
>  include/hw/virtio/vhost-user.h    |  1 +
>  4 files changed, 80 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d52ba719d5..51f01d1d84 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 7 |
> ++-------------+---------+------------+----+------------+

8 regions may not be enough. The max according to the VIRTIO spec is
256 because virtio-pci uses an 8-bit cap.id field for the shmid. I think
the maximum number should be 256 here.

(I haven't checked the QEMU vhost-user code to see whether it's
reasonable to hardcode to 256 or some logic if needed to dynamically
size the buffer depending on the "num regions" field.)

> +
> +:num regions: a 32-bit number of regions
> +
> +:padding: 32-bit
> +
> +:mem size: 64-bit size of VIRTIO Shared Memory Region
> +
>  C structure
>  -----------
>  
> @@ -369,6 +382,10 @@ 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;

Why are these added by this patch? Please add them in the same patch
where they are introduced.

> +          VhostUserShMemConfig shmem;
>        };
>    } QEMU_PACKED VhostUserMsg;
>  
> @@ -1051,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
>  -----------------------
> @@ -1725,6 +1743,19 @@ 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.

Is the information returned by SHMEM_CONFIG valid and unchanged for the
entire lifetime of the vhost-user connection?

I think the answer is yes because the enumeration that virtio-pci and
virtio-mmio transports support is basically a one-time operation at
driver startup and it is static (Shared Memory Regions do not appear or
go away at runtime). Please be explicit how VHOST_USER_GET_SHMEM_CONFIG
is intended to be used.

> +
>  Back-end message types
>  ----------------------
>  
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 7ee8a472c6..57406dc8b4 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -104,6 +104,7 @@ typedef enum VhostUserRequest {
>      VHOST_USER_GET_SHARED_OBJECT = 41,
>      VHOST_USER_SET_DEVICE_STATE_FD = 42,
>      VHOST_USER_CHECK_DEVICE_STATE = 43,
> +    VHOST_USER_GET_SHMEM_CONFIG = 44,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
> @@ -138,6 +139,12 @@ typedef struct VhostUserMemRegMsg {
>      VhostUserMemoryRegion region;
>  } VhostUserMemRegMsg;
>  
> +typedef struct VhostUserShMemConfig {
> +    uint32_t nregions;
> +    uint32_t padding;
> +    uint64_t memory_sizes[VHOST_MEMORY_BASELINE_NREGIONS];
> +} VhostUserShMemConfig;
> +
>  typedef struct VhostUserLog {
>      uint64_t mmap_size;
>      uint64_t mmap_offset;
> @@ -245,6 +252,7 @@ typedef union {
>          VhostUserShared object;
>          VhostUserTransferDeviceState transfer_state;
>          VhostUserMMap mmap;
> +        VhostUserShMemConfig shmem;
>  } VhostUserPayload;
>  
>  typedef struct VhostUserMsg {
> @@ -3136,6 +3144,39 @@ static int vhost_user_check_device_state(struct vhost_dev *dev, Error **errp)
>      return 0;
>  }
>  
> +static int vhost_user_get_shmem_config(struct vhost_dev *dev,
> +                                       int *nregions,
> +                                       uint64_t *memory_sizes,
> +                                       Error **errp)
> +{
> +    int ret;
> +    VhostUserMsg msg = {
> +        .hdr.request = VHOST_USER_GET_SHMEM_CONFIG,
> +        .hdr.flags = VHOST_USER_VERSION,
> +    };
> +
> +    if (!virtio_has_feature(dev->protocol_features,
> +                            VHOST_USER_PROTOCOL_F_SHMEM)) {
> +        return 0;
> +    }
> +
> +    ret = vhost_user_write(dev, &msg, NULL, 0);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = vhost_user_read(dev, &msg);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    *nregions = msg.payload.shmem.nregions;

Missing input validation from the untrusted vhost-user backend. nregions
may be out of range.

> +    memcpy(memory_sizes,
> +           &msg.payload.shmem.memory_sizes,
> +           sizeof(uint64_t) * VHOST_MEMORY_BASELINE_NREGIONS);
> +    return 0;
> +}
> +
>  const VhostOps user_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_USER,
>          .vhost_backend_init = vhost_user_backend_init,
> @@ -3174,4 +3215,5 @@ const VhostOps user_ops = {
>          .vhost_supports_device_state = vhost_user_supports_device_state,
>          .vhost_set_device_state_fd = vhost_user_set_device_state_fd,
>          .vhost_check_device_state = vhost_user_check_device_state,
> +        .vhost_get_shmem_config = vhost_user_get_shmem_config,
>  };
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 70c2e8ffee..f9c2955420 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -159,6 +159,11 @@ typedef int (*vhost_set_device_state_fd_op)(struct vhost_dev *dev,
>                                              int *reply_fd,
>                                              Error **errp);
>  typedef int (*vhost_check_device_state_op)(struct vhost_dev *dev, Error **errp);
> +typedef int (*vhost_get_shmem_config_op)(struct vhost_dev *dev,
> +                                         int *nregions,
> +                                         uint64_t *memory_sizes,
> +                                         Error **errp);
> +
>  
>  typedef struct VhostOps {
>      VhostBackendType backend_type;
> @@ -214,6 +219,7 @@ typedef struct VhostOps {
>      vhost_supports_device_state_op vhost_supports_device_state;
>      vhost_set_device_state_fd_op vhost_set_device_state_fd;
>      vhost_check_device_state_op vhost_check_device_state;
> +    vhost_get_shmem_config_op vhost_get_shmem_config;
>  } VhostOps;
>  
>  int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index d7c09ffd34..e1b587a908 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -32,6 +32,7 @@ enum VhostUserProtocolFeature {
>      /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
>      VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
>      VHOST_USER_PROTOCOL_F_DEVICE_STATE = 19,
> +    VHOST_USER_PROTOCOL_F_SHMEM = 20,
>      VHOST_USER_PROTOCOL_F_MAX
>  };
>  
> -- 
> 2.45.2
> 
Re: [RFC PATCH v2 2/5] vhost_user: Add frontend command for shmem config
Posted by Albert Esteve 1 week, 5 days ago
On Thu, Jul 11, 2024 at 10:10 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:

> On Fri, Jun 28, 2024 at 04:57:07PM +0200, Albert Esteve wrote:
> > The frontend can use this command to retrieve
> > VIRTIO Shared Memory Regions configuration from
> > the backend. The response contains the number of
> > shared memory regions, their size, and shmid.
> >
> > This is useful when the frontend is unaware of
> > specific backend type and configuration,
> > for example, in the `vhost-user-device` case.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >  docs/interop/vhost-user.rst       | 31 +++++++++++++++++++++++
> >  hw/virtio/vhost-user.c            | 42 +++++++++++++++++++++++++++++++
> >  include/hw/virtio/vhost-backend.h |  6 +++++
> >  include/hw/virtio/vhost-user.h    |  1 +
> >  4 files changed, 80 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index d52ba719d5..51f01d1d84 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 7 |
> > ++-------------+---------+------------+----+------------+
>
> 8 regions may not be enough. The max according to the VIRTIO spec is
> 256 because virtio-pci uses an 8-bit cap.id field for the shmid. I think
> the maximum number should be 256 here.
>

Ok, I'll set it to 255, as it starts at 0.


>
> (I haven't checked the QEMU vhost-user code to see whether it's
> reasonable to hardcode to 256 or some logic if needed to dynamically
> size the buffer depending on the "num regions" field.)
>
> > +
> > +:num regions: a 32-bit number of regions
> > +
> > +:padding: 32-bit
> > +
> > +:mem size: 64-bit size of VIRTIO Shared Memory Region
> > +
> >  C structure
> >  -----------
> >
> > @@ -369,6 +382,10 @@ 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;
>
> Why are these added by this patch? Please add them in the same patch
> where they are introduced.


For object and transfer_state that's a ship that already sailed.
In order to still align the excerpt with the actual code, I will
split them into their own commit to avoid confusion.


> > +          VhostUserShMemConfig shmem;
> >        };
> >    } QEMU_PACKED VhostUserMsg;
> >
> > @@ -1051,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
> >  -----------------------
> > @@ -1725,6 +1743,19 @@ 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.
>
> Is the information returned by SHMEM_CONFIG valid and unchanged for the
> entire lifetime of the vhost-user connection?
>
> I think the answer is yes because the enumeration that virtio-pci and
> virtio-mmio transports support is basically a one-time operation at
> driver startup and it is static (Shared Memory Regions do not appear or
> go away at runtime). Please be explicit how VHOST_USER_GET_SHMEM_CONFIG
> is intended to be used.
>

Yes, I will be explicit.


>
> > +
> >  Back-end message types
> >  ----------------------
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 7ee8a472c6..57406dc8b4 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -104,6 +104,7 @@ typedef enum VhostUserRequest {
> >      VHOST_USER_GET_SHARED_OBJECT = 41,
> >      VHOST_USER_SET_DEVICE_STATE_FD = 42,
> >      VHOST_USER_CHECK_DEVICE_STATE = 43,
> > +    VHOST_USER_GET_SHMEM_CONFIG = 44,
> >      VHOST_USER_MAX
> >  } VhostUserRequest;
> >
> > @@ -138,6 +139,12 @@ typedef struct VhostUserMemRegMsg {
> >      VhostUserMemoryRegion region;
> >  } VhostUserMemRegMsg;
> >
> > +typedef struct VhostUserShMemConfig {
> > +    uint32_t nregions;
> > +    uint32_t padding;
> > +    uint64_t memory_sizes[VHOST_MEMORY_BASELINE_NREGIONS];
> > +} VhostUserShMemConfig;
> > +
> >  typedef struct VhostUserLog {
> >      uint64_t mmap_size;
> >      uint64_t mmap_offset;
> > @@ -245,6 +252,7 @@ typedef union {
> >          VhostUserShared object;
> >          VhostUserTransferDeviceState transfer_state;
> >          VhostUserMMap mmap;
> > +        VhostUserShMemConfig shmem;
> >  } VhostUserPayload;
> >
> >  typedef struct VhostUserMsg {
> > @@ -3136,6 +3144,39 @@ static int vhost_user_check_device_state(struct
> vhost_dev *dev, Error **errp)
> >      return 0;
> >  }
> >
> > +static int vhost_user_get_shmem_config(struct vhost_dev *dev,
> > +                                       int *nregions,
> > +                                       uint64_t *memory_sizes,
> > +                                       Error **errp)
> > +{
> > +    int ret;
> > +    VhostUserMsg msg = {
> > +        .hdr.request = VHOST_USER_GET_SHMEM_CONFIG,
> > +        .hdr.flags = VHOST_USER_VERSION,
> > +    };
> > +
> > +    if (!virtio_has_feature(dev->protocol_features,
> > +                            VHOST_USER_PROTOCOL_F_SHMEM)) {
> > +        return 0;
> > +    }
> > +
> > +    ret = vhost_user_write(dev, &msg, NULL, 0);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    ret = vhost_user_read(dev, &msg);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    *nregions = msg.payload.shmem.nregions;
>
> Missing input validation from the untrusted vhost-user backend. nregions
> may be out of range.
>
> > +    memcpy(memory_sizes,
> > +           &msg.payload.shmem.memory_sizes,
> > +           sizeof(uint64_t) * VHOST_MEMORY_BASELINE_NREGIONS);
> > +    return 0;
> > +}
> > +
> >  const VhostOps user_ops = {
> >          .backend_type = VHOST_BACKEND_TYPE_USER,
> >          .vhost_backend_init = vhost_user_backend_init,
> > @@ -3174,4 +3215,5 @@ const VhostOps user_ops = {
> >          .vhost_supports_device_state = vhost_user_supports_device_state,
> >          .vhost_set_device_state_fd = vhost_user_set_device_state_fd,
> >          .vhost_check_device_state = vhost_user_check_device_state,
> > +        .vhost_get_shmem_config = vhost_user_get_shmem_config,
> >  };
> > diff --git a/include/hw/virtio/vhost-backend.h
> b/include/hw/virtio/vhost-backend.h
> > index 70c2e8ffee..f9c2955420 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -159,6 +159,11 @@ typedef int (*vhost_set_device_state_fd_op)(struct
> vhost_dev *dev,
> >                                              int *reply_fd,
> >                                              Error **errp);
> >  typedef int (*vhost_check_device_state_op)(struct vhost_dev *dev, Error
> **errp);
> > +typedef int (*vhost_get_shmem_config_op)(struct vhost_dev *dev,
> > +                                         int *nregions,
> > +                                         uint64_t *memory_sizes,
> > +                                         Error **errp);
> > +
> >
> >  typedef struct VhostOps {
> >      VhostBackendType backend_type;
> > @@ -214,6 +219,7 @@ typedef struct VhostOps {
> >      vhost_supports_device_state_op vhost_supports_device_state;
> >      vhost_set_device_state_fd_op vhost_set_device_state_fd;
> >      vhost_check_device_state_op vhost_check_device_state;
> > +    vhost_get_shmem_config_op vhost_get_shmem_config;
> >  } VhostOps;
> >
> >  int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
> > diff --git a/include/hw/virtio/vhost-user.h
> b/include/hw/virtio/vhost-user.h
> > index d7c09ffd34..e1b587a908 100644
> > --- a/include/hw/virtio/vhost-user.h
> > +++ b/include/hw/virtio/vhost-user.h
> > @@ -32,6 +32,7 @@ enum VhostUserProtocolFeature {
> >      /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
> >      VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
> >      VHOST_USER_PROTOCOL_F_DEVICE_STATE = 19,
> > +    VHOST_USER_PROTOCOL_F_SHMEM = 20,
> >      VHOST_USER_PROTOCOL_F_MAX
> >  };
> >
> > --
> > 2.45.2
> >
>