From 4277e61e22b7532dc476c44a356081053da470f8 Mon Sep 17 00:00:00 2001 From: Roman Bogorodskiy Date: Sun, 20 Sep 2020 18:53:57 +0400 Subject: [PATCH] bhyve: soften requirements for slot 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, slot 1 is only allowed to be used by the LPC device. Relax this requirement and allow to use slot 1 if it was explicitly specified by the user for any other device type. In this case the LPC device will have the next available address. If slot 1 was not used by the user, it'll be reserved for the LPC device, even if it is not configured to make address assignment consistent in case the LPC device becomes necessary (e.g. the user adds a console or a video device which require LPC). Signed-off-by: Roman Bogorodskiy Reviewed-by: Daniel P. Berrangé --- po/POTFILES.in | 1 - src/bhyve/bhyve_device.c | 50 +++++++++---------- ...argv-addr-non-isa-controller-on-slot-1.xml | 1 + tests/bhyvexml2argvtest.c | 2 +- 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 54f78e7861..9782b2beb4 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -12,7 +12,6 @@ @SRCDIR@src/admin/libvirt-admin.c @SRCDIR@src/bhyve/bhyve_capabilities.c @SRCDIR@src/bhyve/bhyve_command.c -@SRCDIR@src/bhyve/bhyve_device.c @SRCDIR@src/bhyve/bhyve_domain.c @SRCDIR@src/bhyve/bhyve_driver.c @SRCDIR@src/bhyve/bhyve_monitor.c diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 2295acf552..e2e1efd97e 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -42,21 +42,8 @@ bhyveCollectPCIAddress(virDomainDefPtr def G_GNUC_UNUSED, virDomainPCIAddressSetPtr addrs = opaque; virPCIDeviceAddressPtr addr = &info->addr.pci; - if (addr->domain == 0 && addr->bus == 0) { - if (addr->slot == 0) { + if (addr->domain == 0 && addr->bus == 0 && addr->slot == 0) { return 0; - } else if (addr->slot == 1) { - if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER && - device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("PCI bus 0 slot 1 is reserved for the implicit " - "LPC PCI-ISA bridge")); - return -1; - } else { - /* We reserve slot 1 for LPC in bhyveAssignDevicePCISlots(), so exit early */ - return 0; - } - } } if (virDomainPCIAddressReserveAddr(addrs, addr, @@ -98,29 +85,38 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, 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; - if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { - return -1; - } + /* If the user didn't explicitly specify slot 1 for some of the devices, + reserve it for LPC, even if there's no LPC device configured. + If the slot 1 is used by some other device, LPC will have an address + auto-assigned. - for (i = 0; i < def->ncontrollers; i++) { - if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && - virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { - def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - def->controllers[i]->info.addr.pci = lpc_addr; - break; - } + The idea behind that is to try to use slot 1 for the LPC device unless + user specifically configured otherwise.*/ + if (!virDomainPCIAddressSlotInUse(addrs, &lpc_addr)) { + if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { + return -1; + } + + for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && + virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci = lpc_addr; + break; + } + } } for (i = 0; i < def->ncontrollers; i++) { if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) || (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) || ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && - (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI))) { + (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI)) || + def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) { if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || !virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) continue; diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml index 88ad9aebb7..222a11b7d0 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml @@ -5,6 +5,7 @@ 1 hvm + /path/to/test.fd diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index b86a4243c2..2167cd6310 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -213,7 +213,7 @@ mymain(void) DO_TEST("addr-multiple-virtio-disks"); DO_TEST("addr-isa-controller-on-slot-1"); DO_TEST("addr-isa-controller-on-slot-31"); - DO_TEST_FAILURE("addr-non-isa-controller-on-slot-1"); + DO_TEST("addr-non-isa-controller-on-slot-1"); /* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;