linux-kernel - Re: [PATCH v3 2/5] usb: dwc3: Add remote wakeup handling

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]

Date:   Wed, 8 Feb 2023 02:11:38 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Elson Serrao <quic_eserrao@...cinc.com>
CC:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "quic_wcheng@...cinc.com" <quic_wcheng@...cinc.com>,
        "quic_jackp@...cinc.com" <quic_jackp@...cinc.com>
Subject: Re: [PATCH v3 2/5] usb: dwc3: Add remote wakeup handling

On Tue, Feb 07, 2023, Elson Serrao wrote:
> 
> 
> On 2/7/2023 5:10 PM, Thinh Nguyen wrote:
> > On Tue, Feb 07, 2023, Elson Serrao wrote:
> > > On 2/6/2023 4:48 PM, Thinh Nguyen wrote:
> > > > > +static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async)
> > > > >    {
> > > > >    	int			retries;
> > > > > @@ -2296,9 +2309,14 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
> > > > >    	link_state = DWC3_DSTS_USBLNKST(reg);
> > > > >    	switch (link_state) {
> > > > > +	case DWC3_LINK_STATE_U3:	/* in HS, means SUSPEND */
> > > > 
> > > > It's also possible to do remote wakeup in L1 for highspeed.
> > > > 
> > > 
> > > The rw_configured flag here is in context of triggering remote wakeup from
> > > bus suspend only.
> > > 
> > > The remote wakeup setting for l1 in HighSpeed is controlled through LPM
> > > token and overrides/ignores the config desc bmAttributes wakeup bit.
> > > 
> > > Section 4.1 of USB2_LinkPowerMangement_ECN[final] spec "The host system sets the Remote Wake Flag parameter in this request to
> > > enable or disable the addressed device
> > > for remote wake from L1. The value of this flag will temporarily (while in
> > > L1) override the current setting of the
> > > Remote Wake feature settable by the standard Set/ClearFeature() commands
> > > defined in Universal Serial Bus Specification, revision 2.0, Chapter 9."
> > > 
> > > Please let me know if I am missing something.
> > > 
> > 
> > It overrides the setting of the SetFeature request, not the device
> > configuration.
> > 
> > The rw_configured reflects the user configuration. Whether the host
> > tries to enable the remote wakeup through SetFeature request or LPM
> > token, the device should operate within the user configuration
> > limitation.
> > 
> > If the configuration indicates that it doesn't support remote wakeup, we
> > should prevent unexpected behavior from the device. For simplicity, we
> > can just return failure to wakeup for all states.
> > 
> > Thanks,
> > Thinh
> 
> L1 entry/exit is HW controlled and the remote wakeup is conditional.(Section
> 7.1/Table7.2 of dwc3 data book). Even though we block it from
> SW the l1 exit will still happen from HW point of view.
> 
> To correlate the user configuration with LPM token, I experimented by
> disabling the wakeup bit in the bmAtrributes, but I still see remote wakeup
> bit being set in the LPM token. From the observation it seems like there is

That's because the linux xhci driver enables remote wakeup bit in its
port without regard for the device configuration.

> no correlation between the wakeup bit in the bmAtrributes and the wakeup bit
> in the LPM token.
> 

The host can bring the device out of L1, that's probably what you saw.
The controller doesn't initiate remote wakeup by itself.

Thanks,
Thinh