From 8e2e95ef04a8d40d59d1d735e3ebd48324c27417 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 11 Jul 2019 15:24:18 -0400 Subject: [PATCH 1/8] xio3130_downstream: typo fix slt ctl/status are passed in incorrect order. Fix this up. Signed-off-by: Michael S. Tsirkin Reported-by: Peter Maydell Reviewed-by: Marcel Apfelbaum --- hw/pci-bridge/xio3130_downstream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c index 899b0fd6c9..182e164f74 100644 --- a/hw/pci-bridge/xio3130_downstream.c +++ b/hw/pci-bridge/xio3130_downstream.c @@ -43,7 +43,7 @@ static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address, { uint16_t slt_ctl, slt_sta; - pcie_cap_slot_get(d, &slt_sta, &slt_ctl); + pcie_cap_slot_get(d, &slt_ctl, &slt_sta); pci_bridge_write_config(d, address, val, len); pcie_cap_flr_write_config(d, address, val, len); pcie_cap_slot_write_config(d, slt_ctl, slt_sta, address, val, len); From 4d90b7a0e4aa8051d4a059d49e458e3378aa39ff Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 11 Jul 2019 15:25:50 -0400 Subject: [PATCH 2/8] pcie: consistent names for function args The function declarations for pci_cap_slot_get and pci_cap_slot_write_config call the argument "slot_ctl", but the function definitions and all the call sites drop the 'o' and call it "slt_ctl". Let's be consistent. Reported-by: Peter Maydell Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum --- include/hw/pci/pcie.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h index 34f277735c..8cf3361fc4 100644 --- a/include/hw/pci/pcie.h +++ b/include/hw/pci/pcie.h @@ -107,9 +107,9 @@ void pcie_cap_lnkctl_reset(PCIDevice *dev); void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot); void pcie_cap_slot_reset(PCIDevice *dev); -void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slot_ctl, uint16_t *slt_sta); +void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta); void pcie_cap_slot_write_config(PCIDevice *dev, - uint16_t old_slot_ctl, uint16_t old_slt_sta, + uint16_t old_slt_ctl, uint16_t old_slt_sta, uint32_t addr, uint32_t val, int len); int pcie_cap_slot_post_load(void *opaque, int version_id); void pcie_cap_slot_push_attention_button(PCIDevice *dev); From 2bbadb08ce272d65e1f78621002008b07d1e0f03 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 10 Jul 2019 16:14:40 +0200 Subject: [PATCH 3/8] virtio-balloon: fix QEMU 4.0 config size migration incompatibility The virtio-balloon config size changed in QEMU 4.0 even for existing machine types. Migration from QEMU 3.1 to 4.0 can fail in some circumstances with the following error: qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: a1 device: 1 cmask: ff wmask: c0 w1cmask:0 This happens because the virtio-balloon config size affects the VIRTIO Legacy I/O Memory PCI BAR size. Introduce a qdev property called "qemu-4-0-config-size" and enable it only for the QEMU 4.0 machine types. This way <4.0 machine types use the old size, 4.0 uses the larger size, and >4.0 machine types use the appropriate size depending on enabled virtio-balloon features. Live migration to and from old QEMUs to QEMU 4.1 works again as long as a versioned machine type is specified (do not use just "pc"!). Originally-by: Wolfgang Bumiller Signed-off-by: Stefan Hajnoczi Message-Id: <20190710141440.27635-1-stefanha@redhat.com> Reviewed-by: Dr. David Alan Gilbert Tested-by: Dr. David Alan Gilbert Tested-by: Wolfgang Bumiller Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/core/machine.c | 2 ++ hw/virtio/virtio-balloon.c | 28 +++++++++++++++++++++++++--- include/hw/virtio/virtio-balloon.h | 2 ++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 2be19ec0cd..c4ead16010 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -34,6 +34,7 @@ GlobalProperty hw_compat_4_0[] = { { "virtio-vga", "edid", "false" }, { "virtio-gpu-pci", "edid", "false" }, { "virtio-device", "use-started", "false" }, + { "virtio-balloon-device", "qemu-4-0-config-size", "true" }, }; const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0); @@ -49,6 +50,7 @@ GlobalProperty hw_compat_3_1[] = { { "usb-tablet", "serial", "42" }, { "virtio-blk-device", "discard", "false" }, { "virtio-blk-device", "write-zeroes", "false" }, + { "virtio-balloon-device", "qemu-4-0-config-size", "false" }, }; const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1); diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 11fad86d64..e85d1c0d5c 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -615,6 +615,22 @@ virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data) return 0; } +static size_t virtio_balloon_config_size(VirtIOBalloon *s) +{ + uint64_t features = s->host_features; + + if (s->qemu_4_0_config_size) { + return sizeof(struct virtio_balloon_config); + } + if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) { + return sizeof(struct virtio_balloon_config); + } + if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { + return offsetof(struct virtio_balloon_config, poison_val); + } + return offsetof(struct virtio_balloon_config, free_page_report_cmd_id); +} + static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) { VirtIOBalloon *dev = VIRTIO_BALLOON(vdev); @@ -635,7 +651,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) } trace_virtio_balloon_get_config(config.num_pages, config.actual); - memcpy(config_data, &config, sizeof(struct virtio_balloon_config)); + memcpy(config_data, &config, virtio_balloon_config_size(dev)); } static int build_dimm_list(Object *obj, void *opaque) @@ -679,7 +695,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, uint32_t oldactual = dev->actual; ram_addr_t vm_ram_size = get_current_ram_size(); - memcpy(&config, config_data, sizeof(struct virtio_balloon_config)); + memcpy(&config, config_data, virtio_balloon_config_size(dev)); dev->actual = le32_to_cpu(config.actual); if (dev->actual != oldactual) { qapi_event_send_balloon_change(vm_ram_size - @@ -766,7 +782,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp) int ret; virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON, - sizeof(struct virtio_balloon_config)); + virtio_balloon_config_size(s)); ret = qemu_add_balloon_handler(virtio_balloon_to_target, virtio_balloon_stat, s); @@ -897,6 +913,12 @@ static Property virtio_balloon_properties[] = { VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false), DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features, VIRTIO_BALLOON_F_FREE_PAGE_HINT, false), + /* QEMU 4.0 accidentally changed the config size even when free-page-hint + * is disabled, resulting in QEMU 3.1 migration incompatibility. This + * property retains this quirk for QEMU 4.1 machine types. + */ + DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon, + qemu_4_0_config_size, false), DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD, IOThread *), DEFINE_PROP_END_OF_LIST(), diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h index 1afafb12f6..5a99293a45 100644 --- a/include/hw/virtio/virtio-balloon.h +++ b/include/hw/virtio/virtio-balloon.h @@ -71,6 +71,8 @@ typedef struct VirtIOBalloon { int64_t stats_poll_interval; uint32_t host_features; PartiallyBalloonedPage *pbp; + + bool qemu_4_0_config_size; } VirtIOBalloon; #endif From ab31b3373eba7e186282aac9f05c4303c309ac27 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 8 Jul 2019 05:24:09 -0400 Subject: [PATCH 4/8] tests: acpi: do not require IASL for dumping AML blobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IASL isn't needed when dumping ACPI tables from guest for rebuild purposes. So move this part out from IASL branch. Makes rebuild-expected-aml.sh work without IASL installed on host. Signed-off-by: Igor Mammedov Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Eric Auger Message-Id: <20190708092410.11167-2-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index d863233fe9..13bd166b81 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -597,12 +597,10 @@ static void test_acpi_one(const char *params, test_data *data) test_acpi_rxsdt_table(data); test_acpi_fadt_table(data); - if (iasl) { - if (getenv(ACPI_REBUILD_EXPECTED_AML)) { - dump_aml_files(data, true); - } else { - test_acpi_asl(data); - } + if (getenv(ACPI_REBUILD_EXPECTED_AML)) { + dump_aml_files(data, true); + } else if (iasl) { + test_acpi_asl(data); } /* From 7b9829bc6c7d84056c3a1c60737f71db4d2871e4 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 8 Jul 2019 05:24:10 -0400 Subject: [PATCH 5/8] tests: acpi: do not skip tests when IASL is not installed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests do binary comparision so we can check tables without IASL. Move IASL condition right before decompilation step and skip it if IASL is not installed. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20190708092410.11167-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 13bd166b81..a356ac3489 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data) all_tables_match = all_tables_match && test_acpi_find_diff_allowed(exp_sdt); + /* + * don't try to decompile if IASL isn't present, in this case user + * will just 'get binary file mismatch' warnings and test failure + */ + if (!iasl) { + continue; + } + err = load_asl(data->tables, sdt); asl = normalize_asl(sdt->asl); @@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data) g_string_free(asl, true); g_string_free(exp_asl, true); } + if (!iasl && !all_tables_match) { + fprintf(stderr, "to see ASL diff between mismatched files install IASL," + " rebuild QEMU from scratch and re-run tests with V=1" + " environment variable set"); + } g_assert(all_tables_match); free_test_data(&exp_data); @@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data) if (getenv(ACPI_REBUILD_EXPECTED_AML)) { dump_aml_files(data, true); - } else if (iasl) { + } else { test_acpi_asl(data); } From f37f47126236c8dc710d97da8ca9d43c38bef4ec Mon Sep 17 00:00:00 2001 From: Pankaj Gupta Date: Fri, 12 Jul 2019 13:05:52 +0530 Subject: [PATCH 6/8] virtio pmem: fix wrong mem region condition Coverity reported memory region returns zero for non-null value. This is because of wrong arguments to '?:' , fixing this. Signed-off-by: Pankaj Gupta Message-Id: <20190712073554.21918-2-pagupta@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefano Garzarella Reviewed-by: Cornelia Huck --- hw/virtio/virtio-pmem-pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c index 8b2d0dbccc..0da6627469 100644 --- a/hw/virtio/virtio-pmem-pci.c +++ b/hw/virtio/virtio-pmem-pci.c @@ -57,7 +57,7 @@ static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState *md, MemoryRegion *mr = vpc->get_memory_region(pmem, errp); /* the plugged size corresponds to the region size */ - return mr ? 0 : memory_region_size(mr); + return mr ? memory_region_size(mr) : 0; } static void virtio_pmem_pci_fill_device_info(const MemoryDeviceState *md, From 7b8a8474249eec97cd6a82f78b91183626fda65b Mon Sep 17 00:00:00 2001 From: Pankaj Gupta Date: Fri, 12 Jul 2019 13:05:53 +0530 Subject: [PATCH 7/8] virtio pmem: remove memdev null check Coverity reports that when we're assigning vi->size we handle the "pmem->memdev is NULL" case; but we then pass it into object_get_canonical_path(), which unconditionally dereferences it and will crash if it is NULL. If this pointer can be NULL then we need to do something else here. We are removing 'pmem->memdev' null check here as memdev will never be null in this function. Signed-off-by: Pankaj Gupta Message-Id: <20190712073554.21918-3-pagupta@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Cornelia Huck --- hw/virtio/virtio-pmem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-pmem.c b/hw/virtio/virtio-pmem.c index adbfb603ab..17c196d107 100644 --- a/hw/virtio/virtio-pmem.c +++ b/hw/virtio/virtio-pmem.c @@ -134,8 +134,8 @@ static void virtio_pmem_fill_device_info(const VirtIOPMEM *pmem, VirtioPMEMDeviceInfo *vi) { vi->memaddr = pmem->start; - vi->size = pmem->memdev ? memory_region_size(&pmem->memdev->mr) : 0; - vi->memdev = object_get_canonical_path(OBJECT(pmem->memdev)); + vi->size = memory_region_size(&pmem->memdev->mr); + vi->memdev = object_get_canonical_path(OBJECT(pmem->memdev)); } static MemoryRegion *virtio_pmem_get_memory_region(VirtIOPMEM *pmem, From 207efa18ac9eb7085a44cad24489d0da54bc5f65 Mon Sep 17 00:00:00 2001 From: Pankaj Gupta Date: Fri, 12 Jul 2019 13:05:54 +0530 Subject: [PATCH 8/8] virtio pmem: remove transitional names Remove transitional & non transitional names for virtio pmem. Only virtio 1.0 and up is supported. Signed-off-by: Pankaj Gupta Message-Id: <20190712073554.21918-4-pagupta@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Cornelia Huck --- hw/virtio/virtio-pmem-pci.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/virtio/virtio-pmem-pci.c b/hw/virtio/virtio-pmem-pci.c index 0da6627469..fe2af00fa1 100644 --- a/hw/virtio/virtio-pmem-pci.c +++ b/hw/virtio/virtio-pmem-pci.c @@ -113,8 +113,6 @@ static void virtio_pmem_pci_instance_init(Object *obj) static const VirtioPCIDeviceTypeInfo virtio_pmem_pci_info = { .base_name = TYPE_VIRTIO_PMEM_PCI, .generic_name = "virtio-pmem-pci", - .transitional_name = "virtio-pmem-pci-transitional", - .non_transitional_name = "virtio-pmem-pci-non-transitional", .instance_size = sizeof(VirtIOPMEMPCI), .instance_init = virtio_pmem_pci_instance_init, .class_init = virtio_pmem_pci_class_init,