[Qemu-devel] [RFC 0/5] vhost-user: Forward virtio device status updates

[Qemu-devel] [RFC 0/5] vhost-user: Forward virtio device status updates

[Maxime Coquelin]

This series introduces a new vhost-user request to notify the
backend with virtio device status updates.

This is done to address the case when the guest driver only
intializes a subset of the virtqueues. For example, it happens
with Windows virtio-net driver, when the virtio-net device has
more queue pairs than vCPUs.

With Virtio 1.0 devices, the driver sets DRIVER_OK after having
intialized all queues, so the backend can use this information
to start the vhost port.

With legacy devices, this is not guaranteed as mentionned in
the spec, so the backend should not rely on DRIVER_OK.
A solution has yet to be found for legacy devices.

Maxime Coquelin (5):
  vhost: send virtio device status update to the backend
  vhost-user: Introduce new request to send virtio device status
  vhost_net: send virtio device status update to the backend
  vhost-user-blk: send virtio status to the backend
  vhost-user-scsi: send virtio status to the backend

 docs/interop/vhost-user.txt       | 14 ++++++++++++++
 hw/block/vhost-user-blk.c         |  1 +
 hw/net/vhost_net.c                | 10 ++++++++++
 hw/net/virtio-net.c               |  7 ++++++-
 hw/scsi/vhost-user-scsi.c         |  2 ++
 hw/virtio/vhost-user.c            | 35 +++++++++++++++++++++++++++++++++++
 hw/virtio/vhost.c                 | 11 +++++++++++
 include/hw/virtio/vhost-backend.h |  3 +++
 include/hw/virtio/vhost.h         |  3 +++
 include/net/vhost_net.h           |  2 ++
 10 files changed, 87 insertions(+), 1 deletion(-)

-- 
2.14.3

[Qemu-devel] [RFC 1/5] vhost: send virtio device status update to the backend

[Maxime Coquelin]

This patch adds a function to notify the vhost backend with
virtio device status updates.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost.c                 | 11 +++++++++++
 include/hw/virtio/vhost-backend.h |  3 +++
 include/hw/virtio/vhost.h         |  3 +++
 3 files changed, 17 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 338e4395b7..95981e5a32 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1546,6 +1546,17 @@ void vhost_dev_set_config_notifier(struct vhost_dev *hdev,
     hdev->config_ops = ops;
 }
 
+int vhost_dev_set_virtio_status(struct vhost_dev *hdev, uint8_t status)
+{
+    const VhostOps *vhost_ops = hdev->vhost_ops;
+
+    if (vhost_ops && vhost_ops->vhost_set_virtio_status) {
+        return vhost_ops->vhost_set_virtio_status(hdev, status);
+    }
+
+    return 0;
+}
+
 /* Host notifiers must be enabled at this point. */
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 592254f40d..45f6b8f6c5 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -94,6 +94,8 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *data,
                                    uint32_t flags);
 typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
                                    uint32_t config_len);
+typedef int (*vhost_set_virtio_status_op)(struct vhost_dev *dev,
+                                          uint8_t status);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -130,6 +132,7 @@ typedef struct VhostOps {
     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;
+    vhost_set_virtio_status_op vhost_set_virtio_status;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 1dc2d73d76..fe055b2bd5 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -121,4 +121,7 @@ int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data,
  */
 void vhost_dev_set_config_notifier(struct vhost_dev *dev,
                                    const VhostDevConfigOps *ops);
+
+int vhost_dev_set_virtio_status(struct vhost_dev *hdev, uint8_t status);
+
 #endif
-- 
2.14.3

[Qemu-devel] [RFC 2/5] vhost-user: Introduce new request to send virtio device status

[Maxime Coquelin]

This patch implements the .vhost_set_virtio_status() backend
callback for user backend by intooducing a new vhost-user
VHOST_USER_SET_VIRTIO_STATUS request.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 docs/interop/vhost-user.txt | 14 ++++++++++++++
 hw/virtio/vhost-user.c      | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index 9fcf48d611..daa452bd36 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -368,6 +368,7 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_MTU            4
 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
+#define VHOST_USER_PROTOCOL_F_VIRTIO_STATUS  7
 
 Master message types
 --------------------
@@ -663,6 +664,19 @@ Master message types
       field, and slaves MUST NOT accept SET_CONFIG for read-only
       configuration space fields unless the live migration bit is set.
 
+* VHOST_USER_SET_VIRTIO_STATUS
+
+      Id: 26
+      Equivalent ioctl: N/A
+      Master payload: u64
+      Slave payload: N/A
+
+      Sent by the vhost-user master to notify of virtio device status change.
+      The payload is a u64 representing the virtio device status as defined in
+      the virtio specification.
+      The request should be sent only when VHOST_USER_PROTOCOL_F_VIRTIO_STATUS
+      protocol feature has been negotiated.
+
 Slave message types
 -------------------
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 6eb97980ad..519646799b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -39,6 +39,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_NET_MTU = 4,
     VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
     VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
+    VHOST_USER_PROTOCOL_F_VIRTIO_STATUS = 7,
 
     VHOST_USER_PROTOCOL_F_MAX
 };
@@ -72,6 +73,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_ENDIAN = 23,
     VHOST_USER_GET_CONFIG = 24,
     VHOST_USER_SET_CONFIG = 25,
+    VHOST_USER_SET_VIRTIO_STATUS = 26,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -1054,6 +1056,38 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
     return 0;
 }
 
+static int vhost_user_set_virtio_status(struct vhost_dev *dev, uint8_t status)
+{
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                              VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    VhostUserMsg msg = {
+        .hdr.request = VHOST_USER_SET_VIRTIO_STATUS,
+        .hdr.flags = VHOST_USER_VERSION,
+        .hdr.size = sizeof(msg.payload.u64),
+        .payload.u64 = status,
+    };
+
+    if (!virtio_has_feature(dev->protocol_features,
+                            VHOST_USER_PROTOCOL_F_VIRTIO_STATUS)) {
+        return 0;
+    }
+
+    if (reply_supported) {
+        msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
+    }
+
+    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,
@@ -1082,4 +1116,5 @@ const VhostOps user_ops = {
         .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,
+        .vhost_set_virtio_status = vhost_user_set_virtio_status,
 };
-- 
2.14.3

[Qemu-devel] [RFC 3/5] vhost_net: send virtio device status update to the backend

[Maxime Coquelin]

This patch adds ans uses a new function to send virtio device
status updates to the backend.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/net/vhost_net.c      | 10 ++++++++++
 hw/net/virtio-net.c     |  7 ++++++-
 include/net/vhost_net.h |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index e037db63a3..75e2165163 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -450,6 +450,11 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
     return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
 }
 
+int vhost_net_set_virtio_status(struct vhost_net *net, uint8_t status)
+{
+    return vhost_dev_set_virtio_status(&net->dev, status);
+}
+
 #else
 uint64_t vhost_net_get_max_queues(VHostNetState *net)
 {
@@ -521,4 +526,9 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
 {
     return 0;
 }
+
+int vhost_net_set_virtio_status(struct vhost_net *net, uint8_t status)
+{
+    return 0;
+}
 #endif
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 38674b08aa..06c431ea28 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -136,7 +136,7 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
 
     if ((virtio_net_started(n, status) && !nc->peer->link_down) ==
         !!n->vhost_started) {
-        return;
+        goto out;
     }
     if (!n->vhost_started) {
         int r, i;
@@ -175,11 +175,16 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
             error_report("unable to start vhost net: %d: "
                          "falling back on userspace virtio", -r);
             n->vhost_started = 0;
+
+            return;
         }
     } else {
         vhost_net_stop(vdev, n->nic->ncs, queues);
         n->vhost_started = 0;
     }
+
+out:
+    vhost_net_set_virtio_status(get_vhost_net(nc->peer), status);
 }
 
 static int virtio_net_set_vnet_endian_one(VirtIODevice *vdev,
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index afc1499eb9..bf083e2d2e 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -37,4 +37,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
 
 int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
 
+int vhost_net_set_virtio_status(struct vhost_net *net, uint8_t status);
+
 #endif
-- 
2.14.3

[Qemu-devel] [RFC 4/5] vhost-user-blk: send virtio status to the backend

[Maxime Coquelin]

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/block/vhost-user-blk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index b53b4c9c57..a88b6f13a4 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -190,6 +190,7 @@ static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
         vhost_user_blk_stop(vdev);
     }
 
+    vhost_dev_set_virtio_status(&s->dev, status);
 }
 
 static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
-- 
2.14.3

[Qemu-devel] [RFC 5/5] vhost-user-scsi: send virtio status to the backend

[Maxime Coquelin]

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/scsi/vhost-user-scsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 9389ed48e0..da36a3b7f5 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -58,6 +58,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
     } else {
         vhost_scsi_common_stop(vsc);
     }
+
+    vhost_dev_set_virtio_status(&vsc->dev, status);
 }
 
 static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
-- 
2.14.3

Re: [Qemu-devel] [RFC 2/5] vhost-user: Introduce new request to send virtio device status

[Michael S. Tsirkin]

On Fri, Feb 16, 2018 at 06:29:07PM +0100, Maxime Coquelin wrote:
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index 9fcf48d611..daa452bd36 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -368,6 +368,7 @@ Protocol features
>  #define VHOST_USER_PROTOCOL_F_MTU            4
>  #define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
>  #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
> +#define VHOST_USER_PROTOCOL_F_VIRTIO_STATUS  7
>  
>  Master message types
>  --------------------
> @@ -663,6 +664,19 @@ Master message types
>        field, and slaves MUST NOT accept SET_CONFIG for read-only
>        configuration space fields unless the live migration bit is set.
>  
> +* VHOST_USER_SET_VIRTIO_STATUS
> +
> +      Id: 26
> +      Equivalent ioctl: N/A
> +      Master payload: u64
> +      Slave payload: N/A
> +
> +      Sent by the vhost-user master to notify of virtio device status change.
> +      The payload is a u64 representing the virtio device status as defined in
> +      the virtio specification.
> +      The request should be sent only when VHOST_USER_PROTOCOL_F_VIRTIO_STATUS
> +      protocol feature has been negotiated.
> +
>  Slave message types
>  -------------------
>  

So for now backend was only activated after DRIVER_OK. Does this message
mean that we must send updates such as _DRIVER as well?

Further, this is kind of one-way, but there are several cases where device
modifies the status.  One is NEEDS_RESET. Another is clearing
of FEATURES_OK.


-- 
MST

Re: [Qemu-devel] [RFC 2/5] vhost-user: Introduce new request to send virtio device status

[Maxime Coquelin]

On 02/27/2018 04:01 PM, Michael S. Tsirkin wrote:
> On Fri, Feb 16, 2018 at 06:29:07PM +0100, Maxime Coquelin wrote:
>> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
>> index 9fcf48d611..daa452bd36 100644
>> --- a/docs/interop/vhost-user.txt
>> +++ b/docs/interop/vhost-user.txt
>> @@ -368,6 +368,7 @@ Protocol features
>>   #define VHOST_USER_PROTOCOL_F_MTU            4
>>   #define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
>>   #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
>> +#define VHOST_USER_PROTOCOL_F_VIRTIO_STATUS  7
>>   
>>   Master message types
>>   --------------------
>> @@ -663,6 +664,19 @@ Master message types
>>         field, and slaves MUST NOT accept SET_CONFIG for read-only
>>         configuration space fields unless the live migration bit is set.
>>   
>> +* VHOST_USER_SET_VIRTIO_STATUS
>> +
>> +      Id: 26
>> +      Equivalent ioctl: N/A
>> +      Master payload: u64
>> +      Slave payload: N/A
>> +
>> +      Sent by the vhost-user master to notify of virtio device status change.
>> +      The payload is a u64 representing the virtio device status as defined in
>> +      the virtio specification.
>> +      The request should be sent only when VHOST_USER_PROTOCOL_F_VIRTIO_STATUS
>> +      protocol feature has been negotiated.
>> +
>>   Slave message types
>>   -------------------
>>   
> 
> So for now backend was only activated after DRIVER_OK. Does this message
> mean that we must send updates such as _DRIVER as well?

Yes, even if I don't see a use for _ACKNOWLEDGE and _DRIVER today.

> Further, this is kind of one-way, but there are several cases where device
> modifies the status.  One is NEEDS_RESET. Another is clearing
> of FEATURES_OK.

Do you mean we should also notify the backend in case of NEEDS_RESET, or
clearing of FEATURES_OK?

Or you mean we should provide a way for the backend to update the device
status, e.g. by having a slave-initiated VHOST_USER_SET_VIRTIO_STATUS
request?

Thanks,
Maxime