linux-kernel - Re: [PATCH 0/3] Bluetooth: Power down controller when suspending

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:   Mon, 23 Nov 2020 12:46:02 +0100
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
Cc:     BlueZ development <linux-bluetooth@...r.kernel.org>,
        chromeos-bluetooth-upstreaming@...omium.org, mcchou@...omium.org,
        danielwinkler@...omium.org,
        "David S. Miller" <davem@...emloft.net>,
        Johan Hedberg <johan.hedberg@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH 0/3] Bluetooth: Power down controller when suspending

Hi Abhishek,

> This patch series adds support for a quirk that will power down the
> Bluetooth controller when suspending and power it back up when resuming.
> 
> On Marvell SDIO Bluetooth controllers (SD8897 and SD8997), we are seeing
> a large number of suspend failures with the following log messages:
> 
> [ 4764.773873] Bluetooth: hci_cmd_timeout() hci0 command 0x0c14 tx timeout
> [ 4767.777897] Bluetooth: btmrvl_enable_hs() Host sleep enable command failed
> [ 4767.777920] Bluetooth: btmrvl_sdio_suspend() HS not actived, suspend failed!
> [ 4767.777946] dpm_run_callback(): pm_generic_suspend+0x0/0x48 returns -16
> [ 4767.777963] call mmc2:0001:2+ returned -16 after 4882288 usecs
> 
> The daily failure rate with this signature is quite significant and
> users are likely facing this at least once a day (and some unlucky users
> are likely facing it multiple times a day).
> 
> Given the severity, we'd like to power off the controller during suspend
> so the driver doesn't need to take any action (or block in any way) when
> suspending and power on during resume. This will break wake-on-bt for
> users but should improve the reliability of suspend.
> 
> We don't want to force all users of MVL8897 and MVL8997 to encounter
> this behavior if they're not affected (especially users that depend on
> Bluetooth for keyboard/mouse input) so the new behavior is enabled via
> module param. We are limiting this quirk to only Chromebooks (i.e.
> laptop). Chromeboxes will continue to have the old behavior since users
> may depend on BT HID to wake and use the system.

I don’t have a super great feeling with this change.

So historically only hciconfig hci0 up/down was doing a power cycle of the controller and when adding the mgmt interface we moved that to the mgmt interface. In addition we added a special case of power up via hdev->setup. We never had an intention that the kernel otherwise can power up/down the controller as it pleases.

Can we ask Marvell first to investigate why this is fundamentally broken with their hardware? Since what you are proposing is a pretty heavy change that might has side affects. For example the state machine for the mgmt interface has no concept of a power down/up from the kernel. It is all triggered by bluetoothd.

I am careful here since the whole power up/down path is already complicated enough.

Regards

Marcel