Project

General

Profile

Bug #8173

workaround qemu-xhci HCIVERSION bug

Added by Michal Nowak over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Start date:
2017-05-07
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

I boot 2017.04 OpenIndiana USB image on Linux KVM QEMU 2.9.0 with qemu-xhci device and it fails to find root on the USB 3.0 device (I guess). There's, notably, this warning:

WARNING: xhci0: Encountered unsupported xHCI version 0. 0

QEMU xHCI driver's code: http://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-xhci.c;hb=HEAD. It seems to me that it implement version 1.0 of the xHCI standard, maybe is the illumox xhci driver reading it wrong?

This is how I run it:

qemu-system-x86_64 -drive if=none,id=usbstick,file=/home/newman/tmp/OI-hipster-gui-20170502.usb,format=raw -device qemu-xhci,id=xhci -device usb-storage,bus=xhci.0,port=1,drive=usbstick -m 2048 -cpu host -enable-kvm

Screenshot attached. Booting Fedora 25 in this scenario works fine.

nec-usb-xhci instead of qemu-xhci is the same, I guess they share much of the code, anyways.


Files

OI-2017.04-xHCI-QEMU.png (14.9 KB) OI-2017.04-xHCI-QEMU.png Michal Nowak, 2017-05-07 08:09 AM
trace.dump (190 KB) trace.dump Michal Nowak, 2017-05-11 07:46 AM

Related issues

Related to illumos gate - Bug #8174: VirtualBox xHCI: WARNING: /pci@0,0/pci8086,1e31@c (xhci0): Connecting device on port 1 failedNew2017-05-07

Actions

History

#1

Updated by Michal Nowak over 2 years ago

  • Related to Bug #8174: VirtualBox xHCI: WARNING: /pci@0,0/pci8086,1e31@c (xhci0): Connecting device on port 1 failed added
#2

Updated by Robert Mustacchi over 2 years ago

Thanks for reporting this. I'm taking a look at the QEMU source code. It does appear to report version 1, but what's not clear is how the emulation is supposed to handle a request to read the capability section at offset 0x2. If you look at http://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-xhci.c;hb=HEAD#l2902, it doesn't seem to have an explicit offset for 0x2, which is treated as separate registers here. I need to spend some more time looking at their emulation to understand how our request for a 2 byte read at offset two is treated here.

#3

Updated by Robert Mustacchi over 2 years ago

Michal Nowak, do you have the ability to run / deploy qemu with its tracing features? If so, I'd be curious to see if we hit either:

http://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-xhci.c;hb=HEAD#l2963

or

http://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-xhci.c;hb=HEAD#l2967

#4

Updated by Robert Mustacchi over 2 years ago

So, from looking around. I suspect this is an issue with QEMU, though we can attempt to work around in the OS. I think what's happening is that the way their logic for combining reads into the memory access means that they'll fail a read at offset 0x2, which is what we're doing for the identification information, which is why we end up getting a version of zero, which isn't what happens in hardware or VMware Fusion.

From looking at other OSes, it looks like Linux does a four byte read of the initial capacity register which is allowed, but also means they're reading reserved bits and then just mask it off. FreeBSD only seems to read the version for debug purposes; however, OpenBSD will be in the same place.

Michal, if you're able to confirm that, that'd be useful information. Depending on what we find there, we can go and take a look at working around this in the OS and also working with QEMU upstream to get it fixed.

#5

Updated by Michal Nowak over 2 years ago

Robert Mustacchi wrote:

Michal Nowak, do you have the ability to run / deploy qemu with its tracing features?

Thanks for looking into this, Robert. This time I run QEMU with -trace enable='usb_xhci*',file=MyEvents.dump. Resulting dump is attached (50 MB after extraction). Let me know if you need trace with all events (*), or anything else.

#6

Updated by Michal Nowak over 2 years ago

Redmine did not let me upload it. So, attaching smaller file with 1000 lines from the beginning and 1000 lines from the end of the original dump.

#7

Updated by Robert Mustacchi over 2 years ago

That actually had exactly what I was looking for -- thanks. That confirms it and actually shows it for another register as well. So, given this, I think we can work around this in QEMU and I may try to work with the QEMU folks to file a bug.

If I was able to put together a patch to test this, do you think that you'd be able to test that or would it help to have a full OI image put together. If that's the case, I'll have to work with some other folks to help produce that.

#8

Updated by Michal Nowak over 2 years ago

Let me know bugref if you decide to file bug to upstream QEMU, please.

Never build illumos myself before, but I guess I can challenge myself to do so :), if you provide a patch against illumos-gate.

#9

Updated by Robert Mustacchi over 2 years ago

I filed a QEMU bug: https://bugs.launchpad.net/qemu/+bug/1693050. Here's a patch. Would you mind trying this out:

diff --git a/usr/src/uts/common/io/usb/hcd/xhci/xhci.c b/usr/src/uts/common/io/usb/hcd/xhci/xhci.c
index 17d190f..b43d7df 100644
--- a/usr/src/uts/common/io/usb/hcd/xhci/xhci.c
+++ b/usr/src/uts/common/io/usb/hcd/xhci/xhci.c
@@ -1170,14 +1170,20 @@ static boolean_t
 xhci_read_params(xhci_t *xhcip)
 {
        uint8_t usb;
-       uint16_t vers;
        uint32_t struc1, struc2, struc3, cap1, cap2, pgsz;
-       uint32_t psize, pbit;
+       uint32_t psize, pbit, vers;
        xhci_capability_t *xcap;
        unsigned long ps;

+       /*
+        * While it's tempting to do a 16-bit read at offset 0x2, unfortunately,
+        * a few emulated systems don't support reading at offset 0x2 for the
+        * version. Instead we need to read the caplength register and get the
+        * upper two bytes.
+        */
+       vers = xhci_get32(xhcip, XHCI_R_CAP, XHCI_CAPLENGTH);
+       vers = XHCI_VERSION_MASK(vers);
        usb = pci_config_get8(xhcip->xhci_cfg_handle, PCI_XHCI_USBREV);
-       vers = xhci_get16(xhcip, XHCI_R_CAP, XHCI_HCIVERSION);
        struc1 = xhci_get32(xhcip, XHCI_R_CAP, XHCI_HCSPARAMS1);
        struc2 = xhci_get32(xhcip, XHCI_R_CAP, XHCI_HCSPARAMS2);
        struc3 = xhci_get32(xhcip, XHCI_R_CAP, XHCI_HCSPARAMS3);
diff --git a/usr/src/uts/common/sys/usb/hcd/xhci/xhcireg.h b/usr/src/uts/common/sys/usb/hcd/xhci/xhcireg.h
index 7e23463..a5a295d 100644
--- a/usr/src/uts/common/sys/usb/hcd/xhci/xhcireg.h
+++ b/usr/src/uts/common/sys/usb/hcd/xhci/xhcireg.h
@@ -58,6 +58,8 @@ extern "C" {
 #define        XHCI_HCIVERSION_0_9     0x0090  /* xHCI version 0.9 */
 #define        XHCI_HCIVERSION_1_0     0x0100  /* xHCI version 1.0 */

+#define        XHCI_VERSION_MASK(x)    (((x) >> 16) & 0xffff)
+
 /*
  * Structural Parameters 1 - xHCI 1.1 / 5.3.3
  */
#10

Updated by Michal Nowak over 2 years ago

Robert Mustacchi wrote:

[...] Here's a patch. Would you mind trying this out:

I just verified with emulated USB tablet connected to xHCI, that the patch against current illumos works. Thank you!

#11

Updated by Robert Mustacchi over 2 years ago

OK, great. Thank you very much for testing this. If you watch illumos-developer, I'll get this sent out for review shortly.

#12

Updated by Robert Mustacchi over 2 years ago

  • Subject changed from qemu-xhci: WARNING: xhci0: Encountered unsupported xHCI version 0. 0 to workaround qemu-xhci HCIVERSION bug
#13

Updated by Electric Monk over 2 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 6f2302d279a7dc8c31e7bae29eee66e95743b74e

commit  6f2302d279a7dc8c31e7bae29eee66e95743b74e
Author: Robert Mustacchi <rm@joyent.com>
Date:   2017-06-11T21:16:54.000Z

    8173 workaround qemu-xhci HCIVERSION bug
    Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
    Reviewed by: Igor Kozhukhov <igor@dilos.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF