[RESEND,v1,36/37] vhost-user/msg: handling VHOST_USER_SET_FEATURES

Commit Message

Wang, Wei W Dec. 19, 2016, 5:59 a.m. UTC

If the featuer bits sent by the slave are not equal to the ones that
were sent by the master, perform a reset of the master device.

Signed-off-by: Wei Wang <[e-mail]>
---
 hw/net/vhost_net.c       |  2 ++
 hw/virtio/vhost-user.c   | 20 ++++++++++++++++++++
 hw/virtio/virtio-pci.c   | 20 ++++++++++++++++++++
 hw/virtio/virtio-pci.h   |  2 ++
 include/net/vhost-user.h | 14 ++++++++++++++
 net/vhost-user.c         | 14 +++++---------
 6 files changed, 63 insertions(+), 9 deletions(-)

Comments

Wang, Wei W Dec. 19, 2016, 8:28 a.m. UTC | #1

On 12/19/2016 01:59 PM, Wei Wang wrote:
> If the featuer bits sent by the slave are not equal to the ones that
> were sent by the master, perform a reset of the master device.
>
> Signed-off-by: Wei Wang <[e-mail]>
> ---
>   hw/net/vhost_net.c       |  2 ++
>   hw/virtio/vhost-user.c   | 20 ++++++++++++++++++++
>   hw/virtio/virtio-pci.c   | 20 ++++++++++++++++++++
>   hw/virtio/virtio-pci.h   |  2 ++
>   include/net/vhost-user.h | 14 ++++++++++++++
>   net/vhost-user.c         | 14 +++++---------
>   6 files changed, 63 insertions(+), 9 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 8256018..e8a2d4f 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
>   
>   /* virtio-net-pci */
>   
> +void master_reset_virtio_net(VirtIODevice *vdev)
> +{
> +    VirtIONet *net = VIRTIO_NET(vdev);
> +    VirtIONetPCI *net_pci = container_of(net, VirtIONetPCI, vdev);
> +    VirtIOPCIProxy *proxy = &net_pci->parent_obj;
> +    DeviceState *qdev = DEVICE(proxy);
> +    DeviceState *qdev_new;
> +    Error *err = NULL;
> +
> +    virtio_pci_reset(qdev);
> +    qdev_unplug(qdev, &err);
> +    qdev->realized = false;
> +    qdev_new = qdev_device_add(qdev->opts, &err);
> +    if (!qdev_new) {
> +        qemu_opts_del(qdev->opts);
> +    }
> +    object_unref(OBJECT(qdev));
> +}
> +

I still have a problem with this patch. Looks like the virtio reset here 
only clears the registers and queue related things. Do we have a power 
reset of virtio, which has the same effect as re-plugging into the 
virtio device (the driver probe() are re-invoked and feature bits are 
re-negotiated). Thanks.

Best,
Wei

Patch

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 8256018..e8a2d4f 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -349,6 +349,8 @@  int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         }
     }
 
+    vhost_user_set_master_dev(ncs[0].peer, dev);
+
     return 0;
 
 err_start:
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e9242a9..990275f 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -12,6 +12,7 @@ 
 #include "qapi/error.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-backend.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/vhost-user.h"
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost-user.h"
@@ -75,6 +76,22 @@  fail:
     return -1;
 }
 
+static void handle_slave_acked_features(const char *name, VhostUserMsg *msg)
+{
+    CharBackend *chr_be = net_name_to_chr_be(name);
+    VhostUserState *s = container_of(chr_be, VhostUserState, chr);
+    VirtIODevice *vdev = s->vdev;
+    uint64_t master_features, slave_features;
+
+    master_features = vhost_net_get_acked_features(s->vhost_net)
+                      & ~(1 << VHOST_USER_F_PROTOCOL_FEATURES);
+    slave_features = msg->payload.u64;
+
+    if (master_features != slave_features) {
+        master_reset_virtio_net(vdev);
+    }
+}
+
 int vhost_user_can_read(void *opaque)
 {
     return VHOST_USER_HDR_SIZE;
@@ -109,6 +126,9 @@  void vhost_user_asyn_read(void *opaque, const uint8_t *buf, int size)
     }
 
     switch (msg.request) {
+    case VHOST_USER_SET_FEATURES:
+        handle_slave_acked_features(name, &msg);
+        break;
     default:
         error_report("vhost-user master does not support msg request = %d",
                      msg.request);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 20cbefc..82b2920 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -38,6 +38,7 @@ 
 #include "qemu/range.h"
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/visitor.h"
+#include "monitor/qdev.h"
 
 #define VIRTIO_PCI_REGION_SIZE(dev)     VIRTIO_PCI_CONFIG_OFF(msix_present(dev))
 
@@ -2257,6 +2258,25 @@  static const TypeInfo virtio_serial_pci_info = {
 
 /* virtio-net-pci */
 
+void master_reset_virtio_net(VirtIODevice *vdev)
+{
+    VirtIONet *net = VIRTIO_NET(vdev);
+    VirtIONetPCI *net_pci = container_of(net, VirtIONetPCI, vdev);
+    VirtIOPCIProxy *proxy = &net_pci->parent_obj;
+    DeviceState *qdev = DEVICE(proxy);
+    DeviceState *qdev_new;
+    Error *err = NULL;
+
+    virtio_pci_reset(qdev);
+    qdev_unplug(qdev, &err);
+    qdev->realized = false;
+    qdev_new = qdev_device_add(qdev->opts, &err);
+    if (!qdev_new) {
+        qemu_opts_del(qdev->opts);
+    }
+    object_unref(OBJECT(qdev));
+}
+
 static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 0b4b04f..c41c621 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -383,4 +383,6 @@  struct VirtIOCryptoPCI {
 /* Virtio ABI version, if we increment this, we break the guest driver. */
 #define VIRTIO_PCI_ABI_VERSION          0
 
+void master_reset_virtio_net(VirtIODevice *vdev);
+
 #endif
diff --git a/include/net/vhost-user.h b/include/net/vhost-user.h
index 1bb5f1a..4cd14c9 100644
--- a/include/net/vhost-user.h
+++ b/include/net/vhost-user.h
@@ -12,6 +12,18 @@ 
 #define NET_VHOST_USER_H
 
 #include "sysemu/char.h"
+#include "net/vhost_net.h"
+
+typedef struct VhostUserState {
+    NetClientState nc;
+    CharBackend chr; /* only queue index 0 */
+    VHostNetState *vhost_net;
+    guint watch;
+    uint64_t acked_features;
+    bool started;
+    /* Pointer to the master device */
+    VirtIODevice *vdev;
+} VhostUserState;
 
 struct vhost_net;
 struct vhost_net *vhost_user_get_vhost_net(NetClientState *nc);
@@ -19,4 +31,6 @@  uint64_t vhost_user_get_acked_features(NetClientState *nc);
 
 CharBackend *net_name_to_chr_be(const char *name);
 
+void vhost_user_set_master_dev(NetClientState *nc, VirtIODevice *vdev);
+
 #endif /* VHOST_USER_H */
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 72010c2..5b31224 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -10,7 +10,6 @@ 
 
 #include "qemu/osdep.h"
 #include "clients.h"
-#include "net/vhost_net.h"
 #include "net/vhost-user.h"
 #include "hw/virtio/vhost-user.h"
 #include "qemu/config-file.h"
@@ -18,14 +17,11 @@ 
 #include "qmp-commands.h"
 #include "trace.h"
 
-typedef struct VhostUserState {
-    NetClientState nc;
-    CharBackend chr; /* only queue index 0 */
-    VHostNetState *vhost_net;
-    guint watch;
-    uint64_t acked_features;
-    bool started;
-} VhostUserState;
+void vhost_user_set_master_dev(NetClientState *nc, VirtIODevice *vdev)
+{
+    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+    s->vdev = vdev;
+}
 
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
 {