qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin David Lunt <1859378@bugs.launchpad.net>
To: qemu-devel@nongnu.org
Subject: [Bug 1859378] Re: xhci Control Transfer requiring a Status TRB before starting transfer
Date: Mon, 13 Jan 2020 00:48:47 -0000	[thread overview]
Message-ID: <157887652722.5706.13027541162167379297.malone@gac.canonical.com> (raw)
In-Reply-To: 157887453515.27484.18206056235819884101.malonedeb@chaenomeles.canonical.com

Just a little more information.

In section 4.11.2.2, page 159 of version 1.0 of the xHCI specification,
it states:

<start quote>
• The xHC shall NOT check for the following Control transfer error conditions.
  • If a Data Stage TD follows a Setup Stage TD, where wLength = ‘0’.
  • If a Status Stage TD does not follow a Setup Stage TD, where wLength = ‘0’.
  • If a Data Stage TD does not follow a Setup Stage TD, where wLength > ‘0’21.
  • If the total size of the Data Stage TD is not equal to wLength.
  • If the Data Stage TRB Direction (DIR) flag does not correspond to the definition in Table 7.
  • If the Status Stage TRB Direction (DIR) flag does not correspond to the definition in Table 7.
• The xHC is NOT required to check for the following Control transfer error conditions. 
  If system software is properly designed these error conditions will never occur. However 
  if the xHC does check for these conditions it shall generate a Transfer Event for the TRB
  that the error was detected on with the Completion Code set to TRB Error.
  • If a Status Stage TD does not follow a Data Stage TD.
  • If the Setup Stage TRB defines a Length not = 8.
  • If the Status Stage TRB defines a Length > 0.
<end quote>

I take the first bullet in the second set as it is not required that the
STATUS TRB be on the ring at this point in time.  However, if the
controller (xHC) does check, it must place an event TRB on the Event
ring indicating an error at the SETUP TRB.

Thank you for your time,
Ben

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1859378

Title:
  xhci Control Transfer requiring a Status TRB before starting transfer

Status in QEMU:
  New

Bug description:
  This may not necessarily be a bug, but more of a change.

  A little background may need to be in order.

  With all USB Control transfers, there is a SETUP transfer, zero or
  more DATA transfers, and if successful, a STATUS transfer.  This
  STATUS transfer is used to indicate to the recipient that the previous
  transfers were successful.  For example, in a CONTROL IN transfer, the
  host sends a SETUP packet to the device, receives zero or more DATA
  packets, and then on successful transfer, the HOST sends the STATUS
  packet indicating to the device that all was received.

  If no DATA packets are received, the HOST is not to send a STATUS
  packet.  This could be due to a STALL or other error.

  With this in mind, the STATUS transfer, in this case an xHCI STATUS
  TRB, may not even be on the transfer ring yet.  The HOST software may
  be waiting for a successful transfer before it submits the STATUS
  transfer.

  However, if you look at the test at line 1701
  (https://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-xhci.c#l1701),
  the current code will not start the CONTROL transfer at all if it
  doesn't see that STATUS TRB on the ring.

  In my opinion, this is in error.  It is not required that a STATUS TRB
  be on the ring to start the CONTROL transfer.  This STATUS TRB can be
  placed on the ring after a successful SETUP and zero or more DATA
  transfers followed by a ring to the door bell.  Then after a
  successful transfer to this point, placing this STATUS TRB on the ring
  and another ring to the door bell.

  In my opinion, the check at line 1701 should be removed.

  Thank you,
  Ben

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1859378/+subscriptions


  parent reply	other threads:[~2020-01-13  1:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-13  0:15 [Bug 1859378] [NEW] xhci Control Transfer requiring a Status TRB before starting transfer Benjamin David Lunt
2020-01-13  0:30 ` [Bug 1859378] " Benjamin David Lunt
2020-01-13  0:48 ` Benjamin David Lunt [this message]
2020-01-13 12:05 ` Gerd Hoffmann
2020-01-13 13:53 ` Benjamin David Lunt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=157887652722.5706.13027541162167379297.malone@gac.canonical.com \
    --to=1859378@bugs.launchpad.net \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).