[Qemu-devel] usb_packet_complete assert(p->owner != NULL)

[Qemu-devel] usb_packet_complete assert(p->owner != NULL)

[Stefan Hajnoczi]

Hi,
I hit an assertion in hw/usb.c when passing through a host USB device
on qemu.git/master (e4fc8781db7c49b0c5ac5d24762e17c59dfe0871).  This
device has never worked before and I was curious to see how
qemu.git/master would do.

The assertion is:
void usb_packet_complete(USBDevice *dev, USBPacket *p)
{
    /* Note: p->owner != dev is possible in case dev is a hub */
    assert(p->owner != NULL);

The problem seems to be that usb_packet_complete() is called on the
hub device and the hub calls usb_packet_complete() again on the actual
leaf device.  Since usb_packet_complete() sets packet->owner to NULL
we hit the assertion immediately when trying to invoke the leaf device
.complete().

I don't understand how USB emulation hangs together, so I added this quick hack:

diff --git a/hw/usb-hub.c b/hw/usb-hub.c
index 286e3ad..277cb47 100644
--- a/hw/usb-hub.c
+++ b/hw/usb-hub.c
@@ -210,7 +210,7 @@ static void usb_hub_complete(USBPort *port,
USBPacket *packet)
      * If we ever inplement usb 2.0 split transactions this will
      * become a little more complicated ...
      */
-    usb_packet_complete(&s->dev, packet);
+    s->dev.port->ops->complete(s->dev.port, packet);
 }

 static void usb_hub_handle_reset(USBDevice *dev)

The hub is now directly invoking .complete() and not messing with
packet->owner (which is already NULL).  We don't hit the assertion
anymore.  Unfortunately the device does not work in my Windows guest -
it must be another problem though.

I'm not sending it as a patch because there's probably a better way of
fixing this.  Any ideas?

Stefan

Re: [Qemu-devel] usb_packet_complete assert(p->owner != NULL)

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

   Hi,

> -    usb_packet_complete(&s->dev, packet);
> +    s->dev.port->ops->complete(s->dev.port, packet);
>   }

> The hub is now directly invoking .complete() and not messing with
> packet->owner (which is already NULL).  We don't hit the assertion
> anymore.

Does the attached patch work for you?

cheers,
   Gerd

[-- Attachment #2: 0001-usb-fix-packet-owner-tracking-with-hub.patch --]
[-- Type: text/plain, Size: 913 bytes --]

>From 192f6ed2d669c10d29e8f7f1f5682357fb0ce9c3 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 10 Oct 2011 14:12:09 +0200
Subject: [PATCH] usb: fix packet owner tracking with hub

usb_packet_complete should only clear owner when called from the device
itself, not from a usb hub.  Complements the special case for the hub in
usb_handle_packet().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/usb.c b/hw/usb.c
index 0f163b4..0c2fe86 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -347,7 +347,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
 {
     /* Note: p->owner != dev is possible in case dev is a hub */
     assert(p->owner != NULL);
-    p->owner = NULL;
+    if (p->owner == dev) {
+        p->owner = NULL;
+    }
     dev->port->ops->complete(dev->port, p);
 }
 
-- 
1.7.1