[libvirt] [PATCH v5 00/15] Use more PCIe less legacy PCI
These are just the rebased versions of the patches from v4 that I haven't yet pushed, to make review easier. Patch 1 and 2 (and several others) are already ACKed, but included for completeness. Laine Stump (15): qemu: new functions qemuDomainMachineHasPCI[e]Root() qemu: new functions to calculate/set device pciConnectFlags qemu: set/use info->pciConnectFlags when validating/assigning PCI addresses qemu: set/use proper pciConnectFlags during hotplug qemu: set pciConnectFlags to 0 instead of PCI|HOTPLUGGABLE if device isn't PCI qemu: assign virtio devices to PCIe slot when appropriate qemu: assign e1000e network devices to PCIe slots when appropriate qemu: assign nec-xhci (USB3) controller to a PCIe address when appropriate qemu: only force an available legacy-PCI slot on domains with pci-root qemu: auto-add pcie-root-port/dmi-to-pci-bridge controllers as needed [RFC] qemu: if pci-bridge is in PCIe config w/o dmi-to-pci-bridge, add it qemu: don't force-add a dmi-to-pci-bridge just on principle qemu: add a USB3 controller to Q35 domains by default qemu: try to put ich9 sound device at 00:1B.0 qemu: initially reserve one open pcie-root-port for hotplug src/conf/device_conf.h | 5 + src/conf/domain_addr.c | 112 ++- src/conf/domain_addr.h | 3 +- src/qemu/qemu_domain.c | 52 +- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_domain_address.c | 805 +++++++++++++++++---- src/qemu/qemu_domain_address.h | 4 + src/qemu/qemu_hotplug.c | 20 +- tests/qemuxml2argvdata/qemuxml2argv-autoindex.args | 10 +- .../qemuxml2argv-bios-nvram-secure.args | 1 + .../qemuxml2argv-machine-smm-opt.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 3 +- .../qemuxml2argv-q35-default-devices-only.args | 23 + .../qemuxml2argv-q35-default-devices-only.xml | 18 + .../qemuxml2argv-q35-pcie-autoadd.args | 57 ++ .../qemuxml2argv-q35-pcie-autoadd.xml | 51 ++ tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args | 58 ++ tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml | 67 ++ .../qemuxml2argv-q35-pm-disable-fallback.args | 1 + .../qemuxml2argv-q35-pm-disable.args | 1 + .../qemuxml2argv-q35-virt-manager-basic.args | 56 ++ .../qemuxml2argv-q35-virt-manager-basic.xml | 76 ++ .../qemuxml2argv-q35-virtio-pci.args | 58 ++ .../qemuxml2argv-q35-virtio-pci.xml | 1 + tests/qemuxml2argvtest.c | 164 ++++- .../qemuxml2xmlout-autoindex.xml | 10 +- .../qemuxml2xmlout-pcie-root.xml | 4 - .../qemuxml2xmlout-q35-default-devices-only.xml | 45 ++ .../qemuxml2xmlout-q35-pcie-autoadd.xml | 148 ++++ .../qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml | 152 ++++ .../qemuxml2xmlout-q35-virt-manager-basic.xml | 121 ++++ .../qemuxml2xmlout-q35-virtio-pci.xml | 152 ++++ tests/qemuxml2xmltest.c | 136 +++- 33 files changed, 2189 insertions(+), 228 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml -- 2.7.4
These functions provide a simple one line method of learning if the current domain has a pci-root or pcie-root bus. --- src/qemu/qemu_domain.c | 30 ++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 41ac52d..310bfa6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5393,6 +5393,36 @@ qemuDomainMachineIsI440FX(const virDomainDef *def) bool +qemuDomainMachineHasPCIRoot(const virDomainDef *def) +{ + int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0); + + if (root < 0) + return false; + + if (def->controllers[root]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) + return false; + + return true; +} + + +bool +qemuDomainMachineHasPCIeRoot(const virDomainDef *def) +{ + int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0); + + if (root < 0) + return false; + + if (def->controllers[root]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) + return false; + + return true; +} + + +bool qemuDomainMachineNeedsFDC(const virDomainDef *def) { char *p = STRSKIP(def->os.machine, "pc-q35-"); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 4f9bf82..f896756 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -654,6 +654,8 @@ virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def); bool qemuDomainMachineIsQ35(const virDomainDef *def); bool qemuDomainMachineIsI440FX(const virDomainDef *def); +bool qemuDomainMachineHasPCIRoot(const virDomainDef *def); +bool qemuDomainMachineHasPCIeRoot(const virDomainDef *def); bool qemuDomainMachineNeedsFDC(const virDomainDef *def); bool qemuDomainMachineIsS390CCW(const virDomainDef *def); bool qemuDomainMachineIsVirt(const virDomainDef *def); -- 2.7.4
The lowest level function of this trio (qemuDomainDeviceCalculatePCIConnectFlags()) aims to be the single authority for the virDomainPCIConnectFlags to use for any given device using a particular arch/machinetype/qemu-binary. qemuDomainFillDevicePCIConnectFlags() sets info->pciConnectFlags in a single device (unless it has no virDomainDeviceInfo, in which case it's a NOP). qemuDomainFillAllPCIConnectFlags() sets info->pciConnectFlags in all devices that have a virDomainDeviceInfo The latter two functions aren't called anywhere yet. This commit is just making them available. Later patches will replace all the current hodge-podge of flag settings with calls to this single authority. --- src/conf/device_conf.h | 5 + src/qemu/qemu_domain_address.c | 367 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 372 insertions(+) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 8443de6..f435fb5 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -142,6 +142,11 @@ typedef struct _virDomainDeviceInfo { /* bootIndex is only used for disk, network interface, hostdev * and redirdev devices */ unsigned int bootIndex; + + /* pciConnectFlags is only used internally during address + * assignment, never saved and never reported. + */ + int pciConnectFlags; /* enum virDomainPCIConnectFlags */ } virDomainDeviceInfo, *virDomainDeviceInfoPtr; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e206b9f..602179c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -407,6 +407,373 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, } +/** + * qemuDomainDeviceCalculatePCIConnectFlags: + * + * @dev: The device to be checked + * @pcieFlags: flags to use for a known PCI Express device + * @virtioFlags: flags to use for a virtio device (properly vetted + * for the current qemu binary and arch/machinetype) + * + * Lowest level function to determine PCI connectFlags for a + * device. This function relies on the next higher-level function + * determining the value for pcieFlags and virtioFlags in advance - + * this is to make it more efficient to call multiple times. + * + * Returns appropriate virDomainPCIConnectFlags for this device in + * this domain, or 0 if the device doesn't connect using PCI. There + * is no failure. + */ +static virDomainPCIConnectFlags +qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, + virDomainPCIConnectFlags pcieFlags + ATTRIBUTE_UNUSED, + virDomainPCIConnectFlags virtioFlags + ATTRIBUTE_UNUSED) +{ + virDomainPCIConnectFlags pciFlags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | + VIR_PCI_CONNECT_HOTPLUGGABLE); + + switch ((virDomainDeviceType)dev->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: { + virDomainControllerDefPtr cont = dev->data.controller; + + switch ((virDomainControllerType)cont->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + return virDomainPCIControllerModelToConnectType(cont->model); + + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: + return pciFlags; + + case VIR_DOMAIN_CONTROLLER_TYPE_USB: + switch ((virDomainControllerModelUSB)cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: + return pciFlags; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: + return pciFlags; + + case VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB1: /* xen only */ + case VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2: /* xen only */ + case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST: + /* should be 0 */ + return pciFlags; + } + + case VIR_DOMAIN_CONTROLLER_TYPE_IDE: + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + return pciFlags; + + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: + /* should be 0 */ + return pciFlags; + } + } + + case VIR_DOMAIN_DEVICE_FS: + /* the only type of filesystem so far is virtio-9p-pci */ + return pciFlags; + + case VIR_DOMAIN_DEVICE_NET: { + virDomainNetDefPtr net = dev->data.net; + + /* NB: a type='hostdev' will use PCI, but its + * address is assigned when we're assigning the + * addresses for other hostdev devices. + */ + if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || + STREQ(net->model, "usb-net")) { + /* should be 0 */ + return pciFlags; + } + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_SOUND: + switch ((virDomainSoundModel)dev->data.sound->model) { + case VIR_DOMAIN_SOUND_MODEL_ES1370: + case VIR_DOMAIN_SOUND_MODEL_AC97: + case VIR_DOMAIN_SOUND_MODEL_ICH6: + case VIR_DOMAIN_SOUND_MODEL_ICH9: + return pciFlags; + + case VIR_DOMAIN_SOUND_MODEL_SB16: + case VIR_DOMAIN_SOUND_MODEL_PCSPK: + case VIR_DOMAIN_SOUND_MODEL_USB: + case VIR_DOMAIN_SOUND_MODEL_LAST: + /* should be 0 */ + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_DISK: + switch ((virDomainDiskBus)dev->data.disk->bus) { + case VIR_DOMAIN_DISK_BUS_VIRTIO: + return pciFlags; /* only virtio disks use PCI */ + + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: + /* should be 0 */ + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_HOSTDEV: + return pciFlags; + + case VIR_DOMAIN_DEVICE_MEMBALLOON: + switch ((virDomainMemballoonModel)dev->data.memballoon->model) { + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: + return pciFlags; + + case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: + case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: + case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: + /* should be 0 (not PCI) */ + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_RNG: + switch ((virDomainRNGModel)dev->data.rng->model) { + case VIR_DOMAIN_RNG_MODEL_VIRTIO: + return pciFlags; + + case VIR_DOMAIN_RNG_MODEL_LAST: + /* should be 0 */ + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_WATCHDOG: + /* only one model connects using PCI */ + switch ((virDomainWatchdogModel)dev->data.watchdog->model) { + case VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB: + return pciFlags; + + case VIR_DOMAIN_WATCHDOG_MODEL_IB700: + case VIR_DOMAIN_WATCHDOG_MODEL_DIAG288: + case VIR_DOMAIN_WATCHDOG_MODEL_LAST: + /* should be 0 */ + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_VIDEO: + switch ((virDomainVideoType)dev->data.video->type) { + case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + case VIR_DOMAIN_VIDEO_TYPE_XEN: + case VIR_DOMAIN_VIDEO_TYPE_VBOX: + case VIR_DOMAIN_VIDEO_TYPE_QXL: + case VIR_DOMAIN_VIDEO_TYPE_PARALLELS: + return pciFlags; + + case VIR_DOMAIN_VIDEO_TYPE_LAST: + /* should be 0 */ + return pciFlags; + } + + + case VIR_DOMAIN_DEVICE_SHMEM: + return pciFlags; + + case VIR_DOMAIN_DEVICE_INPUT: + switch ((virDomainInputBus)dev->data.input->bus) { + case VIR_DOMAIN_INPUT_BUS_VIRTIO: + return pciFlags; + + case VIR_DOMAIN_INPUT_BUS_PS2: + case VIR_DOMAIN_INPUT_BUS_USB: + case VIR_DOMAIN_INPUT_BUS_XEN: + case VIR_DOMAIN_INPUT_BUS_PARALLELS: + case VIR_DOMAIN_INPUT_BUS_LAST: + /* should be 0 */ + return pciFlags; + } + + case VIR_DOMAIN_DEVICE_CHR: + switch ((virDomainChrSerialTargetType)dev->data.chr->targetType) { + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: + return pciFlags; + + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: + /* should be 0 */ + return pciFlags; + } + + /* These devices don't ever connect with PCI */ + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + /* These devices don't even have a DeviceInfo */ + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_LAST: + case VIR_DOMAIN_DEVICE_NONE: + /* should be 0 */ + return pciFlags; + } + + /* We can never get here, because all cases are covered in the + * switch, and they all return, but the compiler will still + * complain "control reaches end of non-void function" unless + * we add the following return. + */ + return 0; +} + + +typedef struct { + virDomainPCIConnectFlags virtioFlags; + virDomainPCIConnectFlags pcieFlags; +} qemuDomainFillDevicePCIConnectFlagsIterData; + +/** + * qemuDomainFillDevicePCIConnectIterInit: + * + * Initialize the iterator data that is used when calling + * qemuDomainCalculateDevicePCIConnectFlags(). + */ +static void +qemuDomainFillDevicePCIConnectIterInit(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + qemuDomainFillDevicePCIConnectFlagsIterData *data) +{ + if (qemuDomainMachineHasPCIeRoot(def)) { + data->pcieFlags = (VIR_PCI_CONNECT_TYPE_PCIE_DEVICE | + VIR_PCI_CONNECT_HOTPLUGGABLE); + } else { + data->pcieFlags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | + VIR_PCI_CONNECT_HOTPLUGGABLE); + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY)) { + data->virtioFlags = data->pcieFlags; + } else { + data->virtioFlags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | + VIR_PCI_CONNECT_HOTPLUGGABLE); + } +} + + +/** + * qemuDomainFillDevicePCIConnectFlagsIter: + * + * @def: the entire DomainDef + * @dev: The device to be checked + * @info: virDomainDeviceInfo within the device + * @opaque: points to iterator data setup beforehand. + * + * A function with properly formatted arguments to be called by + * virDomainDeviceInfoIterate(). Sets the pciConnectFlags for a single + * device's info. + * + * Always returns 0 - there is no failure. + */ +static int +qemuDomainFillDevicePCIConnectFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info, + void *opaque) +{ + qemuDomainFillDevicePCIConnectFlagsIterData *data = opaque; + + info->pciConnectFlags + = qemuDomainDeviceCalculatePCIConnectFlags(dev, data->pcieFlags, + data->virtioFlags); + return 0; +} + + +/** + * qemuDomainFillAllPCIConnectFlags: + * + * @def: the entire DomainDef + * @qemuCaps: as you'd expect + * + * Set the info->pciConnectFlags for all devices in the domain. + * + * Returns 0 on success or -1 on failure (the only possibility of + * failure would be some internal problem with + * virDomainDeviceInfoIterate()) + */ +static int ATTRIBUTE_UNUSED +qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + qemuDomainFillDevicePCIConnectFlagsIterData data; + + qemuDomainFillDevicePCIConnectIterInit(def, qemuCaps, &data); + + return virDomainDeviceInfoIterate(def, + qemuDomainFillDevicePCIConnectFlagsIter, + &data); +} + + +/** + * qemuDomainFillDevicePCIConnectFlags: + * + * @def: the entire DomainDef + * @dev: The device to be checked + * @qemuCaps: as you'd expect + * + * Set the info->pciConnectFlags for a single device. + * + * No return value. + */ +static void ATTRIBUTE_UNUSED +qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def, + virDomainDeviceDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + + if (info) { + /* qemuDomainDeviceCalculatePCIConnectFlags() is called with + * the data setup in the ...IterData by ...IterInit() rather + * than setting the values directly here. It may seem like + * pointless posturing, but it's done this way to eliminate + * duplicated setup code while allowing more efficient + * operation when it's being done repeatedly with the device + * iterator (since qemuDomainFillAllPCIConnectFlags() only + * calls ...IterInit() once for all devices). + */ + qemuDomainFillDevicePCIConnectFlagsIterData data; + + qemuDomainFillDevicePCIConnectIterInit(def, qemuCaps, &data); + + info->pciConnectFlags + = qemuDomainDeviceCalculatePCIConnectFlags(dev, data.pcieFlags, + data.virtioFlags); + } +} + + static int qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, -- 2.7.4
Set pciConnectFlags in each device's DeviceInfo and then use those flags later when validating existing addresses in qemuDomainCollectPCIAddress() and when assigning new addresses with qemuDomainPCIAddressReserveNextAddr() (rather than scattering the logic about which devices need which type of slot all over the place). Note that the exact flags set by qemuDomainDeviceCalculatePCIConnectFlags() are different from the flags previously set manually in qemuDomainCollectPCIAddress(), but this doesn't matter because all validation of addresses in that case ignores the setting of the HOTPLUGGABLE flag, and treats PCIE_DEVICE and PCI_DEVICE the same (this lax checking was done on purpose, because there are some things that we want to allow the user to specify manually, e.g. assigning a PCIe device to a PCI slot, that we *don't* ever want libvirt to do automatically. The flag settings that we *really* want to match are 1) the old flag settings in qemuDomainAssignDevicePCISlots() (which is HOTPLUGGABLE | PCI_DEVICE for everything except PCI controllers) and the new flag settings done by qemuDomainDeviceCalculatePCIConnectFlags() (which are currently exactly that - HOTPLUGGABLE | PCI_DEVICE for everything except PCI controllers). --- src/qemu/qemu_domain_address.c | 205 +++++++++++++++-------------------------- 1 file changed, 74 insertions(+), 131 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 602179c..d731b19 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -721,7 +721,7 @@ qemuDomainFillDevicePCIConnectFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED, * failure would be some internal problem with * virDomainDeviceInfoIterate()) */ -static int ATTRIBUTE_UNUSED +static int qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { @@ -777,21 +777,20 @@ qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def, static int qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, - virDomainPCIConnectFlags flags, unsigned int function, bool reserveEntireSlot) { - return virDomainPCIAddressReserveNextAddr(addrs, dev, flags, + return virDomainPCIAddressReserveNextAddr(addrs, dev, + dev->pciConnectFlags, function, reserveEntireSlot); } static int qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev, - virDomainPCIConnectFlags flags) + virDomainDeviceInfoPtr dev) { - return qemuDomainPCIAddressReserveNextAddr(addrs, dev, flags, 0, true); + return qemuDomainPCIAddressReserveNextAddr(addrs, dev, 0, true); } @@ -805,9 +804,6 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, int ret = -1; virPCIDeviceAddressPtr addr = &info->addr.pci; bool entireSlot; - /* flags may be changed from default below */ - virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI_DEVICE); if (!virDeviceInfoPCIAddressPresent(info) || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && @@ -819,71 +815,6 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } - /* Change flags according to differing requirements of different - * devices. - */ - switch (device->type) { - case VIR_DOMAIN_DEVICE_CONTROLLER: - switch (device->data.controller->type) { - case VIR_DOMAIN_CONTROLLER_TYPE_PCI: - flags = virDomainPCIControllerModelToConnectType(device->data.controller->model); - break; - - case VIR_DOMAIN_CONTROLLER_TYPE_SATA: - /* SATA controllers aren't hot-plugged, and can be put in - * either a PCI or PCIe slot - */ - flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE - | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); - break; - - case VIR_DOMAIN_CONTROLLER_TYPE_USB: - /* allow UHCI and EHCI controllers to be manually placed on - * the PCIe bus (but don't put them there automatically) - */ - switch (device->data.controller->model) { - case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: - 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_DEVICE; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: - /* should this be PCIE-only? Or do we need to allow PCI - * for backward compatibility? - */ - flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE - | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); - break; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: - /* Allow these for PCI only */ - break; - } - } - break; - - case VIR_DOMAIN_DEVICE_SOUND: - switch (device->data.sound->model) { - case VIR_DOMAIN_SOUND_MODEL_ICH6: - case VIR_DOMAIN_SOUND_MODEL_ICH9: - flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; - break; - } - break; - - case VIR_DOMAIN_DEVICE_VIDEO: - /* video cards aren't hot-plugged, and can be put in either a - * PCI or PCIe slot - */ - flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE - | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); - break; - } - /* Ignore implicit controllers on slot 0:0:1.0: * implicit IDE controller on 0:0:1.1 (no qemu command line) * implicit USB controller on 0:0:1.2 (-usb) @@ -926,7 +857,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, entireSlot = (addr->function == 0 && addr->multi != VIR_TRISTATE_SWITCH_ON); - if (virDomainPCIAddressReserveAddr(addrs, addr, flags, + if (virDomainPCIAddressReserveAddr(addrs, addr, info->pciConnectFlags, entireSlot, true) < 0) goto cleanup; @@ -1082,8 +1013,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { if (qemuDomainPCIAddressReserveNextSlot(addrs, - &primaryVideo->info, - flags) < 0) { + &primaryVideo->info) < 0) { goto cleanup; } } else { @@ -1272,8 +1202,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { if (qemuDomainPCIAddressReserveNextSlot(addrs, - &primaryVideo->info, - flags) < 0) + &primaryVideo->info) < 0) goto cleanup; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1394,7 +1323,6 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, virDomainPCIAddressSetPtr addrs) { size_t i, j; - virDomainPCIConnectFlags flags = 0; /* initialize to quiet gcc warning */ /* PCI controllers */ for (i = 0; i < def->ncontrollers; i++) { @@ -1408,33 +1336,18 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&cont->info)) continue; - /* convert the type of controller into a "CONNECT_TYPE" - * flag to use when searching for the proper - * controller/bus to connect it to on the upstream side. - */ - flags = virDomainPCIControllerModelToConnectType(model); - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &cont->info, flags) < 0) { + if (qemuDomainPCIAddressReserveNextSlot(addrs, &cont->info) < 0) goto error; - } } } - /* all other devices that plug into a PCI slot are treated as a - * PCI endpoint devices that require a hotplug-capable slot - * (except for some special cases which have specific handling - * below) - */ - flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI_DEVICE; - for (i = 0; i < def->nfss; i++) { if (!virDeviceInfoPCIAddressWanted(&def->fss[i]->info)) continue; /* Only support VirtIO-9p-pci so far. If that changes, * we might need to skip devices here */ - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info) < 0) goto error; } @@ -1450,7 +1363,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&net->info)) { continue; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &net->info, flags) < 0) + + if (qemuDomainPCIAddressReserveNextSlot(addrs, &net->info) < 0) goto error; } @@ -1468,7 +1382,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, continue; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &sound->info, flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &sound->info) < 0) goto error; } @@ -1535,7 +1449,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (foundAddr) { /* Reserve this function on the slot we found */ - if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, + if (virDomainPCIAddressReserveAddr(addrs, &addr, + cont->info.pciConnectFlags, false, true) < 0) goto error; @@ -1544,8 +1459,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, } else { /* This is the first part of the controller, so need * to find a free slot & then reserve this function */ - if (qemuDomainPCIAddressReserveNextAddr(addrs, - &cont->info, flags, + if (qemuDomainPCIAddressReserveNextAddr(addrs, &cont->info, addr.function, false) < 0) { goto error; @@ -1554,10 +1468,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, cont->info.addr.pci.multi = addr.multi; } } else { - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &cont->info, flags) < 0) { - goto error; - } + if (qemuDomainPCIAddressReserveNextSlot(addrs, &cont->info) < 0) + goto error; } } @@ -1588,8 +1500,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, goto error; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->disks[i]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->disks[i]->info) < 0) goto error; } @@ -1601,8 +1512,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, def->hostdevs[i]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, + def->hostdevs[i]->info) < 0) goto error; } @@ -1611,8 +1522,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && virDeviceInfoPCIAddressWanted(&def->memballoon->info)) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->memballoon->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, + &def->memballoon->info) < 0) goto error; } @@ -1622,8 +1533,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&def->rngs[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->rngs[i]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->rngs[i]->info) < 0) goto error; } @@ -1631,8 +1541,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (def->watchdog && def->watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB && virDeviceInfoPCIAddressWanted(&def->watchdog->info)) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info) < 0) goto error; } @@ -1640,16 +1549,15 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, * assigned address. */ if (def->nvideos > 0 && virDeviceInfoPCIAddressWanted(&def->videos[0]->info)) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info) < 0) goto error; } for (i = 1; i < def->nvideos; i++) { if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info, - flags) < 0) + + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info) < 0) goto error; } @@ -1658,8 +1566,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (!virDeviceInfoPCIAddressWanted(&def->shmems[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->shmems[i]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->shmems[i]->info) < 0) goto error; } for (i = 0; i < def->ninputs; i++) { @@ -1667,8 +1574,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&def->inputs[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->inputs[i]->info, - flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->inputs[i]->info) < 0) goto error; } for (i = 0; i < def->nparallels; i++) { @@ -1681,7 +1587,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&chr->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &chr->info, flags) < 0) + if (qemuDomainPCIAddressReserveNextSlot(addrs, &chr->info) < 0) goto error; } for (i = 0; i < def->nchannels; i++) { @@ -1838,8 +1744,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, int rv; bool buses_reserved = true; - virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; - for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; @@ -1851,10 +1755,23 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, nbuses = max_idx + 1; + /* set the connect type flags (pci vs. pcie) in the DeviceInfo + * of all devices. This will be used to pick an appropriate + * bus when assigning addresses. + */ + if (qemuDomainFillAllPCIConnectFlags(def, qemuCaps) < 0) + goto cleanup; + if (nbuses > 0 && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { + /* This is a dummy info used to reserve a slot for a legacy + * PCI device that doesn't exist, but may in the future, e.g. + * if another device is hotplugged into the domain. + */ virDomainDeviceInfo info; + info.pciConnectFlags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; + /* 1st pass to figure out how many PCI bridges we need */ if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) goto cleanup; @@ -1878,24 +1795,50 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, */ if (!buses_reserved && !qemuDomainMachineIsVirt(def) && - qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) + qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) goto cleanup; if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup; for (i = 1; i < addrs->nbuses; i++) { + virDomainDeviceDef dev; + int contIndex; virDomainPCIAddressBusPtr bus = &addrs->buses[i]; if ((rv = virDomainDefMaybeAddController( def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, i, bus->model)) < 0) goto cleanup; - /* If we added a new bridge, we will need one more address */ - if (rv > 0 && - qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) + + if (rv == 0) + continue; /* no new controller added */ + + /* We did add a new controller, so we will need one more + * address (and we need to set the new controller's + * pciConnectFlags) + */ + contIndex = virDomainControllerFind(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCI, + i); + if (contIndex < 0) { + /* this should never happen - we just added it */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find auto-added %s controller " + "with index %zu"), + virDomainControllerModelPCITypeToString(bus->model), + i); + goto cleanup; + } + dev.type = VIR_DOMAIN_DEVICE_CONTROLLER; + dev.data.controller = def->controllers[contIndex]; + /* set connect flags so it will be properly addressed */ + qemuDomainFillDevicePCIConnectFlags(def, &dev, qemuCaps); + if (qemuDomainPCIAddressReserveNextSlot(addrs, + &dev.data.controller->info) < 0) goto cleanup; } + nbuses = addrs->nbuses; virDomainPCIAddressSetFree(addrs); addrs = NULL; -- 2.7.4
On Tue, 2016-10-25 at 09:49 -0400, Laine Stump wrote:
Set pciConnectFlags in each device's DeviceInfo and then use those flags later when validating existing addresses in qemuDomainCollectPCIAddress() and when assigning new addresses with qemuDomainPCIAddressReserveNextAddr() (rather than scattering the logic about which devices need which type of slot all over the place). Â Note that the exact flags set by qemuDomainDeviceCalculatePCIConnectFlags() are different from the flags previously set manually in qemuDomainCollectPCIAddress(), but this doesn't matter because all validation of addresses in that case ignores the setting of the HOTPLUGGABLE flag, and treats PCIE_DEVICE and PCI_DEVICE the same (this lax checking was done on purpose, because there are some things that we want to allow the user to specify manually, e.g. assigning a PCIe device to a PCI slot, that we *don't* ever want libvirt to do automatically. The flag settings that we *really* want to match are 1) the old flag settings in qemuDomainAssignDevicePCISlots() (which is HOTPLUGGABLE | PCI_DEVICE for everything except PCI controllers) and the new flag settings done
[...] and 2) the new [...]
by qemuDomainDeviceCalculatePCIConnectFlags() (which are currently exactly that - HOTPLUGGABLE | PCI_DEVICE for everything except PCI controllers). ---   src/qemu/qemu_domain_address.c | 205 +++++++++++++++--------------------------   1 file changed, 74 insertions(+), 131 deletions(-)  diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 602179c..d731b19 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -721,7 +721,7 @@ qemuDomainFillDevicePCIConnectFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED,    * failure would be some internal problem with    * virDomainDeviceInfoIterate())    */ -static int ATTRIBUTE_UNUSED +static int   qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def,                                    virQEMUCapsPtr qemuCaps)   {
You should remove ATTRIBUTE_UNUSED from qemuDomainFillDevicePCIConnectFlags() as well. [...]
@@ -926,7 +857,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, Â Â Â Â Â Â entireSlot = (addr->function == 0 && Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â addr->multi != VIR_TRISTATE_SWITCH_ON); Â Â -Â Â Â Â if (virDomainPCIAddressReserveAddr(addrs, addr, flags, +Â Â Â Â if (virDomainPCIAddressReserveAddr(addrs, addr, info->pciConnectFlags, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â entireSlot, true) < 0)
Would it be cleaner to have a qemuDomainPCIAddressReserveAddr() function that takes @info directly? It would be used only a single time, so it could very well be argued that it would be overkill... On the other hand, it would be neat not to use virDomainPCIAddressReserve*() functions at all in the qemu driver and rely solely on the wrappers instead. Speaking of which, even with the full series applied there are still a bunch of uses of virDomainPCIAddressReserveAddr() and virDomainPCIAddressReserveSlot(), mostly in qemuDomainValidateDevicePCISlots{PIIX3,Q35}(). The attached patch makes all of them go away, except a few that actually make sense because they set aside addresses for potential future devices and as such don't have access to a virDomainDeviceInfo yet. I'm not saying we should deal with this right away: I'd rather go back later to tidy up the whole thing than hold up the series or go through another round of reviews for what is ultimately a cosmetic issue. [...]
@@ -1878,24 +1795,50 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, Â Â Â Â Â Â Â Â Â Â Â */ Â Â Â Â Â Â Â Â Â Â if (!buses_reserved && Â Â Â Â Â Â Â Â Â Â Â Â Â Â !qemuDomainMachineIsVirt(def) && -Â Â Â Â Â Â Â Â Â Â Â Â qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) +Â Â Â Â Â Â Â Â Â Â Â Â qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto cleanup; Â Â Â Â Â Â Â Â Â Â Â Â if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto cleanup; Â Â Â Â Â Â Â Â Â Â Â Â for (i = 1; i < addrs->nbuses; i++) { +Â Â Â Â Â Â Â Â Â Â Â Â virDomainDeviceDef dev; +Â Â Â Â Â Â Â Â Â Â Â Â int contIndex; Â Â Â Â Â Â Â Â Â Â Â Â Â Â virDomainPCIAddressBusPtr bus = &addrs->buses[i]; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if ((rv = virDomainDefMaybeAddController( Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â i, bus->model)) < 0) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto cleanup; -Â Â Â Â Â Â Â Â Â Â Â Â /* If we added a new bridge, we will need one more address */ -Â Â Â Â Â Â Â Â Â Â Â Â if (rv > 0 && -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) + +Â Â Â Â Â Â Â Â Â Â Â Â if (rv == 0) +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue; /* no new controller added */
Alternatively, you could turn this into  if (rv > 0) {       virDomainDeviceDef dev;       int contIndex;       /* The code below */  } but I leave up to you whether to go one way or the other.
+ +Â Â Â Â Â Â Â Â Â Â Â Â /* We did add a new controller, so we will need one more +Â Â Â Â Â Â Â Â Â Â Â Â Â * address (and we need to set the new controller's +Â Â Â Â Â Â Â Â Â Â Â Â Â * pciConnectFlags) +Â Â Â Â Â Â Â Â Â Â Â Â Â */ +Â Â Â Â Â Â Â Â Â Â Â Â contIndex = virDomainControllerFind(def, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â VIR_DOMAIN_CONTROLLER_TYPE_PCI, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â i); +Â Â Â Â Â Â Â Â Â Â Â Â if (contIndex < 0) { +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* this should never happen - we just added it */ +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â virReportError(VIR_ERR_INTERNAL_ERROR, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("Could not find auto-added %s controller " +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "with index %zu"), +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â virDomainControllerModelPCITypeToString(bus->model), +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â i); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto cleanup; +Â Â Â Â Â Â Â Â Â Â Â Â } +Â Â Â Â Â Â Â Â Â Â Â Â dev.type = VIR_DOMAIN_DEVICE_CONTROLLER; +Â Â Â Â Â Â Â Â Â Â Â Â dev.data.controller = def->controllers[contIndex]; +Â Â Â Â Â Â Â Â Â Â Â Â /* set connect flags so it will be properly addressed */ +Â Â Â Â Â Â Â Â Â Â Â Â qemuDomainFillDevicePCIConnectFlags(def, &dev, qemuCaps); +Â Â Â Â Â Â Â Â Â Â Â Â if (qemuDomainPCIAddressReserveNextSlot(addrs, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &dev.data.controller->info) < 0) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto cleanup; Â Â Â Â Â Â Â Â Â Â } + Â Â Â Â Â Â Â Â Â Â nbuses = addrs->nbuses; Â Â Â Â Â Â Â Â Â Â virDomainPCIAddressSetFree(addrs); Â Â Â Â Â Â Â Â Â Â addrs = NULL;
ACK --Â Andrea Bolognani / Red Hat / Virtualization
On 10/25/2016 12:53 PM, Andrea Bolognani wrote:
On Tue, 2016-10-25 at 09:49 -0400, Laine Stump wrote:
Set pciConnectFlags in each device's DeviceInfo and then use those flags later when validating existing addresses in qemuDomainCollectPCIAddress() and when assigning new addresses with qemuDomainPCIAddressReserveNextAddr() (rather than scattering the logic about which devices need which type of slot all over the place).
Note that the exact flags set by qemuDomainDeviceCalculatePCIConnectFlags() are different from the flags previously set manually in qemuDomainCollectPCIAddress(), but this doesn't matter because all validation of addresses in that case ignores the setting of the HOTPLUGGABLE flag, and treats PCIE_DEVICE and PCI_DEVICE the same (this lax checking was done on purpose, because there are some things that we want to allow the user to specify manually, e.g. assigning a PCIe device to a PCI slot, that we *don't* ever want libvirt to do automatically. The flag settings that we *really* want to match are 1) the old flag settings in qemuDomainAssignDevicePCISlots() (which is HOTPLUGGABLE | PCI_DEVICE for everything except PCI controllers) and the new flag settings done [...] and 2) the new [...]
by qemuDomainDeviceCalculatePCIConnectFlags() (which are currently exactly that - HOTPLUGGABLE | PCI_DEVICE for everything except PCI controllers). --- src/qemu/qemu_domain_address.c | 205 +++++++++++++++-------------------------- 1 file changed, 74 insertions(+), 131 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 602179c..d731b19 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -721,7 +721,7 @@ qemuDomainFillDevicePCIConnectFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED, * failure would be some internal problem with * virDomainDeviceInfoIterate()) */ -static int ATTRIBUTE_UNUSED +static int qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { You should remove ATTRIBUTE_UNUSED from qemuDomainFillDevicePCIConnectFlags() as well.
[...]
@@ -926,7 +857,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, entireSlot = (addr->function == 0 && addr->multi != VIR_TRISTATE_SWITCH_ON);
- if (virDomainPCIAddressReserveAddr(addrs, addr, flags, + if (virDomainPCIAddressReserveAddr(addrs, addr, info->pciConnectFlags, entireSlot, true) < 0) Would it be cleaner to have a qemuDomainPCIAddressReserveAddr() function that takes @info directly?
Actually in a later series (the one that cleans up the *Slot() vs *Addr() naming), I eliminated all but one of the qemuDomainPCIAddressReserve*() functions anyway. After that series, there are only two *PCIAddressReserve*() functions used in this file: qemuDomainPCIAddressReserveNextAddr() (21 times), and virDomainPCIAddressReserveAddr() (12 times). The latter can't have a nice flags-removing wrapper added in qemu_domain_address.c (like the former does) because it often is called with a bare address - no DeviceInfo (Well, I don't know, maybe it could be done by reorganizing some of the calls, I'll have to look at it).
It would be used only a single time, so it could very well be argued that it would be overkill... On the other hand, it would be neat not to use virDomainPCIAddressReserve*() functions at all in the qemu driver and rely solely on the wrappers instead.
Speaking of which, even with the full series applied there are still a bunch of uses of virDomainPCIAddressReserveAddr() and virDomainPCIAddressReserveSlot(), mostly in qemuDomainValidateDevicePCISlots{PIIX3,Q35}().
Yeah, my later series turns all of those into virDomainPCIAddressReserveAddr().
The attached patch makes all of them go away, except a few that actually make sense because they set aside addresses for potential future devices and as such don't have access to a virDomainDeviceInfo yet.
I'm not saying we should deal with this right away: I'd rather go back later to tidy up the whole thing than hold up the series or go through another round of reviews for what is ultimately a cosmetic issue.
[...]
@@ -1878,24 +1795,50 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, */ if (!buses_reserved && !qemuDomainMachineIsVirt(def) && - qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) + qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) goto cleanup;
if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup;
for (i = 1; i < addrs->nbuses; i++) { + virDomainDeviceDef dev; + int contIndex; virDomainPCIAddressBusPtr bus = &addrs->buses[i];
if ((rv = virDomainDefMaybeAddController( def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, i, bus->model)) < 0) goto cleanup; - /* If we added a new bridge, we will need one more address */ - if (rv > 0 && - qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) + + if (rv == 0) + continue; /* no new controller added */ Alternatively, you could turn this into
if (rv > 0) { virDomainDeviceDef dev; int contIndex;
/* The code below */ }
but I leave up to you whether to go one way or the other.
I like the reduced indent level of doing it this way :-)
+ + /* We did add a new controller, so we will need one more + * address (and we need to set the new controller's + * pciConnectFlags) + */ + contIndex = virDomainControllerFind(def, + VIR_DOMAIN_CONTROLLER_TYPE_PCI, + i); + if (contIndex < 0) { + /* this should never happen - we just added it */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find auto-added %s controller " + "with index %zu"), + virDomainControllerModelPCITypeToString(bus->model), + i); + goto cleanup; + } + dev.type = VIR_DOMAIN_DEVICE_CONTROLLER; + dev.data.controller = def->controllers[contIndex]; + /* set connect flags so it will be properly addressed */ + qemuDomainFillDevicePCIConnectFlags(def, &dev, qemuCaps); + if (qemuDomainPCIAddressReserveNextSlot(addrs, + &dev.data.controller->info) < 0) goto cleanup; } + nbuses = addrs->nbuses; virDomainPCIAddressSetFree(addrs); addrs = NULL; ACK
-- Andrea Bolognani / Red Hat / Virtualization
On Tue, 2016-10-25 at 14:08 -0400, Laine Stump wrote:
@@ -926,7 +857,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,         entireSlot = (addr->function == 0 &&                       addr->multi != VIR_TRISTATE_SWITCH_ON);     -    if (virDomainPCIAddressReserveAddr(addrs, addr, flags, +    if (virDomainPCIAddressReserveAddr(addrs, addr, info->pciConnectFlags,                                            entireSlot, true) < 0) Would it be cleaner to have a qemuDomainPCIAddressReserveAddr() function that takes @info directly?  Actually in a later series (the one that cleans up the *Slot() vs *Addr() naming), I eliminated all but one of the qemuDomainPCIAddressReserve*() functions anyway. After that series,Â
[...] there are only two *PCIAddressReserve*() functions used in this file: qemuDomainPCIAddressReserveNextAddr() (21 times), and virDomainPCIAddressReserveAddr() (12 times). The latter can't have a nice flags-removing wrapper added in qemu_domain_address.c (like the former does) because it often is called with a bare address - no DeviceInfo  (Well, I don't know, maybe it could be done by reorganizing some of the calls, I'll have to look at it).  It would be used only a single time, so it could very well be argued that it would be overkill... On the other hand, it would be neat not to use virDomainPCIAddressReserve*() functions at all in the qemu driver and rely solely on the wrappers instead.  Speaking of which, even with the full series applied there are still a bunch of uses of virDomainPCIAddressReserveAddr() and virDomainPCIAddressReserveSlot(), mostly in qemuDomainValidateDevicePCISlots{PIIX3,Q35}().  Yeah, my later series turns all of those into virDomainPCIAddressReserveAddr().
Sorry, I haven't looked at any of your follow-up series at all yet. Disregard my comments then :) --Â Andrea Bolognani / Red Hat / Virtualization
Before now, all the qemu hotplug functions assumed that all devices to be hotplugged were legacy PCI endpoint devices (VIR_PCI_CONNECT_TYPE_PCI_DEVICE). This worked out "okay", because all devices *are* legacy PCI endpoint devices on x86/440fx machinetypes, and hotplug didn't work properly on machinetypes using PCIe anyway (hotplugging onto a legacy PCI slot doesn't work, and until commit b87703cf any attempt to manually specify a PCIe address for a hotplugged device would be erroneously rejected). This patch makes all qemu hotplug operations honor the pciConnectFlags set by the single all-knowing function qemuDomainDeviceCalculatePCIConnectFlags(). This is done in 3 steps, but in a single commit since we would have to touch the other points at each step anyway: 1) add a flags argument to the hypervisor-agnostic virDomainPCIAddressEnsureAddr() (previously it hardcoded ..._PCI_DEVICE) 2) add a new qemu-specific function qemuDomainEnsurePCIAddress() which gets the correct pciConnectFlags for the device from qemuDomainDeviceConnectFlags(), then calls virDomainPCIAddressEnsureAddr(). 3) in qemu_hotplug.c replace all calls to virDomainPCIAddressEnsureAddr() with calls to qemuDomainEnsurePCIAddress() So in effect, we're putting a "shim" on top of all calls to virDomainPCIAddressEnsureAddr() that sets the right pciConnectFlags. --- src/conf/domain_addr.c | 10 ++-------- src/conf/domain_addr.h | 3 ++- src/qemu/qemu_domain_address.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_domain_address.h | 4 ++++ src/qemu/qemu_hotplug.c | 20 ++++++++++++++------ 5 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 3db02f3..2195a95 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -487,17 +487,11 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev) + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) { int ret = -1; char *addrStr = NULL; - /* Flags should be set according to the particular device, - * but only the caller knows the type of device. Currently this - * function is only used for hot-plug, though, and hot-plug is - * only supported for standard PCI devices, so we can safely use - * the setting below */ - virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI_DEVICE); if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci))) goto cleanup; diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 4d6d12a..92ee04c 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -144,7 +144,8 @@ int virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev) + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d731b19..aa54d6b 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2126,6 +2126,33 @@ qemuDomainAssignAddresses(virDomainDefPtr def, return 0; } +/** + * qemuDomainEnsurePCIAddress: + * + * if @dev should have a PCI address but doesn't, assign an address on + * a compatible PCI bus, and set it in @dev->...info. If there is an + * address already, validate that it is on a compatible bus, based on + * @dev->...info.pciConnectFlags. + * + * @obj: the virDomainObjPtr for the domain. This will include + * qemuCaps and address cache (if there is one) + * + * @dev: the device that we need to ensure has a PCI address + * + * returns 0 on success -1 on failure. + */ +int +qemuDomainEnsurePCIAddress(virDomainObjPtr obj, + virDomainDeviceDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + + qemuDomainFillDevicePCIConnectFlags(obj->def, dev, priv->qemuCaps); + + return virDomainPCIAddressEnsureAddr(priv->pciaddrs, info, + info->pciConnectFlags); +} void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index 11d6e92..800859c 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -37,6 +37,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, bool newDomain) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuDomainEnsurePCIAddress(virDomainObjPtr obj, + virDomainDeviceDefPtr dev) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainDeviceInfoPtr info, const char *devstr); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cf69945..4d8d086 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -304,6 +304,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, int ret = -1; int rv; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_DISK, { .disk = disk } }; virErrorPtr orig_err; char *devstr = NULL; char *drivestr = NULL; @@ -344,7 +345,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } else if (!disk->info.type || disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto error; } releaseaddr = true; @@ -462,6 +463,8 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, const char* type = virDomainControllerTypeToString(controller->type); char *devstr = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_CONTROLLER, + { .controller = controller } }; virDomainCCWAddressSetPtr ccwaddrs = NULL; bool releaseaddr = false; @@ -501,7 +504,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto cleanup; } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) @@ -914,6 +917,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainNetDefPtr net) { qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; char **tapfdName = NULL; int *tapfd = NULL; size_t tapfdSize = 0; @@ -1110,7 +1114,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-s390 net device cannot be hotplugged.")); goto cleanup; - } else if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) { + } else if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) { goto cleanup; } @@ -1339,6 +1343,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev) { qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV, + { .hostdev = hostdev } }; int ret; char *devstr = NULL; int configfd = -1; @@ -1409,7 +1415,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto error; - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto error; releaseaddr = true; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO && @@ -1702,6 +1708,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm, { virDomainDefPtr def = vm->def; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_CHR, { .chr = chr } }; int ret = -1; virDomainVirtioSerialAddrSetPtr vioaddrs = NULL; @@ -1717,7 +1724,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm, } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto cleanup; ret = 1; @@ -1857,6 +1864,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_RNG, { .rng = rng } }; virErrorPtr orig_err; char *devstr = NULL; char *charAlias = NULL; @@ -1896,7 +1904,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &rng->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto cleanup; } else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) -- 2.7.4
This patch cleans up the connect flags for certain types/models of devices that aren't PCI to return 0. In the future that may be used as an indicator to the caller about whether or not a device needs a PCI address. For now it's just ignored, except for in virDomainPCIAddressEnsureAddr() - called during device hotplug - (and in some cases actually needs to be re-set to PCI|HOTPLUGGABLE just in case someone (in some old config) has manually set a PCI address for a device that isn't PCI. --- src/conf/domain_addr.c | 6 +++++ src/qemu/qemu_domain_address.c | 54 +++++++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 2195a95..48e9872 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -493,6 +493,12 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, int ret = -1; char *addrStr = NULL; + /* if flags is 0, the particular model of this device on this + * machinetype doesn't need a PCI address, so we're done. + */ + if (!flags) + return 0; + if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci))) goto cleanup; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index aa54d6b..ccfea4d 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -465,8 +465,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2: /* xen only */ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE: case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST: - /* should be 0 */ - return pciFlags; + return 0; } case VIR_DOMAIN_CONTROLLER_TYPE_IDE: @@ -495,8 +494,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, */ if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || STREQ(net->model, "usb-net")) { - /* should be 0 */ - return pciFlags; + return 0; } return pciFlags; } @@ -513,8 +511,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_SOUND_MODEL_PCSPK: case VIR_DOMAIN_SOUND_MODEL_USB: case VIR_DOMAIN_SOUND_MODEL_LAST: - /* should be 0 */ - return pciFlags; + return 0; } case VIR_DOMAIN_DEVICE_DISK: @@ -531,8 +528,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DISK_BUS_SATA: case VIR_DOMAIN_DISK_BUS_SD: case VIR_DOMAIN_DISK_BUS_LAST: - /* should be 0 */ - return pciFlags; + return 0; } case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -546,8 +542,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: - /* should be 0 (not PCI) */ - return pciFlags; + return 0; } case VIR_DOMAIN_DEVICE_RNG: @@ -556,8 +551,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, return pciFlags; case VIR_DOMAIN_RNG_MODEL_LAST: - /* should be 0 */ - return pciFlags; + return 0; } case VIR_DOMAIN_DEVICE_WATCHDOG: @@ -569,8 +563,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_WATCHDOG_MODEL_IB700: case VIR_DOMAIN_WATCHDOG_MODEL_DIAG288: case VIR_DOMAIN_WATCHDOG_MODEL_LAST: - /* should be 0 */ - return pciFlags; + return 0; } case VIR_DOMAIN_DEVICE_VIDEO: @@ -586,8 +579,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, return pciFlags; case VIR_DOMAIN_VIDEO_TYPE_LAST: - /* should be 0 */ - return pciFlags; + return 0; } @@ -604,8 +596,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_INPUT_BUS_XEN: case VIR_DOMAIN_INPUT_BUS_PARALLELS: case VIR_DOMAIN_INPUT_BUS_LAST: - /* should be 0 */ - return pciFlags; + return 0; } case VIR_DOMAIN_DEVICE_CHR: @@ -616,8 +607,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB: case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST: - /* should be 0 */ - return pciFlags; + return 0; } /* These devices don't ever connect with PCI */ @@ -634,8 +624,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_IOMMU: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_NONE: - /* should be 0 */ - return pciFlags; + return 0; } /* We can never get here, because all cases are covered in the @@ -815,6 +804,27 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } + /* If we get to here, the device has a PCI address assigned in the + * config and we should mark it as in-use. But if the + * pciConnectFlags are 0, then this device shouldn't have a PCI + * address associated with it. *BUT* since there are cases in the + * past where we've apparently allowed that, we need to pretend + * for now that it's okay, otherwise an existing domain could + * "disappear" from the list of domains due to a parse failure. We + * can fix this by just forcing the pciConnectFlags to be + * PCI_DEVICE (and then relying on validation functions to report + * inappropriate address types. + */ + if (info->pciConnectFlags == 0) { + char *addrStr = virDomainPCIAddressAsString(&info->addr.pci); + + VIR_WARN("qemuDomainDeviceCalculatePCIConnectFlags() thinks that the " + "device with PCI address %s should not have a PCI address", + addrStr ? addrStr : "(unknown)"); + VIR_FREE(addrStr); + info->pciConnectFlags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; + } + /* Ignore implicit controllers on slot 0:0:1.0: * implicit IDE controller on 0:0:1.1 (no qemu command line) * implicit USB controller on 0:0:1.2 (-usb) -- 2.7.4
libvirt previously assigned nearly all devices to a "hotpluggable" legacy PCI slot even on machines with a PCIe root bus (and even though most such machines don't even support hotplug on legacy PCI slots!) Forcing all devices onto legacy PCI slots means that the domain will need a dmi-to-pci-bridge (to convert from PCIe to legacy PCI) and a pci-bridge (to provide hotpluggable legacy PCI slots which, again, usually aren't hotpluggable anyway). To help reduce the need for these legacy controllers, this patch tries to assign virtio-1.0-capable devices to PCIe slots whenever possible, by setting appropriate connectFlags in virDomainCalculateDevicePCIConnectFlags(). Happily, when that function was written (just a few commits ago) it was created with a "virtioFlags" argument, set by both of its callers, which is the proper connectFlags to set for any virtio-*-pci device - depending on the arch/machinetype of the domain, and whether or not the qemu binary supports virtio-1.0, that flag will have either been set to PCI or PCIE. This patch merely enables the functionality by setting the flags for the device to whatever is in virtioFlags if the device is a virtio-*-pci device. NB: the first virtio video device will be placed directly on bus 0 slot 1 rather than on a pcie-root-port due to the override for primary video devices in qemuDomainValidateDevicePCISlotsQ35(). Whether or not to change that is a topic of discussion, but this patch doesn't change that particular behavior. NB2: since the slot must be hotpluggable, and pcie-root (the PCIe root complex) does *not* support hotplug, this means that suitable controllers must also be in the config (i.e. either pcie-root-port, or pcie-downstream-port). For now, libvirt doesn't add those automatically, so if you put virtio devices in a config for a qemu that has PCIe-capable virtio devices, you'll need to add extra pcie-root-ports yourself. That requirement will be eliminated in a future patch, but for now, it's simple to do this: <controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-root-port'/> ... Partially Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1330024 --- src/qemu/qemu_domain_address.c | 41 ++++-- tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args | 58 ++++++++ tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml | 61 ++++++++ .../qemuxml2argv-q35-virtio-pci.args | 58 ++++++++ .../qemuxml2argv-q35-virtio-pci.xml | 1 + tests/qemuxml2argvtest.c | 48 +++++++ .../qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml | 154 +++++++++++++++++++++ .../qemuxml2xmlout-q35-virtio-pci.xml | 154 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 43 ++++++ 9 files changed, 609 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index ccfea4d..b90e4d1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -428,8 +428,7 @@ static virDomainPCIConnectFlags qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, virDomainPCIConnectFlags pcieFlags ATTRIBUTE_UNUSED, - virDomainPCIConnectFlags virtioFlags - ATTRIBUTE_UNUSED) + virDomainPCIConnectFlags virtioFlags) { virDomainPCIConnectFlags pciFlags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | VIR_PCI_CONNECT_HOTPLUGGABLE); @@ -469,9 +468,28 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, } case VIR_DOMAIN_CONTROLLER_TYPE_IDE: + return pciFlags; + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + switch ((virDomainControllerModelSCSI)cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: + return virtioFlags; + + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078: + return pciFlags; + + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST: + return 0; + } + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: - return pciFlags; + return virtioFlags; case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: @@ -483,7 +501,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_FS: /* the only type of filesystem so far is virtio-9p-pci */ - return pciFlags; + return virtioFlags; case VIR_DOMAIN_DEVICE_NET: { virDomainNetDefPtr net = dev->data.net; @@ -496,6 +514,10 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, STREQ(net->model, "usb-net")) { return 0; } + + if (STREQ(net->model, "virtio")) + return virtioFlags; + return pciFlags; } @@ -517,7 +539,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_DISK: switch ((virDomainDiskBus)dev->data.disk->bus) { case VIR_DOMAIN_DISK_BUS_VIRTIO: - return pciFlags; /* only virtio disks use PCI */ + return virtioFlags; /* only virtio disks use PCI */ case VIR_DOMAIN_DISK_BUS_IDE: case VIR_DOMAIN_DISK_BUS_FDC: @@ -537,7 +559,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_MEMBALLOON: switch ((virDomainMemballoonModel)dev->data.memballoon->model) { case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: - return pciFlags; + return virtioFlags; case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: @@ -548,7 +570,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_RNG: switch ((virDomainRNGModel)dev->data.rng->model) { case VIR_DOMAIN_RNG_MODEL_VIRTIO: - return pciFlags; + return virtioFlags; case VIR_DOMAIN_RNG_MODEL_LAST: return 0; @@ -569,6 +591,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_VIDEO: switch ((virDomainVideoType)dev->data.video->type) { case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: + return virtioFlags; + case VIR_DOMAIN_VIDEO_TYPE_VGA: case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: case VIR_DOMAIN_VIDEO_TYPE_VMVGA: @@ -582,14 +606,13 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, return 0; } - case VIR_DOMAIN_DEVICE_SHMEM: return pciFlags; case VIR_DOMAIN_DEVICE_INPUT: switch ((virDomainInputBus)dev->data.input->bus) { case VIR_DOMAIN_INPUT_BUS_VIRTIO: - return pciFlags; + return virtioFlags; case VIR_DOMAIN_INPUT_BUS_PS2: case VIR_DOMAIN_INPUT_BUS_USB: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args new file mode 100644 index 0000000..f07c085 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args @@ -0,0 +1,58 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name q35-test \ +-S \ +-M q35 \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=4,id=pci.4,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=5,id=pci.5,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=6,id=pci.6,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=7,id=pci.7,bus=pcie.0,addr=0x6 \ +-device ioh3420,port=0x38,chassis=8,id=pci.8,bus=pcie.0,addr=0x7 \ +-device ioh3420,port=0x40,chassis=9,id=pci.9,bus=pcie.0,addr=0x8 \ +-device ioh3420,port=0x48,chassis=10,id=pci.10,bus=pcie.0,addr=0x9 \ +-device ioh3420,port=0x50,chassis=11,id=pci.11,bus=pcie.0,addr=0xa \ +-device ioh3420,port=0x58,chassis=12,id=pci.12,bus=pcie.0,addr=0xb \ +-device ioh3420,port=0x60,chassis=13,id=pci.13,bus=pcie.0,addr=0xc \ +-device ioh3420,port=0x68,chassis=14,id=pci.14,bus=pcie.0,addr=0xd \ +-device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,\ +addr=0x1d \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 \ +-device virtio-scsi-pci,id=scsi0,bus=pci.6,addr=0x0 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.5,addr=0x0 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.7,addr=0x0,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/to/guest \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=/import/from/host,\ +bus=pci.3,addr=0x0 \ +-netdev user,id=hostnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.4,\ +addr=0x0 \ +-device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.10,\ +addr=0x0 \ +-device virtio-mouse-pci,id=input1,bus=pci.11,addr=0x0 \ +-device virtio-keyboard-pci,id=input2,bus=pci.12,addr=0x0 \ +-device virtio-tablet-pci,id=input3,bus=pci.13,addr=0x0 \ +-device virtio-gpu-pci,id=video0,bus=pcie.0,addr=0x1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.8,addr=0x0 \ +-object rng-random,id=objrng0,filename=/dev/urandom \ +-device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,bus=pci.9,\ +addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml new file mode 100644 index 0000000..be2439e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml @@ -0,0 +1,61 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' model='pcie-root'/> + <controller type='pci' model='dmi-to-pci-bridge'/> + <controller type='pci' model='pci-bridge'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> + <controller type='virtio-serial'/> + <controller type='scsi' model='virtio-scsi'/> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vdb' bus='virtio'/> + </disk> + <filesystem type='mount'> + <source dir='/export/to/guest'/> + <target dir='/import/from/host'/> + </filesystem> + <video> + <model type='virtio'/> + </video> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + </interface> + <memballoon model='virtio'/> + <rng model='virtio'> + <rate bytes='123' period='1234'/> + <backend model='random'>/dev/urandom</backend> + </rng> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + </input> + <input type='mouse' bus='virtio'/> + <input type='keyboard' bus='virtio'/> + <input type='tablet' bus='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args new file mode 100644 index 0000000..98284a2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args @@ -0,0 +1,58 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name q35-test \ +-S \ +-M q35 \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=4,id=pci.4,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=5,id=pci.5,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=6,id=pci.6,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=7,id=pci.7,bus=pcie.0,addr=0x6 \ +-device ioh3420,port=0x38,chassis=8,id=pci.8,bus=pcie.0,addr=0x7 \ +-device ioh3420,port=0x40,chassis=9,id=pci.9,bus=pcie.0,addr=0x8 \ +-device ioh3420,port=0x48,chassis=10,id=pci.10,bus=pcie.0,addr=0x9 \ +-device ioh3420,port=0x50,chassis=11,id=pci.11,bus=pcie.0,addr=0xa \ +-device ioh3420,port=0x58,chassis=12,id=pci.12,bus=pcie.0,addr=0xb \ +-device ioh3420,port=0x60,chassis=13,id=pci.13,bus=pcie.0,addr=0xc \ +-device ioh3420,port=0x68,chassis=14,id=pci.14,bus=pcie.0,addr=0xd \ +-device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,\ +addr=0x1d \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 \ +-device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x4 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x3 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.2,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/to/guest \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=/import/from/host,\ +bus=pci.2,addr=0x1 \ +-netdev user,id=hostnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.2,\ +addr=0x2 \ +-device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.2,\ +addr=0x8 \ +-device virtio-mouse-pci,id=input1,bus=pci.2,addr=0x9 \ +-device virtio-keyboard-pci,id=input2,bus=pci.2,addr=0xa \ +-device virtio-tablet-pci,id=input3,bus=pci.2,addr=0xb \ +-device virtio-gpu-pci,id=video0,bus=pcie.0,addr=0x1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x6 \ +-object rng-random,id=objrng0,filename=/dev/urandom \ +-device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,bus=pci.2,\ +addr=0x7 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.xml new file mode 120000 index 0000000..fc8c0ad --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.xml @@ -0,0 +1 @@ +qemuxml2argv-q35-pcie.xml \ No newline at end of file diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 52d85fa..7be18b7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1748,6 +1748,54 @@ mymain(void) QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); + /* verify that devices with pcie capability are assigned to a pcie slot */ + DO_TEST("q35-pcie", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + /* same XML as q35-pcie, but don't set + * QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, so virtio devices should + * be assigned to legacy pci slots + */ + DO_TEST("q35-virtio-pci", + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root-port", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml new file mode 100644 index 0000000..48e9b02 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml @@ -0,0 +1,154 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='4' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='5' port='0x20'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='6' port='0x28'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='7' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='7' port='0x30'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <controller type='pci' index='8' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='8' port='0x38'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </controller> + <controller type='pci' index='9' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='9' port='0x40'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </controller> + <controller type='pci' index='10' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='10' port='0x48'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <controller type='pci' index='11' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='11' port='0x50'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <controller type='pci' index='12' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='12' port='0x58'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <controller type='pci' index='13' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='13' port='0x60'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </controller> + <controller type='pci' index='14' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='14' port='0x68'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> + </controller> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/to/guest'/> + <target dir='/import/from/host'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </filesystem> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + </interface> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> + </input> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> + </memballoon> + <rng model='virtio'> + <rate bytes='123' period='1234'/> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml new file mode 100644 index 0000000..ae541d3 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml @@ -0,0 +1,154 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x05' function='0x0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='4' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='5' port='0x20'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='6' port='0x28'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='7' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='7' port='0x30'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <controller type='pci' index='8' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='8' port='0x38'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </controller> + <controller type='pci' index='9' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='9' port='0x40'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </controller> + <controller type='pci' index='10' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='10' port='0x48'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <controller type='pci' index='11' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='11' port='0x50'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <controller type='pci' index='12' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='12' port='0x58'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <controller type='pci' index='13' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='13' port='0x60'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </controller> + <controller type='pci' index='14' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='14' port='0x68'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x0'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x04' function='0x0'/> + </controller> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/to/guest'/> + <target dir='/import/from/host'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </filesystem> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> + </interface> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x08' function='0x0'/> + </input> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x09' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x0a' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x0b' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x06' function='0x0'/> + </memballoon> + <rng model='virtio'> + <rate bytes='123' period='1234'/> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x02' slot='0x07' function='0x0'/> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 64da80a..dace6c9 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -695,6 +695,49 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); + DO_TEST("q35-pcie", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + /* same XML as q35-pcie, but don't set + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY */ + DO_TEST("q35-virtio-pci", + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, -- 2.7.4
The e1000e is an emulated network device based on the Intel 82574, present in qemu 2.7.0 and later. Among other differences from the e1000, it presents itself as a PCIe device rather than legacy PCI. In order to get it assigned to a PCIe controller, this patch updates the flags setting for network devices when the model name is "e1000e". (Note that for some reason libvirt has never validated the network device model names other than to check that there are no dangerous characters in them. That should probably change, but is the subject of another patch.) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1343094 --- src/qemu/qemu_domain_address.c | 6 ++++-- tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args | 23 ++++++++++++---------- tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml | 4 ++++ .../qemuxml2argv-q35-virtio-pci.args | 3 +++ .../qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml | 23 +++++++++++++--------- .../qemuxml2xmlout-q35-virtio-pci.xml | 5 +++++ 6 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b90e4d1..9d00825 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -426,8 +426,7 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, */ static virDomainPCIConnectFlags qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, - virDomainPCIConnectFlags pcieFlags - ATTRIBUTE_UNUSED, + virDomainPCIConnectFlags pcieFlags, virDomainPCIConnectFlags virtioFlags) { virDomainPCIConnectFlags pciFlags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | @@ -518,6 +517,9 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, if (STREQ(net->model, "virtio")) return virtioFlags; + if (STREQ(net->model, "e1000e")) + return pcieFlags; + return pciFlags; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args index f07c085..d92e0b8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args @@ -35,10 +35,10 @@ QEMU_AUDIO_DRV=none \ addr=0x1d \ -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 \ -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 \ --device virtio-scsi-pci,id=scsi0,bus=pci.6,addr=0x0 \ --device virtio-serial-pci,id=virtio-serial0,bus=pci.5,addr=0x0 \ +-device virtio-scsi-pci,id=scsi0,bus=pci.7,addr=0x0 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.6,addr=0x0 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ --device virtio-blk-pci,bus=pci.7,addr=0x0,drive=drive-virtio-disk1,\ +-device virtio-blk-pci,bus=pci.8,addr=0x0,drive=drive-virtio-disk1,\ id=virtio-disk1 \ -fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/to/guest \ -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=/import/from/host,\ @@ -46,13 +46,16 @@ bus=pci.3,addr=0x0 \ -netdev user,id=hostnet0 \ -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.4,\ addr=0x0 \ --device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.10,\ +-netdev user,id=hostnet1 \ +-device e1000e,netdev=hostnet1,id=net1,mac=00:11:22:33:44:66,bus=pci.5,\ addr=0x0 \ --device virtio-mouse-pci,id=input1,bus=pci.11,addr=0x0 \ --device virtio-keyboard-pci,id=input2,bus=pci.12,addr=0x0 \ --device virtio-tablet-pci,id=input3,bus=pci.13,addr=0x0 \ +-device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.11,\ +addr=0x0 \ +-device virtio-mouse-pci,id=input1,bus=pci.12,addr=0x0 \ +-device virtio-keyboard-pci,id=input2,bus=pci.13,addr=0x0 \ +-device virtio-tablet-pci,id=input3,bus=pci.14,addr=0x0 \ -device virtio-gpu-pci,id=video0,bus=pcie.0,addr=0x1 \ --device virtio-balloon-pci,id=balloon0,bus=pci.8,addr=0x0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.9,addr=0x0 \ -object rng-random,id=objrng0,filename=/dev/urandom \ --device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,bus=pci.9,\ -addr=0x0 +-device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,\ +bus=pci.10,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml index be2439e..39db5f0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml @@ -46,6 +46,10 @@ <mac address='00:11:22:33:44:55'/> <model type='virtio'/> </interface> + <interface type='user'> + <mac address='00:11:22:33:44:66'/> + <model type='e1000e'/> + </interface> <memballoon model='virtio'/> <rng model='virtio'> <rate bytes='123' period='1234'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args index 98284a2..68d19ca 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args @@ -46,6 +46,9 @@ bus=pci.2,addr=0x1 \ -netdev user,id=hostnet0 \ -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.2,\ addr=0x2 \ +-netdev user,id=hostnet1 \ +-device e1000e,netdev=hostnet1,id=net1,mac=00:11:22:33:44:66,bus=pci.3,\ +addr=0x0 \ -device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.2,\ addr=0x8 \ -device virtio-mouse-pci,id=input1,bus=pci.2,addr=0x9 \ diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml index 48e9b02..5a23a51 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml @@ -17,7 +17,7 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='vdb' bus='virtio'/> - <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> </disk> <controller type='pci' index='0' model='pcie-root'/> <controller type='pci' index='1' model='dmi-to-pci-bridge'> @@ -90,10 +90,10 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </controller> <controller type='virtio-serial' index='0'> - <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> - <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> </controller> <controller type='usb' index='0' model='ich9-ehci1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/> @@ -123,18 +123,23 @@ <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </interface> + <interface type='user'> + <mac address='00:11:22:33:44:66'/> + <model type='e1000e'/> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </interface> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> - <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> </input> <input type='mouse' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> </input> <input type='keyboard' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> </input> <input type='tablet' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0e' slot='0x00' function='0x0'/> </input> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> @@ -143,12 +148,12 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </video> <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> </memballoon> <rng model='virtio'> <rate bytes='123' period='1234'/> <backend model='random'>/dev/urandom</backend> - <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> </rng> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml index ae541d3..22c2c68 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml @@ -123,6 +123,11 @@ <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> </interface> + <interface type='user'> + <mac address='00:11:22:33:44:66'/> + <model type='e1000e'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </interface> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x02' slot='0x08' function='0x0'/> -- 2.7.4
The nec-usb-xhci device (which is a USB3 controller) has always presented itself as a PCI device when plugged into a legacy PCI slot, and a PCIe device when plugged into a PCIe slot, but libvirt has always auto-assigned it to a legacy PCI slot. This patch changes that behavior to auto-assign to a PCIe slot on systems that have pcie-root (e.g. Q35 and aarch64/virt). Since we don't yet auto-create pcie-*-port controllers on demand, this means a config with an nec-xhci USB controller that has no PCI address assigned will also need to have an otherwise-unused pcie-*-port controller specified: <controller type='pci' model='pcie-root-port'/> <controller type='usb' model='nec-xhci'/> (this assumes there is an otherwise-unused slot on pcie-root to accept the pcie-root-port) --- src/qemu/qemu_domain_address.c | 2 +- tests/qemuxml2argvdata/qemuxml2argv-autoindex.args | 10 +++---- tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args | 21 ++++++------- tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml | 2 ++ .../qemuxml2argv-q35-virtio-pci.args | 7 ++--- tests/qemuxml2argvtest.c | 2 ++ .../qemuxml2xmlout-autoindex.xml | 10 +++---- .../qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml | 35 +++++++++------------- .../qemuxml2xmlout-q35-virtio-pci.xml | 21 +++++-------- tests/qemuxml2xmltest.c | 2 ++ 10 files changed, 49 insertions(+), 63 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 9d00825..f1f46b7 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -446,7 +446,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CONTROLLER_TYPE_USB: switch ((virDomainControllerModelUSB)cont->model) { case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: - return pciFlags; + return pcieFlags; case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args b/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args index 43b9661..fc14dc4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-autoindex.args @@ -43,11 +43,11 @@ addr=0x1 \ -device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.2,addr=0x1.0x1 \ -device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.2,addr=0x1.0x2 \ -device ich9-usb-ehci1,id=usb2,bus=pci.2,addr=0x1.0x7 \ --device nec-usb-xhci,id=usb3,bus=pci.2,addr=0x2 \ +-device nec-usb-xhci,id=usb3,bus=pci.5,addr=0x0 \ -device ich9-usb-uhci1,masterbus=usb4.0,firstport=0,bus=pci.2,multifunction=on,\ -addr=0x3 \ --device ich9-usb-uhci2,masterbus=usb4.0,firstport=2,bus=pci.2,addr=0x3.0x1 \ --device ich9-usb-uhci3,masterbus=usb4.0,firstport=4,bus=pci.2,addr=0x3.0x2 \ --device ich9-usb-ehci1,id=usb4,bus=pci.2,addr=0x3.0x7 \ +addr=0x2 \ +-device ich9-usb-uhci2,masterbus=usb4.0,firstport=2,bus=pci.2,addr=0x2.0x1 \ +-device ich9-usb-uhci3,masterbus=usb4.0,firstport=4,bus=pci.2,addr=0x2.0x2 \ +-device ich9-usb-ehci1,id=usb4,bus=pci.2,addr=0x2.0x7 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-sata0-0-0 \ -device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args index d92e0b8..2738749 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.args @@ -30,15 +30,12 @@ QEMU_AUDIO_DRV=none \ -device ioh3420,port=0x58,chassis=12,id=pci.12,bus=pcie.0,addr=0xb \ -device ioh3420,port=0x60,chassis=13,id=pci.13,bus=pcie.0,addr=0xc \ -device ioh3420,port=0x68,chassis=14,id=pci.14,bus=pcie.0,addr=0xd \ --device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7 \ --device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,\ -addr=0x1d \ --device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 \ --device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 \ +-device ioh3420,port=0x70,chassis=15,id=pci.15,bus=pcie.0,addr=0xe \ +-device nec-usb-xhci,id=usb,bus=pci.8,addr=0x0 \ -device virtio-scsi-pci,id=scsi0,bus=pci.7,addr=0x0 \ -device virtio-serial-pci,id=virtio-serial0,bus=pci.6,addr=0x0 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ --device virtio-blk-pci,bus=pci.8,addr=0x0,drive=drive-virtio-disk1,\ +-device virtio-blk-pci,bus=pci.9,addr=0x0,drive=drive-virtio-disk1,\ id=virtio-disk1 \ -fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/to/guest \ -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=/import/from/host,\ @@ -49,13 +46,13 @@ addr=0x0 \ -netdev user,id=hostnet1 \ -device e1000e,netdev=hostnet1,id=net1,mac=00:11:22:33:44:66,bus=pci.5,\ addr=0x0 \ --device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.11,\ +-device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.12,\ addr=0x0 \ --device virtio-mouse-pci,id=input1,bus=pci.12,addr=0x0 \ --device virtio-keyboard-pci,id=input2,bus=pci.13,addr=0x0 \ --device virtio-tablet-pci,id=input3,bus=pci.14,addr=0x0 \ +-device virtio-mouse-pci,id=input1,bus=pci.13,addr=0x0 \ +-device virtio-keyboard-pci,id=input2,bus=pci.14,addr=0x0 \ +-device virtio-tablet-pci,id=input3,bus=pci.15,addr=0x0 \ -device virtio-gpu-pci,id=video0,bus=pcie.0,addr=0x1 \ --device virtio-balloon-pci,id=balloon0,bus=pci.9,addr=0x0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.10,addr=0x0 \ -object rng-random,id=objrng0,filename=/dev/urandom \ -device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,\ -bus=pci.10,addr=0x0 +bus=pci.11,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml index 39db5f0..b6b971b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml @@ -29,8 +29,10 @@ <controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-root-port'/> + <controller type='pci' model='pcie-root-port'/> <controller type='virtio-serial'/> <controller type='scsi' model='virtio-scsi'/> + <controller type='usb' model='nec-xhci'/> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='vdb' bus='virtio'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args index 68d19ca..cada05e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virtio-pci.args @@ -30,11 +30,8 @@ QEMU_AUDIO_DRV=none \ -device ioh3420,port=0x58,chassis=12,id=pci.12,bus=pcie.0,addr=0xb \ -device ioh3420,port=0x60,chassis=13,id=pci.13,bus=pcie.0,addr=0xc \ -device ioh3420,port=0x68,chassis=14,id=pci.14,bus=pcie.0,addr=0xd \ --device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7 \ --device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,\ -addr=0x1d \ --device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 \ --device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 \ +-device ioh3420,port=0x70,chassis=15,id=pci.15,bus=pcie.0,addr=0xe \ +-device nec-usb-xhci,id=usb,bus=pci.4,addr=0x0 \ -device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x4 \ -device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x3 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7be18b7..2fdde2e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1770,6 +1770,7 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); /* same XML as q35-pcie, but don't set * QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, so virtio devices should @@ -1795,6 +1796,7 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root-port", QEMU_CAPS_DEVICE_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml index 086a1cf..2a8a44a 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-autoindex.xml @@ -126,22 +126,22 @@ <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x7'/> </controller> <controller type='usb' index='3' model='nec-xhci'> - <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> </controller> <controller type='usb' index='4' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x0' multifunction='on'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0' multifunction='on'/> </controller> <controller type='usb' index='4' model='ich9-uhci2'> <master startport='2'/> - <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x1'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x1'/> </controller> <controller type='usb' index='4' model='ich9-uhci3'> <master startport='4'/> - <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x2'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x2'/> </controller> <controller type='usb' index='4' model='ich9-ehci1'> - <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x7'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x7'/> </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml index 5a23a51..8e727fb 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml @@ -17,7 +17,7 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='vdb' bus='virtio'/> - <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> </disk> <controller type='pci' index='0' model='pcie-root'/> <controller type='pci' index='1' model='dmi-to-pci-bridge'> @@ -89,26 +89,19 @@ <target chassis='14' port='0x68'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </controller> + <controller type='pci' index='15' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='15' port='0x70'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </controller> <controller type='virtio-serial' index='0'> <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> </controller> - <controller type='usb' index='0' model='ich9-ehci1'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci1'> - <master startport='0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci2'> - <master startport='2'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci3'> - <master startport='4'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/> + <controller type='usb' index='0' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> @@ -130,16 +123,16 @@ </interface> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> - <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> </input> <input type='mouse' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> </input> <input type='keyboard' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0e' slot='0x00' function='0x0'/> </input> <input type='tablet' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0e' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0f' slot='0x00' function='0x0'/> </input> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> @@ -148,12 +141,12 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </video> <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> </memballoon> <rng model='virtio'> <rate bytes='123' period='1234'/> <backend model='random'>/dev/urandom</backend> - <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> </rng> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml index 22c2c68..c4bd357 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virtio-pci.xml @@ -89,26 +89,19 @@ <target chassis='14' port='0x68'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </controller> + <controller type='pci' index='15' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='15' port='0x70'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </controller> <controller type='virtio-serial' index='0'> <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x0'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> <address type='pci' domain='0x0000' bus='0x02' slot='0x04' function='0x0'/> </controller> - <controller type='usb' index='0' model='ich9-ehci1'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci1'> - <master startport='0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci2'> - <master startport='2'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/> - </controller> - <controller type='usb' index='0' model='ich9-uhci3'> - <master startport='4'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/> + <controller type='usb' index='0' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index dace6c9..d57402a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -715,6 +715,7 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); /* same XML as q35-pcie, but don't set QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY */ @@ -737,6 +738,7 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root", -- 2.7.4
Andrea had the right idea when he disabled the "reserve an extra unused slot" bit for aarch64/virt. For *any* PCI Express-based machine, it is pointless since 1) an extra legacy-PCI slot can't be used for hotplug, since hotplug into legacy PCI slots doesn't work on PCI Express machinetypes, and 2) even for "coldplug" expansion, everybody will want to expand using Express controllers, not legacy PCI. This patch eliminates the extra slot reserve unless the system has a pci-root (i.e. legacy PCI) --- src/qemu/qemu_domain_address.c | 44 ++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index f1f46b7..eb754f1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1805,8 +1805,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, */ virDomainDeviceInfo info; - info.pciConnectFlags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; - /* 1st pass to figure out how many PCI bridges we need */ if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) goto cleanup; @@ -1815,23 +1813,35 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, addrs) < 0) goto cleanup; - for (i = 0; i < addrs->nbuses; i++) { - if (!qemuDomainPCIBusFullyReserved(&addrs->buses[i])) - buses_reserved = false; - } - - /* Reserve 1 extra slot for a (potential) bridge only if buses - * are not fully reserved yet. + /* For domains that have pci-root, reserve 1 extra slot for a + * (potential) bridge (for future expansion) only if buses are + * not fully reserved yet (if all buses are fully reserved + * with manually/previously assigned addresses, any attempt to + * reserve an extra slot would fail anyway. But if all buses + * are *not* fully reserved, this extra reservation might push + * the config to add a new pci-bridge to plug into the final + * available slot, thus preserving the ability to expand) * - * We don't reserve the extra slot for aarch64 mach-virt guests - * either because we want to be able to have pure virtio-mmio - * guests, and reserving this slot would force us to add at least - * a dmi-to-pci-bridge to an otherwise PCI-free topology + * We only do this for those domains that have pci-root, since + * those with pcie-root will usually want to expand using PCIe + * controllers, which we will do after assigning addresses for + * all *actual* devices. */ - if (!buses_reserved && - !qemuDomainMachineIsVirt(def) && - qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) - goto cleanup; + + if (qemuDomainMachineHasPCIRoot(def)) { + info.pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI_DEVICE); + + for (i = 0; i < addrs->nbuses; i++) { + if (!qemuDomainPCIBusFullyReserved(&addrs->buses[i])) { + buses_reserved = false; + break; + } + } + if (!buses_reserved && + qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) + goto cleanup; + } if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup; -- 2.7.4
Previously libvirt would only add pci-bridge devices automatically when an address was requested for a device that required a legacy PCI slot and none was available. This patch expands that support to dmi-to-pci-bridge (which is needed in order to add a pci-bridge on a machine with a pcie-root), and pcie-root-port (which is needed to add a hotpluggable PCIe device). It does *not* automatically add pcie-switch-upstream-ports or pcie-switch-downstream-ports (and currently there are no plans for that). Given the existing code to auto-add pci-bridge devices, automatically adding pcie-root-ports is fairly straightforward. The dmi-to-pci-bridge support is a bit tricky though, for a few reasons: 1) Although the only reason to add a dmi-to-pci-bridge is so that there is a reasonable place to plug in a pci-bridge controller, most of the time it's not the presence of a pci-bridge *in the config* that triggers the requirement to add a dmi-to-pci-bridge. Rather, it is the presence of a legacy-PCI device in the config, which triggers auto-add of a pci-bridge, which triggers auto-add of a dmi-to-pci-bridge (this is handled in virDomainPCIAddressSetGrow() - if there's a request to add a pci-bridge we'll check if there is a suitable bus to plug it into; if not, we first add a dmi-to-pci-bridge). 2) Once there is already a single dmi-to-pci-bridge on the system, there won't be a need for any more, even if it's full, as long as there is a pci-bridge with an open slot - you can also plug pci-bridges into existing pci-bridges. So we have to make sure we don't add a dmi-to-pci-bridge unless there aren't any dmi-to-pci-bridges *or* any pci-bridges. 3) Although it is strongly discouraged, it is legal for a pci-bridge to be directly plugged into pcie-root, and we don't want to auto-add a dmi-to-pci-bridge if there is already a pci-bridge that's been forced directly into pcie-root. Although libvirt will now automatically create a dmi-to-pci-bridge when it's needed, the code still remains for now that forces a dmi-to-pci-bridge on all domains with pcie-root (in qemuDomainDefAddDefaultDevices()). That will be removed in the next patch. For now, the pcie-root-ports are added one to a slot, which is a bit wasteful and means it will fail after 31 total PCIe devices (30 if there are also some PCI devices), but helps keep the changeset down for this patch. A future patch will have 8 pcie-root-ports sharing the functions on a single slot. --- src/conf/domain_addr.c | 96 +++++++++++--- src/qemu/qemu_domain_address.c | 55 +++++--- .../qemuxml2argv-q35-pcie-autoadd.args | 57 ++++++++ .../qemuxml2argv-q35-pcie-autoadd.xml | 51 +++++++ tests/qemuxml2argvtest.c | 24 ++++ .../qemuxml2xmlout-q35-pcie-autoadd.xml | 147 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 25 +++- 7 files changed, 421 insertions(+), 34 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 48e9872..718fde7 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -82,6 +82,30 @@ virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model) return 0; } + +static int +virDomainPCIControllerConnectTypeToModel(virDomainPCIConnectFlags flags) +{ + if (flags & VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT) + return VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT; + if (flags & VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT) + return VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT; + if (flags & VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT) + return VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT; + if (flags & VIR_PCI_CONNECT_TYPE_DMI_TO_PCI_BRIDGE) + return VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE; + if (flags & VIR_PCI_CONNECT_TYPE_PCI_EXPANDER_BUS) + return VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS; + if (flags & VIR_PCI_CONNECT_TYPE_PCIE_EXPANDER_BUS) + return VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS; + if (flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE) + return VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE; + + /* some connect types don't correspond to a controller model */ + return -1; +} + + bool virDomainPCIAddressFlagsCompatible(virPCIDeviceAddressPtr addr, const char *addrStr, @@ -334,10 +358,7 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, } -/* Ensure addr fits in the address set, by expanding it if needed. - * This will only grow if the flags say that we need a normal - * hot-pluggable PCI slot. If we need a different type of slot, it - * will fail. +/* Ensure addr fits in the address set, by expanding it if needed * * Return value: * -1 = OOM @@ -351,32 +372,73 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, { int add; size_t i; + int model; + bool needDMIToPCIBridge = false; add = addr->bus - addrs->nbuses + 1; - i = addrs->nbuses; if (add <= 0) return 0; - /* auto-grow only works when we're adding plain PCI devices */ - if (!(flags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot automatically add a new PCI bus for a " - "device requiring a slot other than standard PCI.")); + if (flags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE) { + model = VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE; + + /* if there aren't yet any buses that will accept a + * pci-bridge, and the caller is asking for one, we'll need to + * add a dmi-to-pci-bridge first. + */ + needDMIToPCIBridge = true; + for (i = 0; i < addrs->nbuses; i++) { + if (addrs->buses[i].flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE) { + needDMIToPCIBridge = false; + break; + } + } + if (needDMIToPCIBridge && add == 1) { + /* we need to add at least two buses - one dmi-to-pci, + * and the other the requested pci-bridge + */ + add++; + addr->bus++; + } + } else if (flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE && + addrs->buses[0].model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { + model = VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE; + } else if (flags & (VIR_PCI_CONNECT_TYPE_PCIE_DEVICE | + VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT)) { + model = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT; + } else { + int existingContModel = virDomainPCIControllerConnectTypeToModel(flags); + + if (existingContModel >= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("a PCI slot is needed to connect a PCI controller " + "model='%s', but none is available, and it " + "cannot be automatically added"), + virDomainControllerModelPCITypeToString(existingContModel)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot automatically add a new PCI bus for a " + "device with connect flags %.2x"), flags); + } return -1; } + i = addrs->nbuses; + if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, add) < 0) return -1; - for (; i < addrs->nbuses; i++) { - /* Any time we auto-add a bus, we will want a multi-slot - * bus. Currently the only type of bus we will auto-add is a - * pci-bridge, which is hot-pluggable and provides standard - * PCI slots. + if (needDMIToPCIBridge) { + /* first of the new buses is dmi-to-pci-bridge, the + * rest are of the requested type */ - virDomainPCIAddressBusSetModel(&addrs->buses[i], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); + virDomainPCIAddressBusSetModel(&addrs->buses[i++], + VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE); } + + for (; i < addrs->nbuses; i++) + virDomainPCIAddressBusSetModel(&addrs->buses[i], model); + return add; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index eb754f1..9e71ed5 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -908,27 +908,14 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, { virDomainPCIAddressSetPtr addrs; size_t i; + bool hasPCIeRoot = false; + virDomainControllerModelPCI defaultModel; if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) return NULL; addrs->dryRun = dryRun; - /* As a safety measure, set default model='pci-root' for first pci - * controller and 'pci-bridge' for all subsequent. After setting - * those defaults, then scan the config and set the actual model - * for all addrs[idx]->bus that already have a corresponding - * controller in the config. - * - */ - if (nbuses > 0) - virDomainPCIAddressBusSetModel(&addrs->buses[0], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); - for (i = 1; i < nbuses; i++) { - virDomainPCIAddressBusSetModel(&addrs->buses[i], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); - } - for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; size_t idx = cont->idx; @@ -945,7 +932,43 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], cont->model) < 0) goto error; - } + + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) + hasPCIeRoot = true; + } + + if (nbuses > 0 && !addrs->buses[0].model) { + /* This is just here to replicate a safety measure already in + * an older version of this code. In practice, the root bus + * should have already been added at index 0 prior to + * assigning addresses to devices. + */ + if (virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) + goto error; + } + + /* Now fill in a reasonable model for all the buses in the set + * that don't yet have a corresponding controller in the domain + * config. + */ + + if (hasPCIeRoot) + defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT; + else + defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE; + + for (i = 1; i < addrs->nbuses; i++) { + + if (addrs->buses[i].model) + continue; + + if (virDomainPCIAddressBusSetModel(&addrs->buses[i], defaultModel) < 0) + goto error; + + VIR_DEBUG("Auto-adding <controller type='pci' model='%s' index='%zu'/>", + virDomainControllerModelPCITypeToString(defaultModel), i); + } if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0) goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args new file mode 100644 index 0000000..70c759e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args @@ -0,0 +1,57 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name q35-test \ +-S \ +-M q35 \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device ioh3420,port=0x10,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 \ +-device ioh3420,port=0x38,chassis=7,id=pci.7,bus=pcie.0,addr=0x7 \ +-device ioh3420,port=0x40,chassis=8,id=pci.8,bus=pcie.0,addr=0x8 \ +-device ioh3420,port=0x48,chassis=9,id=pci.9,bus=pcie.0,addr=0x9 \ +-device ioh3420,port=0x50,chassis=10,id=pci.10,bus=pcie.0,addr=0xa \ +-device ioh3420,port=0x58,chassis=11,id=pci.11,bus=pcie.0,addr=0xb \ +-device ioh3420,port=0x60,chassis=12,id=pci.12,bus=pcie.0,addr=0xc \ +-device ioh3420,port=0x68,chassis=13,id=pci.13,bus=pcie.0,addr=0xd \ +-device ioh3420,port=0x70,chassis=14,id=pci.14,bus=pcie.0,addr=0xe \ +-device nec-usb-xhci,id=usb,bus=pci.7,addr=0x0 \ +-device virtio-scsi-pci,id=scsi0,bus=pci.6,addr=0x0 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.5,addr=0x0 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.8,addr=0x0,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/to/guest \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=/import/from/host,\ +bus=pci.2,addr=0x0 \ +-netdev user,id=hostnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.3,\ +addr=0x0 \ +-netdev user,id=hostnet1 \ +-device e1000e,netdev=hostnet1,id=net1,mac=00:11:22:33:44:66,bus=pci.4,\ +addr=0x0 \ +-device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.11,\ +addr=0x0 \ +-device virtio-mouse-pci,id=input1,bus=pci.12,addr=0x0 \ +-device virtio-keyboard-pci,id=input2,bus=pci.13,addr=0x0 \ +-device virtio-tablet-pci,id=input3,bus=pci.14,addr=0x0 \ +-device virtio-gpu-pci,id=video0,bus=pcie.0,addr=0x1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.9,addr=0x0 \ +-object rng-random,id=objrng0,filename=/dev/urandom \ +-device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,\ +bus=pci.10,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.xml new file mode 100644 index 0000000..06cdf61 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.xml @@ -0,0 +1,51 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='virtio-serial'/> + <controller type='scsi' model='virtio-scsi'/> + <controller type='usb' model='nec-xhci'/> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vdb' bus='virtio'/> + </disk> + <filesystem type='mount'> + <source dir='/export/to/guest'/> + <target dir='/import/from/host'/> + </filesystem> + <video> + <model type='virtio'/> + </video> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:66'/> + <model type='e1000e'/> + </interface> + <memballoon model='virtio'/> + <rng model='virtio'> + <rate bytes='123' period='1234'/> + <backend model='random'>/dev/urandom</backend> + </rng> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + </input> + <input type='mouse' bus='virtio'/> + <input type='keyboard' bus='virtio'/> + <input type='tablet' bus='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2fdde2e..6d9d101 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1798,6 +1798,30 @@ mymain(void) QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + /* same as q35-pcie, but all PCI controllers are added automatically */ + DO_TEST("q35-pcie-autoadd", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root-port", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml new file mode 100644 index 0000000..b27dbe7 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml @@ -0,0 +1,147 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> + </disk> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> + </controller> + <controller type='usb' index='0' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='4' port='0x20'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='5' port='0x28'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='6' port='0x30'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <controller type='pci' index='7' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='7' port='0x38'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </controller> + <controller type='pci' index='8' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='8' port='0x40'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </controller> + <controller type='pci' index='9' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='9' port='0x48'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <controller type='pci' index='10' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='10' port='0x50'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <controller type='pci' index='11' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='11' port='0x58'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <controller type='pci' index='12' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='12' port='0x60'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </controller> + <controller type='pci' index='13' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='13' port='0x68'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + </controller> + <controller type='pci' index='14' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='14' port='0x70'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/to/guest'/> + <target dir='/import/from/host'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </filesystem> + <interface type='user'> + <mac address='00:11:22:33:44:55'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:44:66'/> + <model type='e1000e'/> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + </interface> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> + </input> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x0e' slot='0x00' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> + </memballoon> + <rng model='virtio'> + <rate bytes='123' period='1234'/> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d57402a..6186bba 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -740,7 +740,30 @@ mymain(void) QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); - + /* same as q35-pcie, but all PCI controllers are added automatically */ + DO_TEST("q35-pcie-autoadd", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, -- 2.7.4
I'm undecided if it is worthwhile to add this... Up until now it has been legal to have something like this in the xml: <controller type='pci' model='pcie-root' index='0'/> <controller type='pci' model='pci-bridge' index='2'/> (for example, see the existing test case "usb-controller-default-q35"). This is handled in qemuDomainPCIAddressSetCreate() when it's adding in controllers to fill holes in the indexes.) Since we have always added the following to every config: <controller type='pci' model='dmi-to-pci-bridge' index='1'/> there has always been a proper place for the unaddressed pci-bridge to plug in. A upcoming commit will eliminate the forced adding of a dmi-to-pci-bridge to *every* Q35 domain config, though. This would mean the above-mentioned test case (and one other) would begin to fail. There are two possible solutions to this problem: 1) change the test cases, and made the above construct an error (note that in the future it will work just fine to not list *any* pci controller, and they will all be auto-added as needed). or 2) This patch, which specifically looks for the case where we have a pci-bridge (but no dmi-to-pci-bridge) in a PCIe domain config and auto-adds the necessary dmi-to-pci-bridge. This can only work in the case that the pci-bridge has been given an index such that there is an unused index with a lower value for the dmi-to-pci-bridge to occupy. This seems like a kind of odd corner case that nobody should need to do in the future anyway. On the other hand, I already wrote the code and it works, so I might as well send it and see what opinions (if any) it generates. --- src/qemu/qemu_domain_address.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 9e71ed5..e1e1f77 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -909,6 +909,9 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, virDomainPCIAddressSetPtr addrs; size_t i; bool hasPCIeRoot = false; + unsigned int lowestDMIToPCIBridge = nbuses; + unsigned int lowestUnaddressedPCIBridge = nbuses; + unsigned int lowestAddressedPCIBridge = nbuses; virDomainControllerModelPCI defaultModel; if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) @@ -933,8 +936,24 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], cont->model) < 0) goto error; - if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) + /* we'll use all this info later to determine if we need + * to add a dmi-to-pci-bridge due to unaddressed pci-bridge controllers + */ + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { hasPCIeRoot = true; + } else if (cont->model == + VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) { + if (lowestDMIToPCIBridge > idx) + lowestDMIToPCIBridge = idx; + } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) { + if (virDeviceInfoPCIAddressWanted(&cont->info)) { + if (lowestUnaddressedPCIBridge > idx) + lowestUnaddressedPCIBridge = idx; + } else { + if (lowestAddressedPCIBridge > idx) + lowestAddressedPCIBridge = idx; + } + } } if (nbuses > 0 && !addrs->buses[0].model) { @@ -950,13 +969,21 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, /* Now fill in a reasonable model for all the buses in the set * that don't yet have a corresponding controller in the domain - * config. + * config. In the rare (and ... strange, but still allowed) case + * that a domain has 1) a pcie-root at index 0, 2) *no* + * dmi-to-pci-bridge (or pci-bridge that was manually addressed to + * sit directly on pcie-root), and 3) does have an unaddressed + * pci-bridge at an index > 1, then we need to add a + * dmi-to-pci-bridge. */ - if (hasPCIeRoot) - defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT; - else + if (!hasPCIeRoot) defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE; + else if (lowestUnaddressedPCIBridge < MIN(lowestAddressedPCIBridge, + lowestDMIToPCIBridge)) + defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE; + else + defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT; for (i = 1; i < addrs->nbuses; i++) { @@ -968,6 +995,11 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, VIR_DEBUG("Auto-adding <controller type='pci' model='%s' index='%zu'/>", virDomainControllerModelPCITypeToString(defaultModel), i); + /* only add a single dmi-to-pci-bridge, then add pcie-root-port + * for any other unspecified controller indexes. + */ + if (hasPCIeRoot) + defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT; } if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0) -- 2.7.4
Now the a dmi-to-pci-bridge is automatically added just as it's needed (when a pci-bridge is being added), we no longer have any need to force-add one to every single Q35 domain. --- src/qemu/qemu_domain.c | 12 ---- tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 3 +- .../qemuxml2argv-q35-pcie-autoadd.args | 55 +++++++-------- .../qemuxml2xmlout-pcie-root.xml | 4 -- .../qemuxml2xmlout-q35-pcie-autoadd.xml | 82 ++++++++++------------ 5 files changed, 67 insertions(+), 89 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 310bfa6..5327add 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2125,18 +2125,6 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) { goto cleanup; } - /* Add a dmi-to-pci-bridge bridge if there are no PCI controllers - * other than the pcie-root. This is so that there will be hot-pluggable - * PCI slots available. - * - * We skip this step for aarch64 mach-virt guests, where we want to - * be able to have a pure virtio-mmio topology - */ - if (virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1) < 0 && - !qemuDomainMachineIsVirt(def) && - !virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1, - VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE)) - goto cleanup; } if (addDefaultMemballoon && !def->memballoon) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args index 7ef03d3..59a849f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args @@ -15,5 +15,4 @@ QEMU_AUDIO_DRV=none \ -nodefaults \ -monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ -no-acpi \ --boot c \ --device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e +-boot c diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args index 70c759e..daecf96 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args @@ -16,42 +16,41 @@ QEMU_AUDIO_DRV=none \ -monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ -no-acpi \ -boot c \ --device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ --device ioh3420,port=0x10,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 \ --device ioh3420,port=0x18,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 \ --device ioh3420,port=0x20,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 \ --device ioh3420,port=0x28,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 \ --device ioh3420,port=0x30,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 \ --device ioh3420,port=0x38,chassis=7,id=pci.7,bus=pcie.0,addr=0x7 \ --device ioh3420,port=0x40,chassis=8,id=pci.8,bus=pcie.0,addr=0x8 \ --device ioh3420,port=0x48,chassis=9,id=pci.9,bus=pcie.0,addr=0x9 \ --device ioh3420,port=0x50,chassis=10,id=pci.10,bus=pcie.0,addr=0xa \ --device ioh3420,port=0x58,chassis=11,id=pci.11,bus=pcie.0,addr=0xb \ --device ioh3420,port=0x60,chassis=12,id=pci.12,bus=pcie.0,addr=0xc \ --device ioh3420,port=0x68,chassis=13,id=pci.13,bus=pcie.0,addr=0xd \ --device ioh3420,port=0x70,chassis=14,id=pci.14,bus=pcie.0,addr=0xe \ --device nec-usb-xhci,id=usb,bus=pci.7,addr=0x0 \ --device virtio-scsi-pci,id=scsi0,bus=pci.6,addr=0x0 \ --device virtio-serial-pci,id=virtio-serial0,bus=pci.5,addr=0x0 \ +-device ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=2,id=pci.2,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=3,id=pci.3,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=4,id=pci.4,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=5,id=pci.5,bus=pcie.0,addr=0x6 \ +-device ioh3420,port=0x38,chassis=6,id=pci.6,bus=pcie.0,addr=0x7 \ +-device ioh3420,port=0x40,chassis=7,id=pci.7,bus=pcie.0,addr=0x8 \ +-device ioh3420,port=0x48,chassis=8,id=pci.8,bus=pcie.0,addr=0x9 \ +-device ioh3420,port=0x50,chassis=9,id=pci.9,bus=pcie.0,addr=0xa \ +-device ioh3420,port=0x58,chassis=10,id=pci.10,bus=pcie.0,addr=0xb \ +-device ioh3420,port=0x60,chassis=11,id=pci.11,bus=pcie.0,addr=0xc \ +-device ioh3420,port=0x68,chassis=12,id=pci.12,bus=pcie.0,addr=0xd \ +-device ioh3420,port=0x70,chassis=13,id=pci.13,bus=pcie.0,addr=0xe \ +-device nec-usb-xhci,id=usb,bus=pci.6,addr=0x0 \ +-device virtio-scsi-pci,id=scsi0,bus=pci.5,addr=0x0 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.4,addr=0x0 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk1 \ --device virtio-blk-pci,bus=pci.8,addr=0x0,drive=drive-virtio-disk1,\ +-device virtio-blk-pci,bus=pci.7,addr=0x0,drive=drive-virtio-disk1,\ id=virtio-disk1 \ -fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/to/guest \ -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=/import/from/host,\ -bus=pci.2,addr=0x0 \ +bus=pci.1,addr=0x0 \ -netdev user,id=hostnet0 \ --device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.3,\ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.2,\ addr=0x0 \ -netdev user,id=hostnet1 \ --device e1000e,netdev=hostnet1,id=net1,mac=00:11:22:33:44:66,bus=pci.4,\ +-device e1000e,netdev=hostnet1,id=net1,mac=00:11:22:33:44:66,bus=pci.3,\ addr=0x0 \ --device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.11,\ +-device virtio-input-host-pci,id=input0,evdev=/dev/input/event1234,bus=pci.10,\ addr=0x0 \ --device virtio-mouse-pci,id=input1,bus=pci.12,addr=0x0 \ --device virtio-keyboard-pci,id=input2,bus=pci.13,addr=0x0 \ --device virtio-tablet-pci,id=input3,bus=pci.14,addr=0x0 \ +-device virtio-mouse-pci,id=input1,bus=pci.11,addr=0x0 \ +-device virtio-keyboard-pci,id=input2,bus=pci.12,addr=0x0 \ +-device virtio-tablet-pci,id=input3,bus=pci.13,addr=0x0 \ -device virtio-gpu-pci,id=video0,bus=pcie.0,addr=0x1 \ --device virtio-balloon-pci,id=balloon0,bus=pci.9,addr=0x0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.8,addr=0x0 \ -object rng-random,id=objrng0,filename=/dev/urandom \ --device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,\ -bus=pci.10,addr=0x0 +-device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,bus=pci.9,\ +addr=0x0 diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml index 80dc35e..b53ce24 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml @@ -18,10 +18,6 @@ <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> </controller> - <controller type='pci' index='1' model='dmi-to-pci-bridge'> - <model name='i82801b11-bridge'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> - </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml index b27dbe7..d784801 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml @@ -17,117 +17,113 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='vdb' bus='virtio'/> - <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> </disk> <controller type='virtio-serial' index='0'> - <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> - <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> </controller> <controller type='usb' index='0' model='nec-xhci'> - <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/> </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> </controller> <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='dmi-to-pci-bridge'> - <model name='i82801b11-bridge'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> - </controller> - <controller type='pci' index='2' model='pcie-root-port'> + <controller type='pci' index='1' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='2' port='0x10'/> + <target chassis='1' port='0x10'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> - <controller type='pci' index='3' model='pcie-root-port'> + <controller type='pci' index='2' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='3' port='0x18'/> + <target chassis='2' port='0x18'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller> - <controller type='pci' index='4' model='pcie-root-port'> + <controller type='pci' index='3' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='4' port='0x20'/> + <target chassis='3' port='0x20'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </controller> - <controller type='pci' index='5' model='pcie-root-port'> + <controller type='pci' index='4' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='5' port='0x28'/> + <target chassis='4' port='0x28'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> - <controller type='pci' index='6' model='pcie-root-port'> + <controller type='pci' index='5' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='6' port='0x30'/> + <target chassis='5' port='0x30'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </controller> - <controller type='pci' index='7' model='pcie-root-port'> + <controller type='pci' index='6' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='7' port='0x38'/> + <target chassis='6' port='0x38'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> - <controller type='pci' index='8' model='pcie-root-port'> + <controller type='pci' index='7' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='8' port='0x40'/> + <target chassis='7' port='0x40'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </controller> - <controller type='pci' index='9' model='pcie-root-port'> + <controller type='pci' index='8' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='9' port='0x48'/> + <target chassis='8' port='0x48'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> - <controller type='pci' index='10' model='pcie-root-port'> + <controller type='pci' index='9' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='10' port='0x50'/> + <target chassis='9' port='0x50'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </controller> - <controller type='pci' index='11' model='pcie-root-port'> + <controller type='pci' index='10' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='11' port='0x58'/> + <target chassis='10' port='0x58'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> </controller> - <controller type='pci' index='12' model='pcie-root-port'> + <controller type='pci' index='11' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='12' port='0x60'/> + <target chassis='11' port='0x60'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> </controller> - <controller type='pci' index='13' model='pcie-root-port'> + <controller type='pci' index='12' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='13' port='0x68'/> + <target chassis='12' port='0x68'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </controller> - <controller type='pci' index='14' model='pcie-root-port'> + <controller type='pci' index='13' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='14' port='0x70'/> + <target chassis='13' port='0x70'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> - <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </filesystem> <interface type='user'> <mac address='00:11:22:33:44:55'/> <model type='virtio'/> - <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </interface> <interface type='user'> <mac address='00:11:22:33:44:66'/> <model type='e1000e'/> - <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> </interface> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> - <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> </input> <input type='mouse' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/> </input> <input type='keyboard' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/> </input> <input type='tablet' bus='virtio'> - <address type='pci' domain='0x0000' bus='0x0e' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> </input> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> @@ -136,12 +132,12 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </video> <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/> </memballoon> <rng model='virtio'> <rate bytes='123' period='1234'/> <backend model='random'>/dev/urandom</backend> - <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/> </rng> </devices> </domain> -- 2.7.4
Previously we added a set of EHCI+UHCI controllers to Q35 machines to mimic real hardware as closely as possible, but recent discussions have pointed out that the nec-usb-xhci (USB3) controller is much more virtualization-friendly (uses less CPU), so this patch switches the default for Q35 machinetypes to add an XHCI instead (if it's supported, which it of course *will* be). Since none of the existing test cases left out USB controllers in the input XML, a new Q35 test case was added which has *no* devices, so ends up with only the defaults always put in by qemu, plus those added by libvirt. --- src/qemu/qemu_domain.c | 10 ++++-- .../qemuxml2argv-q35-default-devices-only.args | 22 ++++++++++++ .../qemuxml2argv-q35-default-devices-only.xml | 18 ++++++++++ tests/qemuxml2argvtest.c | 23 +++++++++++++ .../qemuxml2xmlout-q35-default-devices-only.xml | 40 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 23 +++++++++++++ 6 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5327add..cddb91f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2017,10 +2017,14 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addPCIeRoot = true; addImplicitSATA = true; - /* add a USB2 controller set, but only if the - * ich9-usb-ehci1 device is supported + /* Prefer adding USB3 controller if supported + * (nec-usb-xhci). Failing that, add a USB2 controller set + * if the ich9-usb-ehci1 device is supported. Otherwise + * don't add anything. */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_USB_EHCI1)) + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) + usbModel = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_USB_EHCI1)) usbModel = VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1; else addDefaultUSB = false; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args new file mode 100644 index 0000000..9ac30dd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name q35-test \ +-S \ +-M q35 \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 \ +-device ioh3420,port=0x10,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 \ +-device nec-usb-xhci,id=usb,bus=pci.1,addr=0x0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.xml new file mode 100644 index 0000000..7436583 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6d9d101..3e6537f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1822,6 +1822,29 @@ mymain(void) QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + DO_TEST("q35-default-devices-only", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root-port", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml new file mode 100644 index 0000000..8d7bc9d --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='usb' index='0' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 6186bba..ba38c88 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -764,6 +764,29 @@ mymain(void) QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + DO_TEST("q35-default-devices-only", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, -- 2.7.4
Real Q35 hardware has an ICH9 chip that includes several integrated devices at particular addresses (see the file docs/q35-chipset.cfg in the qemu source). libvirt already attempts to put the first two sets of ich9 USB2 controllers it finds at 00:1D.* and 00:1A.* to match the real hardware. This patch does the same for the ich9 "HD audio" device. The main inspiration for this patch is that currently the *only* device in a reasonable "workstation" type virtual machine config that requires a legacy PCI slot is the audio device, Without this patch, the standard Q35 machine created by virt-manager will have a dmi-to-pci-bridge and a pci-bridge just for the sound device; with the patch (and if you change the sound device model from the default "ich6" to "ich9"), the machine definition constructed by virt-manager has absolutely no legacy PCI controllers - any legacy PCI devices (e.g. video and sound) are on pcie-root as integrated devices. --- src/qemu/qemu_domain_address.c | 26 +++++ .../qemuxml2argv-q35-virt-manager-basic.args | 55 ++++++++++ .../qemuxml2argv-q35-virt-manager-basic.xml | 76 ++++++++++++++ tests/qemuxml2argvtest.c | 34 ++++++ .../qemuxml2xmlout-q35-virt-manager-basic.xml | 116 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 24 +++++ 6 files changed, 331 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e1e1f77..773e707 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1332,6 +1332,32 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, goto cleanup; } } + + memset(&tmp_addr, 0, sizeof(tmp_addr)); + tmp_addr.slot = 0x1B; + if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + /* Since real Q35 hardware has an ICH9 chip that has an + * integrated HD audio device at 0000:00:1B.0 put any + * unaddressed ICH9 audio device at that address if it's not + * already taken. If there's something already there, let the + * normal device addressing assign something later. + */ + for (i = 0; i < def->nsounds; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + + if (sound->model != VIR_DOMAIN_SOUND_MODEL_ICH9 || + !virDeviceInfoPCIAddressWanted(&sound->info)) { + continue; + } + if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + goto cleanup; + + sound->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + sound->info.addr.pci = tmp_addr; + break; + } + } + ret = 0; cleanup: VIR_FREE(addrStr); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args new file mode 100644 index 0000000..d3217d5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args @@ -0,0 +1,55 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu-system_x86_64 \ +-name virt-manager-basic \ +-S \ +-M pc-q35-2.7 \ +-m 4096 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 1b826c23-8767-47ad-a6b5-c83a88277f71 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-virt-manager-basic/monitor.sock,server,nowait \ +-rtc base=utc,driftfix=slew \ +-no-kvm-pit-reinjection \ +-global ICH9-LPC.disable_s3=1 \ +-global ICH9-LPC.disable_s4=1 \ +-boot c \ +-device ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=2,id=pci.2,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=3,id=pci.3,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=4,id=pci.4,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=5,id=pci.5,bus=pcie.0,addr=0x6 \ +-device nec-usb-xhci,id=usb,bus=pci.2,addr=0x0 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 \ +-drive file=/var/lib/libvirt/images/basic.qcow2,format=qcow2,if=none,\ +id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.4,addr=0x0,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-netdev user,id=hostnet0 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:9a:e6:c6,bus=pci.1,\ +addr=0x0 \ +-serial pty \ +-chardev socket,id=charchannel0,\ +path=/tmp/channel/domain--1-virt-manager-basic/org.qemu.guest_agent.0,server,\ +nowait \ +-device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\ +id=channel0,name=org.qemu.guest_agent.0 \ +-chardev spicevmc,id=charchannel1,name=vdagent \ +-device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\ +id=channel1,name=com.redhat.spice.0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ +-spice port=5901,tls-port=5902,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice,\ +image-compression=off \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pcie.0,\ +addr=0x1 \ +-device ich9-intel-hda,id=sound0,bus=pcie.0,addr=0x1b \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \ +-chardev spicevmc,id=charredir0,name=usbredir \ +-device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 \ +-chardev spicevmc,id=charredir1,name=usbredir \ +-device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.5,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.xml new file mode 100644 index 0000000..6d212a4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.xml @@ -0,0 +1,76 @@ +<domain type='kvm'> + <name>virt-manager-basic</name> + <uuid>1b826c23-8767-47ad-a6b5-c83a88277f71</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.7'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <clock offset='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system_x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/basic.qcow2'/> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' model='nec-xhci'/> + <controller type='sata'/> + <controller type='virtio-serial' index='0'/> + <interface type='user'> + <mac address='52:54:00:9a:e6:c6'/> + <model type='virtio'/> + </interface> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <channel type='unix'> + <target type='virtio' name='org.qemu.guest_agent.0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <input type='tablet' bus='usb'> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes'> + <listen type='address'/> + <image compression='off'/> + </graphics> + <sound model='ich9'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/> + </video> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='2'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='3'/> + </redirdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3e6537f..d7bf608 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1845,6 +1845,40 @@ mymain(void) QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + DO_TEST("q35-virt-manager-basic", + QEMU_CAPS_KVM, + QEMU_CAPS_RTC, + QEMU_CAPS_NO_KVM_PIT, + QEMU_CAPS_ICH9_DISABLE_S3, + QEMU_CAPS_ICH9_DISABLE_S4, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_DEVICE_ICH9_INTEL_HDA, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_SPICE, + QEMU_CAPS_CHARDEV_SPICEVMC, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_USB_REDIR); DO_TEST("pcie-root-port", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml new file mode 100644 index 0000000..fe9ea4e --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml @@ -0,0 +1,116 @@ +<domain type='kvm'> + <name>virt-manager-basic</name> + <uuid>1b826c23-8767-47ad-a6b5-c83a88277f71</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.7'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <clock offset='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system_x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/basic.qcow2'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + </disk> + <controller type='usb' index='0' model='nec-xhci'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='1' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x20'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='4' port='0x28'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='5' port='0x30'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <interface type='user'> + <mac address='52:54:00:9a:e6:c6'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </interface> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <channel type='unix'> + <target type='virtio' name='org.qemu.guest_agent.0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <input type='tablet' bus='usb'> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes'> + <listen type='address'/> + <image compression='off'/> + </graphics> + <sound model='ich9'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1b' function='0x0'/> + </sound> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </video> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='2'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='3'/> + </redirdev> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ba38c88..d01451e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -787,6 +787,30 @@ mymain(void) QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + DO_TEST("q35-virt-manager-basic", + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_NETDEV, + QEMU_CAPS_DEVICE_VIRTIO_NET, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_FSDEV, + QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_NEC_USB_XHCI, + QEMU_CAPS_DEVICE_ICH9_INTEL_HDA, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("pcie-root", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, -- 2.7.4
For machinetypes with a pci-root bus (all legacy PCI), libvirt will make a "fake" reservation for one extra slot prior to assigning addresses to unaddressed PCI endpoint devices in the domain. This will trigger auto-adding of a pci-bridge for the final device to be assigned an address *if that device would have otherwise instead been the last device on the last available pci-bridge*; thus it assures that there will always be at least one slot left open in the domain's bus topology for expansion (which is important both for hotplug (since a new pci-bridge can't be added while the guest is running) as well as for offline additions to the config (since adding a new device might otherwise in some cases require re-addressing existing devices, which we want to avoid)). It's important to note that for the above case (legacy PCI), we must check for the special case of all slots on all buses being occupied *prior to assigning any addresses*, and avoid attempting to reserve the extra address in that case, because there is no free address in the existing topology, so no place to auto-add a pci-bridge for expansion (i.e. it would always fail anyway). Since that condition can only be reached by manual intervention, this is acceptable. For machinetypes with pcie-root (Q35, aarch64 virt), libvirt's methodology for automatically expanding the bus topology is different - pcie-root-ports are plugged into slots (soon to be functions) of pcie-root as needed, and the new endpoint devices are assigned to the single slot in each pcie-root-port. This is done so that the devices are, by default, hotpluggable (the slots of pcie-root don't support hotplug, but the single slot of the pcie-root-port does). Since pcie-root-ports can only be plugged into pcie-root, and we don't auto-assign endpoint devices to the pcie-root slots, this means topology expansion doesn't compete with endpoint devices for slots, so we don't need to worry about checking for all "useful" slots being free *prior* to assigning addresses to new endpoint devices - as a matter of fact, if we attempt to reserve the open slots before the used slots, it can lead to errors. Instead this patch just reserves one slot for a "future potential" PCIe device after doing the assignment for actual devices, but only if the only PCI controller defined prior to starting address assignment was pcie-root, and only if we auto-added at least one PCI controller during address assignment. This assures two things: 1) that reserving the open slots will only be done when the domain is initially defined, never at any time after, and 2) that if the user understands enough about PCI controllers that they are adding them manually, that we don't mess up their plan by adding extras - if they know enough to add one pcie-root-port, or to manually assign addresses such that no pcie-root-ports are needed, they know enough to add extra pcie-root-ports if they want them (this could be called the "libguestfs clause", since libguestfs needs to be able to create domains with as few devices/controllers as possible). This is set to reserve a single free port for now, but could be increased in the future if public sentiment goes in that direction (it's easy to increase later, but essential impossible to decrease) --- src/qemu/qemu_domain_address.c | 30 ++++++++++++++++++++ .../qemuxml2argv-bios-nvram-secure.args | 1 + .../qemuxml2argv-machine-smm-opt.args | 1 + .../qemuxml2argv-q35-default-devices-only.args | 1 + .../qemuxml2argv-q35-pcie-autoadd.args | 1 + .../qemuxml2argv-q35-pm-disable-fallback.args | 1 + .../qemuxml2argv-q35-pm-disable.args | 1 + .../qemuxml2argv-q35-virt-manager-basic.args | 1 + tests/qemuxml2argvtest.c | 33 ++++++++++++++++++---- .../qemuxml2xmlout-q35-default-devices-only.xml | 5 ++++ .../qemuxml2xmlout-q35-pcie-autoadd.xml | 5 ++++ .../qemuxml2xmlout-q35-virt-manager-basic.xml | 5 ++++ tests/qemuxml2xmltest.c | 19 +++++++++++-- 13 files changed, 96 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 773e707..42af435 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1927,6 +1927,36 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup; + /* Only for *new* domains with pcie-root (and no other + * manually specified PCI controllers in the definition): If, + * after assigning addresses/reserving slots for all devices, + * we see that any extra buses have been auto-added, we + * understand that the application has left management of PCI + * addresses and controllers up to libvirt. In order to allow + * such applications to easily support hotplug, we will do a + * "one time" reservation of one extra PCIE|HOTPLUGGABLE + * slots, which should cause su to auto-add 1 extra + * pcie-root-ports The single slot in this root-port will be + * available for hotplug, or may also be used when a device is + * added to the config offline. + */ + + if (max_idx <= 0 && + addrs->nbuses > max_idx + 1 && + qemuDomainMachineHasPCIeRoot(def)) { + info.pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); + + /* if there isn't an empty pcie-root-port, this will + * cause one to be added + */ + if (qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) + goto cleanup; + } + + /* now reflect any controllers auto-added to addrs into the + * domain controllers list + */ for (i = 1; i < addrs->nbuses; i++) { virDomainDeviceDef dev; int contIndex; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args index c014254..c5fe72b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args @@ -21,6 +21,7 @@ readonly=on \ -boot c \ -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ -device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x1 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-scsi0-0-0-0 \ -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args index e49d7e9..cc9c355 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args @@ -18,6 +18,7 @@ QEMU_AUDIO_DRV=none \ -boot c \ -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ -device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x1 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-scsi0-0-0-0 \ -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args index 9ac30dd..9d13466 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-default-devices-only.args @@ -18,5 +18,6 @@ QEMU_AUDIO_DRV=none \ -boot c \ -device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 \ -device ioh3420,port=0x10,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 \ -device nec-usb-xhci,id=usb,bus=pci.1,addr=0x0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args index daecf96..ba26326 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie-autoadd.args @@ -29,6 +29,7 @@ QEMU_AUDIO_DRV=none \ -device ioh3420,port=0x60,chassis=11,id=pci.11,bus=pcie.0,addr=0xc \ -device ioh3420,port=0x68,chassis=12,id=pci.12,bus=pcie.0,addr=0xd \ -device ioh3420,port=0x70,chassis=13,id=pci.13,bus=pcie.0,addr=0xe \ +-device ioh3420,port=0x78,chassis=14,id=pci.14,bus=pcie.0,addr=0xf \ -device nec-usb-xhci,id=usb,bus=pci.6,addr=0x0 \ -device virtio-scsi-pci,id=scsi0,bus=pci.5,addr=0x0 \ -device virtio-serial-pci,id=virtio-serial0,bus=pci.4,addr=0x0 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable-fallback.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable-fallback.args index deae687..739c918 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable-fallback.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable-fallback.args @@ -20,4 +20,5 @@ QEMU_AUDIO_DRV=none \ -boot n \ -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ -device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable.args index 871340f..5ee16b7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pm-disable.args @@ -20,4 +20,5 @@ QEMU_AUDIO_DRV=none \ -boot n \ -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ -device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args index d3217d5..60af251 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-virt-manager-basic.args @@ -23,6 +23,7 @@ QEMU_AUDIO_DRV=spice \ -device ioh3420,port=0x20,chassis=3,id=pci.3,bus=pcie.0,addr=0x4 \ -device ioh3420,port=0x28,chassis=4,id=pci.4,bus=pcie.0,addr=0x5 \ -device ioh3420,port=0x30,chassis=5,id=pci.5,bus=pcie.0,addr=0x6 \ +-device ioh3420,port=0x38,chassis=6,id=pci.6,bus=pcie.0,addr=0x7 \ -device nec-usb-xhci,id=usb,bus=pci.2,addr=0x0 \ -device virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 \ -drive file=/var/lib/libvirt/images/basic.qcow2,format=qcow2,if=none,\ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d7bf608..60800c0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -656,6 +656,7 @@ mymain(void) DO_TEST("machine-smm-opt", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_SMM_OPT, @@ -673,10 +674,12 @@ mymain(void) DO_TEST("boot-floppy-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); DO_TEST("bootindex-floppy-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_BOOT_MENU, QEMU_CAPS_BOOTINDEX); DO_TEST("boot-multi", QEMU_CAPS_BOOT_MENU); @@ -723,6 +726,7 @@ mymain(void) DO_TEST("bios-nvram-secure", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_SMM_OPT, @@ -1319,18 +1323,22 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("usb-controller-default-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_NEC_USB_XHCI); DO_TEST_FAILURE("usb-controller-default-unavailable-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_NEC_USB_XHCI); DO_TEST("usb-controller-explicit-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_NEC_USB_XHCI); DO_TEST_FAILURE("usb-controller-explicit-unavailable-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PIIX3_USB_UHCI); DO_TEST("usb-controller-xhci", @@ -1702,10 +1710,12 @@ mymain(void) DO_TEST("pcie-root", QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420); DO_TEST("q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, @@ -1720,16 +1730,19 @@ mymain(void) QEMU_CAPS_DEVICE_IOH3420); DO_TEST("q35-pm-disable", QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4, QEMU_CAPS_ICH9_DISABLE_S3, QEMU_CAPS_ICH9_DISABLE_S4); DO_TEST("q35-pm-disable-fallback", QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4); DO_TEST("q35-usb2", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, @@ -1737,6 +1750,7 @@ mymain(void) DO_TEST("q35-usb2-multi", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, @@ -1744,6 +1758,7 @@ mymain(void) DO_TEST("q35-usb2-reorder", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, @@ -1906,6 +1921,7 @@ mymain(void) DO_TEST_PARSE_ERROR("q35-wrong-root", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, @@ -2019,6 +2035,7 @@ mymain(void) DO_TEST("pcihole64-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL, @@ -2051,13 +2068,15 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420); DO_TEST("aarch64-virt-2.6-virtio-pci-default", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420); /* Example of using virtio-pci with no explicit PCI controller but with manual PCI addresses */ DO_TEST("aarch64-virtio-pci-manual-addresses", @@ -2065,7 +2084,9 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_VIRTIO_SCSI); DO_TEST("aarch64-video-virtio-gpu-pci", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420, @@ -2321,7 +2342,9 @@ mymain(void) DO_TEST("acpi-table", NONE); DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("intel-iommu-machine", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_MACHINE_IOMMU); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml index 8d7bc9d..e64b80c 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-default-devices-only.xml @@ -31,6 +31,11 @@ <target chassis='2' port='0x10'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='virtio'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml index d784801..3742e14 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie-autoadd.xml @@ -97,6 +97,11 @@ <target chassis='13' port='0x70'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </controller> + <controller type='pci' index='14' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='14' port='0x78'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0f' function='0x0'/> + </controller> <filesystem type='mount' accessmode='passthrough'> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml index fe9ea4e..236d955 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-virt-manager-basic.xml @@ -68,6 +68,11 @@ <target chassis='5' port='0x30'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='6' port='0x38'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </controller> <interface type='user'> <mac address='52:54:00:9a:e6:c6'/> <model type='virtio'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d01451e..0cd1044 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -593,10 +593,12 @@ mymain(void) QEMU_CAPS_PIIX3_USB_UHCI); DO_TEST("usb-controller-default-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_NEC_USB_XHCI); DO_TEST("usb-controller-explicit-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_NEC_USB_XHCI); DO_TEST("ppc64-usb-controller", @@ -675,23 +677,27 @@ mymain(void) DO_TEST("q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); DO_TEST("q35-usb2", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); DO_TEST("q35-usb2-multi", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); DO_TEST("q35-usb2-reorder", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); @@ -905,6 +911,7 @@ mymain(void) DO_TEST("pcihole64-none", NONE); DO_TEST("pcihole64-q35", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); @@ -948,13 +955,17 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_VIRTIO_SCSI); DO_TEST("aarch64-virtio-pci-manual-addresses", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, - QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI); + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_VIRTIO_SCSI); DO_TEST("aarch64-video-virtio-gpu-pci", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420, @@ -1002,7 +1013,9 @@ mymain(void) DO_TEST("video-virtio-gpu-secondary", NONE); DO_TEST("intel-iommu", - QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE); + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420); qemuTestDriverFree(&driver); -- 2.7.4
participants (2)
-
Andrea Bolognani -
Laine Stump