On 06/17/2016 08:43 AM, Andrea Bolognani wrote:
There has been some progress lately in enabling virtio-pci on
aarch64 guests; however, guest OS support is still spotty at best,
so most guests are going to be using virtio-mmio instead.
Currently, mach-virt guests are closely modeled after q35 guests,
and that includes always adding a dmi-to-pci-bridge that's just
impossible to get rid of.
Yeah. Sorry my simple patches from yesterday didn't get you as far as
you needed to go.
While that's acceptable (if suboptimal)
for q35, where you will always need some kind of PCI device anyway,
mach-virt guests should be allowed to avoid it.
---
src/qemu/qemu_domain.c | 8 ++++++--
src/qemu/qemu_domain_address.c | 8 +++++++-
.../qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args | 1 -
.../qemuxml2argv-aarch64-virtio-pci-default.args | 1 -
.../qemuxml2argv-aarch64-virtio-pci-manual-addresses.args | 5 +++--
.../qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml | 14 ++++++++++++--
.../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 4 ----
.../qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml | 13 +++++++++----
8 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1eb5644..595ad64 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1951,11 +1951,15 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) {
goto cleanup;
}
- /* add a dmi-to-pci-bridge and a pci-bridge if there are no pci controllers
+ /* Add a dmi-to-pci-bridge bridge if there are no PCI controllers
That should have been in my patch yesterday, but too late for that now!
Thanks for fixing it.
* other than the pcie-root. This is so that there will be hot-pluggable
- * PCI slots available
+ * PCI slots available.
+ *
+ * We skip this step for aarch64 mach-virt guests, where we want to
+ * be able to have a pure virtio-mmio topology
*/
if (virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1) < 0 &&
+ !qemuDomainMachineIsVirt(def) &&
You're assuming that the only virt* machinetypes will be aarch64, which
may be reasonable now, but not in the future (periodically someone from
qemu will mention the idea of a "virt" machinetype for x86, which is
legacy-free and accepts only virtio devices). Wouldn't a more specific
comparison be better here (and in the other places in this patch)?
!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE))
goto cleanup;
Okay, so pcie-root is still always "there", which isn't a problem since
the presence of pcie-root in the config doesn't actually change anything
in the qemu commandline or resulting virtual machine (it's a default
device in qemu that can't be removed).
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index ca3adc0..883264a 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1483,9 +1483,15 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
}
/* Reserve 1 extra slot for a (potential) bridge only if buses
- * are not fully reserved yet
+ * are not fully reserved yet.
+ *
+ * We don't reserve the extra slot for aarch64 mach-virt guests
+ * either because we want to be able to have pure virtio-mmio
+ * guests, and reserving this slot would force us to add at least
+ * a dmi-to-pci-bridge to an otherwise PCI-free topology
*/
if (!buses_reserved &&
+ !qemuDomainMachineIsVirt(def) &&
virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0)
goto cleanup;
You could save some time by just putting the whole thing from "for (i =
0; i < addrs->nbuses; i++)" down to that "goto cleanup" inside an "if
(!qemuDomainMachineIsVirt(def)) { ... }". (maybe replacing
qemuDomainMachineIsVirt() with something more specific, as noted above).
As we've discussed in IRC, eventually there should be a way to disable
this for *every* platform, not just aarch64/virt. Possibly an
"openPCISlots" attribute somewhere in the domain that tells how many
slots should be left open for hotplug (with default being 1 for
x86/440fx, and up for discussion on other platforms).
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args
index 2a5702f..3e6bee9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args
@@ -21,7 +21,6 @@ QEMU_AUDIO_DRV=none \
-initrd /aarch64.initrd \
-append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \
-dtb /aarch64.dtb \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
-device virtio-serial-device,id=virtio-serial0 \
-usb \
-drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
index a2df858..566bee2 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
@@ -21,7 +21,6 @@ QEMU_AUDIO_DRV=none \
-initrd /aarch64.initrd \
-append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \
-dtb /aarch64.dtb \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
-device virtio-serial-device,id=virtio-serial0 \
-usb \
-drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args
index 0234404..4e5dbdb 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args
@@ -23,12 +23,13 @@ QEMU_AUDIO_DRV=none \
-dtb /aarch64.dtb \
-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
--device virtio-scsi-pci,id=scsi0,bus=pcie.0,addr=0x3 \
+-device pci-bridge,chassis_nr=3,id=pci.3,bus=pci.1,addr=0x1 \
+-device virtio-scsi-pci,id=scsi0,bus=pci.3,addr=0x1 \
-usb \
-drive file=/aarch64.raw,format=raw,if=none,id=drive-scsi0-0-0-0 \
-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\
id=scsi0-0-0-0 \
--device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:09:a4:37,bus=pcie.0,addr=0x2 \
+-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:09:a4:37,bus=pci.3,addr=0x2 \
-net user,vlan=0,name=hostnet0 \
-device virtio-net-pci,vlan=1,id=net1,mac=52:54:00:09:a4:38,bus=pci.2,addr=0x1 \
-net user,vlan=1,name=hostnet1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml
index bf0f249..5e1b494 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml
@@ -31,13 +31,23 @@
<target dev='sda' bus='scsi'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
+ <controller type='pci' index='0' model='pcie-root'/>
This surprised me for a minute, since you're now explicitly adding
pcie-root even though it's still implicitly added for you (and, for
example, qemuxml2argvtest completes successfully without it). But I
guess it doesn't hurt anything to have it in the file, as it makes it
obvious that the dmi-to-pci-bridge isn't just being added on top of nothing.
(Hopefully all of this will soon be a thing of the past - for *all*
arches/machinetypes if you put in a device with <address type='pci'/>
(or a device that can only be connected via PCI), then all the necessary
pci controllers should just magically appear, otherwise not).
ACK.
+ <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+ <model name='i82801b11-bridge'/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='2'/>
+ <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+ </controller>
<controller type='scsi' index='0' model='virtio-scsi'>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+ <address type='pci' domain='0x0000' bus='0x03' slot='0x01' function='0x0'/>
</controller>
<interface type='user'>
<mac address='52:54:00:09:a4:37'/>
<model type='virtio'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ <address type='pci' domain='0x0000' bus='0x03' slot='0x02' function='0x0'/>
</interface>
<interface type='user'>
<mac address='52:54:00:09:a4:38'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
index a212601..7c3fc19 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
@@ -33,10 +33,6 @@
<address type='virtio-mmio'/>
</disk>
<controller type='pci' index='0' model='pcie-root'/>
- <controller type='pci' index='1' model='dmi-to-pci-bridge'>
- <model name='i82801b11-bridge'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
- </controller>
<controller type='virtio-serial' index='0'>
<address type='virtio-mmio'/>
</controller>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml
index 4fdedac..1b50f75 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml
@@ -32,9 +32,6 @@
<target dev='sda' bus='scsi'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
- <controller type='scsi' index='0' model='virtio-scsi'>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
- </controller>
<controller type='pci' index='0' model='pcie-root'/>
<controller type='pci' index='1' model='dmi-to-pci-bridge'>
<model name='i82801b11-bridge'/>
@@ -45,10 +42,18 @@
<target chassisNr='2'/>
<address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
</controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <address type='pci' domain='0x0000' bus='0x03' slot='0x01' function='0x0'/>
+ </controller>
+ <controller type='pci' index='3' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='3'/>
+ <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/>
+ </controller>
<interface type='user'>
<mac address='52:54:00:09:a4:37'/>
<model type='virtio'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ <address type='pci' domain='0x0000' bus='0x03' slot='0x02' function='0x0'/>
</interface>
<interface type='user'>
<mac address='52:54:00:09:a4:38'/>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list