diff mbox

xhci: child detach fix

Message ID 33183CC9F5247A488A2544077AF19020815E7FD4@SZXEMA503-MBS.china.huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) May 13, 2014, 3:53 a.m. UTC 
> -----Original Message-----
> From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org
> [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> Behalf Of Gerd Hoffmann
> Sent: Monday, May 12, 2014 8:50 PM
> To: qemu-devel@nongnu.org
> Cc: Gerd Hoffmann
> Subject: [Qemu-devel] [PATCH] xhci: child detach fix
> 
> xhci_child_detach() zaps the wrong slot when unplugging a device
> connected via usb-hub:  Instead of the device's slot the slot of the
> usb-hub is used.  Fix it.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1075846
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index ef3177a..6753a42 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3435,7 +3435,7 @@ static void xhci_child_detach(USBPort *uport,
> USBDevice *child)
>      USBBus *bus = usb_bus_from_device(child);
>      XHCIState *xhci = container_of(bus, XHCIState, bus);
> 
> -    xhci_detach_slot(xhci, uport);
> +    xhci_detach_slot(xhci, child->port);
>  }
> 
>  static USBPortOps xhci_uport_ops = {
> --
> 1.8.3.1
> 
Reviewed-by: Gonglei <arei.gonglei@huawei.com>

BTW, in usb_release_port(), the detached port should be insert
the head of bus->free list table. Because of the save/restore will
cause qemu crash, after hot plug/hot unplug multi times. For example,
save the port '2' to memory file, but when we restore it, the port will
be assign to '1' over again, which not match with the memory file,
and then crash qemu.

The error log:

"Unknown savevm section or instance '0000:00:04.0/3/usb-host' 2"

The below patch can solve this problem.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gerd Hoffmann May 13, 2014, 6:56 a.m. UTC | #1 
Hi,

> BTW, in usb_release_port(), the detached port should be insert
> the head of bus->free list table. Because of the save/restore will
> cause qemu crash, after hot plug/hot unplug multi times. For example,
> save the port '2' to memory file, but when we restore it, the port will
> be assign to '1' over again, which not match with the memory file,
> and then crash qemu.

If you want hotplug and live migration play well you have to explicitly
assign devices ports, i.e.

  -device usb-host,port=2,$args

Otherwise it will simply not work reliable.

cheers,
  Gerd
Gonglei (Arei) May 13, 2014, 7:13 a.m. UTC | #2 
> -----Original Message-----

> From: Gerd Hoffmann [mailto:kraxel@redhat.com]

> Sent: Tuesday, May 13, 2014 2:56 PM

> 

> > BTW, in usb_release_port(), the detached port should be insert

> > the head of bus->free list table. Because of the save/restore will

> > cause qemu crash, after hot plug/hot unplug multi times. For example,

> > save the port '2' to memory file, but when we restore it, the port will

> > be assign to '1' over again, which not match with the memory file,

> > and then crash qemu.

> 

> If you want hotplug and live migration play well you have to explicitly

> assign devices ports, i.e.

> 

>   -device usb-host,port=2,$args

> 

> Otherwise it will simply not work reliable.

> 

Yep, I have noticed that case, which pass-through two USB devices, and
then hot-unplug the first one, then save/restore, which will also crash qemu.

So maybe assign ports ahead was the exclusive method. Thanks,

Best regards,
-Gonglei
diff mbox

Patch

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index e48b19f..4e6ccad 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -442,7 +442,7 @@  void usb_release_port(USBDevice *dev)
     dev->port = NULL;
     port->dev = NULL;
 
-    QTAILQ_INSERT_TAIL(&bus->free, port, next);
+    QTAILQ_INSERT_HEAD(&bus->free, port, next);
     bus->nfree++;
 }