[PATCH v1 0/1] USB xHCI wrong act_len calculation in case of using multipe TRB

[bigunclemax at gmail.com]

From: Maksim Kiselev <bigunclemax at gmail.com>

Hello everyone!

I've encountered an issue where the actual length of received data is
calculated incorrectly in the case of a multiple TRB request.

Below, I'll try to describe the essence of the problem:

A USB-ethernet adapter ASIX ax88179 is connected to my board Li4pi,
and I'm sending a DHCP request to the server via dhpc command.

The response from the DHCP server always has the same length (0x168).
However, in some cases, I noticed that the received response had
an incorrect length and network subsystem is completely ingnore
incoming packets.

The problem turned out to be in the xhci_bulk_tx() function.
I added some debugging[1] to better understand what's happening.

Here's the log from a working case:
```
    dev=000000057f786b90, pipe=c0010283, buffer=000000057f787540, length=20480
    PUSH. trb_len: 0x5000
    POP. trb_len: 0x4e98, comp_code: 0xd
    udev->act_len: 0x168
```

And here's the log from a non-working case:
```
    dev=000000057f78b610, pipe=c0010283, buffer=000000057f78bfc0, length=20480
    PUSH. trb_len: 0x4040
    PUSH. trb_len: 0xfc0
    POP. trb_len: 0x3ed8, comp_code: 0xd
    POP. trb_len: 0x0, comp_code: 0x1
    udev->act_len: 0x1128
```
As you can see, in the second case, the buffer spans a 64KB boundary
(buffer=000000057f78bfc0 + 0x5000).
Therefore, it is split into two TRBs with lengths 0x4040 and 0xfc0.

Then, act_len is calculated as the difference between length and
trans_event.transfer_len:
```
0x5000 - 0x3ed8 = 0x1128
```

However, it seems that this is not entirely correct,
and act_len should be calculated as:

```
trb_buff_len - trans_event.transfer_len
(0x4040 - 0x3ed8 = 0x168)
```

0x168 is the correct length, which is the same as the length
obtained in the first case where network rx works fine.

Also I looked at the Linux xhci-ring code where urb->actual_length
calculated as:
[2]
```
    requested = td->urb->transfer_buffer_length;
    remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
```
[3]
```
    td->urb->actual_length = requested - remaining;
```

Perhaps I missed something or misunderstood, so I would appreciate any help.

---

[1] Patch for debug output
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 89d2e54f20a..b1433a16a99 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -791,6 +791,8 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
                trb_fields[2] = length_field;
                trb_fields[3] = field | TRB_TYPE(TRB_NORMAL);
 
+               debug("PUSH. trb_len: 0x%x\n", trb_buff_len);
+
                last_transfer_trb_addr = queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
 
                --num_trbs;
@@ -816,6 +818,10 @@ again:
                return -ETIMEDOUT;
        }
 
+       debug("POP. trb_len: 0x%x, comp_code: 0x%x\n",
+             (int)EVENT_TRB_LEN(le32_to_cpu(event->trans_event.transfer_len)),
+             GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len)));
+
        if ((uintptr_t)(le64_to_cpu(event->trans_event.buffer)) !=
            (uintptr_t)last_transfer_trb_addr) {
                available_length -=
@@ -831,6 +837,7 @@ again:
        available_length -= first_trb_trimmed_sz;
 
        record_transfer_result(udev, event, available_length);
+       debug("udev->act_len: 0x%x\n", udev->act_len);
        xhci_acknowledge_event(ctrl);
        xhci_inval_cache((uintptr_t)buffer, length);
        xhci_dma_unmap(ctrl, buf_64, length);


[2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/host/xhci-ring.c?h=v6.14-rc7#n2346
[3] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/host/xhci-ring.c?h=v6.14-rc7#n2413

Maksim Kiselev (1):
  usb: xhci: fix calculation of act_len in case of using multipe TRB

 drivers/usb/host/xhci-ring.c | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.45.2