Fix logic for assigning PCI addresses to USB2 companion controllers

Currently each USB2 companion controller gets put on a separate
PCI slot. Not only is this wasteful of PCI slots, but it is not
in compliance with the spec for USB2 controllers. The master
echi1 and all companion controllers should be in the same slot,
with echi1 in function 7, and uhci1-3 in functions 0-2 respectively.

* src/qemu/qemu_command.c: Special case handling of USB2 controllers
  to apply correct pci slot assignment
* tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args,
  tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml: Expand
  test to cover automatic slot assignment
(cherry picked from commit 1ebd52cb871f87b7868503b28448e96d59e41d63)

Conflicts:

	tests/qemuxml2xmltest.c

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ee184c2ef0c7a74254b3ea2fd24c66e34c6fb2ba..366913b340eb98c522b02c9b59c76f45f0c4a250 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1079,8 +1079,7 @@ void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs)
 }
 
 
-int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
-                                    virDomainDeviceInfoPtr dev)
+static int qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs)
 {
     int i;
     int iteration;
@@ -1107,20 +1106,10 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
             continue;
         }
 
-        VIR_DEBUG("Allocating PCI addr %s", addr);
+        VIR_DEBUG("Found free PCI addr %s", addr);
         VIR_FREE(addr);
 
-        if (qemuDomainPCIAddressReserveSlot(addrs, i) < 0)
-            return -1;
-
-        dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-        dev->addr.pci = maybe.addr.pci;
-
-        addrs->nextslot = i + 1;
-        if (QEMU_PCI_ADDRESS_LAST_SLOT < addrs->nextslot)
-            addrs->nextslot = 0;
-
-        return 0;
+        return i;
     }
 
     qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1128,6 +1117,38 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
     return -1;
 }
 
+int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
+                                    virDomainDeviceInfoPtr dev)
+{
+    int slot = qemuDomainPCIAddressGetNextSlot(addrs);
+
+    if (slot < 0)
+        return -1;
+
+    if (qemuDomainPCIAddressReserveSlot(addrs, slot) < 0)
+        return -1;
+
+    dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+    dev->addr.pci.bus = 0;
+    dev->addr.pci.domain = 0;
+    dev->addr.pci.slot = slot;
+    dev->addr.pci.function = 0;
+
+    addrs->nextslot = slot + 1;
+    if (QEMU_PCI_ADDRESS_LAST_SLOT < addrs->nextslot)
+        addrs->nextslot = 0;
+
+    return 0;
+}
+
+
+#define IS_USB2_CONTROLLER(ctrl) \
+    (((ctrl)->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && \
+     ((ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1 || \
+      (ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1 || \
+      (ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2 || \
+      (ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3))
+
 /*
  * This assigns static PCI slots to all configured devices.
  * The ordering here is chosen to match the ordering used
@@ -1163,7 +1184,7 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
 int
 qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
 {
-    int i;
+    size_t i, j;
     bool reservedIDE = false;
     bool reservedUSB = false;
     bool reservedVGA = false;
@@ -1292,7 +1313,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
             goto error;
     }
 
-    /* Disk controllers (SCSI only for now) */
+    /* Device controllers (SCSI, USB, but not IDE, FDC or CCID) */
     for (i = 0; i < def->ncontrollers ; i++) {
         /* FDC lives behind the ISA bridge; CCID is a usb device */
         if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC ||
@@ -1307,8 +1328,58 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
 
         if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
             continue;
-        if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info) < 0)
-            goto error;
+
+        /* USB2 needs special handling to put all companions in the same slot */
+        if (IS_USB2_CONTROLLER(def->controllers[i])) {
+            virDomainDevicePCIAddress addr = { 0, 0, 0, 0, false };
+            for (j = 0 ; j < i ; j++) {
+                if (IS_USB2_CONTROLLER(def->controllers[j]) &&
+                    def->controllers[j]->idx == def->controllers[i]->idx) {
+                    addr = def->controllers[j]->info.addr.pci;
+                    break;
+                }
+            }
+
+            switch (def->controllers[i]->model) {
+            case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+                addr.function = 7;
+                break;
+            case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+                addr.function = 0;
+                addr.multi = VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON;
+                break;
+            case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
+                addr.function = 1;
+                break;
+            case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
+                addr.function = 2;
+                break;
+            }
+
+            if (addr.slot == 0) {
+                /* This is the first part of the controller, so need
+                 * to find a free slot & then reserve a function */
+                int slot = qemuDomainPCIAddressGetNextSlot(addrs);
+                if (slot < 0)
+                    goto error;
+
+                addr.slot = slot;
+                addrs->nextslot = addr.slot + 1;
+                if (QEMU_PCI_ADDRESS_LAST_SLOT < addrs->nextslot)
+                    addrs->nextslot = 0;
+            }
+            /* Finally we can reserve the slot+function */
+            if (qemuDomainPCIAddressReserveFunction(addrs,
+                                                    addr.slot,
+                                                    addr.function) < 0)
+                goto error;
+
+            def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+            def->controllers[i]->info.addr.pci = addr;
+        } else {
+            if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info) < 0)
+                goto error;
+        }
     }
 
     /* Disks (VirtIO only for now */

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args
index babd4f804a5a1a07968de9d219ef743a0a17adaa..cf070a1741ce5f29804e6fa239f086302ff686b9 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args
@@ -2,5 +2,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \
 -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
 -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \
 -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \
--device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \
--device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
+-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x3.0x7 \
+-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x3 \
+-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x3.0x2 \
+-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x3.0x1 \
+-device ich9-usb-ehci1,id=usb1,bus=pci.0,addr=0x4.0x7 \
+-device ich9-usb-uhci1,masterbus=usb1.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \
+-device ich9-usb-uhci3,masterbus=usb1.0,firstport=4,bus=pci.0,addr=0x4.0x2 \
+-device ich9-usb-uhci2,masterbus=usb1.0,firstport=2,bus=pci.0,addr=0x4.0x1 \
+-device ich9-usb-ehci1,id=usb2,bus=pci.0,addr=0x5.0x7 \
+-device ich9-usb-uhci1,masterbus=usb2.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 \
+-device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.0,addr=0x5.0x2 \
+-device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.0,addr=0x5.0x1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml
index 49a57e229117ae3c3c37b950b50915343a286349..4849dbe90d555d99ea99450e47b1f536781946a5 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml
@@ -10,8 +10,43 @@
   </os>
   <devices>
     <emulator>/usr/bin/qemu</emulator>
+    <!-- Intentionally mixed up ordering to check we assign
+         addresses to the correct matching companions -->
     <controller type='usb' index='0' model='ich9-ehci1'>
-      <address type='pci' domain='0' bus='0' slot='4' function='7'/>
+    </controller>
+    <controller type='usb' index='2' model='ich9-ehci1'>
+    </controller>
+    <controller type='usb' index='1' model='ich9-ehci1'>
+    </controller>
+
+    <controller type='usb' index='0' model='ich9-uhci1'>
+      <master startport='0'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci1'>
+      <master startport='0'/>
+    </controller>
+    <controller type='usb' index='2' model='ich9-uhci1'>
+      <master startport='0'/>
+    </controller>
+
+    <controller type='usb' index='0' model='ich9-uhci3'>
+      <master startport='4'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci3'>
+      <master startport='4'/>
+    </controller>
+    <controller type='usb' index='2' model='ich9-uhci3'>
+      <master startport='4'/>
+    </controller>
+
+    <controller type='usb' index='2' model='ich9-uhci2'>
+      <master startport='2'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci2'>
+      <master startport='2'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci2'>
+      <master startport='2'/>
     </controller>
     <memballoon model='virtio'/>
   </devices>

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-ich9-ehci-addr.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-ich9-ehci-addr.xml
new file mode 100644 (file)
index 0000000..2bb1661
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-usb-ich9-ehci-addr.xml
@@ -0,0 +1,49 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>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>
+    <emulator>/usr/bin/qemu</emulator>
+    <controller type='usb' index='0' model='ich9-ehci1'/>
+    <controller type='usb' index='0' model='ich9-uhci1'>
+      <master startport='0'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci3'>
+      <master startport='4'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci2'>
+      <master startport='2'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-ehci1'/>
+    <controller type='usb' index='1' model='ich9-uhci1'>
+      <master startport='0'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci3'>
+      <master startport='4'/>
+    </controller>
+    <controller type='usb' index='1' model='ich9-uhci2'>
+      <master startport='2'/>
+    </controller>
+    <controller type='usb' index='2' model='ich9-ehci1'/>
+    <controller type='usb' index='2' model='ich9-uhci1'>
+      <master startport='0'/>
+    </controller>
+    <controller type='usb' index='2' model='ich9-uhci3'>
+      <master startport='4'/>
+    </controller>
+    <controller type='usb' index='2' model='ich9-uhci2'>
+      <master startport='2'/>
+    </controller>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index af635d9596f770df8363ca57042adfc288f6999b..ef5ddfc477688ac5fa590ae27998b4cc08f72f42 100644 (file)
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -199,6 +199,7 @@ mymain(void)
     DO_TEST_DIFFERENT("console-virtio");
     DO_TEST_DIFFERENT("serial-target-port-auto");
     DO_TEST_DIFFERENT("graphics-listen-network2");
+    DO_TEST_DIFFERENT("usb-ich9-ehci-addr");
 
     virCapabilitiesFree(driver.caps);