From d50c6c8b0fc28c2dd91f3f7ab2a0bbb56419214b Mon Sep 17 00:00:00 2001 From: Alexey Korolev Date: Wed, 29 Feb 2012 14:35:14 +1300 Subject: piix_pci: fix typo in i400FX chipset init code There is a typo in i440FX init code. This is causing problems when somebody wants to access the 64bit PCI range. Signed-off-by: Alexey Korolev Signed-off-by: Michael S. Tsirkin --- hw/piix_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 179d9a6..09e84f5 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -349,7 +349,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, b = i440fx_common_init("i440FX", pi440fx_state, piix3_devfn, isa_bus, pic, address_space_mem, address_space_io, ram_size, pci_hole_start, pci_hole_size, - pci_hole64_size, pci_hole64_size, + pci_hole64_start, pci_hole64_size, pci_memory, ram_memory); return b; } -- cgit v1.1 From d6c730086cbf24382eb8cff25551798769edfd84 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 26 Mar 2012 11:26:16 +0200 Subject: pc: reduce duplication in compat machine types Make it easier to add compat properties, by adding macros for properties duplicated across machine types. Note: there could be bugs in compat properties, this patch does not attempt to address them, the code is bug for bug identical to the original. Tested by: generated a preprocessed file, sorted and compared to sorted original. Lightly tested on x86_64. Signed-off-by: Michael S. Tsirkin --- hw/pc_piix.c | 288 +++++++++++++++-------------------------------------------- 1 file changed, 73 insertions(+), 215 deletions(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index fadca4c..0f61f00 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -359,50 +359,69 @@ static QEMUMachine pc_machine_v1_1 = { .is_default = 1, }; +#define PC_COMPAT_1_0 \ + {\ + .driver = "pc-sysfw",\ + .property = "rom_only",\ + .value = stringify(1),\ + }, {\ + .driver = "isa-fdc",\ + .property = "check_media_rate",\ + .value = "off",\ + } + static QEMUMachine pc_machine_v1_0 = { .name = "pc-1.0", .desc = "Standard PC", .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) { - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), - }, { - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, + PC_COMPAT_1_0, { /* end of list */ } }, }; +#define PC_COMPAT_0_15 \ + PC_COMPAT_1_0 + static QEMUMachine pc_machine_v0_15 = { .name = "pc-0.15", .desc = "Standard PC", .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) { - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), - }, { - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, + PC_COMPAT_0_15, { /* end of list */ } }, }; +#define PC_COMPAT_0_14 \ + PC_COMPAT_0_15,\ + {\ + .driver = "virtio-blk-pci",\ + .property = "event_idx",\ + .value = "off",\ + },{\ + .driver = "virtio-serial-pci",\ + .property = "event_idx",\ + .value = "off",\ + },{\ + .driver = "virtio-net-pci",\ + .property = "event_idx",\ + .value = "off",\ + },{\ + .driver = "virtio-balloon-pci",\ + .property = "event_idx",\ + .value = "off",\ + } + static QEMUMachine pc_machine_v0_14 = { .name = "pc-0.14", .desc = "Standard PC", .init = pc_init_pci, .max_cpus = 255, .compat_props = (GlobalProperty[]) { + PC_COMPAT_0_14, { .driver = "qxl", .property = "revision", @@ -411,42 +430,30 @@ static QEMUMachine pc_machine_v0_14 = { .driver = "qxl-vga", .property = "revision", .value = stringify(2), - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, { /* end of list */ } }, }; +#define PC_COMPAT_0_13 \ + PC_COMPAT_0_14,\ + {\ + .driver = "PCI",\ + .property = "command_serr_enable",\ + .value = "off",\ + },{\ + .driver = "AC97",\ + .property = "use_broken_id",\ + .value = stringify(1),\ + } + static QEMUMachine pc_machine_v0_13 = { .name = "pc-0.13", .desc = "Standard PC", .init = pc_init_pci_no_kvmclock, .max_cpus = 255, .compat_props = (GlobalProperty[]) { + PC_COMPAT_0_13, { .driver = "virtio-9p-pci", .property = "vectors", @@ -459,59 +466,31 @@ static QEMUMachine pc_machine_v0_13 = { .driver = "vmware-svga", .property = "rombar", .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, { /* end of list */ } }, }; +#define PC_COMPAT_0_12 \ + PC_COMPAT_0_13,\ + {\ + .driver = "virtio-serial-pci",\ + .property = "max_ports",\ + .value = stringify(1),\ + },{\ + .driver = "virtio-serial-pci",\ + .property = "vectors",\ + .value = stringify(0),\ + } + static QEMUMachine pc_machine_v0_12 = { .name = "pc-0.12", .desc = "Standard PC", .init = pc_init_pci_no_kvmclock, .max_cpus = 255, .compat_props = (GlobalProperty[]) { + PC_COMPAT_0_12, { - .driver = "virtio-serial-pci", - .property = "max_ports", - .value = stringify(1), - },{ - .driver = "virtio-serial-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver = "VGA", .property = "rombar", .value = stringify(0), @@ -519,63 +498,27 @@ static QEMUMachine pc_machine_v0_12 = { .driver = "vmware-svga", .property = "rombar", .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, { /* end of list */ } } }; +#define PC_COMPAT_0_11 \ + PC_COMPAT_0_12,\ + {\ + .driver = "virtio-blk-pci",\ + .property = "vectors",\ + .value = stringify(0),\ + } + static QEMUMachine pc_machine_v0_11 = { .name = "pc-0.11", .desc = "Standard PC, qemu 0.11", .init = pc_init_pci_no_kvmclock, .max_cpus = 255, .compat_props = (GlobalProperty[]) { + PC_COMPAT_0_11, { - .driver = "virtio-blk-pci", - .property = "vectors", - .value = stringify(0), - },{ - .driver = "virtio-serial-pci", - .property = "max_ports", - .value = stringify(1), - },{ - .driver = "virtio-serial-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver = "ide-drive", .property = "ver", .value = "0.11", @@ -583,43 +526,6 @@ static QEMUMachine pc_machine_v0_11 = { .driver = "scsi-disk", .property = "ver", .value = "0.11", - },{ - .driver = "PCI", - .property = "rombar", - .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, { /* end of list */ } } @@ -631,6 +537,7 @@ static QEMUMachine pc_machine_v0_10 = { .init = pc_init_pci_no_kvmclock, .max_cpus = 255, .compat_props = (GlobalProperty[]) { + PC_COMPAT_0_11, { .driver = "virtio-blk-pci", .property = "class", @@ -640,22 +547,10 @@ static QEMUMachine pc_machine_v0_10 = { .property = "class", .value = stringify(PCI_CLASS_DISPLAY_OTHER), },{ - .driver = "virtio-serial-pci", - .property = "max_ports", - .value = stringify(1), - },{ - .driver = "virtio-serial-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver = "virtio-net-pci", .property = "vectors", .value = stringify(0), },{ - .driver = "virtio-blk-pci", - .property = "vectors", - .value = stringify(0), - },{ .driver = "ide-drive", .property = "ver", .value = "0.10", @@ -663,43 +558,6 @@ static QEMUMachine pc_machine_v0_10 = { .driver = "scsi-disk", .property = "ver", .value = "0.10", - },{ - .driver = "PCI", - .property = "rombar", - .value = stringify(0), - },{ - .driver = "PCI", - .property = "command_serr_enable", - .value = "off", - },{ - .driver = "virtio-blk-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-serial-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-net-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "virtio-balloon-pci", - .property = "event_idx", - .value = "off", - },{ - .driver = "AC97", - .property = "use_broken_id", - .value = stringify(1), - },{ - .driver = "isa-fdc", - .property = "check_media_rate", - .value = "off", - }, - { - .driver = "pc-sysfw", - .property = "rom_only", - .value = stringify(1), }, { /* end of list */ } }, -- cgit v1.1 From e314672a8a95f5dc98534f0682fce50fb83dbc5c Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 13 Aug 2010 09:54:52 -0400 Subject: vhost: Fix size of dirty log sync on resize When the vhost log is resized, we want to sync up to the size of the old log. With that end address in place, ignore regions that start after then end rather than hitting assert. This also addresses the following crash report: When migrating a vm using vhost-net we hit the following assertion: qemu-kvm: /usr/src/packages/BUILD/qemu-kvm-0.15.1/hw/vhost.c:30: vhost_dev_sync_region: Assertion `start / (0x1000 * (8 * sizeof(vhost_log_chunk_t))) < dev->log_size' failed. The cases which the end < start check is intended to catch, such as for vga video memory, will also likely trigger the assertion. Reorder the code to handle this correctly. Reported-by: Josh Durgin Signed-off-by: Bruce Rogers Signed-off-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- hw/vhost.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index 8d3ba5b..7e282dd 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -31,11 +31,11 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, vhost_log_chunk_t *to = dev->log + end / VHOST_LOG_CHUNK + 1; uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK; - assert(end / VHOST_LOG_CHUNK < dev->log_size); - assert(start / VHOST_LOG_CHUNK < dev->log_size); if (end < start) { return; } + assert(end / VHOST_LOG_CHUNK < dev->log_size); + for (;from < to; ++from) { vhost_log_chunk_t log; int bit; @@ -277,8 +277,9 @@ static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size) r = ioctl(dev->control, VHOST_SET_LOG_BASE, &log_base); assert(r >= 0); for (i = 0; i < dev->n_mem_sections; ++i) { - vhost_sync_dirty_bitmap(dev, &dev->mem_sections[i], - 0, (target_phys_addr_t)~0x0ull); + /* Sync only the range covered by the old log */ + vhost_sync_dirty_bitmap(dev, &dev->mem_sections[i], 0, + dev->log_size * VHOST_LOG_CHUNK - 1); } if (dev->log) { g_free(dev->log); -- cgit v1.1 From fbbaf9ae88286f18b2ab32fb4174ebdbdc1d5919 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 1 Apr 2012 11:39:43 +0300 Subject: vhost: readd assert statement It's clear from the surrounding code that start < end so it's enough to assert end < log_size. However, it's better to make this explicit in case we refactor the code again. Signed-off-by: Michael S. Tsirkin --- hw/vhost.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/vhost.c b/hw/vhost.c index 7e282dd..43664e7 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -35,6 +35,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, return; } assert(end / VHOST_LOG_CHUNK < dev->log_size); + assert(start / VHOST_LOG_CHUNK < dev->log_size); for (;from < to; ++from) { vhost_log_chunk_t log; -- cgit v1.1 From 4490c71191b59dce2dd88f5f9ab49f2c92ab410c Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 5 Dec 2011 21:48:43 +0200 Subject: ivshmem: add missing msix calls ivshmem used msix but didn't call it on either reset or config write paths. This used to partically work since guests don't use all of msi-x configuration fields, and reset is rarely used, but the patch 'msix: track function masked in pci device state' broke that. Fix by adding appropriate calls. Signed-off-by: Michael S. Tsirkin Reported-by: Cam Macdonell Tested-by: Cam Macdonell --- hw/ivshmem.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/hw/ivshmem.c b/hw/ivshmem.c index b80aa8f..c2bdd92 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -509,11 +509,29 @@ static void ivshmem_read(void *opaque, const uint8_t * buf, int flags) return; } +/* Select the MSI-X vectors used by device. + * ivshmem maps events to vectors statically, so + * we just enable all vectors on init and after reset. */ +static void ivshmem_use_msix(IVShmemState * s) +{ + int i; + + if (!msix_present(&s->dev)) { + return; + } + + for (i = 0; i < s->vectors; i++) { + msix_vector_use(&s->dev, i); + } +} + static void ivshmem_reset(DeviceState *d) { IVShmemState *s = DO_UPCAST(IVShmemState, dev.qdev, d); s->intrstatus = 0; + msix_reset(&s->dev); + ivshmem_use_msix(s); return; } @@ -544,12 +562,8 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { return value; } -static void ivshmem_setup_msi(IVShmemState * s) { - - int i; - - /* allocate the MSI-X vectors */ - +static void ivshmem_setup_msi(IVShmemState * s) +{ memory_region_init(&s->msix_bar, "ivshmem-msix", 4096); if (!msix_init(&s->dev, s->vectors, &s->msix_bar, 1, 0)) { pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, @@ -560,13 +574,10 @@ static void ivshmem_setup_msi(IVShmemState * s) { exit(1); } - /* 'activate' the vectors */ - for (i = 0; i < s->vectors; i++) { - msix_vector_use(&s->dev, i); - } - /* allocate QEMU char devices for receiving interrupts */ s->eventfd_table = g_malloc0(s->vectors * sizeof(EventfdEntry)); + + ivshmem_use_msix(s); } static void ivshmem_save(QEMUFile* f, void *opaque) @@ -590,7 +601,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) IVSHMEM_DPRINTF("ivshmem_load\n"); IVShmemState *proxy = opaque; - int ret, i; + int ret; if (version_id > 0) { return -EINVAL; @@ -608,9 +619,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) { msix_load(&proxy->dev, f); - for (i = 0; i < proxy->vectors; i++) { - msix_vector_use(&proxy->dev, i); - } + ivshmem_use_msix(proxy); } else { proxy->intrstatus = qemu_get_be32(f); proxy->intrmask = qemu_get_be32(f); @@ -619,6 +628,13 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) return 0; } +static void ivshmem_write_config(PCIDevice *pci_dev, uint32_t address, + uint32_t val, int len) +{ + pci_default_write_config(pci_dev, address, val, len); + msix_write_config(pci_dev, address, val, len); +} + static int pci_ivshmem_init(PCIDevice *dev) { IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev); @@ -744,6 +760,8 @@ static int pci_ivshmem_init(PCIDevice *dev) } + s->dev.config_write = ivshmem_write_config; + return 0; } -- cgit v1.1 From 2ba1d381c2f5f5868fe071b45977c2ed459d78f0 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 3 Apr 2012 17:24:11 +0300 Subject: virtio-pci: change virtio balloon PCI class code Currently the virtio balloon device, when using the virtio-pci interface advertises itself with PCI class code MEMORY_RAM. This is wrong; the balloon is vaguely related to memory, but is nothing like a PCI memory device in the meaning of the class code, and this code is not required or suggested by the virtio PCI specification. Worse, this patch causes problems on the pseries machine, because the firmware, seeing this class code, advertises the device as memory in the device tree, and then a guest kernel bug causes it to see this "memory" before the real system memory, leading to a crash in early boot. This patch fixes the problem by removing the bogus PCI class code on the balloon device. The backwards compatibility PC machines get new compat properties so that they don't change. Cc: Rusty Russell Signed-off-by: David Gibson Signed-off-by: Michael S. Tsirkin --- hw/pc_piix.c | 5 +++++ hw/virtio-pci.c | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 0f61f00..5c08245 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -28,6 +28,7 @@ #include "pc.h" #include "apic.h" #include "pci.h" +#include "pci_ids.h" #include "net.h" #include "boards.h" #include "ide.h" @@ -368,6 +369,10 @@ static QEMUMachine pc_machine_v1_1 = { .driver = "isa-fdc",\ .property = "check_media_rate",\ .value = "off",\ + }, {\ + .driver = "virtio-balloon-pci",\ + .property = "class",\ + .value = stringify(PCI_CLASS_MEMORY_RAM),\ } static QEMUMachine pc_machine_v1_0 = { diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index a0fb7c1..4a4413d 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -790,6 +790,11 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); VirtIODevice *vdev; + if (proxy->class_code != PCI_CLASS_OTHERS && + proxy->class_code != PCI_CLASS_MEMORY_RAM) { /* qemu < 1.1 */ + proxy->class_code = PCI_CLASS_OTHERS; + } + vdev = virtio_balloon_init(&pci_dev->qdev); if (!vdev) { return -1; @@ -906,6 +911,7 @@ static TypeInfo virtio_serial_info = { static Property virtio_balloon_properties[] = { DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), + DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -919,7 +925,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON; k->revision = VIRTIO_PCI_ABI_VERSION; - k->class_id = PCI_CLASS_MEMORY_RAM; + k->class_id = PCI_CLASS_OTHERS; dc->reset = virtio_pci_reset; dc->props = virtio_balloon_properties; } -- cgit v1.1 From ba737541edddf9d0026460eb7b1d1c599b4c8ae9 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Thu, 5 Apr 2012 11:07:08 -0600 Subject: acpi_piix4: Disallow write to up/down PCI hotplug registers The write side of these registers is never used and actually can't be used as defined because any read/modify/write sequence from the guest potentially races with qemu. Drop the write support and define these as read-only registers. Signed-off-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- docs/specs/acpi_pci_hotplug.txt | 4 ++-- hw/acpi_piix4.c | 44 +++++++++++++---------------------------- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt index f0f74a7..1e2c8a2 100644 --- a/docs/specs/acpi_pci_hotplug.txt +++ b/docs/specs/acpi_pci_hotplug.txt @@ -15,14 +15,14 @@ PCI slot injection notification pending (IO port 0xae00-0xae03, 4-byte access): Slot injection notification pending. One bit per slot. Read by ACPI BIOS GPE.1 handler to notify OS of injection -events. +events. Read-only. PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access): ----------------------------------------------------- Slot removal notification pending. One bit per slot. Read by ACPI BIOS GPE.1 handler to notify OS of removal -events. +events. Read-only. PCI device eject (IO port 0xae08-0xae0b, 4-byte access): ---------------------------------------- diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 797ed24..5e8b261 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -40,7 +40,8 @@ #define GPE_BASE 0xafe0 #define GPE_LEN 4 -#define PCI_BASE 0xae00 +#define PCI_UP_BASE 0xae00 +#define PCI_DOWN_BASE 0xae04 #define PCI_EJ_BASE 0xae08 #define PCI_RMV_BASE 0xae0c @@ -448,38 +449,22 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val) PIIX4_DPRINTF("gpe write %x <== %d\n", addr, val); } -static uint32_t pcihotplug_read(void *opaque, uint32_t addr) +static uint32_t pci_up_read(void *opaque, uint32_t addr) { - uint32_t val = 0; - struct pci_status *g = opaque; - switch (addr) { - case PCI_BASE: - val = g->up; - break; - case PCI_BASE + 4: - val = g->down; - break; - default: - break; - } + PIIX4PMState *s = opaque; + uint32_t val = s->pci0_status.up; - PIIX4_DPRINTF("pcihotplug read %x == %x\n", addr, val); + PIIX4_DPRINTF("pci_up_read %x\n", val); return val; } -static void pcihotplug_write(void *opaque, uint32_t addr, uint32_t val) +static uint32_t pci_down_read(void *opaque, uint32_t addr) { - struct pci_status *g = opaque; - switch (addr) { - case PCI_BASE: - g->up = val; - break; - case PCI_BASE + 4: - g->down = val; - break; - } - - PIIX4_DPRINTF("pcihotplug write %x <== %d\n", addr, val); + PIIX4PMState *s = opaque; + uint32_t val = s->pci0_status.down; + + PIIX4_DPRINTF("pci_down_read %x\n", val); + return val; } static uint32_t pciej_read(void *opaque, uint32_t addr) @@ -523,14 +508,13 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) { - struct pci_status *pci0_status = &s->pci0_status; register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s); register_ioport_read(GPE_BASE, GPE_LEN, 1, gpe_readb, s); acpi_gpe_blk(&s->ar, GPE_BASE); - register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status); - register_ioport_read(PCI_BASE, 8, 4, pcihotplug_read, pci0_status); + register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s); + register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s); register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus); register_ioport_read(PCI_EJ_BASE, 4, 4, pciej_read, bus); -- cgit v1.1 From 7faa8075d898ae56d2c533c530569bb25ab86eaf Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Thu, 5 Apr 2012 11:07:15 -0600 Subject: acpi_piix4: Fix PCI hotplug race As Michael Tsirkin demonstrated, current PCI hotplug is vulnerable to a few races. The first is a race with other hotplug operations because we clear the up & down registers at each event. If a new event comes before the last is processed, up/down is cleared and the event is lost. To fix this for the down register, we create a life cycle for the event request that starts with the hot unplug request in piix4_device_hotplug() and ends when the device is ejected. This allows us to mask and clear individual bits, preserving them against races. For the up register, we have no clear end point for when the event is finished. We could modify the BIOS to acknowledge the bit and clear it, but this creates BIOS compatibiliy issues without offering a complete solution. Instead we note that gratuitous ACPI device checks are not harmful, which allows us to issue a device check for every slot. We know which slots are present and we know which slots are hotpluggable, so we can easily reduce this to a more manageable set for the guest. The other race Michael noted was that an unplug request followed by reset may also lose the eject notification, which may also result in the eject request being lost which a subsequent add or remove. Once we're in reset, the device is unused and we can flush the queue of device removals ourselves. Previously if a device_del was issued to a guest without ACPI PCI hotplug support, it was necessary to shutdown the guest to recover the device. With this, a guest reboot is sufficient. Signed-off-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- hw/acpi_piix4.c | 74 +++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 5e8b261..0e7af51 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -48,7 +48,7 @@ #define PIIX4_PCI_HOTPLUG_STATUS 2 struct pci_status { - uint32_t up; + uint32_t up; /* deprecated, maintained for migration compatibility */ uint32_t down; }; @@ -70,6 +70,7 @@ typedef struct PIIX4PMState { /* for pci hotplug */ struct pci_status pci0_status; uint32_t pci0_hotplug_enable; + uint32_t pci0_slot_device_present; } PIIX4PMState; static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s); @@ -205,6 +206,17 @@ static void pm_write_config(PCIDevice *d, pm_io_space_update((PIIX4PMState *)d); } +static void vmstate_pci_status_pre_save(void *opaque) +{ + struct pci_status *pci0_status = opaque; + PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status); + + /* We no longer track up, so build a safe value for migrating + * to a version that still does... of course these might get lost + * by an old buggy implementation, but we try. */ + pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable; +} + static int vmstate_acpi_post_load(void *opaque, int version_id) { PIIX4PMState *s = opaque; @@ -241,6 +253,7 @@ static const VMStateDescription vmstate_pci_status = { .version_id = 1, .minimum_version_id = 1, .minimum_version_id_old = 1, + .pre_save = vmstate_pci_status_pre_save, .fields = (VMStateField []) { VMSTATE_UINT32(up, struct pci_status), VMSTATE_UINT32(down, struct pci_status), @@ -269,13 +282,38 @@ static const VMStateDescription vmstate_acpi = { } }; +static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots) +{ + DeviceState *qdev, *next; + BusState *bus = qdev_get_parent_bus(&s->dev.qdev); + int slot = ffs(slots) - 1; + + /* Mark request as complete */ + s->pci0_status.down &= ~(1U << slot); + + QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) { + PCIDevice *dev = PCI_DEVICE(qdev); + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); + if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) { + s->pci0_slot_device_present &= ~(1U << slot); + qdev_free(qdev); + } + } +} + static void piix4_update_hotplug(PIIX4PMState *s) { PCIDevice *dev = &s->dev; BusState *bus = qdev_get_parent_bus(&dev->qdev); DeviceState *qdev, *next; + /* Execute any pending removes during reset */ + while (s->pci0_status.down) { + acpi_piix_eject_slot(s, s->pci0_status.down); + } + s->pci0_hotplug_enable = ~0; + s->pci0_slot_device_present = 0; QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) { PCIDevice *pdev = PCI_DEVICE(qdev); @@ -283,8 +321,10 @@ static void piix4_update_hotplug(PIIX4PMState *s) int slot = PCI_SLOT(pdev->devfn); if (pc->no_hotplug) { - s->pci0_hotplug_enable &= ~(1 << slot); + s->pci0_hotplug_enable &= ~(1U << slot); } + + s->pci0_slot_device_present |= (1U << slot); } } @@ -452,7 +492,11 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val) static uint32_t pci_up_read(void *opaque, uint32_t addr) { PIIX4PMState *s = opaque; - uint32_t val = s->pci0_status.up; + uint32_t val; + + /* Manufacture an "up" value to cause a device check on any hotplug + * slot with a device. Extra device checks are harmless. */ + val = s->pci0_slot_device_present & s->pci0_hotplug_enable; PIIX4_DPRINTF("pci_up_read %x\n", val); return val; @@ -475,18 +519,7 @@ static uint32_t pciej_read(void *opaque, uint32_t addr) static void pciej_write(void *opaque, uint32_t addr, uint32_t val) { - BusState *bus = opaque; - DeviceState *qdev, *next; - int slot = ffs(val) - 1; - - QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) { - PCIDevice *dev = PCI_DEVICE(qdev); - PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); - if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) { - qdev_free(qdev); - } - } - + acpi_piix_eject_slot(opaque, val); PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val); } @@ -516,8 +549,8 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s); register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s); - register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus); - register_ioport_read(PCI_EJ_BASE, 4, 4, pciej_read, bus); + register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s); + register_ioport_read(PCI_EJ_BASE, 4, 4, pciej_read, s); register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s); register_ioport_read(PCI_RMV_BASE, 4, 4, pcirmv_read, s); @@ -528,13 +561,13 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) static void enable_device(PIIX4PMState *s, int slot) { s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; - s->pci0_status.up |= (1 << slot); + s->pci0_slot_device_present |= (1U << slot); } static void disable_device(PIIX4PMState *s, int slot) { s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS; - s->pci0_status.down |= (1 << slot); + s->pci0_status.down |= (1U << slot); } static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, @@ -548,11 +581,10 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, * it is present on boot, no hotplug event is necessary. We do send an * event when the device is disabled later. */ if (state == PCI_COLDPLUG_ENABLED) { + s->pci0_slot_device_present |= (1U << slot); return 0; } - s->pci0_status.up = 0; - s->pci0_status.down = 0; if (state == PCI_HOTPLUG_ENABLED) { enable_device(s, slot); } else { -- cgit v1.1 From 31745aabcd6dce5583dbd0e5ddee93ff9fdfe3e6 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Thu, 5 Apr 2012 11:07:21 -0600 Subject: acpi_piix4: Remove PCI_RMV_BASE write code Clarify this register as read-only and remove write code. No change in existing behavior. Signed-off-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- docs/specs/acpi_pci_hotplug.txt | 2 +- hw/acpi_piix4.c | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt index 1e2c8a2..1883d63 100644 --- a/docs/specs/acpi_pci_hotplug.txt +++ b/docs/specs/acpi_pci_hotplug.txt @@ -34,4 +34,4 @@ PCI removability status (IO port 0xae0c-0xae0f, 4-byte access): ----------------------------------------------- Used by ACPI BIOS _RMV method to indicate removability status to OS. One -bit per slot. +bit per slot. Read-only diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 0e7af51..5d3b0ba 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -531,11 +531,6 @@ static uint32_t pcirmv_read(void *opaque, uint32_t addr) return s->pci0_hotplug_enable; } -static void pcirmv_write(void *opaque, uint32_t addr, uint32_t val) -{ - return; -} - static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, PCIHotplugState state); @@ -552,7 +547,6 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s); register_ioport_read(PCI_EJ_BASE, 4, 4, pciej_read, s); - register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s); register_ioport_read(PCI_RMV_BASE, 4, 4, pcirmv_read, s); pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev); -- cgit v1.1 From 9290f364c1f0c0a5a2ee8e03607f4804455c0d0e Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Thu, 5 Apr 2012 11:07:28 -0600 Subject: acpi_piix4: Re-define PCI hotplug eject register read The PCI hotplug eject register has always returned 0, so let's redefine it as a hotplug feature register. The existing model of using separate up & down read-only registers and an eject via write to this register becomes the base implementation. As we make use of new interfaces we'll set bits here to allow the BIOS and AML implementation to optimize for the platform implementation. Signed-off-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- docs/specs/acpi_pci_hotplug.txt | 12 ++++++++++-- hw/acpi_piix4.c | 7 ++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt index 1883d63..a839434 100644 --- a/docs/specs/acpi_pci_hotplug.txt +++ b/docs/specs/acpi_pci_hotplug.txt @@ -27,8 +27,16 @@ events. Read-only. PCI device eject (IO port 0xae08-0xae0b, 4-byte access): ---------------------------------------- -Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot. -Reads return 0. +Write: Used by ACPI BIOS _EJ0 method to request device removal. +One bit per slot. + +Read: Hotplug features register. Used by platform to identify features +available. Current base feature set (no bits set): + - Read-only "up" register @0xae00, 4-byte access, bit per slot + - Read-only "down" register @0xae04, 4-byte access, bit per slot + - Read/write "eject" register @0xae08, 4-byte access, + write: bit per slot eject, read: hotplug feature set + - Read-only hotplug capable register @0xae0c, 4-byte access, bit per slot PCI removability status (IO port 0xae0c-0xae0f, 4-byte access): ----------------------------------------------- diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 5d3b0ba..11c1f85 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -511,9 +511,10 @@ static uint32_t pci_down_read(void *opaque, uint32_t addr) return val; } -static uint32_t pciej_read(void *opaque, uint32_t addr) +static uint32_t pci_features_read(void *opaque, uint32_t addr) { - PIIX4_DPRINTF("pciej read %x\n", addr); + /* No feature defined yet */ + PIIX4_DPRINTF("pci_features_read %x\n", 0); return 0; } @@ -545,7 +546,7 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s) register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s); register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s); - register_ioport_read(PCI_EJ_BASE, 4, 4, pciej_read, s); + register_ioport_read(PCI_EJ_BASE, 4, 4, pci_features_read, s); register_ioport_read(PCI_RMV_BASE, 4, 4, pcirmv_read, s); -- cgit v1.1 From 54bfa546a0b5af335128ef5c477f8af9834df498 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 15 Apr 2012 12:00:52 +0300 Subject: acpi: explicitly account for >1 device per slot Slot present bit is cleared apparently for each device. Hotplug and non hotplug devices should not mix normally, and we only set the bit when we add a device so it should all work out, but it's more robust to explicitly account for more than one device per slot. Acked-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- hw/acpi_piix4.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 11c1f85..585da4e 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -287,6 +287,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots) DeviceState *qdev, *next; BusState *bus = qdev_get_parent_bus(&s->dev.qdev); int slot = ffs(slots) - 1; + bool slot_free = true; /* Mark request as complete */ s->pci0_status.down &= ~(1U << slot); @@ -294,11 +295,17 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots) QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) { PCIDevice *dev = PCI_DEVICE(qdev); PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); - if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) { - s->pci0_slot_device_present &= ~(1U << slot); - qdev_free(qdev); + if (PCI_SLOT(dev->devfn) == slot) { + if (pc->no_hotplug) { + slot_free = false; + } else { + qdev_free(qdev); + } } } + if (slot_free) { + s->pci0_slot_device_present &= ~(1U << slot); + } } static void piix4_update_hotplug(PIIX4PMState *s) -- cgit v1.1 From cdde6ffc27517bdf069734fbc5693ce2b14edc75 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Wed, 4 Jan 2012 16:28:42 +0200 Subject: pci: fix corrupted pci conf index register by unaligned write Commit d0ed8076cbdc261 converted the PCI config access to the memory API, but also inadvertantly changed it to accept unaligned writes, and corrupt the index register in the process. This causes a regression booting NetBSD. Fix by ignoring unaligned or non-dword writes. https://bugs.launchpad.net/qemu/+bug/897771 Reported-by: Andreas Gustafsson Signed-off-by: Avi Kivity Signed-off-by: Michael S. Tsirkin --- hw/pci_host.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/pci_host.c b/hw/pci_host.c index 44c6c20..8041778 100644 --- a/hw/pci_host.c +++ b/hw/pci_host.c @@ -101,6 +101,9 @@ static void pci_host_config_write(void *opaque, target_phys_addr_t addr, PCI_DPRINTF("%s addr " TARGET_FMT_plx " len %d val %"PRIx64"\n", __func__, addr, len, val); + if (addr != 0 || len != 4) { + return; + } s->config_reg = val; } -- cgit v1.1