linux-kernel - Re: [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of the SDIO card

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] [day] [month] [year] [list]

Message-ID: <CANFp7mUKRRQT0m9jRB4aeE3GeSW8UM6d-NVJ3CZmHibhSny3+g@mail.gmail.com>
Date:   Fri, 20 Mar 2020 13:56:23 -0700
From:   Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        BlueZ <linux-bluetooth@...r.kernel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] Bluetooth: btmrvl: Detect hangs and force a reset of
 the SDIO card

Thanks for the heads up Doug. I'll resend the patch once I handle the
case where the reset is immediate and not a full unplug/replug.

In the meantime, please do not merge this patch.

Thanks
Abhishek

On Fri, Mar 20, 2020 at 1:00 PM Doug Anderson <dianders@...omium.org> wrote:
>
> Hi,
>
> On Thu, Mar 19, 2020 at 7:02 PM Abhishek Pandit-Subedi
> <abhishekpandit@...omium.org> wrote:
> >
> > From: Matthias Kaehlcke <mka@...omium.org>
> >
> > When scanning for BLE devices for a longer period (e.g. because a
> > BLE device is paired, but not connected) the Marvell 8997 often
> > ends up in a borked state, which manifests through failures on
> > certain SDIO transactions.
> >
> > When such a SDIO failure is detected force a reset of the SDIO
> > card to initialize it from scratch. Since the SDIO bus is shared
> > with the WiFi part of the chip this also involves a reset of WiFi.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
> > ---
> >
> >  drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++++
> >  drivers/bluetooth/btmrvl_sdio.h |  1 +
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
> > index 0f3a020703ab..69a8b6b3c11c 100644
> > --- a/drivers/bluetooth/btmrvl_sdio.c
> > +++ b/drivers/bluetooth/btmrvl_sdio.c
> > @@ -22,6 +22,8 @@
> >  #include <linux/slab.h>
> >  #include <linux/suspend.h>
> >
> > +#include <linux/mmc/core.h>
> > +#include <linux/mmc/card.h>
> >  #include <linux/mmc/sdio_ids.h>
> >  #include <linux/mmc/sdio_func.h>
> >  #include <linux/module.h>
> > @@ -59,6 +61,23 @@ static const struct of_device_id btmrvl_sdio_of_match_table[] = {
> >         { }
> >  };
> >
> > +static void btmrvl_sdio_card_reset_work(struct work_struct *work)
> > +{
> > +       struct btmrvl_sdio_card *card =
> > +               container_of(work, struct btmrvl_sdio_card, reset_work);
> > +       struct sdio_func *func = card->func;
> > +
> > +       sdio_claim_host(func);
> > +       mmc_hw_reset(func->card->host);
>
> The fact that you don't check the return value here seems like a
> problem.  See specifically how commit cdb2256f795e ("mwifiex: Re-work
> support for SDIO HW reset") handles it.
>
> This is a distinct difference between the solution that we landed in
> Chrome OS 4.19 and what landed upstream.  In Chrome OS 4.19 we went
> the simple approach and said that there was only one way to reset the
> card: treat it as a full unplug / replug of the card.  ...but upstream
> adopted a different solution.  For upstream if there is only a single
> function on the SD card it will not trigger a full unplug/replug and
> it's up to the function driver to re-init itself.  This wasn't such a
> big deal for the WiFi driver since it already had a way to re-init
> itself (mwifiex_reinit_sw).  I don't know how hard it will be for
> Bluetooth, but that needs to be part of this patch I think?
>
> -Doug