[Qemu-devel] [PATCH] xhci: only update dequeue ptr on completed transfers

[Qemu-devel] [PATCH] xhci: only update dequeue ptr on completed transfers

[Gerd Hoffmann]

The dequeue pointer should only be updated in case the transfer
is actually completed.  If we update it for inflight transfers
we will not pick them up again after migration, which easily
triggers with HID devices as they typically have a pending
transfer, waiting for user input to happen.

Fixes: 243afe858b95765b98d16a1f0dd50dca262858ad
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451631
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index a0c7960a7b..760135c0d2 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1912,6 +1912,8 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
         }
         assert(!xfer->running_retry);
         if (xfer->complete) {
+            /* update ring dequeue ptr */
+            xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
             xhci_ep_free_xfer(epctx->retry);
         }
         epctx->retry = NULL;
@@ -1962,6 +1964,8 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
             xhci_fire_transfer(xhci, xfer, epctx);
         }
         if (xfer->complete) {
+            /* update ring dequeue ptr */
+            xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
             xhci_ep_free_xfer(xfer);
             xfer = NULL;
         }
@@ -1979,8 +1983,6 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
             break;
         }
     }
-    /* update ring dequeue ptr */
-    xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
     epctx->kick_active--;
 
     ep = xhci_epid_to_usbep(epctx);
-- 
2.9.3

Re: [Qemu-devel] [PATCH] xhci: only update dequeue ptr on completed transfers

[Laurent Vivier]

On 08/06/2017 09:41, Gerd Hoffmann wrote:
> The dequeue pointer should only be updated in case the transfer
> is actually completed.  If we update it for inflight transfers
> we will not pick them up again after migration, which easily
> triggers with HID devices as they typically have a pending
> transfer, waiting for user input to happen.
> 
> Fixes: 243afe858b95765b98d16a1f0dd50dca262858ad
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451631
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Tested-by: Laurent Vivier <lvivier@redhat.com>

> ---
>  hw/usb/hcd-xhci.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index a0c7960a7b..760135c0d2 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -1912,6 +1912,8 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>          }
>          assert(!xfer->running_retry);
>          if (xfer->complete) {
> +            /* update ring dequeue ptr */
> +            xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
>              xhci_ep_free_xfer(epctx->retry);
>          }
>          epctx->retry = NULL;
> @@ -1962,6 +1964,8 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>              xhci_fire_transfer(xhci, xfer, epctx);
>          }
>          if (xfer->complete) {
> +            /* update ring dequeue ptr */
> +            xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
>              xhci_ep_free_xfer(xfer);
>              xfer = NULL;
>          }
> @@ -1979,8 +1983,6 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>              break;
>          }
>      }
> -    /* update ring dequeue ptr */
> -    xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
>      epctx->kick_active--;
>  
>      ep = xhci_epid_to_usbep(epctx);
>