[Qemu-devel] [PATCH] xhci: child detach fix

[Qemu-devel] [PATCH] xhci: child detach fix

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH] xhci: child detach fix

[Gonglei (Arei)]

> -----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(-)

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++;
 }
 
-- 
1.7.12.4


Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH] xhci: child detach fix

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [PATCH] xhci: child detach fix

[Gonglei (Arei)]

> -----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