[coreboot-gerrit] New patch to review for coreboot: cd2bc1d libpayload: xhci: Use Event Data TRBs for transfer event generation

[Isaac Christensen (isaac.christensen@se-eng.com)]

Isaac Christensen (isaac.christensen at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6516

-gerrit

commit cd2bc1d33c43d26bc011f7d883bb6fa67276f266
Author: Julius Werner <jwerner at chromium.org>
Date:   Fri Sep 27 12:45:11 2013 -0700

    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.
    
    Change-Id: Idab412d61edf30655ec69c80066bfffd80290403
    Signed-off-by: Julius Werner <jwerner at chromium.org>
    Reviewed-on: https://chromium-review.googlesource.com/170980
    Reviewed-by: Stefan Reinauer <reinauer at google.com>
    Reviewed-by: Kees Cook <keescook at chromium.org>
    (cherry picked from commit e512c8bcaa5b8e05cae3b9d04cd4947298de999d)
    Signed-off-by: Isaac Christensen <isaac.christensen at se-eng.com>
---
 payloads/libpayload/drivers/usb/xhci.c         | 19 +++++++++++--------
 payloads/libpayload/drivers/usb/xhci_private.h |  3 ++-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/xhci.c b/payloads/libpayload/drivers/usb/xhci.c
index 2e9dd3b..d31a8e2 100644
--- a/payloads/libpayload/drivers/usb/xhci.c
+++ b/payloads/libpayload/drivers/usb/xhci.c
@@ -551,6 +551,7 @@ xhci_enqueue_td(transfer_ring_t *const tr, const int ep, const size_t mps,
 		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) {
@@ -560,17 +561,19 @@ xhci_enqueue_td(transfer_ring_t *const tr, const int ep, const size_t mps,
 			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
@@ -583,7 +586,7 @@ xhci_control(usbdev_t *const dev, const direction_t dir,
 	transfer_ring_t *const tr = di->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;
 	}
@@ -670,7 +673,7 @@ xhci_bulk(endpoint_t *const ep,
 	transfer_ring_t *const tr = di->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;
 	}
diff --git a/payloads/libpayload/drivers/usb/xhci_private.h b/payloads/libpayload/drivers/usb/xhci_private.h
index 945c7f1..c7048f4 100644
--- a/payloads/libpayload/drivers/usb/xhci_private.h
+++ b/payloads/libpayload/drivers/usb/xhci_private.h
@@ -65,7 +65,7 @@ enum { XHCI_FULL_SPEED = 1, XHCI_LOW_SPEED = 2, XHCI_HIGH_SPEED = 3, XHCI_SUPER_
 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,
@@ -150,6 +150,7 @@ typedef struct {
 	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;