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

2018-02-27 Thread 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_MTU4
  #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

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

2018-02-27 Thread 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_MTU4
>  #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

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

2018-02-16 Thread 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 
Signed-off-by: Maxime Coquelin 
---
 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_MTU4
 #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