From 1e40d19877031120631295348c26f43bea08afed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 Jun 2019 16:58:25 +0200 Subject: [PATCH 01/11] vhost-user-gpu: do not send scanout update if no GPU socket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should fix coverity CID 1401760. Signed-off-by: Marc-André Lureau Message-Id: <20190605145829.7674-2-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-gpu/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/vhost-user-gpu/main.c b/contrib/vhost-user-gpu/main.c index f9e2146b69..9614c9422c 100644 --- a/contrib/vhost-user-gpu/main.c +++ b/contrib/vhost-user-gpu/main.c @@ -354,7 +354,7 @@ vg_disable_scanout(VuGpu *g, int scanout_id) scanout->width = 0; scanout->height = 0; - { + if (g->sock_fd >= 0) { VhostUserGpuMsg msg = { .request = VHOST_USER_GPU_SCANOUT, .size = sizeof(VhostUserGpuScanout), From 24af03b946ce6889a435b2f3fc99daddde127874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 Jun 2019 16:58:26 +0200 Subject: [PATCH 02/11] vhost-user: check unix_listen() return value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This check shouldn't be necessary, since &error_fatal is given as argument and will exit() on failure. However, this change should silence coverity CID 1401761 & 1401705. Signed-off-by: Marc-André Lureau Message-Id: <20190605145829.7674-3-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-gpu/main.c | 4 ++++ contrib/vhost-user-input/main.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/contrib/vhost-user-gpu/main.c b/contrib/vhost-user-gpu/main.c index 9614c9422c..e0b6df5b4d 100644 --- a/contrib/vhost-user-gpu/main.c +++ b/contrib/vhost-user-gpu/main.c @@ -1160,6 +1160,10 @@ main(int argc, char *argv[]) if (opt_socket_path) { int lsock = unix_listen(opt_socket_path, &error_fatal); + if (lsock < 0) { + g_printerr("Failed to listen on %s.\n", opt_socket_path); + exit(EXIT_FAILURE); + } fd = accept(lsock, NULL, NULL); close(lsock); } else { diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c index 8d493f598e..8b854117f5 100644 --- a/contrib/vhost-user-input/main.c +++ b/contrib/vhost-user-input/main.c @@ -367,6 +367,10 @@ main(int argc, char *argv[]) if (opt_socket_path) { int lsock = unix_listen(opt_socket_path, &error_fatal); + if (lsock < 0) { + g_printerr("Failed to listen on %s.\n", opt_socket_path); + exit(EXIT_FAILURE); + } fd = accept(lsock, NULL, NULL); close(lsock); } else { From f55411cf143f868e70cd769171fa1db8953a626e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 Jun 2019 16:58:27 +0200 Subject: [PATCH 03/11] vhost-user: improve error report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit g_printerr() needs a trailing \n Signed-off-by: Marc-André Lureau Message-Id: <20190605145829.7674-4-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/vhost-user-gpu/main.c | 2 +- contrib/vhost-user-input/main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/vhost-user-gpu/main.c b/contrib/vhost-user-gpu/main.c index e0b6df5b4d..0ef649ffaa 100644 --- a/contrib/vhost-user-gpu/main.c +++ b/contrib/vhost-user-gpu/main.c @@ -1170,7 +1170,7 @@ main(int argc, char *argv[]) fd = opt_fdnum; } if (fd == -1) { - g_printerr("Invalid socket"); + g_printerr("Invalid vhost-user socket.\n"); exit(EXIT_FAILURE); } diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c index 8b854117f5..54f882602a 100644 --- a/contrib/vhost-user-input/main.c +++ b/contrib/vhost-user-input/main.c @@ -377,7 +377,7 @@ main(int argc, char *argv[]) fd = opt_fdnum; } if (fd == -1) { - g_printerr("Invalid socket"); + g_printerr("Invalid vhost-user socket.\n"); exit(EXIT_FAILURE); } vug_init(&vi.dev, fd, vi_panic, &vuiface); From be32fd9ee1d85e682d652d11b6e32e7f700420bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 Jun 2019 16:58:28 +0200 Subject: [PATCH 04/11] vhost-user-input: check ioctl(EVIOCGNAME) return value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should fix coverity CID 1401704. Signed-off-by: Marc-André Lureau Message-Id: <20190605145829.7674-5-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Peter Maydell --- contrib/vhost-user-input/main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c index 54f882602a..8b4e7d2536 100644 --- a/contrib/vhost-user-input/main.c +++ b/contrib/vhost-user-input/main.c @@ -342,7 +342,11 @@ main(int argc, char *argv[]) vi.config = g_array_new(false, false, sizeof(virtio_input_config)); memset(&id, 0, sizeof(id)); - ioctl(vi.evdevfd, EVIOCGNAME(sizeof(id.u.string) - 1), id.u.string); + if (ioctl(vi.evdevfd, EVIOCGNAME(sizeof(id.u.string) - 1), + id.u.string) < 0) { + g_printerr("Failed to get evdev name: %s\n", g_strerror(errno)); + exit(EXIT_FAILURE); + } id.select = VIRTIO_INPUT_CFG_ID_NAME; id.size = strlen(id.u.string); g_array_append_val(vi.config, id); From c715130a6495aeb015813e067f0cfb62f788a6e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 Jun 2019 16:58:29 +0200 Subject: [PATCH 05/11] vhost-user-gpu: initialize msghdr & iov at declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should fix uninitialized fields found by coverity CID 1401762. Signed-off-by: Marc-André Lureau Message-Id: <20190605145829.7674-6-marcandre.lureau@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Peter Maydell --- contrib/vhost-user-gpu/main.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/contrib/vhost-user-gpu/main.c b/contrib/vhost-user-gpu/main.c index 0ef649ffaa..04b753046f 100644 --- a/contrib/vhost-user-gpu/main.c +++ b/contrib/vhost-user-gpu/main.c @@ -138,22 +138,20 @@ static int vg_sock_fd_write(int sock, const void *buf, ssize_t buflen, int fd) { ssize_t ret; - struct msghdr msg; - struct iovec iov; + struct iovec iov = { + .iov_base = (void *)buf, + .iov_len = buflen, + }; + struct msghdr msg = { + .msg_iov = &iov, + .msg_iovlen = 1, + }; union { struct cmsghdr cmsghdr; char control[CMSG_SPACE(sizeof(int))]; } cmsgu; struct cmsghdr *cmsg; - iov.iov_base = (void *)buf; - iov.iov_len = buflen; - - msg.msg_name = NULL; - msg.msg_namelen = 0; - msg.msg_iov = &iov; - msg.msg_iovlen = 1; - if (fd != -1) { msg.msg_control = cmsgu.control; msg.msg_controllen = sizeof(cmsgu.control); @@ -164,9 +162,6 @@ vg_sock_fd_write(int sock, const void *buf, ssize_t buflen, int fd) cmsg->cmsg_type = SCM_RIGHTS; *((int *)CMSG_DATA(cmsg)) = fd; - } else { - msg.msg_control = NULL; - msg.msg_controllen = 0; } do { From 00ab8cb1411720d3aba4ef311651604452a9cdc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 Jun 2019 15:12:21 +0200 Subject: [PATCH 06/11] docs/vhost-user.json: some firmware.json copy leftovers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20190605131221.29432-1-marcandre.lureau@redhat.com> Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- docs/interop/vhost-user.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/interop/vhost-user.json b/docs/interop/vhost-user.json index ae88c03117..da6aaf51c8 100644 --- a/docs/interop/vhost-user.json +++ b/docs/interop/vhost-user.json @@ -178,11 +178,11 @@ # # - /usr/share/qemu/vhost-user/50-crosvm-gpu.json # -# then the sysadmin can prevent the default QEMU being used at all with +# then the sysadmin can prevent the default QEMU GPU being used at all with # # $ touch /etc/qemu/vhost-user/50-qemu-gpu.json # -# The sysadmin can replace/alter the distro default OVMF with +# The sysadmin can replace/alter the distro default QEMU GPU with # # $ vim /etc/qemu/vhost-user/50-qemu-gpu.json # @@ -190,7 +190,7 @@ # # $ vim /etc/qemu/vhost-user/10-qemu-gpu.json # -# or they can provide a parallel OVMF with lower priority +# or they can provide a parallel QEMU GPU with lower priority # # $ vim /etc/qemu/vhost-user/99-qemu-gpu.json # From 240e647a14df9677b3a501f7b8b870e40aac3fd5 Mon Sep 17 00:00:00 2001 From: Li Hangjing Date: Mon, 3 Jun 2019 14:15:24 +0800 Subject: [PATCH 07/11] vhost: fix vhost_log size overflow during migration When a guest which doesn't support multiqueue is migrated with a multi queues vhost-user-blk deivce, a crash will occur like: 0 qemu_memfd_alloc (name=, size=562949953421312, seals=, fd=0x7f87171fe8b4, errp=0x7f87171fe8a8) at util/memfd.c:153 1 0x00007f883559d7cf in vhost_log_alloc (size=70368744177664, share=true) at hw/virtio/vhost.c:186 2 0x00007f88355a0758 in vhost_log_get (listener=0x7f8838bd7940, enable=1) at qemu-2-12/hw/virtio/vhost.c:211 3 vhost_dev_log_resize (listener=0x7f8838bd7940, enable=1) at hw/virtio/vhost.c:263 4 vhost_migration_log (listener=0x7f8838bd7940, enable=1) at hw/virtio/vhost.c:787 5 0x00007f88355463d6 in memory_global_dirty_log_start () at memory.c:2503 6 0x00007f8835550577 in ram_init_bitmaps (f=0x7f88384ce600, opaque=0x7f8836024098) at migration/ram.c:2173 7 ram_init_all (f=0x7f88384ce600, opaque=0x7f8836024098) at migration/ram.c:2192 8 ram_save_setup (f=0x7f88384ce600, opaque=0x7f8836024098) at migration/ram.c:2219 9 0x00007f88357a419d in qemu_savevm_state_setup (f=0x7f88384ce600) at migration/savevm.c:1002 10 0x00007f883579fc3e in migration_thread (opaque=0x7f8837530400) at migration/migration.c:2382 11 0x00007f8832447893 in start_thread () from /lib64/libpthread.so.0 12 0x00007f8832178bfd in clone () from /lib64/libc.so.6 This is because vhost_get_log_size() returns a overflowed vhost-log size. In this function, it uses the uninitialized variable vqs->used_phys and vqs->used_size to get the vhost-log size. Signed-off-by: Li Hangjing Reviewed-by: Xie Yongji Reviewed-by: Chai Wen Message-Id: <20190603061524.24076-1-lihangjing@baidu.com> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 60747a6f93..bc899fc60e 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -131,6 +131,11 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, } for (i = 0; i < dev->nvqs; ++i) { struct vhost_virtqueue *vq = dev->vqs + i; + + if (!vq->used_phys && !vq->used_size) { + continue; + } + vhost_dev_sync_region(dev, section, start_addr, end_addr, vq->used_phys, range_get_last(vq->used_phys, vq->used_size)); } @@ -168,6 +173,11 @@ static uint64_t vhost_get_log_size(struct vhost_dev *dev) } for (i = 0; i < dev->nvqs; ++i) { struct vhost_virtqueue *vq = dev->vqs + i; + + if (!vq->used_phys && !vq->used_size) { + continue; + } + uint64_t last = vq->used_phys + vq->used_size - 1; log_size = MAX(log_size, last / VHOST_LOG_CHUNK + 1); } From 82f76c6702e6d376ff5cf0326ea2e30f1e514e8e Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Mon, 10 Jun 2019 09:18:30 +0800 Subject: [PATCH 08/11] hw/acpi: extract acpi_add_rom_blob() arm and i386 has almost the same function acpi_add_rom_blob(), except giving different FWCfgCallback function. This patch moves acpi_add_rom_blob() to utils.c by passing FWCfgCallback to it. Signed-off-by: Wei Yang Reviewed-by: Igor Mammedov v7: * rebase on top of current master because of conflict v6: * change author from Igor to Michael v5: * remove unnecessary header glib/gprintf.h * rearrange include header to make it more suitable v4: * extract -> moves * adjust comment in source to make checkpatch happy v3: * put acpi_add_rom_blob() to hw/acpi/utils.c v2: * remove unused header in original source file Message-Id: <20190610011830.28398-1-richardw.yang@linux.intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/Makefile.objs | 2 +- hw/acpi/utils.c | 35 +++++++++++++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 26 ++++++++++---------------- hw/i386/acpi-build.c | 26 ++++++++++---------------- include/hw/acpi/utils.h | 9 +++++++++ 5 files changed, 65 insertions(+), 33 deletions(-) create mode 100644 hw/acpi/utils.c create mode 100644 include/hw/acpi/utils.h diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index 661a9b8c2f..9bb2101e3b 100644 --- a/hw/acpi/Makefile.objs +++ b/hw/acpi/Makefile.objs @@ -10,7 +10,7 @@ common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o common-obj-y += acpi_interface.o common-obj-y += bios-linker-loader.o -common-obj-y += aml-build.o +common-obj-y += aml-build.o utils.o common-obj-$(CONFIG_ACPI_PCI) += pci.o common-obj-$(CONFIG_TPM) += tpm.o diff --git a/hw/acpi/utils.c b/hw/acpi/utils.c new file mode 100644 index 0000000000..a134a4d554 --- /dev/null +++ b/hw/acpi/utils.c @@ -0,0 +1,35 @@ +/* + * Utilities for generating ACPI tables and passing them to Guests + * + * Copyright (C) 2019 Intel Corporation + * Copyright (C) 2019 Red Hat Inc + * + * Author: Wei Yang + * Author: Michael S. Tsirkin + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License along + * with this program; if not, see . + */ + +#include "qemu/osdep.h" +#include "hw/acpi/aml-build.h" +#include "hw/acpi/utils.h" +#include "hw/loader.h" + +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque, + GArray *blob, const char *name, + uint64_t max_size) +{ + return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, + name, update, opaque, NULL, true); +} diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 9e8b84988d..0afb372769 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -36,9 +36,9 @@ #include "hw/acpi/acpi.h" #include "hw/nvram/fw_cfg.h" #include "hw/acpi/bios-linker-loader.h" -#include "hw/loader.h" #include "hw/hw.h" #include "hw/acpi/aml-build.h" +#include "hw/acpi/utils.h" #include "hw/acpi/pci.h" #include "hw/pci/pcie_host.h" #include "hw/pci/pci.h" @@ -865,14 +865,6 @@ static void virt_acpi_build_reset(void *build_opaque) build_state->patched = false; } -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state, - GArray *blob, const char *name, - uint64_t max_size) -{ - return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, - name, virt_acpi_build_update, build_state, NULL, true); -} - static const VMStateDescription vmstate_virt_acpi_build = { .name = "virt_acpi_build", .version_id = 1, @@ -904,20 +896,22 @@ void virt_acpi_setup(VirtMachineState *vms) virt_acpi_build(vms, &tables); /* Now expose it all to Guest */ - build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data, - ACPI_BUILD_TABLE_FILE, - ACPI_BUILD_TABLE_MAX_SIZE); + build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update, + build_state, tables.table_data, + ACPI_BUILD_TABLE_FILE, + ACPI_BUILD_TABLE_MAX_SIZE); assert(build_state->table_mr != NULL); build_state->linker_mr = - acpi_add_rom_blob(build_state, tables.linker->cmd_blob, - "etc/table-loader", 0); + acpi_add_rom_blob(virt_acpi_build_update, build_state, + tables.linker->cmd_blob, "etc/table-loader", 0); fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data, acpi_data_len(tables.tcpalog)); - build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp, - ACPI_BUILD_RSDP_FILE, 0); + build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update, + build_state, tables.rsdp, + ACPI_BUILD_RSDP_FILE, 0); qemu_register_reset(virt_acpi_build_reset, build_state); virt_acpi_build_reset(build_state); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 45c369c923..f2db7c1e22 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -37,7 +37,6 @@ #include "hw/acpi/piix4.h" #include "hw/nvram/fw_cfg.h" #include "hw/acpi/bios-linker-loader.h" -#include "hw/loader.h" #include "hw/isa/isa.h" #include "hw/block/fdc.h" #include "hw/acpi/memory_hotplug.h" @@ -58,6 +57,7 @@ #include "hw/i386/x86-iommu.h" #include "hw/acpi/aml-build.h" +#include "hw/acpi/utils.h" #include "hw/acpi/pci.h" #include "qom/qom-qobject.h" @@ -2823,14 +2823,6 @@ static void acpi_build_reset(void *build_opaque) build_state->patched = 0; } -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state, - GArray *blob, const char *name, - uint64_t max_size) -{ - return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, - name, acpi_build_update, build_state, NULL, true); -} - static const VMStateDescription vmstate_acpi_build = { .name = "acpi_build", .version_id = 1, @@ -2872,14 +2864,15 @@ void acpi_setup(void) acpi_build(&tables, MACHINE(pcms)); /* Now expose it all to Guest */ - build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data, - ACPI_BUILD_TABLE_FILE, - ACPI_BUILD_TABLE_MAX_SIZE); + build_state->table_mr = acpi_add_rom_blob(acpi_build_update, + build_state, tables.table_data, + ACPI_BUILD_TABLE_FILE, + ACPI_BUILD_TABLE_MAX_SIZE); assert(build_state->table_mr != NULL); build_state->linker_mr = - acpi_add_rom_blob(build_state, tables.linker->cmd_blob, - "etc/table-loader", 0); + acpi_add_rom_blob(acpi_build_update, build_state, + tables.linker->cmd_blob, "etc/table-loader", 0); fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data, acpi_data_len(tables.tcpalog)); @@ -2916,8 +2909,9 @@ void acpi_setup(void) build_state->rsdp_mr = NULL; } else { build_state->rsdp = NULL; - build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp, - ACPI_BUILD_RSDP_FILE, 0); + build_state->rsdp_mr = acpi_add_rom_blob(acpi_build_update, + build_state, tables.rsdp, + ACPI_BUILD_RSDP_FILE, 0); } qemu_register_reset(acpi_build_reset, build_state); diff --git a/include/hw/acpi/utils.h b/include/hw/acpi/utils.h new file mode 100644 index 0000000000..140b4de603 --- /dev/null +++ b/include/hw/acpi/utils.h @@ -0,0 +1,9 @@ +#ifndef HW_ACPI_UTILS_H +#define HW_ACPI_UTILS_H + +#include "hw/nvram/fw_cfg.h" + +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque, + GArray *blob, const char *name, + uint64_t max_size); +#endif From 4a4418369d6dca4ffa88126413ead743d3841666 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 7 Jun 2019 09:34:29 +0200 Subject: [PATCH 09/11] q35: fix mmconfig and PCI0._CRS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch changes the handling of the mmconfig area. Thanks to the pci(e) expander devices we already have the logic to exclude address ranges from PCI0._CRS. We can simply add the mmconfig address range to the list get it excluded as well. With that in place we can go with a fixed pci hole which covers the whole area from the end of (low) ram to the ioapic. This will make the whole logic alot less fragile. No matter where the firmware places the mmconfig xbar, things should work correctly. The guest also gets a bit more PCI address space (seabios boot): # cat /proc/iomem [ ... ] 7ffdd000-7fffffff : reserved 80000000-afffffff : PCI Bus 0000:00 <<-- this is new b0000000-bfffffff : PCI MMCONFIG 0000 [bus 00-ff] b0000000-bfffffff : reserved c0000000-febfffff : PCI Bus 0000:00 f8000000-fbffffff : 0000:00:01.0 [ ... ] So this is a guest visible change. Cc: László Érsek Signed-off-by: Gerd Hoffmann Reviewed-by: Igor Mammedov Message-Id: <20190607073429.3436-1-kraxel@redhat.com> --- hw/i386/acpi-build.c | 14 ++++++++++++ hw/pci-host/q35.c | 31 +++++++-------------------- tests/bios-tables-test-allowed-diff.h | 8 +++++++ 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f2db7c1e22..31a1c1e3ad 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -121,6 +121,8 @@ typedef struct FwCfgTPMConfig { uint8_t tpmppi_version; } QEMU_PACKED FwCfgTPMConfig; +static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg); + static void init_common_fadt_data(Object *o, AcpiFadtData *data) { uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL); @@ -1806,6 +1808,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, CrsRangeSet crs_range_set; PCMachineState *pcms = PC_MACHINE(machine); PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine); + AcpiMcfgInfo mcfg; uint32_t nr_mem = machine->ram_slots; int root_bus_limit = 0xFF; PCIBus *bus = NULL; @@ -1920,6 +1923,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, } } + /* + * At this point crs_range_set has all the ranges used by pci + * busses *other* than PCI0. These ranges will be excluded from + * the PCI0._CRS. Add mmconfig to the set so it will be excluded + * too. + */ + if (acpi_get_mcfg(&mcfg)) { + crs_range_insert(crs_range_set.mem_ranges, + mcfg.base, mcfg.base + mcfg.size - 1); + } + scope = aml_scope("\\_SB.PCI0"); /* build PCI0._CRS */ crs = aml_resource_template(); diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 172b0bc435..0a010be4cf 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -260,15 +260,6 @@ static void q35_host_initfn(Object *obj) object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION, (Object **) &s->mch.address_space_io, qdev_prop_allow_set_link_before_realize, 0, NULL); - - /* Leave enough space for the biggest MCFG BAR */ - /* TODO: this matches current bios behaviour, but - * it's not a power of two, which means an MTRR - * can't cover it exactly. - */ - range_set_bounds(&s->mch.pci_hole, - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX, - IO_APIC_DEFAULT_ADDRESS - 1); } static const TypeInfo q35_host_info = { @@ -340,20 +331,6 @@ static void mch_update_pciexbar(MCHPCIState *mch) } addr = pciexbar & addr_mask; pcie_host_mmcfg_update(pehb, enable, addr, length); - /* Leave enough space for the MCFG BAR */ - /* - * TODO: this matches current bios behaviour, but it's not a power of two, - * which means an MTRR can't cover it exactly. - */ - if (enable) { - range_set_bounds(&mch->pci_hole, - addr + length, - IO_APIC_DEFAULT_ADDRESS - 1); - } else { - range_set_bounds(&mch->pci_hole, - MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT, - IO_APIC_DEFAULT_ADDRESS - 1); - } } /* PAM */ @@ -486,6 +463,14 @@ static void mch_update(MCHPCIState *mch) mch_update_pam(mch); mch_update_smram(mch); mch_update_ext_tseg_mbytes(mch); + + /* + * pci hole goes from end-of-low-ram to io-apic. + * mmconfig will be excluded by the dsdt builder. + */ + range_set_bounds(&mch->pci_hole, + mch->below_4g_mem_size, + IO_APIC_DEFAULT_ADDRESS - 1); } static int mch_post_load(void *opaque, int version_id) diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h index dfb8523c8b..3bbd22c62a 100644 --- a/tests/bios-tables-test-allowed-diff.h +++ b/tests/bios-tables-test-allowed-diff.h @@ -1 +1,9 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/q35/DSDT", +"tests/data/acpi/q35/DSDT.bridge", +"tests/data/acpi/q35/DSDT.mmio64", +"tests/data/acpi/q35/DSDT.ipmibt", +"tests/data/acpi/q35/DSDT.cphp", +"tests/data/acpi/q35/DSDT.memhp", +"tests/data/acpi/q35/DSDT.numamem", +"tests/data/acpi/q35/DSDT.dimmpxm", From 500eb6db5b5baef3f642415cdeb4b3e728537f81 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 16 Jun 2019 16:33:14 -0400 Subject: [PATCH 10/11] q35: update DSDT update expected files and drop them from allowed diff list. Fixes: 4a4418369d6 ("q35: fix mmconfig and PCI0._CRS") Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test-allowed-diff.h | 8 -------- tests/data/acpi/q35/DSDT | Bin 7815 -> 7841 bytes tests/data/acpi/q35/DSDT.bridge | Bin 7832 -> 7858 bytes tests/data/acpi/q35/DSDT.cphp | Bin 8278 -> 8304 bytes tests/data/acpi/q35/DSDT.dimmpxm | Bin 9468 -> 9494 bytes tests/data/acpi/q35/DSDT.ipmibt | Bin 7890 -> 7916 bytes tests/data/acpi/q35/DSDT.memhp | Bin 9174 -> 9200 bytes tests/data/acpi/q35/DSDT.mmio64 | Bin 8945 -> 8971 bytes tests/data/acpi/q35/DSDT.numamem | Bin 7821 -> 7847 bytes 9 files changed, 8 deletions(-) diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h index 3bbd22c62a..dfb8523c8b 100644 --- a/tests/bios-tables-test-allowed-diff.h +++ b/tests/bios-tables-test-allowed-diff.h @@ -1,9 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/q35/DSDT", -"tests/data/acpi/q35/DSDT.bridge", -"tests/data/acpi/q35/DSDT.mmio64", -"tests/data/acpi/q35/DSDT.ipmibt", -"tests/data/acpi/q35/DSDT.cphp", -"tests/data/acpi/q35/DSDT.memhp", -"tests/data/acpi/q35/DSDT.numamem", -"tests/data/acpi/q35/DSDT.dimmpxm", diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT index 7576ffcd05991ad5a3901c0f7698a52fffc6d6e2..f9f36d1645c9b57aea38350d67dfaa143845697d 100644 GIT binary patch delta 65 zcmZp-U1-bY66_MPP>z9t(QYHxMHw+i!5F>xV5j&1XHNr;c;}#CK`(BuZIeIB#Pf3e R|NnnI0|czt>@4fT2mo%L69@nR delta 51 zcmZ2z+iuI{66_MvF2}&Y*szi7qKue3e~eyyuv2`1v!?+^ymL^npaU1zoXH<$;x}i> Hnll0bdb|$6 diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge index c623cc5d72a2e346793fa9128e7e88b6781241b2..29176832ca9842c6654273ae1246321aa38b2821 100644 GIT binary patch delta 65 zcmbPXyUCWzCD R|NsB>3=ptlv$Jd{BLJG%6YKy0 delta 51 zcmdmFJHwXCCD%j;Bc5Du$ diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp index 7ac526e4669fd84048b2d8ec6af8661503e1a9fa..19bdb5d21050f24aaacbafb1f84d6e1d541876c6 100644 GIT binary patch delta 65 zcmccS@WFx0CDZo3farV2oaTuv2`1v!?+^ymL^npcgmSw#jMA@w^=W R|Nmdl00ApD?@)GO1OQ#b61)Ha delta 51 zcmbQ{^~aOTCDxV5j&1XHNr;c;}#CK`(BuZIf+e<9Rv$ R|Np<90RmQR?vr(41OR*267c{4 delta 51 zcmaE3d&!o|CDGO}Xs{4sj*!A|i3&YlJw@y@!5F>xV5j&1XHNr;c;}#CK`(BuZId08;(0m# R|Np<90RmQRo}}c$2mpN;68-=H delta 51 zcmez1e$AcBCD`3 diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64 index f60ee77fb4d655e77c7ef1e8c205d741cc288939..20f627ed08a0cae4e144f3e4dd7dd5f1d8d0318c 100644 GIT binary patch delta 65 zcmez9+U>^W66_Mft<1o{_H$66_N4QHgoQ`Ff-!pW!A|i3&YlJw@y Date: Sun, 16 Jun 2019 16:43:03 -0400 Subject: [PATCH 11/11] tests/rebuild-expected-aml.sh: blow out difflist As expected files have been updated, make sure we do not forget to remove them from the allowed diff list. Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/rebuild-expected-aml.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/data/acpi/rebuild-expected-aml.sh b/tests/data/acpi/rebuild-expected-aml.sh index d2853218dd..f89d4624bc 100755 --- a/tests/data/acpi/rebuild-expected-aml.sh +++ b/tests/data/acpi/rebuild-expected-aml.sh @@ -29,5 +29,8 @@ for qemu in $qemu_bins; do TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu tests/bios-tables-test done +eval `grep SRC_PATH= config-host.mak` + +echo '/* List of comma-separated changed AML files to ignore */' > ${SRC_PATH}/tests/bios-tables-test-allowed-diff.h echo "The files were rebuilt and can be added to git."