[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[PATCH v6 3/3] 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>
---
 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 @@
   <vcpu>1</vcpu>
   <os>
     <type>hvm</type>
+    <loader readonly="yes" type="pflash">/path/to/test.fd</loader>
   </os>
   <devices>
     <disk type='file'>
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;
-- 
2.27.0


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]