linux-kernel - RE: [PATCH 2/3] usb: aspeed-vhub: support remote wakeup feature

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: <HK0PR06MB3202F55EEE02B9931D9CD4AD80699@HK0PR06MB3202.apcprd06.prod.outlook.com>
Date:   Thu, 2 Dec 2021 03:03:27 +0000
From:   Neal Liu <neal_liu@...eedtech.com>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Cai Huoqing <caihuoqing@...du.com>,
        Tao Ren <rentao.bupt@...il.com>,
        Julia Lawall <julia.lawall@...ia.fr>,
        kernel test robot <lkp@...el.com>,
        Sasha Levin <sashal@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>
CC:     BMC-SW <BMC-SW@...eedtech.com>
Subject: RE: [PATCH 2/3] usb: aspeed-vhub: support remote wakeup feature

> -----Original Message-----
> From: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> Sent: Wednesday, December 1, 2021 7:38 AM
> To: Neal Liu <neal_liu@...eedtech.com>; Felipe Balbi <balbi@...nel.org>;
> Greg Kroah-Hartman <gregkh@...uxfoundation.org>; Joel Stanley
> <joel@....id.au>; Andrew Jeffery <andrew@...id.au>; Cai Huoqing
> <caihuoqing@...du.com>; Tao Ren <rentao.bupt@...il.com>; Julia Lawall
> <julia.lawall@...ia.fr>; kernel test robot <lkp@...el.com>; Sasha Levin
> <sashal@...nel.org>; linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; linux-aspeed@...ts.ozlabs.org
> Subject: Re: [PATCH 2/3] usb: aspeed-vhub: support remote wakeup feature
> 
> On Tue, 2021-11-30 at 09:47 +0000, Neal Liu wrote:
> > > Should this  be controlled by d->wakeup_en ? IE, we have a feature
> > > for the host to enable/disable remote wakeup, should we honor it ?
> >
> > For KVM usage, remote keyboard packet would be sent if user wants to do
> remote wakeup.
> > In this case, d->wakeup_en is not used.
> > Set VHUB_CTRL_AUTO_REMOTE_WAKEUP to enable HW automatically
> signaling
> > wakeup if any packet would be transferred.
> 
> Sorry, I don't fully understand your explanation here.
> 
> Normally, a USB device will do remote wakeup if it's instructed to do so via the
> appropriate feature being set, which is what wakeup_en reflects. I hadn't
> originally plumbed it in, I forgot why, I think something was either not properly
> documented or not working when I wrote that driver.
> 
> You seem to want to override the behaviour and always send a remote wakeup
> packet no matter what. I am not sure this is desirable for all use cases, and
> might be something we want to make configurable, no ?
> 
> I'm trying to understand your sentence, you seem to imply that the only use
> case here is "KVM" (as in remote USB on a server system) which I can probably
> agree with... mostly.
> 
> And you say in that case, we should always do remote wakeup whenever an
> emulated USB device has any activity (keyboard or otherwise), regardless of
> whether the server has enabled the feature or not.
> 
> Am I correct ? What's the rationale here ?
> 
> Cheers,
> Ben.
> 

Let's me describe more details for our hardware behavior and hope you understand.

HUB00[3]: MANUAL_REMOTE_WAKEUP
HUB00[4]: AUTO_REMOTE_WAKEUP

Set HUB00[3] implies USB device will do remote wakeup if any write command to vhub register.
Set HUB00[4] implies USB device will do remote wakeup. It can only be set in suspend state.

For current design, d->wakeup_en only controls whether HUB00[4] can be set through usb_gadget_ops.wakeup().
If some applications (take KVM as example) want to wakeup host by sending a packet, it won't go through sb_gadget_ops.wakeup().
We enable HUB00[3] to fix this problem. It won't override above mentioned behavior.
If host has enabled the USB_DEVICE_REMOTE_WAKEUP feature, it has 2 ways to wakeup host.
1. set srp 1 (/sys/device/platform/xxxxxxxxx/udc/xxxxxx/srp)
2. emulated device has activity
If host has disabled the USB_DEVICE_REMOTE_WAKEUP feature, these 2 ways still cannot wakeup host even if USB bus is in resume state.
Thanks

-Neal