From f13a944ca6d50efa1dc4cca3a31262b677a2a715 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Tue, 21 May 2019 14:28:35 +0800 Subject: [PATCH 01/17] hw/acpi: Consolidate build_mcfg to pci.c Now we have two identical build_mcfg functions. Consolidate them in acpi/pci.c. Signed-off-by: Wei Yang v4: * ACPI_PCI depends on both ACPI and PCI * rebase on latest master, adjust arm Kconfig v3: * adjust changelog based on Igor's suggestion Message-Id: <20190521062836.6541-2-richardw.yang@linux.intel.com> Reviewed-by: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- default-configs/i386-softmmu.mak | 1 + hw/acpi/Kconfig | 4 +++ hw/acpi/Makefile.objs | 1 + hw/acpi/pci.c | 46 ++++++++++++++++++++++++++++++++ hw/arm/Kconfig | 1 + hw/arm/virt-acpi-build.c | 17 ------------ hw/i386/acpi-build.c | 18 +------------ include/hw/acpi/pci.h | 1 + 8 files changed, 55 insertions(+), 34 deletions(-) create mode 100644 hw/acpi/pci.c diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index ba3fb3ff50..cd5ea391e8 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -25,3 +25,4 @@ CONFIG_ISAPC=y CONFIG_I440FX=y CONFIG_Q35=y +CONFIG_ACPI_PCI=y diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig index eca3beed75..7c59cf900b 100644 --- a/hw/acpi/Kconfig +++ b/hw/acpi/Kconfig @@ -23,6 +23,10 @@ config ACPI_NVDIMM bool depends on ACPI +config ACPI_PCI + bool + depends on ACPI && PCI + config ACPI_VMGENID bool default y diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index 2d46e3789a..661a9b8c2f 100644 --- a/hw/acpi/Makefile.objs +++ b/hw/acpi/Makefile.objs @@ -11,6 +11,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-$(CONFIG_ACPI_PCI) += pci.o common-obj-$(CONFIG_TPM) += tpm.o common-obj-$(CONFIG_IPMI) += ipmi.o diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c new file mode 100644 index 0000000000..fa0fa30bb9 --- /dev/null +++ b/hw/acpi/pci.c @@ -0,0 +1,46 @@ +/* + * Support for generating PCI related ACPI tables and passing them to Guests + * + * Copyright (C) 2006 Fabrice Bellard + * Copyright (C) 2008-2010 Kevin O'Connor + * Copyright (C) 2013-2019 Red Hat Inc + * Copyright (C) 2019 Intel Corporation + * + * 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/pci.h" +#include "hw/pci/pcie_host.h" + +void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) +{ + AcpiTableMcfg *mcfg; + int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]); + + mcfg = acpi_data_push(table_data, len); + mcfg->allocation[0].address = cpu_to_le64(info->base); + + /* Only a single allocation so no need to play with segments */ + mcfg->allocation[0].pci_segment = cpu_to_le16(0); + mcfg->allocation[0].start_bus_number = 0; + mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1); + + build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL); +} + diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index af8cffde9c..9aced9d54d 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -19,6 +19,7 @@ config ARM_VIRT select PLATFORM_BUS select SMBIOS select VIRTIO_MMIO + select ACPI_PCI config CHEETAH bool diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index e7c96d658e..4a64f9985c 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -546,23 +546,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) "SRAT", table_data->len - srat_start, 3, NULL, NULL); } -static void -build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) -{ - AcpiTableMcfg *mcfg; - int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]); - - mcfg = acpi_data_push(table_data, len); - mcfg->allocation[0].address = cpu_to_le64(info->base); - - /* Only a single allocation so no need to play with segments */ - mcfg->allocation[0].pci_segment = cpu_to_le16(0); - mcfg->allocation[0].start_bus_number = 0; - mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1); - - build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL); -} - /* GTDT */ static void build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 0d78d73894..85dc1640bc 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2405,22 +2405,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) table_data->len - srat_start, 1, NULL, NULL); } -static void -build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) -{ - AcpiTableMcfg *mcfg; - int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]); - - mcfg = acpi_data_push(table_data, len); - mcfg->allocation[0].address = cpu_to_le64(info->base); - /* Only a single allocation so no need to play with segments */ - mcfg->allocation[0].pci_segment = cpu_to_le16(0); - mcfg->allocation[0].start_bus_number = 0; - mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1); - - build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL); -} - /* * VT-d spec 8.1 DMA Remapping Reporting Structure * (version Oct. 2014 or later) @@ -2690,7 +2674,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine) } if (acpi_get_mcfg(&mcfg)) { acpi_add_table(table_offsets, tables_blob); - build_mcfg_q35(tables_blob, tables->linker, &mcfg); + build_mcfg(tables_blob, tables->linker, &mcfg); } if (x86_iommu_get_default()) { IommuType IOMMUType = x86_iommu_get_type(); diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h index 124af7d32a..8bbd32cf45 100644 --- a/include/hw/acpi/pci.h +++ b/include/hw/acpi/pci.h @@ -30,4 +30,5 @@ typedef struct AcpiMcfgInfo { uint32_t size; } AcpiMcfgInfo; +void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info); #endif From e4610781635404d494120c19bade8dad6f00f0b3 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Tue, 21 May 2019 14:28:36 +0800 Subject: [PATCH 02/17] acpi: pci: use build_append_foo() API to construct MCFG build_append_foo() API doesn't need explicit endianness conversions which eliminates a source of errors and it makes build_mcfg() look like declarative definition of MCFG table in ACPI spec, which makes it easy to review. Signed-off-by: Wei Yang Suggested-by: Igor Mammedov Reviewed-by: Igor Mammedov v3: * add some comment on the Configuration Space base address allocation structure v2: * miss the reserved[8] of MCFG in last version, add it back * drop SOBs and make sure bios-tables-test all OK Message-Id: <20190521062836.6541-3-richardw.yang@linux.intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/pci.c | 33 ++++++++++++++++++++++++--------- include/hw/acpi/acpi-defs.h | 18 ------------------ 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c index fa0fa30bb9..9510597a19 100644 --- a/hw/acpi/pci.c +++ b/hw/acpi/pci.c @@ -30,17 +30,32 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info) { - AcpiTableMcfg *mcfg; - int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]); + int mcfg_start = table_data->len; - mcfg = acpi_data_push(table_data, len); - mcfg->allocation[0].address = cpu_to_le64(info->base); + /* + * PCI Firmware Specification, Revision 3.0 + * 4.1.2 MCFG Table Description. + */ + acpi_data_push(table_data, sizeof(AcpiTableHeader)); + /* Reserved */ + build_append_int_noprefix(table_data, 0, 8); - /* Only a single allocation so no need to play with segments */ - mcfg->allocation[0].pci_segment = cpu_to_le16(0); - mcfg->allocation[0].start_bus_number = 0; - mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->size - 1); + /* + * Memory Mapped Enhanced Configuration Space Base Address Allocation + * Structure + */ + /* Base address, processor-relative */ + build_append_int_noprefix(table_data, info->base, 8); + /* PCI segment group number */ + build_append_int_noprefix(table_data, 0, 2); + /* Starting PCI Bus number */ + build_append_int_noprefix(table_data, 0, 1); + /* Final PCI Bus number */ + build_append_int_noprefix(table_data, PCIE_MMCFG_BUS(info->size - 1), 1); + /* Reserved */ + build_append_int_noprefix(table_data, 0, 4); - build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL); + build_header(linker, table_data, (void *)(table_data->data + mcfg_start), + "MCFG", table_data->len - mcfg_start, 1, NULL, NULL); } diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index f9aa4bd398..57a3f58b0c 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -449,24 +449,6 @@ struct AcpiSratProcessorGiccAffinity { typedef struct AcpiSratProcessorGiccAffinity AcpiSratProcessorGiccAffinity; -/* PCI fw r3.0 MCFG table. */ -/* Subtable */ -struct AcpiMcfgAllocation { - uint64_t address; /* Base address, processor-relative */ - uint16_t pci_segment; /* PCI segment group number */ - uint8_t start_bus_number; /* Starting PCI Bus number */ - uint8_t end_bus_number; /* Final PCI Bus number */ - uint32_t reserved; -} QEMU_PACKED; -typedef struct AcpiMcfgAllocation AcpiMcfgAllocation; - -struct AcpiTableMcfg { - ACPI_TABLE_HEADER_DEF; - uint8_t reserved[8]; - AcpiMcfgAllocation allocation[0]; -} QEMU_PACKED; -typedef struct AcpiTableMcfg AcpiTableMcfg; - /* * TCPA Description Table * From 2f57db8a2781787a0a31d3a9ce4a286fc45a42b3 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:37 +1000 Subject: [PATCH 03/17] pcie: Simplify pci_adjust_config_limit() Since c2077e2c "pci: Adjust PCI config limit based on bus topology", pci_adjust_config_limit() has been used in the config space read and write paths to only permit access to extended config space on buses which permit it. Specifically it prevents access on devices below a vanilla-PCI bus via some combination of bridges, even if both the host bridge and the device itself are PCI-E. It accomplishes this with a somewhat complex call up the chain of bridges to see if any of them prohibit extended config space access. This is overly complex, since we can always know if the bus will support such access at the point it is constructed. This patch simplifies the test by using a flag in the PCIBus instance indicating whether extended configuration space is accessible. It is false for vanilla PCI buses. For PCI-E buses, it is true for root buses and equal to the parent bus's's capability otherwise. For the special case of sPAPR's paravirtualized PCI root bus, which acts mostly like vanilla PCI, but does allow extended config space access, we override the default value of the flag from the host bridge code. This should cause no behavioural change. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Message-Id: <20190513061939.3464-4-david@gibson.dropbear.id.au> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 41 ++++++++++++++++++++++------------------ hw/pci/pci_host.c | 13 +++---------- hw/ppc/spapr_pci.c | 34 ++++++++++----------------------- include/hw/pci/pci.h | 1 - include/hw/pci/pci_bus.h | 8 +++++++- 5 files changed, 43 insertions(+), 54 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b386777045..7e5f8d001b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -120,6 +120,27 @@ static void pci_bus_realize(BusState *qbus, Error **errp) vmstate_register(NULL, -1, &vmstate_pcibus, bus); } +static void pcie_bus_realize(BusState *qbus, Error **errp) +{ + PCIBus *bus = PCI_BUS(qbus); + + pci_bus_realize(qbus, errp); + + /* + * A PCI-E bus can support extended config space if it's the root + * bus, or if the bus/bridge above it does as well + */ + if (pci_bus_is_root(bus)) { + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; + } else { + PCIBus *parent_bus = pci_get_bus(bus->parent_dev); + + if (pci_bus_allows_extended_config_space(parent_bus)) { + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; + } + } +} + static void pci_bus_unrealize(BusState *qbus, Error **errp) { PCIBus *bus = PCI_BUS(qbus); @@ -142,11 +163,6 @@ static uint16_t pcibus_numa_node(PCIBus *bus) return NUMA_NODE_UNASSIGNED; } -static bool pcibus_allows_extended_config_space(PCIBus *bus) -{ - return false; -} - static void pci_bus_class_init(ObjectClass *klass, void *data) { BusClass *k = BUS_CLASS(klass); @@ -161,7 +177,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data) pbc->bus_num = pcibus_num; pbc->numa_node = pcibus_numa_node; - pbc->allows_extended_config_space = pcibus_allows_extended_config_space; } static const TypeInfo pci_bus_info = { @@ -182,16 +197,11 @@ static const TypeInfo conventional_pci_interface_info = { .parent = TYPE_INTERFACE, }; -static bool pciebus_allows_extended_config_space(PCIBus *bus) -{ - return true; -} - static void pcie_bus_class_init(ObjectClass *klass, void *data) { - PCIBusClass *pbc = PCI_BUS_CLASS(klass); + BusClass *k = BUS_CLASS(klass); - pbc->allows_extended_config_space = pciebus_allows_extended_config_space; + k->realize = pcie_bus_realize; } static const TypeInfo pcie_bus_info = { @@ -410,11 +420,6 @@ bool pci_bus_is_express(PCIBus *bus) return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS); } -bool pci_bus_allows_extended_config_space(PCIBus *bus) -{ - return PCI_BUS_GET_CLASS(bus)->allows_extended_config_space(bus); -} - void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, const char *name, MemoryRegion *address_space_mem, diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index 9d64b2e12f..5f3497256c 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -53,16 +53,9 @@ static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr) static void pci_adjust_config_limit(PCIBus *bus, uint32_t *limit) { - if (*limit > PCI_CONFIG_SPACE_SIZE) { - if (!pci_bus_allows_extended_config_space(bus)) { - *limit = PCI_CONFIG_SPACE_SIZE; - return; - } - - if (!pci_bus_is_root(bus)) { - PCIDevice *bridge = pci_bridge_get_device(bus); - pci_adjust_config_limit(pci_get_bus(bridge), limit); - } + if ((*limit > PCI_CONFIG_SPACE_SIZE) && + !pci_bus_allows_extended_config_space(bus)) { + *limit = PCI_CONFIG_SPACE_SIZE; } } diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 97961b0128..9cf2c41b8c 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1626,28 +1626,6 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp) memory_region_del_subregion(get_system_memory(), &sphb->mem32window); } -static bool spapr_phb_allows_extended_config_space(PCIBus *bus) -{ - SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(BUS(bus)->parent); - - return sphb->pcie_ecs; -} - -static void spapr_phb_root_bus_class_init(ObjectClass *klass, void *data) -{ - PCIBusClass *pbc = PCI_BUS_CLASS(klass); - - pbc->allows_extended_config_space = spapr_phb_allows_extended_config_space; -} - -#define TYPE_SPAPR_PHB_ROOT_BUS "pci" - -static const TypeInfo spapr_phb_root_bus_info = { - .name = TYPE_SPAPR_PHB_ROOT_BUS, - .parent = TYPE_PCI_BUS, - .class_init = spapr_phb_root_bus_class_init, -}; - static void spapr_phb_realize(DeviceState *dev, Error **errp) { /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user @@ -1753,7 +1731,16 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb, &sphb->memspace, &sphb->iospace, PCI_DEVFN(0, 0), PCI_NUM_PINS, - TYPE_SPAPR_PHB_ROOT_BUS); + TYPE_PCI_BUS); + + /* + * Despite resembling a vanilla PCI bus in most ways, the PAPR + * para-virtualized PCI bus *does* permit PCI-E extended config + * space access + */ + if (sphb->pcie_ecs) { + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; + } phb->bus = bus; qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb), NULL); @@ -2348,7 +2335,6 @@ void spapr_pci_rtas_init(void) static void spapr_pci_register_types(void) { type_register_static(&spapr_phb_info); - type_register_static(&spapr_phb_root_bus_info); } type_init(spapr_pci_register_types) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index edf44de21d..da20c915ef 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -395,7 +395,6 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); #define TYPE_PCIE_BUS "PCIE" bool pci_bus_is_express(PCIBus *bus); -bool pci_bus_allows_extended_config_space(PCIBus *bus); void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, const char *name, diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h index aea98d5040..0714f578af 100644 --- a/include/hw/pci/pci_bus.h +++ b/include/hw/pci/pci_bus.h @@ -17,12 +17,13 @@ typedef struct PCIBusClass { int (*bus_num)(PCIBus *bus); uint16_t (*numa_node)(PCIBus *bus); - bool (*allows_extended_config_space)(PCIBus *bus); } PCIBusClass; enum PCIBusFlags { /* This bus is the root of a PCI domain */ PCI_BUS_IS_ROOT = 0x0001, + /* PCIe extended configuration space is accessible on this bus */ + PCI_BUS_EXTENDED_CONFIG_SPACE = 0x0002, }; struct PCIBus { @@ -57,4 +58,9 @@ static inline bool pci_bus_is_root(PCIBus *bus) return !!(bus->flags & PCI_BUS_IS_ROOT); } +static inline bool pci_bus_allows_extended_config_space(PCIBus *bus) +{ + return !!(bus->flags & PCI_BUS_EXTENDED_CONFIG_SPACE); +} + #endif /* QEMU_PCI_BUS_H */ From 91f4c995f26795dbb6d1b09392a6448e389ed029 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:38 +1000 Subject: [PATCH 04/17] pci: Make is_bridge a bool The is_bridge field in PCIDevice acts as a bool, but is declared as an int. Declare it as a bool for clarity, and change everything that writes it to use true/false instead of 0/1 to match. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Message-Id: <20190513061939.3464-5-david@gibson.dropbear.id.au> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci-bridge/dec.c | 4 ++-- hw/pci-bridge/i82801b11.c | 2 +- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci-bridge/pcie_pci_bridge.c | 2 +- hw/pci-bridge/pcie_root_port.c | 2 +- hw/pci-bridge/simba.c | 2 +- hw/pci-bridge/xio3130_downstream.c | 2 +- hw/pci-bridge/xio3130_upstream.c | 2 +- include/hw/pci/pci.h | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c index 8484bfd434..ca40253730 100644 --- a/hw/pci-bridge/dec.c +++ b/hw/pci-bridge/dec.c @@ -68,7 +68,7 @@ static void dec_21154_pci_bridge_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_DEC; k->device_id = PCI_DEVICE_ID_DEC_21154; k->config_write = pci_bridge_write_config; - k->is_bridge = 1; + k->is_bridge = true; dc->desc = "DEC 21154 PCI-PCI bridge"; dc->reset = pci_bridge_reset; dc->vmsd = &vmstate_pci_device; @@ -129,7 +129,7 @@ static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data) k->device_id = PCI_DEVICE_ID_DEC_21154; k->revision = 0x02; k->class_id = PCI_CLASS_BRIDGE_PCI; - k->is_bridge = 1; + k->is_bridge = true; /* * PCI-facing part of the host bridge, not usable without the * host-facing part, which can't be device_add'ed, yet. diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c index 10e590e5c6..6d8b0f54a7 100644 --- a/hw/pci-bridge/i82801b11.c +++ b/hw/pci-bridge/i82801b11.c @@ -90,7 +90,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); - k->is_bridge = 1; + k->is_bridge = true; k->vendor_id = PCI_VENDOR_ID_INTEL; k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11; k->revision = ICH9_D2P_A2_REVISION; diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c index ff6b8323da..c56ed1f52f 100644 --- a/hw/pci-bridge/pci_bridge_dev.c +++ b/hw/pci-bridge/pci_bridge_dev.c @@ -253,7 +253,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_REDHAT; k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE; k->class_id = PCI_CLASS_BRIDGE_PCI; - k->is_bridge = 1, + k->is_bridge = true; dc->desc = "Standard PCI Bridge"; dc->reset = qdev_pci_bridge_dev_reset; dc->props = pci_bridge_dev_properties; diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c index d491b40d04..9a4fba413a 100644 --- a/hw/pci-bridge/pcie_pci_bridge.c +++ b/hw/pci-bridge/pcie_pci_bridge.c @@ -143,7 +143,7 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass); - k->is_bridge = 1; + k->is_bridge = true; k->vendor_id = PCI_VENDOR_ID_REDHAT; k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE; k->realize = pcie_pci_bridge_realize; diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c index e94d918b6d..be3f4d5e03 100644 --- a/hw/pci-bridge/pcie_root_port.c +++ b/hw/pci-bridge/pcie_root_port.c @@ -162,7 +162,7 @@ static void rp_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->is_bridge = 1; + k->is_bridge = true; k->config_write = rp_write_config; k->realize = rp_realize; k->exit = rp_exit; diff --git a/hw/pci-bridge/simba.c b/hw/pci-bridge/simba.c index dea4c8c5e7..7cf0d6e047 100644 --- a/hw/pci-bridge/simba.c +++ b/hw/pci-bridge/simba.c @@ -76,7 +76,7 @@ static void simba_pci_bridge_class_init(ObjectClass *klass, void *data) k->device_id = PCI_DEVICE_ID_SUN_SIMBA; k->revision = 0x11; k->config_write = pci_bridge_write_config; - k->is_bridge = 1; + k->is_bridge = true; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); dc->reset = pci_bridge_reset; dc->vmsd = &vmstate_pci_device; diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c index 467bbabe4c..ab2a51e15d 100644 --- a/hw/pci-bridge/xio3130_downstream.c +++ b/hw/pci-bridge/xio3130_downstream.c @@ -152,7 +152,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->is_bridge = 1; + k->is_bridge = true; k->config_write = xio3130_downstream_write_config; k->realize = xio3130_downstream_realize; k->exit = xio3130_downstream_exitfn; diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c index b524908cf1..1d41a49ab0 100644 --- a/hw/pci-bridge/xio3130_upstream.c +++ b/hw/pci-bridge/xio3130_upstream.c @@ -126,7 +126,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->is_bridge = 1; + k->is_bridge = true; k->config_write = xio3130_upstream_write_config; k->realize = xio3130_upstream_realize; k->exit = xio3130_upstream_exitfn; diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index da20c915ef..d082707dfa 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -234,7 +234,7 @@ typedef struct PCIDeviceClass { * This doesn't mean pci host switch. * When card bus bridge is supported, this would be enhanced. */ - int is_bridge; + bool is_bridge; /* rom bar */ const char *romfile; From 2ad778b8c2175cb16f731d1e789e151a691172ce Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:39 +1000 Subject: [PATCH 05/17] pci: Fold pci_get_bus_devfn() into its sole caller The only remaining caller of pci_get_bus_devfn() is pci_nic_init_nofail(), itself an old compatibility function. Fold the two together to avoid re-using the stale interface. While we're there replace the explicit fprintf()s with error_report(). Signed-off-by: David Gibson Message-Id: <20190513061939.3464-6-david@gibson.dropbear.id.au> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Greg Kurz --- hw/pci/pci.c | 60 ++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 7e5f8d001b..d3893bdfe1 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -723,37 +723,6 @@ static int pci_parse_devaddr(const char *addr, int *domp, int *busp, return 0; } -static PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, - const char *devaddr) -{ - int dom, bus; - unsigned slot; - - if (!root) { - fprintf(stderr, "No primary PCI bus\n"); - return NULL; - } - - assert(!root->parent_dev); - - if (!devaddr) { - *devfnp = -1; - return pci_find_bus_nr(root, 0); - } - - if (pci_parse_devaddr(devaddr, &dom, &bus, &slot, NULL) < 0) { - return NULL; - } - - if (dom != 0) { - fprintf(stderr, "No support for non-zero PCI domains\n"); - return NULL; - } - - *devfnp = PCI_DEVFN(slot, 0); - return pci_find_bus_nr(root, bus); -} - static void pci_init_cmask(PCIDevice *dev) { pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff); @@ -1895,6 +1864,8 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, DeviceState *dev; int devfn; int i; + int dom, busnr; + unsigned slot; if (nd->model && !strcmp(nd->model, "virtio")) { g_free(nd->model); @@ -1928,7 +1899,32 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, exit(1); } - bus = pci_get_bus_devfn(&devfn, rootbus, devaddr); + if (!rootbus) { + error_report("No primary PCI bus"); + exit(1); + } + + assert(!rootbus->parent_dev); + + if (!devaddr) { + devfn = -1; + busnr = 0; + } else { + if (pci_parse_devaddr(devaddr, &dom, &busnr, &slot, NULL) < 0) { + error_report("Invalid PCI device address %s for device %s", + devaddr, nd->model); + exit(1); + } + + if (dom != 0) { + error_report("No support for non-zero PCI domains"); + exit(1); + } + + devfn = PCI_DEVFN(slot, 0); + } + + bus = pci_find_bus_nr(rootbus, busnr); if (!bus) { error_report("Invalid PCI device address %s for device %s", devaddr, nd->model); From 3fdbd1d70159157f6223f545cd462580254b7e5c Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Tue, 21 May 2019 16:23:31 +0200 Subject: [PATCH 06/17] docs: smbios: remove family=x from type2 entry description 'family' option is not part of type 2 table and if user tries to use it as such QEMU will error out with an unknow option error. Drop it from docs lest it confuse users. Fixes: b155eb1d04 ("smbios: document cmdline options for smbios type 2-4, 17 structures") Signed-off-by: Igor Mammedov Message-Id: <1558448611-315074-1-git-send-email-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 39dc170429..0d8beb4afd 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2080,7 +2080,7 @@ Specify SMBIOS type 0 fields @item -smbios type=1[,manufacturer=@var{str}][,product=@var{str}][,version=@var{str}][,serial=@var{str}][,uuid=@var{uuid}][,sku=@var{str}][,family=@var{str}] Specify SMBIOS type 1 fields -@item -smbios type=2[,manufacturer=@var{str}][,product=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,location=@var{str}][,family=@var{str}] +@item -smbios type=2[,manufacturer=@var{str}][,product=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,location=@var{str}] Specify SMBIOS type 2 fields @item -smbios type=3[,manufacturer=@var{str}][,version=@var{str}][,serial=@var{str}][,asset=@var{str}][,sku=@var{str}] From c39eb88da17616f8cdb3260c3b701609dc5c790c Mon Sep 17 00:00:00 2001 From: Jie Wang Date: Tue, 30 Apr 2019 14:10:14 +0800 Subject: [PATCH 07/17] vhost: remove the dead code remove the dead code Signed-off-by: Jie Wang Message-Id: <1556604614-32081-1-git-send-email-wangjie88@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- hw/virtio/vhost.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 7f61018f2a..2303a8c206 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1650,7 +1650,6 @@ fail_vq: hdev->vqs + i, hdev->vq_index + i); } - i = hdev->nvqs; fail_mem: fail_features: From 31618958cc48d8c51e7e256285b467c1c56af3b9 Mon Sep 17 00:00:00 2001 From: Jie Wang Date: Tue, 30 Apr 2019 14:29:33 +0800 Subject: [PATCH 08/17] vhost: fix incorrect print type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix incorrect print type in vhost_virtqueue_stop Signed-off-by: Jie Wang Message-Id: <1556605773-42019-1-git-send-email-wangjie88@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Philippe Mathieu-Daudé --- hw/virtio/vhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2303a8c206..60747a6f93 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1081,7 +1081,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev, r = dev->vhost_ops->vhost_get_vring_base(dev, &state); if (r < 0) { - VHOST_OPS_DEBUG("vhost VQ %d ring restore failed: %d", idx, r); + VHOST_OPS_DEBUG("vhost VQ %u ring restore failed: %d", idx, r); /* Connection to the backend is broken, so let's sync internal * last avail idx to the device used idx. */ From 386cff49ebd1748b0319c88e5282236f9e56b81b Mon Sep 17 00:00:00 2001 From: Jie Wang Date: Tue, 30 Apr 2019 15:15:00 +0800 Subject: [PATCH 09/17] vhost: fix memory leak in vhost_user_scsi_realize fix memory leak in vhost_user_scsi_realize Signed-off-by: Jie Wang Message-Id: <1556608500-12183-1-git-send-email-wangjie88@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- hw/scsi/vhost-user-scsi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 8b1e6876db..a9fd8ea305 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -69,6 +69,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp) VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); VHostUserSCSI *s = VHOST_USER_SCSI(dev); VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); + struct vhost_virtqueue *vqs = NULL; Error *err = NULL; int ret; @@ -93,6 +94,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp) vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs); vsc->dev.vq_index = 0; vsc->dev.backend_features = 0; + vqs = vsc->dev.vqs; ret = vhost_dev_init(&vsc->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0); @@ -100,6 +102,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp) error_setg(errp, "vhost-user-scsi: vhost initialization failed: %s", strerror(-ret)); vhost_user_cleanup(&s->vhost_user); + g_free(vqs); return; } From ab50f22309a17c772c51931940596e707c200739 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 21 May 2019 17:38:47 -0400 Subject: [PATCH 10/17] bios-tables-test: add diff allowed list Expected table change is then handled like this: 1. add table to diff allowed list 2. change generating code (can be combined with 1) 3. maintainer runs a script to update expected + blows away allowed diff list Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test-allowed-diff.h | 1 + tests/bios-tables-test.c | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/bios-tables-test-allowed-diff.h diff --git a/tests/bios-tables-test-allowed-diff.h b/tests/bios-tables-test-allowed-diff.h new file mode 100644 index 0000000000..dfb8523c8b --- /dev/null +++ b/tests/bios-tables-test-allowed-diff.h @@ -0,0 +1 @@ +/* List of comma-separated changed AML files to ignore */ diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 11e07be093..28d7d427e0 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -342,6 +342,22 @@ try_again: return exp_tables; } +static bool test_acpi_find_diff_allowed(AcpiSdtTable *sdt) +{ + const gchar *allowed_diff_file[] = { +#include "bios-tables-test-allowed-diff.h" + NULL + }; + const gchar **f; + + for (f = allowed_diff_file; *f; ++f) { + if (!g_strcmp0(sdt->aml_file, *f)) { + return true; + } + } + return false; +} + /* test the list of tables in @data->tables against reference tables */ static void test_acpi_asl(test_data *data) { @@ -396,7 +412,8 @@ static void test_acpi_asl(test_data *data) "see ASL difference."); } } - } + } + g_assert(test_acpi_find_diff_allowed(exp_sdt)); } g_string_free(asl, true); g_string_free(exp_asl, true); From c6d369fd78c654fa618e382b39da850393b8a0da Mon Sep 17 00:00:00 2001 From: Nir Weiner Date: Tue, 16 Apr 2019 15:59:10 +0300 Subject: [PATCH 11/17] vhost-scsi: The vhost backend should be stopped when the VM is not running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vhost-scsi doesn’t takes into account whether the VM is running or not in order to decide if it should start/stop vhost backend. This would lead to vhost backend still being active when VM's RunState suddenly change to stopped. An example of when this issue is encountered is when Live-Migration Pre-Copy phase completes. As in this case, VM state will be changed to stopped (while vhost backend is still active), which will result in virtio_vmstate_change() -> virtio_set_status() -> vhost_scsi_set_status() executed but vhost_scsi_set_status() will just return without stopping vhost backend. To handle this, change code to consider that vhost processing should be stopped when VM is not running. Similar to how it is done in vhost-vsock device at vhost_vsock_set_status(). Fixes: 5e9be92d7752 ("vhost-scsi: new device supporting the tcm_vhost Linux kernel module”) Reviewed-by: Bijan Mottahedeh Reviewed-by: Liran Alon Signed-off-by: Nir Weiner Message-Id: <20190416125912.44001-2-liran.alon@oracle.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- hw/scsi/vhost-scsi.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 61e2e57da9..ca42cff1b9 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -114,6 +114,10 @@ static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); bool start = (val & VIRTIO_CONFIG_S_DRIVER_OK); + if (!vdev->vm_running) { + start = false; + } + if (vsc->dev.started == start) { return; } From 4ea57425584798ccde1da6343cbfbb22497f2e4e Mon Sep 17 00:00:00 2001 From: Nir Weiner Date: Tue, 16 Apr 2019 15:59:11 +0300 Subject: [PATCH 12/17] vhost-scsi: Add VMState descriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As preparation of enabling migration of vhost-scsi device, define it’s VMState. Note, we keep the convention of verifying in the pre_save() method that the vhost backend must be stopped before attempting to save the device state. Similar to how it is done for vhost-vsock. Reviewed-by: Bijan Mottahedeh Reviewed-by: Liran Alon Signed-off-by: Nir Weiner Message-Id: <20190416125912.44001-3-liran.alon@oracle.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- hw/scsi/vhost-scsi.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ca42cff1b9..eb0cf9e131 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -139,6 +139,28 @@ static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) { } +static int vhost_scsi_pre_save(void *opaque) +{ + VHostSCSICommon *vsc = opaque; + + /* At this point, backend must be stopped, otherwise + * it might keep writing to memory. */ + assert(!vsc->dev.started); + + return 0; +} + +static const VMStateDescription vmstate_virtio_vhost_scsi = { + .name = "virtio-vhost_scsi", + .minimum_version_id = 1, + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, + .pre_save = vhost_scsi_pre_save, +}; + static void vhost_scsi_realize(DeviceState *dev, Error **errp) { VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); @@ -256,6 +278,7 @@ static void vhost_scsi_class_init(ObjectClass *klass, void *data) FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(klass); dc->props = vhost_scsi_properties; + dc->vmsd = &vmstate_virtio_vhost_scsi; set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); vdc->realize = vhost_scsi_realize; vdc->unrealize = vhost_scsi_unrealize; From b3e89c941a85f8ed74def2e817859fef5269659a Mon Sep 17 00:00:00 2001 From: Liran Alon Date: Tue, 16 Apr 2019 15:59:12 +0300 Subject: [PATCH 13/17] vhost-scsi: Allow user to enable migration In order to perform a valid migration of a vhost-scsi device, the following requirements must be met: (1) The virtio-scsi device state needs to be saved & loaded. (2) The vhost backend must be stopped before virtio-scsi device state is saved: (2.1) Sync vhost backend state to virtio-scsi device state. (2.2) No further I/O requests are made by vhost backend to target SCSI device. (2.3) No further guest memory access takes place after VM is stopped. (3) Requests in-flight to target SCSI device are completed before migration handover. (4) Target SCSI device state needs to be saved & loaded into the destination host target SCSI device. Previous commit ("vhost-scsi: Add VMState descriptor") add support to save & load the device state using VMState. This meets requirement (1). When VM is stopped by migration thread (On Pre-Copy complete), the following code path is executed: migration_completion() -> vm_stop_force_state() -> vm_stop() -> do_vm_stop(). do_vm_stop() calls first pause_all_vcpus() which pause all guest vCPUs and then call vm_state_notify(). In case of vhost-scsi device, this will lead to the following code path to be executed: vm_state_notify() -> virtio_vmstate_change() -> virtio_set_status() -> vhost_scsi_set_status() -> vhost_scsi_stop(). vhost_scsi_stop() then calls vhost_scsi_clear_endpoint() and vhost_scsi_common_stop(). vhost_scsi_clear_endpoint() sends VHOST_SCSI_CLEAR_ENDPOINT ioctl to vhost backend which will reach kernel's vhost_scsi_clear_endpoint() which process all pending I/O requests and wait for them to complete (vhost_scsi_flush()). This meets requirement (3). vhost_scsi_common_stop() will stop the vhost backend. As part of this stop, dirty-bitmap is synced and vhost backend state is synced with virtio-scsi device state. As at this point guest vCPUs are already paused, this meets requirement (2). At this point we are left with requirement (4) which is target SCSI device specific and therefore cannot be done by QEMU. Which is the main reason why vhost-scsi adds a migration blocker. However, as this can be handled either by an external orchestrator or by using shared-storage (i.e. iSCSI), there is no reason to limit the orchestrator from being able to explictly specify it wish to enable migration even when VM have a vhost-scsi device. Considering all the above, this commit allows orchestrator to explictly specify that it is responsbile for taking care of requirement (4) and therefore vhost-scsi should not add a migration blocker. Reviewed-by: Nir Weiner Reviewed-by: Bijan Mottahedeh Signed-off-by: Liran Alon Message-Id: <20190416125912.44001-4-liran.alon@oracle.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- hw/scsi/vhost-scsi.c | 30 ++++++++++++++++++--------- include/hw/virtio/vhost-scsi-common.h | 1 + 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index eb0cf9e131..6b01accf61 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -199,13 +199,18 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) goto close_fd; } - error_setg(&vsc->migration_blocker, - "vhost-scsi does not support migration"); - migrate_add_blocker(vsc->migration_blocker, &err); - if (err) { - error_propagate(errp, err); - error_free(vsc->migration_blocker); - goto close_fd; + if (!vsc->migratable) { + error_setg(&vsc->migration_blocker, + "vhost-scsi does not support migration in all cases. " + "When external environment supports it (Orchestrator migrates " + "target SCSI device state or use shared storage over network), " + "set 'migratable' property to true to enable migration."); + migrate_add_blocker(vsc->migration_blocker, &err); + if (err) { + error_propagate(errp, err); + error_free(vsc->migration_blocker); + goto close_fd; + } } vsc->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; @@ -230,7 +235,9 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) return; free_vqs: - migrate_del_blocker(vsc->migration_blocker); + if (!vsc->migratable) { + migrate_del_blocker(vsc->migration_blocker); + } g_free(vsc->dev.vqs); close_fd: close(vhostfd); @@ -243,8 +250,10 @@ static void vhost_scsi_unrealize(DeviceState *dev, Error **errp) VHostSCSICommon *vsc = VHOST_SCSI_COMMON(dev); struct vhost_virtqueue *vqs = vsc->dev.vqs; - migrate_del_blocker(vsc->migration_blocker); - error_free(vsc->migration_blocker); + if (!vsc->migratable) { + migrate_del_blocker(vsc->migration_blocker); + error_free(vsc->migration_blocker); + } /* This will stop vhost backend. */ vhost_scsi_set_status(vdev, 0); @@ -268,6 +277,7 @@ static Property vhost_scsi_properties[] = { DEFINE_PROP_BIT64("t10_pi", VHostSCSICommon, host_features, VIRTIO_SCSI_F_T10_PI, false), + DEFINE_PROP_BOOL("migratable", VHostSCSICommon, migratable, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/virtio/vhost-scsi-common.h b/include/hw/virtio/vhost-scsi-common.h index 57fb1d87b5..4eab767ee8 100644 --- a/include/hw/virtio/vhost-scsi-common.h +++ b/include/hw/virtio/vhost-scsi-common.h @@ -36,6 +36,7 @@ typedef struct VHostSCSICommon { int target; int lun; uint64_t host_features; + bool migratable; } VHostSCSICommon; int vhost_scsi_common_start(VHostSCSICommon *vsc); From df7cafdeb68b6572fa81d2be9a1910547c4dfafc Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 2 Jun 2019 21:29:27 -0400 Subject: [PATCH 14/17] bios-tables-test: list all tables that differ Fail after comparing all tables: this way user gets the full list of tables that need to be updated or whitelisted. Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 28d7d427e0..70895e1014 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -364,7 +364,7 @@ static void test_acpi_asl(test_data *data) int i; AcpiSdtTable *sdt, *exp_sdt; test_data exp_data; - gboolean exp_err, err; + gboolean exp_err, err, all_tables_match = true; memset(&exp_data, 0, sizeof(exp_data)); exp_data.tables = load_expected_aml(data); @@ -413,11 +413,13 @@ static void test_acpi_asl(test_data *data) } } } - g_assert(test_acpi_find_diff_allowed(exp_sdt)); + all_tables_match = all_tables_match && + test_acpi_find_diff_allowed(exp_sdt); } g_string_free(asl, true); g_string_free(exp_asl, true); } + g_assert(all_tables_match); free_test_data(&exp_data); } From 200dbf3789486554cda5f34fffb0bb7218ea6b49 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 3 Jun 2019 13:22:08 +0200 Subject: [PATCH 15/17] tests: add expected ACPI tables for arm/virt board MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Igor Mammedov Tested-by: Philippe Mathieu-Daudé Message-Id: <1559560929-260254-2-git-send-email-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/data/acpi/virt/APIC | Bin 0 -> 168 bytes tests/data/acpi/virt/DSDT | Bin 0 -> 18476 bytes tests/data/acpi/virt/FACP | Bin 0 -> 268 bytes tests/data/acpi/virt/GTDT | Bin 0 -> 96 bytes tests/data/acpi/virt/MCFG | Bin 0 -> 60 bytes tests/data/acpi/virt/SPCR | Bin 0 -> 80 bytes 6 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/data/acpi/virt/APIC create mode 100644 tests/data/acpi/virt/DSDT create mode 100644 tests/data/acpi/virt/FACP create mode 100644 tests/data/acpi/virt/GTDT create mode 100644 tests/data/acpi/virt/MCFG create mode 100644 tests/data/acpi/virt/SPCR diff --git a/tests/data/acpi/virt/APIC b/tests/data/acpi/virt/APIC new file mode 100644 index 0000000000000000000000000000000000000000..797dfde2841c51b7e72065602e99ce1714347f0d GIT binary patch literal 168 zcmZ<^@N{0mz`($~*~#D8BUr&HBEZ=ZD8>jB1F=Cg4Dd+6SPUF6788)c?E~X6Fu>G{ hBZPn~MyPrgD9sGlkD?67;f3451Xcqw&w(L;0RYV=2>}2A literal 0 HcmV?d00001 diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT new file mode 100644 index 0000000000000000000000000000000000000000..20e85c7f89f645c69935c615c07084e221419960 GIT binary patch literal 18476 zcmc)ScU)EVAII^7km3at6myGa+R|dUS|(gjDG>rqi;BuJN5jfYEla7?tSq%Xt!(eT zr!AYd_uhN&!S8+U(D&=t`StIw9$oH>d%mB0pK#D~&LeJRL*=*uql2K;?26j>=!V`E z6YJuY`dmg31mXSgWB#J~S-UqiR5Udjdl!92h)D&$WWX;hthf7M5uZl}Dl25#TNmhEvu#pquBa=&ZuBsU? zNU5HsVO)7EM{DBc|GlzR+b&ufK3RFzF7@fJLGsy(?FFt|xgHw}TBWeXJ_0W|JtBPC ze~f4qtRGR58c`9xic&YHN5oo1&B(GDr9Pu9azX2pRsl1jJ8oT zI@pY@ZAJ-xjMnWRv8Wk^He(x~5xJZ4ha|nLZ)h{N^%>FoX>Qc(=wdUr^BK|mX0kpAo&Ewu_q4!)6Tk8PWS``=}YcY{m{gBYHp0kDAfjW{mI|(fesZ)QpX6M!C<3 z-cLJ3&DhvxRQQbO{j_7$jQ%zwZZk@w_tV0t8JpUSN}mzEpLU9xvANBt@)^4s4= z2H1>|J|lWR?Hn~@OPevuXGHI(U7}_b+YJ3Rp7d((_T?CwzC;y zeMa%s@e)fW`SdejB+pgPzu=p7X6ze?Sk6-*5#>0_Xck_RDm_2W7&y)ZK;$m)=j zmDAD^jB3z`rSORJHgor&Q5eH z-MJy08^WmrUUw?p*%{8xaCW9s=}w($+V`^yoH{^ur_!BW;p_@$S2~sM>;`8yICVhn zPNh4$!`U6q?sO{M*#piVaOwcuol1B1gtI4{J?T`svlpDb;M4)TJC*J%g0l$DB081s z>=Vowj2IppUD&4s` zoSVa`6B>6a-MIyvTfn&mol18OfO7zxIstO0(wzh090=z?I+gC+63#8*)CrS2mG0aM z&aL3wicY0FbzRQ>ye)=PCs^)Ox>J|qv@(6&Yenk>N1--2g5m-PNh3_am}1V;M579JC*L#B{g#n zg>xvKN_Xl)nmLETsS`wZD&48eXXe}*&aLTGx>Fa;%()GmI-zu@(w(|gX3lNl+?GzI zJ9UA~oZG>v6Hs?5-KooB=G-36?depyQy0U`IUG)%u)0&}PF(^s=MHf0K&R53Bj6kX z=LkBL?ktD19L{n&mG0Cvlh-;I+gCMgR>6KIy#l^+!@ZD;oO-{r9124tcSCnPNh2=;B0`iflj46 z$H6%c&T(`q-8ml4@o=hF zJDcEag0qQEr91b6b1yjeqEqS4z2V#&&b{eWx^o{m_knXCI+gC61m`3;C()^N=e}_6 z3+KLcD&488-uCD1esJzbr_!C1;hYTTWIC1Z+#k;U;oP52r8}p7@UW}c^I8acg}=! zCY&?rRJ!wUI1h*Oa5|OloCW7BIA_tRbmtLp9s%bObSm9B8_wBq&Zbl8&N*<-fpZR> zN_WnMb1s~7=~TM&NH~v#^GG_C?mP<4qu@M>PNh5N!8s4kd2}k>c{H3y!+A8FN_QRu z=P_^|L#NW6^WmHi=X^Sq?py%p0yr1YsdQ&EoXv1H)2Vdlv2Y#>=dpAu-FY0G$H93V zol18e59jf49#5yzoeSYy2w=OQ>4(W!LjVmKGWxtLC+J5PY~1UOHiQ|Znn za4vy!37txJo(SiOaGpq~(w!&4c@mr_(W!Lj$#9+w=gD*`-MJLbrEo5#Q|ZoA;5-G+ zQ|MH>^Hexbh4WN8mF`>y=Q22#(W!LjayXa6xtva=J6FKD0?rk5D&2V+oTtHg8l6ga zo(|{faGp-5(w%3(c?O(k(5ZCinQ)#7=b3aW-FX(AXTfZ#o=vCHo#()L z4xH!EsdVSLaGne2xpXSsc^;hS!Fe8?N_URJ!wGI4_3tVmg)Xyadim;Jk!Rr8`%_xeCrzbSm9> zDV&$Wc`2PrcU}hPWpG|br_!C5!+ANJm(!_q=M`{X0p}HTD&2V{oL9nmC7nulUIph> za9%~H(w$esc{Q9@)2VdlHE>=7=QVUH-FYpX*TQ)%ol19J2j_KgUPq_Wo!7&8J)GCm zsdVQJaNYpt4Rk8qc_W-R!g(W|N_XA_=S^_lM5of7H^X@|oHx^{bmuK_-U8<>bSm9> zE1b8&c`Kbtcisl)ZE)U3r_!Cb!+ATLx6`R~=N)j~0p}fbD&2V}oOi-`C!I=n-Ua7f zaNb3y(w%q1c{iMQ)2VdlJ#gLw=RI^P-FYvZ_riHEol1A!2j_io-bbg>o%h3eKb-f| zsdVQ9a6SO%19U3g`5>GR!ucSbN_Rd4=Rqcl;e4DHzD&6@EoX^1d44q1MJ`3lwa6U_? z(w%GJTnpz~I+gBx4$kM`e2z|~JD-R1c{rb^Q|ZnZ;Cunj7wA;F^F=scg!4r@mF|2A z&X?ePiB6?EUxxE#IA5ky>CRW+d$8ci?;n&Uffk zy7OH)--YvCI+gBx56<`Ce2-40JKu-%eK_BzQ|ZnR;QRp259n07^Fug4g!4l>mG1lq z&X3^yh)$(DKZf&TI6tOS>CR8!`~=QV=v2D%Q#e0`^HVyN?)(hS&*1!wPNh3Phx2nd zKc`da&M)Bn0?se!RJ!v^IKPDROFEVA{0h#m;QWeCr8~cd^J_T2rc>$8Z{Yj}&Tr^c zy7OB&zlHN#I+gDH4$kl3{EkkgJHLnXdpN(RQ|Znh;QRs3ALvxN^G7&;g!4x_mG1lr z&Y$4?iB6?Ee}?mCIDe*7>CRu^`~}Wm=v2D%S2%x#^H(~R?)(kT-{Aa>PNh44hx2zh zf2ULF&OhM%1I|C_RJ!v|IRAw6Pdb(E{0q*%;QWhDr91zI^KUr+rc>$87C2krY@t(` zbT&3uXX$^8vEMh17mrN-KB;c&^rjx|VmXO7^5`2R-^e3;qYr+ruys>IeM3fSRO^S&ss7)h*wT1Svj`NiYWyhWBH9WZC8OmrRdkX@gRI%+|-U z8DWHT!aHT*s2N9wx3Qi<_+e#-w| zCa1oxxGDL|)s4+HS@l)vx2#F-LE(GJgg*#nvEqMxyAr#GzF9>hQs1W3hy3tk_y#kh z+l;EKP5s1`C*DVANccWF>wb|tH9P&T8$ss!chlJnyQM+f`xSx>MLUN8=H&5JIVJQNjlBbPnKQWME+3?d0$55v<@85#a0w6axw|fY>0KxE!S15v<@85#a0&6k`O6f!H7#8OTC8azL5|h^3)?DJYFj0RVOU B2mt^9 literal 0 HcmV?d00001 diff --git a/tests/data/acpi/virt/MCFG b/tests/data/acpi/virt/MCFG new file mode 100644 index 0000000000000000000000000000000000000000..e8987e1af0ec3829770bf4fe11fab02b06160dd2 GIT binary patch literal 60 scmeZuc5}C3U|?YMck*}k2v%^42ypfViZKGkKx`0=1Oyx)oc|yS05YNo0RR91 literal 0 HcmV?d00001 diff --git a/tests/data/acpi/virt/SPCR b/tests/data/acpi/virt/SPCR new file mode 100644 index 0000000000000000000000000000000000000000..377271a0e7817cc21a28c02123a89facad63604f GIT binary patch literal 80 zcmWFza1IJ!U|?VpcJg=j2v%^42yhMtiZKGkKx`1r48#l^3?L>agsBLmm>C$E7#RKo I0Z0r60QF4^0RR91 literal 0 HcmV?d00001 From ab6b6a7777449905a70f7e80790719779e7f3313 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 3 Jun 2019 13:22:09 +0200 Subject: [PATCH 16/17] tests: acpi: add simple arm/virt testcase adds simple arm/virt test case that starts guest with bios-tables-test.aarch64.iso.qcow2 boot image which initializes UefiTestSupport* structure in RAM once guest is booted. * see commit: tests: acpi: add acpi_find_rsdp_address_uefi() helper Signed-off-by: Igor Mammedov Message-Id: <1559560929-260254-3-git-send-email-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/Makefile.include | 4 ++++ tests/bios-tables-test.c | 18 ++++++++++++++++++ tests/data/acpi/rebuild-expected-aml.sh | 2 +- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 1865f6b322..2334969f1f 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -267,6 +267,10 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF) check-qtest-aarch64-y = tests/numa-test$(EXESUF) check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) check-qtest-aarch64-y += tests/migration-test$(EXESUF) +# TODO: once aarch64 TCG is fixed on ARM 32 bit host, make test unconditional +ifneq ($(ARCH),arm) +check-qtest-aarch64-y += tests/bios-tables-test$(EXESUF) +endif check-qtest-microblazeel-y += $(check-qtest-microblaze-y) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 70895e1014..d80ea7f7f3 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -832,6 +832,22 @@ static void test_acpi_piix4_tcg_dimm_pxm(void) test_acpi_tcg_dimm_pxm(MACHINE_PC); } +static void test_acpi_virt_tcg(void) +{ + test_data data = { + .machine = "virt", + .accel = "tcg", + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", + .ram_start = 0x40000000ULL, + .scan_len = 128ULL * 1024 * 1024, + }; + + test_acpi_one("-cpu cortex-a57", &data); + free_test_data(&data); +} + int main(int argc, char *argv[]) { const char *arch = qtest_get_arch(); @@ -860,6 +876,8 @@ int main(int argc, char *argv[]) qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem); qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm); qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm); + } else if (strcmp(arch, "aarch64") == 0) { + qtest_add_func("acpi/virt", test_acpi_virt_tcg); } ret = g_test_run(); boot_sector_cleanup(disk); diff --git a/tests/data/acpi/rebuild-expected-aml.sh b/tests/data/acpi/rebuild-expected-aml.sh index ff7e62249d..d2853218dd 100755 --- a/tests/data/acpi/rebuild-expected-aml.sh +++ b/tests/data/acpi/rebuild-expected-aml.sh @@ -12,7 +12,7 @@ # This work is licensed under the terms of the GNU GPLv2. # See the COPYING.LIB file in the top-level directory. -qemu_bins="x86_64-softmmu/qemu-system-x86_64" +qemu_bins="x86_64-softmmu/qemu-system-x86_64 aarch64-softmmu/qemu-system-aarch64" if [ ! -e "tests/bios-tables-test" ]; then echo "Test: bios-tables-test is required! Run make check before this script." From 7f36f0930ffec11a551844c0452dbce33f80a525 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 4 Jun 2019 23:41:32 -0400 Subject: [PATCH 17/17] bios-tables-test: ignore identical binaries when binary of the tables is identical, there is no need to run iasl to check that they are functionally equivalent. Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index d80ea7f7f3..0ce55182f2 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -375,6 +375,20 @@ static void test_acpi_asl(test_data *data) sdt = &g_array_index(data->tables, AcpiSdtTable, i); exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i); + if (sdt->aml_len == exp_sdt->aml_len && + !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) { + /* Identical table binaries: no need to disassemble. */ + continue; + } + + fprintf(stderr, + "acpi-test: Warning! %.4s binary file mismatch. " + "Actual [aml:%s], Expected [aml:%s].\n", + exp_sdt->aml, sdt->aml_file, exp_sdt->aml_file); + + all_tables_match = all_tables_match && + test_acpi_find_diff_allowed(exp_sdt); + err = load_asl(data->tables, sdt); asl = normalize_asl(sdt->asl); @@ -413,8 +427,6 @@ static void test_acpi_asl(test_data *data) } } } - all_tables_match = all_tables_match && - test_acpi_find_diff_allowed(exp_sdt); } g_string_free(asl, true); g_string_free(exp_asl, true);