[Qemu-devel] Large USB patch

[Qemu-devel] Large USB patch

[nix.wie.weg]

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

Hello everybody,

I have extended the USB support for Qemu. The patch is included and
compiles just fine against todays cvs repository. As explained below,
this patch touches 15 different files, which makes it not so easy to
keep it applying on that very fast developing project. Thats why I would
ask you to test it quickly and apply it to cvs as soon as possible.

With this patch applied I could detect a USB Epson Scanner and a USB
Epson Printer from Windows 98 + XP and I could even print pages with
the printer (see known problems below).

reasons for this patch:
I was looking for a way to address my USB printer with windows while
working on linux. As I started the work I had to recognize that my
printer was not even detected under qemu. So I started to work on it.

changes I made:
First I eliminated all potential error sources, which could be avoided.
One of these sources where the

1. usb-hub - which I transfered to an extra file usb-hub.c, then I
added the usb-libusb.c devices directly to the UHCI controller this
introduced the changes of 2)
2. I enhanced the usb add device potentialities so that you should be
able to add a device to the UHCI controller. This device can be a usb
hub or a usb device. Behind this device you can add another hub or
device ... (remember this feature is now theoretically possible but not
tested yet), this let to a new usb syntax:
#$ qemu -usb controller=uhci,busnum=001 device=host:2:3,addto=001:001
(the short form is:
#$ qemu -usb controller=uhci -usb device=host:2:3)
so you can build up a complete usb tree.
3. I changed the usb_generic_handle_packet() function and implemented a
state machine, which is able to handle the usb packets correct.
4. I changed the linux standard usb-host library to libusb. I personally
will always prefer a portable solution if possible.
5. I changed large parts of usb-libusb.c. Some changes were made because
I found the source very awkward, some others because I fixed errors and
some because I changed the general usb api (see item 2).
6. I tried to join as many usb functions as possible to the usb related
files. So that hopefully nobody has to change 15 files again.
7. I made minor changes to usb-uhci - mainly I applied the new api and
changed the handling of special messages like usb_reset or usb_attach
8. I made the necessary changes to usb-hid.c and usb-hub.c
9. I wrote a lot of source comments

this patch breaks some things:
Sorry guys but I could not fix all of it, so I need your help, I didn't
want to destroy anybodys work, but the new api makes it necessary to
change some files:
1. usb-linux.c and usb-bsd.c will not work without adoption of the new api
2. I did not test usb-hid and usb-hub

known problems:
1. under linux the uhci controller reports an error if no usb device is
connected
2. the printer and the scanner are recognized under Windows 98/XP and
Linux, but the scanner goes into STALL state as soon as a packet in
usb_write_bulk() or usb_read_bulk()
3. the libusb usb_host_reset() function does not work as expected and I
don't know why (i have commented out this part of the source)
4. the printer stops printing on large images and is then in a state,
where it will not resume his work (probably a timing issue)

With kind regards,
Tino H. Seifert

[-- Attachment #2: usb-2006-04-20.patch.gz --]
[-- Type: application/x-gunzip, Size: 32699 bytes --]

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

nix.wie.weg@gmx.de wrote:

>With this patch applied I could detect a USB Epson Scanner and a USB
>Epson Printer from Windows 98 + XP and I could even print pages with
>the printer (see known problems below).
>
>reasons for this patch:
>I was looking for a way to address my USB printer with windows while
>working on linux. As I started the work I had to recognize that my
>printer was not even detected under qemu. So I started to work on it.
>  
>
   Are you sure such vast changes are necessary?  What was it exactly 
that happened when you attached the printer/scanner?  I'd be interested 
in seeing a packet log without all the changes that seem to "break stuff."

>changes I made:
>First I eliminated all potential error sources, which could be avoided.
>One of these sources where the
>
>1. usb-hub - which I transfered to an extra file usb-hub.c, then I
>added the usb-libusb.c devices directly to the UHCI controller this
>introduced the changes of 2)
>2. I enhanced the usb add device potentialities so that you should be
>able to add a device to the UHCI controller. This device can be a usb
>hub or a usb device. Behind this device you can add another hub or
>device ... (remember this feature is now theoretically possible but not
>tested yet), this let to a new usb syntax:
>#$ qemu -usb controller=uhci,busnum=001 device=host:2:3,addto=001:001
>(the short form is:
>#$ qemu -usb controller=uhci -usb device=host:2:3)
>so you can build up a complete usb tree.
>3. I changed the usb_generic_handle_packet() function and implemented a
>state machine, which is able to handle the usb packets correct.
>4. I changed the linux standard usb-host library to libusb. I personally
>will always prefer a portable solution if possible.
>5. I changed large parts of usb-libusb.c. Some changes were made because
>I found the source very awkward, some others because I fixed errors and
>some because I changed the general usb api (see item 2).
>6. I tried to join as many usb functions as possible to the usb related
>files. So that hopefully nobody has to change 15 files again.
>7. I made minor changes to usb-uhci - mainly I applied the new api and
>changed the handling of special messages like usb_reset or usb_attach
>8. I made the necessary changes to usb-hid.c and usb-hub.c
>9. I wrote a lot of source comments
>  
>
   I'm in favor of a new api but with only one controller there is 
almost no point in doing this yet.  It may make more sense to either be 
able to specify either grabbing all or a few interfaces to proxy to the 
guest.  Also, libusb is ok for a generic handler, but there is no way 
you'll get someone to jump through all the hoops necessary to get usb 
working under windows with libusb-win32 or even mac os x.  On win32 host 
you have to manually create an inf file with the PID/VID and then 
install that for every device you try to use.  It's not a good idea to 
use the filter driver unless the corresponding host driver is unbinded 
(especially for mass storage).  On mac os x you would supposedly creates 
codeless kernel extensions with the PID/VID to unbind the device.  That 
could be done through scripts, but none exist.

   Also keep in mind the people here probably don't know about the 
all-in-one patch on my webserver.  I've never posted about it here 
except for on the user forums.

>this patch breaks some things:
>Sorry guys but I could not fix all of it, so I need your help, I didn't
>want to destroy anybodys work, but the new api makes it necessary to
>change some files:
>1. usb-linux.c and usb-bsd.c will not work without adoption of the new api
>2. I did not test usb-hid and usb-hub
>  
>
   I'll try to look at the patch this weekend.

>known problems:
>1. under linux the uhci controller reports an error if no usb device is
>connected
>2. the printer and the scanner are recognized under Windows 98/XP and
>Linux, but the scanner goes into STALL state as soon as a packet in
>usb_write_bulk() or usb_read_bulk()
>3. the libusb usb_host_reset() function does not work as expected and I
>don't know why (i have commented out this part of the source)
>  
>
    The reset function in libusb causes the device to have a new address 
when it reconnects therefore forcing you to rescan and open the device 
again.  Perhaps this is the problem.

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

Hello Lonnie,

First,  thank you for the answer.

Lonnie Mendez wrote:
> nix.wie.weg@gmx.de wrote:
>
>> printer was not even detected under qemu. So I started to work on it.
>>
>   Are you sure such vast changes are necessary?  What was it exactly
> that happened when you attached the printer/scanner?  I'd be
> interested in seeing a packet log without all the changes that seem to
> "break stuff."
Yes I am absolutly sure we need this changes. The usb protocoll is a
very sophisticated work. There is an exactly defined sequence in which
packets are send. (I have made some small documentation about this:
http://217.20.126.200/tino/usb-order-of-events.pdf
http://217.20.126.200/tino/usb-order-of-events.odg)
If you do not keep track of this, you will never be able to get most
devices running. The qemu-specialcase-1.patch is the result of ignoring
these sequence. At the moment I'm not even sure, if we have to implement
the states in which a device is in (I mean default state, adress state
... USB Spec. 1.1 chapter 9).
>
>> changed the handling of special messages like usb_reset or usb_attach
>> 8. I made the necessary changes to usb-hid.c and usb-hub.c
>> 9. I wrote a lot of source comments
>>  
>>
>   I'm in favor of a new api but with only one controller there is
> almost no point in doing this yet.  
Sorry I can't agree on that point with you. We will get more
controller/devices if we can provide an easy api for implementing them.
I would really be interrested  to see an EHCI Controller - maybe I will
even implement it by myself.
> It may make more sense to either be able to specify either grabbing
> all or a few interfaces to proxy to the guest.  Also, libusb is ok for
> a generic handler, but there is no way you'll get someone to jump
> through all the hoops necessary to get usb working under windows with
> libusb-win32 or even mac os x.  On win32 host you have to manually
> create an inf file with the PID/VID and then install that for every
> device you try to use.  It's not a good idea to use the filter driver
> unless the corresponding host driver is unbinded (especially for mass
> storage).  On mac os x you would supposedly creates codeless kernel
> extensions with the PID/VID to unbind the device.  That could be done
> through scripts, but none exist.
>
On that point you have probably misunderstood me. I dont want to
liquidate any native usb-host files. On the contrary, with that patch it
is easier to get more such native interfaces running. We could even
implement on some platforms more than one interface. I for instance
would like it, if I could have libusb and linux native support enabled
at the same time so I could make such things like:
$ qemu -usb controller=uhci -usb device=libusb:002:003,addto=001:001
-usb device=linux:001:002,addto=001:002
and it should now not be so difficult to implement.
>   Also keep in mind the people here probably don't know about the
> all-in-one patch on my webserver.  I've never posted about it here
> except for on the user forums.
>> this patch breaks some things:
>>
>   I'll try to look at the patch this weekend.
Thanks
>> known problems:
> The reset function in libusb causes the device to have a new address
> when it reconnects therefore forcing you to rescan and open the device
> again.  Perhaps this is the problem.
Yes I know this and I implemented it. But it does still not work. We get
a STALL on the devices when the code is enabled.


With kind regards
Tino H. Seifert

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

nix.wie.weg@gmx.de wrote:

   lo

>Yes I am absolutly sure we need this changes. The usb protocoll is a
>very sophisticated work. There is an exactly defined sequence in which
>packets are send. (I have made some small documentation about this:
>http://217.20.126.200/tino/usb-order-of-events.pdf
>http://217.20.126.200/tino/usb-order-of-events.odg)
>If you do not keep track of this, you will never be able to get most
>devices running. The qemu-specialcase-1.patch is the result of ignoring
>these sequence. At the moment I'm not even sure, if we have to implement
>the states in which a device is in (I mean default state, adress state
>... USB Spec. 1.1 chapter 9).
>  
>
   Well I'm glad someone took a deeper look at it.  I never addressed it 
as a correct solution to the problem.  And indeed with the patch applied 
the transaction log is clean.

>>>changed the handling of special messages like usb_reset or usb_attach
>>>8. I made the necessary changes to usb-hid.c and usb-hub.c
>>>9. I wrote a lot of source comments
>>> 
>>>
>>>      
>>>
>>  I'm in favor of a new api but with only one controller there is
>>almost no point in doing this yet.  
>>    
>>
>Sorry I can't agree on that point with you. We will get more
>controller/devices if we can provide an easy api for implementing them.
>I would really be interrested  to see an EHCI Controller - maybe I will
>even implement it by myself.
>  
>
   Sounds good.  Not sure what I was going on about there.

>>It may make more sense to either be able to specify either grabbing
>>all or a few interfaces to proxy to the guest.  Also, libusb is ok for
>>a generic handler, but there is no way you'll get someone to jump
>>through all the hoops necessary to get usb working under windows with
>>libusb-win32 or even mac os x.  On win32 host you have to manually
>>create an inf file with the PID/VID and then install that for every
>>device you try to use.  It's not a good idea to use the filter driver
>>unless the corresponding host driver is unbinded (especially for mass
>>storage).  On mac os x you would supposedly creates codeless kernel
>>extensions with the PID/VID to unbind the device.  That could be done
>>through scripts, but none exist.
>>
>>    
>>
>On that point you have probably misunderstood me. I dont want to
>liquidate any native usb-host files. On the contrary, with that patch it
>is easier to get more such native interfaces running. We could even
>implement on some platforms more than one interface. I for instance
>would like it, if I could have libusb and linux native support enabled
>at the same time so I could make such things like:
>$ qemu -usb controller=uhci -usb device=libusb:002:003,addto=001:001
>-usb device=linux:001:002,addto=001:002
>and it should now not be so difficult to implement.
>  
>
   Yes, I have misunderstood you.  It sounds like a good plan.  I'll 
ready the bsd redirector around the api tomorrow if possible.  From some 
limited testing it fails rather early on a linux guest so it shouldn't 
be too hard to fix on that.  There is quite a bit here and the person 
that would merge this into CVS is Fabrice Bellard.  He would have to 
review all this before it ever touches CVS.

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

nix.wie.weg@gmx.de wrote:

>this patch breaks some things:
>Sorry guys but I could not fix all of it, so I need your help, I didn't
>want to destroy anybodys work, but the new api makes it necessary to
>change some files:
>1. usb-linux.c and usb-bsd.c will not work without adoption of the new api
>  
>
   Looking at that now.

>2. I did not test usb-hid and usb-hub
>  
>
   There was a usb tablet device added recently.  You may want to cvs up 
to account for that.

>known problems:
>1. under linux the uhci controller reports an error if no usb device is
>connected
>  
>
   The port registers on the uhci controller shows no appropriate 
response to an HCHALT command that is issued and so the hcd puts the 
controller into global suspend.

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

Lonnie Mendez wrote:

>> 2. I did not test usb-hid and usb-hub
>>  
>
>   There was a usb tablet device added recently.  You may want to cvs 
> up to account for that.
>
   Disregard that.  Just saw it ;p

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

Lonnie Mendez wrote:

>> known problems:
>> 1. under linux the uhci controller reports an error if no usb device is
>> connected
>>  
>
>   The port registers on the uhci controller shows no appropriate 
> response to an HCHALT command that is issued and so the hcd puts the 
> controller into global suspend.

   The problem is more likely that the controller is being suspended but 
there is currently nothing being done to signal it to wake up on 
appropriate events.  If the error/warning message you got was something 
like "controller still running" then changing uhci writew to always set 
UHCI_STS_HCHALTED on writing 0 to bit 0 of cmd register seems to clear 
the message.  That means having the bit set in the frame is a bit 
redundant and also makes the bit a less accurate indicator of hchalt.

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

   lo.  The attached patch applied on top of your patchset seems to work 
as far as signaling resume goes.

[-- Attachment #2: qemu-usb-resume.diff --]
[-- Type: text/plain, Size: 1782 bytes --]

--- a/qemu/hw/usb-uhci.c	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb-uhci.c	2006-04-21 11:17:08.000000000 -0500
@@ -32,6 +32,8 @@
 // #define DEBUG
 // #define DEBUG_PACKET
 
+#define UHCI_CMD_FGR      (1 << 4)
+#define UHCI_CMD_EGSM     (1 << 3)
 #define UHCI_CMD_GRESET   (1 << 2)
 #define UHCI_CMD_HCRESET  (1 << 1)
 #define UHCI_CMD_RS       (1 << 0)
@@ -109,7 +111,8 @@
         ((s->status & UHCI_STS_USBERR) && (s->intr & (1 << 0))) ||
         ((s->status & UHCI_STS_RD) && (s->intr & (1 << 1))) ||
         (s->status & UHCI_STS_HSERR) ||
-        (s->status & UHCI_STS_HCPERR)) {
+        (s->status & UHCI_STS_HCPERR) ||
+        ((s->status & UHCI_STS_RD) && (s->intr & (1 << 1)))) {
         level = 1;
     } else {
         level = 0;
@@ -188,7 +191,7 @@
             /* start frame processing */
             qemu_mod_timer(s->frame_timer, qemu_get_clock(vm_clock));
             s->status &= ~UHCI_STS_HCHALTED;
-        } else if (!(val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
+        } else if (!(val & UHCI_CMD_RS)) {
             s->status |= UHCI_STS_HCHALTED;
         }
         if (val & UHCI_CMD_GRESET) {
@@ -335,6 +338,14 @@
     UHCIState *s = hub->opaque;
     UHCIPort  *port;
     int i;
+
+    // wakeup the controller if suspended
+    if (s->cmd & UHCI_CMD_EGSM) {
+        s->cmd |= UHCI_CMD_FGR;
+        s->status |= UHCI_STS_RD;
+        uhci_update_irq(s);
+    }
+
     if (dev) {
         if( portnum >= NB_PORTS ) {
 #ifdef DEBUG        
@@ -575,8 +586,6 @@
 
     if (!(s->cmd & UHCI_CMD_RS)) {
         qemu_del_timer(s->frame_timer);
-        /* set hchalted bit in status - UHCI11D 2.1.2 */
-        s->status |= UHCI_STS_HCHALTED;
         return;
     }
     frame_addr = s->fl_base_addr + ((s->frnum & 0x3ff) << 2);

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

Hello,

Lonnie Mendez wrote:
> Lonnie Mendez wrote:
>
>>
>>   The port registers on the uhci controller shows no appropriate
>> response to an HCHALT command that is issued and so the hcd puts the
>> controller into global suspend.
>
>   The problem is more likely that the controller is being suspended
> but there is currently nothing being done to signal it to wake up on
> appropriate events.  If the error/warning message you got was
> something like "controller still running" then changing uhci writew to
> always set UHCI_STS_HCHALTED on writing 0 to bit 0 of cmd register
> seems to clear the message.  That means having the bit set in the
> frame is a bit redundant and also makes the bit a less accurate
> indicator of hchalt.
Yes this seems to be the case. I will take a look at it. Maybe I can get
the signaling correct. I will read the specification, what the correct
handling should be.
I must tell you: very impressive and fast error detection. I take my hat :)

With kind regards
Tino H. Seifert

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

you are too fast for me :)

Lonnie Mendez wrote:
>   lo.  The attached patch applied on top of your patchset seems to
> work as far as signaling resume goes.
> ------------------------------------------------------------------------
>
>   
>

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

nix.wie.weg@gmx.de wrote:

>you are too fast for me :)
>
   Had to rediff it.  Fabrice already put the necessary bits in 
uhci_update_irq.  Right in front of my eyes too.

   There is some funkiness going on with removing the device in the 
linux guest once attached.  Not sure what it is yet.

[-- Attachment #2: qemu-usb-resume.diff --]
[-- Type: text/plain, Size: 1387 bytes --]

--- a/qemu/hw/usb-uhci.c	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb-uhci.c	2006-04-21 11:17:08.000000000 -0500
@@ -32,6 +32,8 @@
 // #define DEBUG
 // #define DEBUG_PACKET
 
+#define UHCI_CMD_FGR      (1 << 4)
+#define UHCI_CMD_EGSM     (1 << 3)
 #define UHCI_CMD_GRESET   (1 << 2)
 #define UHCI_CMD_HCRESET  (1 << 1)
 #define UHCI_CMD_RS       (1 << 0)
@@ -188,7 +191,7 @@
             /* start frame processing */
             qemu_mod_timer(s->frame_timer, qemu_get_clock(vm_clock));
             s->status &= ~UHCI_STS_HCHALTED;
-        } else if (!(val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
+        } else if (!(val & UHCI_CMD_RS)) {
             s->status |= UHCI_STS_HCHALTED;
         }
         if (val & UHCI_CMD_GRESET) {
@@ -335,6 +338,14 @@
     UHCIState *s = hub->opaque;
     UHCIPort  *port;
     int i;
+
+    // wakeup the controller if suspended
+    if (s->cmd & UHCI_CMD_EGSM) {
+        s->cmd |= UHCI_CMD_FGR;
+        s->status |= UHCI_STS_RD;
+        uhci_update_irq(s);
+    }
+
     if (dev) {
         if( portnum >= NB_PORTS ) {
 #ifdef DEBUG        
@@ -575,8 +586,6 @@
 
     if (!(s->cmd & UHCI_CMD_RS)) {
         qemu_del_timer(s->frame_timer);
-        /* set hchalted bit in status - UHCI11D 2.1.2 */
-        s->status |= UHCI_STS_HCHALTED;
         return;
     }
     frame_addr = s->fl_base_addr + ((s->frnum & 0x3ff) << 2);

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

Lonnie Mendez wrote:

>   There is some funkiness going on with removing the device in the 
> linux guest once attached.  Not sure what it is yet.

   This problem is addressed in the attached patch.

[-- Attachment #2: qemu-uhci-portdisc.diff --]
[-- Type: text/plain, Size: 539 bytes --]

--- a/qemu/hw/usb-uhci.c	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb-uhci.c	2006-04-21 12:57:04.000000000 -0500
@@ -382,8 +392,9 @@
     } else {
         port = &s->ports[portnum];
         /* set connect status */
-        if (!(port->ctrl & UHCI_PORT_CCS)) {
-            port->ctrl |= UHCI_PORT_CCS | UHCI_PORT_CSC;
+        if (port->ctrl & UHCI_PORT_CCS) {
+            port->ctrl &= ~UHCI_PORT_CCS;
+            port->ctrl |= UHCI_PORT_CSC;
         }
         /* disable port */
         if (port->ctrl & UHCI_PORT_EN) {

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

   lo.  A few more things to note in the current problems category:

-emulated devices don't seem to be interacting:

frame 36: pid=SETUP addr=0x00 ep=0 len=8
     data_out= 80 06 00 01 00 00 40 00
frame 37: pid=SETUP addr=0x00 ep=0 len=8
     data_out= 80 06 00 01 00 00 40 00
frame 38: pid=SETUP addr=0x00 ep=0 len=8
     data_out= 80 06 00 01 00 00 40 00

-qemu segfaults after giving invalid string for device removal:

   usb_add tablet
   usb_del tablet
   usb_del 001:001

   This only happens with the emulated devices.

-performing usb_del on a valid string handle gives error message:

(qemu) usb_add host:001:021
(qemu) info usb
     Controller 001: uhci
          001:001 = host:001:021
     Summary: 1 USB Controller, 1 USB Devices
(qemu) usb_del 001:001
Could not remove USB device '001:001'
(qemu) info usb
     Controller 001: uhci
     Summary: 1 USB Controller, 0 USB Devices
(qemu)

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

   Another patch.  This one does a few things:

-fixes minor output bugs and some various OBO bugs
-adds some improvements to the emulated hub
-sets up the emulated devices to use the generic message handler (they 
now work again)
-makes tablet device visible to usb_add

   There are of course more bugs I've found.  Namely being able to 
usb_add any particular string with that string showing up as a new 
device even though no valid entry for it exists.

[-- Attachment #2: lusb-upd1.diff --]
[-- Type: text/plain, Size: 6133 bytes --]

--- a/qemu/hw/usb.c	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb.c	2006-04-21 15:27:19.000000000 -0500
@@ -455,6 +455,10 @@
         /* we found a guest usb mouse */
         tree->dev= usb_mouse_init ();
         return add_usb_device (tree);
+    } else if (strcmp (tree->name,"tablet") == 0) {
+        /* we found a guest usb tablet */
+        tree->dev = usb_tablet_init ();
+        return add_usb_device (tree);
     }
     return 1;
 }
@@ -491,6 +495,7 @@
     usb_host_info();
     usb_hub_info();
     usb_mouse_info();
+    usb_tablet_info();
 }
 
 void usb_print_childs (USBTree *tree, int layer) {
--- a/qemu/hw/usb.h	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb.h	2006-04-21 15:27:58.000000000 -0500
@@ -242,7 +242,9 @@
 void        usb_hub_info                (void);
 /* usb-hid.c */
 USBDevice*  usb_mouse_init              (void);
+USBDevice*  usb_tablet_init             (void);
 void        usb_mouse_info              (void);
+void        usb_tablet_info             (void);
 
 
 /* The usb dummy device functions, they exist only to make it easier to
--- a/qemu/hw/usb-hub.c	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb-hub.c	2006-04-21 15:23:05.000000000 -0500
@@ -165,9 +165,9 @@
     0x0a,           /* u16  wHubCharacteristics; */
     0x00,           /*   (per-port OC, no power switching) */
     0x01,           /*  u8  bPwrOn2pwrGood; 2ms */
-    0x00,           /*  u8  bHubContrCurrent; 0 mA */
-    0x00,           /*  u8  DeviceRemovable; *** 7 Ports max *** */
-    0xff            /*  u8  PortPwrCtrlMask; *** 7 ports max *** */
+    0x00            /*  u8  bHubContrCurrent; 0 mA */
+
+    /* DeviceRemovable and PortPwrCtrlMask patched in later */
 };
 
 static int usb_hub_attach (USBDevice *hub, USBDevice *dev, int portnum)
@@ -260,6 +260,12 @@
         }
         ret = 0;
         break;
+    case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
+        if (value == 0 && index != 0x81) { /* clear ep halt */
+            goto fail;
+        }
+        ret = 0;
+        break;
     case DeviceOutRequest | USB_REQ_SET_FEATURE:
         if (value == USB_DEVICE_REMOTE_WAKEUP) {
             dev->remote_wakeup = 1;
@@ -282,6 +288,11 @@
         case USB_DT_CONFIG:
             memcpy(data, qemu_hub_config_descriptor, 
                    sizeof(qemu_hub_config_descriptor));
+
+            /* status change endpoint size based on number
+             * of ports */
+            data[22] = (s->nb_ports + 1 + 7) / 8;
+
             ret = sizeof(qemu_hub_config_descriptor);
             break;
         case USB_DT_STRING:
@@ -427,11 +438,29 @@
         }
         break;
     case GetHubDescriptor:
-        memcpy(data, qemu_hub_hub_descriptor, 
-               sizeof(qemu_hub_hub_descriptor));
-        data[2] = s->nb_ports;
-        ret = sizeof(qemu_hub_hub_descriptor);
-        break;
+        {
+            unsigned int n, limit, var_hub_size = 0;
+            memcpy(data, qemu_hub_hub_descriptor, 
+                   sizeof(qemu_hub_hub_descriptor));
+            data[2] = s->nb_ports;
+
+            /* fill DeviceRemovable bits */
+            limit = ((s->nb_ports + 1 + 7) / 8) + 7;
+            for (n = 7; n < limit; n++) {
+                data[n] = 0x00;
+                var_hub_size++;
+            }
+
+            /* fill PortPwrCtrlMask bits */
+            limit = limit + ((s->nb_ports + 7) / 8);
+            for (;n < limit; n++) {
+                data[n] = 0xff;
+                var_hub_size++;
+            }
+
+            ret = sizeof(qemu_hub_hub_descriptor) + var_hub_size;
+            break;
+        }
     default:
     fail:
         ret = USB_RET_STALL;
@@ -453,8 +482,11 @@
             unsigned int status;
             int i, n;
             n = (s->nb_ports + 1 + 7) / 8;
-            if (n > len)
+            if (len == 1) { /* FreeBSD uhub workaround */
+                n = 1;
+            } else if (n > len) {
                 return USB_RET_BABBLE;
+            }
             status = 0;
             for(i = 0; i < s->nb_ports; i++) {
                 port = &s->ports[i];
@@ -467,7 +499,7 @@
                 }
                 ret = n;
             } else {
-                ret = 0;
+                ret = USB_RET_NAK; /* usb_20 11.12.1 */
             }
         } else {
             goto fail;
@@ -541,6 +573,7 @@
         return NULL;
     dev->opaque=                s;
     dev->speed=                 USB_SPEED_FULL;
+    dev->handle_msg=            usb_generic_handle_msg;
     dev->handle_packet=         usb_hub_handle_packet;
     dev->handle_attach=         usb_hub_attach;
     dev->handle_reset=          usb_hub_handle_reset;
@@ -558,5 +591,5 @@
 
 void usb_hub_info(void) {
     term_printf("  Device usbhub, Manufacturer QEMU, Product QEMU USB Hub\n" );
-    term_printf ("     VendorID:ProductID 0000:0000, USB-Standard: 01.01\n");
+    term_printf ("     VendorID:ProductID 0000:0000, USB-Standard: 01.10\n");
 }
--- a/qemu/hw/usb-hid.c	2006-04-21 11:15:40.000000000 -0500
+++ b/qemu/hw/usb-hid.c	2006-04-21 15:31:55.000000000 -0500
@@ -515,6 +515,7 @@
         return NULL;
     dev->opaque=            s;
     dev->speed=             USB_SPEED_FULL;
+    dev->handle_msg=        usb_generic_handle_msg;
     dev->handle_packet=     usb_generic_handle_packet;
 
     dev->handle_reset=      usb_mouse_handle_reset;
@@ -536,6 +539,7 @@
         return NULL;
     dev->opaque=            s;
     dev->speed=             USB_SPEED_FULL;
+    dev->handle_msg=        usb_generic_handle_msg;
     dev->handle_packet=     usb_generic_handle_packet;
 
     dev->handle_reset=      usb_mouse_handle_reset;
@@ -549,5 +553,10 @@
 
 void usb_mouse_info() {
     term_printf("  Device mouse, Manufacturer QEMU, Product QEMU USB Mouse\n" );
-    term_printf ("     VendorID:ProductID 2706:0001, USB-Standard: 01.00\n");
+    term_printf ("     VendorID:ProductID 0627:0001, USB-Standard: 01.00\n");
+}
+
+void usb_tablet_info() {
+    term_printf("  Device tablet, Manufacturer QEMU, Product QEMU USB Tablet\n");
+    term_printf ("     VendorID:ProductID 0627:0001, USB-Standard: 01.00\n");
 }

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

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

Hello,

Lonnie Mendez wrote:
>
>   There are of course more bugs I've found.  Namely being able to
> usb_add any particular string with that string showing up as a new
> device even though no valid entry for it exists.
I have fixed this issue, also I have found the segfault on usb_del.
Patch is attached.
Next problem:
Linux does not recognize it, if I add a "tablet" while linux is allready
running. The attach is not delivered to the operating system.

[-- Attachment #2: lusb-upd2.diff --]
[-- Type: text/plain, Size: 4297 bytes --]

diff -Nur qemu-last-snapshot/hw/usb-hid.c qemu/hw/usb-hid.c
--- qemu-last-snapshot/hw/usb-hid.c	2006-04-22 10:23:40.000000000 +0200
+++ qemu/hw/usb-hid.c	2006-04-22 11:13:31.000000000 +0200
@@ -339,6 +339,13 @@
     return 1;
 }
 
+static int usb_mouse_handle_close(USBDevice *dev)
+{
+    USBMouseState *s = (USBMouseState *)dev->opaque;
+    qemu_free (s);
+    return 1;
+}
+
 static int usb_mouse_handle_control(USBDevice *dev, int request, int value,
                                   int index, int length, uint8_t *data)
 {
@@ -541,6 +548,7 @@
     dev->handle_packet=     usb_generic_handle_packet;
 
     dev->handle_reset=      usb_mouse_handle_reset;
+    dev->handle_close=      usb_mouse_handle_close;
     dev->handle_control=    usb_mouse_handle_control;
     dev->handle_data=       usb_mouse_handle_data;
     s->kind=                USB_MOUSE;
diff -Nur qemu-last-snapshot/hw/usb.c qemu/hw/usb.c
--- qemu-last-snapshot/hw/usb.c	2006-04-22 10:23:40.000000000 +0200
+++ qemu/hw/usb.c	2006-04-22 11:15:46.000000000 +0200
@@ -372,19 +372,27 @@
         return 1;
     USBDevice *dev= (*tree)->dev;
     for (;tmp != NULL; tmp= tmp->next) {
-        if( tmp == *tree ) {
+        if (tmp == *tree) {
+            if (dev != NULL){
+                if( dev->father != NULL &&
+                    dev->father->handle_attach 
+                                (dev->father, NULL, dev->father_port) < 0) {
+#ifdef DEBUG
+                    printf ("Could not dettach from father\n");
+#endif
+                    return -1;                                                
+                }
+                if (dev->handle_close(dev) < 0) {
+#ifdef DEBUG
+                    printf ("Could not close device\n");
+#endif
+                    return -2;
+                }
+            }
             if( last != NULL ) {
                 last->next= (*tree)->next;
             }
-            if( dev != NULL && dev->father != NULL &&             
-                !dev->father->handle_attach 
-                                (dev->father, NULL, dev->father_port)) {
-                return -1;
-            } else {
-                if( dev != NULL && !dev->handle_close(dev) )
-                    return -2;
-            }
-            free (*tree);
+            qemu_free (*tree);
             *tree= last;
             return 1;
         }
@@ -441,8 +449,11 @@
             if( tree->dev == NULL ) {
                 usb_remove_device(&tree);
                 return -1;
+            } else {
+                return 0;
             }
         }
+        return -1;
     } else if (strstr (tree->name, "host:") == tree->name) {
         /* we found a host device */
         tree->dev= usb_host_init (tree->name);
@@ -459,8 +470,10 @@
         /* we found a guest usb tablet */
         tree->dev = usb_tablet_init ();
         return add_usb_device (tree);
-    } 
-    return 1;
+    } else {
+        usb_remove_device(&tree);
+        return -1;
+    }
 }
 
 /* this function connects or removes devices according to usb_tree */
@@ -555,10 +568,11 @@
         dev->setup_index=       0;
         dev->handle_packet=     &usb_dummy_handle_packet;
         dev->handle_reset=      &usb_dummy_handle_reset;
+        dev->handle_close=      &usb_dummy_handle_close;   
         dev->handle_control=    &usb_dummy_handle_control;
         dev->handle_msg=        &usb_dummy_handle_msg;
         dev->handle_data=       &usb_dummy_handle_data;
-        dev->handle_attach=     &usb_dummy_handle_attach;   
+        dev->handle_attach=     &usb_dummy_handle_attach;
     }
     return dev;
 }
diff -Nur qemu-last-snapshot/vl.c qemu/vl.c
--- qemu-last-snapshot/vl.c	2006-04-22 10:23:40.000000000 +0200
+++ qemu/vl.c	2006-04-22 10:57:09.000000000 +0200
@@ -3274,6 +3274,7 @@
         usb_tree= tmp;
         tmp->next= NULL;
     }
+    tmp->dev= NULL;
     memcpy (tmp->name, devname, nameend-devname);
     tmp->name[nameend-devname+1]= '\0';
     memcpy (tmp->path, bus, 4);
@@ -3334,6 +3335,7 @@
             last->next= qemu_malloc (sizeof (USBTree));
             tree= last->next;
             tree->next= NULL;
+            tree->dev= NULL;
             strcpy (tree->name, name);
             strcpy( tree->path, treepath );
             tree->device_status= USB_ADD_DEVICE;

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

Hello,

I have made a new patch which includes all changes from yesterday,
because there were already inconsistencies Also attached is the log of
my USB scanner (The last lines are repeated for ever). It is recognized
but when the first bulk packet is send to it, I get a ETIMEDOUT Error
and I don't know why. I have already tried to give back a STALL and I
have also tried to clear the stall first ... .

With kind regards
Tino H. Seifert

patch:
http://217.20.126.200/tino/usb-2006-04-22.patch
scanner.log
http://217.20.126.200/tino/usb-scanner.log

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

nix.wie.weg@gmx.de wrote:

>I have fixed this issue, also I have found the segfault on usb_del.
>Patch is attached.
>Next problem:
>Linux does not recognize it, if I add a "tablet" while linux is allready
>running. The attach is not delivered to the operating system.
>  
>
   Hm.  There also seems to be some unchecked bounds when adding devices:

(qemu) usb_add host:001:021
Could not add USB Device 'host:001:021'
(qemu) info usb
     Controller 001: uhci
          001:001 = mouse
          001:002 = tablet
          001:001 = host:001:021
     Summary: 1 USB Controller, 3 USB Devices
(qemu)

   Also I've a question on where the emulated hub comes into play.  
Specifically how do you currently add new devices to it?  The ,addto 
syntax doesn't seem to apply to the emulated hub as it takes a bus 
address (the controller) and an address on the root hub.  But the hub 
itself would be centrally connected to a bus number identifying a 
controller and not have its own.  It seems we've presently traded a hub 
with dynamic ports for 2 static ones.  I'm glad there is a lot more 
internal tracking here as with the previous system you could usb_add the 
same host device several times and it wouldn't care.  This should also 
make it easier to reconnect the ps2 mouse when the hid mouse is dettached.

   Adding a device by Vendor id and Product id doesn't seem to work.  If 
this is intentional then perhaps the functionality should be restored.  
qemu users will find this convenient as it is a fixed/static 
identification for a single usb device which is most likely going to be 
the common usage.

   In regards to the linux guest problem I'm not seeing it.  On an older 
debian SID install which is currently using linux kernel 2.6.14 all 
seems well.  The code here is merely the first patch combined with CVS 
along with the already submitted patches.  I'll try to find some 
variation to test with.  In which distro is this happening?

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

Hello Lonnie,

Lonnie Mendez wrote:
> nix.wie.weg@gmx.de wrote:
>
>> Next problem:
>> Linux does not recognize it, if I add a "tablet" while linux is allready
>> running. The attach is not delivered to the operating system.
I have solved the problem, it was located in a rejected diff. (it was
the reason I made the new patch earlier)
> (qemu) usb_add host:001:021
> Could not add USB Device 'host:001:021'
> (qemu) info usb
>     Controller 001: uhci
>          001:001 = mouse
>          001:002 = tablet
>          001:001 = host:001:021
>     Summary: 1 USB Controller, 3 USB Devices
> (qemu)
>
solved see attached patch.
>   Also I've a question on where the emulated hub comes into play. 
> Specifically how do you currently add new devices to it?  The ,addto
> syntax doesn't seem to apply to the emulated hub as it takes a bus
> address (the controller) and an address on the root hub.  But the hub
> itself would be centrally connected to a bus number identifying a
> controller and not have its own.  It seems we've presently traded a
> hub with dynamic ports for 2 static ones.  I'm glad there is a lot
> more internal tracking here as with the previous system you could
> usb_add the same host device several times and it wouldn't care.  This
> should also make it easier to reconnect the ps2 mouse when the hid
> mouse is dettached.
>
Let me explain it on an example, lets assume you add
usb_add usbhub to 001:001
then you should be able (in fact it is absolutly untested)
usb_add host:2:3 to 001:001:001

it should even work something like:
usb_add  usbhub to 001:001:002
usb_add  host:2:2 to 001:001:002:001

you see, you build up a path. Maximum depth at the moment ist 8.
>   Adding a device by Vendor id and Product id doesn't seem to work. 
> If this is intentional then perhaps the functionality should be
> restored.  qemu users will find this convenient as it is a
> fixed/static identification for a single usb device which is most
> likely going to be the common usage.
I aggree with that, and normaly it should work. The complete name
"host:AAAxBBB" is delivered to usb_host_init(), I will look at it.
>   In regards to the linux guest problem I'm not seeing it.  On an
> older debian SID install which is currently using linux kernel 2.6.14
> all seems well.  The code here is merely the first patch combined with
> CVS along with the already submitted patches.  I'll try to find some
> variation to test with.  In which distro is this happening?
With kind regards,
Tino H. Seifert

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

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

Sorry have forgotten the patch.


>   

[-- Attachment #2: multiple-portadds.patch --]
[-- Type: text/x-patch, Size: 4223 bytes --]

--- qemu-2006-04-22/hw/usb.c	2006-04-22 14:44:21.000000000 +0200
+++ qemu/hw/usb.c	2006-04-22 17:24:06.000000000 +0200
@@ -403,73 +403,74 @@
 
 /* this function adds a newly created device and cares for the attach and
    handles all errors */
-int add_usb_device (USBTree *tree) 
+int add_usb_device (USBTree **tree) 
 {
     char father[USB_PATH_MAX_LENGTH];
     char child[USB_PATH_MAX_LENGTH];
     int port;
     USBDevice *fatherdev;
-    if (tree->dev == NULL) {
-        usb_remove_device(&tree);
+    if ((*tree)->dev == NULL) {
+        usb_remove_device(tree);
         return -1;
     }
-    usb_str_father_and_child (tree->path, father, child);
+    usb_str_father_and_child ((*tree)->path, father, child);
     fatherdev= usb_find_device (father);
     if (!fatherdev ) {
 #ifdef DEBUG                
-        printf ( "Could not find father father USB for %s.\n", tree->name);
+        printf ( "Could not find father father USB for %s.\n", (*tree)->name);
 #endif
-        usb_remove_device(&tree);
+        usb_remove_device(tree);
         return -1;
     }
-    tree->dev->father= fatherdev;
     port= atoi (child)-1;
-    port= fatherdev->handle_attach(fatherdev, tree->dev, port);
+    port= fatherdev->handle_attach(fatherdev, (*tree)->dev, port);
     if( port < 0 ) {
 #ifdef DEBUG                
-        printf ( "Could not attach USB host device %s.\n", tree->name);
-#endif          
+        printf ( "Could not attach USB host device %s.\n", (*tree)->name);
+#endif  
+        usb_remove_device(tree);        
         return -1;
     }
-    tree->dev->father_port= port;
-    snprintf( tree->path, USB_PATH_MAX_LENGTH,"%s:%03i", father, port+1);
+    (*tree)->dev->father= fatherdev;
+    (*tree)->dev->father_port= port;
+    snprintf( (*tree)->path, USB_PATH_MAX_LENGTH,"%s:%03i", father, port+1);
     return 0;
 }
 
 
 /* this function does decide which device should be added, if you write a new
    device driver than you must add it here */
-int usb_add_device( PCIBus *pci_bus, USBTree *tree ) 
+int usb_add_device (PCIBus *pci_bus, USBTree **tree) 
 {
-    if (strcmp(tree->name, "uhci") == 0) {
+    if (strcmp((*tree)->name, "uhci") == 0) {
         /* uhci controller found */
         /* controller are allways root devices, so no add is tried */
         if (pci_bus != NULL) {
-            tree->dev= usb_uhci_init (pci_bus);
-            if( tree->dev != NULL ) {
+            (*tree)->dev= usb_uhci_init (pci_bus);
+            if( (*tree)->dev != NULL ) {
                 return 0;
             }
         }
-        usb_remove_device(&tree);
+        usb_remove_device(tree);
         return -1;
-    } else if (strstr (tree->name, "host:") == tree->name) {
+    } else if (strstr ((*tree)->name, "host:") == (*tree)->name) {
         /* we found a host device */
-        tree->dev= usb_host_init (tree->name);
+        (*tree)->dev= usb_host_init ((*tree)->name);
         return add_usb_device (tree);
-    } else if (strcmp (tree->name,"usbhub") == 0) {
+    } else if (strcmp ((*tree)->name,"usbhub") == 0) {
         /* we found a guest usb hub */
-        tree->dev= usb_hub_init (4);
+        (*tree)->dev= usb_hub_init (4);
         return add_usb_device (tree);
-    } else if (strcmp (tree->name,"mouse") == 0) {
+    } else if (strcmp ((*tree)->name,"mouse") == 0) {
         /* we found a guest usb mouse */
-        tree->dev= usb_mouse_init ();
+        (*tree)->dev= usb_mouse_init ();
         return add_usb_device (tree);
-    } else if (strcmp (tree->name,"tablet") == 0) {
+    } else if (strcmp ((*tree)->name,"tablet") == 0) {
         /* we found a guest usb tablet */
-        tree->dev = usb_tablet_init ();
+        (*tree)->dev = usb_tablet_init ();
         return add_usb_device (tree);
     } else {
-        usb_remove_device(&tree);
+        usb_remove_device(tree);
         return -1;
     }
 }
@@ -482,7 +483,7 @@
     for (;tmp != NULL; tmp= tmp->next) {
         switch (tmp->device_status) {
         case USB_ADD_DEVICE:
-            if (usb_add_device (pci_bus, tmp) < 0)
+            if (usb_add_device (pci_bus, &tmp) < 0)
                 ret= -1;
             else
                 tmp->device_status= USB_STANDARD_DEVICE;

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

>
>   Adding a device by Vendor id and Product id doesn't seem to work. 
> If this is intentional then perhaps the functionality should be
> restored.  qemu users will find this convenient as it is a
> fixed/static identification for a single usb device which is most
> likely going to be the common usage.
>
In which case did it not work, when I test it, it is functioning (only
with libusb)
usb_add host:04b6x0005

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

nix.wie.weg@gmx.de wrote:

>In which case did it not work, when I test it, it is functioning (only
>with libusb)
>usb_add host:04b6x0005
>
   Sorry about that.  I was used to the previous syntax:
usb_add host:04b6:0005

   My guess is that the new syntax is to distinguish it from the 
busaddr:addr syntax?  It would seem checking for length would 
differentiate the two in that case.

   For the linux guest problem which distro is being used?

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

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



Lonnie Mendez wrote:
> nix.wie.weg@gmx.de wrote:
>
> Sorry about that.  I was used to the previous syntax:
> usb_add host:04b6:0005
>
>   My guess is that the new syntax is to distinguish it from the
> busaddr:addr syntax?  It would seem checking for length would
> differentiate the two in that case.
Yes maybe, but not a real good solution, especially if you know that
with the solution we have now, you can omit leading zeros.
>
>   For the linux guest problem which distro is being used?
I don't have this problem any more. It was an incomplete merged patch.

I have attached a small patch, with which you can really add a device to
a hub, if this is then working is another question, but it should only
depend on the implementation of usb-hub.c

With kind regards,
Tino H. Seifert

[-- Attachment #2: lusb-upd3.diff --]
[-- Type: text/plain, Size: 2825 bytes --]

diff -Nur qemu-last-snapshot/hw/usb-hub.c qemu/hw/usb-hub.c
--- qemu-last-snapshot/hw/usb-hub.c	2006-04-22 17:53:27.000000000 +0200
+++ qemu/hw/usb-hub.c	2006-04-22 18:24:11.000000000 +0200
@@ -172,18 +172,16 @@
 
 static int usb_hub_attach (USBDevice *hub, USBDevice *dev, int portnum)
 {
-    USBHubState *s = (USBHubState *)(dev->father->opaque);
+    USBHubState *s = (USBHubState *)(hub->opaque);
     USBHubPort  *port;
     int i;
     
     if (dev) {
-         if( portnum >= MAX_PORTS ) {
+         if (portnum >= MAX_PORTS || portnum < 0) {
 #ifdef DEBUG        
-            printf( "Not so many ports available on USB hub \n" );
+            printf( "This usb hub port does not exist.\n" );
 #endif
             portnum= 0;
-        } else if( portnum < 0 ){
-            portnum= 0;
         }
         if (s->ports[portnum].dev != NULL) {
 #ifdef DEBUG
@@ -201,7 +199,7 @@
             }
             if( portnum < 0 ) {
 #ifdef DEBUG    
-                printf( "Could not find a Hub Port to connect to!" );        
+                printf( "Could not find a Hub Port to connect to! \n" );        
 #endif
                 return 0;
             }
@@ -215,7 +213,8 @@
         else
             port->wPortStatus &= ~PORT_STAT_LOW_SPEED;
         port->dev = dev;
-        return 1;    
+        port->dev->handle_msg (port->dev, USB_MSG_ATTACH);
+        return portnum;    
     } else {
         port = &s->ports[portnum];
         dev = port->dev;
@@ -226,11 +225,13 @@
                 port->wPortStatus &= ~PORT_STAT_ENABLE;
                 port->wPortChange |= PORT_STAT_C_ENABLE;
             }
+            port->dev->handle_msg(port->dev, USB_MSG_DETACH);
             port->dev = NULL;
-            return 1;
+            return portnum;
+        } else {
+            return -1;
         }
     }
-    return -1;
 }
 
 int usb_hub_handle_reset(USBDevice *dev)
diff -Nur qemu-last-snapshot/usb-libusb.c qemu/usb-libusb.c
--- qemu-last-snapshot/usb-libusb.c	2006-04-22 17:53:29.000000000 +0200
+++ qemu/usb-libusb.c	2006-04-22 18:03:47.000000000 +0200
@@ -230,8 +230,8 @@
 }
 
 /* the syntax is : 
-   'bus.addr' (decimal numbers) or 
-   'vendor_id:product_id' (hexa numbers) */
+   'bus:addr' (decimal numbers) or 
+   'vendor_idxproduct_id' (hexa numbers) */
 void usb_host_find_device(const char *devname, struct usb_device **device)
 {
     *device= NULL;
@@ -399,7 +399,7 @@
     usbVersion[1]= (bcd >> 8) & 7;
     usbVersion[2]= (bcd >> 4) & 7;
     usbVersion[3]= bcd & 7;
-    term_printf("     VendorID:ProductID %04x:%04x, USB-Standard: %i%i.%i%i\n",
+    term_printf("     VendorID:ProductID %04xx%04x, USB-Standard: %i%i.%i%i\n",
                 dev->descriptor.idVendor, dev->descriptor.idProduct, 
                 usbVersion[0], usbVersion[1], usbVersion[2], usbVersion[3] );
 }

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

nix.wie.weg@gmx.de wrote:

>Lonnie Mendez wrote:
>  
>
>>nix.wie.weg@gmx.de wrote:
>>
>>Sorry about that.  I was used to the previous syntax:
>>usb_add host:04b6:0005
>>
>>  My guess is that the new syntax is to distinguish it from the
>>busaddr:addr syntax?  It would seem checking for length would
>>differentiate the two in that case.
>>    
>>
>Yes maybe, but not a real good solution, especially if you know that
>with the solution we have now, you can omit leading zeros.
>  
>
   Aye.

>
>I have attached a small patch, with which you can really add a device to
>a hub, if this is then working is another question, but it should only
>depend on the implementation of usb-hub.c
>  
>
   That works but not in the literal means of the attached device 
actually functioning (see below).  Here is something interesting:

(qemu) usb_add mouse
     Controller 001: uhci
          001:001 = mouse
     Summary: 1 USB Controller, 1 USB Devices
(qemu) usb_add tablet,addto 001:001:001
(qemu) info usb
     Controller 001: uhci
          001:001 = mouse
               001:001:001 = tablet
     Summary: 1 USB Controller, 2 USB Devices
(qemu)

  The device attached on the hub doesn't seem to be receiving 
transactions (like before with the emulated devices):

frame 1227: pid=SETUP addr=0x00 ep=0 len=8
     data_out= 80 06 00 01 00 00 40 00
uhci readw port=0x0002 val=0x0001
uhci writew port=0x0002 val=0x0001
uhci readw port=0x0006 val=0x04cc
frame 1228: pid=SETUP addr=0x00 ep=0 len=8
     data_out= 80 06 00 01 00 00 40 00
frame 1229: pid=SETUP addr=0x00 ep=0 len=8
     data_out= 80 06 00 01 00 00 40 00
uhci readw port=0x0002 val=0x0002
uhci writew port=0x0002 val=0x0002

   There are some problems with the port status request traffic from the 
hub as well:

frame 1174: pid=SETUP addr=0x03 ep=0 len=8
     data_out= 23 03 04 00 01 00 00 00
error usb_generic_handle_packet: unknown USB-Token - 258
     ret=0

   The interface is also giving incorrect errors at certain times:

(qemu) usb_add usbhub
usb_add: extraneous characters at the end of line
(qemu)


   Attached is a patch to get the linux redirector working with the 
changes.  It has a few other modifications as well but nothing radical.  
The default redirector for linux host was changed to the linux 
redirector as well (just a heads up).  I'm still using the libusb 
redirector for testing here but it's good to restore the linux 
redirector as default on linux host.


[-- Attachment #2: lusb-upd4.diff --]
[-- Type: text/plain, Size: 13121 bytes --]

--- a/qemu/configure	2006-04-22 21:58:54.000000000 -0500
+++ b/qemu/configure	2006-04-22 22:00:43.000000000 -0500
@@ -105,7 +105,7 @@
 ;;
 MINGW32*)
 mingw32="yes"
-usb="linux"
+usb="generic"
 ;;
 FreeBSD)
 bsd="yes"
@@ -130,7 +130,7 @@
 oss="yes"
 linux="yes"
 user="yes"
-usb="generic"
+usb="linux"
 if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
     kqemu="yes"
 fi
--- a/qemu/usb-linux.c	2006-04-22 21:58:54.000000000 -0500
+++ b/qemu/usb-linux.c	2006-04-22 22:02:10.000000000 -0500
@@ -42,11 +42,14 @@
     void *data;
 };
 
-typedef int USBScanFunc(void *opaque, int bus_num, int addr, int class_id,
+typedef int USBScanFunc(void *opaque, int bus_num, int addr,
                         int vendor_id, int product_id, 
-                        const char *product_name, int speed);
+                        const char *spec_version,
+                        const char *manf_string,
+                        const char *prod_string);
 static int usb_host_find_device(int *pbus_num, int *paddr, 
                                 const char *devname);
+int usb_host_handle_close(USBDevice *opaque);
 
 //#define DEBUG
 
@@ -63,32 +66,19 @@
             "error code:    %i\n", ret );
 # endif
     switch(ret) {
-    case -ETIMEDOUT:
+    case ETIMEDOUT:
 # ifdef DEBUG
         printf( "USB_ERROR: ETIMEDOUT\n" );
 #endif
         return USB_RET_NAK;
         break;
-    case -EIO: 
-# ifdef DEBUG
-        printf( "USB_ERROR: EIO\n" );
-#endif
-        /* stall condition on win32 */
-        /* XXX: clear stall */
-        return USB_RET_STALL;
-        break;
-    case -EBUSY:
-        /* stall on linux */
-        /* XXX: clear stall */
-        return USB_RET_STALL;
-        break;
-    case -EPIPE:
+    case EPIPE:
     default:
         return USB_RET_STALL;
     }
 }
 
-static void usb_host_handle_reset(USBDevice *dev)
+static int usb_host_handle_reset(USBDevice *dev)
 {
 #if 0
     USBHostDevice *s = (USBHostDevice *)dev;
@@ -96,6 +86,7 @@
        done by the host OS */
     ioctl(s->fd, USBDEVFS_RESET);
 #endif
+    return USB_RET_ACK;
 } 
 
 static int usb_host_handle_control(USBDevice *dev,
@@ -118,7 +109,7 @@
     ct.data = data;
     ret = ioctl(s->fd, USBDEVFS_CONTROL, &ct);
     if (ret < 0) {
-        usb_host_handle_error( ret );
+        return usb_host_handle_error( errno );
     } else {
         return ret;
     }
@@ -142,14 +133,14 @@
     bt.data = data;
     ret = ioctl(s->fd, USBDEVFS_BULK, &bt);
     if (ret < 0) {
-        usb_host_handle_error( ret );
+        return usb_host_handle_error( errno );
     } else {
         return ret;
     }
 }
 
 /* XXX: exclude high speed devices or implement EHCI */
-USBDevice *usb_host_device_open(const char *devname)
+USBDevice *usb_host_init(const char *devname)
 {
     int fd, interface, ret, i;
     USBHostDevice *hostdev;
@@ -162,7 +153,7 @@
 
     if (usb_host_find_device(&bus_num, &addr, devname) < 0) 
         return NULL;
-    
+
     snprintf(buf, sizeof(buf), USBDEVFS_PATH "/%03d/%03d", 
              bus_num, addr);
     fd = open(buf, O_RDWR);
@@ -272,12 +263,16 @@
     return q - buf;
 }
 
-void usb_host_device_close(USBDevice *opaque)
+int usb_host_handle_close(USBDevice *opaque)
 {
     USBHostDevice *s = (USBHostDevice *)opaque;
 
-    if (s->fd >= 0)
+    if (s->fd >= 0) {
        close(s->fd);
+       return 1;
+    }
+
+    return -1;
 }
 
 static int usb_host_scan(void *opaque, USBScanFunc *func)
@@ -285,9 +280,11 @@
     FILE *f;
     char line[1024];
     char buf[1024];
-    int bus_num, addr, speed, device_count, class_id, product_id, vendor_id;
+    int bus_num, addr, device_count, product_id, vendor_id;
     int ret;
-    char product_name[512];
+    char manf_name[512];
+    char prod_name[512];
+    char spec_version[6];
     
     f = fopen(USBDEVFS_PATH "/devices", "r");
     if (!f) {
@@ -295,7 +292,7 @@
         return 0;
     }
     device_count = 0;
-    bus_num = addr = speed = class_id = product_id = vendor_id = 0;
+    bus_num = addr = product_id = vendor_id = 0;
     ret = 0;
     for(;;) {
         if (fgets(line, sizeof(line), f) == NULL)
@@ -305,8 +302,8 @@
         if (line[0] == 'T' && line[1] == ':') {
             if (device_count && (vendor_id || product_id)) {
                 /* New device.  Add the previously discovered device.  */
-                ret = func(opaque, bus_num, addr, class_id, vendor_id, 
-                           product_id, product_name, speed);
+                ret = func(opaque, bus_num, addr, vendor_id, product_id,
+                           spec_version, manf_name, prod_name);
                 if (ret)
                     goto the_end;
             }
@@ -316,19 +313,17 @@
             if (get_tag_value(buf, sizeof(buf), line, "Dev#=", " ") < 0)
                 goto fail;
             addr = atoi(buf);
-            if (get_tag_value(buf, sizeof(buf), line, "Spd=", " ") < 0)
-                goto fail;
-            if (!strcmp(buf, "480"))
-                speed = USB_SPEED_HIGH;
-            else if (!strcmp(buf, "1.5"))
-                speed = USB_SPEED_LOW;
-            else
-                speed = USB_SPEED_FULL;
-            product_name[0] = '\0';
-            class_id = 0xff;
+            strcpy(spec_version, "01.10");
+            strcpy(manf_name, "unknown");
+            strcpy(prod_name, "unknown");
             device_count++;
             product_id = 0;
             vendor_id = 0;
+        } else if (line[0] == 'D' && line[1] == ':') {
+            if (get_tag_value(&buf[1], sizeof(buf) - 1, line, "Ver=", " ") < 0)
+                goto fail;
+            buf[0] = '0';
+            pstrcpy(spec_version, sizeof(spec_version), buf);
         } else if (line[0] == 'P' && line[1] == ':') {
             if (get_tag_value(buf, sizeof(buf), line, "Vendor=", " ") < 0)
                 goto fail;
@@ -337,20 +332,22 @@
                 goto fail;
             product_id = strtoul(buf, NULL, 16);
         } else if (line[0] == 'S' && line[1] == ':') {
-            if (get_tag_value(buf, sizeof(buf), line, "Product=", "") < 0)
-                goto fail;
-            pstrcpy(product_name, sizeof(product_name), buf);
-        } else if (line[0] == 'D' && line[1] == ':') {
-            if (get_tag_value(buf, sizeof(buf), line, "Cls=", " (") < 0)
-                goto fail;
-            class_id = strtoul(buf, NULL, 16);
+            if (get_tag_value(buf, sizeof(buf), line, "Manufacturer=", "") < 0) {
+                if (get_tag_value(buf, sizeof(buf), line, "Product=", "") < 0) {
+                    goto fail;
+                } else {
+                    pstrcpy(prod_name, sizeof(prod_name), buf);
+                }
+            } else {
+                pstrcpy(manf_name, sizeof(manf_name), buf);
+            }
         }
     fail: ;
     }
     if (device_count && (vendor_id || product_id)) {
         /* Add the last device.  */
-        ret = func(opaque, bus_num, addr, class_id, vendor_id, 
-                   product_id, product_name, speed);
+        ret = func(opaque, bus_num, addr, vendor_id, product_id, spec_version,
+                   manf_name, prod_name);
     }
  the_end:
     fclose(f);
@@ -364,10 +361,11 @@
     int addr;
 } FindDeviceState;
 
-static int usb_host_find_device_scan(void *opaque, int bus_num, int addr, 
-                                     int class_id,
-                                     int vendor_id, int product_id, 
-                                     const char *product_name, int speed)
+static int usb_host_find_device_scan(void *opaque, int bus_num, int addr,
+                                     int vendor_id, int product_id,
+                                     const char *spec_version,
+                                     const char *manf_string,
+                                     const char *prod_string)
 {
     FindDeviceState *s = opaque;
     if (vendor_id == s->vendor_id &&
@@ -386,108 +384,38 @@
 static int usb_host_find_device(int *pbus_num, int *paddr, 
                                 const char *devname)
 {
-    const char *p;
-    int ret;
     FindDeviceState fs;
 
-    p = strchr(devname, '.');
-    if (p) {
-        *pbus_num = strtoul(devname, NULL, 0);
-        *paddr = strtoul(p + 1, NULL, 0);
-        return 0;
-    }
-    p = strchr(devname, ':');
-    if (p) {
-        fs.vendor_id = strtoul(devname, NULL, 16);
-        fs.product_id = strtoul(p + 1, NULL, 16);
-        ret = usb_host_scan(&fs, usb_host_find_device_scan);
-        if (ret) {
-            *pbus_num = fs.bus_num;
-            *paddr = fs.addr;
-            return 0;
+    if (sscanf(devname, "host:%03d:%03d", pbus_num, paddr) != 2) {
+        if (sscanf(devname, "host:%04xx%04x", &fs.vendor_id, &fs.product_id) == 2) {
+            if (usb_host_scan(&fs, usb_host_find_device_scan)) {
+                *pbus_num = fs.bus_num;
+                *paddr = fs.addr;
+                return 0;
+            } else {
+                return -1;
+            }
+        } else {
+            return -1;
         }
+    } else {
+        return 0;
     }
-    return -1;
 }
 
 /**********************/
 /* USB host device info */
 
-struct usb_class_info {
-    int class;
-    const char *class_name;
-};
-
-static const struct usb_class_info usb_class_info[] = {
-    { USB_CLASS_AUDIO, "Audio"},
-    { USB_CLASS_COMM, "Communication"},
-    { USB_CLASS_HID, "HID"},
-    { USB_CLASS_HUB, "Hub" },
-    { USB_CLASS_PHYSICAL, "Physical" },
-    { USB_CLASS_PRINTER, "Printer" },
-    { USB_CLASS_MASS_STORAGE, "Storage" },
-    { USB_CLASS_CDC_DATA, "Data" },
-    { USB_CLASS_APP_SPEC, "Application Specific" },
-    { USB_CLASS_VENDOR_SPEC, "Vendor Specific" },
-    { USB_CLASS_STILL_IMAGE, "Still Image" },
-    { USB_CLASS_CSCID, 	"Smart Card" },
-    { USB_CLASS_CONTENT_SEC, "Content Security" },
-    { -1, NULL }
-};
-
-static const char *usb_class_str(uint8_t class)
-{
-    const struct usb_class_info *p;
-    for(p = usb_class_info; p->class != -1; p++) {
-        if (p->class == class)
-            break;
-    }
-    return p->class_name;
-}
-
-void usb_info_device(int bus_num, int addr, int class_id,
-                     int vendor_id, int product_id, 
-                     const char *product_name,
-                     int speed)
-{
-    const char *class_str, *speed_str;
-
-    switch(speed) {
-    case USB_SPEED_LOW: 
-        speed_str = "1.5"; 
-        break;
-    case USB_SPEED_FULL: 
-        speed_str = "12"; 
-        break;
-    case USB_SPEED_HIGH: 
-        speed_str = "480"; 
-        break;
-    default:
-        speed_str = "?"; 
-        break;
-    }
-
-    term_printf("  Device %d.%d, speed %s Mb/s\n", 
-                bus_num, addr, speed_str);
-    class_str = usb_class_str(class_id);
-    if (class_str) 
-        term_printf("    %s:", class_str);
-    else
-        term_printf("    Class %02x:", class_id);
-    term_printf(" USB device %04x:%04x", vendor_id, product_id);
-    if (product_name[0] != '\0')
-        term_printf(", %s", product_name);
-    term_printf("\n");
-}
-
 static int usb_host_info_device(void *opaque, int bus_num, int addr, 
-                                int class_id,
-                                int vendor_id, int product_id, 
-                                const char *product_name,
-                                int speed)
-{
-    usb_info_device(bus_num, addr, class_id, vendor_id, product_id,
-                    product_name, speed);
+                                int vendor_id, int product_id,
+                                const char *spec_ver,
+                                const char *manf_string,
+                                const char *prod_string)
+{
+    term_printf("  Device host:%03d:%03d, Manufacturer %s, Product %s\n",
+                bus_num, addr, manf_string, prod_string );
+    term_printf("     VendorID:ProductID %04xx%04x, USB-Standard: %s\n",
+                vendor_id, product_id, spec_ver );
     return 0;
 }
 
--- a/qemu/usb-libusb.c	2006-04-22 21:59:14.000000000 -0500
+++ b/qemu/usb-libusb.c	2006-04-22 22:03:02.000000000 -0500
@@ -368,12 +368,7 @@
 /**********************/
 /* USB host device info */
 
-struct usb_class_info {
-    int class;
-    const char *class_name;
-};
 
- 
 void usb_host_info_device( struct usb_bus *bus, struct usb_device *dev, struct usb_dev_handle *handle )
 {
     char str1[256], str2[256];
--- a/qemu/hw/usb-hub.c	2006-04-22 21:59:14.000000000 -0500
+++ b/qemu/hw/usb-hub.c	2006-04-22 13:51:23.000000000 -0500
@@ -177,7 +177,7 @@
     int i;
     
     if (dev) {
-         if (portnum >= MAX_PORTS || portnum < 0) {
+         if (portnum >= s->nb_ports || portnum < 0) {
 #ifdef DEBUG        
             printf( "This usb hub port does not exist.\n" );
 #endif
@@ -188,7 +188,7 @@
             printf( "Hub Port %i is allready occupied! \n", portnum );
 #endif
             portnum= -1;
-            for (i= 0; i < MAX_PORTS; i++) {
+            for (i= 0; i < s->nb_ports; i++) {
                 if (s->ports[i].dev == NULL) {
 #ifdef DEBUG
                     printf( "Will use Hub Port %i instead! \n", i );

Re: [Qemu-devel] Large USB patch

[Fabrice Bellard]

Hi,

Could you make a small patch containing just the bug fixes of the state 
machine ?

Concerning your API changes, I am relunctant to include them now, but my 
mind can evolve. An API change that I would consider as very useful 
could be to be able to make asynchronous USB I/Os to avoid blocking QEMU 
while doing host USB I/Os.

Regards,

Fabrice.

nix.wie.weg@gmx.de wrote:
> Hello everybody,
> 
> I have extended the USB support for Qemu. The patch is included and
> compiles just fine against todays cvs repository. As explained below,
> this patch touches 15 different files, which makes it not so easy to
> keep it applying on that very fast developing project. Thats why I would
> ask you to test it quickly and apply it to cvs as soon as possible.
> 
> With this patch applied I could detect a USB Epson Scanner and a USB
> Epson Printer from Windows 98 + XP and I could even print pages with
> the printer (see known problems below).
> 
> reasons for this patch:
> I was looking for a way to address my USB printer with windows while
> working on linux. As I started the work I had to recognize that my
> printer was not even detected under qemu. So I started to work on it.
> 
> changes I made:
> First I eliminated all potential error sources, which could be avoided.
> One of these sources where the
> 
> 1. usb-hub - which I transfered to an extra file usb-hub.c, then I
> added the usb-libusb.c devices directly to the UHCI controller this
> introduced the changes of 2)
> 2. I enhanced the usb add device potentialities so that you should be
> able to add a device to the UHCI controller. This device can be a usb
> hub or a usb device. Behind this device you can add another hub or
> device ... (remember this feature is now theoretically possible but not
> tested yet), this let to a new usb syntax:
> #$ qemu -usb controller=uhci,busnum=001 device=host:2:3,addto=001:001
> (the short form is:
> #$ qemu -usb controller=uhci -usb device=host:2:3)
> so you can build up a complete usb tree.
> 3. I changed the usb_generic_handle_packet() function and implemented a
> state machine, which is able to handle the usb packets correct.
> 4. I changed the linux standard usb-host library to libusb. I personally
> will always prefer a portable solution if possible.
> 5. I changed large parts of usb-libusb.c. Some changes were made because
> I found the source very awkward, some others because I fixed errors and
> some because I changed the general usb api (see item 2).
> 6. I tried to join as many usb functions as possible to the usb related
> files. So that hopefully nobody has to change 15 files again.
> 7. I made minor changes to usb-uhci - mainly I applied the new api and
> changed the handling of special messages like usb_reset or usb_attach
> 8. I made the necessary changes to usb-hid.c and usb-hub.c
> 9. I wrote a lot of source comments
> 
> this patch breaks some things:
> Sorry guys but I could not fix all of it, so I need your help, I didn't
> want to destroy anybodys work, but the new api makes it necessary to
> change some files:
> 1. usb-linux.c and usb-bsd.c will not work without adoption of the new api
> 2. I did not test usb-hid and usb-hub
> 
> known problems:
> 1. under linux the uhci controller reports an error if no usb device is
> connected
> 2. the printer and the scanner are recognized under Windows 98/XP and
> Linux, but the scanner goes into STALL state as soon as a packet in
> usb_write_bulk() or usb_read_bulk()
> 3. the libusb usb_host_reset() function does not work as expected and I
> don't know why (i have commented out this part of the source)
> 4. the printer stops printing on large images and is then in a state,
> where it will not resume his work (probably a timing issue)
> 
> With kind regards,
> Tino H. Seifert
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

Hello Fabrice,

I can understand, that you are not so happy with such an huge patch. But
sorry it can't be done with just patching the state machine. The main
problem is, that the cvs source needs the usbhub. But this hub is not
operational at its current state. So in fact whenever we try to find a
usb problem, we have to look in at least 4 completly different USB
components (uhci controller, usb state machine, usb hub, usb device). It
is a great help to reduce these components by one. Now if you go so far,
you have to touch all these 15 files and then it makes no sence not to
implement a better, and more flexible api. I have done this work and I
was successful, as in the last days we (Lonnie Mendez and I) have found
a lot of bugs. Some bugs were surely introduced by the new api, but we
have also fixed bugs which were simply not detected earlier because the
USB system was never bevor working so good. Today I have even detected
why my usb scanner was not recognised. I will add the patch later on.

So for the foregoing reasons, I will clearly reject to produce a patch
which will only cover the state machine. I have worked very hard on that
patch and some times you have to take what you get :)

But as I said before I can understand, that it is a problem to add such
a huge patch. So I want to submit the following offer: I and maybe also
Lonnie Mendez (hey Lonnie this is a request for some more of your
wonderful help :) ) will test the patch one week longer and get it a
little more stable. On saturday I will make another comprehensive patch
and you will please add it to cvs. Is that an offer?

Thanks for your time and your work so far,
Tino H. Seifert

PS: Your feature request is noted and I will look into it after we have
successfully dealt with the issues above. At the moment I'm not sure if
it is possible at all.

Fabrice Bellard wrote:
> Hi,
>
> Could you make a small patch containing just the bug fixes of the
> state machine ?
>
> Concerning your API changes, I am relunctant to include them now, but
> my mind can evolve. An API change that I would consider as very useful
> could be to be able to make asynchronous USB I/Os to avoid blocking
> QEMU while doing host USB I/Os.
>
> Regards,
>
> Fabrice.
>
>
>

Re: [Qemu-devel] Large USB patch

[nix.wie.weg]

Hello,

Lonnie Mendez wrote:
>
>   That works but not in the literal means of the attached device
> actually functioning (see below).  Here is something interesting:
>
> (qemu) usb_add mouse
>     Controller 001: uhci
>          001:001 = mouse
>     Summary: 1 USB Controller, 1 USB Devices
> (qemu) usb_add tablet,addto 001:001:001
> (qemu) info usb
>     Controller 001: uhci
>          001:001 = mouse
>               001:001:001 = tablet
>     Summary: 1 USB Controller, 2 USB Devices
> (qemu)
I have changed that, the usb_dummy_ funtions did return 0, but must
return -1
>  The device attached on the hub doesn't seem to be receiving
> transactions (like before with the emulated devices):
>
> frame 1227: pid=SETUP addr=0x00 ep=0 len=8
>     data_out= 80 06 00 01 00 00 40 00
> uhci readw port=0x0002 val=0x0001
> uhci writew port=0x0002 val=0x0001
> uhci readw port=0x0006 val=0x04cc
> frame 1228: pid=SETUP addr=0x00 ep=0 len=8
>     data_out= 80 06 00 01 00 00 40 00
> frame 1229: pid=SETUP addr=0x00 ep=0 len=8
>     data_out= 80 06 00 01 00 00 40 00
> uhci readw port=0x0002 val=0x0002
> uhci writew port=0x0002 val=0x0002
>
>   There are some problems with the port status request traffic from
> the hub as well:
>
> frame 1174: pid=SETUP addr=0x03 ep=0 len=8
>     data_out= 23 03 04 00 01 00 00 00
> error usb_generic_handle_packet: unknown USB-Token - 258
>     ret=0
Yes this is clear, the usbhub does not work at all. If I find the time I
will look at it, this week.
>
>   The interface is also giving incorrect errors at certain times:
>
> (qemu) usb_add usbhub
> usb_add: extraneous characters at the end of line
> (qemu)
Yes I also noticed this. It's very strange. If you add the same line
again, it will work in most cases. I don't know why it happens. It is a
bug which is located in monitor.c.
>
>   Attached is a patch to get the linux redirector working with the
> changes.  It has a few other modifications as well but nothing
> radical.  The default redirector for linux host was changed to the
> linux redirector as well (just a heads up).  I'm still using the
> libusb redirector for testing here but it's good to restore the linux
> redirector as default on linux host.
Thanks works very well.

I have found today an error in the state machine and fixed it. You will
find the fix in the patch:
http://217.20.126.200/tino/usb-2006-04-23.patch
which includes all of our patches until today.

With kind regards,
Tino H. Seifert

[Qemu-devel] Update for cvs 2006-04-24

[nix.wie.weg]

Hello everybody,

As Fabrice is already adding some changes to cvs. The patch is getting
smaller (by fivehundert lines today). If we keep this speed the patch
will be gone in only 9 more days.

Here is the patch - as allways applying cleanly to todays cvs:
http://217.20.126.200/tino/usb-2006-04-24.patch


With kind regards,
Tino H. Seifert

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

   lo.  The bug with the usb hub is a simple fix.  We're simply using 
handle_packet to pass USB_MSG_RESET when this should be passed to 
handle_msg:

hw/usb-hub.c - line 388:

            case PORT_RESET:
                if (dev) {
                    dev->handle_packet(dev,
                                       USB_MSG_RESET, 0, 0, NULL, 0);
                    port->wPortChange |= PORT_STAT_C_RESET;

   The simple fix is to change the function call to dev->handle_msg(dev, 
USB_MSG_RESET) so that the state machine is correctly set up.

   I'm going to go over the port status bits with a fine comb to make 
sure everything is looking good.

   It also looks like we'll need to be able to propogate resume signal 
from usb_hub_attach to the uhci controller.  This means putting the 
resume code in uhci_attach:

    // wakeup the controller if suspended
    if (s->cmd & UHCI_CMD_EGSM) {
        s->cmd |= UHCI_CMD_FGR;
        s->status |= UHCI_STS_RD;
        uhci_update_irq(s);
    }

into a static function inside usb-uhci.c and then setting up a way of 
calling this when the controller needs to be resumed.  If a device is 
set for remote_wakeup then it can call this function to resume the bus 
if necessary.

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

   Ran into this as well when testing a gps device.  Looking at hw/usb.c 
and usb_20 8.5.3 now.

frame 1346: pid=OUT addr=0x03 ep=2 len=1
     data_out= 30
usb_generic_handle_data(): handle setup
usb.c: usb_generic_handle_data, send more than oneconfig paket out not 
implemented yet - FIXME
frame 1346: pid=SETUP addr=0x03 ep=0 len=8
     data_out= 21 09 00 03 00 00 08 00
usb_generic_handle_setup(): New setup Token
     ret=8
uhci readw port=0x0002 val=0x0001
uhci writew port=0x0002 val=0x0001
uhci readw port=0x0006 val=0x0543
frame 1347: pid=OUT addr=0x03 ep=2 len=1
     data_out= 30
usb_generic_handle_data(): handle setup
usb.c: usb_generic_handle_data, send more than oneconfig paket out not 
implemented yet - FIXME

   Is it intentional that once the hub is added it can't be removed?

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

   lo.  The attached patch addresses the problem with removing the 
usbhub/tablet devices and switches the handle_packet call in the port 
reset call to handle_msg which just happens to make the hub functional.

[-- Attachment #2: lusb-upd5.diff --]
[-- Type: text/plain, Size: 1678 bytes --]

--- a/qemu/hw/usb-hub.c	2006-04-28 19:44:18.000000000 -0500
+++ b/qemu/hw/usb-hub.c	2006-04-29 18:55:46.000000000 -0500
@@ -240,6 +240,13 @@
     return 1;
 }
 
+int usb_hub_handle_close(USBDevice *dev)
+{
+    USBHubState *s = (USBHubState *)(dev->opaque);
+    qemu_free(s);
+    return 1;
+}
+
 int usb_hub_handle_control(USBDevice *dev, int request, int value,
                            int index, int length, uint8_t *data)
 {
@@ -385,8 +392,7 @@
                 break;
             case PORT_RESET:
                 if (dev) {
-                    dev->handle_packet(dev, 
-                                       USB_MSG_RESET, 0, 0, NULL, 0);
+                    dev->handle_msg(dev, USB_MSG_RESET);
                     port->wPortChange |= PORT_STAT_C_RESET;
                     /* set enable bit */
 //                    port->wPortChange |= PORT_STAT_C_ENABLE;
@@ -578,6 +584,7 @@
     dev->handle_packet=         usb_hub_handle_packet;
     dev->handle_attach=         usb_hub_attach;
     dev->handle_reset=          usb_hub_handle_reset;
+    dev->handle_close=          usb_hub_handle_close;
     dev->handle_control=        usb_hub_handle_control;
     dev->handle_data=           usb_hub_handle_data;
 
--- a/qemu/hw/usb-hid.c	2006-04-28 19:44:18.000000000 -0500
+++ b/qemu/hw/usb-hid.c	2006-04-29 18:56:15.000000000 -0500
@@ -526,6 +526,7 @@
     dev->handle_packet=     usb_generic_handle_packet;
 
     dev->handle_reset=      usb_mouse_handle_reset;
+    dev->handle_close=      usb_mouse_handle_close;
     dev->handle_control=    usb_mouse_handle_control;
     dev->handle_data=       usb_mouse_handle_data;
     s->kind=                USB_TABLET;

Re: [Qemu-devel] Large USB patch

[Lonnie Mendez]

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

   Attached is another patch to implement the said resume code.  With 
this patch, devices attached to windows xp guest are able to resume the 
controller when necessary.  Before, if a device was attached to the 
usbhub and windows had the root hub set for power savings then the bus 
would stay suspended.

   It implements the function usb_wakeup_controller which can be called 
with a USBDevice handle for the device that is causing the event.

[-- Attachment #2: lusb-upd6.diff --]
[-- Type: text/plain, Size: 4837 bytes --]

--- a/qemu/hw/usb-uhci.c	2006-04-30 14:32:43.000000000 -0500
+++ b/qemu/hw/usb-uhci.c	2006-04-30 15:03:46.000000000 -0500
@@ -102,6 +102,7 @@
 } UHCI_QH;
 
 static int uhci_attach (USBDevice *hub, USBDevice *dev, int portnum);
+static void uhci_resume (void *opaque);
 
 static void uhci_update_irq (UHCIState *s)
 {
@@ -338,13 +339,6 @@
     UHCIPort  *port;
     int i;
 
-    // wakeup the controller if suspended
-    if (s->cmd & UHCI_CMD_EGSM) {
-        s->cmd |= UHCI_CMD_FGR;
-        s->status |= UHCI_STS_RD;
-        uhci_update_irq(s);
-    }
-
     if (dev) {
         if( portnum >= NB_PORTS ) {
 #ifdef DEBUG        
@@ -385,6 +379,9 @@
             port->ctrl |= UHCI_PORT_LSDA;
         else
             port->ctrl &= ~UHCI_PORT_LSDA;
+
+        uhci_resume(s);
+
         /* send the attach message */
         port->dev= dev;
         port->dev->handle_msg (port->dev, USB_MSG_ATTACH);
@@ -401,6 +398,9 @@
             port->ctrl &= ~UHCI_PORT_EN;
             port->ctrl |= UHCI_PORT_ENC;
         }
+
+        uhci_resume(s);
+
         if (port->dev) {
             /* send the detach message */
             port->dev->handle_msg(port->dev, USB_MSG_DETACH);
@@ -412,6 +412,21 @@
     }
 }
 
+/* signal resume if controller suspended */
+static void uhci_resume (void *opaque)
+{
+    UHCIState *s = (UHCIState *)opaque;
+
+    if (!s)
+        return;
+
+    if (s->cmd & UHCI_CMD_EGSM) {
+        s->cmd |= UHCI_CMD_FGR;
+        s->status |= UHCI_STS_RD;
+        uhci_update_irq(s);
+    }
+}
+
 static int uhci_broadcast_packet(UHCIState *s, uint8_t pid, 
                                  uint8_t devaddr, uint8_t devep,
                                  uint8_t *data, int len)
@@ -732,6 +738,7 @@
         dev->speed= USB_SPEED_FULL;
         dev->state= USB_STATE_ATTACHED;
         dev->handle_attach= &uhci_attach;
+        dev->handle_resume= &uhci_resume;
         for(i = 0; i < NB_PORTS; i++) {
             s->ports[i].dev= NULL;
         }
--- a/qemu/hw/usb.h	2006-04-30 14:32:43.000000000 -0500
+++ b/qemu/hw/usb.h	2006-04-30 15:14:23.000000000 -0500
@@ -187,6 +187,7 @@
     int         (*handle_data)    (USBDevice *dev, int pid, uint8_t devep,
                                    uint8_t *data, int len);
     int         (*handle_attach)  (USBDevice *hub, USBDevice *dev, int portnum);
+    void        (*handle_resume)  (void *opaque);
 };
 
 /* Maximum of simultaneous USB Devices including all USB Controllers */
@@ -212,6 +213,9 @@
 USBDevice*  usb_find_device             (char *path);
 char*       usb_find_name               (USBDevice *device);
 
+/* resumes a suspended controller that given device is attached to */
+void usb_wakeup_controller(USBDevice *dev);
+
 /* function which adds or removes the usb devices according to usb_tree */
 int         usb_update_devices          (PCIBus *pci_bus);
 /* functions which show info on the available usb devices - used by monitor.c*/
--- a/qemu/hw/usb.c	2006-04-30 14:32:43.000000000 -0500
+++ b/qemu/hw/usb.c	2006-04-30 15:14:35.000000000 -0500
@@ -401,6 +408,27 @@
     }
 }
 
+/* communicate resume to host controller */
+void usb_wakeup_controller(USBDevice *dev)
+{
+    USBTree *tree = usb_tree;
+    USBDevice *controller = NULL;
+    char bus_path[5];
+
+    if (dev == NULL)
+        return;
+
+    for (; tree != NULL; tree= tree->next) {
+        if (tree->dev == dev) { // have match - next find root controller
+            strncpy(bus_path, tree->path, 3);
+            controller = usb_find_device(bus_path);
+            if (controller && controller->handle_resume) // send resume if implemented
+                controller->handle_resume(controller->opaque);
+            break;
+        }
+    }
+}
+
 /* remove a usb device, the following steps are taken:
     1. find the device
     2. let his father know 
@@ -608,6 +636,7 @@
         dev->handle_msg=        &usb_dummy_handle_msg;
         dev->handle_data=       &usb_dummy_handle_data;
         dev->handle_attach=     &usb_dummy_handle_attach;
+        dev->handle_resume=     NULL;
     }
     return dev;
 }
--- a/qemu/hw/usb-hub.c	2006-04-30 14:33:29.000000000 -0500
+++ b/qemu/hw/usb-hub.c	2006-04-30 15:13:22.000000000 -0500
@@ -212,12 +212,20 @@
             port->wPortStatus |= PORT_STAT_LOW_SPEED;
         else
             port->wPortStatus &= ~PORT_STAT_LOW_SPEED;
+
+        if (hub->remote_wakeup)
+            usb_wakeup_controller(dev);
+
         port->dev = dev;
         port->dev->handle_msg (port->dev, USB_MSG_ATTACH);
         return portnum;    
     } else {
         port = &s->ports[portnum];
         dev = port->dev;
+
+        if (hub->remote_wakeup)
+            usb_wakeup_controller(dev);
+
         if (dev) {
             port->wPortStatus &= ~PORT_STAT_CONNECTION;
             port->wPortChange |= PORT_STAT_C_CONNECTION;