From 861fec459bb4db69123c3b320e1bad9585cdb524 Mon Sep 17 00:00:00 2001 From: Chao Gao Date: Fri, 17 Nov 2017 14:24:23 +0800 Subject: [PATCH 1/7] i386/msi: Correct mask of destination ID in MSI address According to SDM 10.11.1, only [19:12] bits of MSI address are Destination ID, change the mask to avoid ambiguity for VT-d spec has used the bit 4 to indicate a remappable interrupt request. Signed-off-by: Chao Gao Signed-off-by: Lan Tianyu Reviewed-by: Anthony PERARD Reviewed-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/i386/apic-msidef.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/i386/apic-msidef.h b/include/hw/i386/apic-msidef.h index 8b4d4cca55..420b41167d 100644 --- a/include/hw/i386/apic-msidef.h +++ b/include/hw/i386/apic-msidef.h @@ -26,6 +26,6 @@ #define MSI_ADDR_DEST_ID_SHIFT 12 #define MSI_ADDR_DEST_IDX_SHIFT 4 -#define MSI_ADDR_DEST_ID_MASK 0x00ffff0 +#define MSI_ADDR_DEST_ID_MASK 0x000ff000 #endif /* HW_APIC_MSIDEF_H */ From 2d4ba6cc741df15df6fbb4feaa706a02e103083a Mon Sep 17 00:00:00 2001 From: Maxime Coquelin Date: Thu, 16 Nov 2017 19:48:34 +0100 Subject: [PATCH 2/7] virtio: Add queue interface to restore avail index from vring used index In case of backend crash, it is not possible to restore internal avail index from the backend value as vhost_get_vring_base callback fails. This patch provides a new interface to restore internal avail index from the vring used index, as done by some vhost-user backend on reconnection. Signed-off-by: Maxime Coquelin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 10 ++++++++++ include/hw/virtio/virtio.h | 1 + 2 files changed, 11 insertions(+) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index ea532dc35f..703e672f3d 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2310,6 +2310,16 @@ void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx) vdev->vq[n].shadow_avail_idx = idx; } +void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n) +{ + rcu_read_lock(); + if (vdev->vq[n].vring.desc) { + vdev->vq[n].last_avail_idx = vring_used_idx(&vdev->vq[n]); + vdev->vq[n].shadow_avail_idx = vdev->vq[n].last_avail_idx; + } + rcu_read_unlock(); +} + void virtio_queue_update_used_idx(VirtIODevice *vdev, int n) { rcu_read_lock(); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 5abada6966..098bdaaea3 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -272,6 +272,7 @@ hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n); hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n); uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n); void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx); +void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n); void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n); void virtio_queue_update_used_idx(VirtIODevice *vdev, int n); VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n); From 2ae39a113af311cb56a0c35b7f212dafcef15303 Mon Sep 17 00:00:00 2001 From: Maxime Coquelin Date: Thu, 16 Nov 2017 19:48:35 +0100 Subject: [PATCH 3/7] vhost: restore avail index from vring used index on disconnection vhost_virtqueue_stop() gets avail index value from the backend, except if the backend is not responding. It happens when the backend crashes, and in this case, internal state of the virtio queue is inconsistent, making packets to corrupt the vring state. With a Linux guest, it results in following error message on backend reconnection: [ 22.444905] virtio_net virtio0: output.0:id 0 is not a head! [ 22.446746] net enp0s3: Unexpected TXQ (0) queue failure: -5 [ 22.476360] net enp0s3: Unexpected TXQ (0) queue failure: -5 Fixes: 283e2c2adcb8 ("net: virtio-net discards TX data after link down") Cc: qemu-stable@nongnu.org Signed-off-by: Maxime Coquelin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index ddc42f0f93..54041948cf 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1138,6 +1138,10 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev, r = dev->vhost_ops->vhost_get_vring_base(dev, &state); if (r < 0) { VHOST_OPS_DEBUG("vhost VQ %d ring restore failed: %d", idx, r); + /* Connection to the backend is broken, so let's sync internal + * last avail idx to the device used idx. + */ + virtio_queue_restore_last_avail_idx(vdev, idx); } else { virtio_queue_set_last_avail_idx(vdev, idx, state.num); } From d36d0a9d152316a41e02c2613a71f5859f407da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Fri, 1 Dec 2017 12:37:44 +0100 Subject: [PATCH 4/7] dump-guest-memory.py: fix No symbol "vmcoreinfo_find" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When qemu is compiled without debug, the dump gdb python script can fail with: Error occurred in Python command: No symbol "vmcoreinfo_find" in current context. Because vmcoreinfo_find() is inlined and not exported. Use the underlying object_resolve_path_type() to get the instance instead. Signed-off-by: Marc-André Lureau Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- scripts/dump-guest-memory.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py index 69dd5efadf..1af26c1a45 100644 --- a/scripts/dump-guest-memory.py +++ b/scripts/dump-guest-memory.py @@ -546,13 +546,15 @@ shape and this command should mostly work.""" return None def add_vmcoreinfo(self): - if not gdb.parse_and_eval("vmcoreinfo_find()") \ - or not gdb.parse_and_eval("vmcoreinfo_find()->has_vmcoreinfo"): + vmci = '(VMCoreInfoState *)' + \ + 'object_resolve_path_type("", "vmcoreinfo", 0)' + if not gdb.parse_and_eval("%s" % vmci) \ + or not gdb.parse_and_eval("(%s)->has_vmcoreinfo" % vmci): return - fmt = gdb.parse_and_eval("vmcoreinfo_find()->vmcoreinfo.guest_format") - addr = gdb.parse_and_eval("vmcoreinfo_find()->vmcoreinfo.paddr") - size = gdb.parse_and_eval("vmcoreinfo_find()->vmcoreinfo.size") + fmt = gdb.parse_and_eval("(%s)->vmcoreinfo.guest_format" % vmci) + addr = gdb.parse_and_eval("(%s)->vmcoreinfo.paddr" % vmci) + size = gdb.parse_and_eval("(%s)->vmcoreinfo.size" % vmci) fmt = le16_to_cpu(fmt) addr = le64_to_cpu(addr) From 2fe45ec3bffbd3a26f2ed39f60bab0ca5217d8f6 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 30 Nov 2017 22:39:59 +0100 Subject: [PATCH 5/7] vhost: fix error check in vhost_verify_ring_mappings() Since commit f1f9e6c5 "vhost: adapt vhost_verify_ring_mappings() to virtio 1 ring layout", we check the mapping of each part (descriptor table, available ring and used ring) of each virtqueue separately. The checking of a part is done by the vhost_verify_ring_part_mapping() function: it returns either 0 on success or a negative errno if the part cannot be mapped at the same place. Unfortunately, the vhost_verify_ring_mappings() function checks its return value the other way round. It means that we either: - only verify the descriptor table of the first virtqueue, and if it is valid we ignore all the other mappings - or ignore all broken mappings until we reach a valid one ie, we only raise an error if all mappings are broken, and we consider all mappings are valid otherwise (false success), which is obviously wrong. This patch ensures that vhost_verify_ring_mappings() only returns success if ALL mappings are okay. Reported-by: Dr. David Alan Gilbert Signed-off-by: Greg Kurz Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 54041948cf..e4290ce93d 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -493,21 +493,21 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev, j = 0; r = vhost_verify_ring_part_mapping(dev, vq->desc, vq->desc_phys, vq->desc_size, start_addr, size); - if (!r) { + if (r) { break; } j++; r = vhost_verify_ring_part_mapping(dev, vq->avail, vq->avail_phys, vq->avail_size, start_addr, size); - if (!r) { + if (r) { break; } j++; r = vhost_verify_ring_part_mapping(dev, vq->used, vq->used_phys, vq->used_size, start_addr, size); - if (!r) { + if (r) { break; } } From 758ead31c7e17bf17a9ef2e0ca1c3e86ab296b43 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 29 Nov 2017 23:14:27 +0530 Subject: [PATCH 6/7] virtio: check VirtQueue Vring object is set A guest could attempt to use an uninitialised VirtQueue object or unset Vring.align leading to a arithmetic exception. Add check to avoid it. Reported-by: Zhangboxian Signed-off-by: Prasad J Pandit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi Reviewed-by: Cornelia Huck --- hw/virtio/virtio.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 703e672f3d..ad564b0132 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -182,7 +182,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n) { VRing *vring = &vdev->vq[n].vring; - if (!vring->desc) { + if (!vring->num || !vring->desc || !vring->align) { /* not yet setup -> nothing to do */ return; } @@ -1414,6 +1414,9 @@ void virtio_config_modern_writel(VirtIODevice *vdev, void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) { + if (!vdev->vq[n].vring.num) { + return; + } vdev->vq[n].vring.desc = addr; virtio_queue_update_rings(vdev, n); } @@ -1426,6 +1429,9 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n) void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, hwaddr avail, hwaddr used) { + if (!vdev->vq[n].vring.num) { + return; + } vdev->vq[n].vring.desc = desc; vdev->vq[n].vring.avail = avail; vdev->vq[n].vring.used = used; @@ -1494,8 +1500,10 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) */ assert(k->has_variable_vring_alignment); - vdev->vq[n].vring.align = align; - virtio_queue_update_rings(vdev, n); + if (align) { + vdev->vq[n].vring.align = align; + virtio_queue_update_rings(vdev, n); + } } static bool virtio_queue_notify_aio_vq(VirtQueue *vq) From 75ba2ddb188fa07c3442446766782036e3085cba Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 20 Nov 2017 18:19:23 +0100 Subject: [PATCH 7/7] pc: fix crash on attempted cpu unplug when qemu is started with '-no-acpi' CLI option, an attempt to unplug a CPU using device_del results in null pointer dereference at: #0 object_get_class #1 pc_machine_device_unplug_request_cb #2 qmp_marshal_device_del which is caused by pcms->acpi_dev == NULL due to ACPI support being disabled. Considering that ACPI support is necessary for unplug to work, check that it's enabled and fail unplug request gracefully if no acpi device were found. Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c3afe5b7f1..186545d2a4 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1842,6 +1842,11 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev, X86CPU *cpu = X86_CPU(dev); PCMachineState *pcms = PC_MACHINE(hotplug_dev); + if (!pcms->acpi_dev) { + error_setg(&local_err, "CPU hot unplug not supported without ACPI"); + goto out; + } + pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx); assert(idx != -1); if (idx == 0) {