[Qemu-devel] [PATCH] xhci: flush dequeue pointer to endpoint context

[Qemu-devel] [PATCH] xhci: flush dequeue pointer to endpoint context

[Gerd Hoffmann]

When done processing a endpoint ring we must update the dequeue pointer
in the endpoint context in guest memory.  This is needed to make sure
the guest has a correct view of things and also to make live migration
work properly, because xhci post_load restores alot of the state from
xhci data structures in guest memory.

Add xhci_set_ep_state() call to do that.

The recursive calls stopped by commit
ddb603ab6c981c1d67cb42266fc700c33e5b2d8f had the (unintentional) side
effect to hiding this bug.  xhci_set_ep_state() was called before
processing, to set the state to running, which updated the dequeue
pointer too.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 8958f95..a6929e5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2063,7 +2063,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
 static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
 {
     XHCIState *xhci = epctx->xhci;
-    XHCIStreamContext *stctx;
+    XHCIStreamContext *stctx = NULL;
     XHCITransfer *xfer;
     XHCIRing *ring;
     USBEndpoint *ep = NULL;
@@ -2192,6 +2192,8 @@ 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: flush dequeue pointer to endpoint context

[Dr. David Alan Gilbert]

* Gerd Hoffmann (kraxel@redhat.com) wrote:
> When done processing a endpoint ring we must update the dequeue pointer
> in the endpoint context in guest memory.  This is needed to make sure
> the guest has a correct view of things and also to make live migration
> work properly, because xhci post_load restores alot of the state from
> xhci data structures in guest memory.
> 
> Add xhci_set_ep_state() call to do that.
> 
> The recursive calls stopped by commit
> ddb603ab6c981c1d67cb42266fc700c33e5b2d8f had the (unintentional) side
> effect to hiding this bug.  xhci_set_ep_state() was called before
> processing, to set the state to running, which updated the dequeue
> pointer too.
> 
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks, that passes the basic test reported in the bz; boot the VM,
lsblk, migrate, fdisk -l

Dave

> ---
>  hw/usb/hcd-xhci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 8958f95..a6929e5 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -2063,7 +2063,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
>  static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
>  {
>      XHCIState *xhci = epctx->xhci;
> -    XHCIStreamContext *stctx;
> +    XHCIStreamContext *stctx = NULL;
>      XHCITransfer *xfer;
>      XHCIRing *ring;
>      USBEndpoint *ep = NULL;
> @@ -2192,6 +2192,8 @@ 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
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK