[Qemu-devel] [PULL 0/6] pci, pc, virtio bug fixes for 1.7

[Qemu-devel] [PULL 0/6] pci, pc, virtio bug fixes for 1.7

[Michael S. Tsirkin]

The following changes since commit c2d30667760e3d7b81290d801e567d4f758825ca:

  rtc: remove dead SQW IRQ code (2013-11-05 20:04:03 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony

for you to fetch changes up to ef9e455d645bed6d2360cd658dc00ca11a849877:

  Revert "exec: limit system memory size" (2013-11-10 15:11:01 +0200)

----------------------------------------------------------------
pci, pc, virtio bug fixes

This reverts PCI master abort support - we'll want it
eventually but it exposes too many core bugs to be safe for 1.7.
This also reverts a recent exec.c change that was an
attempt to work-around some of these core bugs.

Also included are small fixes in pc and virtio,
and a core loader fix for PPC bamboo.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Cole Robinson (1):
      Fix pc migration from qemu <= 1.5

Jason Wang (1):
      virtio-net: only delete bh that existed

Marcel Apfelbaum (1):
      Revert "hw/pci: partially handle pci master abort"

Michael S. Tsirkin (3):
      acpi-build: disable with -no-acpi
      loader: drop return value for rom_add_blob_fixed
      Revert "exec: limit system memory size"

 include/hw/i386/pc.h      | 16 ++++++++++++++++
 include/hw/loader.h       |  2 +-
 include/hw/pci-host/q35.h |  1 +
 include/hw/pci/pci_bus.h  |  1 -
 exec.c                    |  7 +------
 hw/i386/acpi-build.c      |  5 +++++
 hw/net/virtio-net.c       |  2 +-
 hw/pci-host/piix.c        |  9 ++++++++-
 hw/pci-host/q35.c         | 10 ++++++++--
 hw/pci/pci.c              | 26 --------------------------
 hw/ppc/ppc440_bamboo.c    |  3 ++-
 11 files changed, 43 insertions(+), 39 deletions(-)

-- 
MST

[Qemu-devel] [PULL 1/6] Fix pc migration from qemu <= 1.5

[Michael S. Tsirkin]

From: Cole Robinson <crobinso@redhat.com>

The following commit introduced a migration incompatibility:

commit 568f0690fd9aa4d39d84b04c1a5dbb53a915c3fe
Author: David Gibson <david@gibson.dropbear.id.au>
Date:   Thu Jun 6 18:48:49 2013 +1000

    pci: Replace pci_find_domain() with more general pci_root_bus_path()

The issue is that i440fx savevm idstr went from 0000:00:00.0/I440FX to
0000:00.0/I440FX. Unfortunately we are stuck with the breakage for
1.6 machine types.

Add a compat property to maintain the busted idstr for the 1.6 machine
types, but revert to the old style format for 1.7+, and <= 1.5.

Tested with migration from qemu 1.5, qemu 1.6, and qemu.git.

Cc: qemu-stable@nongnu.org
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/pc.h      | 16 ++++++++++++++++
 include/hw/pci-host/q35.h |  1 +
 hw/pci-host/piix.c        |  9 ++++++++-
 hw/pci-host/q35.c         | 10 ++++++++--
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 03cc0ba..57e8d16 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -260,6 +260,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
             .driver   = "qemu32-" TYPE_X86_CPU,\
             .property = "model",\
             .value    = stringify(3),\
+        },{\
+            .driver   = "i440FX-pcihost",\
+            .property = "short_root_bus",\
+            .value    = stringify(1),\
+        },{\
+            .driver   = "q35-pcihost",\
+            .property = "short_root_bus",\
+            .value    = stringify(1),\
         }
 
 #define PC_COMPAT_1_5 \
@@ -296,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
             .driver = TYPE_X86_CPU,\
             .property = "pmu",\
             .value = "on",\
+        },{\
+            .driver   = "i440FX-pcihost",\
+            .property = "short_root_bus",\
+            .value    = stringify(0),\
+        },{\
+            .driver   = "q35-pcihost",\
+            .property = "short_root_bus",\
+            .value    = stringify(0),\
         }
 
 #define PC_COMPAT_1_4 \
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index aee91aa..309065f 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -61,6 +61,7 @@ typedef struct MCHPCIState {
     ram_addr_t above_4g_mem_size;
     uint64_t pci_hole64_size;
     PcGuestInfo *guest_info;
+    uint32_t short_root_bus;
 } MCHPCIState;
 
 typedef struct Q35PCIHost {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index bad3953..edc974e 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -48,6 +48,7 @@ typedef struct I440FXState {
     PCIHostState parent_obj;
     PcPciInfo pci_info;
     uint64_t pci_hole64_size;
+    uint32_t short_root_bus;
 } I440FXState;
 
 #define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
@@ -720,13 +721,19 @@ static const TypeInfo i440fx_info = {
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                 PCIBus *rootbus)
 {
+    I440FXState *s = I440FX_PCI_HOST_BRIDGE(host_bridge);
+
     /* For backwards compat with old device paths */
-    return "0000";
+    if (s->short_root_bus) {
+        return "0000";
+    }
+    return "0000:00";
 }
 
 static Property i440fx_props[] = {
     DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState,
                      pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE),
+    DEFINE_PROP_UINT32("short_root_bus", I440FXState, short_root_bus, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b8feed1..c043998 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -61,8 +61,13 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
 static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
                                           PCIBus *rootbus)
 {
-    /* For backwards compat with old device paths */
-    return "0000";
+    Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge);
+
+     /* For backwards compat with old device paths */
+    if (s->mch.short_root_bus) {
+        return "0000";
+    }
+    return "0000:00";
 }
 
 static void q35_host_get_pci_hole_start(Object *obj, Visitor *v,
@@ -124,6 +129,7 @@ static Property mch_props[] = {
                         MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT),
     DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost,
                      mch.pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE),
+    DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
MST

[Qemu-devel] [PULL 2/6] virtio-net: only delete bh that existed

[Michael S. Tsirkin]

From: Jason Wang <jasowang@redhat.com>

We delete without check whether it existed during exit. This will lead NULL
pointer deference since it was created conditionally depends on guest driver
status and features. So add a check of existence before trying to delete it.

Cc: qemu-stable@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 22dbd05..ae51d96 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1601,7 +1601,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
         if (q->tx_timer) {
             timer_del(q->tx_timer);
             timer_free(q->tx_timer);
-        } else {
+        } else if (q->tx_bh) {
             qemu_bh_delete(q->tx_bh);
         }
     }
-- 
MST

[Qemu-devel] [PULL 3/6] acpi-build: disable with -no-acpi

[Michael S. Tsirkin]

QEMU will currently crash if started with -no-acpi flag
since acpi build code probes the PM device which isn't present
in this configuration.

To fix, don't expose ACPI tables to guest when acpi has been
disabled from command line.

Fixes LP# 1248854
https://bugs.launchpad.net/qemu/+bug/1248854

Reported-by: chao zhou <chao.zhou@intel.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/acpi-build.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6cfa044..486e705 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1182,6 +1182,11 @@ void acpi_setup(PcGuestInfo *guest_info)
         return;
     }
 
+    if (!acpi_enabled) {
+        ACPI_BUILD_DPRINTF(3, "ACPI disabled. Bailing out.\n");
+        return;
+    }
+
     build_state = g_malloc0(sizeof *build_state);
 
     build_state->guest_info = guest_info;
-- 
MST

[Qemu-devel] [PULL 4/6] loader: drop return value for rom_add_blob_fixed

[Michael S. Tsirkin]

rom_add_blob never fails, and neither does rom_add_blob_fixed,
so there's no need to return value from it.

In fact, rom_add_blob_fixed was erroneously returning -1 unconditionally
which made the only system that checked the return value -M bamboo fail
to start.

Drop the return value and drop checks from ppc440_bamboo to
fix this failure.

Reported-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/loader.h    | 2 +-
 hw/ppc/ppc440_bamboo.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 58eca98..7a23d6b 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -55,7 +55,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
 #define rom_add_file_fixed(_f, _a, _i)          \
     rom_add_file(_f, NULL, _a, _i)
 #define rom_add_blob_fixed(_f, _b, _l, _a)      \
-    (rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL) ? 0 : -1)
+    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
 
 #define PC_ROM_MIN_VGA     0xc0000
 #define PC_ROM_MIN_OPTION  0xc8000
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 655e499..67597df 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -110,8 +110,9 @@ static int bamboo_load_device_tree(hwaddr addr,
     qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
                               tb_freq);
 
-    ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
+    rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     g_free(fdt);
+    return 0;
 
 out:
 
-- 
MST

[Qemu-devel] [PULL 5/6] Revert "hw/pci: partially handle pci master abort"

[Michael S. Tsirkin]

From: Marcel Apfelbaum <marcel.a@redhat.com>

This reverts commit a53ae8e934cd54686875b5bcfc2f434244ee55d6.

The patch being reverted introduced a low-priority memory region
covering all 64 bit pci address space.  This exposed the following bugs
elsewhere in the code:
 1. Some memory regions have INT64_MAX size, where the
    intent was all 64 bit address space.
    This results in a sub-page region, should be UINT64_MAX.
 2. page table rendering in exec.c ignores physical address bits
    above TARGET_PHYS_ADDR_SPACE_BITS.
    Access outside this range (e.g. from device DMA, or gdb stub)
    ends up with a wrong region.  Registering a region outside this
    range leads to page table corruption.
 3. Some regions overlap PCI hole and have same priority.
    This only works as long as no device uses the overlapping address.

It doesn't look like we can resolve all issues in time for 1.7.
Let's fix the bugs first and apply afterwards for 1.8.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci_bus.h |  1 -
 hw/pci/pci.c             | 26 --------------------------
 2 files changed, 27 deletions(-)

diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 2ad5edb..9df1788 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -23,7 +23,6 @@ struct PCIBus {
     PCIDevice *parent_dev;
     MemoryRegion *address_space_mem;
     MemoryRegion *address_space_io;
-    MemoryRegion master_abort_mem;
 
     QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
     QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a98c8a0..ed32059 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -283,24 +283,6 @@ const char *pci_root_bus_path(PCIDevice *dev)
     return rootbus->qbus.name;
 }
 
-static uint64_t master_abort_mem_read(void *opaque, hwaddr addr, unsigned size)
-{
-   return -1ULL;
-}
-
-static void master_abort_mem_write(void *opaque, hwaddr addr, uint64_t val,
-                                   unsigned size)
-{
-}
-
-static const MemoryRegionOps master_abort_mem_ops = {
-    .read = master_abort_mem_read,
-    .write = master_abort_mem_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-#define MASTER_ABORT_MEM_PRIORITY INT_MIN
-
 static void pci_bus_init(PCIBus *bus, DeviceState *parent,
                          const char *name,
                          MemoryRegion *address_space_mem,
@@ -312,14 +294,6 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
     bus->address_space_mem = address_space_mem;
     bus->address_space_io = address_space_io;
 
-
-    memory_region_init_io(&bus->master_abort_mem, OBJECT(bus),
-                          &master_abort_mem_ops, bus, "pci-master-abort",
-                          memory_region_size(bus->address_space_mem));
-    memory_region_add_subregion_overlap(bus->address_space_mem,
-                                        0, &bus->master_abort_mem,
-                                        MASTER_ABORT_MEM_PRIORITY);
-
     /* host bridge */
     QLIST_INIT(&bus->child);
 
-- 
MST

[Qemu-devel] [PULL 6/6] Revert "exec: limit system memory size"

[Michael S. Tsirkin]

This reverts commit 818f86b88394b7b2b59d313e51043fe15a8004db.

This was a work-around for bugs elsewhere in the system,
exposed by commit a53ae8e934cd54686875b5bcfc2f434244ee55d6:
    "hw/pci: partially handle pci master abort"
since that's reverted now, the work-around is not required for 1.7
anymore.
The proper fix is supporting full 64 bit addresses in the radix tree.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 exec.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 79610ce..b453713 100644
--- a/exec.c
+++ b/exec.c
@@ -1741,12 +1741,7 @@ void address_space_destroy_dispatch(AddressSpace *as)
 static void memory_map_init(void)
 {
     system_memory = g_malloc(sizeof(*system_memory));
-
-    assert(TARGET_PHYS_ADDR_SPACE_BITS <= 64);
-
-    memory_region_init(system_memory, NULL, "system",
-                       TARGET_PHYS_ADDR_SPACE_BITS == 64 ?
-                       UINT64_MAX : (0x1ULL << TARGET_PHYS_ADDR_SPACE_BITS));
+    memory_region_init(system_memory, NULL, "system", INT64_MAX);
     address_space_init(&address_space_memory, system_memory, "memory");
 
     system_io = g_malloc(sizeof(*system_io));
-- 
MST