* [Qemu-devel] Large USB patch
@ 2006-04-20 19:59 nix.wie.weg
2006-04-21 2:23 ` Lonnie Mendez
` (4 more replies)
0 siblings, 5 replies; 31+ messages in thread
From: nix.wie.weg @ 2006-04-20 19:59 UTC (permalink / raw)
To: qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [Qemu-devel] Large USB patch 2006-04-20 19:59 [Qemu-devel] Large USB patch nix.wie.weg @ 2006-04-21 2:23 ` Lonnie Mendez 2006-04-21 5:59 ` nix.wie.weg 2006-04-21 14:53 ` Lonnie Mendez ` (3 subsequent siblings) 4 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 2:23 UTC (permalink / raw) To: qemu-devel 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 2:23 ` Lonnie Mendez @ 2006-04-21 5:59 ` nix.wie.weg 2006-04-21 7:04 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: nix.wie.weg @ 2006-04-21 5:59 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 5:59 ` nix.wie.weg @ 2006-04-21 7:04 ` Lonnie Mendez 0 siblings, 0 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 7:04 UTC (permalink / raw) To: qemu-devel 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-20 19:59 [Qemu-devel] Large USB patch nix.wie.weg 2006-04-21 2:23 ` Lonnie Mendez @ 2006-04-21 14:53 ` Lonnie Mendez 2006-04-21 15:00 ` Lonnie Mendez 2006-04-21 15:50 ` Lonnie Mendez 2006-04-22 14:15 ` nix.wie.weg ` (2 subsequent siblings) 4 siblings, 2 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 14:53 UTC (permalink / raw) To: qemu-devel 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 14:53 ` Lonnie Mendez @ 2006-04-21 15:00 ` Lonnie Mendez 2006-04-21 15:50 ` Lonnie Mendez 1 sibling, 0 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 15:00 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 14:53 ` Lonnie Mendez 2006-04-21 15:00 ` Lonnie Mendez @ 2006-04-21 15:50 ` Lonnie Mendez 2006-04-21 16:19 ` Lonnie Mendez 2006-04-21 16:26 ` nix.wie.weg 1 sibling, 2 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 15:50 UTC (permalink / raw) To: qemu-devel 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 15:50 ` Lonnie Mendez @ 2006-04-21 16:19 ` Lonnie Mendez 2006-04-21 16:29 ` nix.wie.weg 2006-04-21 16:26 ` nix.wie.weg 1 sibling, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 16:19 UTC (permalink / raw) To: qemu-devel [-- 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); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 16:19 ` Lonnie Mendez @ 2006-04-21 16:29 ` nix.wie.weg 2006-04-21 17:28 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: nix.wie.weg @ 2006-04-21 16:29 UTC (permalink / raw) To: qemu-devel 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. > ------------------------------------------------------------------------ > > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 16:29 ` nix.wie.weg @ 2006-04-21 17:28 ` Lonnie Mendez 2006-04-21 18:06 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 17:28 UTC (permalink / raw) To: qemu-devel [-- 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); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 17:28 ` Lonnie Mendez @ 2006-04-21 18:06 ` Lonnie Mendez 2006-04-21 18:38 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 18:06 UTC (permalink / raw) To: qemu-devel [-- 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) { ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 18:06 ` Lonnie Mendez @ 2006-04-21 18:38 ` Lonnie Mendez 2006-04-21 20:50 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 18:38 UTC (permalink / raw) To: qemu-devel 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) ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 18:38 ` Lonnie Mendez @ 2006-04-21 20:50 ` Lonnie Mendez 2006-04-22 9:33 ` nix.wie.weg 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-21 20:50 UTC (permalink / raw) To: qemu-devel [-- 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"); } ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 20:50 ` Lonnie Mendez @ 2006-04-22 9:33 ` nix.wie.weg 2006-04-22 14:36 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: nix.wie.weg @ 2006-04-22 9:33 UTC (permalink / raw) To: qemu-devel [-- 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; ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 9:33 ` nix.wie.weg @ 2006-04-22 14:36 ` Lonnie Mendez 2006-04-22 15:36 ` nix.wie.weg 2006-04-22 16:00 ` nix.wie.weg 0 siblings, 2 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-22 14:36 UTC (permalink / raw) To: qemu-devel 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? ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 14:36 ` Lonnie Mendez @ 2006-04-22 15:36 ` nix.wie.weg 2006-04-22 15:38 ` nix.wie.weg 2006-04-22 16:00 ` nix.wie.weg 1 sibling, 1 reply; 31+ messages in thread From: nix.wie.weg @ 2006-04-22 15:36 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 15:36 ` nix.wie.weg @ 2006-04-22 15:38 ` nix.wie.weg 0 siblings, 0 replies; 31+ messages in thread From: nix.wie.weg @ 2006-04-22 15:38 UTC (permalink / raw) To: qemu-devel [-- 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; ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 14:36 ` Lonnie Mendez 2006-04-22 15:36 ` nix.wie.weg @ 2006-04-22 16:00 ` nix.wie.weg 2006-04-22 16:19 ` Lonnie Mendez 1 sibling, 1 reply; 31+ messages in thread From: nix.wie.weg @ 2006-04-22 16:00 UTC (permalink / raw) To: qemu-devel > > 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 16:00 ` nix.wie.weg @ 2006-04-22 16:19 ` Lonnie Mendez 2006-04-22 16:35 ` nix.wie.weg 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-22 16:19 UTC (permalink / raw) To: qemu-devel 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? ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 16:19 ` Lonnie Mendez @ 2006-04-22 16:35 ` nix.wie.weg 2006-04-23 3:38 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: nix.wie.weg @ 2006-04-22 16:35 UTC (permalink / raw) To: qemu-devel [-- 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] ); } ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-22 16:35 ` nix.wie.weg @ 2006-04-23 3:38 ` Lonnie Mendez 2006-04-23 21:54 ` nix.wie.weg 2006-04-29 1:03 ` Lonnie Mendez 0 siblings, 2 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-23 3:38 UTC (permalink / raw) To: qemu-devel [-- 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 ); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-23 3:38 ` Lonnie Mendez @ 2006-04-23 21:54 ` nix.wie.weg 2006-04-29 1:03 ` Lonnie Mendez 1 sibling, 0 replies; 31+ messages in thread From: nix.wie.weg @ 2006-04-23 21:54 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-23 3:38 ` Lonnie Mendez 2006-04-23 21:54 ` nix.wie.weg @ 2006-04-29 1:03 ` Lonnie Mendez 2006-04-29 3:29 ` Lonnie Mendez 1 sibling, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-29 1:03 UTC (permalink / raw) To: qemu-devel 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. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-29 1:03 ` Lonnie Mendez @ 2006-04-29 3:29 ` Lonnie Mendez 2006-04-30 0:46 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-29 3:29 UTC (permalink / raw) To: qemu-devel 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? ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-29 3:29 ` Lonnie Mendez @ 2006-04-30 0:46 ` Lonnie Mendez 2006-04-30 20:56 ` Lonnie Mendez 0 siblings, 1 reply; 31+ messages in thread From: Lonnie Mendez @ 2006-04-30 0:46 UTC (permalink / raw) To: qemu-devel [-- 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; ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-30 0:46 ` Lonnie Mendez @ 2006-04-30 20:56 ` Lonnie Mendez 0 siblings, 0 replies; 31+ messages in thread From: Lonnie Mendez @ 2006-04-30 20:56 UTC (permalink / raw) To: qemu-devel [-- 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; ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-21 15:50 ` Lonnie Mendez 2006-04-21 16:19 ` Lonnie Mendez @ 2006-04-21 16:26 ` nix.wie.weg 1 sibling, 0 replies; 31+ messages in thread From: nix.wie.weg @ 2006-04-21 16:26 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-20 19:59 [Qemu-devel] Large USB patch nix.wie.weg 2006-04-21 2:23 ` Lonnie Mendez 2006-04-21 14:53 ` Lonnie Mendez @ 2006-04-22 14:15 ` nix.wie.weg 2006-04-23 15:02 ` Fabrice Bellard 2006-04-24 23:50 ` [Qemu-devel] Update for cvs 2006-04-24 nix.wie.weg 4 siblings, 0 replies; 31+ messages in thread From: nix.wie.weg @ 2006-04-22 14:15 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-20 19:59 [Qemu-devel] Large USB patch nix.wie.weg ` (2 preceding siblings ...) 2006-04-22 14:15 ` nix.wie.weg @ 2006-04-23 15:02 ` Fabrice Bellard 2006-04-23 16:11 ` nix.wie.weg 2006-04-24 23:50 ` [Qemu-devel] Update for cvs 2006-04-24 nix.wie.weg 4 siblings, 1 reply; 31+ messages in thread From: Fabrice Bellard @ 2006-04-23 15:02 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] Large USB patch 2006-04-23 15:02 ` Fabrice Bellard @ 2006-04-23 16:11 ` nix.wie.weg 0 siblings, 0 replies; 31+ messages in thread From: nix.wie.weg @ 2006-04-23 16:11 UTC (permalink / raw) To: qemu-devel 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. > > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] Update for cvs 2006-04-24 2006-04-20 19:59 [Qemu-devel] Large USB patch nix.wie.weg ` (3 preceding siblings ...) 2006-04-23 15:02 ` Fabrice Bellard @ 2006-04-24 23:50 ` nix.wie.weg 4 siblings, 0 replies; 31+ messages in thread From: nix.wie.weg @ 2006-04-24 23:50 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2006-04-30 20:57 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-04-20 19:59 [Qemu-devel] Large USB patch nix.wie.weg 2006-04-21 2:23 ` Lonnie Mendez 2006-04-21 5:59 ` nix.wie.weg 2006-04-21 7:04 ` Lonnie Mendez 2006-04-21 14:53 ` Lonnie Mendez 2006-04-21 15:00 ` Lonnie Mendez 2006-04-21 15:50 ` Lonnie Mendez 2006-04-21 16:19 ` Lonnie Mendez 2006-04-21 16:29 ` nix.wie.weg 2006-04-21 17:28 ` Lonnie Mendez 2006-04-21 18:06 ` Lonnie Mendez 2006-04-21 18:38 ` Lonnie Mendez 2006-04-21 20:50 ` Lonnie Mendez 2006-04-22 9:33 ` nix.wie.weg 2006-04-22 14:36 ` Lonnie Mendez 2006-04-22 15:36 ` nix.wie.weg 2006-04-22 15:38 ` nix.wie.weg 2006-04-22 16:00 ` nix.wie.weg 2006-04-22 16:19 ` Lonnie Mendez 2006-04-22 16:35 ` nix.wie.weg 2006-04-23 3:38 ` Lonnie Mendez 2006-04-23 21:54 ` nix.wie.weg 2006-04-29 1:03 ` Lonnie Mendez 2006-04-29 3:29 ` Lonnie Mendez 2006-04-30 0:46 ` Lonnie Mendez 2006-04-30 20:56 ` Lonnie Mendez 2006-04-21 16:26 ` nix.wie.weg 2006-04-22 14:15 ` nix.wie.weg 2006-04-23 15:02 ` Fabrice Bellard 2006-04-23 16:11 ` nix.wie.weg 2006-04-24 23:50 ` [Qemu-devel] Update for cvs 2006-04-24 nix.wie.weg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).