[Qemu-devel] [usb][patch] anomaly with windows guests queuing an extra TD

[Qemu-devel] [usb][patch] anomaly with windows guests queuing an extra TD

[Lonnie Mendez]

  Hello list.  I've noticed something odd going on with Windows guests 
in that they will queue up and extra IN TD on a setup packet having a 32 
byte data stage.  So far I've seen this happen twice.  Here is one 
occurance:

frame 41: pid=SETUP addr=0x02 ep=0 len=8
     data_out= 80 06 00 02 00 00 ff 00
     ret=32
frame 42: pid=IN addr=0x02 ep=0 len=8
     ret=8 data_in= 09 02 20 00 01 01 00 c0
frame 43: pid=IN addr=0x02 ep=0 len=8
     ret=8 data_in= 01 09 04 00 00 02 00 00
frame 44: pid=IN addr=0x02 ep=0 len=8
     ret=8 data_in= 00 00 07 05 81 03 08 00
frame 45: pid=IN addr=0x02 ep=0 len=8
     ret=8 data_in= 0a 07 05 02 03 08 00 0a
frame 46: pid=IN addr=0x02 ep=0 len=8
     ret=-3

   A test case device that allows anyone to see the bug in action is 
linked here:

http://gnome.dnsalias.net/patches/qemu-x10dev-bugged.diff (usb_add mysdev)

   Here is a kind of workaround for this linked below.  In the 
setup_state SETUP_STATE_ACK if the packet has direction IN then it will 
change setup state back to SETUP_STATE_ACK to wait for the further 
queued OUT TD (ACK) and then return an appropriate value to the uhci 
emulation so that it can handle the condition.  At present it marks the 
TD inactive and invalidates the frame.  Setting an error condition 
results in transfer failure and letting the frame continue without frame 
invalidation results in the TD being queued several more times until the 
hcd gives up and sends the OUT TD.
 
http://gnome.dnsalias.net/patches/qemu-usbquirk-wildtd.patch

Re: [Qemu-devel] [usb][patch] anomaly with windows guests queuing an extra TD

[Fabrice Bellard]

Can you try this simpler patch ?

Index: usb.c
===================================================================
RCS file: /sources/qemu/qemu/hw/usb.c,v
retrieving revision 1.4
diff -u -w -r1.4 usb.c
--- usb.c       11 Mar 2006 20:37:58 -0000      1.4
+++ usb.c       23 Apr 2006 18:37:22 -0000
@@ -91,8 +91,8 @@
          case 0:
              switch(s->setup_state) {
              case SETUP_STATE_ACK:
-                s->setup_state = SETUP_STATE_IDLE;
                  if (!(s->setup_buf[0] & USB_DIR_IN)) {
+                    s->setup_state = SETUP_STATE_IDLE;
                      ret = s->handle_control(s,
                                        (s->setup_buf[0] << 8) | 
s->setup_buf[1],                                       (s->setup_buf[3] 
<< 8) | s->setup_buf[2],@@ -102,7 +102,7 @@
                      if (ret > 0)
                          ret = 0;
                  } else {
-                    goto fail;
+                    /* return 0 byte */
                  }
                  break;
              case SETUP_STATE_DATA:
@@ -136,11 +136,11 @@
          case 0:
              switch(s->setup_state) {
              case SETUP_STATE_ACK:
-                s->setup_state = SETUP_STATE_IDLE;
                  if (s->setup_buf[0] & USB_DIR_IN) {
+                    s->setup_state = SETUP_STATE_IDLE;
                      /* transfer OK */
                  } else {
-                    goto fail;
+                    /* ignore additionnal output */
                  }
                  break;
              case SETUP_STATE_DATA:

Fabrice.

Lonnie Mendez wrote:
>  Hello list.  I've noticed something odd going on with Windows guests in 
> that they will queue up and extra IN TD on a setup packet having a 32 
> byte data stage.  So far I've seen this happen twice.  Here is one 
> occurance:
> 
> frame 41: pid=SETUP addr=0x02 ep=0 len=8
>     data_out= 80 06 00 02 00 00 ff 00
>     ret=32
> frame 42: pid=IN addr=0x02 ep=0 len=8
>     ret=8 data_in= 09 02 20 00 01 01 00 c0
> frame 43: pid=IN addr=0x02 ep=0 len=8
>     ret=8 data_in= 01 09 04 00 00 02 00 00
> frame 44: pid=IN addr=0x02 ep=0 len=8
>     ret=8 data_in= 00 00 07 05 81 03 08 00
> frame 45: pid=IN addr=0x02 ep=0 len=8
>     ret=8 data_in= 0a 07 05 02 03 08 00 0a
> frame 46: pid=IN addr=0x02 ep=0 len=8
>     ret=-3
> 
>   A test case device that allows anyone to see the bug in action is 
> linked here:
> 
> http://gnome.dnsalias.net/patches/qemu-x10dev-bugged.diff (usb_add mysdev)
> 
>   Here is a kind of workaround for this linked below.  In the 
> setup_state SETUP_STATE_ACK if the packet has direction IN then it will 
> change setup state back to SETUP_STATE_ACK to wait for the further 
> queued OUT TD (ACK) and then return an appropriate value to the uhci 
> emulation so that it can handle the condition.  At present it marks the 
> TD inactive and invalidates the frame.  Setting an error condition 
> results in transfer failure and letting the frame continue without frame 
> invalidation results in the TD being queued several more times until the 
> hcd gives up and sends the OUT TD.
> 
> http://gnome.dnsalias.net/patches/qemu-usbquirk-wildtd.patch
> 
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
> 
>