When support for the pcie-root and dmi-to-pci-bridge buses on a Q35 machinetype was added, I was concerned that even though qemu at the time allowed plugging a PCI device into a PCIe port, that it might not be supported in the future. To prevent painful backtracking in the possible future where this happened, I disallowed such connections except in a few specific cases requested by qemu developers (indicated in the code with the flag VIR_PCI_CONNEcT_TYPE_EITHER_IF_CONFIG). Now that a couple years have passed, there is a clear message from qemu that there is no danger in allowing PCI devices to be plugged into PCIe ports. This patch eliminates VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG and changes the code to always allow PCI->PCIe or PCIe->PCI connection *when the PCI address is specified in the config. (For newly added devices that haven't yet been given a PCI address, the auto-placement still prefers using the correct type of bus). --- docs/formatdomain.html.in | 18 +++++++++++------- src/conf/domain_addr.c | 18 +++++++++++------- src/conf/domain_addr.h | 16 ++++++++++------ src/qemu/qemu_command.c | 6 ++---- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4e85b51..1dca2ac 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3034,17 +3034,21 @@ bus (for example, the machine types based on the Q35 chipset), the pcie-root controller with index=0 is auto-added to the domain's configuration. pcie-root has also no address, provides - 31 slots (numbered 1-31) and can only be used to attach PCIe - devices. In order to connect standard PCI devices on a system - which has a pcie-root controller, a pci controller + 31 slots (numbered 1-31) that can be used to attach PCIe or PCI + devices (although libvirt will never auto-assign a PCI device to + a PCIe slot, it will allow manual specification of such an + assignment). Devices connected to pcie-root cannot be + hotplugged. In order to make standard PCI slots available on a + system which has a pcie-root controller, a pci controller with <code>model='dmi-to-pci-bridge'</code> is automatically - added. A dmi-to-pci-bridge controller plugs into a PCIe slot (as - provided by pcie-root), and itself provides 31 standard PCI - slots (which are not hot-pluggable). In order to have + added, usually at the defacto standard location of slot=0x1e. A + dmi-to-pci-bridge controller plugs into a PCIe slot (as provided + by pcie-root), and itself provides 31 standard PCI slots (which + also do not support device hotplug). In order to have hot-pluggable PCI slots in the guest system, a pci-bridge controller will also be automatically created and connected to one of the slots of the auto-created dmi-to-pci-bridge - controller; all guest devices with PCI addresses that are + controller; all guest PCI devices with addresses that are auto-determined by libvirt will be placed on this pci-bridge device. (<span class="since">since 1.1.2</span>). </p> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index d657c7f..93c6043 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1,7 +1,7 @@ /* * domain_addr.c: helper APIs for managing domain device addresses * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -42,16 +42,21 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, { virErrorNumber errType = (fromConfig ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); - virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK; - if (fromConfig) - flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG; + if (fromConfig) { + /* If the requested connection was manually specified in + * config, allow a PCI device to connect to a PCIe slot, or + * vice versa. + */ + if (busFlags & VIR_PCI_CONNECT_TYPES_ENDPOINT) + busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT; + } /* If this bus doesn't allow the type of connection (PCI * vs. PCIe) required by the device, or if the device requires * hot-plug and this bus doesn't have it, return false. */ - if (!(devFlags & busFlags & flagsMatchMask)) { + if (!(devFlags & busFlags & VIR_PCI_CONNECT_TYPES_MASK)) { if (reportError) { if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) { virReportError(errType, @@ -174,8 +179,7 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, * specified in user config *and* the particular device being * attached also allows it */ - bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE | - VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + bus->flags = VIR_PCI_CONNECT_TYPE_PCIE; bus->minSlot = 1; bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index c18e130..dcfd2e5 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -1,7 +1,7 @@ /* * domain_addr.h: helper APIs for managing domain device addresses * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -39,10 +39,6 @@ typedef enum { /* PCI devices can connect to this bus */ VIR_PCI_CONNECT_TYPE_PCIE = 1 << 3, /* PCI Express devices can connect to this bus */ - VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG = 1 << 4, - /* PCI *and* PCIe devices allowed, if the address - * was specified in the config by the user - */ } virDomainPCIConnectFlags; typedef struct { @@ -70,12 +66,20 @@ struct _virDomainPCIAddressSet { typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet; typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; -/* a combination of all bit that describe the type of connections +/* a combination of all bits that describe the type of connections * allowed, e.g. PCI, PCIe, switch */ # define VIR_PCI_CONNECT_TYPES_MASK \ (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE) +/* combination of all bits that could be used to connect a normal + * endpoint device (i.e. excluding the connection possible between an + * upstream and downstream switch port, or a PCIe root port and a PCIe + * port) + */ +# define VIR_PCI_CONNECT_TYPES_ENDPOINT \ + (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE) + char *virDomainPCIAddressAsString(virDevicePCIAddressPtr addr) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5e5cfe6..4952797 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1621,8 +1621,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: - flags = (VIR_PCI_CONNECT_TYPE_PCI | - VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + flags = VIR_PCI_CONNECT_TYPE_PCI; break; case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: /* should this be PCIE-only? Or do we need to allow PCI @@ -1643,8 +1642,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, switch (device->data.sound->model) { case VIR_DOMAIN_SOUND_MODEL_ICH6: case VIR_DOMAIN_SOUND_MODEL_ICH9: - flags = (VIR_PCI_CONNECT_TYPE_PCI | - VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + flags = VIR_PCI_CONNECT_TYPE_PCI; break; } break; -- 2.1.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list