bhyve: soften requirements for slot 1

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 <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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

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->domain == 0 && addr->bus == 0 && addr->slot == 0) {
-        if (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,10 +85,17 @@ 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 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.
+
+       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;
@@ -115,12 +109,14 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
                  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;

tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml

@@ -5,6 +5,7 @@
   <vcpu>1</vcpu>
   <os>
     <type>hvm</type>
+    <loader readonly="yes" type="pflash">/path/to/test.fd</loader>
   </os>
   <devices>
     <disk type='file'>

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;