[Qemu-devel] [PULL v4 02/29] vhost-user: add new vhost user messages to support virtio config space

Michael S. Tsirkin posted 29 patches 8 years ago
[Qemu-devel] [PULL v4 02/29] vhost-user: add new vhost user messages to support virtio config space
Posted by Michael S. Tsirkin 8 years ago
From: Changpeng Liu <changpeng.liu@intel.com>

Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
used for live migration of vhost user devices, also vhost user devices
can benefit from the messages to get/set virtio config space from/to the
I/O target. For the purpose to support virtio config space change,
VHOST_USER_SLAVE_CONFIG_CHANGE_MSG message is added as the event notifier
in case virtio config space change in the slave I/O target.

Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/interop/vhost-user.txt       |  55 ++++++++++++++++++
 include/hw/virtio/vhost-backend.h |  12 ++++
 include/hw/virtio/vhost.h         |  15 +++++
 hw/virtio/vhost-user.c            | 118 ++++++++++++++++++++++++++++++++++++++
 hw/virtio/vhost.c                 |  32 +++++++++++
 5 files changed, 232 insertions(+)

diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index d49444e..0875ef4 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -119,6 +119,19 @@ Depending on the request type, payload can be:
     - 3: IOTLB invalidate
     - 4: IOTLB access fail
 
+ * Virtio device config space
+   -----------------------------------
+   | offset | size | flags | payload |
+   -----------------------------------
+
+   Offset: a 32-bit offset of virtio device's configuration space
+   Size: a 32-bit configuration space access size in bytes
+   Flags: a 32-bit value:
+    - 0: Vhost master messages used for writeable fields
+    - 1: Vhost master messages used for live migration
+   Payload: Size bytes array holding the contents of the virtio
+       device's configuration space
+
 In QEMU the vhost-user message is implemented with the following struct:
 
 typedef struct VhostUserMsg {
@@ -132,6 +145,7 @@ typedef struct VhostUserMsg {
         VhostUserMemory memory;
         VhostUserLog log;
         struct vhost_iotlb_msg iotlb;
+        VhostUserConfig config;
     };
 } QEMU_PACKED VhostUserMsg;
 
@@ -623,6 +637,32 @@ Master message types
       and expect this message once (per VQ) during device configuration
       (ie. before the master starts the VQ).
 
+ * VHOST_USER_GET_CONFIG
+
+      Id: 24
+      Equivalent ioctl: N/A
+      Master payload: virtio device config space
+      Slave payload: virtio device config space
+
+      Submitted by the vhost-user master to fetch the contents of the virtio
+      device configuration space, vhost-user slave's payload size MUST match
+      master's request, vhost-user slave uses zero length of payload to
+      indicate an error to vhost-user master. The vhost-user master may
+      cache the contents to avoid repeated VHOST_USER_GET_CONFIG calls.
+
+* VHOST_USER_SET_CONFIG
+
+      Id: 25
+      Equivalent ioctl: N/A
+      Master payload: virtio device config space
+      Slave payload: N/A
+
+      Submitted by the vhost-user master when the Guest changes the virtio
+      device configuration space and also can be used for live migration
+      on the destination host. The vhost-user slave must check the flags
+      field, and slaves MUST NOT accept SET_CONFIG for read-only
+      configuration space fields unless the live migration bit is set.
+
 Slave message types
 -------------------
 
@@ -641,6 +681,21 @@ Slave message types
       This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
       has been successfully negotiated.
 
+* VHOST_USER_SLAVE_CONFIG_CHANGE_MSG
+
+     Id: 2
+     Equivalent ioctl: N/A
+     Slave payload: N/A
+     Master payload: N/A
+
+     Vhost-user slave sends such messages to notify that the virtio device's
+     configuration space has changed, for those host devices which can support
+     such feature, host driver can send VHOST_USER_GET_CONFIG message to slave
+     to get the latest content. If VHOST_USER_PROTOCOL_F_REPLY_ACK is
+     negotiated, and slave set the VHOST_USER_NEED_REPLY flag, master must
+     respond with zero when operation is successfully completed, or non-zero
+     otherwise.
+
 VHOST_USER_PROTOCOL_F_REPLY_ACK:
 -------------------------------
 The original vhost-user specification only demands replies for certain
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index a7a5f22..592254f 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -20,6 +20,11 @@ typedef enum VhostBackendType {
     VHOST_BACKEND_TYPE_MAX = 3,
 } VhostBackendType;
 
+typedef enum VhostSetConfigType {
+    VHOST_SET_CONFIG_TYPE_MASTER = 0,
+    VHOST_SET_CONFIG_TYPE_MIGRATION = 1,
+} VhostSetConfigType;
+
 struct vhost_dev;
 struct vhost_log;
 struct vhost_memory;
@@ -84,6 +89,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev,
                                            int enabled);
 typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev,
                                               struct vhost_iotlb_msg *imsg);
+typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *data,
+                                   uint32_t offset, uint32_t size,
+                                   uint32_t flags);
+typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
+                                   uint32_t config_len);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -118,6 +128,8 @@ typedef struct VhostOps {
     vhost_vsock_set_running_op vhost_vsock_set_running;
     vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
     vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg;
+    vhost_get_config_op vhost_get_config;
+    vhost_set_config_op vhost_set_config;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 467dc77..1dc2d73 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -46,6 +46,12 @@ struct vhost_iommu {
     QLIST_ENTRY(vhost_iommu) iommu_next;
 };
 
+typedef struct VhostDevConfigOps {
+    /* Vhost device config space changed callback
+     */
+    int (*vhost_dev_config_notifier)(struct vhost_dev *dev);
+} VhostDevConfigOps;
+
 struct vhost_memory;
 struct vhost_dev {
     VirtIODevice *vdev;
@@ -76,6 +82,7 @@ struct vhost_dev {
     QLIST_ENTRY(vhost_dev) entry;
     QLIST_HEAD(, vhost_iommu) iommu_list;
     IOMMUNotifier n;
+    const VhostDevConfigOps *config_ops;
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
@@ -106,4 +113,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
                           struct vhost_vring_file *file);
 
 int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
+int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
+                         uint32_t config_len);
+int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
+                         uint32_t offset, uint32_t size, uint32_t flags);
+/* notifier callback in case vhost device config space changed
+ */
+void vhost_dev_set_config_notifier(struct vhost_dev *dev,
+                                   const VhostDevConfigOps *ops);
 #endif
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 093675e..8b94688 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -26,6 +26,11 @@
 #define VHOST_MEMORY_MAX_NREGIONS    8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+/*
+ * Maximum size of virtio device config space
+ */
+#define VHOST_USER_MAX_CONFIG_SIZE 256
+
 enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_MQ = 0,
     VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
@@ -65,12 +70,15 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_SLAVE_REQ_FD = 21,
     VHOST_USER_IOTLB_MSG = 22,
     VHOST_USER_SET_VRING_ENDIAN = 23,
+    VHOST_USER_GET_CONFIG = 24,
+    VHOST_USER_SET_CONFIG = 25,
     VHOST_USER_MAX
 } VhostUserRequest;
 
 typedef enum VhostUserSlaveRequest {
     VHOST_USER_SLAVE_NONE = 0,
     VHOST_USER_SLAVE_IOTLB_MSG = 1,
+    VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2,
     VHOST_USER_SLAVE_MAX
 }  VhostUserSlaveRequest;
 
@@ -92,6 +100,18 @@ typedef struct VhostUserLog {
     uint64_t mmap_offset;
 } VhostUserLog;
 
+typedef struct VhostUserConfig {
+    uint32_t offset;
+    uint32_t size;
+    uint32_t flags;
+    uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
+} VhostUserConfig;
+
+static VhostUserConfig c __attribute__ ((unused));
+#define VHOST_USER_CONFIG_HDR_SIZE (sizeof(c.offset) \
+                                   + sizeof(c.size) \
+                                   + sizeof(c.flags))
+
 typedef struct VhostUserMsg {
     VhostUserRequest request;
 
@@ -109,6 +129,7 @@ typedef struct VhostUserMsg {
         VhostUserMemory memory;
         VhostUserLog log;
         struct vhost_iotlb_msg iotlb;
+        VhostUserConfig config;
     } payload;
 } QEMU_PACKED VhostUserMsg;
 
@@ -608,6 +629,21 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
     return 0;
 }
 
+static int vhost_user_slave_handle_config_change(struct vhost_dev *dev)
+{
+    int ret = -1;
+
+    if (!dev->config_ops) {
+        return -1;
+    }
+
+    if (dev->config_ops->vhost_dev_config_notifier) {
+        ret = dev->config_ops->vhost_dev_config_notifier(dev);
+    }
+
+    return ret;
+}
+
 static void slave_read(void *opaque)
 {
     struct vhost_dev *dev = opaque;
@@ -640,6 +676,9 @@ static void slave_read(void *opaque)
     case VHOST_USER_SLAVE_IOTLB_MSG:
         ret = vhost_backend_handle_iotlb_msg(dev, &msg.payload.iotlb);
         break;
+    case VHOST_USER_SLAVE_CONFIG_CHANGE_MSG :
+        ret = vhost_user_slave_handle_config_change(dev);
+        break;
     default:
         error_report("Received unexpected msg type.");
         ret = -EINVAL;
@@ -922,6 +961,83 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
     /* No-op as the receive channel is not dedicated to IOTLB messages. */
 }
 
+static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
+                                 uint32_t config_len)
+{
+    VhostUserMsg msg = {
+        msg.request = VHOST_USER_GET_CONFIG,
+        msg.flags = VHOST_USER_VERSION,
+        msg.size = VHOST_USER_CONFIG_HDR_SIZE + config_len,
+    };
+
+    if (config_len > VHOST_USER_MAX_CONFIG_SIZE) {
+        return -1;
+    }
+
+    msg.payload.config.offset = 0;
+    msg.payload.config.size = config_len;
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        return -1;
+    }
+
+    if (vhost_user_read(dev, &msg) < 0) {
+        return -1;
+    }
+
+    if (msg.request != VHOST_USER_GET_CONFIG) {
+        error_report("Received unexpected msg type. Expected %d received %d",
+                     VHOST_USER_GET_CONFIG, msg.request);
+        return -1;
+    }
+
+    if (msg.size != VHOST_USER_CONFIG_HDR_SIZE + config_len) {
+        error_report("Received bad msg size.");
+        return -1;
+    }
+
+    memcpy(config, msg.payload.config.region, config_len);
+
+    return 0;
+}
+
+static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
+                                 uint32_t offset, uint32_t size, uint32_t flags)
+{
+    uint8_t *p;
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                              VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    VhostUserMsg msg = {
+        msg.request = VHOST_USER_SET_CONFIG,
+        msg.flags = VHOST_USER_VERSION,
+        msg.size = VHOST_USER_CONFIG_HDR_SIZE + size,
+    };
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_REPLY_MASK;
+    }
+
+    if (size > VHOST_USER_MAX_CONFIG_SIZE) {
+        return -1;
+    }
+
+    msg.payload.config.offset = offset,
+    msg.payload.config.size = size,
+    msg.payload.config.flags = flags,
+    p = msg.payload.config.region;
+    memcpy(p, data, size);
+
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        return -1;
+    }
+
+    if (reply_supported) {
+        return process_message_reply(dev, &msg);
+    }
+
+    return 0;
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_init,
@@ -948,4 +1064,6 @@ const VhostOps user_ops = {
         .vhost_net_set_mtu = vhost_user_net_set_mtu,
         .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
         .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
+        .vhost_get_config = vhost_user_get_config,
+        .vhost_set_config = vhost_user_set_config,
 };
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e4290ce..386aef8 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1505,6 +1505,38 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
     }
 }
 
+int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
+                         uint32_t config_len)
+{
+    assert(hdev->vhost_ops);
+
+    if (hdev->vhost_ops->vhost_get_config) {
+        return hdev->vhost_ops->vhost_get_config(hdev, config, config_len);
+    }
+
+    return -1;
+}
+
+int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *data,
+                         uint32_t offset, uint32_t size, uint32_t flags)
+{
+    assert(hdev->vhost_ops);
+
+    if (hdev->vhost_ops->vhost_set_config) {
+        return hdev->vhost_ops->vhost_set_config(hdev, data, offset,
+                                                 size, flags);
+    }
+
+    return -1;
+}
+
+void vhost_dev_set_config_notifier(struct vhost_dev *hdev,
+                                   const VhostDevConfigOps *ops)
+{
+    assert(hdev->vhost_ops);
+    hdev->config_ops = ops;
+}
+
 /* Host notifiers must be enabled at this point. */
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
-- 
MST


Re: [Qemu-devel] [PULL v4 02/29] vhost-user: add new vhost user messages to support virtio config space
Posted by Maxime Coquelin 7 years, 10 months ago
Hi,

On 01/18/2018 09:44 PM, Michael S. Tsirkin wrote:
> From: Changpeng Liu <changpeng.liu@intel.com>
> 
> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
> used for live migration of vhost user devices, also vhost user devices
> can benefit from the messages to get/set virtio config space from/to the
> I/O target. For the purpose to support virtio config space change,
> VHOST_USER_SLAVE_CONFIG_CHANGE_MSG message is added as the event notifier
> in case virtio config space change in the slave I/O target.
> 
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   docs/interop/vhost-user.txt       |  55 ++++++++++++++++++
>   include/hw/virtio/vhost-backend.h |  12 ++++
>   include/hw/virtio/vhost.h         |  15 +++++
>   hw/virtio/vhost-user.c            | 118 ++++++++++++++++++++++++++++++++++++++
>   hw/virtio/vhost.c                 |  32 +++++++++++
>   5 files changed, 232 insertions(+)
> 

As discussed with Changpeng on DPDK ML, we need a vhost-user protocol
feature bit to back these new requests, else QEMU cannot whether the
user backend implement these.

With current and older DPDK version, if an unknown request is received,
the socket connection is closed.

Do we still have time before QEMU v2.12 is out?

Thanks,
Maxime

Re: [Qemu-devel] [PULL v4 02/29] vhost-user: add new vhost user messages to support virtio config space
Posted by Michael S. Tsirkin 7 years, 10 months ago
On Wed, Mar 28, 2018 at 12:03:36PM +0200, Maxime Coquelin wrote:
> Hi,
> 
> On 01/18/2018 09:44 PM, Michael S. Tsirkin wrote:
> > From: Changpeng Liu <changpeng.liu@intel.com>
> > 
> > Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
> > used for live migration of vhost user devices, also vhost user devices
> > can benefit from the messages to get/set virtio config space from/to the
> > I/O target. For the purpose to support virtio config space change,
> > VHOST_USER_SLAVE_CONFIG_CHANGE_MSG message is added as the event notifier
> > in case virtio config space change in the slave I/O target.
> > 
> > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   docs/interop/vhost-user.txt       |  55 ++++++++++++++++++
> >   include/hw/virtio/vhost-backend.h |  12 ++++
> >   include/hw/virtio/vhost.h         |  15 +++++
> >   hw/virtio/vhost-user.c            | 118 ++++++++++++++++++++++++++++++++++++++
> >   hw/virtio/vhost.c                 |  32 +++++++++++
> >   5 files changed, 232 insertions(+)
> > 
> 
> As discussed with Changpeng on DPDK ML, we need a vhost-user protocol
> feature bit to back these new requests, else QEMU cannot whether the
> user backend implement these.
> 
> With current and older DPDK version, if an unknown request is received,
> the socket connection is closed.
> 
> Do we still have time before QEMU v2.12 is out?
> 
> Thanks,
> Maxime

We certainly do, and I agree it's a concern as we get maintain
protocol extensions forever.

It's my bad that I forgot to check that there's a feature bit, but I
think we should act before the release.

There is probably time enough to add it - but if no one
can spare then time it would be better to disable the new messages in
QEMU for now than ship 2.12 without a feature bit.

I wonder what's Changpeng Liu's take on this.

-- 
MST