[qemu] Fix crash on migration/snapshot (bz #1144490)
Cole Robinson
crobinso at fedoraproject.org
Sun Sep 21 16:42:19 UTC 2014
commit e84b901375af387840a54ac39df05a30b8a02adf
Author: Cole Robinson <crobinso at redhat.com>
Date: Sun Sep 21 12:42:19 2014 -0400
Fix crash on migration/snapshot (bz #1144490)
0006-virtio-net-drop-assert-on-vm-stop.patch | 30 +++++
...rt-virtio-don-t-call-device-on-vm_running.patch | 53 +++++++++
...rtio-pci-enable-bus-master-for-old-guests.patch | 44 ++++++++
...rtio-pci-fix-migration-for-pci-bus-master.patch | 117 ++++++++++++++++++++
...-pc-leave-more-space-for-BIOS-allocations.patch | 47 ++++++++
qemu.spec | 17 +++-
6 files changed, 307 insertions(+), 1 deletions(-)
---
diff --git a/0006-virtio-net-drop-assert-on-vm-stop.patch b/0006-virtio-net-drop-assert-on-vm-stop.patch
new file mode 100644
index 0000000..920bc13
--- /dev/null
+++ b/0006-virtio-net-drop-assert-on-vm-stop.patch
@@ -0,0 +1,30 @@
+From b6bb785977e725e1a0f3ed203f5ffe3def56e03f Mon Sep 17 00:00:00 2001
+From: "Michael S. Tsirkin" <mst at redhat.com>
+Date: Thu, 11 Sep 2014 18:32:51 +0300
+Subject: [PATCH] virtio-net: drop assert on vm stop
+
+On vm stop, vm_running state set to stopped
+before device is notified, so callbacks can get envoked with
+vm_running = false; and this is not an error.
+
+Cc: qemu-stable at nongnu.org
+Acked-by: Jason Wang <jasowang at redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
+(cherry picked from commit 131c5221fe25a9547c4a388a3d26ff7fd14843e5)
+---
+ hw/net/virtio-net.c | 2 --
+ 1 file changed, 2 deletions(-)
+
+diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
+index 826a2a5..2040eac 100644
+--- a/hw/net/virtio-net.c
++++ b/hw/net/virtio-net.c
+@@ -1125,8 +1125,6 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
+ return num_packets;
+ }
+
+- assert(vdev->vm_running);
+-
+ if (q->async_tx.elem.out_num) {
+ virtio_queue_set_notification(q->tx_vq, 0);
+ return num_packets;
diff --git a/0007-Revert-virtio-don-t-call-device-on-vm_running.patch b/0007-Revert-virtio-don-t-call-device-on-vm_running.patch
new file mode 100644
index 0000000..1d60661
--- /dev/null
+++ b/0007-Revert-virtio-don-t-call-device-on-vm_running.patch
@@ -0,0 +1,53 @@
+From 6b448e66c6e2e993b2c5666cbc0747e6405e80fa Mon Sep 17 00:00:00 2001
+From: "Michael S. Tsirkin" <mst at redhat.com>
+Date: Thu, 11 Sep 2014 18:42:02 +0300
+Subject: [PATCH] Revert "virtio: don't call device on !vm_running"
+
+This reverts commit a1bc7b827e422e1ff065640d8ec5347c4aadfcd8.
+ virtio: don't call device on !vm_running
+It turns out that virtio net assumes that vm_running
+is updated before device status callback in many places,
+so this change leads to asserts.
+Previous commit fixes the root issue that motivated
+a1bc7b827e422e1ff065640d8ec5347c4aadfcd8 differently,
+so there's no longer a need for this change.
+
+In the future, we might be able to drop checking vm_running
+completely, and check vm state directly.
+
+Reported-by: Dietmar Maurer <dietmar at proxmox.com>
+Cc: qemu-stable at nongnu.org
+Acked-by: Jason Wang <jasowang at redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
+(cherry picked from commit 9e8e8c48653471fa5fed447e388fdef57d4f6998)
+---
+ hw/virtio/virtio.c | 9 +--------
+ 1 file changed, 1 insertion(+), 8 deletions(-)
+
+diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
+index ac22238..5c98180 100644
+--- a/hw/virtio/virtio.c
++++ b/hw/virtio/virtio.c
+@@ -1108,10 +1108,7 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
+ BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+ bool backend_run = running && (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK);
+-
+- if (running) {
+- vdev->vm_running = running;
+- }
++ vdev->vm_running = running;
+
+ if (backend_run) {
+ virtio_set_status(vdev, vdev->status);
+@@ -1124,10 +1121,6 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
+ if (!backend_run) {
+ virtio_set_status(vdev, vdev->status);
+ }
+-
+- if (!running) {
+- vdev->vm_running = running;
+- }
+ }
+
+ void virtio_init(VirtIODevice *vdev, const char *name,
diff --git a/0008-virtio-pci-enable-bus-master-for-old-guests.patch b/0008-virtio-pci-enable-bus-master-for-old-guests.patch
new file mode 100644
index 0000000..5c163ab
--- /dev/null
+++ b/0008-virtio-pci-enable-bus-master-for-old-guests.patch
@@ -0,0 +1,44 @@
+From b8ecf2e833811f9fce6bcf3a87ca52fb490e2f13 Mon Sep 17 00:00:00 2001
+From: "Michael S. Tsirkin" <mst at redhat.com>
+Date: Thu, 11 Sep 2014 18:45:33 +0200
+Subject: [PATCH] virtio-pci: enable bus master for old guests
+
+commit cc943c36faa192cd4b32af8fe5edb31894017d35
+ pci: Use bus master address space for delivering MSI/MSI-X messages
+breaks virtio-net for rhel6.[56] x86 guests because they don't
+enable bus mastering for virtio PCI devices. For the same reason,
+rhel6.[56] ppc64 guests cannot boot on a virtio-blk disk anymore.
+
+Old guests forgot to enable bus mastering, enable it automatically on
+DRIVER (guests use some devices before DRIVER_OK).
+
+Reported-by: Greg Kurz <gkurz at linux.vnet.ibm.com>
+Reviewed-by: Greg Kurz <gkurz at linux.vnet.ibm.com>
+Tested-by: Greg Kurz <gkurz at linux.vnet.ibm.com>
+Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
+(cherry picked from commit e43c0b2ea5574efb0bedebf6a7d05916eefeba52)
+---
+ hw/virtio/virtio-pci.c | 10 ++++++++++
+ 1 file changed, 10 insertions(+)
+
+diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
+index 3007319..58ebbcf 100644
+--- a/hw/virtio/virtio-pci.c
++++ b/hw/virtio/virtio-pci.c
+@@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+ msix_unuse_all_vectors(&proxy->pci_dev);
+ }
+
++ /* Linux before 2.6.34 drives the device without enabling
++ the PCI device bus master bit. Enable it automatically
++ for the guest. This is a PCI spec violation but so is
++ initiating DMA with bus master bit clear. */
++ if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) {
++ pci_default_write_config(&proxy->pci_dev, PCI_COMMAND,
++ proxy->pci_dev.config[PCI_COMMAND] |
++ PCI_COMMAND_MASTER, 1);
++ }
++
+ /* Linux before 2.6.34 sets the device as OK without enabling
+ the PCI device bus master bit. In this case we need to disable
+ some safety checks. */
diff --git a/0009-virtio-pci-fix-migration-for-pci-bus-master.patch b/0009-virtio-pci-fix-migration-for-pci-bus-master.patch
new file mode 100644
index 0000000..5784067
--- /dev/null
+++ b/0009-virtio-pci-fix-migration-for-pci-bus-master.patch
@@ -0,0 +1,117 @@
+From 53db3d7b0836c13268f141bb46653a7898011d1e Mon Sep 17 00:00:00 2001
+From: "Michael S. Tsirkin" <mst at redhat.com>
+Date: Thu, 11 Sep 2014 18:34:29 +0300
+Subject: [PATCH] virtio-pci: fix migration for pci bus master
+
+Current support for bus master (clearing OK bit)
+together with the need to support guests which do not
+enable PCI bus mastering, leads to extra state in
+VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust
+in case of cross-version migration for the case when
+guests use the device before setting DRIVER_OK.
+
+Rip out VIRTIO_PCI_FLAG_BUS_MASTER_BUG and implement a simpler
+work-around: treat clearing of PCI_COMMAND as a virtio reset. Old
+guests never touch this bit so they will work.
+
+As reset clears device status, DRIVER and MASTER bits are
+now in sync, so we can fix up cross-version migration simply
+by synchronising them, without need to detect a buggy guest
+explicitly.
+
+Drop tracking VIRTIO_PCI_FLAG_BUS_MASTER_BUG completely.
+
+As reset makes the device quiescent, in the future we'll be able to drop
+checking OK bit in a bunch of places.
+
+Cc: Jason Wang <jasowang at redhat.com>
+Cc: Greg Kurz <gkurz at linux.vnet.ibm.com>
+Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
+(cherry picked from commit 4d43d3f3c8147ade184df9a1e9e82826edd39e19)
+---
+ hw/virtio/virtio-pci.c | 39 ++++++++++++++++++++-------------------
+ 1 file changed, 20 insertions(+), 19 deletions(-)
+
+diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
+index 58ebbcf..c19c4d6 100644
+--- a/hw/virtio/virtio-pci.c
++++ b/hw/virtio/virtio-pci.c
+@@ -86,9 +86,6 @@
+ * 12 is historical, and due to x86 page size. */
+ #define VIRTIO_PCI_QUEUE_ADDR_SHIFT 12
+
+-/* Flags track per-device state like workarounds for quirks in older guests. */
+-#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG (1 << 0)
+-
+ static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
+ VirtIOPCIProxy *dev);
+
+@@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+ proxy->pci_dev.config[PCI_COMMAND] |
+ PCI_COMMAND_MASTER, 1);
+ }
+-
+- /* Linux before 2.6.34 sets the device as OK without enabling
+- the PCI device bus master bit. In this case we need to disable
+- some safety checks. */
+- if ((val & VIRTIO_CONFIG_S_DRIVER_OK) &&
+- !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+- proxy->flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
+- }
+ break;
+ case VIRTIO_MSI_CONFIG_VECTOR:
+ msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
+@@ -480,13 +469,18 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
+ VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
++ uint8_t cmd = proxy->pci_dev.config[PCI_COMMAND];
++
+ pci_default_write_config(pci_dev, address, val, len);
+
+ if (range_covers_byte(address, len, PCI_COMMAND) &&
+ !(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
+- !(proxy->flags & VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
++ (cmd & PCI_COMMAND_MASTER)) {
++ /* Bus driver disables bus mastering - make it act
++ * as a kind of reset to render the device quiescent. */
+ virtio_pci_stop_ioeventfd(proxy);
+- virtio_set_status(vdev, vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK);
++ virtio_reset(vdev);
++ msix_unuse_all_vectors(&proxy->pci_dev);
+ }
+ }
+
+@@ -895,11 +889,19 @@ static void virtio_pci_vmstate_change(DeviceState *d, bool running)
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+ if (running) {
+- /* Try to find out if the guest has bus master disabled, but is
+- in ready state. Then we have a buggy guest OS. */
+- if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+- !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+- proxy->flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
++ /* Linux before 2.6.34 drives the device without enabling
++ the PCI device bus master bit. Enable it automatically
++ for the guest. This is a PCI spec violation but so is
++ initiating DMA with bus master bit clear.
++ Note: this only makes a difference when migrating
++ across QEMU versions from an old QEMU, as for new QEMU
++ bus master and driver bits are always in sync.
++ TODO: consider enabling conditionally for compat machine types. */
++ if (vdev->status & (VIRTIO_CONFIG_S_ACKNOWLEDGE |
++ VIRTIO_CONFIG_S_DRIVER)) {
++ pci_default_write_config(&proxy->pci_dev, PCI_COMMAND,
++ proxy->pci_dev.config[PCI_COMMAND] |
++ PCI_COMMAND_MASTER, 1);
+ }
+ virtio_pci_start_ioeventfd(proxy);
+ } else {
+@@ -1043,7 +1045,6 @@ static void virtio_pci_reset(DeviceState *qdev)
+ virtio_pci_stop_ioeventfd(proxy);
+ virtio_bus_reset(bus);
+ msix_unuse_all_vectors(&proxy->pci_dev);
+- proxy->flags &= ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
+ }
+
+ static Property virtio_pci_properties[] = {
diff --git a/0010-pc-leave-more-space-for-BIOS-allocations.patch b/0010-pc-leave-more-space-for-BIOS-allocations.patch
new file mode 100644
index 0000000..75e3314
--- /dev/null
+++ b/0010-pc-leave-more-space-for-BIOS-allocations.patch
@@ -0,0 +1,47 @@
+From beb8650b34906da8f87e07238ab33aa8ffc54a67 Mon Sep 17 00:00:00 2001
+From: "Michael S. Tsirkin" <mst at redhat.com>
+Date: Thu, 18 Sep 2014 16:32:07 +0300
+Subject: [PATCH] pc: leave more space for BIOS allocations
+
+Since QEMU 2.1, we are allocating more space for ACPI tables, so no
+space is left after initrd for the BIOS to allocate memory.
+
+Besides ACPI tables, there are a few other uses of high memory in
+SeaBIOS: SMBIOS tables and USB drivers use it in particular. These uses
+allocate a very small amount of memory. Malloc metadata also lives
+there. So we need _some_ extra padding there to avoid initrd breakage,
+but not much.
+
+John Snow found a case where RHEL5 was broken by the recent change to
+ACPI_TABLE_SIZE; in his case 4KB of extra padding are fine, but just to
+be safe I am adding 32KB, which is roughly the same amount of padding
+that was left by QEMU 2.0 and earlier.
+
+Move initrd to leave some space for the BIOS.
+
+Cc: qemu-stable at nongnu.org
+Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
+Reported-by: John Snow <jsnow at redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
+(cherry picked from commit 438f92ee9f6a4f78f8adcc399809e252b6da72a2)
+---
+ hw/i386/pc.c | 6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/hw/i386/pc.c b/hw/i386/pc.c
+index 97932a6..ef9fad8 100644
+--- a/hw/i386/pc.c
++++ b/hw/i386/pc.c
+@@ -72,8 +72,10 @@
+ #define DPRINTF(fmt, ...)
+ #endif
+
+-/* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables. */
+-unsigned acpi_data_size = 0x20000;
++/* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables
++ * (128K) and other BIOS datastructures (less than 4K reported to be used at
++ * the moment, 32K should be enough for a while). */
++unsigned acpi_data_size = 0x20000 + 0x8000;
+ void pc_set_legacy_acpi_data_size(void)
+ {
+ acpi_data_size = 0x10000;
diff --git a/qemu.spec b/qemu.spec
index d58377c..c6ccc85 100644
--- a/qemu.spec
+++ b/qemu.spec
@@ -152,7 +152,7 @@
Summary: QEMU is a FAST! processor emulator
Name: qemu
Version: 2.1.1
-Release: 1%{?dist}
+Release: 2%{?dist}
Epoch: 2
License: GPLv2+ and LGPLv2+ and BSD
Group: Development/Tools
@@ -199,6 +199,12 @@ Patch0002: 0002-aarch64-Allow-kernel-option-to-take-a-gzip-compresse.patch
Patch0003: 0003-block.curl-adding-timeout-option.patch
Patch0004: 0004-curl-Allow-a-cookie-or-cookies-to-be-sent-with-http-.patch
Patch0005: 0005-curl-Don-t-deref-NULL-pointer-in-call-to-aio_poll.patch
+# Fix crash on migration/snapshot (bz #1144490)
+Patch0006: 0006-virtio-net-drop-assert-on-vm-stop.patch
+Patch0007: 0007-Revert-virtio-don-t-call-device-on-vm_running.patch
+Patch0008: 0008-virtio-pci-enable-bus-master-for-old-guests.patch
+Patch0009: 0009-virtio-pci-fix-migration-for-pci-bus-master.patch
+Patch0010: 0010-pc-leave-more-space-for-BIOS-allocations.patch
BuildRequires: SDL2-devel
BuildRequires: zlib-devel
@@ -731,6 +737,12 @@ CAC emulation development files.
%patch0003 -p1
%patch0004 -p1
%patch0005 -p1
+# Fix crash on migration/snapshot (bz #1144490)
+%patch0006 -p1
+%patch0007 -p1
+%patch0008 -p1
+%patch0009 -p1
+%patch0010 -p1
%build
@@ -1510,6 +1522,9 @@ getent passwd qemu >/dev/null || \
%endif
%changelog
+* Sun Sep 21 2014 Cole Robinson <crobinso at redhat.com> - 2:2.1.1-2
+- Fix crash on migration/snapshot (bz #1144490)
+
* Thu Sep 11 2014 Cole Robinson <crobinso at redhat.com> - 2:2.1.1-1
- Rebased to version 2.1.1
- CVE-2014-5388: out of bounds memory access (bz #1132962, bz #1132956)
More information about the scm-commits
mailing list