Hi Marc-André Lureau > Subject: Re: [PATCH v4 7/8] hw/usb/redirect-server: Stream interrupt IN > endpoints > > > Interrupt IN is not a request and an answer in usbredir. The host asks > > once with start_interrupt_receiving. After that it expects a packet > > every time the device has data. > > > > Park one IN packet on the device for each streaming endpoint. A device > > that answers USB_RET_ASYNC holds the packet and completes it when it > > has data. A device that answers USB_RET_NAK gives the packet back at > > once, so park a new one when the bus reports a wakeup on that endpoint. > > A slow timer parks one too, for a device that does not report a wakeup. > > > > Wait for the endpoint's bInterval before parking the next one. Some > > devices answer every time. Without the wait we would ask as fast as > > the main loop runs, and the guest would get no time. > > > > Signed-off-by: Jamin Lin <[email protected]> > > Message-ID: <[email protected]> > > > > diff --git a/hw/usb/redirect-server.c b/hw/usb/redirect-server.c index > > bf5286b64996..c613147fbd0a 100644 > > --- a/hw/usb/redirect-server.c > > +++ b/hw/usb/redirect-server.c > > @@ -56,6 +56,7 @@ > > #include "qemu/main-loop.h" > > #include "qemu/module.h" > > #include "migration/vmstate.h" > > +#include "qemu/timer.h" > > #include "qemu/cutils.h" > > #include "hw/core/sysbus.h" > > #include "hw/usb/usb.h" > > @@ -82,16 +83,30 @@ OBJECT_DECLARE_SIMPLE_TYPE(USBRedirServer, > > USB_REDIR_SERVER) #define USBREDIR_SERVER_MAX_EP 32 #define > > USBREDIR_SERVER_EP_IN_BASE 16 > > > > +/* An endpoint number is 4 bits, so 0 to 15. */ #define > > +USBREDIR_SERVER_MAX_EP_NR 16 > > + > > /* > > * The bulk length field is 32 bits, so the host can ask for up to 4 GB. > > * This is the largest transfer accepted. > > */ > > #define USBREDIR_SERVER_MAX_BULK (1 * MiB) > > > > +/* Buffer size for an interrupt IN endpoint before its descriptor is > > +seen. */ #define USBREDIR_SERVER_INTR_DEFAULT_LEN 64 > > + > > +/* > > + * Ask again this often after an endpoint answered NAK. Most devices > > +wake > > + * the bus when data arrives, and then the next ask happens at once. > > +This > > + * timer is for the devices that do not wake the bus. > > + */ > > +#define USBREDIR_SERVER_INTR_RETRY_MS 1000 > > + > > #define USBREDIR_SERVER_CTRL_SETUP 0 > > #define USBREDIR_SERVER_CTRL_STATUS 1 #define > USBREDIR_SERVER_BULK 2 > > #define USBREDIR_SERVER_INTR 3 > > +#define USBREDIR_SERVER_INTR_STREAM 4 > > > > /* Which message answers the host when a control transfer ends. */ > > typedef enum { @@ -140,6 +155,16 @@ struct USBRedirServer { > > bool host_connected; > > bool device_announced; > > > > + /* > > + * Interrupt IN streaming. intr_in_started is indexed by endpoint > > + * number. intr_timer asks the device again, after the endpoint's > > + * interval or after a NAK. intr_bh asks at once when the device says > > + * it has data. > > + */ > > + bool intr_in_started[USBREDIR_SERVER_MAX_EP_NR]; > > + QEMUBH *intr_bh; > > + QEMUTimer *intr_timer; > > Why not have one timer per ep? >
Thanks for the review and suggestion. Will do Jamin > -- > Marc-André Lureau <[email protected]>
