Re: [libvirt] [PATCH 2/4] bhyve: model PCI ISA bridge

bhyve(8) uses PCI ISA bridge to attach serial ports and a boot ROM. In the libvirt driver a PCI slot 1 was always reserved for that, and if a domain used serial ports or a boot ROM, then it would be added to the command line. However, some guests require the ISA bridge to have PCI slot other than 1. To make things more flexible, explicitly model that in XML. So now the behavior is as follows: * The 'isa-bridge' PCI controller is added to the domain if it uses video or serial devices, or boot ROM * If user didn't assign a PCI address for that controller, slot 1 will reserved for it * If user did assign a PCI address for it, this address will be used, and PCI slot 1 will be free (available for reservation for other devices) * If user assigned slot 1 to other device, slot is checked. If it's also assigned already, then the next available PCI slot is used for LPC PCI-ISA bridge, and a warning about that is emitted

Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; --- src/bhyve/bhyve_command.c | 32 +++++-------- src/bhyve/bhyve_device.c | 47 ++++++++++++------- src/bhyve/bhyve_domain.c | 6 +++ .../bhyvexml2argv-acpiapic.args | 2 +- ...xml2argv-addr-more-than-32-sata-disks.args | 6 +-- ...hyvexml2argv-addr-multiple-sata-disks.args | 4 +- ...vexml2argv-addr-multiple-virtio-disks.args | 6 +-- ...rgv-addr-no32devs-multiple-sata-disks.args | 6 +-- ...l2argv-addr-no32devs-single-sata-disk.args | 4 +- .../bhyvexml2argv-addr-single-sata-disk.args | 4 +- ...bhyvexml2argv-addr-single-virtio-disk.args | 2 +- ...vexml2argv-addr-slot-1-and-31-not-lpc.args | 10 ++++ ...xml2argv-addr-slot-1-and-31-not-lpc.ldargs | 3 ++ ...yvexml2argv-addr-slot-1-and-31-not-lpc.xml | 27 +++++++++++ .../bhyvexml2argv-addr-slot-1-not-lpc.args | 10 ++++ .../bhyvexml2argv-addr-slot-1-not-lpc.ldargs | 3 ++ .../bhyvexml2argv-addr-slot-1-not-lpc.xml | 27 +++++++++++ .../bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder1.args | 2 +- .../bhyvexml2argv-bhyveload-bootorder3.args | 2 +- .../bhyvexml2argv-bhyveload-explicitargs.args | 2 +- .../bhyvexml2argv-commandline.args | 2 +- .../bhyvexml2argv-console.args | 2 +- .../bhyvexml2argv-cputopology.args | 2 +- .../bhyvexml2argv-custom-loader.args | 2 +- .../bhyvexml2argv-disk-cdrom-grub.args | 2 +- .../bhyvexml2argv-disk-cdrom.args | 2 +- .../bhyvexml2argv-explicit-lpc.args | 10 ++++ .../bhyvexml2argv-explicit-lpc.ldargs | 3 ++ .../bhyvexml2argv-explicit-lpc.xml | 26 ++++++++++ .../bhyvexml2argv-grub-bootorder.args | 2 +- .../bhyvexml2argv-grub-bootorder2.args | 2 +- .../bhyvexml2argv-grub-defaults.args | 2 +- .../bhyvexml2argv-input-xhci-tablet.args | 4 +- .../bhyvexml2argv-localtime.args | 2 +- .../bhyvexml2argv-macaddr.args | 2 +- .../bhyvexml2argv-net-e1000.args | 2 +- .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub.args | 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +- .../bhyvexml2argv-vnc-autoport.args | 4 +- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +- .../bhyvexml2argv-vnc-vgaconf-off.args | 4 +- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +- .../bhyvexml2argv-wired.args | 2 +- tests/bhyvexml2argvtest.c | 3 ++ .../bhyvexml2xmlout-acpiapic.xml | 2 +- ...ml2xmlout-addr-more-than-32-sata-disks.xml | 6 +-- ...yvexml2xmlout-addr-multiple-sata-disks.xml | 4 +- ...exml2xmlout-addr-multiple-virtio-disks.xml | 6 +-- ...lout-addr-no32devs-multiple-sata-disks.xml | 8 ++-- ...2xmlout-addr-no32devs-single-sata-disk.xml | 4 +- .../bhyvexml2xmlout-addr-single-sata-disk.xml | 4 +- ...hyvexml2xmlout-addr-single-virtio-disk.xml | 2 +- ...exml2xmlout-addr-slot-1-and-31-not-lpc.xml | 36 ++++++++++++++ .../bhyvexml2xmlout-addr-slot-1-not-lpc.xml | 36 ++++++++++++++ .../bhyvexml2xmlout-base.xml | 2 +- .../bhyvexml2xmlout-bhyveload-bootorder.xml | 2 +- .../bhyvexml2xmlout-bhyveload-bootorder1.xml | 2 +- .../bhyvexml2xmlout-bhyveload-bootorder2.xml | 2 +- .../bhyvexml2xmlout-bhyveload-bootorder3.xml | 2 +- .../bhyvexml2xmlout-bhyveload-bootorder4.xml | 2 +- ...bhyvexml2xmlout-bhyveload-explicitargs.xml | 2 +- .../bhyvexml2xmlout-commandline.xml | 2 +- .../bhyvexml2xmlout-console.xml | 3 ++ .../bhyvexml2xmlout-custom-loader.xml | 2 +- .../bhyvexml2xmlout-disk-cdrom-grub.xml | 2 +- .../bhyvexml2xmlout-disk-cdrom.xml | 2 +- .../bhyvexml2xmlout-explicit-lpc.xml | 36 ++++++++++++++ .../bhyvexml2xmlout-grub-bootorder.xml | 2 +- .../bhyvexml2xmlout-grub-bootorder2.xml | 2 +- .../bhyvexml2xmlout-grub-defaults.xml | 2 +- .../bhyvexml2xmlout-input-xhci-tablet.xml | 4 +- .../bhyvexml2xmlout-localtime.xml | 2 +- .../bhyvexml2xmlout-macaddr.xml | 2 +- .../bhyvexml2xmlout-metadata.xml | 4 +- .../bhyvexml2xmlout-serial-grub-nocons.xml | 3 ++ .../bhyvexml2xmlout-serial-grub.xml | 3 ++ .../bhyvexml2xmlout-serial.xml | 3 ++ .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-io.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-on.xml | 3 ++ .../bhyvexml2xmlout-vnc.xml | 3 ++ .../bhyvexml2xmlout-wired.xml | 2 +- tests/bhyvexml2xmltest.c | 3 ++ 89 files changed, 400 insertions(+), 127 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-slot-1-and-31-not-lpc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-slot-1-and-31-not-lpc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-slot-1-and-31-not-lpc.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-slot-1-not-lpc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-slot-1-not-lpc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-slot-1-not-lpc.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-explicit-lpc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-explicit-lpc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-explicit-lpc.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-slot-1-and-31-not-lpc.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-slot-1-not-lpc.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-explicit-lpc.xml diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 1f215dac08..0842484086 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -325,14 +325,6 @@ bhyveBuildVirtIODiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return 0; } -static int -bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, - virCommandPtr cmd) -{ - virCommandAddArgList(cmd, "-s", "1,lpc", NULL); - return 0; -} - static int bhyveBuildGraphicsArgStr(const virDomainDef *def, virDomainGraphicsDefPtr graphics, @@ -460,7 +452,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, * vm0 */ size_t i; - bool add_lpc = false; int nusbcontrollers = 0; unsigned int nvcpus = virDomainDefGetVcpus(def); @@ -549,7 +540,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Installed bhyve binary does not support " @@ -563,12 +553,20 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virDomainControllerDefPtr controller = def->controllers[i]; switch (controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_PCI: - if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("unsupported PCI controller model: only PCI root supported")); - goto error; - } + switch (controller->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ISA_BRIDGE: + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "%d:0,lpc", + controller->info.addr.pci.slot); + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("unsupported PCI controller model: only PCI root supported")); + goto error; + } + break; case VIR_DOMAIN_CONTROLLER_TYPE_SATA: if (bhyveBuildAHCIControllerArgStr(def, controller, conn, cmd) < 0) goto error; @@ -613,7 +611,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], conn, cmd, dryRun) < 0) goto error; - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Multiple graphics devices are not supported")); @@ -621,9 +618,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } } - if (add_lpc || def->nserials) - bhyveBuildLPCArgStr(def, cmd); - if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error; diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 201044d9e6..d9ac1911a3 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -43,16 +43,8 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainPCIAddressSetPtr addrs = opaque; virPCIDeviceAddressPtr addr = &info->addr.pci; - if (addr->domain == 0 && addr->bus == 0) { - if (addr->slot == 0) { - return 0; - } else if (addr->slot == 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("PCI bus 0 slot 1 is reserved for the implicit " - "LPC PCI-ISA bridge")); - return -1; - } - } + if (addr->domain == 0 && addr->bus == 0 && addr->slot == 0) + return 0; if (virDomainPCIAddressReserveAddr(addrs, addr, VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { @@ -92,15 +84,36 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, virDomainPCIAddressSetPtr addrs) { size_t i; - virPCIDeviceAddress lpc_addr; - /* explicitly reserve slot 1 for LPC-ISA bridge */ - memset(&lpc_addr, 0, sizeof(lpc_addr)); - lpc_addr.slot = 0x1; + /* Look for isa-bridge first, if it has no address assigned, we want to reserve + PCI slot 1 for it before it's used by some other device */ + for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) && + (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ISA_BRIDGE) && + virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { + virPCIDeviceAddress lpc_addr; + memset(&lpc_addr, 0, sizeof(lpc_addr)); + lpc_addr.slot = 0x1; + + if (virDomainPCIAddressSlotInUse(addrs, &lpc_addr)) { + lpc_addr.slot = 0x1f; + + if (virDomainPCIAddressSlotInUse(addrs, &lpc_addr)) { + VIR_WARN("Cannot use PCI slots 1 and 31 for LPC PCI-ISA bridge " + "as they are already reserved, using the next available " + "address"); + continue; + } + } + + if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { + goto error; + }

- if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { - goto error; + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci = lpc_addr; + } } for (i = 0; i < def->ncontrollers; i++) { diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 79cf103d28..6e09cbc484 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -73,6 +73,12 @@ bhyveDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) return -1; + if ((def->os.bootloader == NULL && def->os.loader) || + (def->nconsoles || def->nserials) || (def->ngraphics && def->nvideos))