[Qemu-commits] [qemu/qemu] aa6857: xhci: generate a Transfer Event for each Transfer ...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: aa6857891df614c620e6e9fc4bc4af6e0e49cafd
      
https://github.com/qemu/qemu/commit/aa6857891df614c620e6e9fc4bc4af6e0e49cafd
  Author: Laszlo Ersek <address@hidden>
  Date:   2015-03-03 (Tue, 03 Mar 2015)

  Changed paths:
    M hw/usb/hcd-xhci.c

  Log Message:
  -----------
  xhci: generate a Transfer Event for each Transfer TRB with the IOC bit set

At the moment, when the XHCI driver in edk2
(MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf) runs on QEMU, with the options

  -device nec-usb-xhci -device usb-kbd

it crashes with:

  ASSERT MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c(1759):
  TrsRing != ((void*) 0)

The crash hits in the following edk2 call sequence (all files under
MdeModulePkg/Bus/):

UsbEnumerateNewDev()                         [Usb/UsbBusDxe/UsbEnumer.c]
  UsbBuildDescTable()                        [Usb/UsbBusDxe/UsbDesc.c]
    UsbGetDevDesc()                          [Usb/UsbBusDxe/UsbDesc.c]
      UsbCtrlGetDesc(USB_REQ_GET_DESCRIPTOR) [Usb/UsbBusDxe/UsbDesc.c]
  UsbCtrlRequest()                     [Usb/UsbBusDxe/UsbDesc.c]
    UsbHcControlTransfer()             [Usb/UsbBusDxe/UsbUtility.c]
      XhcControlTransfer()             [Pci/XhciDxe/Xhci.c]
        XhcCreateUrb()                 [Pci/XhciDxe/XhciSched.c]
          XhcCreateTransferTrb()       [Pci/XhciDxe/XhciSched.c]
        XhcExecTransfer()              [Pci/XhciDxe/XhciSched.c]
          XhcCheckUrbResult()          [Pci/XhciDxe/XhciSched.c]
            //
            // look for TRB_TYPE_DATA_STAGE event [1]
            //
        //
        // Store a copy of the device descriptor, as the hub device
        // needs this info to configure endpoint. [2]
        //
  UsbSetConfig()                             [Usb/UsbBusDxe/UsbDesc.c]
    UsbCtrlRequest(USB_REQ_SET_CONFIG)       [Usb/UsbBusDxe/UsbDesc.c]
      UsbHcControlTransfer()                 [Usb/UsbBusDxe/UsbUtility.c]
  XhcControlTransfer()                 [Pci/XhciDxe/Xhci.c]
    XhcSetConfigCmd()                  [Pci/XhciDxe/XhciSched.c]
      XhcInitializeEndpointContext()   [Pci/XhciDxe/XhciSched.c]
        //
        // allocate transfer ring for the endpoint [3]
        //

USBKeyboardDriverBindingStart()              [Usb/UsbKbDxe/EfiKey.c]
  UsbIoAsyncInterruptTransfer()              [Usb/UsbBusDxe/UsbBus.c]
    UsbHcAsyncInterruptTransfer()            [Usb/UsbBusDxe/UsbUtility.c]
      XhcAsyncInterruptTransfer()            [Pci/XhciDxe/Xhci.c]
  XhcCreateUrb()                       [Pci/XhciDxe/Xhci.c]
    XhcCreateTransferTrb()             [Pci/XhciDxe/XhciSched.c]
      XhcSyncTrsRing()                 [Pci/XhciDxe/XhciSched.c]
        ASSERT (TrsRing != NULL) [4]

UsbEnumerateNewDev() in the USB bus driver issues a GET_DESCRIPTOR
request, in order to determine the number of configurations that the
endpoint supports. The requests consists of three stages (three TRBs),
setup, data, and status. The length of the response is determined in [1],
namely from the transfer event that the host controller generates in
response to the request's middle stage (ie. the data stage).

If the length of the answer is correct (a full GET_DESCRIPTOR request
takes 18 bytes), then the XHCI driver that underlies the USB bus driver
"snoops" (caches) the descriptor data for later [2].

Later, the USB bus driver sends a SET_CONFIG request. The underlying XHCI
driver allocates a transfer ring for the endpoint, relying on the data
snooped and cached in step [2].

Finally, the USB keyboard driver submits an asynchronous interrupt
transfer to manage the keyboard. As part of this it asserts [4] that the
ring has been allocated in step [3].

And this ASSERT() fires. The root cause can be found in the way QEMU
handles the initial GET_DESCRIPTOR request.

Again, that request consists of three stages (TRBs, Transfer Request
Blocks), "setup", "data", and "status". The XhcCreateTransferTrb()
function sets the IOC ("Interrupt on Completion") flag in each of these
TRBs.

According to the XHCI specification, the host controller shall generate a
Transfer Event in response to *each* individual TRB of the request that
had the IOC flag set. This means that QEMU should queue three events:
setup, data, and status, for edk2's XHCI driver.

However, QEMU only generates two events:
- one for the setup (ie. 1st) stage,
- another for the status (ie. 3rd) stage.

No event is generated for the middle (ie. data) stage. The loop in QEMU's
xhci_xfer_report() function runs three times, but due to the "reported"
variable, only the first and the last TRBs elicit events, the middle (data
stage) results in no event queued.

As a consequence:
- When handling the GET_DESCRIPTOR request, XhcCheckUrbResult() in [1]
  does not update the response length from zero.

- XhcControlTransfer() thinks that the response is invalid (it has zero
  length payload instead of 18 bytes), hence [2] is not reached; the
  device descriptor is not stashed for later, and the number of possible
  configurations is left at zero.

- When handling the SET_CONFIG request, (NumConfigurations == 0) from
  above prevents the allocation of the endpoint's transfer ring.

- When the keyboard driver tries to use the endpoint, the ASSERT() blows
  up.

The solution is to correct the emulation in QEMU, and to generate a
transfer event whenever IOC is set in a TRB.

The patch replaces

  !reported && (IOC || foo)    == !reported && IOC ||
                            !reported && foo

with

  IOC || (!reported && foo)    == IOC ||
                            !reported && foo

which only changes how

  reported && IOC

is handled. (Namely, it now generates an event.)

Tested with edk2 built for "qemu-system-aarch64 -M virt" (ie.
"ArmVirtualizationQemu.dsc", aka "AAVMF"), and guest Linux.

Signed-off-by: Laszlo Ersek <address@hidden>
Signed-off-by: Gerd Hoffmann <address@hidden>


  Commit: b6d527fbc0b64a2ba7d83623e47b05c745b88043
      
https://github.com/qemu/qemu/commit/b6d527fbc0b64a2ba7d83623e47b05c745b88043
  Author: Peter Maydell <address@hidden>
  Date:   2015-03-08 (Sun, 08 Mar 2015)

  Changed paths:
    M hw/usb/hcd-xhci.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-20150303-1' into 
staging

xhci: generate a Transfer Event for each Transfer TRB with the IOC bit set

# gpg: Signature made Tue Mar  3 07:38:43 2015 GMT using RSA key ID D3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <address@hidden>"
# gpg:                 aka "Gerd Hoffmann <address@hidden>"
# gpg:                 aka "Gerd Hoffmann (private) <address@hidden>"

* remotes/kraxel/tags/pull-usb-20150303-1:
  xhci: generate a Transfer Event for each Transfer TRB with the IOC bit set

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/fff795ab383e...b6d527fbc0b6