[PATCH] xhci: recheck slot status

[PATCH] xhci: recheck slot status

[Gerd Hoffmann]

Factor out slot status check into a helper function.  Add an additional
check after completing transfers.  This is needed in case a guest
queues multiple transfers in a row and a device unplug happens while
qemu processes them.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786413
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 80988bb305a1..c198a2d150c9 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1861,6 +1861,13 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
     xhci_kick_epctx(epctx, streamid);
 }
 
+static bool xhci_slot_ok(XHCIState *xhci, int slotid)
+{
+    return (xhci->slots[slotid - 1].uport &&
+            xhci->slots[slotid - 1].uport->dev &&
+            xhci->slots[slotid - 1].uport->dev->attached);
+}
+
 static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
 {
     XHCIState *xhci = epctx->xhci;
@@ -1878,9 +1885,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
 
     /* If the device has been detached, but the guest has not noticed this
        yet the 2 above checks will succeed, but we must NOT continue */
-    if (!xhci->slots[epctx->slotid - 1].uport ||
-        !xhci->slots[epctx->slotid - 1].uport->dev ||
-        !xhci->slots[epctx->slotid - 1].uport->dev->attached) {
+    if (!xhci_slot_ok(xhci, epctx->slotid)) {
         return;
     }
 
@@ -1987,6 +1992,10 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
         } else {
             xhci_fire_transfer(xhci, xfer, epctx);
         }
+        if (!xhci_slot_ok(xhci, epctx->slotid)) {
+            /* surprise removal -> stop processing */
+            break;
+        }
         if (xfer->complete) {
             /* update ring dequeue ptr */
             xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
-- 
2.18.1

Re: [PATCH] xhci: recheck slot status

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200107083606.12393-1-kraxel@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

N/A. Internal error while reading log file



The full log is available at
http://patchew.org/logs/20200107083606.12393-1-kraxel@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH] xhci: recheck slot status

[Philippe Mathieu-Daudé]

On 1/7/20 9:36 AM, Gerd Hoffmann wrote:
> Factor out slot status check into a helper function.  Add an additional
> check after completing transfers.  This is needed in case a guest
> queues multiple transfers in a row and a device unplug happens while
> qemu processes them.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786413
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/usb/hcd-xhci.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 80988bb305a1..c198a2d150c9 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -1861,6 +1861,13 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
>       xhci_kick_epctx(epctx, streamid);
>   }
>   
> +static bool xhci_slot_ok(XHCIState *xhci, int slotid)
> +{
> +    return (xhci->slots[slotid - 1].uport &&
> +            xhci->slots[slotid - 1].uport->dev &&
> +            xhci->slots[slotid - 1].uport->dev->attached);
> +}
> +
>   static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>   {
>       XHCIState *xhci = epctx->xhci;
> @@ -1878,9 +1885,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>   
>       /* If the device has been detached, but the guest has not noticed this
>          yet the 2 above checks will succeed, but we must NOT continue */
> -    if (!xhci->slots[epctx->slotid - 1].uport ||
> -        !xhci->slots[epctx->slotid - 1].uport->dev ||
> -        !xhci->slots[epctx->slotid - 1].uport->dev->attached) {
> +    if (!xhci_slot_ok(xhci, epctx->slotid)) {
>           return;
>       }
>   
> @@ -1987,6 +1992,10 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>           } else {
>               xhci_fire_transfer(xhci, xfer, epctx);
>           }
> +        if (!xhci_slot_ok(xhci, epctx->slotid)) {
> +            /* surprise removal -> stop processing */

I'd use "unexpected removal", but surprise is funnier :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +            break;
> +        }
>           if (xfer->complete) {
>               /* update ring dequeue ptr */
>               xhci_set_ep_state(xhci, epctx, stctx, epctx->state);
>