Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1438682
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
src/qemu/qemu_domain.c | 13 +++++++++--
...uxml2argv-aarch64-usb-controller-qemu-xhci.args | 20 ++++++++++++++++
...muxml2argv-aarch64-usb-controller-qemu-xhci.xml | 27 ++++++++++++++++++++++
...emuxml2argv-ppc64-usb-controller-qemu-xhci.args | 19 +++++++++++++++
...qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml | 1 +
tests/qemuxml2argvtest.c | 8 +++++++
6 files changed, 86 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args
create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2c17abaf84..d2e76dbbe4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3172,14 +3172,23 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
} else if (ARCH_IS_PPC64(def->os.arch)) {
/* To not break migration we need to set default USB controller
* for ppc64 to pci-ohci if we cannot change ABI of the VM.
- * The nec-usb-xhci controller is used as default only for
- * newly defined domains or devices. */
+ * The nec-usb-xhci or qemu-xhci controller is used as default
+ * only for newly defined domains or devices. */
if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) {
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
+ } else if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) {
cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) {
cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
}
+ } else if (def->os.arch == VIR_ARCH_AARCH64 &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) {
+ /* If possible use qemu-xhci as default controller for
+ * aarch64, it's USB3 controller and we want to avoid using
+ * nec-usb-xhci. */
+ cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
} else {
/* Default USB controller for anything else is piix3-uhci */
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args
new file mode 100644
index 0000000000..ab6284470d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.args
@@ -0,0 +1,20 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name QEMUGuest1 \
+-S \
+-M virt \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device qemu-xhci,id=usb,bus=pcie.0,addr=0x1 \
+-device virtio-balloon-pci,id=balloon0,bus=pcie.0,addr=0x6
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml
new file mode 100644
index 0000000000..ebadce1af6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml
@@ -0,0 +1,27 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='aarch64' machine='virt'>hvm</type>
+ </os>
+ <features>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-aarch64</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+ </controller>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args
new file mode 100644
index 0000000000..0f19eb7e66
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.args
@@ -0,0 +1,19 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name QEMUGuest1 \
+-S \
+-M pseries \
+-m 256 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-boot c \
+-device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml
new file mode 120000
index 0000000000..831d9d4f1c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller-qemu-xhci.xml
@@ -0,0 +1 @@
+qemuxml2argv-ppc64-usb-controller.xml
\ No newline at end of file
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e58a72120e..2f3eec0c22 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2472,6 +2472,14 @@ mymain(void)
QEMU_CAPS_PCI_OHCI);
DO_TEST("ppc64-usb-controller-legacy",
QEMU_CAPS_PIIX3_USB_UHCI);
+ DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0,
+ VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE,
+ QEMU_CAPS_DEVICE_QEMU_XHCI);
+
+ DO_TEST("aarch64-usb-controller-qemu-xhci",
+ QEMU_CAPS_OBJECT_GPEX,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_QEMU_XHCI);
DO_TEST_PARSE_FLAGS_ERROR("missing-machine",
VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS,
--
2.12.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[David and Drew added to CC, feel free to skip to the bottom]
On Thu, 2017-04-20 at 15:44 +0200, Pavel Hrdina wrote:
[...]
> + } else if (def->os.arch == VIR_ARCH_AARCH64 &&
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) {
> + /* If possible use qemu-xhci as default controller for
> + * aarch64, it's USB3 controller and we want to avoid using
> + * nec-usb-xhci. */
> + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
If qemu-xhci is not available but usb-nec-xhci is, it would
probably be nicer to pick the latter rather than leaving it
unspecified (which will result in an error later on).
So I would basically use the same code as ppc64 here, minus
the pci-ohci part of course.
[...]
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-usb-controller-qemu-xhci.xml
> @@ -0,0 +1,27 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219100</memory>
> + <currentMemory unit='KiB'>219100</currentMemory>
You don't need <currentMemory>, ...
[...]
> + <features>
> + <apic/>
> + <pae/>
> + </features>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>restart</on_crash>
... you can remove all these...
> + <devices>
> + <emulator>/usr/bin/qemu-system-aarch64</emulator>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> + </controller>
> + <memballoon model='virtio'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> + </memballoon>
... and set the model for <memballoon> to 'none'.
[...]
> @@ -2472,6 +2472,14 @@ mymain(void)
> QEMU_CAPS_PCI_OHCI);
> DO_TEST("ppc64-usb-controller-legacy",
> QEMU_CAPS_PIIX3_USB_UHCI);
> + DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0,
> + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE,
> + QEMU_CAPS_DEVICE_QEMU_XHCI);
> +
> + DO_TEST("aarch64-usb-controller-qemu-xhci",
> + QEMU_CAPS_OBJECT_GPEX,
> + QEMU_CAPS_DEVICE_IOH3420,
You don't need QEMU_CAPS_DEVICE_IOH3420 here.
The rest looks good, but I'd like to make sure changing the
default is something that's okay with everyone.
David, Drew, do you have anything against changing the
default USB controller model for ppc64 and aarch64 guests
respectively to qemu-xhci?
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[*actually* added David and Drew to CC smh] On Wed, 2017-04-26 at 18:13 +0200, Andrea Bolognani wrote: > The rest looks good, but I'd like to make sure changing the > default is something that's okay with everyone. > > David, Drew, do you have anything against changing the > default USB controller model for ppc64 and aarch64 guests > respectively to qemu-xhci? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 26 Apr 2017 18:20:24 +0200 Andrea Bolognani <abologna@redhat.com> wrote: > [*actually* added David and Drew to CC smh] > > On Wed, 2017-04-26 at 18:13 +0200, Andrea Bolognani wrote: > > The rest looks good, but I'd like to make sure changing the > > default is something that's okay with everyone. > > > > David, Drew, do you have anything against changing the > > default USB controller model for ppc64 and aarch64 guests > > respectively to qemu-xhci? No objection here. We do want to make the decision one way or another ASAP, so QE know what to test (they've already asked about testing nec-xhci versus qemu-xhci). -- David Gibson <dgibson@redhat.com> Principal Software Engineer, Virtualization, Red Hat -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Apr 27, 2017 at 11:14:42AM +1000, David Gibson wrote: > On Wed, 26 Apr 2017 18:20:24 +0200 > Andrea Bolognani <abologna@redhat.com> wrote: > > > [*actually* added David and Drew to CC smh] > > > > On Wed, 2017-04-26 at 18:13 +0200, Andrea Bolognani wrote: > > > The rest looks good, but I'd like to make sure changing the > > > default is something that's okay with everyone. > > > > > > David, Drew, do you have anything against changing the > > > default USB controller model for ppc64 and aarch64 guests > > > respectively to qemu-xhci? > > No objection here. We do want to make the decision one way or another > ASAP, so QE know what to test (they've already asked about testing > nec-xhci versus qemu-xhci). No objection, and actually the contrary. I would object to using nec-xhci on mach-virt, when the more general qemu-xhci is available. Thanks, drew -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 26, 2017 at 06:13:44PM +0200, Andrea Bolognani wrote:
> [David and Drew added to CC, feel free to skip to the bottom]
>
> On Thu, 2017-04-20 at 15:44 +0200, Pavel Hrdina wrote:
> [...]
> > + } else if (def->os.arch == VIR_ARCH_AARCH64 &&
> > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) {
> > + /* If possible use qemu-xhci as default controller for
> > + * aarch64, it's USB3 controller and we want to avoid using
> > + * nec-usb-xhci. */
> > + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
>
> If qemu-xhci is not available but usb-nec-xhci is, it would
> probably be nicer to pick the latter rather than leaving it
> unspecified (which will result in an error later on).
This is not true, if the qemu-xhci is not available libvirt will set
piix3-uhci if available as a default for aarch64. Only in case that
piix3-uhci is not available it will fail.
> So I would basically use the same code as ppc64 here, minus
> the pci-ohci part of course.
That should be addressed by a separate patch.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2017-04-26 at 18:59 +0200, Pavel Hrdina wrote: > > If qemu-xhci is not available but usb-nec-xhci is, it would > > probably be nicer to pick the latter rather than leaving it > > unspecified (which will result in an error later on). > > This is not true, if the qemu-xhci is not available libvirt will set > piix3-uhci if available as a default for aarch64. Only in case that > piix3-uhci is not available it will fail. You're technically correct[1]. However, piix3-uhci is another piece of Intel-derived hardware so in practice qemu-system-aarch64 is very unlikely to have it compiled in and most users will end up getting the error instead. > > So I would basically use the same code as ppc64 here, minus > > the pci-ohci part of course. > > That should be addressed by a separate patch. Agreed. I suggest adding a patch at the beginning of the series that makes nec-xhci the default for aarch64 if available as in ppc64, and then make qemu-xhci the new default for both in this patch. Would that work for you? [1] Which is the best kind of correct! ;) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Apr 27, 2017 at 09:42:24AM +0200, Andrea Bolognani wrote: > On Wed, 2017-04-26 at 18:59 +0200, Pavel Hrdina wrote: > > > If qemu-xhci is not available but usb-nec-xhci is, it would > > > probably be nicer to pick the latter rather than leaving it > > > unspecified (which will result in an error later on). > > > > This is not true, if the qemu-xhci is not available libvirt will set > > piix3-uhci if available as a default for aarch64. Only in case that > > piix3-uhci is not available it will fail. > > You're technically correct[1]. However, piix3-uhci is > another piece of Intel-derived hardware so in practice > qemu-system-aarch64 is very unlikely to have it compiled > in and most users will end up getting the error instead. Isn't the nec-xhci also Intel hardware, so the same would apply to that controller as well. Moreover, this probably happens only for downstream builds of QEMU (most likely only RHEL/CentOS) as there is no configure option for that. QEMU has some default configs for different architectures but in upstream QEMU the set of UHCI usb controllers is enabled by default for aarch64. > > > So I would basically use the same code as ppc64 here, minus > > > the pci-ohci part of course. > > > > That should be addressed by a separate patch. > > Agreed. I suggest adding a patch at the beginning of the > series that makes nec-xhci the default for aarch64 if > available as in ppc64, and then make qemu-xhci the new > default for both in this patch. Would that work for you? I don't mind if we add the same logic for ppc64 and aarch64, so I'll send a v2 with this change included. Thanks for review. Pavel > [1] Which is the best kind of correct! ;) > -- > Andrea Bolognani / Red Hat / Virtualization > > -- > 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
On Thu, 2017-04-27 at 10:24 +0200, Pavel Hrdina wrote: > > You're technically correct[1]. However, piix3-uhci is > > another piece of Intel-derived hardware so in practice > > qemu-system-aarch64 is very unlikely to have it compiled > > in and most users will end up getting the error instead. > > Isn't the nec-xhci also Intel hardware, so the same would apply to that > controller as well. Not quite: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] [8086:7020] NEC Corporation uPD720200 USB 3.0 Host Controller [1033:0194] The vendor for piix3-uhci is Intel Corporation, the vendor for nec-xhci is NEC Corporation. I guess both are unlikely to show up in actual aarch64 hardware, but the former is clearly x86-specific while the latter is somewhat more architecture-agnostic. > Moreover, this probably happens only for downstream > builds of QEMU (most likely only RHEL/CentOS) as there is no configure > option for that. QEMU has some default configs for different architectures > but in upstream QEMU the set of UHCI usb controllers is enabled by default > for aarch64. The upstream QEMU configuration takes the kitchen sink approach, eg. qemu-system-ppc64 will include allwinner-ahci and other devices that clearly have no place in a ppc64 guest, so I don't think we should take that as an indication that piix-uhci is something anyone will want to reasonably use on aarch64 :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Apr 27, 2017 at 04:13:43PM +0200, Andrea Bolognani wrote: > On Thu, 2017-04-27 at 10:24 +0200, Pavel Hrdina wrote: > > > You're technically correct[1]. However, piix3-uhci is > > > another piece of Intel-derived hardware so in practice > > > qemu-system-aarch64 is very unlikely to have it compiled > > > in and most users will end up getting the error instead. > > > > Isn't the nec-xhci also Intel hardware, so the same would apply to that > > controller as well. > > Not quite: > > Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] [8086:7020] > NEC Corporation uPD720200 USB 3.0 Host Controller [1033:0194] > > The vendor for piix3-uhci is Intel Corporation, the vendor > for nec-xhci is NEC Corporation. > > I guess both are unlikely to show up in actual aarch64 > hardware, but the former is clearly x86-specific while the > latter is somewhat more architecture-agnostic. > > > Moreover, this probably happens only for downstream > > builds of QEMU (most likely only RHEL/CentOS) as there is no configure > > option for that. QEMU has some default configs for different architectures > > but in upstream QEMU the set of UHCI usb controllers is enabled by default > > for aarch64. > > The upstream QEMU configuration takes the kitchen sink > approach, eg. qemu-system-ppc64 will include allwinner-ahci > and other devices that clearly have no place in a ppc64 > guest, so I don't think we should take that as an indication > that piix-uhci is something anyone will want to reasonably > use on aarch64 :) My point was that we should not make any decisions based on downstream configurations :). Like I said, I'll send v2 with nec-xhci enabled for aarch64. Pavel > > -- > Andrea Bolognani / Red Hat / Virtualization > > -- > 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
© 2016 - 2026 Red Hat, Inc.