diff mbox

[BUG] USB assertion triggers in usb_packet_complete()

Message ID 20111011093511.03741ed6@BR8GGW75.de.ibm.com
State New
Headers show

Commit Message

Thomas Huth Oct. 11, 2011, 7:35 a.m. UTC 
Am Mon, 10 Oct 2011 15:03:41 +0200
schrieb Thomas Huth <thuth@linux.vnet.ibm.com>:
> 
> I am currently facing a problem when running QEMU (up-to-date git
> version) with OHCI and a lot of virtual USB devices.
> The emulator dies with the following assertion:
> 
> qemu-system-arm: hw/usb.c:337: usb_packet_complete:
> Assertion `p->owner != ((void *)0)' failed.

Not sure whether this is the right solution, but this patch fixes the
problem for me:



 Thomas

Comments

Stefan Hajnoczi Oct. 12, 2011, 10:02 a.m. UTC | #1 
On Tue, Oct 11, 2011 at 8:35 AM, Thomas Huth <thuth@linux.vnet.ibm.com> wrote:
> Am Mon, 10 Oct 2011 15:03:41 +0200
> schrieb Thomas Huth <thuth@linux.vnet.ibm.com>:
>>
>> I am currently facing a problem when running QEMU (up-to-date git
>> version) with OHCI and a lot of virtual USB devices.
>> The emulator dies with the following assertion:
>>
>> qemu-system-arm: hw/usb.c:337: usb_packet_complete:
>> Assertion `p->owner != ((void *)0)' failed.

Hi Thomas,
I hit the same bug recently and Gerd has posted a patch which you can test:
http://patchwork.ozlabs.org/patch/118726/

Stefan
Thomas Huth Oct. 12, 2011, 11:17 a.m. UTC | #2 
Am Wed, 12 Oct 2011 11:02:42 +0100
schrieb Stefan Hajnoczi <stefanha@gmail.com>:

> On Tue, Oct 11, 2011 at 8:35 AM, Thomas Huth <thuth@linux.vnet.ibm.com> wrote:
> > Am Mon, 10 Oct 2011 15:03:41 +0200
> > schrieb Thomas Huth <thuth@linux.vnet.ibm.com>:
> >>
> >> I am currently facing a problem when running QEMU (up-to-date git
> >> version) with OHCI and a lot of virtual USB devices.
> >> The emulator dies with the following assertion:
> >>
> >> qemu-system-arm: hw/usb.c:337: usb_packet_complete:
> >> Assertion `p->owner != ((void *)0)' failed.
> 
> Hi Thomas,
> I hit the same bug recently and Gerd has posted a patch which you can test:
> http://patchwork.ozlabs.org/patch/118726/

Thanks for the hint, Stefan, you're right, that seems to be the same
bug. Your patch is working fine in my scenario, too.

However, Gerd's patch is not working for me, the assertion still
triggers. It seems like usb_packet_complete() is called for the leaf
node before it is called for the hub node, so the leaf node already set
p->owner = NULL.

 Thomas
Gerd Hoffmann Oct. 13, 2011, 10:51 a.m. UTC | #3 
Hi,

>> Hi Thomas,
>> I hit the same bug recently and Gerd has posted a patch which you can test:
>> http://patchwork.ozlabs.org/patch/118726/
>
> Thanks for the hint, Stefan, you're right, that seems to be the same
> bug. Your patch is working fine in my scenario, too.
>
> However, Gerd's patch is not working for me, the assertion still
> triggers. It seems like usb_packet_complete() is called for the leaf
> node before it is called for the hub node, so the leaf node already set
> p->owner = NULL.

Ah, right, on completion the call chain goes the other way around, so 
the usb_handle_packet() style approach doesn't fly.

I think going with Stefans approach + a big fat comment is the best 
solution then.  I'll go queue up a patch.

cheers,
   Gerd
diff mbox

Patch

diff --git a/hw/usb.c b/hw/usb.c
index fa90204..7cef9e2 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -25,6 +25,7 @@ 
  */
 #include "qemu-common.h"
 #include "usb.h"
+#include "usb-desc.h"
 #include "iov.h"
 
 void usb_attach(USBPort *port)
@@ -334,7 +335,9 @@  int usb_handle_packet(USBDevice *dev, USBPacket *p)
 void usb_packet_complete(USBDevice *dev, USBPacket *p)
 {
     /* Note: p->owner != dev is possible in case dev is a hub */
-    assert(p->owner != NULL);
+    if (dev->device->bDeviceClass != USB_CLASS_HUB) {
+        assert(p->owner != NULL);
+    }
     p->owner = NULL;
     dev->port->ops->complete(dev->port, p);
 }