linux-kernel - Re: [PATCH v5 5/5] usb: host: enable sideband transfer during system sleep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOuDEK2U1Ok=a=X36R_xjkW7MJm2kQ=G7ohh_oC=+f=rGa2eiA@mail.gmail.com>
Date: Fri, 18 Oct 2024 19:59:00 +0800
From: Guan-Yu Lin <guanyulin@...gle.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Thinh.Nguyen@...opsys.com, gregkh@...uxfoundation.org, 
	mathias.nyman@...el.com, yajun.deng@...ux.dev, sumit.garg@...aro.org, 
	kekrby@...il.com, oneukum@...e.com, dianders@...omium.org, perex@...ex.cz, 
	tiwai@...e.com, niko.mauno@...sala.com, andreyknvl@...il.com, 
	christophe.jaillet@...adoo.fr, tj@...nel.org, stanley_chang@...ltek.com, 
	quic_jjohnson@...cinc.com, ricardo@...liere.net, linux-usb@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-sound@...r.kernel.org, badhri@...gle.com, 
	albertccwang@...gle.com, quic_wcheng@...cinc.com, pumahsu@...gle.com
Subject: Re: [PATCH v5 5/5] usb: host: enable sideband transfer during system sleep

On Wed, Oct 16, 2024 at 10:45 PM Alan Stern <stern@...land.harvard.edu> wrote:
>
> On Wed, Oct 16, 2024 at 03:40:00PM +0800, Guan-Yu Lin wrote:
> > On Tue, Oct 15, 2024 at 10:43 PM Alan Stern <stern@...land.harvard.edu> wrote:
> > >
> > > On Tue, Oct 15, 2024 at 11:56:00AM +0800, Guan-Yu Lin wrote:
> > > > On Mon, Oct 14, 2024 at 11:56 PM Alan Stern <stern@...land.harvard.edu> wrote:
> > > > > I'm not so sure about this.  By returning early, you prevent the drivers
> > > > > bound to this device from suspending.  But they can't operate properly
> > > > > when the system is in a low-power mode.  Won't that cause problems?
> > > > >
> > > > > Maybe this really belongs in usb_suspend_device(), and its counterpart
> > > > > belongs in usb_resume_device().
> > > > >
> > > >
> > > > To my understanding, after the system is suspended, the USB driver
> > > > will do nothing as the main processor has been suspended. May I check
> > > > what forms of low power mode and operation we are discussing here?
> > >
> > > S3 suspend.  You are right that the driver will do nothing while the
> > > CPU is suspended.  But what about the times before and after that,
> > > while the suspend and resume procedures are underway?  The driver
> > > needs to be told to cancel any ongoing transfers while the system
> > > suspends and then restart them while the system resumes.
> > >
> >
> > Regarding the cancellation of ongoing transfers during suspend, I
> > believe usb_hcd_flush_endpoint() handles this as discussed below.
>
> There's more to it than that.  If you cancel ongoing transfers by
> calling usb_hcd_flush_endpoint() without informing the driver first, the
> driver will get very confused and think the device has failed.
>
> > Besides calling usb_hcd_flush_endpoint(), are there any other
> > necessary changes before suspending the driver in our scenario? Maybe
> > we could discuss setting usb_device_state to USB_STATE_SUSPENDED.
> > However, my understanding is that this variable reflects the actual
> > device state. Since the device remains active via the sideband in our
> > case,  changing usb_device_state seems unnecessary.
>
> That's right.
>
> I don't think anything else is needed.  Just call
> usb_suspend_interface() like the normal pathway in usb_suspend_both()
> does, but skip calling usb_suspend_device().
>
> Alan Stern

Thanks for the suggestions, let me address them in the next version.
After some local development, our experiments suggest it may be
necessary to skip usb_suspend_interface() & usb_hcd_flush_endpoint()
for connection changes behind a hub and HID events in our scenario.

Typically, when the system sleeps, the hub uses remote wakeup to
reactivate upstream devices and resume the interface to handle
connection changes. However, our current conclusion is to maintain the
device in an active state while suspending the interface. This
deviates from the norm, as remote wakeup is designed to function when
devices and links are suspended. We're concerned that this discrepancy
might interfere with the remote wakeup mechanism.
To address this, we're currently bypassing usb_suspend_interface() and
usb_hcd_flush_endpoint(). This effectively simulates an "active
system" state, allowing the USB controller to notify the kernel about
connection changes via interrupts. This workaround applies to HID
events as well.

Which approach do you recommend? Should we invest in integrating with
the remote wakeup framework, or is it acceptable to keep necessary
components active, mirroring an "active system" state?

Regards,
Guan-Yu