Re: [PATCH] usbredir: remove 'remote wake' capability from configuration descriptor

[Yuri Benditovich]

On Wed, Nov 27, 2019 at 11:40 AM Gerd Hoffmann <address@hidden> wrote:
>
> On Wed, Nov 27, 2019 at 09:36:21AM +0200, Yuri Benditovich wrote:
> > On Wed, Nov 27, 2019 at 8:36 AM Markus Armbruster <address@hidden> wrote:
> > >
> > > Yuri Benditovich <address@hidden> writes:
> > >
> > > > If the redirected device has this capability, Windows guest may
> > > > place the device into D2 and expect it to wake when the device
> > > > becomes active, but this will never happen. For example, when
> > > > internal Bluetooth adapter is redirected, keyboards and mice
> > > > connected to it do not work. Setting global property
> > > > 'usb-redir.nowake=off' keeps 'remote wake' as is.
> > >
> > > "usb-redir.nowake=off" is a double negation.  Gets weirder when dusted
> > > with syntactic sugar: "usb-redir.nonowake".  Can we think of a better
> > > name?  Naming is hard...  What about "usb-redir.wakeup=on"?
> > '"wakeup" is good but "wakeup=on" makes an impression that we add the
> > capability to the device even if it does not have one.
> > disable_wake? suppress_wake? clear_wake? wake_allowed?
>
> remote-wakeup=on,off ?

This is like wakeup=on, suggesting that we turn wake on even if it is
not supported.
Anyway, I agree with any name.

>
> > > > +    DEFINE_PROP_BOOL("nowake", USBRedirDevice, suppress_remote_wake, 
> > > > true),
> > > >      DEFINE_PROP_END_OF_LIST(),
> > > >  };
> > >
> > > The default is .nowake=on.  Is that a guest-visible change?
>
> Yes, usb descriptors change, which the guest can see.
>
> > And, of course, we can keep existing behavior by default and advise to
> > turn this property on to make these devices work.
>
> In that case a compat property would not be needed.
>
> But, after all the question is whenever that is the best way to solve
> the problem.  Most likely there is just a usb_wakeup() call missing
> somewhere ...
>

Indeed, it would be good to call usb_wakeup(), but ... there is no
trigger to do that.

When the guest places the device to D2, it cancels all the urbs that
were pending, so there are no request that will be completed on spice
client side that can call usb_wakeup on qemu side.
The device on spice client side is powered up without any active
request so the device will not produce wake up event.
Usb-redir protocol, and client-side libusb and its kernel partner of
libusb (in case of Windows client - UsbDk/winusb) can't process such
flow correctly.
Of course, AFAIK.

Similar problem happens with local redirection, BTW. But this is for
another patch.

> cheers,
>   Gerd
>