From f13a944ca6d50efa1dc4cca3a31262b677a2a715 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Tue, 21 May 2019 14:28:35 +0800 Subject: 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 --- 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 +----------------- 6 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 hw/acpi/pci.c (limited to 'hw') diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig index eca3bee..7c59cf9 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 2d46e37..661a9b8 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 0000000..fa0fa30 --- /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 af8cffd..9aced9d 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 e7c96d6..4a64f99 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 0d78d73..85dc164 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(); -- cgit v1.1 From e4610781635404d494120c19bade8dad6f00f0b3 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Tue, 21 May 2019 14:28:36 +0800 Subject: 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 | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'hw') diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c index fa0fa30..9510597 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]); - - 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); + int mcfg_start = table_data->len; + + /* + * 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); + + /* + * 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 *)(table_data->data + mcfg_start), + "MCFG", table_data->len - mcfg_start, 1, NULL, NULL); } -- cgit v1.1 From 2f57db8a2781787a0a31d3a9ce4a286fc45a42b3 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:37 +1000 Subject: 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 ++++++++++------------------------ 3 files changed, 36 insertions(+), 52 deletions(-) (limited to 'hw') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b386777..7e5f8d0 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 9d64b2e..5f34972 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 97961b0..9cf2c41 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) -- cgit v1.1 From 91f4c995f26795dbb6d1b09392a6448e389ed029 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:38 +1000 Subject: 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 +- 8 files changed, 9 insertions(+), 9 deletions(-) (limited to 'hw') diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c index 8484bfd..ca40253 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 10e590e..6d8b0f5 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 ff6b832..c56ed1f 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 d491b40..9a4fba4 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 e94d918..be3f4d5 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 dea4c8c..7cf0d6e 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 467bbab..ab2a51e 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 b524908..1d41a49 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; -- cgit v1.1 From 2ad778b8c2175cb16f731d1e789e151a691172ce Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:39 +1000 Subject: 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(-) (limited to 'hw') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 7e5f8d0..d3893bd 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); -- cgit v1.1 From c39eb88da17616f8cdb3260c3b701609dc5c790c Mon Sep 17 00:00:00 2001 From: Jie Wang Date: Tue, 30 Apr 2019 14:10:14 +0800 Subject: 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(-) (limited to 'hw') diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 7f61018..2303a8c 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: -- cgit v1.1 From 31618958cc48d8c51e7e256285b467c1c56af3b9 Mon Sep 17 00:00:00 2001 From: Jie Wang Date: Tue, 30 Apr 2019 14:29:33 +0800 Subject: 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(-) (limited to 'hw') diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2303a8c..60747a6 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. */ -- cgit v1.1 From 386cff49ebd1748b0319c88e5282236f9e56b81b Mon Sep 17 00:00:00 2001 From: Jie Wang Date: Tue, 30 Apr 2019 15:15:00 +0800 Subject: 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(+) (limited to 'hw') diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 8b1e687..a9fd8ea 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; } -- cgit v1.1 From c6d369fd78c654fa618e382b39da850393b8a0da Mon Sep 17 00:00:00 2001 From: Nir Weiner Date: Tue, 16 Apr 2019 15:59:10 +0300 Subject: 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(+) (limited to 'hw') diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 61e2e57..ca42cff 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; } -- cgit v1.1 From 4ea57425584798ccde1da6343cbfbb22497f2e4e Mon Sep 17 00:00:00 2001 From: Nir Weiner Date: Tue, 16 Apr 2019 15:59:11 +0300 Subject: 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(+) (limited to 'hw') diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ca42cff..eb0cf9e 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; -- cgit v1.1 From b3e89c941a85f8ed74def2e817859fef5269659a Mon Sep 17 00:00:00 2001 From: Liran Alon Date: Tue, 16 Apr 2019 15:59:12 +0300 Subject: 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 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) (limited to 'hw') diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index eb0cf9e..6b01acc 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(), }; -- cgit v1.1