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

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

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