inc_deq() is called both for rings with link trbs and the event ring
without link trbs.
The last_trb() check in inc_deq() has a off by one error, going beyond
allocated array when checking if trb == [TRBS_PER_SEGMENT], and the whole
inc_deq() depend on this.

Rewrite the inc_deq() funciton, remove the faulty last_trb() helper, add
new last_trb_on_seg() and last_trb_on_ring() helpers

Signed-off-by: Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-ring.c | 68 +++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4de8a2b..086b871 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -102,19 +102,6 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, 
struct xhci_ring *ring,
                return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
 }
 
-/* Is this TRB a link TRB or was the last TRB the last TRB in this event ring
- * segment?  I.e. would the updated event TRB pointer step off the end of the
- * event seg?
- */
-static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
-               struct xhci_segment *seg, union xhci_trb *trb)
-{
-       if (ring == xhci->event_ring)
-               return trb == &seg->trbs[TRBS_PER_SEGMENT];
-       else
-               return TRB_TYPE_LINK_LE32(trb->link.control);
-}
-
 static bool trb_is_link(union xhci_trb *trb)
 {
        return TRB_TYPE_LINK_LE32(trb->link.control);
@@ -126,6 +113,17 @@ static int enqueue_is_link_trb(struct xhci_ring *ring)
        return TRB_TYPE_LINK_LE32(link->control);
 }
 
+static bool last_trb_on_seg(struct xhci_segment *seg, union xhci_trb *trb)
+{
+       return trb == &seg->trbs[TRBS_PER_SEGMENT - 1];
+}
+
+static bool last_trb_on_ring(struct xhci_ring *ring,
+                       struct xhci_segment *seg, union xhci_trb *trb)
+{
+       return last_trb_on_seg(seg, trb) && (seg->next == ring->first_seg);
+}
+
 /* Updates trb to point to the next TRB in the ring, and updates seg if the 
next
  * TRB is in a new segment.  This does not skip over link TRBs, and it does not
  * effect the ring dequeue or enqueue pointers.
@@ -151,31 +149,29 @@ static void inc_deq(struct xhci_hcd *xhci, struct 
xhci_ring *ring)
 {
        ring->deq_updates++;
 
-       /*
-        * If this is not event ring, and the dequeue pointer
-        * is not on a link TRB, there is one more usable TRB
-        */
-       if (ring->type != TYPE_EVENT && !trb_is_link(ring->dequeue))
-               ring->num_trbs_free++;
-
-       do {
-               /*
-                * Update the dequeue pointer further if that was a link TRB or
-                * we're at the end of an event ring segment (which doesn't have
-                * link TRBS)
-                */
-               if (last_trb(xhci, ring, ring->deq_seg, ring->dequeue)) {
-                       if (ring->type == TYPE_EVENT &&
-                                       last_trb_on_last_seg(xhci, ring,
-                                               ring->deq_seg, ring->dequeue)) {
-                               ring->cycle_state ^= 1;
-                       }
-                       ring->deq_seg = ring->deq_seg->next;
-                       ring->dequeue = ring->deq_seg->trbs;
-               } else {
+       /* event ring doesn't have link trbs, check for last trb */
+       if (ring->type == TYPE_EVENT) {
+               if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
                        ring->dequeue++;
+                       return;
                }
-       } while (last_trb(xhci, ring, ring->deq_seg, ring->dequeue));
+               if (last_trb_on_ring(ring, ring->deq_seg, ring->dequeue))
+                       ring->cycle_state ^= 1;
+               ring->deq_seg = ring->deq_seg->next;
+               ring->dequeue = ring->deq_seg->trbs;
+               return;
+       }
+
+       /* All other rings have link trbs */
+       if (!trb_is_link(ring->dequeue)) {
+               ring->dequeue++;
+               ring->num_trbs_free++;
+       }
+       while (trb_is_link(ring->dequeue)) {
+               ring->deq_seg = ring->deq_seg->next;
+               ring->dequeue = ring->deq_seg->trbs;
+       }
+       return;
 }
 
 /*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to