libpayload: xhci: Use Event Data TRBs for transfer event generation
The current XHCI code only sets IOC on the last TRB of a TD, and
doesn't set ISP anywhere. On my Synopsys DesignWare3 controller, this
won't generate an event at all when we have a short transfer that is not
on the last TRB of a TD, resulting in event ring desync and everyone
having a bad time. However, just setting ISP on other TRBs doesn't
really make for a nice solution: we then need to do ugly special casing
to fish out the spurious second transfer event you get for short
packets, and we still need a way to figure out how many bytes were
transferred. Since the Short Packet transfer event only reports
untransferred bytes for the current TRB, we would have to manually walk
the rest of the unprocessed TRB chain and add up the bytes. Check out
U-Boot and the Linux kernel to see how complicated this looks in
practice.
Now what if we had a way to just tell the HC "I want an event at exactly
*this* point in the TD, I want it to have the right completion code for
the whole TD, and to contain the exact number of bytes written"? Enter
the Event Data TRB: this little gizmo really does pretty much exactly
what any sane XHCI driver would want, and I have no idea why it isn't
used more often. It solves both the short packet event generation and
counting the transferred bytes without requiring any special magic in
software.
BUG=chrome-os-partner:21969
TEST=Manual
Change-Id: Idab412d61edf30655ec69c80066bfffd80290403
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/170980
Reviewed-by: Stefan Reinauer <reinauer@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/payloads/libpayload/drivers/usb/xhci.c b/payloads/libpayload/drivers/usb/xhci.c
index 87b716c..6a9939c 100644
--- a/payloads/libpayload/drivers/usb/xhci.c
+++ b/payloads/libpayload/drivers/usb/xhci.c
@@ -536,6 +536,7 @@
trb->ptr_low = virt_to_phys(cur_start);
TRB_SET(TL, trb, cur_length);
TRB_SET(TDS, trb, packets);
+ TRB_SET(CH, trb, 1);
/* Check for first, data stage TRB */
if (!trb_count && ep == 1) {
@@ -545,17 +546,19 @@
TRB_SET(TT, trb, TRB_NORMAL);
}
- /* Check for last TRB */
- if (!length)
- TRB_SET(IOC, trb, 1);
- else
- TRB_SET(CH, trb, 1);
-
xhci_enqueue_trb(tr);
cur_start += cur_length;
++trb_count;
}
+
+ trb = tr->cur;
+ xhci_clear_trb(trb, tr->pcs);
+ trb->ptr_low = virt_to_phys(trb); /* for easier debugging only */
+ TRB_SET(TT, trb, TRB_EVENT_DATA);
+ TRB_SET(IOC, trb, 1);
+
+ xhci_enqueue_trb(tr);
}
static int
@@ -569,7 +572,7 @@
transfer_ring_t *const tr = xhci->dev[dev->address].transfer_rings[1];
const size_t off = (size_t)data & 0xffff;
- if ((off + dalen) > ((TRANSFER_RING_SIZE - 3) << 16)) {
+ if ((off + dalen) > ((TRANSFER_RING_SIZE - 4) << 16)) {
xhci_debug("Unsupported transfer size\n");
return -1;
}
@@ -624,11 +627,11 @@
xhci->dbreg[dev->address] = 1;
/* Wait for transfer events */
- int i, residue = 0;
+ int i, transferred = 0;
const int n_stages = 2 + !!dalen;
for (i = 0; i < n_stages; ++i) {
const int ret = xhci_wait_for_transfer(xhci, dev->address, 1);
- residue += ret;
+ transferred += ret;
if (ret < 0) {
if (ret == TIMEOUT) {
xhci_debug("Stopping ID %d EP 1\n",
@@ -650,8 +653,8 @@
}
if (dir == IN && data != src)
- memcpy(src, data, dalen - residue);
- return dalen - residue;
+ memcpy(src, data, transferred);
+ return transferred;
}
/* finalize == 1: if data is of packet aligned size, add a zero length packet */
@@ -670,7 +673,7 @@
transfer_ring_t *const tr = xhci->dev[slot_id].transfer_rings[ep_id];
const size_t off = (size_t)data & 0xffff;
- if ((off + size) > ((TRANSFER_RING_SIZE - 1) << 16)) {
+ if ((off + size) > ((TRANSFER_RING_SIZE - 2) << 16)) {
xhci_debug("Unsupported transfer size\n");
return -1;
}
@@ -718,8 +721,8 @@
}
if (ep->direction == IN && data != src)
- memcpy(src, data, size - ret);
- return size - ret;
+ memcpy(src, data, ret);
+ return ret;
}
static trb_t *
diff --git a/payloads/libpayload/drivers/usb/xhci_events.c b/payloads/libpayload/drivers/usb/xhci_events.c
index b947c7d..dacb5d86 100644
--- a/payloads/libpayload/drivers/usb/xhci_events.c
+++ b/payloads/libpayload/drivers/usb/xhci_events.c
@@ -306,7 +306,7 @@
return cc;
}
-/* returns amount of bytes not transferred on success, negative CC on error */
+/* returns amount of bytes transferred on success, negative CC on error */
int
xhci_wait_for_transfer(xhci_t *const xhci, const int slot_id, const int ep_id)
{
diff --git a/payloads/libpayload/drivers/usb/xhci_private.h b/payloads/libpayload/drivers/usb/xhci_private.h
index 9981f07..fbffdb3 100644
--- a/payloads/libpayload/drivers/usb/xhci_private.h
+++ b/payloads/libpayload/drivers/usb/xhci_private.h
@@ -64,7 +64,7 @@
enum {
TRB_NORMAL = 1,
TRB_SETUP_STAGE = 2, TRB_DATA_STAGE = 3, TRB_STATUS_STAGE = 4,
- TRB_LINK = 6,
+ TRB_LINK = 6, TRB_EVENT_DATA = 7,
TRB_CMD_ENABLE_SLOT = 9, TRB_CMD_DISABLE_SLOT = 10, TRB_CMD_ADDRESS_DEV = 11,
TRB_CMD_CONFIGURE_EP = 12, TRB_CMD_EVAL_CTX = 13, TRB_CMD_RESET_EP = 14,
TRB_CMD_STOP_EP = 15, TRB_CMD_SET_TR_DQ = 16, TRB_CMD_NOOP = 23,
@@ -149,6 +149,7 @@
u8 adv;
} event_ring_t;
+/* Never raise this above 256 to prevent transfer event length overflow! */
#define TRANSFER_RING_SIZE 32
typedef struct {
trb_t *ring;
