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: <CAOuDEK3mmR9052FWEJAVBkUanVJ1MLLebZoANiasUpD9TDBjfg@mail.gmail.com>
Date: Wed, 16 Oct 2024 15:40: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 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.
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.

> > usb_suspend_device() did close the required port/bus to maintain usb
> > transfer.
>
> I don't know what you mean by that.
>

Nothing special here, I'm just echoing what you've mentioned and
trying to bring up the discussion about usb_hcd_flush_endpoint().

> >  However, there are additional usb_hcd_flush_endpoint() calls
> > aside from usb_suspend_device(). Maybe we should consider not doing
> > those also since some of the endpoints are now handled by the
> > sideband.
>
> Those calls should not be skipped.  If the sideband controller wants
> to handle the endpoints on its own, it can go right ahead.  The main
> controller and the USB core need to know that they shouldn't use the
> endpoint while the system is suspending.
>
> Alan Stern

Got it, let me update the patch and put the related changes into
usb_suspend_device()/usb_resume_device().

Regards,
Guan-Yu