From cd7d1d26b0a333bf2fca715e332690bbd738c097 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Tue, 19 Aug 2014 12:56:29 +0800 Subject: [PATCH 1/4] vhost_net: start/stop guest notifiers properly commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 "vhost: multiqueue support" changed the order of stopping the device. Previously vhost_dev_stop would disable backend and only afterwards, unset guest notifiers. We now unset guest notifiers while vhost is still active. This can lose interrupts causing guest networking to fail. In particular, this has been observed during migration. To fix this, several other changes are needed: - remove the hdev->started assertion in vhost.c since we may want to start the guest notifiers before vhost starts and stop the guest notifiers after vhost is stopped. - introduce the vhost_net_set_vq_index() and call it before setting guest notifiers. This is to guarantee vhost_net has the correct virtqueue index when setting guest notifiers. MST: fix up error handling. Cc: qemu-stable@nongnu.org Cc: Jason Wang Signed-off-by: Michael S. Tsirkin Tested-by: Andrey Korolyov Reported-by: "Zhangjie (HZ)" Tested-by: William Dauchy Signed-off-by: Jason Wang Reviewed-by: Michael S. Tsirkin --- hw/net/vhost_net.c | 41 +++++++++++++++++++++++++++-------------- hw/virtio/vhost.c | 2 -- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 9bbf2ee4ce..3819044b02 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -188,16 +188,19 @@ bool vhost_net_query(VHostNetState *net, VirtIODevice *dev) return vhost_dev_query(&net->dev, dev); } +static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index) +{ + net->dev.vq_index = vq_index; +} + static int vhost_net_start_one(struct vhost_net *net, - VirtIODevice *dev, - int vq_index) + VirtIODevice *dev) { struct vhost_vring_file file = { }; int r; net->dev.nvqs = 2; net->dev.vqs = net->vqs; - net->dev.vq_index = vq_index; r = vhost_dev_enable_notifiers(&net->dev, dev); if (r < 0) { @@ -286,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); - int r, i = 0; + int r, e, i; if (!vhost_net_device_endian_ok(dev)) { error_report("vhost-net does not support cross-endian"); @@ -301,11 +304,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, } for (i = 0; i < total_queues; i++) { - r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev, i * 2); - - if (r < 0) { - goto err; - } + vhost_net_set_vq_index(get_vhost_net(ncs[i].peer), i * 2); } r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true); @@ -314,12 +313,26 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, goto err; } + for (i = 0; i < total_queues; i++) { + r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); + + if (r < 0) { + goto err_start; + } + } + return 0; -err: +err_start: while (--i >= 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } + e = k->set_guest_notifiers(qbus->parent, total_queues * 2, false); + if (e < 0) { + fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", e); + fflush(stderr); + } +err: return r; } @@ -331,16 +344,16 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); int i, r; + for (i = 0; i < total_queues; i++) { + vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); + } + r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false); if (r < 0) { fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r); fflush(stderr); } assert(r >= 0); - - for (i = 0; i < total_queues; i++) { - vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); - } } void vhost_net_cleanup(struct vhost_net *net) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index e55fe1cc7e..5d7c40ac04 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -976,7 +976,6 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n) { struct vhost_virtqueue *vq = hdev->vqs + n - hdev->vq_index; - assert(hdev->started); assert(n >= hdev->vq_index && n < hdev->vq_index + hdev->nvqs); return event_notifier_test_and_clear(&vq->masked_notifier); } @@ -988,7 +987,6 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, struct VirtQueue *vvq = virtio_get_queue(vdev, n); int r, index = n - hdev->vq_index; - assert(hdev->started); assert(n >= hdev->vq_index && n < hdev->vq_index + hdev->nvqs); struct vhost_vring_file file = { From b49ae9138d5cadb47fb868297fbcdac8292fb666 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Wed, 3 Sep 2014 14:25:30 +0800 Subject: [PATCH 2/4] vhost_net: init acked_features to backend_features commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. As this field is now uninitialized, vhost initialization will sometimes fail. To fix, initialize acked_features on each ack. Tested-by: Andrey Korolyov Cc: Nikolay Nikolaev Cc: qemu-stable@nongnu.org Signed-off-by: Jason Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 3819044b02..b21e7a434f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { + net->dev.acked_features = net->dev.backend_features; vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features); } From 3a1655fc53a2d0375dc0b8cd358405c2cae288e3 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 3 Sep 2014 12:00:12 +0300 Subject: [PATCH 3/4] vhost-scsi: init backend features earlier As vhost core can use backend_features during init, clear it earlier to avoid using uninitialized memory. This use would be harmless since vhost scsi ignores the result anyway, but initializing earlier will help prevent valgrind errors, and make scsi and net behave similarly. Cc: qemu-stable@nongnu.org Acked-by: Paolo Bonzini Acked-by: Jason Wang Signed-off-by: Michael S. Tsirkin --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ddfe76aed0..7146e0ec49 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -238,6 +238,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs); s->dev.vq_index = 0; + s->dev.backend_features = 0; ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL, true); @@ -246,7 +247,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) strerror(-ret)); return; } - s->dev.backend_features = 0; error_setg(&s->migration_blocker, "vhost-scsi does not support migration"); From 07b81ed937b37e4c1974626c38e2f192ce08f8f5 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 29 Aug 2014 11:52:51 +0800 Subject: [PATCH 4/4] acpi-build: Set FORCE_APIC_CLUSTER_MODEL bit for FADT flags If we start Windows 2008 R2 DataCenter with number of cpu less than 8, The system will use APIC Flat Logical destination mode as default configuration, Which has an upper limit of 8 CPUs. The fault is that VM can not show all processors within Task Manager if we hot-add cpus when the number of cpus in VM extends the limit of 8. If we use cluster destination model, the problem will be solved. Note: This flag was introduced later than ACPI v1.0 specification while QEMU generates v1.0 tables only, but... linux kernel ignores this flag, so patch has no influence on it. Tested with Win[XPsp3|Srv2003EE|Srv2008DC|Srv2008R2|Srv2012R2], there isn't BSODs and guests boot just fine. In cases guest doesn't support cpu-hotplug, cpu becomes visible after reboot and in case the guest supports cpu-hotplug, it works as expected with this patch. Cc: qemu-stable@nongnu.org Signed-off-by: huangzhichao Signed-off-by: zhanghailiang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-By: Igor Mammedov --- hw/i386/acpi-build.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 3e7fba3822..a3133211a8 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -551,6 +551,12 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm) (1 << ACPI_FADT_F_SLP_BUTTON) | (1 << ACPI_FADT_F_RTC_S4)); fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK); + /* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs + * For more than 8 CPUs, "Clustered Logical" mode has to be used + */ + if (max_cpus > 8) { + fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL); + } }