This new subelement is used in PCI controllers: the toplevel *attribute* "model" of a controller denotes what kind of PCI controller is being described, e.g. a "dmi-to-pci-bridge", "pci-bridge", or "pci-root". But in the future there will be different implementations of some of those types of PCI controllers, which behave similarly from libvirt's point of view (and so should have the same model), but use a different device in qemu (and present themselves as a different piece of hardware in the guest). In an ideal world we (i.e. "I") would have thought of that back when the pci controllers were added, and used some sort of type/class/model notation (where class was used in the way we are now using model, and model was used for the actual manufacturer's model number of a particular family of PCI controller), but that opportunity is long past, so as an alternative, this patch allows selecting a particular implementation of a pci controller with the "name" attribute of the <model> subelement, e.g.: <controller type='pci' model='dmi-to-pci-bridge' index='1'> <model name='i82801b11-bridge'/> </controller> In this case, "dmi-to-pci-bridge" is the kind of controller (one that has a single PCIe port upstream, and 32 standard PCI ports downstream, which are not hotpluggable), and the qemu device to be used to implement this kind of controller is named "i82801b11-bridge". Implementing the above now will allow us in the future to add a new kind of dmi-to-pci-bridge that doesn't use qemu's i82801b11-bridge device, but instead uses something else (which doesn't yet exist, but qemu people have been discussing it), all without breaking existing configs. (note that for the existing "pci-bridge" type of PCI controller, both the model attribute and <model> name are 'pci-bridge'. This is just a coincidence, since it turns out that in this case the device name in qemu really is a generic 'pci-bridge' rather than being the name of some real-world chip) --- change from V2: * attribute is now called "name" instead of "type" * different possible model names are stored internally as an enum value rather than a string. * more than one occurence of <model> in a single controller is now considered an error * docs say "1.2.18" instead of "1.3.0" docs/formatdomain.html.in | 13 +++++++ docs/schemas/domaincommon.rng | 13 +++++++ src/conf/domain_conf.c | 46 +++++++++++++++++++++++-- src/conf/domain_conf.h | 16 +++++++++ src/libvirt_private.syms | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 8 +++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 8 +++-- 7 files changed, 100 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d0c1741..9abceee 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3042,6 +3042,19 @@ (set to 0). <span class="since">Since 1.1.2 (QEMU only)</span> </p> <p> + PCI controllers also have an optional + subelement <code>&lt;model&gt;</code> with an attribute + <code>name</code>. The name attribute holds the name of the + specific device that qemu is emulating (e.g. "i82801b11-bridge") + rather than simply the class of device ("dmi-to-pci-bridge", + "pci-bridge"), which is set in the controller element's + model <b>attribute</b>. In almost all cases, you should not + manually add a <code>&lt;model&gt;</code> subelement to a + controller, nor should you modify one that is automatically + generated by libvirt. <span class="since">Since 1.2.18 (QEMU + only).</span> + </p> + <p> For machine types which provide an implicit PCI bus, the pci-root controller with index=0 is auto-added and required to use PCI devices. pci-root has no address. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1120003..a4c1c9b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1731,6 +1731,19 @@ <attribute name="type"> <value>pci</value> </attribute> + <optional> + <element name="model"> + <attribute name="name"> + <choice> + <!-- implementations of 'pci-bridge' --> + <value>pci-bridge</value> + <!-- implementations of 'dmi-to-pci-bridge' --> + <value>i82801b11-bridge</value> + </choice> + </attribute> + <empty/> + </element> + </optional> <!-- *-root controllers have an optional element "pcihole64"--> <choice> <group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6b557d1..d58e679 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -326,6 +326,12 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, "pci-bridge", "dmi-to-pci-bridge") +VIR_ENUM_IMPL(virDomainControllerPCIModelName, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST, + "none", + "pci-bridge", + "i82801b11-bridge") + VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", "buslogic", @@ -7766,6 +7772,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *queues = NULL; char *cmd_per_lun = NULL; char *max_sectors = NULL; + bool processedModel = false; + char *modelName = NULL; xmlNodePtr saved = ctxt->node; int rc; @@ -7809,6 +7817,15 @@ virDomainControllerDefParseXML(xmlNodePtr node, queues = virXMLPropString(cur, "queues"); cmd_per_lun = virXMLPropString(cur, "cmd_per_lun"); max_sectors = virXMLPropString(cur, "max_sectors"); + } else if (xmlStrEqual(cur->name, BAD_CAST "model")) { + if (processedModel) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Multiple <model> elements in " + "controller definition not allowed")); + goto error; + } + modelName = virXMLPropString(cur, "name"); + processedModel = true; } } cur = cur->next; @@ -7917,6 +7934,15 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->opts.pciopts.pcihole64size = VIR_DIV_UP(bytes, 1024); } } + if (modelName && + (def->opts.pciopts.modelName + = virDomainControllerPCIModelNameTypeFromString(modelName)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown PCI controller model name '%s'"), + modelName); + goto error; + } + break; default: break; @@ -7941,6 +7967,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(queues); VIR_FREE(cmd_per_lun); VIR_FREE(max_sectors); + VIR_FREE(modelName); return def; @@ -18950,7 +18977,8 @@ virDomainControllerDefFormat(virBufferPtr buf, { const char *type = virDomainControllerTypeToString(def->type); const char *model = NULL; - bool pcihole64 = false; + const char *modelName = NULL; + bool pcihole64 = false, pciModel = false; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -18990,17 +19018,31 @@ virDomainControllerDefFormat(virBufferPtr buf, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: if (def->opts.pciopts.pcihole64) pcihole64 = true; + if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) + pciModel = true; break; default: break; } - if (def->queues || def->cmd_per_lun || def->max_sectors || + if (pciModel || + def->queues || def->cmd_per_lun || def->max_sectors || virDomainDeviceInfoNeedsFormat(&def->info, flags) || pcihole64) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); + if (pciModel) { + modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected model name value %d"), + def->opts.pciopts.modelName); + return -1; + } + virBufferAsprintf(buf, "<model name='%s'/>\n", modelName); + } + if (def->queues || def->cmd_per_lun || def->max_sectors) { virBufferAddLit(buf, "<driver"); if (def->queues) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0fe6b1a..fd65792 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -757,6 +757,14 @@ typedef enum { } virDomainControllerModelPCI; typedef enum { + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE, + + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST +} virDomainControllerPCIModelName; + +typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC, @@ -797,6 +805,13 @@ typedef virDomainPCIControllerOpts *virDomainPCIControllerOptsPtr; struct _virDomainPCIControllerOpts { bool pcihole64; unsigned long pcihole64size; + + /* the exact controller name is in the "model" subelement, e.g.: + * <controller type='pci' model='pcie-root-port'> + * <model name='ioh3420''/> + * ... + */ + int modelName; /* the exact name of the device in hypervisor */ }; /* Stores the virtual disk controller configuration */ @@ -2978,6 +2993,7 @@ VIR_ENUM_DECL(virDomainDiskDiscard) VIR_ENUM_DECL(virDomainDiskMirrorState) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModelPCI) +VIR_ENUM_DECL(virDomainControllerPCIModelName) VIR_ENUM_DECL(virDomainControllerModelSCSI) VIR_ENUM_DECL(virDomainControllerModelUSB) VIR_ENUM_DECL(virDomainFS) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ff322d6..37323ad 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -192,6 +192,8 @@ virDomainControllerModelSCSITypeFromString; virDomainControllerModelSCSITypeToString; virDomainControllerModelUSBTypeFromString; virDomainControllerModelUSBTypeToString; +virDomainControllerPCIModelNameTypeFromString; +virDomainControllerPCIModelNameTypeToString; virDomainControllerRemove; virDomainControllerTypeToString; virDomainCpuPlacementModeTypeFromString; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml index 05967a4..132c15f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml @@ -20,8 +20,12 @@ <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='dmi-to-pci-bridge'/> - <controller type='pci' index='2' model='pci-bridge'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + </controller> <video> <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> </video> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml index 9dd4162..4d8fc5f 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml @@ -20,8 +20,12 @@ <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='dmi-to-pci-bridge'/> - <controller type='pci' index='2' model='pci-bridge'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + </controller> <controller type='sata' index='0'/> <video> <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> -- 2.1.0

On 08/03/2015 05:55 AM, Martin Kletzander wrote: If it was by itself, yes. (As a matter of fact that's how I original added the "don't allow multiples" to the patch). But as you found out in patch 3, similar checking is needed for <target>, and target has multiple attributes; rather than checking for "chassis || chassisNr || port" there, I gave that one a single bool that gets set, so I came back and made this one the same to be consistent - the more similar code looks, the easier it is to follow the logic (and to potentially make a utility function or combine code in some other way in the future). Besides, I like to pretend that I'm consistent :-)

This patch provides qemu support for the contents of <model> in <controller> for the two existing PCI controller types that need it (i.e. the two controller types that are backed by a device that must be specified on the qemu commandline): 1) pci-bridge - sets <model> name attribute default as "pci-bridge" 2) dmi-to-pci-bridge - sets <model> name attribute default as "i82801b11-bridge". These both match current hardcoded practice. The defaults are set at the end of qemuDomainAssignPCIAddresses(), It can't be done earlier because some of the options that will be autogenerated need full PCI address info for the controller and because qemuDomainAssignPCIAddresses() might create extra controllers which would need default settings added, and that hasn't been done at the time the PostParse callbacks are being run. qemuDomainAssignPCIAddresses() is still prior to the XML being written to disk, though, so the autogenerated defaults are persistent. qemu capabilities bits aren't checked until the commandline is actually created (so the domain can possibly be defined on a host that doesn't yet have support for the give n device, or a host different from the one where it will eventually be run). At that time we compare the type strings to known qemu device names and check for the capabilities bit for that device. --- Changes from V2: use enum instead of string model name. src/qemu/qemu_command.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 09f30c4..6a19d15 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2251,11 +2251,33 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainControllerDefPtr cont = def->controllers[i]; int idx = cont->idx; virDevicePCIAddressPtr addr; + virDomainPCIControllerOptsPtr options; if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) continue; addr = &cont->info.addr.pci; + options = &cont->opts.pciopts; + + /* set defaults for any other auto-generated config + * options for this controller that haven't been + * specified in config. + */ + switch ((virDomainControllerModelPCI)cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) + options->modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) + options->modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + /* check if every PCI bridge controller's ID is greater than * the bus it is placed onto */ @@ -4505,6 +4527,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, { virBuffer buf = VIR_BUFFER_INITIALIZER; int model = def->model; + const char *modelName = NULL; if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { if ((qemuSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) @@ -4626,17 +4649,67 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } switch (def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=%s", - def->idx, def->info.alias); + if (def->opts.pciopts.modelName + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pci-bridge options not set")); + goto error; + } + + modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown pci-bridge model name value %d"), + def->opts.pciopts.modelName); + goto error; + } + if (def->opts.pciopts.modelName + != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' " + "is not valid for a pci-bridge"), + modelName); + goto error; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pci-bridge controller " + "is not supported in this QEMU binary")); + goto error; + } + virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s", + modelName, def->idx, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + if (def->opts.pciopts.modelName + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated dmi-to-pci-bridge options not set")); + goto error; + } + + modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown dmi-to-pci-bridge model name value %d"), + def->opts.pciopts.modelName); + goto error; + } + if (def->opts.pciopts.modelName + != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' " + "is not valid for a dmi-to-pci-bridge"), + modelName); + goto error; + } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("The dmi-to-pci-bridge (i82801b11-bridge) " + _("the dmi-to-pci-bridge (i82801b11-bridge) " "controller is not supported in this QEMU binary")); goto error; } - virBufferAsprintf(&buf, "i82801b11-bridge,id=%s", def->info.alias); + virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; } break; -- 2.1.0

On 08/03/2015 06:05 AM, Martin Kletzander wrote: Checking whether the particular modelName given in the XML (or auto-assigned later) is supported by this QEMU isn't something that's done by the parser or post-parse callbacks. It's done while we're building the commandline because it's then that we have accurate and up-to-date capability info. (Actually I just looked and up until just a few weeks ago, qemuCaps wasn't used anywhere in the post-parse callbacks; just recently code was added to the domain post-parse callback to retrieve the qemuCaps and use it when deciding whether to add an implicit pcie-root controller to an ARM virtual machine. This is done because apparently you need to check the capabilities to see if the machine has a pcie-root, but that seems troublesome to me, because a machine that has a pcie-root and one that don't should *not* have the same machinetype name in my opinion (that is a *very* serious difference in ABI). Also, I wonder if this ARM "virt-*" machinetype really supports the i82801b11-bridge controller (since it is modelled on a controller that in the real world is only available integrated into an Intel x86 chipset). The things you find while cscoping around at midnight on Saturday...) not in the code as it exists now - PCI addresses are assigned by a separate function that gets called after return from the parser (and post parse callbacks), but every place that calls the parser also calls the function that assigns PCI addresses. See your previous sentence. Checking for an empty modelName is a sanity check. I like to be careful, especially when there is separation in space and time between when the value should have been set and when it is used. Today it's true that it's always set, but if someone unknowingly changes the code over in "the other place" some day, I'd rather get a clear error about it than a segv. I missed that suggestion, but would welcome the consolidation. However we still have the situation that each modelName only works with one particular model, so it would take some extra thought to get the right amount of the duplicated code into the common function and in the right order (so I'll just leave it as is for now.

There are some configuration options to some types of pci controllers that are currently automatically derived from other parts of the controller's configuration. For example, in qemu a pci-bridge controller has an option that is called "chassis_nr"; up until now libvirt has always set chassis_nr to the index of the pci-bridge. So this: <controller type='pci' model='pci-bridge' index='2'/> will always result in: -device pci-bridge,chassis_nr=2,... on the qemu commandline. In the future we may decide there is a better way to derive that option, but even in that case we will need for existing domains to retain the same chassis_nr they were using in the past - that is something that is visible to the guest so it is part of the guest ABI and changing it would lead to problems for migrating guests (or just guests with very picky OSes). The <target> subelement has been added as a place to put the new "chassisNr" attribute that will be filled in by libvirt when it auto-generates the chassisNr; it will be saved in the config, then reused any time the domain is started: <controller type='pci' model='pci-bridge' index='2'> <model type='pci-bridge'/> <target chassisNr='2'/> </controller> The one oddity of all this is that if the controller configuration is changed (for example to change the index or the pci address where the controller is plugged in), the items in <target> will *not* be re-generated, which might lead to conflict. I can't really see any way around this, but fortunately if there is a material conflict qemu will let us know and we will pass that on to the user. --- changes from V2: * check chassisNr for 0-255 range * multiple <target>s in a single controller is now an error * 1.3.0 -> 1.2.18 docs/formatdomain.html.in | 24 +++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 45 +++++++++++++++++++++++-- src/conf/domain_conf.h | 10 +++++- tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 1 + tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 1 + 6 files changed, 88 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9abceee..fdf7e82 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3055,6 +3055,30 @@ only).</span> </p> <p> + PCI controllers also have an optional + subelement <code>&lt;target&gt;</code> with the attributes + listed below. These are configurable items that 1) are visible + to the guest OS so must be preserved for guest ABI + compatibility, and 2) are usually left to default values or + derived automatically by libvirt. In almost all cases, you + should not manually add a <code>&lt;target&gt;</code> subelement + to a controller, nor should you modify the values in the those + that are automatically generated by + libvirt. <span class="since">Since 1.2.18 (QEMU only).</span> + </p> + <dl> + <dt><code>chassisNr</code></dt> + <dd> + PCI controllers that have attribute model="pci-bridge", can + also have a <code>chassisNr</code> attribute in + the <code>&lt;target&gt;</code> subelement, which is used to + control QEMU's "chassis_nr" option for the pci-bridge device + (normally libvirt automatically sets this to the same value as + the index attribute of the pci controller). If set, chassisNr + must be between 0 and 255. + </dd> + </dl> + <p> For machine types which provide an implicit PCI bus, the pci-root controller with index=0 is auto-added and required to use PCI devices. pci-root has no address. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a4c1c9b..a61e209 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1744,6 +1744,16 @@ <empty/> </element> </optional> + <optional> + <element name="target"> + <optional> + <attribute name='chassisNr'> + <ref name='uint8range'/> + </attribute> + </optional> + <empty/> + </element> + </optional> <!-- *-root controllers have an optional element "pcihole64"--> <choice> <group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d58e679..fbad7e9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1551,6 +1551,8 @@ virDomainControllerDefNew(virDomainControllerType type) def->opts.vioserial.vectors = -1; break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + def->opts.pciopts.chassisNr = -1; + break; case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: @@ -7774,6 +7776,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *max_sectors = NULL; bool processedModel = false; char *modelName = NULL; + bool processedTarget = false; + char *chassisNr = NULL; xmlNodePtr saved = ctxt->node; int rc; @@ -7826,6 +7830,15 @@ virDomainControllerDefParseXML(xmlNodePtr node, } modelName = virXMLPropString(cur, "name"); processedModel = true; + } else if (xmlStrEqual(cur->name, BAD_CAST "target")) { + if (processedTarget) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Multiple <target> elements in " + "controller definition not allowed")); + goto error; + } + chassisNr = virXMLPropString(cur, "chassisNr"); + processedTarget = true; } } cur = cur->next; @@ -7942,6 +7955,23 @@ virDomainControllerDefParseXML(xmlNodePtr node, modelName); goto error; } + if (chassisNr) { + if (virStrToLong_i(chassisNr, NULL, 0, + &def->opts.pciopts.chassisNr) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid chassisNr '%s' in PCI controller"), + chassisNr); + goto error; + } + if (def->opts.pciopts.chassisNr < 0 || + def->opts.pciopts.chassisNr > 255) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller chassisNr '%s' out of range " + "- must be 0-255"), + chassisNr); + goto error; + } + } break; default: @@ -7968,6 +7998,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(cmd_per_lun); VIR_FREE(max_sectors); VIR_FREE(modelName); + VIR_FREE(chassisNr); return def; @@ -18978,7 +19009,7 @@ virDomainControllerDefFormat(virBufferPtr buf, const char *type = virDomainControllerTypeToString(def->type); const char *model = NULL; const char *modelName = NULL; - bool pcihole64 = false, pciModel = false; + bool pcihole64 = false, pciModel = false, pciTarget = false; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -19020,13 +19051,15 @@ virDomainControllerDefFormat(virBufferPtr buf, pcihole64 = true; if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) pciModel = true; + if (def->opts.pciopts.chassisNr != -1) + pciTarget = true; break; default: break; } - if (pciModel || + if (pciModel || pciTarget || def->queues || def->cmd_per_lun || def->max_sectors || virDomainDeviceInfoNeedsFormat(&def->info, flags) || pcihole64) { virBufferAddLit(buf, ">\n"); @@ -19043,6 +19076,14 @@ virDomainControllerDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<model name='%s'/>\n", modelName); } + if (pciTarget) { + virBufferAddLit(buf, "<target"); + if (def->opts.pciopts.chassisNr != -1) + virBufferAsprintf(buf, " chassisNr='%d'", + def->opts.pciopts.chassisNr); + virBufferAddLit(buf, "/>\n"); + } + if (def->queues || def->cmd_per_lun || def->max_sectors) { virBufferAddLit(buf, "<driver"); if (def->queues) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fd65792..761ce8b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -812,7 +812,15 @@ struct _virDomainPCIControllerOpts { * ... */ int modelName; /* the exact name of the device in hypervisor */ -}; + + /* the following items are attributes of the "target" subelement + * of controller type='pci'. They are bits of configuration that + * are specified on the qemu commandline and are visible to the + * guest OS, so they must be preserved to ensure ABI + * compatibility. + */ + int chassisNr; /* used by pci-bridge, -1 == unspecified */ + }; /* Stores the virtual disk controller configuration */ struct _virDomainControllerDef { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml index 132c15f..0c3da85 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml @@ -25,6 +25,7 @@ </controller> <controller type='pci' index='2' model='pci-bridge'> <model name='pci-bridge'/> + <target chassisNr='56'/> </controller> <video> <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml index 4d8fc5f..abb3a0f 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml @@ -25,6 +25,7 @@ </controller> <controller type='pci' index='2' model='pci-bridge'> <model name='pci-bridge'/> + <target chassisNr='56'/> </controller> <controller type='sata' index='0'/> <video> -- 2.1.0

This uses the new subelement/attribute in two ways: 1) If a "pci-bridge" pci controller has no chassisNr attribute, it will automatically be set to the controller's index as soon as the controller's PCI address is known (during qemuDomainAssignPCIAddresses()). 2) when creating the commandline for a pci-bridge device, chassisNr will be used to set qemu's chassis_nr option (rather than the previous practice of hard-coding it to the controller's index). --- changes from V2: readjust for rebase on earlier patches' changes src/qemu/qemu_command.c | 8 ++++++-- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a19d15..078c365 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2267,6 +2267,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) options->modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; + if (options->chassisNr == -1) + options->chassisNr = cont->idx; break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) @@ -4650,7 +4652,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, switch (def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: if (def->opts.pciopts.modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || + def->opts.pciopts.chassisNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-bridge options not set")); goto error; @@ -4678,7 +4681,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, goto error; } virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s", - modelName, def->idx, def->info.alias); + modelName, def->opts.pciopts.chassisNr, + def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: if (def->opts.pciopts.modelName diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args index 888aa6b..e42022d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args @@ -2,7 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,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=0x1 \ +-device pci-bridge,chassis_nr=56,id=pci.2,bus=pci.1,addr=0x1 \ -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ -device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ -vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 -- 2.1.0

This controller can be connected (at domain startup time only - not hotpluggable) only to a port on the pcie root complex ("pcie-root" in libvirt config), hence the new connect type VIR_PCI_CONNECT_TYPE_PCIE_ROOT. It provides a hotpluggable port that will accept any PCI or PCIe device. New attributes must be added to the controller <target> subelement for this - chassis and port are guest-visible option values that will be set by libvirt with values derived from the controller's index and pci address information. --- change from V2: * check chassis/port for 0-255 range * 1.3.0 -> 1.2.18 docs/formatdomain.html.in | 33 +++++++++- docs/schemas/domaincommon.rng | 13 ++++ src/conf/domain_addr.c | 10 ++- src/conf/domain_addr.h | 5 +- src/conf/domain_conf.c | 75 +++++++++++++++++++++- src/conf/domain_conf.h | 8 ++- src/qemu/qemu_command.c | 1 + .../qemuxml2argv-pcie-root-port.xml | 36 +++++++++++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fdf7e82..a9db924 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3029,10 +3029,11 @@ <p> PCI controllers have an optional <code>model</code> attribute with possible values <code>pci-root</code>, <code>pcie-root</code>, - <code>pci-bridge</code>, or <code>dmi-to-pci-bridge</code>. + <code>pcie-root-port</code>, <code>pci-bridge</code>, + or <code>dmi-to-pci-bridge</code>. (pci-root and pci-bridge <span class="since">since 1.0.5</span>, pcie-root and dmi-to-pci-bridge <span class="since">since - 1.1.2</span>) + 1.1.2</span>, pcie-root-port <span class="since">since 1.2.18</span>) The root controllers (<code>pci-root</code> and <code>pcie-root</code>) have an optional <code>pcihole64</code> element specifying how big (in kilobytes, or in the unit specified by <code>pcihole64</code>'s @@ -3077,6 +3078,23 @@ the index attribute of the pci controller). If set, chassisNr must be between 0 and 255. </dd> + <dt><code>chassis</code></dt> + <dd> + pcie-root-port controllers can also have + a <code>chassis</code> attribute in + the <code>&lt;target&gt;</code> subelement, which is used to + set the controller's "chassis" configuration value, which is + visible to the virtual machine. If set, chassis must be + between 0 and 255. + </dd> + <dt><code>port</code></dt> + <dd> + pcie-root-port controllers can also have a <code>port</code> + attribute in the <code>&lt;target&gt;</code> subelement, which + is used to set the controller's "port" configuration value, + which is visible to the virtual machine. If set, port must be + between 0 and 255. + </dd> </dl> <p> For machine types which provide an implicit PCI bus, the pci-root @@ -3123,6 +3141,17 @@ auto-determined by libvirt will be placed on this pci-bridge device. (<span class="since">since 1.1.2</span>). </p> + <p> + Domains with an implicit pcie-root can also add controllers + with <code>model='pcie-root-port'</code>. This is a simple type of + bridge device that can connect only to one of the 31 slots on + the pcie-root bus on the upstream side, and makes a single + (PCIe, hotpluggable) port (at slot='0') available on the + downstream side. This controller can be used to provide a single + slot to later hotplug a PCIe device (but is not itself + hotpluggable - it must be in the configuration when the domain + is started). (<span class="since">since 1.2.18</span>) + </p> <pre> ... &lt;devices&gt; diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a61e209..7d7f412 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1739,6 +1739,8 @@ <value>pci-bridge</value> <!-- implementations of 'dmi-to-pci-bridge' --> <value>i82801b11-bridge</value> + <!-- implementations of 'pcie-root-port' --> + <value>ioh3420</value> </choice> </attribute> <empty/> @@ -1751,6 +1753,16 @@ <ref name='uint8range'/> </attribute> </optional> + <optional> + <attribute name="chassis"> + <ref name='uint8range'/> + </attribute> + </optional> + <optional> + <attribute name="port"> + <ref name='uint8range'/> + </attribute> + </optional> <empty/> </element> </optional> @@ -1774,6 +1786,7 @@ <choice> <value>pci-bridge</value> <value>dmi-to-pci-bridge</value> + <value>pcie-root-port</value> </choice> </attribute> </group> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 9519f79..c2a207b 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -183,9 +183,9 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: /* slots 1 - 31, no hotplug, PCIe only unless the address was * specified in user config *and* the particular device being - * attached also allows it + * attached also allows it. */ - bus->flags = VIR_PCI_CONNECT_TYPE_PCIE; + bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_TYPE_PCIE_ROOT; bus->minSlot = 1; bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; @@ -196,6 +196,12 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, bus->minSlot = 1; bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + /* provides one slot which is pcie and hotpluggable */ + bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE; + bus->minSlot = 0; + bus->maxSlot = 0; + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid PCI controller model %d"), model); diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index dcfd2e5..2a0ff96 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -39,6 +39,8 @@ typedef enum { /* PCI devices can connect to this bus */ VIR_PCI_CONNECT_TYPE_PCIE = 1 << 3, /* PCI Express devices can connect to this bus */ + VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 << 4, + /* for devices that can only connect to pcie-root (i.e. root-port) */ } virDomainPCIConnectFlags; typedef struct { @@ -70,7 +72,8 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; * allowed, e.g. PCI, PCIe, switch */ # define VIR_PCI_CONNECT_TYPES_MASK \ - (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE) + (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE | \ + VIR_PCI_CONNECT_TYPE_PCIE_ROOT) /* combination of all bits that could be used to connect a normal * endpoint device (i.e. excluding the connection possible between an diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fbad7e9..f1723c0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -324,13 +324,15 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, "pci-root", "pcie-root", "pci-bridge", - "dmi-to-pci-bridge") + "dmi-to-pci-bridge", + "pcie-root-port") VIR_ENUM_IMPL(virDomainControllerPCIModelName, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST, "none", "pci-bridge", - "i82801b11-bridge") + "i82801b11-bridge", + "ioh3420") VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", @@ -1552,6 +1554,8 @@ virDomainControllerDefNew(virDomainControllerType type) break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: def->opts.pciopts.chassisNr = -1; + def->opts.pciopts.chassis = -1; + def->opts.pciopts.port = -1; break; case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: @@ -7778,6 +7782,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *modelName = NULL; bool processedTarget = false; char *chassisNr = NULL; + char *chassis = NULL; + char *port = NULL; xmlNodePtr saved = ctxt->node; int rc; @@ -7838,6 +7844,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; } chassisNr = virXMLPropString(cur, "chassisNr"); + chassis = virXMLPropString(cur, "chassis"); + port = virXMLPropString(cur, "port"); processedTarget = true; } } @@ -7972,6 +7980,57 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; } } + if (chassis) { + if (virStrToLong_i(chassis, NULL, 0, + &def->opts.pciopts.chassis) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid chassis '%s' in PCI controller"), + chassis); + goto error; + } + if (def->opts.pciopts.chassis < 0 || + def->opts.pciopts.chassisNr > 255) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller chassis '%s' out of range " + "- must be 0-255"), + chassis); + goto error; + } + } + if (chassis) { + if (virStrToLong_i(chassis, NULL, 0, + &def->opts.pciopts.chassis) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid chassis '%s' in PCI controller"), + chassis); + goto error; + } + if (def->opts.pciopts.chassis < 0 || + def->opts.pciopts.chassis > 255) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller chassis '%s' out of range " + "- must be 0-255"), + chassis); + goto error; + } + } + if (port) { + if (virStrToLong_i(port, NULL, 0, + &def->opts.pciopts.port) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid port '%s' in PCI controller"), + port); + goto error; + } + if (def->opts.pciopts.port < 0 || + def->opts.pciopts.port > 255) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller port '%s' out of range " + "- must be 0-255"), + port); + goto error; + } + } break; default: @@ -7999,6 +8058,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(max_sectors); VIR_FREE(modelName); VIR_FREE(chassisNr); + VIR_FREE(chassis); + VIR_FREE(port); return def; @@ -19051,7 +19112,9 @@ virDomainControllerDefFormat(virBufferPtr buf, pcihole64 = true; if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) pciModel = true; - if (def->opts.pciopts.chassisNr != -1) + if (def->opts.pciopts.chassisNr != -1 || + def->opts.pciopts.chassis != -1 || + def->opts.pciopts.port != -1) pciTarget = true; break; @@ -19081,6 +19144,12 @@ virDomainControllerDefFormat(virBufferPtr buf, if (def->opts.pciopts.chassisNr != -1) virBufferAsprintf(buf, " chassisNr='%d'", def->opts.pciopts.chassisNr); + if (def->opts.pciopts.chassis != -1) + virBufferAsprintf(buf, " chassis='%d'", + def->opts.pciopts.chassis); + if (def->opts.pciopts.port != -1) + virBufferAsprintf(buf, " port='0x%x'", + def->opts.pciopts.port); virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 761ce8b..614a2a6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -752,6 +752,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT, VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE, VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE, + VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST } virDomainControllerModelPCI; @@ -760,6 +761,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; @@ -820,7 +822,11 @@ struct _virDomainPCIControllerOpts { * compatibility. */ int chassisNr; /* used by pci-bridge, -1 == unspecified */ - }; + /* chassis & port used by + * pcie-root-port/pcie-switch-downstream-port, -1 = unspecified */ + int chassis; + int port; +}; /* Stores the virtual disk controller configuration */ struct _virDomainControllerDef { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 078c365..7f58f97 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2274,6 +2274,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) options->modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml new file mode 100644 index 0000000..bc28a71 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml @@ -0,0 +1,36 @@ +<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='sda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'/> + <controller type='pci' index='2' model='pci-bridge'/> + <controller type='pci' index='3' model='pcie-root-port'/> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='40' port='0x1a'/> + </controller> + <controller type='sata' index='0'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7a41a18..6b48bf4 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -567,6 +567,7 @@ mymain(void) DO_TEST_DIFFERENT("pci-autoadd-idx"); DO_TEST_DIFFERENT("pcie-root"); DO_TEST_DIFFERENT("q35"); + DO_TEST("pcie-root-port"); DO_TEST("hostdev-scsi-lsi"); DO_TEST("hostdev-scsi-virtio-scsi"); -- 2.1.0

On Mon, Aug 03, 2015 at 12:17:45PM +0200, Martin Kletzander wrote: Well, John has a favourite, though. And it makes sense as well... The upper one is comparing chassisNr > 255 but it should be 'chassis' instead, I totally missed that. Thanks John!

This is the upstream part of a PCIe switch. It connects to a PCIe port (but not PCI) on the upstream side, and can have up to 31 xio3130-downstream controllers (but no other types of devices) connected to its downstream side. This device will be used to implement the "pcie-switch" model of pci controller. --- change for V2: rebase src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 6 ++++-- 10 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a9dc395..c2ec46d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -289,6 +289,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vhost-user-multiqueue", /* 190 */ "migration-event", "ioh3420", + "x3130-upstream", ); @@ -1570,6 +1571,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "pc-dimm", QEMU_CAPS_DEVICE_PC_DIMM }, { "pci-serial", QEMU_CAPS_DEVICE_PCI_SERIAL }, { "ioh3420", QEMU_CAPS_DEVICE_IOH3420 }, + { "x3130-upstream", QEMU_CAPS_DEVICE_X3130_UPSTREAM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f179e0b..e677065 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -232,6 +232,7 @@ typedef enum { QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ QEMU_CAPS_MIGRATION_EVENT = 191, /* MIGRATION event */ QEMU_CAPS_DEVICE_IOH3420 = 192, /* -device ioh3420 */ + QEMU_CAPS_DEVICE_X3130_UPSTREAM = 193, /* -device x3130-upstream */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index a1fafa6..78d7b82 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -121,4 +121,5 @@ <flag name='qxl.vgamem_mb'/> <flag name='qxl-vga.vgamem_mb'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index 824ef02..7cec7f9 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -136,4 +136,5 @@ <flag name='qxl-vga.vgamem_mb'/> <flag name='pci-serial'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index 7ef5199..f5f0034 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -137,4 +137,5 @@ <flag name='qxl-vga.vgamem_mb'/> <flag name='pci-serial'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 8c3d48e..9f0461a 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -146,4 +146,5 @@ <flag name='qxl-vga.vgamem_mb'/> <flag name='pci-serial'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 72f7625..1b23b82 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -152,4 +152,5 @@ <flag name='qxl-vga.vgamem_mb'/> <flag name='pci-serial'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index d81c80c..ff0427f 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -152,4 +152,5 @@ <flag name='qxl-vga.vgamem_mb'/> <flag name='pci-serial'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 1a39dce..56b27e5 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -168,4 +168,5 @@ <flag name='pc-dimm'/> <flag name='pci-serial'/> <flag name='ioh3420'/> + <flag name='x3130-upstream'/> </qemuCaps> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 6211640..62b9a0c 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -754,7 +754,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_SPLASH_TIMEOUT, QEMU_CAPS_DEVICE_IVSHMEM, - QEMU_CAPS_DEVICE_IOH3420); + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_X3130_UPSTREAM); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -855,7 +856,8 @@ mymain(void) QEMU_CAPS_OBJECT_USB_AUDIO, QEMU_CAPS_SPLASH_TIMEOUT, QEMU_CAPS_DEVICE_IVSHMEM, - QEMU_CAPS_DEVICE_IOH3420); + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_X3130_UPSTREAM); DO_TEST_FULL("qemu-1.2.0", 1002000, 0, 0, VIR_ERR_CONFIG_UNSUPPORTED, QEMU_CAPS_LAST); DO_TEST_FULL("qemu-kvm-1.2.0", 1002000, 1, 0, VIR_ERR_CONFIG_UNSUPPORTED, -- 2.1.0

this is backed by the qemu device x3130-upstream. It can only plug into a pcie-root-port or pcie-switch-downstream-port. --- change from V2: * adjust for enum-based model name src/qemu/qemu_command.c | 46 ++++++++++++++++++++++ .../qemuxml2argv-pcie-switch-upstream-port.args | 12 ++++++ tests/qemuxml2argvtest.c | 9 +++++ 3 files changed, 67 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e3ee911..f7f102f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1612,6 +1612,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, */ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + /* pcie-switch can only connect to a true + * pcie bus, and can't be hot-plugged. + */ + flags = VIR_PCI_CONNECT_TYPE_PCIE_PORT; + break; default: break; } @@ -2289,6 +2295,9 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, options->port = (addr->slot << 3) + addr->function; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) + options->modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM; + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: @@ -2404,6 +2413,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, /* pcie-root-port can only plug into pcie-root */ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + /* pcie-switch really does need a real PCIe + * port, but it doesn't need to be pcie-root + */ + flags = VIR_PCI_CONNECT_TYPE_PCIE_PORT; + break; default: flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; break; @@ -4767,6 +4782,37 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, modelName, def->opts.pciopts.port, def->opts.pciopts.chassis, def->info.alias); break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + if (def->opts.pciopts.modelName + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pcie-switch-upstream-port options not set")); + goto error; + } + modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown pcie-switch-upstream-port model name value %d"), + def->opts.pciopts.modelName); + goto error; + } + if (def->opts.pciopts.modelName + != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' " + "is not valid for a pcie-switch-upstream-port"), + modelName); + goto error; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pcie-switch-upstream-port (x3130-upstream) " + "controller is not supported in this QEMU binary")); + goto error; + } + + virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); + break; } break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args new file mode 100644 index 0000000..c08cd4a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args @@ -0,0 +1,12 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,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=0x1 \ +-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 x3130-upstream,id=pci.5,bus=pci.3,addr=0x0 \ +-device x3130-upstream,id=pci.6,bus=pci.4,addr=0x0 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ +-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ +-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b429bc5..651b860 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1516,6 +1516,15 @@ mymain(void) QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); + DO_TEST("pcie-switch-upstream-port", + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_X3130_UPSTREAM, + QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); + DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI, -- 2.1.0

This controller can be connected only to a port on a pcie-switch-upstream-port. It provides a single hotpluggable port that will accept any PCI or PCIe device, as well as any device requiring a pcie-*-port (the only current example of such a device is the pcie-switch-upstream-port). --- change for V2: * 1.3.0 -> 1.2.18 * possibly reworded documention a bit. docs/formatdomain.html.in | 49 +++++++++++++++------- docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_addr.c | 11 +++++ src/conf/domain_conf.c | 6 ++- src/conf/domain_conf.h | 2 + src/qemu/qemu_command.c | 1 + .../qemuxml2argv-pcie-switch-downstream-port.xml | 44 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d033542..2f61ac8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3030,11 +3030,12 @@ PCI controllers have an optional <code>model</code> attribute with possible values <code>pci-root</code>, <code>pcie-root</code>, <code>pcie-root-port</code>, <code>pci-bridge</code>, - <code>dmi-to-pci-bridge</code>, or <code>pcie-switch-upstream-port</code>. + <code>dmi-to-pci-bridge</code>, <code>pcie-switch-upstream-port</code>, or + <code>pcie-switch-downstream-port</code>. (pci-root and pci-bridge <span class="since">since 1.0.5</span>, pcie-root and dmi-to-pci-bridge <span class="since">since - 1.1.2</span>, pcie-root-port and - pcie-switch-upstream-port <span class="since">since 1.2.18</span>) + 1.1.2</span>, pcie-root-port, pcie-switch-upstream-port, and + pcie-switch-downstream-port <span class="since">since 1.2.18</span>) The root controllers (<code>pci-root</code> and <code>pcie-root</code>) have an optional <code>pcihole64</code> element specifying how big (in kilobytes, or in the unit specified by <code>pcihole64</code>'s @@ -3081,8 +3082,8 @@ </dd> <dt><code>chassis</code></dt> <dd> - pcie-root-port controllers can also have - a <code>chassis</code> attribute in + pcie-root-port and pcie-switch-downstream-port controllers can + also have a <code>chassis</code> attribute in the <code>&lt;target&gt;</code> subelement, which is used to set the controller's "chassis" configuration value, which is visible to the virtual machine. If set, chassis must be @@ -3090,8 +3091,9 @@ </dd> <dt><code>port</code></dt> <dd> - pcie-root-port controllers can also have a <code>port</code> - attribute in the <code>&lt;target&gt;</code> subelement, which + pcie-root-port and pcie-switch-downstream-port controllers can + also have a <code>port</code> attribute in + the <code>&lt;target&gt;</code> subelement, which is used to set the controller's "port" configuration value, which is visible to the virtual machine. If set, port must be between 0 and 255. @@ -3144,14 +3146,31 @@ </p> <p> Domains with an implicit pcie-root can also add controllers - with <code>model='pcie-root-port'</code>. This is a simple type of - bridge device that can connect only to one of the 31 slots on - the pcie-root bus on the upstream side, and makes a single - (PCIe, hotpluggable) port (at slot='0') available on the - downstream side. This controller can be used to provide a single - slot to later hotplug a PCIe device (but is not itself - hotpluggable - it must be in the configuration when the domain - is started). (<span class="since">since 1.2.18</span>) + with <code>model='pcie-root-port'</code>, + <code>model='pcie-switch-upstream-port'</code>, + and <code>model='pcie-switch-downstream-port'</code>. pcie-root-port + is a simple type of bridge device that can connect only to one + of the 31 slots on the pcie-root bus on its upstream side, and + makes a single (PCIe, hotpluggable) port available on the + downstream side (at slot='0'). pcie-root-port can be used to + provide a single slot to later hotplug a PCIe device (but is not + itself hotpluggable - it must be in the configuration when the + domain is started). + (<span class="since">since 1.2.18</span>) + </p> + <p> + pcie-switch-upstream-port is a more flexible (but also more + complex) device that can only plug into a pcie-root-port or + pcie-switch-downstream-port on the upstream side (and only + before the domain is started - it is not hot-pluggable), and + provides 32 ports on the downstream side (slot='0' - slot='31') + that accept only pcie-switch-downstream-port devices; each + pcie-switch-downstream-port device can only plug into a + pcie-switch-upstream-port on its upstream side (again, not + hot-pluggable), and on its downstream side provides a single + hotpluggable pcie port that can accept any standard pci or pcie + device (or another pcie-switch-upstream-port). + (<span class="since">since 1.2.18</span>) </p> <pre> ... diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ce01230..ab02d14 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1743,6 +1743,8 @@ <value>ioh3420</value> <!-- implementations of 'pcie-switch-upstream-port' --> <value>x3130-upstream</value> + <!-- implementations of 'pcie-switch-downstream-port' --> + <value>xio3130-downstream</value> </choice> </attribute> <empty/> @@ -1790,6 +1792,7 @@ <value>dmi-to-pci-bridge</value> <value>pcie-root-port</value> <value>pcie-switch-upstream-port</value> + <value>pcie-switch-downstream-port</value> </choice> </attribute> </group> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 8bd4ac3..561d8cc 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -213,6 +213,17 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, bus->minSlot = 0; bus->maxSlot = 31; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + /* provides one slot which is pcie, can be used by devices + * that must connect to some type of "pcie-*-port", and + * is hotpluggable + */ + bus->flags = VIR_PCI_CONNECT_TYPE_PCIE + | VIR_PCI_CONNECT_TYPE_PCIE_PORT + | VIR_PCI_CONNECT_HOTPLUGGABLE; + bus->minSlot = 0; + bus->maxSlot = 0; + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid PCI controller model %d"), model); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f63972d..0cf111a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -326,7 +326,8 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, "pci-bridge", "dmi-to-pci-bridge", "pcie-root-port", - "pcie-switch-upstream-port") + "pcie-switch-upstream-port", + "pcie-switch-downstream-port") VIR_ENUM_IMPL(virDomainControllerPCIModelName, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST, @@ -334,7 +335,8 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "pci-bridge", "i82801b11-bridge", "ioh3420", - "x3130-upstream") + "x3130-upstream", + "xio3130-downstream") VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a82e955..440d74d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -754,6 +754,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE, VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT, VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT, + VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST } virDomainControllerModelPCI; @@ -764,6 +765,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f7f102f..4e20658 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2298,6 +2298,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (options->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) options->modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml new file mode 100644 index 0000000..fca3149 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml @@ -0,0 +1,44 @@ +<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='sda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'/> + <controller type='pci' index='2' model='pci-bridge'/> + <controller type='pci' index='3' model='pcie-root-port'/> + <controller type='pci' index='4' model='pcie-switch-upstream-port'/> + <controller type='pci' index='5' model='pcie-switch-downstream-port'/> + <controller type='pci' index='6' model='pcie-switch-downstream-port'/> + <controller type='pci' index='7' model='pcie-switch-downstream-port'/> + <controller type='pci' index='8' model='pcie-switch-downstream-port'> + <model name='xio3130-downstream'/> + <target chassis='30' port='0x27'/> + </controller> + <controller type='pci' index='9' model='pcie-switch-upstream-port'/> + <controller type='pci' index='10' model='pcie-switch-downstream-port'/> + <controller type='pci' index='11' model='pcie-switch-downstream-port'/> + <controller type='pci' index='12' model='pcie-switch-downstream-port'/> + <controller type='sata' index='0'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 9a6fb16..b261db2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -570,6 +570,7 @@ mymain(void) DO_TEST("pcie-root-port"); DO_TEST("pcie-root-port-too-many"); DO_TEST("pcie-switch-upstream-port"); + DO_TEST("pcie-switch-downstream-port"); DO_TEST("hostdev-scsi-lsi"); DO_TEST("hostdev-scsi-virtio-scsi"); -- 2.1.0

On 08/03/2015 06:23 AM, Martin Kletzander wrote: It's that way because I didn't know for certain that there wouldn't be some difference between the two that needed to be accounted for (and at one point they were different, but that was due to my naive understanding of the differences between the different types of controllers). I noticed the similarity fairly late in the game, but forgot to do anything about it. I've now combined the two cases.