On 01/28/2016 04:14 PM, Cole Robinson wrote:
This was discussed here:
https://www.redhat.com/archives/libvir-list/2015-December/msg00217.html
The summary is that apps/users are in a tough spot WRT aarch64 -M virt
and PCIe support: qemu has all the plumbing, but most distros don't
support it yet. Presently libvirt adds a PCIe controller to the XML for
new enough qemu, but this patch drops that behavior.
I read back through all of these messages and am more thoroughly
undecided than ever about the proper thing to do.
I guess the end of that thread was that the best thing to do was (as
you're doing in this patch) to temporarily (?) not add a pcie-root
controller to a domain's XML for aarch64 -M virt, and if there is no PCI
controller, then use virtio-mmio for the address of all devices; but if
a pcie-root is manually added, to instead use pci addresses.
My main issue with that is that the XML is then not reflecting the
hardware exactly in cases where the binary is creating a pcie-root
automatically (but we're not using it). The alternative would be to
force specifying the address type of each device though, and since that
is impractical for management apps to deal with, I guess keying off the
presence/absence of a manually-created pcie-root is the less evil of the
options :-P.
Assuming that we're all in agreement with this, ACK to this patch, but
only in combination with whatever followup is necessary to support doing
the auto-addressing based on presence of pcie-root. (I'm actually
wondering if, once we have that 2nd part figured out, it may be
necessary to combine the two patches into one in order to not break any
future bisecting to try and find bugs related to aarch64 that is using
devices with PCI addresses, but we'll get to that later...)
Upcoming patches will instead require users to manually specify a
pcie controller in the XML as a way of telling libvirt 'the OS supports
PCI', at which point libvirt can use it as a target for virtio-pci.
---
src/qemu/qemu_domain.c | 4 ----
.../qemuxml2argv-aarch64-virtio-pci-default.args | 2 --
tests/qemuxml2argvtest.c | 6 ++----
.../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 10 ----------
tests/qemuxml2xmltest.c | 2 ++
5 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1df1b74..aea1ea5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1109,10 +1109,6 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
- if (STREQ(def->os.machine, "virt") ||
- STRPREFIX(def->os.machine, "virt-")) {
- addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX);
- }
break;
case VIR_ARCH_PPC64:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
index a49bc82..f879560 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args
@@ -21,8 +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 pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index cfb46ef..b2f636e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1642,9 +1642,7 @@ mymain(void)
QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM);
/* Demonstrates the virtio-pci default... namely that there isn't any!
- q35 style PCI controllers will be added if the binary supports it,
- but virtio-mmio is always used unless PCI addresses are manually
- specified. */
+ However this might change in the future... */
DO_TEST("aarch64-virtio-pci-default",
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
QEMU_CAPS_DEVICE_VIRTIO_MMIO,
@@ -1653,7 +1651,7 @@ mymain(void)
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
/* Example of using virtio-pci with no explicit PCI controller
but with manual PCI addresses */
- DO_TEST("aarch64-virtio-pci-manual-addresses",
+ DO_TEST_PARSE_ERROR("aarch64-virtio-pci-manual-addresses",
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
QEMU_CAPS_DEVICE_VIRTIO_MMIO,
QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
index 6f1c53b..db2119c 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml
@@ -31,16 +31,6 @@
<target dev='vda' bus='virtio'/>
<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='pci' index='2' model='pci-bridge'>
- <model name='pci-bridge'/>
- <target chassisNr='2'/>
- <address type='pci' domain='0x0000' bus='0x01'
slot='0x01' function='0x0'/>
- </controller>
<controller type='virtio-serial' index='0'>
<address type='virtio-mmio'/>
</controller>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 581129c..cac401c 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -731,12 +731,14 @@ mymain(void)
QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI);
+ /*
DO_TEST_FULL("aarch64-virtio-pci-manual-addresses", WHEN_ACTIVE,
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
QEMU_CAPS_DEVICE_VIRTIO_MMIO,
QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI);
+ */
DO_TEST("aarch64-gic");
DO_TEST("aarch64-gicv3");