linux-kernel - Re: [PATCH] mmc: meson: move mmc_request

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]

Date:   Tue, 6 Jun 2023 21:22:18 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Martin Hundebøll <martin@...nix.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Martin Hundebøll <mhu@....dk>,
        stable@...r.kernel.org, linux-mmc@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mmc: meson: move mmc_request_done() call to irq thread

Hi Martin,

On Tue, Jun 6, 2023 at 8:59 AM Martin Hundebøll <martin@...nix.com> wrote:
[...]
> @@ -1040,6 +1034,13 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>         if (WARN_ON(!cmd))
>                 return IRQ_NONE;
>
> +       if (!meson_mmc_bounce_buf_read(cmd->data) &&
> +           !meson_mmc_get_next_command(cmd)) {
> +               meson_mmc_request_done(host->mmc, cmd->mrq);
> +
> +               return IRQ_HANDLED;
> +       }
> +
I'm wondering if this is changing the behavior when cmd->error is non-zero.

Let's take the "status & IRQ_CRC_ERR" case from meson_mmc_irq() - previously:
- cmd->error is set
- regmap_update_bits(host->regmap, SD_EMMC_START, START_DESC_BUSY, 0);
- IRQ_WAKE_THREAD
- then inside meson_mmc_irq_thread():
-- meson_mmc_wait_desc_stop(host);
-- meson_mmc_request_done(host->mmc, cmd->mrq);

With this patch:
- cmd->error is set
- regmap_update_bits(host->regmap, SD_EMMC_START, START_DESC_BUSY, 0);
- IRQ_WAKE_THREAD
- then inside meson_mmc_irq_thread():
-- !meson_mmc_bounce_buf_read(cmd->data) &&
!meson_mmc_get_next_command(cmd) can be true, which means we only
call:
--- meson_mmc_request_done(host->mmc, cmd->mrq);
--- (note the missing meson_mmc_wait_desc_stop() call before
meson_mmc_request_done())

What do you think?
Maybe we just need to remove the code from meson_mmc_irq() and just
omit any modifications to meson_mmc_irq_thread() (as it seems all
checks that you're adding are already present in that function below
the cmd->error case).


Best regards,
Martin