1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/6] bhyve: model PCI ISA bridge
  4. View patch

[libvirt] [PATCH v2 3/6] bhyve: support 'isa' controller for LPC

Roman Bogorodskiy posted 6 patches 6 years, 11 months ago
[libvirt] [PATCH v2 3/6] bhyve: support 'isa' controller for LPC
Posted by Roman Bogorodskiy 6 years, 11 months ago 
Support modeling of the 'isa' controller for bhyve. When controller is
not present in the domain XML, but domain requires it to be there (e.g.
because bootrom is used), implicitly add addition of this controller to
the command line arguments, and bind it to PCI slot 1.

PCI slot 1 is always reserved still. User can manually define any PCI
slot for the 'isa' controller, including PCI slot 1, but other devices
are not allowed to use this address.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
 src/bhyve/bhyve_command.c                     | 18 +++++++++-
 src/bhyve/bhyve_device.c                      | 25 ++++++++++---
 ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++++++
 ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
 ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++++++++++++++
 ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++++++
 ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
 ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++++++++++++++
 ...argv-addr-non-isa-controller-on-slot-1.xml | 23 ++++++++++++
 .../bhyvexml2argv-isa-controller.args         | 10 ++++++
 .../bhyvexml2argv-isa-controller.ldargs       |  3 ++
 .../bhyvexml2argv-isa-controller.xml          | 24 +++++++++++++
 ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +++++++++++++
 tests/bhyvexml2argvtest.c                     |  5 +++
 ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++++++++++++++++++
 ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++++++++++++++++++
 .../bhyvexml2xmlout-isa-controller.xml        | 36 +++++++++++++++++++
 tests/bhyvexml2xmltest.c                      |  3 ++
 18 files changed, 317 insertions(+), 5 deletions(-)
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index f49dc77118..2c90265a93 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -461,6 +461,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
      */
     size_t i;
     int nusbcontrollers = 0;
+    int nisacontrollers = 0;
     unsigned int nvcpus = virDomainDefGetVcpus(def);
 
     virCommandPtr cmd = virCommandNew(BHYVE);
@@ -581,6 +582,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
                 if (bhyveBuildUSBControllerArgStr(def, controller, cmd) < 0)
                     goto error;
                 break;
+        case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
+                if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) {
+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                       "%s", _("unsupported ISA controller model: only ISA bridge supported"));
+                        goto error;
+                }
+                if (++nisacontrollers > 1) {
+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                       "%s", _("only single ISA controller is supported"));
+                        goto error;
+                }
+                virCommandAddArg(cmd, "-s");
+                virCommandAddArgFormat(cmd, "%d:0,lpc",
+                                       controller->info.addr.pci.slot);
+                break;
         }
     }
     for (i = 0; i < def->nnets; i++) {
@@ -618,7 +634,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
         }
     }
 
-    if (bhyveDomainDefNeedsISAController(def))
+    if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def))
         bhyveBuildLPCArgStr(def, cmd);
 
     if (bhyveBuildConsoleArgStr(def, cmd) < 0)
diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
index 201044d9e6..42093afb7b 100644
--- a/src/bhyve/bhyve_device.c
+++ b/src/bhyve/bhyve_device.c
@@ -47,10 +47,17 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
         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 (!((device->type == VIR_DOMAIN_DEVICE_CONTROLLER) &&
+                 (device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
+                 (device->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE))) {
+                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;
+            }
         }
     }
 
@@ -103,6 +110,16 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
         goto error;
     }
 
+    for (i = 0; i < def->ncontrollers; i++) {
+         if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
+             (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) &&
+              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) ||
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
new file mode 100644
index 0000000000..910d1bbcfa
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
@@ -0,0 +1,10 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-s 1:0,lpc \
+-s 2:0,ahci,hd:/tmp/freebsd.img \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
new file mode 100644
index 0000000000..32538b558e
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
@@ -0,0 +1,3 @@
+/usr/sbin/bhyveload \
+-m 214 \
+-d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
new file mode 100644
index 0000000000..796903a9fa
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
@@ -0,0 +1,26 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory>219136</memory>
+  <vcpu>1</vcpu>
+  <os>
+    <type>hvm</type>
+  </os>
+  <devices>
+    <controller type='isa' index='1' model='isa-bridge'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <disk type='file'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <model type='virtio'/>
+      <source bridge="virbr0"/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
new file mode 100644
index 0000000000..ee833eb460
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
@@ -0,0 +1,10 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-s 31:0,lpc \
+-s 2:0,ahci,hd:/tmp/freebsd.img \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
new file mode 100644
index 0000000000..32538b558e
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
@@ -0,0 +1,3 @@
+/usr/sbin/bhyveload \
+-m 214 \
+-d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
new file mode 100644
index 0000000000..65d7480db6
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
@@ -0,0 +1,26 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory>219136</memory>
+  <vcpu>1</vcpu>
+  <os>
+    <type>hvm</type>
+  </os>
+  <devices>
+    <controller type='isa' index='1' model='isa-bridge'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/>
+    </controller>
+    <disk type='file'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <model type='virtio'/>
+      <source bridge="virbr0"/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
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
new file mode 100644
index 0000000000..88ad9aebb7
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
@@ -0,0 +1,23 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory>219136</memory>
+  <vcpu>1</vcpu>
+  <os>
+    <type>hvm</type>
+  </os>
+  <devices>
+    <disk type='file'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <model type='virtio'/>
+      <source bridge="virbr0"/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
new file mode 100644
index 0000000000..910d1bbcfa
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
@@ -0,0 +1,10 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-s 1:0,lpc \
+-s 2:0,ahci,hd:/tmp/freebsd.img \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
new file mode 100644
index 0000000000..32538b558e
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
@@ -0,0 +1,3 @@
+/usr/sbin/bhyveload \
+-m 214 \
+-d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
new file mode 100644
index 0000000000..71265ea32c
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
@@ -0,0 +1,24 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory>219136</memory>
+  <vcpu>1</vcpu>
+  <os>
+    <type>hvm</type>
+  </os>
+  <devices>
+    <controller type='isa' index='1' model='isa-bridge'/>
+    <disk type='file'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <model type='virtio'/>
+      <source bridge="virbr0"/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
new file mode 100644
index 0000000000..25622ce78a
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
@@ -0,0 +1,25 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory>219136</memory>
+  <vcpu>1</vcpu>
+  <os>
+    <type>hvm</type>
+  </os>
+  <devices>
+    <controller type='isa' index='1' model='isa-bridge'/>
+    <controller type='isa' index='2' model='isa-bridge'/>
+    <disk type='file'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <model type='virtio'/>
+      <source bridge="virbr0"/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 4a7f65a8e2..d120bd43ca 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -211,6 +211,8 @@ mymain(void)
     DO_TEST("cputopology");
     DO_TEST_FAILURE("cputopology-nvcpu-mismatch");
     DO_TEST("commandline");
+    DO_TEST("isa-controller");
+    DO_TEST_FAILURE("isa-multiple-controllers");
 
     /* Address allocation tests */
     DO_TEST("addr-single-sata-disk");
@@ -218,6 +220,9 @@ mymain(void)
     DO_TEST("addr-more-than-32-sata-disks");
     DO_TEST("addr-single-virtio-disk");
     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");
 
     /* The same without 32 devs per controller support */
     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;
diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
new file mode 100644
index 0000000000..db84185fe6
--- /dev/null
+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
@@ -0,0 +1,36 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64'>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>
+    <disk type='file' device='disk'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <controller type='isa' index='1' model='isa-bridge'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </controller>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <source bridge='virbr0'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
new file mode 100644
index 0000000000..6ab5e6ad1a
--- /dev/null
+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
@@ -0,0 +1,36 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64'>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>
+    <disk type='file' device='disk'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <controller type='isa' index='1' model='isa-bridge'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </controller>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <source bridge='virbr0'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
new file mode 100644
index 0000000000..db84185fe6
--- /dev/null
+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
@@ -0,0 +1,36 @@
+<domain type='bhyve'>
+  <name>bhyve</name>
+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64'>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>
+    <disk type='file' device='disk'>
+      <driver name='file' type='raw'/>
+      <source file='/tmp/freebsd.img'/>
+      <target dev='hda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+    </disk>
+    <controller type='isa' index='1' model='isa-bridge'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </controller>
+    <interface type='bridge'>
+      <mac address='52:54:00:b9:94:02'/>
+      <source bridge='virbr0'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+  </devices>
+</domain>
diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index ed421b8839..4b7f59ea29 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -111,6 +111,7 @@ mymain(void)
     DO_TEST_DIFFERENT("vnc-vgaconf-io");
     DO_TEST_DIFFERENT("vnc-autoport");
     DO_TEST_DIFFERENT("commandline");
+    DO_TEST_DIFFERENT("isa-controller");
 
     /* Address allocation tests */
     DO_TEST_DIFFERENT("addr-single-sata-disk");
@@ -118,6 +119,8 @@ mymain(void)
     DO_TEST_DIFFERENT("addr-more-than-32-sata-disks");
     DO_TEST_DIFFERENT("addr-single-virtio-disk");
     DO_TEST_DIFFERENT("addr-multiple-virtio-disks");
+    DO_TEST_DIFFERENT("addr-isa-controller-on-slot-1");
+    DO_TEST_DIFFERENT("addr-isa-controller-on-slot-31");
 
     /* The same without 32 devs per controller support */
     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] bhyve: support 'isa' controller for LPC
Posted by Ján Tomko 6 years, 11 months ago 
On Sun, Feb 17, 2019 at 05:04:02PM +0400, Roman Bogorodskiy wrote:
>Support modeling of the 'isa' controller for bhyve. When controller is
>not present in the domain XML, but domain requires it to be there (e.g.
>because bootrom is used), implicitly add addition of this controller to
>the command line arguments, and bind it to PCI slot 1.
>
>PCI slot 1 is always reserved still. User can manually define any PCI
>slot for the 'isa' controller, including PCI slot 1, but other devices
>are not allowed to use this address.
>

I thought one of the points was to allow the use of slot 1 for users
who request it. But this also works - the restriction can be relaxed
later if needed.

>Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
>---
> src/bhyve/bhyve_command.c                     | 18 +++++++++-
> src/bhyve/bhyve_device.c                      | 25 ++++++++++---
> ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++++++
> ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
> ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++++++++++++++
> ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++++++
> ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
> ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++++++++++++++
> ...argv-addr-non-isa-controller-on-slot-1.xml | 23 ++++++++++++
> .../bhyvexml2argv-isa-controller.args         | 10 ++++++
> .../bhyvexml2argv-isa-controller.ldargs       |  3 ++
> .../bhyvexml2argv-isa-controller.xml          | 24 +++++++++++++
> ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +++++++++++++
> tests/bhyvexml2argvtest.c                     |  5 +++
> ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++++++++++++++++++
> ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++++++++++++++++++
> .../bhyvexml2xmlout-isa-controller.xml        | 36 +++++++++++++++++++
> tests/bhyvexml2xmltest.c                      |  3 ++
> 18 files changed, 317 insertions(+), 5 deletions(-)
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
>
>diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
>index f49dc77118..2c90265a93 100644
>--- a/src/bhyve/bhyve_command.c
>+++ b/src/bhyve/bhyve_command.c
>@@ -461,6 +461,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
>      */
>     size_t i;
>     int nusbcontrollers = 0;
>+    int nisacontrollers = 0;
>     unsigned int nvcpus = virDomainDefGetVcpus(def);
>
>     virCommandPtr cmd = virCommandNew(BHYVE);
>@@ -581,6 +582,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
>                 if (bhyveBuildUSBControllerArgStr(def, controller, cmd) < 0)
>                     goto error;
>                 break;
>+        case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
>+                if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) {
>+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                                       "%s", _("unsupported ISA controller model: only ISA bridge supported"));
>+                        goto error;
>+                }
>+                if (++nisacontrollers > 1) {
>+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                                       "%s", _("only single ISA controller is supported"));
>+                        goto error;
>+                }

This error message can be reported much sooner - since we already forbid
multiple controllers with the same index, all you need to do is forbid
non-zero index virDomainControllerDefValidate

Also, this whole switch is indented by 8 instead of 4 spaces.

>+                virCommandAddArg(cmd, "-s");
>+                virCommandAddArgFormat(cmd, "%d:0,lpc",
>+                                       controller->info.addr.pci.slot);
>+                break;
>         }
>     }
>     for (i = 0; i < def->nnets; i++) {
>@@ -618,7 +634,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
>         }
>     }
>
>-    if (bhyveDomainDefNeedsISAController(def))
>+    if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def))
>         bhyveBuildLPCArgStr(def, cmd);
>
>     if (bhyveBuildConsoleArgStr(def, cmd) < 0)
>diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
>index 201044d9e6..42093afb7b 100644
>--- a/src/bhyve/bhyve_device.c
>+++ b/src/bhyve/bhyve_device.c
>@@ -47,10 +47,17 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
>         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 (!((device->type == VIR_DOMAIN_DEVICE_CONTROLLER) &&
>+                 (device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&

== has higher precedence than &&, the inner parentheses can be dropped

>+                 (device->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE))) {
>+                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;
>+            }
>         }
>     }
>
>@@ -103,6 +110,16 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
>         goto error;
>     }
>
>+    for (i = 0; i < def->ncontrollers; i++) {
>+         if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
>+             (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) &&
>+              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) ||
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
>new file mode 100644
>index 0000000000..910d1bbcfa
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
>@@ -0,0 +1,10 @@
>+/usr/sbin/bhyve \
>+-c 1 \
>+-m 214 \
>+-u \
>+-H \
>+-P \
>+-s 0:0,hostbridge \
>+-s 1:0,lpc \
>+-s 2:0,ahci,hd:/tmp/freebsd.img \
>+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
>new file mode 100644
>index 0000000000..32538b558e
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
>@@ -0,0 +1,3 @@
>+/usr/sbin/bhyveload \
>+-m 214 \
>+-d /tmp/freebsd.img bhyve
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
>new file mode 100644
>index 0000000000..796903a9fa
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
>@@ -0,0 +1,26 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory>219136</memory>
>+  <vcpu>1</vcpu>
>+  <os>
>+    <type>hvm</type>
>+  </os>
>+  <devices>
>+    <controller type='isa' index='1' model='isa-bridge'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>+    </controller>
>+    <disk type='file'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <model type='virtio'/>
>+      <source bridge="virbr0"/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
>new file mode 100644
>index 0000000000..ee833eb460
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
>@@ -0,0 +1,10 @@
>+/usr/sbin/bhyve \
>+-c 1 \
>+-m 214 \
>+-u \
>+-H \
>+-P \
>+-s 0:0,hostbridge \
>+-s 31:0,lpc \
>+-s 2:0,ahci,hd:/tmp/freebsd.img \
>+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
>new file mode 100644
>index 0000000000..32538b558e
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
>@@ -0,0 +1,3 @@
>+/usr/sbin/bhyveload \
>+-m 214 \
>+-d /tmp/freebsd.img bhyve
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
>new file mode 100644
>index 0000000000..65d7480db6
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
>@@ -0,0 +1,26 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory>219136</memory>
>+  <vcpu>1</vcpu>
>+  <os>
>+    <type>hvm</type>
>+  </os>
>+  <devices>
>+    <controller type='isa' index='1' model='isa-bridge'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/>
>+    </controller>
>+    <disk type='file'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <model type='virtio'/>
>+      <source bridge="virbr0"/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>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
>new file mode 100644
>index 0000000000..88ad9aebb7
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
>@@ -0,0 +1,23 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory>219136</memory>
>+  <vcpu>1</vcpu>
>+  <os>
>+    <type>hvm</type>
>+  </os>
>+  <devices>
>+    <disk type='file'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <model type='virtio'/>
>+      <source bridge="virbr0"/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
>new file mode 100644
>index 0000000000..910d1bbcfa
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
>@@ -0,0 +1,10 @@
>+/usr/sbin/bhyve \
>+-c 1 \
>+-m 214 \
>+-u \
>+-H \
>+-P \
>+-s 0:0,hostbridge \
>+-s 1:0,lpc \
>+-s 2:0,ahci,hd:/tmp/freebsd.img \
>+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
>new file mode 100644
>index 0000000000..32538b558e
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
>@@ -0,0 +1,3 @@
>+/usr/sbin/bhyveload \
>+-m 214 \
>+-d /tmp/freebsd.img bhyve
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
>new file mode 100644
>index 0000000000..71265ea32c
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
>@@ -0,0 +1,24 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory>219136</memory>
>+  <vcpu>1</vcpu>
>+  <os>
>+    <type>hvm</type>
>+  </os>
>+  <devices>
>+    <controller type='isa' index='1' model='isa-bridge'/>
>+    <disk type='file'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <model type='virtio'/>
>+      <source bridge="virbr0"/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
>new file mode 100644
>index 0000000000..25622ce78a
>--- /dev/null
>+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
>@@ -0,0 +1,25 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory>219136</memory>
>+  <vcpu>1</vcpu>
>+  <os>
>+    <type>hvm</type>
>+  </os>
>+  <devices>
>+    <controller type='isa' index='1' model='isa-bridge'/>
>+    <controller type='isa' index='2' model='isa-bridge'/>
>+    <disk type='file'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <model type='virtio'/>
>+      <source bridge="virbr0"/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
>index 4a7f65a8e2..d120bd43ca 100644
>--- a/tests/bhyvexml2argvtest.c
>+++ b/tests/bhyvexml2argvtest.c
>@@ -211,6 +211,8 @@ mymain(void)
>     DO_TEST("cputopology");
>     DO_TEST_FAILURE("cputopology-nvcpu-mismatch");
>     DO_TEST("commandline");
>+    DO_TEST("isa-controller");
>+    DO_TEST_FAILURE("isa-multiple-controllers");
>
>     /* Address allocation tests */
>     DO_TEST("addr-single-sata-disk");
>@@ -218,6 +220,9 @@ mymain(void)
>     DO_TEST("addr-more-than-32-sata-disks");
>     DO_TEST("addr-single-virtio-disk");
>     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");
>
>     /* The same without 32 devs per controller support */
>     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;
>diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
>new file mode 100644
>index 0000000000..db84185fe6
>--- /dev/null
>+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
>@@ -0,0 +1,36 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory unit='KiB'>219136</memory>
>+  <currentMemory unit='KiB'>219136</currentMemory>
>+  <vcpu placement='static'>1</vcpu>
>+  <os>
>+    <type arch='x86_64'>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>
>+    <disk type='file' device='disk'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <controller type='isa' index='1' model='isa-bridge'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>+    </controller>
>+    <controller type='pci' index='0' model='pci-root'/>
>+    <controller type='sata' index='0'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
>+    </controller>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <source bridge='virbr0'/>
>+      <model type='virtio'/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
>new file mode 100644
>index 0000000000..6ab5e6ad1a
>--- /dev/null
>+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
>@@ -0,0 +1,36 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory unit='KiB'>219136</memory>
>+  <currentMemory unit='KiB'>219136</currentMemory>
>+  <vcpu placement='static'>1</vcpu>
>+  <os>
>+    <type arch='x86_64'>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>
>+    <disk type='file' device='disk'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <controller type='isa' index='1' model='isa-bridge'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/>
>+    </controller>
>+    <controller type='pci' index='0' model='pci-root'/>
>+    <controller type='sata' index='0'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
>+    </controller>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <source bridge='virbr0'/>
>+      <model type='virtio'/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
>new file mode 100644
>index 0000000000..db84185fe6
>--- /dev/null
>+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
>@@ -0,0 +1,36 @@
>+<domain type='bhyve'>
>+  <name>bhyve</name>
>+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>+  <memory unit='KiB'>219136</memory>
>+  <currentMemory unit='KiB'>219136</currentMemory>
>+  <vcpu placement='static'>1</vcpu>
>+  <os>
>+    <type arch='x86_64'>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>
>+    <disk type='file' device='disk'>
>+      <driver name='file' type='raw'/>
>+      <source file='/tmp/freebsd.img'/>
>+      <target dev='hda' bus='sata'/>
>+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
>+    </disk>
>+    <controller type='isa' index='1' model='isa-bridge'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
>+    </controller>
>+    <controller type='pci' index='0' model='pci-root'/>
>+    <controller type='sata' index='0'>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
>+    </controller>
>+    <interface type='bridge'>
>+      <mac address='52:54:00:b9:94:02'/>
>+      <source bridge='virbr0'/>
>+      <model type='virtio'/>
>+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>+    </interface>
>+  </devices>
>+</domain>
>diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
>index ed421b8839..4b7f59ea29 100644
>--- a/tests/bhyvexml2xmltest.c
>+++ b/tests/bhyvexml2xmltest.c
>@@ -111,6 +111,7 @@ mymain(void)
>     DO_TEST_DIFFERENT("vnc-vgaconf-io");
>     DO_TEST_DIFFERENT("vnc-autoport");
>     DO_TEST_DIFFERENT("commandline");
>+    DO_TEST_DIFFERENT("isa-controller");
>
>     /* Address allocation tests */
>     DO_TEST_DIFFERENT("addr-single-sata-disk");
>@@ -118,6 +119,8 @@ mymain(void)
>     DO_TEST_DIFFERENT("addr-more-than-32-sata-disks");
>     DO_TEST_DIFFERENT("addr-single-virtio-disk");
>     DO_TEST_DIFFERENT("addr-multiple-virtio-disks");
>+    DO_TEST_DIFFERENT("addr-isa-controller-on-slot-1");
>+    DO_TEST_DIFFERENT("addr-isa-controller-on-slot-31");
>
>     /* The same without 32 devs per controller support */
>     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;
>-- 
>2.20.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] bhyve: support 'isa' controller for LPC
Posted by Roman Bogorodskiy 6 years, 11 months ago 
  Ján Tomko wrote:

> On Sun, Feb 17, 2019 at 05:04:02PM +0400, Roman Bogorodskiy wrote:
> >Support modeling of the 'isa' controller for bhyve. When controller is
> >not present in the domain XML, but domain requires it to be there (e.g.
> >because bootrom is used), implicitly add addition of this controller to
> >the command line arguments, and bind it to PCI slot 1.
> >
> >PCI slot 1 is always reserved still. User can manually define any PCI
> >slot for the 'isa' controller, including PCI slot 1, but other devices
> >are not allowed to use this address.
> >
> 
> I thought one of the points was to allow the use of slot 1 for users
> who request it. But this also works - the restriction can be relaxed
> later if needed.

The main goal for now is to allow to choose PCI slot for the LPC
controller. My plan is to relax it later indeed.

> >Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> >---
> > src/bhyve/bhyve_command.c                     | 18 +++++++++-
> > src/bhyve/bhyve_device.c                      | 25 ++++++++++---
> > ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++++++
> > ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
> > ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++++++++++++++
> > ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++++++
> > ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
> > ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++++++++++++++
> > ...argv-addr-non-isa-controller-on-slot-1.xml | 23 ++++++++++++
> > .../bhyvexml2argv-isa-controller.args         | 10 ++++++
> > .../bhyvexml2argv-isa-controller.ldargs       |  3 ++
> > .../bhyvexml2argv-isa-controller.xml          | 24 +++++++++++++
> > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +++++++++++++
> > tests/bhyvexml2argvtest.c                     |  5 +++
> > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++++++++++++++++++
> > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++++++++++++++++++
> > .../bhyvexml2xmlout-isa-controller.xml        | 36 +++++++++++++++++++
> > tests/bhyvexml2xmltest.c                      |  3 ++
> > 18 files changed, 317 insertions(+), 5 deletions(-)
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
> > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
> >
> >diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> >index f49dc77118..2c90265a93 100644
> >--- a/src/bhyve/bhyve_command.c
> >+++ b/src/bhyve/bhyve_command.c
> >@@ -461,6 +461,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
> >      */
> >     size_t i;
> >     int nusbcontrollers = 0;
> >+    int nisacontrollers = 0;
> >     unsigned int nvcpus = virDomainDefGetVcpus(def);
> >
> >     virCommandPtr cmd = virCommandNew(BHYVE);
> >@@ -581,6 +582,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
> >                 if (bhyveBuildUSBControllerArgStr(def, controller, cmd) < 0)
> >                     goto error;
> >                 break;
> >+        case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
> >+                if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) {
> >+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >+                                       "%s", _("unsupported ISA controller model: only ISA bridge supported"));
> >+                        goto error;
> >+                }
> >+                if (++nisacontrollers > 1) {
> >+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >+                                       "%s", _("only single ISA controller is supported"));
> >+                        goto error;
> >+                }
> 
> This error message can be reported much sooner - since we already forbid
> multiple controllers with the same index, all you need to do is forbid
> non-zero index virDomainControllerDefValidate

Do you mean implementing this check in virDomainDefParserConfig.deviceValidateCallback
of the bhyve driver?
> 
> Also, this whole switch is indented by 8 instead of 4 spaces.
> 
> >+                virCommandAddArg(cmd, "-s");
> >+                virCommandAddArgFormat(cmd, "%d:0,lpc",
> >+                                       controller->info.addr.pci.slot);
> >+                break;
> >         }
> >     }
> >     for (i = 0; i < def->nnets; i++) {
> >@@ -618,7 +634,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
> >         }
> >     }
> >
> >-    if (bhyveDomainDefNeedsISAController(def))
> >+    if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def))
> >         bhyveBuildLPCArgStr(def, cmd);
> >
> >     if (bhyveBuildConsoleArgStr(def, cmd) < 0)
> >diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> >index 201044d9e6..42093afb7b 100644
> >--- a/src/bhyve/bhyve_device.c
> >+++ b/src/bhyve/bhyve_device.c
> >@@ -47,10 +47,17 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
> >         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 (!((device->type == VIR_DOMAIN_DEVICE_CONTROLLER) &&
> >+                 (device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
> 
> == has higher precedence than &&, the inner parentheses can be dropped
> 
> >+                 (device->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE))) {
> >+                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;
> >+            }
> >         }
> >     }
> >
> >@@ -103,6 +110,16 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
> >         goto error;
> >     }
> >
> >+    for (i = 0; i < def->ncontrollers; i++) {
> >+         if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
> >+             (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) &&
> >+              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) ||
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> >new file mode 100644
> >index 0000000000..910d1bbcfa
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> >@@ -0,0 +1,10 @@
> >+/usr/sbin/bhyve \
> >+-c 1 \
> >+-m 214 \
> >+-u \
> >+-H \
> >+-P \
> >+-s 0:0,hostbridge \
> >+-s 1:0,lpc \
> >+-s 2:0,ahci,hd:/tmp/freebsd.img \
> >+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> >new file mode 100644
> >index 0000000000..32538b558e
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> >@@ -0,0 +1,3 @@
> >+/usr/sbin/bhyveload \
> >+-m 214 \
> >+-d /tmp/freebsd.img bhyve
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> >new file mode 100644
> >index 0000000000..796903a9fa
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> >@@ -0,0 +1,26 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory>219136</memory>
> >+  <vcpu>1</vcpu>
> >+  <os>
> >+    <type>hvm</type>
> >+  </os>
> >+  <devices>
> >+    <controller type='isa' index='1' model='isa-bridge'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> >+    </controller>
> >+    <disk type='file'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <model type='virtio'/>
> >+      <source bridge="virbr0"/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> >new file mode 100644
> >index 0000000000..ee833eb460
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> >@@ -0,0 +1,10 @@
> >+/usr/sbin/bhyve \
> >+-c 1 \
> >+-m 214 \
> >+-u \
> >+-H \
> >+-P \
> >+-s 0:0,hostbridge \
> >+-s 31:0,lpc \
> >+-s 2:0,ahci,hd:/tmp/freebsd.img \
> >+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> >new file mode 100644
> >index 0000000000..32538b558e
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> >@@ -0,0 +1,3 @@
> >+/usr/sbin/bhyveload \
> >+-m 214 \
> >+-d /tmp/freebsd.img bhyve
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> >new file mode 100644
> >index 0000000000..65d7480db6
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> >@@ -0,0 +1,26 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory>219136</memory>
> >+  <vcpu>1</vcpu>
> >+  <os>
> >+    <type>hvm</type>
> >+  </os>
> >+  <devices>
> >+    <controller type='isa' index='1' model='isa-bridge'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/>
> >+    </controller>
> >+    <disk type='file'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <model type='virtio'/>
> >+      <source bridge="virbr0"/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >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
> >new file mode 100644
> >index 0000000000..88ad9aebb7
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> >@@ -0,0 +1,23 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory>219136</memory>
> >+  <vcpu>1</vcpu>
> >+  <os>
> >+    <type>hvm</type>
> >+  </os>
> >+  <devices>
> >+    <disk type='file'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <model type='virtio'/>
> >+      <source bridge="virbr0"/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> >new file mode 100644
> >index 0000000000..910d1bbcfa
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> >@@ -0,0 +1,10 @@
> >+/usr/sbin/bhyve \
> >+-c 1 \
> >+-m 214 \
> >+-u \
> >+-H \
> >+-P \
> >+-s 0:0,hostbridge \
> >+-s 1:0,lpc \
> >+-s 2:0,ahci,hd:/tmp/freebsd.img \
> >+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> >new file mode 100644
> >index 0000000000..32538b558e
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> >@@ -0,0 +1,3 @@
> >+/usr/sbin/bhyveload \
> >+-m 214 \
> >+-d /tmp/freebsd.img bhyve
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> >new file mode 100644
> >index 0000000000..71265ea32c
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> >@@ -0,0 +1,24 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory>219136</memory>
> >+  <vcpu>1</vcpu>
> >+  <os>
> >+    <type>hvm</type>
> >+  </os>
> >+  <devices>
> >+    <controller type='isa' index='1' model='isa-bridge'/>
> >+    <disk type='file'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <model type='virtio'/>
> >+      <source bridge="virbr0"/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> >new file mode 100644
> >index 0000000000..25622ce78a
> >--- /dev/null
> >+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> >@@ -0,0 +1,25 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory>219136</memory>
> >+  <vcpu>1</vcpu>
> >+  <os>
> >+    <type>hvm</type>
> >+  </os>
> >+  <devices>
> >+    <controller type='isa' index='1' model='isa-bridge'/>
> >+    <controller type='isa' index='2' model='isa-bridge'/>
> >+    <disk type='file'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <model type='virtio'/>
> >+      <source bridge="virbr0"/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
> >index 4a7f65a8e2..d120bd43ca 100644
> >--- a/tests/bhyvexml2argvtest.c
> >+++ b/tests/bhyvexml2argvtest.c
> >@@ -211,6 +211,8 @@ mymain(void)
> >     DO_TEST("cputopology");
> >     DO_TEST_FAILURE("cputopology-nvcpu-mismatch");
> >     DO_TEST("commandline");
> >+    DO_TEST("isa-controller");
> >+    DO_TEST_FAILURE("isa-multiple-controllers");
> >
> >     /* Address allocation tests */
> >     DO_TEST("addr-single-sata-disk");
> >@@ -218,6 +220,9 @@ mymain(void)
> >     DO_TEST("addr-more-than-32-sata-disks");
> >     DO_TEST("addr-single-virtio-disk");
> >     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");
> >
> >     /* The same without 32 devs per controller support */
> >     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;
> >diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> >new file mode 100644
> >index 0000000000..db84185fe6
> >--- /dev/null
> >+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> >@@ -0,0 +1,36 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory unit='KiB'>219136</memory>
> >+  <currentMemory unit='KiB'>219136</currentMemory>
> >+  <vcpu placement='static'>1</vcpu>
> >+  <os>
> >+    <type arch='x86_64'>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>
> >+    <disk type='file' device='disk'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <controller type='isa' index='1' model='isa-bridge'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> >+    </controller>
> >+    <controller type='pci' index='0' model='pci-root'/>
> >+    <controller type='sata' index='0'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> >+    </controller>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <source bridge='virbr0'/>
> >+      <model type='virtio'/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
> >new file mode 100644
> >index 0000000000..6ab5e6ad1a
> >--- /dev/null
> >+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
> >@@ -0,0 +1,36 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory unit='KiB'>219136</memory>
> >+  <currentMemory unit='KiB'>219136</currentMemory>
> >+  <vcpu placement='static'>1</vcpu>
> >+  <os>
> >+    <type arch='x86_64'>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>
> >+    <disk type='file' device='disk'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <controller type='isa' index='1' model='isa-bridge'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/>
> >+    </controller>
> >+    <controller type='pci' index='0' model='pci-root'/>
> >+    <controller type='sata' index='0'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> >+    </controller>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <source bridge='virbr0'/>
> >+      <model type='virtio'/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
> >new file mode 100644
> >index 0000000000..db84185fe6
> >--- /dev/null
> >+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
> >@@ -0,0 +1,36 @@
> >+<domain type='bhyve'>
> >+  <name>bhyve</name>
> >+  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> >+  <memory unit='KiB'>219136</memory>
> >+  <currentMemory unit='KiB'>219136</currentMemory>
> >+  <vcpu placement='static'>1</vcpu>
> >+  <os>
> >+    <type arch='x86_64'>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>
> >+    <disk type='file' device='disk'>
> >+      <driver name='file' type='raw'/>
> >+      <source file='/tmp/freebsd.img'/>
> >+      <target dev='hda' bus='sata'/>
> >+      <address type='drive' controller='0' bus='0' target='2' unit='0'/>
> >+    </disk>
> >+    <controller type='isa' index='1' model='isa-bridge'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> >+    </controller>
> >+    <controller type='pci' index='0' model='pci-root'/>
> >+    <controller type='sata' index='0'>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> >+    </controller>
> >+    <interface type='bridge'>
> >+      <mac address='52:54:00:b9:94:02'/>
> >+      <source bridge='virbr0'/>
> >+      <model type='virtio'/>
> >+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >+    </interface>
> >+  </devices>
> >+</domain>
> >diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
> >index ed421b8839..4b7f59ea29 100644
> >--- a/tests/bhyvexml2xmltest.c
> >+++ b/tests/bhyvexml2xmltest.c
> >@@ -111,6 +111,7 @@ mymain(void)
> >     DO_TEST_DIFFERENT("vnc-vgaconf-io");
> >     DO_TEST_DIFFERENT("vnc-autoport");
> >     DO_TEST_DIFFERENT("commandline");
> >+    DO_TEST_DIFFERENT("isa-controller");
> >
> >     /* Address allocation tests */
> >     DO_TEST_DIFFERENT("addr-single-sata-disk");
> >@@ -118,6 +119,8 @@ mymain(void)
> >     DO_TEST_DIFFERENT("addr-more-than-32-sata-disks");
> >     DO_TEST_DIFFERENT("addr-single-virtio-disk");
> >     DO_TEST_DIFFERENT("addr-multiple-virtio-disks");
> >+    DO_TEST_DIFFERENT("addr-isa-controller-on-slot-1");
> >+    DO_TEST_DIFFERENT("addr-isa-controller-on-slot-31");
> >
> >     /* The same without 32 devs per controller support */
> >     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;
> >-- 
> >2.20.1
> >
> >--
> >libvir-list mailing list
> >libvir-list@redhat.com
> >https://www.redhat.com/mailman/listinfo/libvir-list



Roman Bogorodskiy
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] bhyve: support 'isa' controller for LPC
Posted by Ján Tomko 6 years, 11 months ago 
On Sun, Mar 03, 2019 at 06:00:32PM +0400, Roman Bogorodskiy wrote:
>  Ján Tomko wrote:
>
>> On Sun, Feb 17, 2019 at 05:04:02PM +0400, Roman Bogorodskiy wrote:
>> >Support modeling of the 'isa' controller for bhyve. When controller is
>> >not present in the domain XML, but domain requires it to be there (e.g.
>> >because bootrom is used), implicitly add addition of this controller to
>> >the command line arguments, and bind it to PCI slot 1.
>> >
>> >PCI slot 1 is always reserved still. User can manually define any PCI
>> >slot for the 'isa' controller, including PCI slot 1, but other devices
>> >are not allowed to use this address.
>> >
>>
>> I thought one of the points was to allow the use of slot 1 for users
>> who request it. But this also works - the restriction can be relaxed
>> later if needed.
>
>The main goal for now is to allow to choose PCI slot for the LPC
>controller. My plan is to relax it later indeed.
>
>> >Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
>> >---
>> > src/bhyve/bhyve_command.c                     | 18 +++++++++-
>> > src/bhyve/bhyve_device.c                      | 25 ++++++++++---
>> > ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++++++
>> > ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
>> > ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++++++++++++++
>> > ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++++++
>> > ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
>> > ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++++++++++++++
>> > ...argv-addr-non-isa-controller-on-slot-1.xml | 23 ++++++++++++
>> > .../bhyvexml2argv-isa-controller.args         | 10 ++++++
>> > .../bhyvexml2argv-isa-controller.ldargs       |  3 ++
>> > .../bhyvexml2argv-isa-controller.xml          | 24 +++++++++++++
>> > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +++++++++++++
>> > tests/bhyvexml2argvtest.c                     |  5 +++
>> > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++++++++++++++++++
>> > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++++++++++++++++++
>> > .../bhyvexml2xmlout-isa-controller.xml        | 36 +++++++++++++++++++
>> > tests/bhyvexml2xmltest.c                      |  3 ++
>> > 18 files changed, 317 insertions(+), 5 deletions(-)
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
>> > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
>> > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
>> > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
>> > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
>> >
>> >diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
>> >index f49dc77118..2c90265a93 100644
>> >--- a/src/bhyve/bhyve_command.c
>> >+++ b/src/bhyve/bhyve_command.c
>> >@@ -461,6 +461,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
>> >      */
>> >     size_t i;
>> >     int nusbcontrollers = 0;
>> >+    int nisacontrollers = 0;
>> >     unsigned int nvcpus = virDomainDefGetVcpus(def);
>> >
>> >     virCommandPtr cmd = virCommandNew(BHYVE);
>> >@@ -581,6 +582,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
>> >                 if (bhyveBuildUSBControllerArgStr(def, controller, cmd) < 0)
>> >                     goto error;
>> >                 break;
>> >+        case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
>> >+                if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) {
>> >+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> >+                                       "%s", _("unsupported ISA controller model: only ISA bridge supported"));
>> >+                        goto error;
>> >+                }
>> >+                if (++nisacontrollers > 1) {
>> >+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> >+                                       "%s", _("only single ISA controller is supported"));
>> >+                        goto error;
>> >+                }
>>
>> This error message can be reported much sooner - since we already forbid
>> multiple controllers with the same index, all you need to do is forbid
>> non-zero index virDomainControllerDefValidate
>
>Do you mean implementing this check in virDomainDefParserConfig.deviceValidateCallback
>of the bhyve driver?

I meant the validation function common for all drivers, because I
thought a driver supporting more than one ISA controller is unlikely.

But since bhyve is the only one modeling it, bhyve's validate callback
works too. Hopefully we won't have to model it in other drivers anyway.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.