From 2d6dcbf93fb01b4a7f45a93d276d4d74b16392dd Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Sat, 28 Oct 2017 21:51:36 +0100 Subject: [PATCH 01/23] smbios: support setting OEM strings table The cloud-init program currently allows fetching of its data by repurposing of the 'system' type 'serial' field. This is a clear abuse of the serial field that would clash with other valid usage a virt management app might have for that field. Fortunately the SMBIOS defines an "OEM Strings" table whose puporse is to allow exposing of arbitrary vendor specific strings to the operating system. This is perfect for use with cloud-init, or as a way to pass arguments to OS installers such as anaconda. This patch makes it easier to support this with QEMU. e.g. $QEMU -smbios type=11,value=Hello,value=World,value=Tricky,,value=test Which results in the guest seeing dmidecode data Handle 0x0E00, DMI type 11, 5 bytes OEM Strings String 1: Hello String 2: World String 3: Tricky,value=test It is suggested that any app wanting to make use of this OEM strings capability for accepting data from the host mgmt layer should use its name as a string prefix. e.g. to expose OEM strings targetting both cloud init and anaconda in parallel the mgmt app could set $QEMU -smbios type=11,value=cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/,\ value=anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os which would appear as Handle 0x0E00, DMI type 11, 5 bytes OEM Strings String 1: cloud-init:ds=nocloud-net;s=http://10.10.0.1:8000/ String 2: anaconda:method=http://dl.fedoraproject.org/pub/fedora/linux/releases/25/x86_64/os Use of such string prefixes means the app won't have to care which string slot its data appears in. Signed-off-by: Daniel P. Berrange Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/smbios/smbios.c | 72 ++++++++++++++++++++++++++++++++++++++ hw/smbios/smbios_build.h | 12 +++++++ include/hw/smbios/smbios.h | 6 ++++ 3 files changed, 90 insertions(+) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index 1a5437a07d..5d11f01874 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -95,6 +95,11 @@ static struct { const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part; } type4; +static struct { + size_t nvalues; + const char **values; +} type11; + static struct { const char *loc_pfx, *bank, *manufacturer, *serial, *asset, *part; uint16_t speed; @@ -282,6 +287,14 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = { { /* end of list */ } }; +static const QemuOptDesc qemu_smbios_type11_opts[] = { + { + .name = "value", + .type = QEMU_OPT_STRING, + .help = "OEM string data", + }, +}; + static const QemuOptDesc qemu_smbios_type17_opts[] = { { .name = "type", @@ -590,6 +603,27 @@ static void smbios_build_type_4_table(unsigned instance) smbios_type4_count++; } +static void smbios_build_type_11_table(void) +{ + char count_str[128]; + size_t i; + + if (type11.nvalues == 0) { + return; + } + + SMBIOS_BUILD_TABLE_PRE(11, 0xe00, true); /* required */ + + snprintf(count_str, sizeof(count_str), "%zu", type11.nvalues); + t->count = type11.nvalues; + + for (i = 0; i < type11.nvalues; i++) { + SMBIOS_TABLE_SET_STR_LIST(11, type11.values[i]); + } + + SMBIOS_BUILD_TABLE_POST; +} + #define ONE_KB ((ram_addr_t)1 << 10) #define ONE_MB ((ram_addr_t)1 << 20) #define ONE_GB ((ram_addr_t)1 << 30) @@ -832,6 +866,8 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array, smbios_build_type_4_table(i); } + smbios_build_type_11_table(); + #define MAX_DIMM_SZ (16ll * ONE_GB) #define GET_DIMM_SZ ((i < dimm_cnt - 1) ? MAX_DIMM_SZ \ : ((ram_size - 1) % MAX_DIMM_SZ) + 1) @@ -882,6 +918,38 @@ static void save_opt(const char **dest, QemuOpts *opts, const char *name) } } + +struct opt_list { + const char *name; + size_t *ndest; + const char ***dest; +}; + +static int save_opt_one(void *opaque, + const char *name, const char *value, + Error **errp) +{ + struct opt_list *opt = opaque; + + if (!g_str_equal(name, opt->name)) { + return 0; + } + + *opt->dest = g_renew(const char *, *opt->dest, (*opt->ndest) + 1); + (*opt->dest)[*opt->ndest] = value; + (*opt->ndest)++; + return 0; +} + +static void save_opt_list(size_t *ndest, const char ***dest, + QemuOpts *opts, const char *name) +{ + struct opt_list opt = { + name, ndest, dest, + }; + qemu_opt_foreach(opts, save_opt_one, &opt, NULL); +} + void smbios_entry_add(QemuOpts *opts, Error **errp) { const char *val; @@ -1035,6 +1103,10 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) save_opt(&type4.asset, opts, "asset"); save_opt(&type4.part, opts, "part"); return; + case 11: + qemu_opts_validate(opts, qemu_smbios_type11_opts, &error_fatal); + save_opt_list(&type11.nvalues, &type11.values, opts, "value"); + return; case 17: qemu_opts_validate(opts, qemu_smbios_type17_opts, &error_fatal); save_opt(&type17.loc_pfx, opts, "loc_pfx"); diff --git a/hw/smbios/smbios_build.h b/hw/smbios/smbios_build.h index 68b8b72e09..93b360d520 100644 --- a/hw/smbios/smbios_build.h +++ b/hw/smbios/smbios_build.h @@ -63,6 +63,18 @@ extern unsigned smbios_table_cnt; } \ } while (0) +#define SMBIOS_TABLE_SET_STR_LIST(tbl_type, value) \ + do { \ + int len = (value != NULL) ? strlen(value) + 1 : 0; \ + if (len > 1) { \ + smbios_tables = g_realloc(smbios_tables, \ + smbios_tables_len + len); \ + memcpy(smbios_tables + smbios_tables_len, value, len); \ + smbios_tables_len += len; \ + ++str_index; \ + } \ + } while (0) + #define SMBIOS_BUILD_TABLE_POST \ do { \ size_t term_cnt, t_size; \ diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h index 31e8d5f47e..a83adb93d7 100644 --- a/include/hw/smbios/smbios.h +++ b/include/hw/smbios/smbios.h @@ -195,6 +195,12 @@ struct smbios_type_4 { uint16_t processor_family2; } QEMU_PACKED; +/* SMBIOS type 11 - OEM strings */ +struct smbios_type_11 { + struct smbios_structure_header header; + uint8_t count; +} QEMU_PACKED; + /* SMBIOS type 16 - Physical Memory Array (v2.7) */ struct smbios_type_16 { struct smbios_structure_header header; From 87e6ed5670771b558d733fd21ed8e1e4da0e368a Mon Sep 17 00:00:00 2001 From: Roman Kagan Date: Mon, 27 Nov 2017 16:05:17 +0300 Subject: [PATCH 02/23] qdev-properties: add UUID property type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UUIDs (GUIDs) are widely used in VMBus-related stuff, so a dedicated property type becomes helpful. The property accepts a string-formatted UUID or a special keyword "auto" meaning a randomly generated UUID; the latter is also the default when the property is not given a value explicitly. Signed-off-by: Roman Kagan Reviewed-by: Marc-André Lureau Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/core/qdev-properties.c | 61 ++++++++++++++++++++++++++++++++++++ include/hw/qdev-properties.h | 9 ++++++ 2 files changed, 70 insertions(+) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 1dc80fcea2..24c17800e3 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -10,6 +10,7 @@ #include "net/hub.h" #include "qapi/visitor.h" #include "chardev/char.h" +#include "qemu/uuid.h" void qdev_prop_set_after_realize(DeviceState *dev, const char *name, Error **errp) @@ -883,6 +884,66 @@ const PropertyInfo qdev_prop_pci_host_devaddr = { .set = set_pci_host_devaddr, }; +/* --- UUID --- */ + +static void get_uuid(Object *obj, Visitor *v, const char *name, void *opaque, + Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + QemuUUID *uuid = qdev_get_prop_ptr(dev, prop); + char buffer[UUID_FMT_LEN + 1]; + char *p = buffer; + + qemu_uuid_unparse(uuid, buffer); + + visit_type_str(v, name, &p, errp); +} + +#define UUID_VALUE_AUTO "auto" + +static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque, + Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + QemuUUID *uuid = qdev_get_prop_ptr(dev, prop); + Error *local_err = NULL; + char *str; + + if (dev->realized) { + qdev_prop_set_after_realize(dev, name, errp); + return; + } + + visit_type_str(v, name, &str, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + if (!strcmp(str, UUID_VALUE_AUTO)) { + qemu_uuid_generate(uuid); + } else if (qemu_uuid_parse(str, uuid) < 0) { + error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str); + } + g_free(str); +} + +static void set_default_uuid_auto(Object *obj, const Property *prop) +{ + object_property_set_str(obj, UUID_VALUE_AUTO, prop->name, &error_abort); +} + +const PropertyInfo qdev_prop_uuid = { + .name = "str", + .description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO + "\" for random value (default)", + .get = get_uuid, + .set = set_uuid, + .set_default_value = set_default_uuid_auto, +}; + /* --- support for array properties --- */ /* Used as an opaque for the object properties we add for each diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index e2321f1cc1..97e810eeb5 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -30,6 +30,7 @@ extern const PropertyInfo qdev_prop_vlan; extern const PropertyInfo qdev_prop_pci_devfn; extern const PropertyInfo qdev_prop_blocksize; extern const PropertyInfo qdev_prop_pci_host_devaddr; +extern const PropertyInfo qdev_prop_uuid; extern const PropertyInfo qdev_prop_arraylen; extern const PropertyInfo qdev_prop_link; @@ -213,6 +214,14 @@ extern const PropertyInfo qdev_prop_link; #define DEFINE_PROP_MEMORY_REGION(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *) +#define DEFINE_PROP_UUID(_name, _state, _field) { \ + .name = (_name), \ + .info = &qdev_prop_uuid, \ + .offset = offsetof(_state, _field) \ + + type_check(QemuUUID, typeof_field(_state, _field)), \ + .set_default = true, \ + } + #define DEFINE_PROP_END_OF_LIST() \ {} From 939dd2d350b966016f2a1fa126242c094e71cd82 Mon Sep 17 00:00:00 2001 From: Roman Kagan Date: Mon, 27 Nov 2017 16:05:18 +0300 Subject: [PATCH 03/23] vmgenid: use UUID property type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch vmgenid device to use the UUID property type introduced in the previous patch for its 'guid' property. One semantic change it introduces is that post-realize modification of 'guid' via HMP or QMP will now be rejected with an error; however, according to docs/specs/vmgenid.txt this is actually desirable. Signed-off-by: Roman Kagan Reviewed-by: Marc-André Lureau Reviewed-by: Ben Warren Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/vmgenid.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c index 105044f666..ba6f47b67b 100644 --- a/hw/acpi/vmgenid.c +++ b/hw/acpi/vmgenid.c @@ -162,21 +162,6 @@ static void vmgenid_update_guest(VmGenIdState *vms) } } -static void vmgenid_set_guid(Object *obj, const char *value, Error **errp) -{ - VmGenIdState *vms = VMGENID(obj); - - if (!strcmp(value, "auto")) { - qemu_uuid_generate(&vms->guid); - } else if (qemu_uuid_parse(value, &vms->guid) < 0) { - error_setg(errp, "'%s. %s': Failed to parse GUID string: %s", - object_get_typename(OBJECT(vms)), VMGENID_GUID, value); - return; - } - - vmgenid_update_guest(vms); -} - /* After restoring an image, we need to update the guest memory and notify * it of a potential change to VM Generation ID */ @@ -224,23 +209,24 @@ static void vmgenid_realize(DeviceState *dev, Error **errp) } qemu_register_reset(vmgenid_handle_reset, vms); + + vmgenid_update_guest(vms); } +static Property vmgenid_device_properties[] = { + DEFINE_PROP_UUID(VMGENID_GUID, VmGenIdState, guid), + DEFINE_PROP_END_OF_LIST(), +}; + static void vmgenid_device_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->vmsd = &vmstate_vmgenid; dc->realize = vmgenid_realize; + dc->props = vmgenid_device_properties; dc->hotpluggable = false; set_bit(DEVICE_CATEGORY_MISC, dc->categories); - - object_class_property_add_str(klass, VMGENID_GUID, NULL, - vmgenid_set_guid, NULL); - object_class_property_set_description(klass, VMGENID_GUID, - "Set Global Unique Identifier " - "(big-endian) or auto for random value", - NULL); } static const TypeInfo vmgenid_device_info = { From 4426f06102d31c4062957034edeb417f34c67885 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 29 Nov 2017 23:14:28 +0530 Subject: [PATCH 04/23] tests: add test to check VirtQueue object An uninitialised VirtQueue object or one with Vring.align field set to zero(0) could lead to arithmetic exceptions. Add a unit test to validate it. Signed-off-by: Prasad J Pandit Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- tests/virtio-blk-test.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index e6fb9bac87..45f368dcd9 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -674,6 +674,30 @@ static void pci_hotplug(void) qtest_shutdown(qs); } +/* + * Check that setting the vring addr on a non-existent virtqueue does + * not crash. + */ +static void test_nonexistent_virtqueue(void) +{ + QPCIBar bar0; + QOSState *qs; + QPCIDevice *dev; + + qs = pci_test_start(); + dev = qpci_device_find(qs->pcibus, QPCI_DEVFN(4, 0)); + g_assert(dev != NULL); + + qpci_device_enable(dev); + bar0 = qpci_iomap(dev, 0, NULL); + + qpci_io_writeb(dev, bar0, VIRTIO_PCI_QUEUE_SEL, 2); + qpci_io_writel(dev, bar0, VIRTIO_PCI_QUEUE_PFN, 1); + + g_free(dev); + qtest_shutdown(qs); +} + static void mmio_basic(void) { QVirtioMMIODevice *dev; @@ -724,6 +748,7 @@ int main(int argc, char **argv) qtest_add_func("/virtio/blk/pci/basic", pci_basic); qtest_add_func("/virtio/blk/pci/indirect", pci_indirect); qtest_add_func("/virtio/blk/pci/config", pci_config); + qtest_add_func("/virtio/blk/pci/nxvirtq", test_nonexistent_virtqueue); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { qtest_add_func("/virtio/blk/pci/msix", pci_msix); qtest_add_func("/virtio/blk/pci/idx", pci_idx); From 1115ff6d268bba163a183e8d63cec6b687cc5fc7 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 29 Nov 2017 19:46:22 +1100 Subject: [PATCH 05/23] pci: Rename root bus initialization functions for clarity pci_bus_init(), pci_bus_new_inplace(), pci_bus_new() and pci_register_bus() are misleadingly named. They're not used for initializing *any* PCI bus, but only for a root PCI bus. Non-root buses - i.e. ones under a logical PCI to PCI bridge - are instead created with a direct qbus_create_inplace() (see pci_bridge_initfn()). This patch renames the functions to make it clear they're only used for a root bus. Signed-off-by: David Gibson Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum Reviewed-by: Peter Xu --- hw/alpha/typhoon.c | 8 ++--- hw/mips/gt64xxx_pci.c | 12 +++---- hw/pci-bridge/pci_expander_bridge.c | 4 +-- hw/pci-host/apb.c | 10 +++--- hw/pci-host/bonito.c | 8 ++--- hw/pci-host/gpex.c | 6 ++-- hw/pci-host/grackle.c | 14 ++++---- hw/pci-host/piix.c | 4 +-- hw/pci-host/ppce500.c | 6 ++-- hw/pci-host/prep.c | 4 +-- hw/pci-host/q35.c | 7 ++-- hw/pci-host/uninorth.c | 24 +++++++------- hw/pci-host/versatile.c | 6 ++-- hw/pci-host/xilinx-pcie.c | 6 ++-- hw/pci/pci.c | 51 +++++++++++++++-------------- hw/ppc/ppc4xx_pci.c | 6 ++-- hw/ppc/spapr_pci.c | 8 ++--- hw/s390x/s390-pci-bus.c | 8 ++--- hw/sh4/sh_pci.c | 12 +++---- include/hw/pci/pci.h | 25 +++++++------- 20 files changed, 117 insertions(+), 112 deletions(-) diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index ae11e012c7..6a40869488 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -881,10 +881,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus, memory_region_add_subregion(addr_space, 0x801fc000000ULL, &s->pchip.reg_io); - b = pci_register_bus(dev, "pci", - typhoon_set_irq, sys_map_irq, s, - &s->pchip.reg_mem, &s->pchip.reg_io, - 0, 64, TYPE_PCI_BUS); + b = pci_register_root_bus(dev, "pci", + typhoon_set_irq, sys_map_irq, s, + &s->pchip.reg_mem, &s->pchip.reg_io, + 0, 64, TYPE_PCI_BUS); phb->bus = b; qdev_init_nofail(dev); diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index 5a9dad9aae..a9c222a967 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -1171,12 +1171,12 @@ PCIBus *gt64120_register(qemu_irq *pic) phb = PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX); address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem"); - phb->bus = pci_register_bus(dev, "pci", - gt64120_pci_set_irq, gt64120_pci_map_irq, - pic, - &d->pci0_mem, - get_system_io(), - PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS); + phb->bus = pci_register_root_bus(dev, "pci", + gt64120_pci_set_irq, gt64120_pci_map_irq, + pic, + &d->pci0_mem, + get_system_io(), + PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS); qdev_init_nofail(dev); memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d, "isd-mem", 0x1000); diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index 8c8ac737ad..b2fa829e29 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -230,9 +230,9 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) ds = qdev_create(NULL, TYPE_PXB_HOST); if (pcie) { - bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS); + bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS); } else { - bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS); + bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS); bds = qdev_create(BUS(bus), "pci-bridge"); bds->id = dev_name; qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr); diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c index 64025cd8cc..1df998443d 100644 --- a/hw/pci-host/apb.c +++ b/hw/pci-host/apb.c @@ -714,11 +714,11 @@ PCIBus *pci_apb_init(hwaddr special_base, dev = qdev_create(NULL, TYPE_APB); d = APB_DEVICE(dev); phb = PCI_HOST_BRIDGE(dev); - phb->bus = pci_register_bus(DEVICE(phb), "pci", - pci_apb_set_irq, pci_apb_map_irq, d, - &d->pci_mmio, - &d->pci_ioport, - 0, 32, TYPE_PCI_BUS); + phb->bus = pci_register_root_bus(DEVICE(phb), "pci", + pci_apb_set_irq, pci_apb_map_irq, d, + &d->pci_mmio, + &d->pci_ioport, + 0, 32, TYPE_PCI_BUS); qdev_init_nofail(dev); s = SYS_BUS_DEVICE(dev); /* apb_config */ diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index 9f61e27edc..f08593feab 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -714,10 +714,10 @@ static int bonito_pcihost_initfn(SysBusDevice *dev) { PCIHostState *phb = PCI_HOST_BRIDGE(dev); - phb->bus = pci_register_bus(DEVICE(dev), "pci", - pci_bonito_set_irq, pci_bonito_map_irq, dev, - get_system_memory(), get_system_io(), - 0x28, 32, TYPE_PCI_BUS); + phb->bus = pci_register_root_bus(DEVICE(dev), "pci", + pci_bonito_set_irq, pci_bonito_map_irq, + dev, get_system_memory(), get_system_io(), + 0x28, 32, TYPE_PCI_BUS); return 0; } diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c index edf305b1fd..2583b151a4 100644 --- a/hw/pci-host/gpex.c +++ b/hw/pci-host/gpex.c @@ -89,9 +89,9 @@ static void gpex_host_realize(DeviceState *dev, Error **errp) s->irq_num[i] = -1; } - pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq, - pci_swizzle_map_irq_fn, s, &s->io_mmio, - &s->io_ioport, 0, 4, TYPE_PCIE_BUS); + pci->bus = pci_register_root_bus(dev, "pcie.0", gpex_set_irq, + pci_swizzle_map_irq_fn, s, &s->io_mmio, + &s->io_ioport, 0, 4, TYPE_PCIE_BUS); qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus)); pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq); diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c index 38cd279b6b..3caf1ccb37 100644 --- a/hw/pci-host/grackle.c +++ b/hw/pci-host/grackle.c @@ -82,13 +82,13 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic, memory_region_add_subregion(address_space_mem, 0x80000000ULL, &d->pci_hole); - phb->bus = pci_register_bus(dev, NULL, - pci_grackle_set_irq, - pci_grackle_map_irq, - pic, - &d->pci_mmio, - address_space_io, - 0, 4, TYPE_PCI_BUS); + phb->bus = pci_register_root_bus(dev, NULL, + pci_grackle_set_irq, + pci_grackle_map_irq, + pic, + &d->pci_mmio, + address_space_io, + 0, 4, TYPE_PCI_BUS); pci_create_simple(phb->bus, 0, "grackle"); qdev_init_nofail(dev); diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index a684a7cca9..cf9070186c 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -361,8 +361,8 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type, dev = qdev_create(NULL, host_type); s = PCI_HOST_BRIDGE(dev); - b = pci_bus_new(dev, NULL, pci_address_space, - address_space_io, 0, TYPE_PCI_BUS); + b = pci_root_bus_new(dev, NULL, pci_address_space, + address_space_io, 0, TYPE_PCI_BUS); s->bus = b; object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL); qdev_init_nofail(dev); diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c index 39cd24464d..67edbf744c 100644 --- a/hw/pci-host/ppce500.c +++ b/hw/pci-host/ppce500.c @@ -465,9 +465,9 @@ static int e500_pcihost_initfn(SysBusDevice *dev) /* PIO lives at the bottom of our bus space */ memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2); - b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq, - mpc85xx_pci_map_irq, s, &s->busmem, &s->pio, - PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS); + b = pci_register_root_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq, + mpc85xx_pci_map_irq, s, &s->busmem, &s->pio, + PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS); h->bus = b; /* Set up PCI view of memory */ diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c index 92eed0f3e1..01f67f9db1 100644 --- a/hw/pci-host/prep.c +++ b/hw/pci-host/prep.c @@ -269,8 +269,8 @@ static void raven_pcihost_initfn(Object *obj) memory_region_add_subregion_overlap(address_space_mem, 0x80000000, &s->pci_io_non_contiguous, 1); memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory); - pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL, - &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS); + pci_root_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL, + &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS); /* Bus master address space */ memory_region_init(&s->bm, obj, "bm-raven", UINT32_MAX); diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 6cb9a8d121..a36a1195e4 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -51,9 +51,10 @@ static void q35_host_realize(DeviceState *dev, Error **errp) sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem); sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4); - pci->bus = pci_bus_new(DEVICE(s), "pcie.0", - s->mch.pci_address_space, s->mch.address_space_io, - 0, TYPE_PCIE_BUS); + pci->bus = pci_root_bus_new(DEVICE(s), "pcie.0", + s->mch.pci_address_space, + s->mch.address_space_io, + 0, TYPE_PCIE_BUS); PC_MACHINE(qdev_get_machine())->bus = pci->bus; qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus)); qdev_init_nofail(DEVICE(&s->mch)); diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index ea5c265718..5d8ccaa711 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -233,12 +233,12 @@ PCIBus *pci_pmac_init(qemu_irq *pic, memory_region_add_subregion(address_space_mem, 0x80000000ULL, &d->pci_hole); - h->bus = pci_register_bus(dev, NULL, - pci_unin_set_irq, pci_unin_map_irq, - pic, - &d->pci_mmio, - address_space_io, - PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS); + h->bus = pci_register_root_bus(dev, NULL, + pci_unin_set_irq, pci_unin_map_irq, + pic, + &d->pci_mmio, + address_space_io, + PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS); #if 0 pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north"); @@ -299,12 +299,12 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic, memory_region_add_subregion(address_space_mem, 0x80000000ULL, &d->pci_hole); - h->bus = pci_register_bus(dev, NULL, - pci_unin_set_irq, pci_unin_map_irq, - pic, - &d->pci_mmio, - address_space_io, - PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS); + h->bus = pci_register_root_bus(dev, NULL, + pci_unin_set_irq, pci_unin_map_irq, + pic, + &d->pci_mmio, + address_space_io, + PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS); sysbus_mmio_map(s, 0, 0xf0800000); sysbus_mmio_map(s, 1, 0xf0c00000); diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c index 6394a520fc..8803ada925 100644 --- a/hw/pci-host/versatile.c +++ b/hw/pci-host/versatile.c @@ -399,9 +399,9 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp) memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32); memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32); - pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci", - &s->pci_mem_space, &s->pci_io_space, - PCI_DEVFN(11, 0), TYPE_PCI_BUS); + pci_root_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci", + &s->pci_mem_space, &s->pci_io_space, + PCI_DEVFN(11, 0), TYPE_PCI_BUS); h->bus = &s->pci_bus; object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_VERSATILE_PCI_HOST); diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c index 7659253090..d2f88d11dd 100644 --- a/hw/pci-host/xilinx-pcie.c +++ b/hw/pci-host/xilinx-pcie.c @@ -129,9 +129,9 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(sbd, &pex->mmio); sysbus_init_mmio(sbd, &s->mmio); - pci->bus = pci_register_bus(dev, s->name, xilinx_pcie_set_irq, - pci_swizzle_map_irq_fn, s, &s->mmio, - &s->io, 0, 4, TYPE_PCIE_BUS); + pci->bus = pci_register_root_bus(dev, s->name, xilinx_pcie_set_irq, + pci_swizzle_map_irq_fn, s, &s->mmio, + &s->io, 0, 4, TYPE_PCIE_BUS); qdev_set_parent_bus(DEVICE(&s->root), BUS(pci->bus)); qdev_init_nofail(DEVICE(&s->root)); diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b2d139bd9a..232e7dacf8 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -376,10 +376,10 @@ const char *pci_root_bus_path(PCIDevice *dev) return rootbus->qbus.name; } -static void pci_bus_init(PCIBus *bus, DeviceState *parent, - MemoryRegion *address_space_mem, - MemoryRegion *address_space_io, - uint8_t devfn_min) +static void pci_root_bus_init(PCIBus *bus, DeviceState *parent, + MemoryRegion *address_space_mem, + MemoryRegion *address_space_io, + uint8_t devfn_min) { assert(PCI_FUNC(devfn_min) == 0); bus->devfn_min = devfn_min; @@ -403,25 +403,27 @@ bool pci_bus_is_root(PCIBus *bus) return PCI_BUS_GET_CLASS(bus)->is_root(bus); } -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, - const char *name, +void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, + const char *name, + MemoryRegion *address_space_mem, + MemoryRegion *address_space_io, + uint8_t devfn_min, const char *typename) +{ + qbus_create_inplace(bus, bus_size, typename, parent, name); + pci_root_bus_init(bus, parent, address_space_mem, address_space_io, + devfn_min); +} + +PCIBus *pci_root_bus_new(DeviceState *parent, const char *name, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, uint8_t devfn_min, const char *typename) -{ - qbus_create_inplace(bus, bus_size, typename, parent, name); - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min); -} - -PCIBus *pci_bus_new(DeviceState *parent, const char *name, - MemoryRegion *address_space_mem, - MemoryRegion *address_space_io, - uint8_t devfn_min, const char *typename) { PCIBus *bus; bus = PCI_BUS(qbus_create(typename, parent, name)); - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min); + pci_root_bus_init(bus, parent, address_space_mem, address_space_io, + devfn_min); return bus; } @@ -435,17 +437,18 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0])); } -PCIBus *pci_register_bus(DeviceState *parent, const char *name, - pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, - void *irq_opaque, - MemoryRegion *address_space_mem, - MemoryRegion *address_space_io, - uint8_t devfn_min, int nirq, const char *typename) +PCIBus *pci_register_root_bus(DeviceState *parent, const char *name, + pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, + void *irq_opaque, + MemoryRegion *address_space_mem, + MemoryRegion *address_space_io, + uint8_t devfn_min, int nirq, + const char *typename) { PCIBus *bus; - bus = pci_bus_new(parent, name, address_space_mem, - address_space_io, devfn_min, typename); + bus = pci_root_bus_new(parent, name, address_space_mem, + address_space_io, devfn_min, typename); pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq); return bus; } diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c index 4765dcecca..b7642bac01 100644 --- a/hw/ppc/ppc4xx_pci.c +++ b/hw/ppc/ppc4xx_pci.c @@ -314,9 +314,9 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev) sysbus_init_irq(dev, &s->irq[i]); } - b = pci_register_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq, - ppc4xx_pci_map_irq, s->irq, get_system_memory(), - get_system_io(), 0, 4, TYPE_PCI_BUS); + b = pci_register_root_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq, + ppc4xx_pci_map_irq, s->irq, get_system_memory(), + get_system_io(), 0, 4, TYPE_PCI_BUS); h->bus = b; pci_create_simple(b, 0, "ppc4xx-host-bridge"); diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 5a3122a9f9..9262682116 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1621,10 +1621,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(get_system_memory(), sphb->io_win_addr, &sphb->iowindow); - bus = pci_register_bus(dev, NULL, - pci_spapr_set_irq, pci_spapr_map_irq, sphb, - &sphb->memspace, &sphb->iospace, - PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS); + bus = pci_register_root_bus(dev, NULL, + pci_spapr_set_irq, pci_spapr_map_irq, sphb, + &sphb->memspace, &sphb->iospace, + PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS); phb->bus = bus; qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL); diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 2b1e1409bf..347329dd50 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -554,10 +554,10 @@ static int s390_pcihost_init(SysBusDevice *dev) DPRINTF("host_init\n"); - b = pci_register_bus(DEVICE(dev), NULL, - s390_pci_set_irq, s390_pci_map_irq, NULL, - get_system_memory(), get_system_io(), 0, 64, - TYPE_PCI_BUS); + b = pci_register_root_bus(DEVICE(dev), NULL, + s390_pci_set_irq, s390_pci_map_irq, NULL, + get_system_memory(), get_system_io(), 0, 64, + TYPE_PCI_BUS); pci_setup_iommu(b, s390_pci_dma_iommu, s); bus = BUS(b); diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c index cbb01af57f..4ec2e35500 100644 --- a/hw/sh4/sh_pci.c +++ b/hw/sh4/sh_pci.c @@ -131,12 +131,12 @@ static int sh_pci_device_init(SysBusDevice *dev) for (i = 0; i < 4; i++) { sysbus_init_irq(dev, &s->irq[i]); } - phb->bus = pci_register_bus(DEVICE(dev), "pci", - sh_pci_set_irq, sh_pci_map_irq, - s->irq, - get_system_memory(), - get_system_io(), - PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS); + phb->bus = pci_register_root_bus(DEVICE(dev), "pci", + sh_pci_set_irq, sh_pci_map_irq, + s->irq, + get_system_memory(), + get_system_io(), + PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS); memory_region_init_io(&s->memconfig_p4, OBJECT(s), &sh_pci_reg_ops, s, "sh_pci", 0x224); memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2", diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 8d02a0a383..870ebcfd4b 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -400,26 +400,27 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); bool pci_bus_is_express(PCIBus *bus); bool pci_bus_is_root(PCIBus *bus); -void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, - const char *name, +void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, + const char *name, + MemoryRegion *address_space_mem, + MemoryRegion *address_space_io, + uint8_t devfn_min, const char *typename); +PCIBus *pci_root_bus_new(DeviceState *parent, const char *name, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, uint8_t devfn_min, const char *typename); -PCIBus *pci_bus_new(DeviceState *parent, const char *name, - MemoryRegion *address_space_mem, - MemoryRegion *address_space_io, - uint8_t devfn_min, const char *typename); void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, void *irq_opaque, int nirq); int pci_bus_get_irq_level(PCIBus *bus, int irq_num); /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */ int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin); -PCIBus *pci_register_bus(DeviceState *parent, const char *name, - pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, - void *irq_opaque, - MemoryRegion *address_space_mem, - MemoryRegion *address_space_io, - uint8_t devfn_min, int nirq, const char *typename); +PCIBus *pci_register_root_bus(DeviceState *parent, const char *name, + pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, + void *irq_opaque, + MemoryRegion *address_space_mem, + MemoryRegion *address_space_io, + uint8_t devfn_min, int nirq, + const char *typename); void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn); PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin); bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new); From 791bf3c8f042219a111aab3d00cd09a139629b59 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 29 Nov 2017 19:46:23 +1100 Subject: [PATCH 06/23] pci: Move bridge data structures from pci_bus.h to pci_bridge.h include/hw/pci/pci_bus.h contains several data structures related to PCI bridges that aren't needed by most users of pci_bus.h. We already have a pci_bridge.h, so move them there. Signed-off-by: David Gibson Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum Reviewed-by: Peter Xu --- include/hw/pci-host/xilinx-pcie.h | 2 +- include/hw/pci/pci_bridge.h | 48 +++++++++++++++++++++++++++++ include/hw/pci/pci_bus.h | 51 ++----------------------------- 3 files changed, 51 insertions(+), 50 deletions(-) diff --git a/include/hw/pci-host/xilinx-pcie.h b/include/hw/pci-host/xilinx-pcie.h index bec66b27c5..74c04dc9bb 100644 --- a/include/hw/pci-host/xilinx-pcie.h +++ b/include/hw/pci-host/xilinx-pcie.h @@ -23,7 +23,7 @@ #include "hw/hw.h" #include "hw/sysbus.h" #include "hw/pci/pci.h" -#include "hw/pci/pci_bus.h" +#include "hw/pci/pci_bridge.h" #include "hw/pci/pcie_host.h" #define TYPE_XILINX_PCIE_HOST "xilinx-pcie-host" diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 1acadc2c15..9b44ffd22a 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -27,6 +27,54 @@ #define QEMU_PCI_BRIDGE_H #include "hw/pci/pci.h" +#include "hw/pci/pci_bus.h" + +typedef struct PCIBridgeWindows PCIBridgeWindows; + +/* + * Aliases for each of the address space windows that the bridge + * can forward. Mapped into the bridge's parent's address space, + * as subregions. + */ +struct PCIBridgeWindows { + MemoryRegion alias_pref_mem; + MemoryRegion alias_mem; + MemoryRegion alias_io; + /* + * When bridge control VGA forwarding is enabled, bridges will + * provide positive decode on the PCI VGA defined I/O port and + * MMIO ranges. When enabled forwarding is only qualified on the + * I/O and memory enable bits in the bridge command register. + */ + MemoryRegion alias_vga[QEMU_PCI_VGA_NUM_REGIONS]; +}; + +#define TYPE_PCI_BRIDGE "base-pci-bridge" +#define PCI_BRIDGE(obj) OBJECT_CHECK(PCIBridge, (obj), TYPE_PCI_BRIDGE) + +struct PCIBridge { + /*< private >*/ + PCIDevice parent_obj; + /*< public >*/ + + /* private member */ + PCIBus sec_bus; + /* + * Memory regions for the bridge's address spaces. These regions are not + * directly added to system_memory/system_io or its descendants. + * Bridge's secondary bus points to these, so that devices + * under the bridge see these regions as its address spaces. + * The regions are as large as the entire address space - + * they don't take into account any windows. + */ + MemoryRegion address_space_mem; + MemoryRegion address_space_io; + + PCIBridgeWindows *windows; + + pci_map_irq_fn map_irq; + const char *bus_name; +}; #define PCI_BRIDGE_DEV_PROP_CHASSIS_NR "chassis_nr" #define PCI_BRIDGE_DEV_PROP_MSI "msi" diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h index bc34fd0017..b7da8f555b 100644 --- a/include/hw/pci/pci_bus.h +++ b/include/hw/pci/pci_bus.h @@ -2,10 +2,10 @@ #define QEMU_PCI_BUS_H /* - * PCI Bus and Bridge datastructures. + * PCI Bus datastructures. * * Do not access the following members directly; - * use accessor functions in pci.h, pci_bridge.h + * use accessor functions in pci.h */ typedef struct PCIBusClass { @@ -44,51 +44,4 @@ struct PCIBus { Notifier machine_done; }; -typedef struct PCIBridgeWindows PCIBridgeWindows; - -/* - * Aliases for each of the address space windows that the bridge - * can forward. Mapped into the bridge's parent's address space, - * as subregions. - */ -struct PCIBridgeWindows { - MemoryRegion alias_pref_mem; - MemoryRegion alias_mem; - MemoryRegion alias_io; - /* - * When bridge control VGA forwarding is enabled, bridges will - * provide positive decode on the PCI VGA defined I/O port and - * MMIO ranges. When enabled forwarding is only qualified on the - * I/O and memory enable bits in the bridge command register. - */ - MemoryRegion alias_vga[QEMU_PCI_VGA_NUM_REGIONS]; -}; - -#define TYPE_PCI_BRIDGE "base-pci-bridge" -#define PCI_BRIDGE(obj) OBJECT_CHECK(PCIBridge, (obj), TYPE_PCI_BRIDGE) - -struct PCIBridge { - /*< private >*/ - PCIDevice parent_obj; - /*< public >*/ - - /* private member */ - PCIBus sec_bus; - /* - * Memory regions for the bridge's address spaces. These regions are not - * directly added to system_memory/system_io or its descendants. - * Bridge's secondary bus points to these, so that devices - * under the bridge see these regions as its address spaces. - * The regions are as large as the entire address space - - * they don't take into account any windows. - */ - MemoryRegion address_space_mem; - MemoryRegion address_space_io; - - PCIBridgeWindows *windows; - - pci_map_irq_fn map_irq; - const char *bus_name; -}; - #endif /* QEMU_PCI_BUS_H */ From cdc57472dcc2ddc440545bde26791a11b42232b6 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 29 Nov 2017 19:46:26 +1100 Subject: [PATCH 07/23] pci: Add pci_dev_bus_num() helper A fair proportion of the users of pci_bus_num() want to get the bus number on a specific device, so first have to look up the bus from the device then call it. This adds a helper to do that (since we're going to make looking up the bus slightly more verbose). Signed-off-by: David Gibson Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum Reviewed-by: Peter Xu --- hw/pci/pcie_aer.c | 2 +- hw/s390x/s390-pci-bus.c | 2 +- hw/scsi/megasas.c | 2 +- hw/scsi/mptsas.c | 2 +- hw/xen/xen_pt.c | 6 +++--- include/hw/pci/pci.h | 5 +++++ include/hw/xen/xen_common.h | 8 ++++---- 7 files changed, 16 insertions(+), 11 deletions(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 97200742b4..21f896aadd 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -1025,7 +1025,7 @@ static int do_pcie_aer_inject_error(Monitor *mon, } details->id = id; details->root_bus = pci_root_bus_path(dev); - details->bus = pci_bus_num(dev->bus); + details->bus = pci_dev_bus_num(dev); details->devfn = dev->devfn; return 0; diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 347329dd50..f64ad5943e 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -692,7 +692,7 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev, /* In the case the PCI device does not define an id */ /* we generate one based on the PCI address */ dev->id = g_strdup_printf("auto_%02x:%02x.%01x", - pci_bus_num(pdev->bus), + pci_dev_bus_num(pdev), PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); } diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index d5eae6239a..3e38e9e8aa 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -2372,7 +2372,7 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp) if (!s->sas_addr) { s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) | IEEE_COMPANY_LOCALLY_ASSIGNED) << 36; - s->sas_addr |= (pci_bus_num(dev->bus) << 16); + s->sas_addr |= (pci_dev_bus_num(dev) << 16); s->sas_addr |= (PCI_SLOT(dev->devfn) << 8); s->sas_addr |= PCI_FUNC(dev->devfn); } diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c index f6db1b0103..3f061f3f68 100644 --- a/hw/scsi/mptsas.c +++ b/hw/scsi/mptsas.c @@ -1312,7 +1312,7 @@ static void mptsas_scsi_realize(PCIDevice *dev, Error **errp) if (!s->sas_addr) { s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) | IEEE_COMPANY_LOCALLY_ASSIGNED) << 36; - s->sas_addr |= (pci_bus_num(dev->bus) << 16); + s->sas_addr |= (pci_dev_bus_num(dev) << 16); s->sas_addr |= (PCI_SLOT(dev->devfn) << 8); s->sas_addr |= PCI_FUNC(dev->devfn); } diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 9bba717708..6236f0c391 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -73,7 +73,7 @@ void xen_pt_log(const PCIDevice *d, const char *f, ...) va_start(ap, f); if (d) { - fprintf(stderr, "[%02x:%02x.%d] ", pci_bus_num(d->bus), + fprintf(stderr, "[%02x:%02x.%d] ", pci_dev_bus_num(d), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn)); } vfprintf(stderr, f, ap); @@ -711,7 +711,7 @@ static void xen_pt_destroy(PCIDevice *d) { intx = xen_pt_pci_intx(s); rc = xc_domain_unbind_pt_irq(xen_xc, xen_domid, machine_irq, PT_IRQ_TYPE_PCI, - pci_bus_num(d->bus), + pci_dev_bus_num(d), PCI_SLOT(s->dev.devfn), intx, 0 /* isa_irq */); @@ -867,7 +867,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp) uint8_t e_intx = xen_pt_pci_intx(s); rc = xc_domain_bind_pt_pci_irq(xen_xc, xen_domid, machine_irq, - pci_bus_num(d->bus), + pci_dev_bus_num(d), PCI_SLOT(d->devfn), e_intx); if (rc < 0) { diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 870ebcfd4b..e451235065 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -436,6 +436,11 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, PCIDevice *pci_vga_init(PCIBus *bus); int pci_bus_num(PCIBus *s); +static inline int pci_dev_bus_num(const PCIDevice *dev) +{ + return pci_bus_num(dev->bus); +} + int pci_bus_numa_node(PCIBus *bus); void pci_for_each_device(PCIBus *bus, int bus_num, void (*fn)(PCIBus *bus, PCIDevice *d, void *opaque), diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h index 86c7f26106..64a978e4e0 100644 --- a/include/hw/xen/xen_common.h +++ b/include/hw/xen/xen_common.h @@ -542,10 +542,10 @@ static inline void xen_map_pcidev(domid_t dom, return; } - trace_xen_map_pcidev(ioservid, pci_bus_num(pci_dev->bus), + trace_xen_map_pcidev(ioservid, pci_dev_bus_num(pci_dev), PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn)); xendevicemodel_map_pcidev_to_ioreq_server(xen_dmod, dom, ioservid, 0, - pci_bus_num(pci_dev->bus), + pci_dev_bus_num(pci_dev), PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn)); } @@ -558,10 +558,10 @@ static inline void xen_unmap_pcidev(domid_t dom, return; } - trace_xen_unmap_pcidev(ioservid, pci_bus_num(pci_dev->bus), + trace_xen_unmap_pcidev(ioservid, pci_dev_bus_num(pci_dev), PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn)); xendevicemodel_unmap_pcidev_from_ioreq_server(xen_dmod, dom, ioservid, 0, - pci_bus_num(pci_dev->bus), + pci_dev_bus_num(pci_dev), PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn)); } From fd56e0612b6454a282fa6a953fdb09281a98c589 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 29 Nov 2017 19:46:27 +1100 Subject: [PATCH 08/23] pci: Eliminate redundant PCIDevice::bus pointer The bus pointer in PCIDevice is basically redundant with QOM information. It's always initialized to the qdev_get_parent_bus(), the only difference is the type. Therefore this patch eliminates the field, instead creating a pci_get_bus() helper to do the type mangling to derive it conveniently from the QOM Device object underneath. Signed-off-by: David Gibson Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Eduardo Habkost Reviewed-by: Marcel Apfelbaum Reviewed-by: Peter Xu --- hw/acpi/pcihp.c | 4 +- hw/acpi/piix4.c | 7 +-- hw/i386/xen/xen_platform.c | 12 ++--- hw/isa/lpc_ich9.c | 10 ++-- hw/net/vmxnet3.c | 2 +- hw/pci-bridge/pci_expander_bridge.c | 17 ++++--- hw/pci-host/piix.c | 10 ++-- hw/pci-host/versatile.c | 2 +- hw/pci/pci.c | 76 +++++++++++++++-------------- hw/pci/pci_bridge.c | 6 +-- hw/pci/pcie.c | 5 +- hw/pci/pcie_aer.c | 2 +- hw/ppc/spapr_pci.c | 2 +- hw/s390x/s390-pci-bus.c | 8 +-- hw/scsi/vmw_pvscsi.c | 2 +- hw/usb/hcd-xhci.c | 2 +- hw/vfio/pci.c | 10 ++-- hw/virtio/virtio-pci.c | 4 +- hw/xen/xen_pt.c | 4 +- include/hw/pci/pci.h | 9 ++-- 20 files changed, 102 insertions(+), 92 deletions(-) diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index 7da51c0569..91c82fdc7a 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -223,7 +223,7 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, { PCIDevice *pdev = PCI_DEVICE(dev); int slot = PCI_SLOT(pdev->devfn); - int bsel = acpi_pcihp_get_bsel(pdev->bus); + int bsel = acpi_pcihp_get_bsel(pci_get_bus(pdev)); if (bsel < 0) { error_setg(errp, "Unsupported bus. Bus doesn't have property '" ACPI_PCIHP_PROP_BSEL "' set"); @@ -246,7 +246,7 @@ void acpi_pcihp_device_unplug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, { PCIDevice *pdev = PCI_DEVICE(dev); int slot = PCI_SLOT(pdev->devfn); - int bsel = acpi_pcihp_get_bsel(pdev->bus); + int bsel = acpi_pcihp_get_bsel(pci_get_bus(pdev)); if (bsel < 0) { error_setg(errp, "Unsupported bus. Bus doesn't have property '" ACPI_PCIHP_PROP_BSEL "' set"); diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index a0fb1ce037..8b703455b7 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -460,9 +460,9 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) (memory_region_present(io_as, 0x2f8) ? 0x90 : 0); if (s->use_acpi_pci_hotplug) { - pci_for_each_bus(d->bus, piix4_update_bus_hotplug, s); + pci_for_each_bus(pci_get_bus(d), piix4_update_bus_hotplug, s); } else { - piix4_update_bus_hotplug(d->bus, s); + piix4_update_bus_hotplug(pci_get_bus(d), s); } } @@ -535,7 +535,8 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp) qemu_add_machine_init_done_notifier(&s->machine_ready); qemu_register_reset(piix4_reset, s); - piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, s); + piix4_acpi_system_hot_add_init(pci_address_space_io(dev), + pci_get_bus(dev), s); piix4_pm_add_propeties(s); } diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c index 056b87de0b..9ab54834d5 100644 --- a/hw/i386/xen/xen_platform.c +++ b/hw/i386/xen/xen_platform.c @@ -186,11 +186,11 @@ static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t v if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS | UNPLUG_NVME_DISKS)) { DPRINTF("unplug disks\n"); - pci_unplug_disks(pci_dev->bus, val); + pci_unplug_disks(pci_get_bus(pci_dev), val); } if (val & UNPLUG_ALL_NICS) { DPRINTF("unplug nics\n"); - pci_unplug_nics(pci_dev->bus); + pci_unplug_nics(pci_get_bus(pci_dev)); } break; } @@ -372,17 +372,17 @@ static void xen_platform_ioport_writeb(void *opaque, hwaddr addr, * If VMDP was to control both disk and LAN it would use 4. * If it controlled just disk or just LAN, it would use 8 below. */ - pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS); - pci_unplug_nics(pci_dev->bus); + pci_unplug_disks(pci_get_bus(pci_dev), UNPLUG_IDE_SCSI_DISKS); + pci_unplug_nics(pci_get_bus(pci_dev)); } break; case 8: switch (val) { case 1: - pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS); + pci_unplug_disks(pci_get_bus(pci_dev), UNPLUG_IDE_SCSI_DISKS); break; case 2: - pci_unplug_nics(pci_dev->bus); + pci_unplug_nics(pci_get_bus(pci_dev)); break; default: log_writeb(s, (uint32_t)val); diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index ec3c9f7d0b..adcf077fa5 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -162,7 +162,7 @@ static void ich9_cc_write(void *opaque, hwaddr addr, ich9_cc_addr_len(&addr, &len); memcpy(lpc->chip_config + addr, &val, len); - pci_bus_fire_intx_routing_notifier(lpc->d.bus); + pci_bus_fire_intx_routing_notifier(pci_get_bus(&lpc->d)); ich9_cc_update(lpc); } @@ -218,7 +218,7 @@ static void ich9_lpc_update_pic(ICH9LPCState *lpc, int gsi) int tmp_dis; ich9_lpc_pic_irq(lpc, i, &tmp_irq, &tmp_dis); if (!tmp_dis && tmp_irq == gsi) { - pic_level |= pci_bus_get_irq_level(lpc->d.bus, i); + pic_level |= pci_bus_get_irq_level(pci_get_bus(&lpc->d), i); } } if (gsi == lpc->sci_gsi) { @@ -246,7 +246,7 @@ static void ich9_lpc_update_apic(ICH9LPCState *lpc, int gsi) assert(gsi >= ICH9_LPC_PIC_NUM_PINS); - level |= pci_bus_get_irq_level(lpc->d.bus, ich9_gsi_to_pirq(gsi)); + level |= pci_bus_get_irq_level(pci_get_bus(&lpc->d), ich9_gsi_to_pirq(gsi)); if (gsi == lpc->sci_gsi) { level |= lpc->sci_level; } @@ -524,10 +524,10 @@ static void ich9_lpc_config_write(PCIDevice *d, ich9_lpc_rcba_update(lpc, rcba_old); } if (ranges_overlap(addr, len, ICH9_LPC_PIRQA_ROUT, 4)) { - pci_bus_fire_intx_routing_notifier(lpc->d.bus); + pci_bus_fire_intx_routing_notifier(pci_get_bus(&lpc->d)); } if (ranges_overlap(addr, len, ICH9_LPC_PIRQE_ROUT, 4)) { - pci_bus_fire_intx_routing_notifier(lpc->d.bus); + pci_bus_fire_intx_routing_notifier(pci_get_bus(&lpc->d)); } if (ranges_overlap(addr, len, ICH9_LPC_GEN_PMCON_1, 8)) { ich9_lpc_pmcon_update(lpc); diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index b8404cb2e2..0654d594c1 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2356,7 +2356,7 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) vmxnet3_net_init(s); if (pci_is_express(pci_dev)) { - if (pci_bus_is_express(pci_dev->bus)) { + if (pci_bus_is_express(pci_get_bus(pci_dev))) { pcie_endpoint_cap_init(pci_dev, VMXNET3_EXP_EP_OFFSET); } diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index b2fa829e29..2a81eec943 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -52,7 +52,8 @@ typedef struct PXBDev { static PXBDev *convert_to_pxb(PCIDevice *dev) { - return pci_bus_is_express(dev->bus) ? PXB_PCIE_DEV(dev) : PXB_DEV(dev); + return pci_bus_is_express(pci_get_bus(dev)) + ? PXB_PCIE_DEV(dev) : PXB_DEV(dev); } static GList *pxb_dev_list; @@ -166,7 +167,7 @@ static const TypeInfo pxb_host_info = { */ static void pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus, Error **errp) { - PCIBus *bus = dev->bus; + PCIBus *bus = pci_get_bus(dev); int pxb_bus_num = pci_bus_num(pxb_bus); if (bus->parent_dev) { @@ -180,12 +181,12 @@ static void pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus, Error **errp) return; } } - QLIST_INSERT_HEAD(&dev->bus->child, pxb_bus, sibling); + QLIST_INSERT_HEAD(&pci_get_bus(dev)->child, pxb_bus, sibling); } static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin) { - PCIDevice *pxb = pci_dev->bus->parent_dev; + PCIDevice *pxb = pci_get_bus(pci_dev)->parent_dev; /* * The bios does not index the pxb slot number when @@ -240,8 +241,8 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) } bus->parent_dev = dev; - bus->address_space_mem = dev->bus->address_space_mem; - bus->address_space_io = dev->bus->address_space_io; + bus->address_space_mem = pci_get_bus(dev)->address_space_mem; + bus->address_space_io = pci_get_bus(dev)->address_space_io; bus->map_irq = pxb_map_irq_fn; PCI_HOST_BRIDGE(ds)->bus = bus; @@ -272,7 +273,7 @@ err_register_bus: static void pxb_dev_realize(PCIDevice *dev, Error **errp) { - if (pci_bus_is_express(dev->bus)) { + if (pci_bus_is_express(pci_get_bus(dev))) { error_setg(errp, "pxb devices cannot reside on a PCIe bus"); return; } @@ -324,7 +325,7 @@ static const TypeInfo pxb_dev_info = { static void pxb_pcie_dev_realize(PCIDevice *dev, Error **errp) { - if (!pci_bus_is_express(dev->bus)) { + if (!pci_bus_is_express(pci_get_bus(dev))) { error_setg(errp, "pxb-pcie devices cannot reside on a PCI bus"); return; } diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index cf9070186c..effe3db8e2 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -512,12 +512,12 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin) /* irq routing is changed. so rebuild bitmap */ static void piix3_update_irq_levels(PIIX3State *piix3) { + PCIBus *bus = pci_get_bus(&piix3->dev); int pirq; piix3->pic_levels = 0; for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) { - piix3_set_irq_level(piix3, pirq, - pci_bus_get_irq_level(piix3->dev.bus, pirq)); + piix3_set_irq_level(piix3, pirq, pci_bus_get_irq_level(bus, pirq)); } } @@ -529,7 +529,7 @@ static void piix3_write_config(PCIDevice *dev, PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); int pic_irq; - pci_bus_fire_intx_routing_notifier(piix3->dev.bus); + pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix3->dev)); piix3_update_irq_levels(piix3); for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) { piix3_set_irq_pic(piix3, pic_irq); @@ -601,7 +601,7 @@ static int piix3_post_load(void *opaque, int version_id) piix3->pic_levels = 0; for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) { piix3_set_irq_level_internal(piix3, pirq, - pci_bus_get_irq_level(piix3->dev.bus, pirq)); + pci_bus_get_irq_level(pci_get_bus(&piix3->dev), pirq)); } return 0; } @@ -613,7 +613,7 @@ static int piix3_pre_save(void *opaque) for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) { piix3->pci_irq_levels_vmstate[i] = - pci_bus_get_irq_level(piix3->dev.bus, i); + pci_bus_get_irq_level(pci_get_bus(&piix3->dev), i); } return 0; diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c index 8803ada925..d0b02bdc47 100644 --- a/hw/pci-host/versatile.c +++ b/hw/pci-host/versatile.c @@ -311,7 +311,7 @@ static const MemoryRegionOps pci_vpb_config_ops = { static int pci_vpb_map_irq(PCIDevice *d, int irq_num) { - PCIVPBState *s = container_of(d->bus, PCIVPBState, pci_bus); + PCIVPBState *s = container_of(pci_get_bus(d), PCIVPBState, pci_bus); if (s->irq_mapping == PCI_VPB_IRQMAP_BROKEN) { /* Legacy broken IRQ mapping for compatibility with old and diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 232e7dacf8..567be1bb06 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -222,7 +222,7 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change) { PCIBus *bus; for (;;) { - bus = pci_dev->bus; + bus = pci_get_bus(pci_dev); irq_num = bus->map_irq(pci_dev, irq_num); if (bus->set_irq) break; @@ -349,13 +349,13 @@ PCIBus *pci_find_primary_bus(void) PCIBus *pci_device_root_bus(const PCIDevice *d) { - PCIBus *bus = d->bus; + PCIBus *bus = pci_get_bus(d); while (!pci_bus_is_root(bus)) { d = bus->parent_dev; assert(d != NULL); - bus = d->bus; + bus = pci_get_bus(d); } return bus; @@ -882,7 +882,7 @@ static void pci_config_free(PCIDevice *pci_dev) static void do_pci_unregister_device(PCIDevice *pci_dev) { - pci_dev->bus->devices[pci_dev->devfn] = NULL; + pci_get_bus(pci_dev)->devices[pci_dev->devfn] = NULL; pci_config_free(pci_dev); if (memory_region_is_mapped(&pci_dev->bus_master_enable_region)) { @@ -903,7 +903,7 @@ static uint16_t pci_req_id_cache_extract(PCIReqIDCache *cache) result = pci_get_bdf(cache->dev); break; case PCI_REQ_ID_SECONDARY_BUS: - bus_n = pci_bus_num(cache->dev->bus); + bus_n = pci_dev_bus_num(cache->dev); result = PCI_BUILD_BDF(bus_n, 0); break; default: @@ -933,9 +933,9 @@ static PCIReqIDCache pci_req_id_cache_get(PCIDevice *dev) .type = PCI_REQ_ID_BDF, }; - while (!pci_bus_is_root(dev->bus)) { + while (!pci_bus_is_root(pci_get_bus(dev))) { /* We are under PCI/PCIe bridges */ - parent = dev->bus->parent_dev; + parent = pci_get_bus(dev)->parent_dev; if (pci_is_express(parent)) { if (pcie_cap_get_type(parent) == PCI_EXP_TYPE_PCI_BRIDGE) { /* When we pass through PCIe-to-PCI/PCIX bridges, we @@ -978,7 +978,7 @@ static bool pci_bus_devfn_reserved(PCIBus *bus, int devfn) } /* -1 for devfn means auto assign */ -static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, +static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, const char *name, int devfn, Error **errp) { @@ -987,8 +987,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, PCIConfigWriteFunc *config_write = pc->config_write; Error *local_err = NULL; DeviceState *dev = DEVICE(pci_dev); + PCIBus *bus = pci_get_bus(pci_dev); - pci_dev->bus = bus; /* Only pci bridges can be attached to extra PCI root buses */ if (pci_bus_is_root(bus) && bus->parent_dev && !pc->is_bridge) { error_setg(errp, @@ -1142,8 +1142,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, r->type = type; r->memory = memory; r->address_space = type & PCI_BASE_ADDRESS_SPACE_IO - ? pci_dev->bus->address_space_io - : pci_dev->bus->address_space_mem; + ? pci_get_bus(pci_dev)->address_space_io + : pci_get_bus(pci_dev)->address_space_mem; wmask = ~(size - 1); if (region_num == PCI_ROM_SLOT) { @@ -1185,21 +1185,23 @@ static void pci_update_vga(PCIDevice *pci_dev) void pci_register_vga(PCIDevice *pci_dev, MemoryRegion *mem, MemoryRegion *io_lo, MemoryRegion *io_hi) { + PCIBus *bus = pci_get_bus(pci_dev); + assert(!pci_dev->has_vga); assert(memory_region_size(mem) == QEMU_PCI_VGA_MEM_SIZE); pci_dev->vga_regions[QEMU_PCI_VGA_MEM] = mem; - memory_region_add_subregion_overlap(pci_dev->bus->address_space_mem, + memory_region_add_subregion_overlap(bus->address_space_mem, QEMU_PCI_VGA_MEM_BASE, mem, 1); assert(memory_region_size(io_lo) == QEMU_PCI_VGA_IO_LO_SIZE); pci_dev->vga_regions[QEMU_PCI_VGA_IO_LO] = io_lo; - memory_region_add_subregion_overlap(pci_dev->bus->address_space_io, + memory_region_add_subregion_overlap(bus->address_space_io, QEMU_PCI_VGA_IO_LO_BASE, io_lo, 1); assert(memory_region_size(io_hi) == QEMU_PCI_VGA_IO_HI_SIZE); pci_dev->vga_regions[QEMU_PCI_VGA_IO_HI] = io_hi; - memory_region_add_subregion_overlap(pci_dev->bus->address_space_io, + memory_region_add_subregion_overlap(bus->address_space_io, QEMU_PCI_VGA_IO_HI_BASE, io_hi, 1); pci_dev->has_vga = true; @@ -1208,15 +1210,17 @@ void pci_register_vga(PCIDevice *pci_dev, MemoryRegion *mem, void pci_unregister_vga(PCIDevice *pci_dev) { + PCIBus *bus = pci_get_bus(pci_dev); + if (!pci_dev->has_vga) { return; } - memory_region_del_subregion(pci_dev->bus->address_space_mem, + memory_region_del_subregion(bus->address_space_mem, pci_dev->vga_regions[QEMU_PCI_VGA_MEM]); - memory_region_del_subregion(pci_dev->bus->address_space_io, + memory_region_del_subregion(bus->address_space_io, pci_dev->vga_regions[QEMU_PCI_VGA_IO_LO]); - memory_region_del_subregion(pci_dev->bus->address_space_io, + memory_region_del_subregion(bus->address_space_io, pci_dev->vga_regions[QEMU_PCI_VGA_IO_HI]); pci_dev->has_vga = false; } @@ -1319,7 +1323,7 @@ static void pci_update_mappings(PCIDevice *d) /* now do the real mapping */ if (r->addr != PCI_BAR_UNMAPPED) { - trace_pci_update_mappings_del(d, pci_bus_num(d->bus), + trace_pci_update_mappings_del(d, pci_dev_bus_num(d), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn), i, r->addr, r->size); @@ -1327,7 +1331,7 @@ static void pci_update_mappings(PCIDevice *d) } r->addr = new_addr; if (r->addr != PCI_BAR_UNMAPPED) { - trace_pci_update_mappings_add(d, pci_bus_num(d->bus), + trace_pci_update_mappings_add(d, pci_dev_bus_num(d), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn), i, r->addr, r->size); @@ -1446,9 +1450,9 @@ PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin) PCIBus *bus; do { - bus = dev->bus; - pin = bus->map_irq(dev, pin); - dev = bus->parent_dev; + bus = pci_get_bus(dev); + pin = bus->map_irq(dev, pin); + dev = bus->parent_dev; } while (dev); if (!bus->route_intx_to_irq) { @@ -2018,7 +2022,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) PCIDevice *pci_dev = (PCIDevice *)qdev; PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); Error *local_err = NULL; - PCIBus *bus; bool is_default_rom; /* initialize cap_present for pci_is_express() and pci_config_size() */ @@ -2026,8 +2029,7 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS; } - bus = PCI_BUS(qdev_get_parent_bus(qdev)); - pci_dev = do_pci_register_device(pci_dev, bus, + pci_dev = do_pci_register_device(pci_dev, object_get_typename(OBJECT(qdev)), pci_dev->devfn, errp); if (pci_dev == NULL) @@ -2320,7 +2322,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, error_setg(errp, "%s:%02x:%02x.%x " "Attempt to add PCI capability %x at offset " "%x overlaps existing capability %x at offset %x", - pci_root_bus_path(pdev), pci_bus_num(pdev->bus), + pci_root_bus_path(pdev), pci_dev_bus_num(pdev), PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), cap_id, offset, overlapping_cap, i); return -EINVAL; @@ -2384,7 +2386,7 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent) monitor_printf(mon, "%*sclass %s, addr %02x:%02x.%x, " "pci id %04x:%04x (sub %04x:%04x)\n", - indent, "", ctxt, pci_bus_num(d->bus), + indent, "", ctxt, pci_dev_bus_num(d), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn), pci_get_word(d->config + PCI_VENDOR_ID), pci_get_word(d->config + PCI_DEVICE_ID), @@ -2467,7 +2469,7 @@ static char *pcibus_get_dev_path(DeviceState *dev) /* Calculate # of slots on path between device and root. */; slot_depth = 0; - for (t = d; t; t = t->bus->parent_dev) { + for (t = d; t; t = pci_get_bus(t)->parent_dev) { ++slot_depth; } @@ -2482,7 +2484,7 @@ static char *pcibus_get_dev_path(DeviceState *dev) /* Fill in slot numbers. We walk up from device to root, so need to print * them in the reverse order, last to first. */ p = path + path_len; - for (t = d; t; t = t->bus->parent_dev) { + for (t = d; t; t = pci_get_bus(t)->parent_dev) { p -= slot_len; s = snprintf(slot, sizeof slot, ":%02x.%x", PCI_SLOT(t->devfn), PCI_FUNC(t->devfn)); @@ -2530,12 +2532,12 @@ int pci_qdev_find_device(const char *id, PCIDevice **pdev) MemoryRegion *pci_address_space(PCIDevice *dev) { - return dev->bus->address_space_mem; + return pci_get_bus(dev)->address_space_mem; } MemoryRegion *pci_address_space_io(PCIDevice *dev) { - return dev->bus->address_space_io; + return pci_get_bus(dev)->address_space_io; } static void pci_device_class_init(ObjectClass *klass, void *data) @@ -2563,11 +2565,11 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) { - PCIBus *bus = PCI_BUS(dev->bus); + PCIBus *bus = pci_get_bus(dev); PCIBus *iommu_bus = bus; while(iommu_bus && !iommu_bus->iommu_fn && iommu_bus->parent_dev) { - iommu_bus = PCI_BUS(iommu_bus->parent_dev->bus); + iommu_bus = pci_get_bus(iommu_bus->parent_dev); } if (iommu_bus && iommu_bus->iommu_fn) { return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, dev->devfn); @@ -2638,7 +2640,7 @@ void pci_bus_get_w64_range(PCIBus *bus, Range *range) static bool pcie_has_upstream_port(PCIDevice *dev) { - PCIDevice *parent_dev = pci_bridge_get_device(dev->bus); + PCIDevice *parent_dev = pci_bridge_get_device(pci_get_bus(dev)); /* Device associated with an upstream port. * As there are several types of these, it's easier to check the @@ -2654,12 +2656,14 @@ static bool pcie_has_upstream_port(PCIDevice *dev) PCIDevice *pci_get_function_0(PCIDevice *pci_dev) { + PCIBus *bus = pci_get_bus(pci_dev); + if(pcie_has_upstream_port(pci_dev)) { /* With an upstream PCIe port, we only support 1 device at slot 0 */ - return pci_dev->bus->devices[0]; + return bus->devices[0]; } else { /* Other bus types might support multiple devices at slots 0-31 */ - return pci_dev->bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]; + return bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]; } } diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index a47d257149..b2e50c36a0 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -183,7 +183,7 @@ static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent, static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br) { PCIDevice *pd = PCI_DEVICE(br); - PCIBus *parent = pd->bus; + PCIBus *parent = pci_get_bus(pd); PCIBridgeWindows *w = g_new(PCIBridgeWindows, 1); uint16_t cmd = pci_get_word(pd->config + PCI_COMMAND); @@ -214,7 +214,7 @@ static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br) static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w) { PCIDevice *pd = PCI_DEVICE(br); - PCIBus *parent = pd->bus; + PCIBus *parent = pci_get_bus(pd); memory_region_del_subregion(parent->address_space_io, &w->alias_io); memory_region_del_subregion(parent->address_space_mem, &w->alias_mem); @@ -339,7 +339,7 @@ void pci_bridge_reset(DeviceState *qdev) /* default qdev initialization function for PCI-to-PCI bridge */ void pci_bridge_initfn(PCIDevice *dev, const char *typename) { - PCIBus *parent = dev->bus; + PCIBus *parent = pci_get_bus(dev); PCIBridge *br = PCI_BRIDGE(dev); PCIBus *sec_bus = &br->sec_bus; diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 32191f2a55..6c91bd44a0 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -155,7 +155,8 @@ pcie_endpoint_cap_common_init(PCIDevice *dev, uint8_t offset, uint8_t cap_size) * a regular Endpoint type is exposed on a root complex. These * should instead be Root Complex Integrated Endpoints. */ - if (pci_bus_is_express(dev->bus) && pci_bus_is_root(dev->bus)) { + if (pci_bus_is_express(pci_get_bus(dev)) + && pci_bus_is_root(pci_get_bus(dev))) { type = PCI_EXP_TYPE_RC_END; } @@ -369,7 +370,7 @@ void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev, { uint8_t *exp_cap; PCIDevice *pci_dev = PCI_DEVICE(dev); - PCIBus *bus = pci_dev->bus; + PCIBus *bus = pci_get_bus(pci_dev); pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp); diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 21f896aadd..b009be7f17 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -409,7 +409,7 @@ static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg) */ return; } - dev = pci_bridge_get_device(dev->bus); + dev = pci_bridge_get_device(pci_get_bus(dev)); } } diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 9262682116..f38be2f0b4 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -505,7 +505,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, goto param_error_exit; } - rtas_st(rets, 1, (pci_bus_num(pdev->bus) << 16) + 1); + rtas_st(rets, 1, (pci_bus_num(pci_get_bus(pdev)) << 16) + 1); break; case RTAS_GET_PE_MODE: rtas_st(rets, 1, RTAS_PE_MODE_SHARED); diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index f64ad5943e..7d9c65e719 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -680,10 +680,10 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev, s->bus_no += 1; pci_default_write_config(pdev, PCI_SECONDARY_BUS, s->bus_no, 1); do { - pdev = pdev->bus->parent_dev; + pdev = pci_get_bus(pdev)->parent_dev; pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, s->bus_no, 1); - } while (pdev->bus && pci_bus_num(pdev->bus)); + } while (pci_get_bus(pdev) && pci_dev_bus_num(pdev)); } } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { pdev = PCI_DEVICE(dev); @@ -713,7 +713,7 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev, } pbdev->pdev = pdev; - pbdev->iommu = s390_pci_get_iommu(s, pdev->bus, pdev->devfn); + pbdev->iommu = s390_pci_get_iommu(s, pci_get_bus(pdev), pdev->devfn); pbdev->iommu->pbdev = pbdev; pbdev->state = ZPCI_FS_DISABLED; @@ -807,7 +807,7 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev, s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED, pbdev->fh, pbdev->fid); - bus = pci_dev->bus; + bus = pci_get_bus(pci_dev); devfn = pci_dev->devfn; object_unparent(OBJECT(pci_dev)); s390_pci_msix_free(pbdev); diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c index d564e5caff..27749c0e42 100644 --- a/hw/scsi/vmw_pvscsi.c +++ b/hw/scsi/vmw_pvscsi.c @@ -1133,7 +1133,7 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp) pvscsi_init_msi(s); - if (pci_is_express(pci_dev) && pci_bus_is_express(pci_dev->bus)) { + if (pci_is_express(pci_dev) && pci_bus_is_express(pci_get_bus(pci_dev))) { pcie_endpoint_cap_init(pci_dev, PVSCSI_EXP_EP_OFFSET); } diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index af3a9d88de..228e82b3fb 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -3416,7 +3416,7 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp) PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64, &xhci->mem); - if (pci_bus_is_express(dev->bus) || + if (pci_bus_is_express(pci_get_bus(dev)) || xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) { ret = pcie_endpoint_cap_init(dev, 0xa0); assert(ret > 0); diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index c977ee327f..2c71295125 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1654,8 +1654,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size, return -EINVAL; } - if (!pci_bus_is_express(vdev->pdev.bus)) { - PCIBus *bus = vdev->pdev.bus; + if (!pci_bus_is_express(pci_get_bus(&vdev->pdev))) { + PCIBus *bus = pci_get_bus(&vdev->pdev); PCIDevice *bridge; /* @@ -1680,14 +1680,14 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size, */ while (!pci_bus_is_root(bus)) { bridge = pci_bridge_get_device(bus); - bus = bridge->bus; + bus = pci_get_bus(bridge); } if (pci_bus_is_express(bus)) { return 0; } - } else if (pci_bus_is_root(vdev->pdev.bus)) { + } else if (pci_bus_is_root(pci_get_bus(&vdev->pdev))) { /* * On a Root Complex bus Endpoints become Root Complex Integrated * Endpoints, which changes the type and clears the LNK & LNK2 fields. @@ -1890,7 +1890,7 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev) uint8_t *config; /* Only add extended caps if we have them and the guest can see them */ - if (!pci_is_express(pdev) || !pci_bus_is_express(pdev->bus) || + if (!pci_is_express(pdev) || !pci_bus_is_express(pci_get_bus(pdev)) || !pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) { return; } diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index e92837c42b..42b31fbcf8 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1708,8 +1708,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) { VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); VirtioPCIClass *k = VIRTIO_PCI_GET_CLASS(pci_dev); - bool pcie_port = pci_bus_is_express(pci_dev->bus) && - !pci_bus_is_root(pci_dev->bus); + bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) && + !pci_bus_is_root(pci_get_bus(pci_dev)); if (kvm_enabled() && !kvm_has_many_ioeventfds()) { proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD; diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 6236f0c391..752b6f6d5c 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -602,7 +602,7 @@ static void xen_pt_region_update(XenPCIPassthroughState *s, } args.type = d->io_regions[bar].type; - pci_for_each_device(d->bus, pci_bus_num(d->bus), + pci_for_each_device(pci_get_bus(d), pci_dev_bus_num(d), xen_pt_check_bar_overlap, &args); if (args.rc) { XEN_PT_WARN(d, "Region: %d (addr: %#"FMT_PCIBUS @@ -695,7 +695,7 @@ xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s, PCIDevice *d = &s->dev; gpu_dev_id = dev->device_id; - igd_passthrough_isa_bridge_create(d->bus, gpu_dev_id); + igd_passthrough_isa_bridge_create(pci_get_bus(d), gpu_dev_id); } /* destroy. */ diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index e451235065..597ffb78d3 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -285,7 +285,6 @@ struct PCIDevice { uint8_t *used; /* the following fields are read only */ - PCIBus *bus; int32_t devfn; /* Cached device to fetch requester ID from, to avoid the PCI * tree walking every time we invoke PCI request (e.g., @@ -435,10 +434,14 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, PCIDevice *pci_vga_init(PCIBus *bus); +static inline PCIBus *pci_get_bus(const PCIDevice *dev) +{ + return PCI_BUS(qdev_get_parent_bus(DEVICE(dev))); +} int pci_bus_num(PCIBus *s); static inline int pci_dev_bus_num(const PCIDevice *dev) { - return pci_bus_num(dev->bus); + return pci_bus_num(pci_get_bus(dev)); } int pci_bus_numa_node(PCIBus *bus); @@ -745,7 +748,7 @@ static inline uint32_t pci_config_size(const PCIDevice *d) static inline uint16_t pci_get_bdf(PCIDevice *dev) { - return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn); + return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn); } uint16_t pci_requester_id(PCIDevice *dev); From e492dc5a267e2236b93b8b7192fedd840ef34dc9 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 29 Nov 2017 19:46:28 +1100 Subject: [PATCH 09/23] pci: Eliminate pci_find_primary_bus() pci_find_primary_bus() only has one user, in pc_xen_hvm_init(). That's inside the machine construction code, so it already has easy access to the machine's primary PCI bus. Get it directly, and thereby remove pci_find_primary_bus(). This removes one of only a handful of users of the ugly pci_host_bridges global. Signed-off-by: David Gibson Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum Reviewed-by: Peter Xu --- hw/i386/pc_piix.c | 8 ++------ hw/pci/pci.c | 16 ---------------- include/hw/pci/pci.h | 1 - 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 5e47528993..2febd0e136 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -394,7 +394,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine) static void pc_xen_hvm_init(MachineState *machine) { - PCIBus *bus; + PCMachineState *pcms = PC_MACHINE(machine); if (!xen_enabled()) { error_report("xenfv machine requires the xen accelerator"); @@ -402,11 +402,7 @@ static void pc_xen_hvm_init(MachineState *machine) } pc_xen_hvm_init_pci(machine); - - bus = pci_find_primary_bus(); - if (bus != NULL) { - pci_create_simple(bus, -1, "xen-platform"); - } + pci_create_simple(pcms->bus, -1, "xen-platform"); } #endif diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 567be1bb06..e8f9fc1c27 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -331,22 +331,6 @@ static void pci_host_bus_register(DeviceState *host) QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next); } -PCIBus *pci_find_primary_bus(void) -{ - PCIBus *primary_bus = NULL; - PCIHostState *host; - - QLIST_FOREACH(host, &pci_host_bridges, next) { - if (primary_bus) { - /* We have multiple root buses, refuse to select a primary */ - return NULL; - } - primary_bus = host->bus; - } - - return primary_bus; -} - PCIBus *pci_device_root_bus(const PCIDevice *d) { PCIBus *bus = pci_get_bus(d); diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 597ffb78d3..15ced9648c 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -467,7 +467,6 @@ void pci_for_each_bus(PCIBus *bus, pci_for_each_bus_depth_first(bus, NULL, fn, opaque); } -PCIBus *pci_find_primary_bus(void); PCIBus *pci_device_root_bus(const PCIDevice *d); const char *pci_root_bus_path(PCIDevice *dev); PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn); From 8fc47c876de638353bb635872f2c25bb7f4a3d6e Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 13 Dec 2017 21:59:54 +0200 Subject: [PATCH 10/23] virtio_error: don't invoke status callbacks Backends don't need to know what frontend requested a reset, and notifying then from virtio_error is messy because virtio_error itself might be invoked from backend. Let's just set the status directly. Cc: qemu-stable@nongnu.org Reported-by: Ilya Maximets Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index ad564b0132..d6002ee550 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2469,7 +2469,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) va_end(ap); if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { - virtio_set_status(vdev, vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET); + vdev->status = vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET; virtio_notify_config(vdev); } From 5c96e091e8fb2bb7809bb1bfcd1dc1e954627292 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 19 Dec 2017 15:45:19 +1100 Subject: [PATCH 11/23] tests/pxe-test: Remove unnecessary special case test functions All of the x86 and some of the other test cases here use a common test function, test_pxe_ipv4(), but one ppc and one s390 test use different functions. In the s390 case, this is completely pointless, the right parameter to test_pxe_ipv4() will already do exactly the right thing. For the spapr-vlan case there's a slight difference - it will use IPv6 instead of IPv4. But testing just one case with IPv6 (and NOT IPv4) is rather haphazard. Change everything to use the common test function, until we have a better way of testing IPv6 across the board. Signed-off-by: David Gibson Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/pxe-test.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/pxe-test.c b/tests/pxe-test.c index 937f29e631..eb70aa2bc6 100644 --- a/tests/pxe-test.c +++ b/tests/pxe-test.c @@ -47,16 +47,6 @@ static void test_pxe_ipv4(gconstpointer data) g_free(dev_arg); } -static void test_pxe_spapr_vlan(void) -{ - test_pxe_one("-device spapr-vlan,netdev=" NETNAME, true); -} - -static void test_pxe_virtio_ccw(void) -{ - test_pxe_one("-device virtio-net-ccw,bootindex=1,netdev=" NETNAME, false); -} - int main(int argc, char *argv[]) { int ret; @@ -79,13 +69,14 @@ int main(int argc, char *argv[]) qtest_add_data_func("pxe/vmxnet3", "vmxnet3", test_pxe_ipv4); } } else if (strcmp(arch, "ppc64") == 0) { - qtest_add_func("pxe/spapr-vlan", test_pxe_spapr_vlan); + qtest_add_data_func("pxe/spapr-vlan", "spapr-vlan", test_pxe_ipv4); if (g_test_slow()) { qtest_add_data_func("pxe/virtio", "virtio-net-pci", test_pxe_ipv4); qtest_add_data_func("pxe/e1000", "e1000", test_pxe_ipv4); } } else if (g_str_equal(arch, "s390x")) { - qtest_add_func("pxe/virtio-ccw", test_pxe_virtio_ccw); + qtest_add_data_func("pxe/virtio-ccw", + "virtio-net-ccw,bootindex=1", test_pxe_ipv4); } ret = g_test_run(); boot_sector_cleanup(disk); From 1e88989f6ae6699c3391630c6aac20988a47583a Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 19 Dec 2017 15:45:20 +1100 Subject: [PATCH 12/23] tests/pxe-test: Use table of testcases rather than open-coding Currently pxe-tests open codes the list of tests for each architecture. This changes it to use tables of test parameters, somewhat similar to boot-serial-test. This adds the machine type into the table as well, giving us the ability to perform tests on multiple machine types for architectures where there's more than one machine type that matters. NOTE: This changes the names of the tests in the output, to include the machine type and IPv4 vs. IPv6. I'm not sure if this has the potential to break existing tooling. Signed-off-by: David Gibson Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/pxe-test.c | 86 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 22 deletions(-) diff --git a/tests/pxe-test.c b/tests/pxe-test.c index eb70aa2bc6..4758ce7d20 100644 --- a/tests/pxe-test.c +++ b/tests/pxe-test.c @@ -22,14 +22,51 @@ static char disk[] = "tests/pxe-test-disk-XXXXXX"; -static void test_pxe_one(const char *params, bool ipv6) +typedef struct testdef { + const char *machine; /* Machine type */ + const char *model; /* NIC device model */ +} testdef_t; + +static testdef_t x86_tests[] = { + { "pc", "e1000" }, + { "pc", "virtio-net-pci" }, + { NULL }, +}; + +static testdef_t x86_tests_slow[] = { + { "pc", "ne2k_pci", }, + { "pc", "i82550", }, + { "pc", "rtl8139" }, + { "pc", "vmxnet3" }, + { NULL }, +}; + +static testdef_t ppc64_tests[] = { + { "pseries", "spapr-vlan" }, + { NULL }, +}; + +static testdef_t ppc64_tests_slow[] = { + { "pseries", "virtio-net-pci", }, + { "pseries", "e1000" }, + { NULL }, +}; + +static testdef_t s390x_tests[] = { + { "s390-ccw-virtio", "virtio-net-ccw" }, + { NULL }, +}; + +static void test_pxe_one(const testdef_t *test, bool ipv6) { char *args; - args = g_strdup_printf("-machine accel=kvm:tcg -nodefaults -boot order=n " - "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s," - "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : "on", - ipv6 ? "on" : "off", params); + args = g_strdup_printf( + "-machine %s,accel=kvm:tcg -nodefaults -boot order=n " + "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,ipv4=%s,ipv6=%s " + "-device %s,bootindex=1,netdev=" NETNAME, + test->machine, disk, ipv6 ? "off" : "on", ipv6 ? "on" : "off", + test->model); qtest_start(args); boot_sector_test(); @@ -39,12 +76,24 @@ static void test_pxe_one(const char *params, bool ipv6) static void test_pxe_ipv4(gconstpointer data) { - const char *model = data; - char *dev_arg; + const testdef_t *test = data; - dev_arg = g_strdup_printf("-device %s,netdev=" NETNAME, model); - test_pxe_one(dev_arg, false); - g_free(dev_arg); + test_pxe_one(test, false); +} + +static void test_batch(const testdef_t *tests) +{ + int i; + + for (i = 0; tests[i].machine; i++) { + const testdef_t *test = &tests[i]; + char *testname; + + testname = g_strdup_printf("pxe/ipv4/%s/%s", + test->machine, test->model); + qtest_add_data_func(testname, test, test_pxe_ipv4); + g_free(testname); + } } int main(int argc, char *argv[]) @@ -59,24 +108,17 @@ int main(int argc, char *argv[]) g_test_init(&argc, &argv, NULL); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { - qtest_add_data_func("pxe/e1000", "e1000", test_pxe_ipv4); - qtest_add_data_func("pxe/virtio", "virtio-net-pci", test_pxe_ipv4); + test_batch(x86_tests); if (g_test_slow()) { - qtest_add_data_func("pxe/ne2000", "ne2k_pci", test_pxe_ipv4); - qtest_add_data_func("pxe/eepro100", "i82550", test_pxe_ipv4); - qtest_add_data_func("pxe/pcnet", "pcnet", test_pxe_ipv4); - qtest_add_data_func("pxe/rtl8139", "rtl8139", test_pxe_ipv4); - qtest_add_data_func("pxe/vmxnet3", "vmxnet3", test_pxe_ipv4); + test_batch(x86_tests_slow); } } else if (strcmp(arch, "ppc64") == 0) { - qtest_add_data_func("pxe/spapr-vlan", "spapr-vlan", test_pxe_ipv4); + test_batch(ppc64_tests); if (g_test_slow()) { - qtest_add_data_func("pxe/virtio", "virtio-net-pci", test_pxe_ipv4); - qtest_add_data_func("pxe/e1000", "e1000", test_pxe_ipv4); + test_batch(ppc64_tests_slow); } } else if (g_str_equal(arch, "s390x")) { - qtest_add_data_func("pxe/virtio-ccw", - "virtio-net-ccw,bootindex=1", test_pxe_ipv4); + test_batch(s390x_tests); } ret = g_test_run(); boot_sector_cleanup(disk); From d23895d9ba182825b488a37699bae79c70729f5f Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 19 Dec 2017 15:45:21 +1100 Subject: [PATCH 13/23] tests/pxe-test: Test net booting over IPv6 in some cases This adds IPv6 net boot testing (in addition to IPv4) when in slow test mode on ppc64 or s390. IPv6 PXE doesn't seem to work on x86, I'm guessing our BIOS image doesn't support it. Signed-off-by: David Gibson Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/pxe-test.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/pxe-test.c b/tests/pxe-test.c index 4758ce7d20..5689c7d7ee 100644 --- a/tests/pxe-test.c +++ b/tests/pxe-test.c @@ -81,7 +81,14 @@ static void test_pxe_ipv4(gconstpointer data) test_pxe_one(test, false); } -static void test_batch(const testdef_t *tests) +static void test_pxe_ipv6(gconstpointer data) +{ + const testdef_t *test = data; + + test_pxe_one(test, true); +} + +static void test_batch(const testdef_t *tests, bool ipv6) { int i; @@ -93,6 +100,13 @@ static void test_batch(const testdef_t *tests) test->machine, test->model); qtest_add_data_func(testname, test, test_pxe_ipv4); g_free(testname); + + if (ipv6) { + testname = g_strdup_printf("pxe/ipv6/%s/%s", + test->machine, test->model); + qtest_add_data_func(testname, test, test_pxe_ipv6); + g_free(testname); + } } } @@ -108,17 +122,17 @@ int main(int argc, char *argv[]) g_test_init(&argc, &argv, NULL); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { - test_batch(x86_tests); + test_batch(x86_tests, false); if (g_test_slow()) { - test_batch(x86_tests_slow); + test_batch(x86_tests_slow, false); } } else if (strcmp(arch, "ppc64") == 0) { - test_batch(ppc64_tests); + test_batch(ppc64_tests, g_test_slow()); if (g_test_slow()) { - test_batch(ppc64_tests_slow); + test_batch(ppc64_tests_slow, true); } } else if (g_str_equal(arch, "s390x")) { - test_batch(s390x_tests); + test_batch(s390x_tests, g_test_slow()); } ret = g_test_run(); boot_sector_cleanup(disk); From 18b20bb43a2f37f0c8ae23a3e9b3d9a4a05b6bd4 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 19 Dec 2017 15:45:22 +1100 Subject: [PATCH 14/23] tests/pxe-test: Add some extra tests Previously virtio-net was only tested for ppc64 in "slow" mode. That doesn't make much sense since virtio-net is used much more often in practice than the spapr-vlan device which was tested always. So, move virtio-net to always be tested on ppc64. We had no tests at all for the q35 machine, which doesn't seem wise given its increasing prominence. Add a couple of tests for it, including testing the newer e1000e adapter. Signed-off-by: David Gibson Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/pxe-test.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/pxe-test.c b/tests/pxe-test.c index 5689c7d7ee..5ca84805eb 100644 --- a/tests/pxe-test.c +++ b/tests/pxe-test.c @@ -30,6 +30,8 @@ typedef struct testdef { static testdef_t x86_tests[] = { { "pc", "e1000" }, { "pc", "virtio-net-pci" }, + { "q35", "e1000e" }, + { "q35", "virtio-net-pci", }, { NULL }, }; @@ -43,11 +45,11 @@ static testdef_t x86_tests_slow[] = { static testdef_t ppc64_tests[] = { { "pseries", "spapr-vlan" }, + { "pseries", "virtio-net-pci", }, { NULL }, }; static testdef_t ppc64_tests_slow[] = { - { "pseries", "virtio-net-pci", }, { "pseries", "e1000" }, { NULL }, }; From 05607921e6b8b5f56e9f02e8b07ad869f5788457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 18 Dec 2017 12:12:42 -0300 Subject: [PATCH 15/23] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum --- hw/pci-host/piix.c | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index effe3db8e2..0e608347c1 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -804,60 +804,55 @@ static const IGDHostInfo igd_host_bridge_infos[] = { {0xa8, 4}, /* SNB: base of GTT stolen memory */ }; -static int host_pci_config_read(int pos, int len, uint32_t *val) +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp) { - char path[PATH_MAX]; - int config_fd; - ssize_t size = sizeof(path); + int rc, config_fd; /* Access real host bridge. */ - int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", - 0, 0, 0, 0, "config"); - int ret = 0; - - if (rc >= size || rc < 0) { - return -ENODEV; - } + char *path = g_strdup_printf("/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", + 0, 0, 0, 0, "config"); config_fd = open(path, O_RDWR); if (config_fd < 0) { - return -ENODEV; + error_setg_errno(errp, errno, "Failed to open: %s", path); + goto out; } if (lseek(config_fd, pos, SEEK_SET) != pos) { - ret = -errno; - goto out; + error_setg_errno(errp, errno, "Failed to seek: %s", path); + goto out_close_fd; } do { rc = read(config_fd, (uint8_t *)val, len); } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); if (rc != len) { - ret = -errno; + error_setg_errno(errp, errno, "Failed to read: %s", path); } -out: +out_close_fd: close(config_fd); - return ret; +out: + g_free(path); } -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { uint32_t val = 0; - int rc, i, num; + int i, num; int pos, len; + Error *local_err = NULL; num = ARRAY_SIZE(igd_host_bridge_infos); for (i = 0; i < num; i++) { pos = igd_host_bridge_infos[i].offset; len = igd_host_bridge_infos[i].len; - rc = host_pci_config_read(pos, len, &val); - if (rc) { - return -ENODEV; + host_pci_config_read(pos, len, &val, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; } pci_default_write_config(pci_dev, pos, val, len); } - - return 0; } static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) @@ -865,7 +860,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->init = igd_pt_i440fx_initfn; + k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } From 371e94ba5625a09b0e04ecf511988869a92f319f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 18 Dec 2017 12:12:43 -0300 Subject: [PATCH 16/23] hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marcel Apfelbaum --- hw/pci-host/xilinx-pcie.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c index d2f88d11dd..53b561f81f 100644 --- a/hw/pci-host/xilinx-pcie.c +++ b/hw/pci-host/xilinx-pcie.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/pci/pci_bridge.h" #include "hw/pci-host/xilinx-pcie.h" @@ -267,24 +268,22 @@ static void xilinx_pcie_root_config_write(PCIDevice *d, uint32_t address, } } -static int xilinx_pcie_root_init(PCIDevice *dev) +static void xilinx_pcie_root_realize(PCIDevice *pci_dev, Error **errp) { - BusState *bus = qdev_get_parent_bus(DEVICE(dev)); + BusState *bus = qdev_get_parent_bus(DEVICE(pci_dev)); XilinxPCIEHost *s = XILINX_PCIE_HOST(bus->parent); - pci_set_word(dev->config + PCI_COMMAND, + pci_set_word(pci_dev->config + PCI_COMMAND, PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER); - pci_set_word(dev->config + PCI_MEMORY_BASE, s->mmio_base >> 16); - pci_set_word(dev->config + PCI_MEMORY_LIMIT, + pci_set_word(pci_dev->config + PCI_MEMORY_BASE, s->mmio_base >> 16); + pci_set_word(pci_dev->config + PCI_MEMORY_LIMIT, ((s->mmio_base + s->mmio_size - 1) >> 16) & 0xfff0); - pci_bridge_initfn(dev, TYPE_PCI_BUS); + pci_bridge_initfn(pci_dev, TYPE_PCI_BUS); - if (pcie_endpoint_cap_v1_init(dev, 0x80) < 0) { - hw_error("Failed to initialize PCIe capability"); + if (pcie_endpoint_cap_v1_init(pci_dev, 0x80) < 0) { + error_setg(errp, "Failed to initialize PCIe capability"); } - - return 0; } static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data) @@ -300,7 +299,7 @@ static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data) k->class_id = PCI_CLASS_BRIDGE_HOST; k->is_express = true; k->is_bridge = true; - k->init = xilinx_pcie_root_init; + k->realize = xilinx_pcie_root_realize; k->exit = pci_bridge_exitfn; dc->reset = pci_bridge_reset; k->config_read = xilinx_pcie_root_config_read; From 7722b1a78aa0c87f645516b6693baa9767891837 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 14 Dec 2017 20:22:19 +0000 Subject: [PATCH 17/23] vhost-user: fix indentation in protocol specification Signed-off-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- docs/interop/vhost-user.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index 954771d0d8..fd8ac56562 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -53,8 +53,8 @@ Depending on the request type, payload can be: * A vring state description --------------- - | index | num | - --------------- + | index | num | + --------------- Index: a 32-bit index Num: a 32-bit number From c3d331d28fc31997404456b0b41960fdda3d8619 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 14 Dec 2017 20:22:20 +0000 Subject: [PATCH 18/23] vhost-user: document memory accesses The vhost-user protocol specification does not define "guest address" and "user address". It does not explain how to access memory given such addresses. This patch explains how memory access works, including the IOTLB. Cc: Michael S. Tsirkin Cc: Maxime Coquelin Cc: Wei Wang Signed-off-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Acked-by: Maxime Coquelin --- docs/interop/vhost-user.txt | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index fd8ac56562..d49444e037 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -66,11 +66,14 @@ Depending on the request type, payload can be: Index: a 32-bit vring index Flags: a 32-bit vring flags - Descriptor: a 64-bit user address of the vring descriptor table - Used: a 64-bit user address of the vring used ring - Available: a 64-bit user address of the vring available ring + Descriptor: a 64-bit ring address of the vring descriptor table + Used: a 64-bit ring address of the vring used ring + Available: a 64-bit ring address of the vring available ring Log: a 64-bit guest address for logging + Note that a ring address is an IOVA if VIRTIO_F_IOMMU_PLATFORM has been + negotiated. Otherwise it is a user address. + * Memory regions description --------------------------------------------------- | num regions | padding | region0 | ... | region7 | @@ -273,6 +276,30 @@ Once the source has finished migration, rings will be stopped by the source. No further update must be done before rings are restarted. +Memory access +------------- + +The master sends a list of vhost memory regions to the slave using the +VHOST_USER_SET_MEM_TABLE message. Each region has two base addresses: a guest +address and a user address. + +Messages contain guest addresses and/or user addresses to reference locations +within the shared memory. The mapping of these addresses works as follows. + +User addresses map to the vhost memory region containing that user address. + +When the VIRTIO_F_IOMMU_PLATFORM feature has not been negotiated: + + * Guest addresses map to the vhost memory region containing that guest + address. + +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated: + + * Guest addresses are also called I/O virtual addresses (IOVAs). They are + translated to user addresses via the IOTLB. + + * The vhost memory region guest address is not used. + IOMMU support ------------- From bf33cc75ad5bf117cb080572a04a79182520c434 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Fri, 8 Dec 2017 12:26:53 +0800 Subject: [PATCH 19/23] intel_iommu: remove X86_IOMMU_PCI_DEVFN_MAX We have PCI_DEVFN_MAX now. Signed-off-by: Peter Xu Reviewed-by: Liu, Yi L Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 10 +++++----- include/hw/i386/x86-iommu.h | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 3a5bb0bc2e..c1fa08d205 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -186,7 +186,7 @@ static void vtd_reset_context_cache(IntelIOMMUState *s) g_hash_table_iter_init(&bus_it, s->vtd_as_by_busptr); while (g_hash_table_iter_next (&bus_it, NULL, (void**)&vtd_bus)) { - for (devfn_it = 0; devfn_it < X86_IOMMU_PCI_DEVFN_MAX; ++devfn_it) { + for (devfn_it = 0; devfn_it < PCI_DEVFN_MAX; ++devfn_it) { vtd_as = vtd_bus->dev_as[devfn_it]; if (!vtd_as) { continue; @@ -1002,7 +1002,7 @@ static void vtd_switch_address_space_all(IntelIOMMUState *s) g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { - for (i = 0; i < X86_IOMMU_PCI_DEVFN_MAX; i++) { + for (i = 0; i < PCI_DEVFN_MAX; i++) { if (!vtd_bus->dev_as[i]) { continue; } @@ -1294,7 +1294,7 @@ static void vtd_context_device_invalidate(IntelIOMMUState *s, vtd_bus = vtd_find_as_from_bus_num(s, bus_n); if (vtd_bus) { devfn = VTD_SID_TO_DEVFN(source_id); - for (devfn_it = 0; devfn_it < X86_IOMMU_PCI_DEVFN_MAX; ++devfn_it) { + for (devfn_it = 0; devfn_it < PCI_DEVFN_MAX; ++devfn_it) { vtd_as = vtd_bus->dev_as[devfn_it]; if (vtd_as && ((devfn_it & mask) == (devfn & mask))) { trace_vtd_inv_desc_cc_device(bus_n, VTD_PCI_SLOT(devfn_it), @@ -2699,7 +2699,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) *new_key = (uintptr_t)bus; /* No corresponding free() */ vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \ - X86_IOMMU_PCI_DEVFN_MAX); + PCI_DEVFN_MAX); vtd_bus->bus = bus; g_hash_table_insert(s->vtd_as_by_busptr, new_key, vtd_bus); } @@ -2982,7 +2982,7 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn) IntelIOMMUState *s = opaque; VTDAddressSpace *vtd_as; - assert(0 <= devfn && devfn < X86_IOMMU_PCI_DEVFN_MAX); + assert(0 <= devfn && devfn < PCI_DEVFN_MAX); vtd_as = vtd_find_add_as(s, bus, devfn); return &vtd_as->as; diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h index ef89c0c646..7c71fc7470 100644 --- a/include/hw/i386/x86-iommu.h +++ b/include/hw/i386/x86-iommu.h @@ -31,7 +31,6 @@ #define X86_IOMMU_GET_CLASS(obj) \ OBJECT_GET_CLASS(X86IOMMUClass, obj, TYPE_X86_IOMMU_DEVICE) -#define X86_IOMMU_PCI_DEVFN_MAX 256 #define X86_IOMMU_SID_INVALID (0xffff) typedef struct X86IOMMUState X86IOMMUState; From 4c427a4cf3a5ff08d59a88b4dc6a8a1060e1bf9c Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Fri, 8 Dec 2017 12:26:54 +0800 Subject: [PATCH 20/23] intel_iommu: fix error param in string It should be caching-mode. It may confuse people when it pops up. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Liu, Yi L Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index c1fa08d205..fe15d3ba84 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2327,7 +2327,7 @@ static void vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu, IntelIOMMUNotifierNode *next_node = NULL; if (!s->caching_mode && new & IOMMU_NOTIFIER_MAP) { - error_report("We need to set cache_mode=1 for intel-iommu to enable " + error_report("We need to set caching-mode=1 for intel-iommu to enable " "device assignment with IOMMU protection."); exit(1); } From f2bc54de47404b70b9ac87e2c75489f2652643e7 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 13 Nov 2017 09:45:58 +0100 Subject: [PATCH 21/23] virtio-pci: Don't force Subsystem Vendor ID = Vendor ID The statement being removed doesn't change anything as virtio PCI devices already have Subsystem Vendor ID set to pci_default_sub_vendor_id (0x1af4), same as Vendor ID. And the Virtio spec does not require the two to be equal, either: "The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect the PCI Vendor and Device ID of the environment (for informational purposes by the driver)." Background: Following the recent virtio-win licensing change, several vendors are planning to ship their own certified version of Windows guest Virtio drivers, potentially taking advantage of Windows Update as a distribution channel. It is therefore critical that each vendor uses their own PCI Subsystem Vendor ID for Virtio devices to prevent drivers from other vendors binding to it. This would be trivially done by adding: k->subsystem_vendor_id = ... to virtio_pci_class_init(). Except for the problematic statement deleted by this patch, which reverts the Subsystem Vendor ID back to 0x1af4 for legacy devices for no good reason. Signed-off-by: Ladi Prosek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Gerd Hoffmann --- hw/virtio/virtio-pci.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 42b31fbcf8..6c75cca88a 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1588,9 +1588,11 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) "neither legacy nor transitional device."); return ; } - /* legacy and transitional */ - pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID, - pci_get_word(config + PCI_VENDOR_ID)); + /* + * Legacy and transitional devices use specific subsystem IDs. + * Note that the subsystem vendor ID (config + PCI_SUBSYSTEM_VENDOR_ID) + * is set to PCI_SUBVENDOR_ID_REDHAT_QUMRANET by default. + */ pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus)); } else { /* pure virtio-1.0 */ From bcfdacfe2f56647d07a0093c415d43d33ee954b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 12 Dec 2017 18:22:08 +0100 Subject: [PATCH 22/23] dump-guest-memory.py: fix "You can't do that without a process to debug" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the script is run with a core (no running process), it produces an error: (gdb) dump-guest-memory /tmp/vmcore X86_64 guest RAM blocks: target_start target_end host_addr message count ---------------- ---------------- ---------------- ------- ----- 0000000000000000 00000000000a0000 00007f7935800000 added 1 00000000000a0000 00000000000b0000 00007f7934200000 added 2 00000000000c0000 00000000000ca000 00007f79358c0000 added 3 00000000000ca000 00000000000cd000 00007f79358ca000 joined 3 00000000000cd000 00000000000e8000 00007f79358cd000 joined 3 00000000000e8000 00000000000f0000 00007f79358e8000 joined 3 00000000000f0000 0000000000100000 00007f79358f0000 joined 3 0000000000100000 0000000080000000 00007f7935900000 joined 3 00000000fd000000 00000000fe000000 00007f7934200000 added 4 00000000fffc0000 0000000100000000 00007f7935600000 added 5 Python Exception You can't do that without a process to debug.: Error occurred in Python command: You can't do that without a process to debug. Replace the object_resolve_path_type() function call call with a local volatile variable. Signed-off-by: Marc-André Lureau Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/misc/vmcoreinfo.c | 3 +++ scripts/dump-guest-memory.py | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/misc/vmcoreinfo.c b/hw/misc/vmcoreinfo.c index 31db57ab44..a2805527cb 100644 --- a/hw/misc/vmcoreinfo.c +++ b/hw/misc/vmcoreinfo.c @@ -35,6 +35,8 @@ static void vmcoreinfo_realize(DeviceState *dev, Error **errp) { VMCoreInfoState *s = VMCOREINFO(dev); FWCfgState *fw_cfg = fw_cfg_find(); + /* for gdb script dump-guest-memory.py */ + static VMCoreInfoState * volatile vmcoreinfo_state G_GNUC_UNUSED; /* Given that this function is executing, there is at least one VMCOREINFO * device. Check if there are several. @@ -56,6 +58,7 @@ static void vmcoreinfo_realize(DeviceState *dev, Error **errp) &s->vmcoreinfo, sizeof(s->vmcoreinfo), false); qemu_register_reset(vmcoreinfo_reset, dev); + vmcoreinfo_state = s; } static const VMStateDescription vmstate_vmcoreinfo = { diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py index 1af26c1a45..09bec92b50 100644 --- a/scripts/dump-guest-memory.py +++ b/scripts/dump-guest-memory.py @@ -546,8 +546,7 @@ shape and this command should mostly work.""" return None def add_vmcoreinfo(self): - vmci = '(VMCoreInfoState *)' + \ - 'object_resolve_path_type("", "vmcoreinfo", 0)' + vmci = 'vmcoreinfo_realize::vmcoreinfo_state' if not gdb.parse_and_eval("%s" % vmci) \ or not gdb.parse_and_eval("(%s)->has_vmcoreinfo" % vmci): return From 880b1ffe6ec2f0ae25cc4175716227ad275e8b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Sun, 10 Dec 2017 18:27:03 +0100 Subject: [PATCH 23/23] smbus: do not immediately complete commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PIIX4 errata says that "immediate polling of the Host Status Register BUSY bit may indicate that the SMBus is NOT busy." Due to this, some code does the following steps: (a) set parameters (b) start command (c) check for smbus busy bit set (to know that command started) (d) check for smbus busy bit not set (to know that command finished) Let (c) happen, by immediately setting the busy bit, and really executing the command when status register has been read once. This fixes a problem with AMIBIOS, which can now properly initialize the PIIX4. Signed-off-by: Hervé Poussineau Signed-off-by: Philippe Mathieu-Daudé --- hw/i2c/pm_smbus.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c index 6fc3923f56..ec060d58cc 100644 --- a/hw/i2c/pm_smbus.c +++ b/hw/i2c/pm_smbus.c @@ -63,6 +63,9 @@ static void smb_transaction(PMSMBus *s) I2CBus *bus = s->smbus; int ret; + assert(s->smb_stat & STS_HOST_BUSY); + s->smb_stat &= ~STS_HOST_BUSY; + SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot); /* Transaction isn't exec if STS_DEV_ERR bit set */ if ((s->smb_stat & STS_DEV_ERR) != 0) { @@ -135,6 +138,13 @@ error: } +static void smb_transaction_start(PMSMBus *s) +{ + /* Do not execute immediately the command ; it will be + * executed when guest will read SMB_STAT register */ + s->smb_stat |= STS_HOST_BUSY; +} + static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, unsigned width) { @@ -150,7 +160,7 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, case SMBHSTCNT: s->smb_ctl = val; if (val & 0x40) - smb_transaction(s); + smb_transaction_start(s); break; case SMBHSTCMD: s->smb_cmd = val; @@ -182,6 +192,10 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width) switch(addr) { case SMBHSTSTS: val = s->smb_stat; + if (s->smb_stat & STS_HOST_BUSY) { + /* execute command now */ + smb_transaction(s); + } break; case SMBHSTCNT: s->smb_index = 0;