[PATCH] usb: xhci: fix event trb handling missed

[Bin Meng]

Hi Ran,

On Tue, Sep 22, 2020 at 1:02 PM Ran Wang <ran.wang_1 at nxp.com> wrote:
>
> In functiion xhci_bulk_tx(), when buffer cross 64KB boundary, it will

typo: function

> send request in more than 1 Transfer TRB by chaining them, but then handle
> only 1 event TRB to mark request completed.
>
> However, on Layerscape platforms (LS1028A, LS1088A, etc), we observe xhci
> controller will generated more than 1 event TRB sometimes, this cause that

I am not sure if it's fair to say this is Layerscape specific
behavior. Based on the xHCI spec, the spec indicates 1 event trb
should be generated when IOC/ISP flag is set to 1 or an error occurs.
I will see if I can reproduce your issue on an x86 board.

> function mishandle event TRB in next round call, then system hang due to
> BUG() checking.
>
> This patch adds a loop to make sure the event TRB for last Transfer TRB has
> to be handled in time.
>
> Signed-off-by: Ran Wang <ran.wang_1 at nxp.com>
> ---
>  drivers/usb/host/xhci-ring.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 092ed6e..d77e058 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -578,10 +578,13 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>         int ret;
>         u32 trb_fields[4];
>         u64 val_64 = virt_to_phys(buffer);
> +       void *last_transfer_trb_addr;
> +       int available_length;
>
>         debug("dev=%p, pipe=%lx, buffer=%p, length=%d\n",
>                 udev, pipe, buffer, length);
>
> +       available_length = length;
>         ep_index = usb_pipe_ep_index(pipe);
>         virt_dev = ctrl->devs[slot_id];
>
> @@ -701,7 +704,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>                 trb_fields[2] = length_field;
>                 trb_fields[3] = field | (TRB_NORMAL << TRB_TYPE_SHIFT);
>
> -               queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
> +               last_transfer_trb_addr = queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
>
>                 --num_trbs;
>
> @@ -714,6 +717,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>
>         giveback_first_trb(udev, ep_index, start_cycle, start_trb);
>
> +again:
>         event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
>         if (!event) {
>                 debug("XHCI bulk transfer timed out, aborting...\n");
> @@ -722,14 +726,21 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>                 udev->act_len = 0;
>                 return -ETIMEDOUT;
>         }
> -       field = le32_to_cpu(event->trans_event.flags);
>
> +       if ((void *)event->trans_event.buffer != last_transfer_trb_addr) {

This should be:

if ((void *)le64_to_cpu(event->trans_event.buffer) != last_transfer_trb_addr)

> +               available_length -=
> +                       (int)EVENT_TRB_LEN(le32_to_cpu(event->trans_event.transfer_len));
> +               xhci_acknowledge_event(ctrl);
> +               goto again;
> +       }
> +
> +       field = le32_to_cpu(event->trans_event.flags);
>         BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
>         BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
>         BUG_ON(*(void **)(uintptr_t)le64_to_cpu(event->trans_event.buffer) -
>                 buffer > (size_t)length);
>
> -       record_transfer_result(udev, event, length);
> +       record_transfer_result(udev, event, available_length);
>         xhci_acknowledge_event(ctrl);
>         xhci_inval_cache((uintptr_t)buffer, length);
>

Regards,
Bin