linux-kernel - Re: [PATCH 1/1] mmc: meson-mx-sdio: Set MMC_CAP_WAIT_WHILE

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: <CAFBinCAhfgEdT383x61mh9E0y3-3MLA-fUmZj-4C-_VUdGWtGQ@mail.gmail.com>
Date:   Wed, 15 Apr 2020 23:24:39 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
Cc:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "open list:ARM/Amlogic Meson..." <linux-amlogic@...ts.infradead.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] mmc: meson-mx-sdio: Set MMC_CAP_WAIT_WHILE_BUSY

Hi Ulf,

thank you very much for taking the time to look into this!

On Wed, Apr 15, 2020 at 2:57 PM Ulf Hansson <ulf.hansson@...aro.org> wrote:
[...]
> Thanks for sending this! I assume it's a regression and caused by one
> of my patches that went in for 5.7. Probably this one:
> 0d84c3e6a5b2 mmc: core: Convert to mmc_poll_for_busy() for erase/trim/discard
indeed, I only observed this with 5.7-rc1-ish, before everything was
working fine

> Now, even if enabling MMC_CAP_WAIT_WHILE_BUSY seems like the correct
> thing to do, I suggest we really try understand why it works, so we
> don't overlook some other issue that needs to be fixed.
great, that's why I'm seeking for help!

> Would you be willing to try a few debug patches, according to the below?
sure
while reading your suggestions I went back to the vendor driver and
observed that they don't implement card_busy for this controller
Thus I added the following line to meson_mx_mmc_card_busy for all of
your tests to see what the controller sees in terms of our card busy
implementation:
  dev_info(mmc_dev(host->mmc), "%s read IRQC = 0x%08x\n",
                 __func__, irqc);

> First, can you double check so the original polling with CMD13 is
> still okay, by trying the below minor change. The intent is to force
> polling with CMD13 for the erase/discard operation.
I have tried this one and it seems to work around the problem (before
I reverted my change and dropped MMC_CAP_WAIT_WHILE_BUSY from
mmc->caps)
also I did not see meson_mx_mmc_card_busy being invoked (not even
once, but I don't know if that's expected)

[...]
> Second, if the above works, it looks like the polling with
> ->card_busy() isn't really working for meson-mx-sdio.c, together with
> erase/discard. To narrow down that problem, I suggest to try with a
> longer erase/discard timeout in a retry fashion, while using
> ->card_busy(). Along the lines of the below:
I have tried this one as well (before I reverted the earlier CMD13
patch) and with MMC_CAP_WAIT_WHILE_BUSY unset in mmc->caps
This doesn't seem to work around the issue - kernel log extract attached.
Also I'm seeing that the the current meson_mx_mmc_card_busy
implementation returns that the card is busy.
example: 0x1f001f10 & 0x3c00 = 0x1c00. the busy logic in the driver
is: !!0x1c00 = 1

My conclusion is:
- meson_mx_mmc_card_busy is not working and should be removed (because
I don't know how to make it work). it probably never worked but we
didn't notice until a recent change
- set MMC_CAP_WAIT_WHILE_BUSY as per my initial patch
- use Fixes: ed80a13bb4c4c9 ("mmc: meson-mx-sdio: Add a driver for the
Amlogic Meson8 and Meson8b SoCs")

Does this make sense?
Also please let me know if you want me to try something else


Martin

View attachment "erase-poll-retry.txt" of type "text/plain" (7709 bytes)