linux-kernel - RE: [PATCH] mmc: block: ioctl: Poll for TRAN if possible

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: <DM6PR04MB6575996048A99DF35041BE67FC369@DM6PR04MB6575.namprd04.prod.outlook.com>
Date:   Wed, 9 Jun 2021 10:53:51 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Christian Löhle <CLoehle@...erstone.com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        "shawn.lin@...k-chips.com" <shawn.lin@...k-chips.com>
Subject: RE: [PATCH] mmc: block: ioctl: Poll for TRAN if possible

> Poll for TRAN state if the ioctl command will eventually return to TRAN
> 
> The ioctl submitted command should not be considered completed until
> the card has returned back to TRAN state. Waiting just for the card
> to no longer signal busy is not enough as they might remain in a
> non-busy PROG state for a while after the command.
> Further commands requiring TRAN will fail then.
> It should not be the responsibility of the user to check if their command
> has completed until sending the next via ioctl,
> instead the check should be made here.
> So now, in doubt, wait for TRAN except for the few commands that will
> never return to TRAN state.
> 
> Signed-off-by: Christian Loehle <cloehle@...erstone.com>
> ---
>  drivers/mmc/core/block.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 689eb9afeeed..a02187e4fad7 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -410,7 +410,23 @@ static int mmc_blk_ioctl_copy_to_user(struct
> mmc_ioc_cmd __user *ic_ptr,
>          return 0;
>  }
> 
> -static int card_busy_detect(struct mmc_card *card, unsigned int
> timeout_ms,
> +static int is_return_to_tran_cmd(struct mmc_command *cmd)
> +{
> +       /* Cards will never return to TRAN after completing
Multi-line comment style

> +        * identification commands or MMC_SEND_STATUS if they are not
> selected.
> +        */
> +       return !(cmd->opcode == MMC_SEND_CID
> +                       || cmd->opcode == MMC_ALL_SEND_CID
> +                       || cmd->opcode == MMC_SEND_CSD
> +                       || cmd->opcode == MMC_SEND_STATUS
> +                       || cmd->opcode == MMC_SELECT_CARD
> +                       || cmd->opcode == MMC_APP_CMD
> +                       || cmd->opcode == MMC_GO_INACTIVE_STATE
> +                       || cmd->opcode == MMC_GO_IDLE_STATE);
> +
Aren’t you only interested in cmds that move to tran state from other state?
According to the Device state transitions (table 61 in eMMC5.1) it only concern
cmd7 (stby->tran), cmd12 (data->tran), and cmd14 (btst->tran).

Thanks,
Avri
> +}
> +
> +static int card_poll_until_tran(struct mmc_card *card, unsigned int
> timeout_ms,
>                              u32 *resp_errs)
>  {
>          unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> @@ -593,12 +609,13 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> *card, struct mmc_blk_data *md,
> 
>          memcpy(&(idata->ic.response), cmd.resp, sizeof(cmd.resp));
> 
> -       if (idata->rpmb || (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B) {
> +       if (idata->rpmb || (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B
> +                       || is_return_to_tran_cmd(&cmd)) {
>                  /*
>                   * Ensure RPMB/R1B command has completed by polling CMD13
>                   * "Send Status".
>                   */
> -               err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, NULL);
> +               err = card_poll_until_tran(card, MMC_BLK_TIMEOUT_MS, NULL);
>          }
> 
>          return err;
> @@ -1621,7 +1638,7 @@ static int mmc_blk_fix_state(struct mmc_card
> *card, struct request *req)
> 
>          mmc_blk_send_stop(card, timeout);
> 
> -       err = card_busy_detect(card, timeout, NULL);
> +       err = card_poll_until_tran(card, timeout, NULL);
> 
>          mmc_retune_release(card->host);
> 
> @@ -1845,7 +1862,7 @@ static int mmc_blk_card_busy(struct mmc_card
> *card, struct request *req)
>          if (mmc_host_is_spi(card->host) || rq_data_dir(req) == READ)
>                  return 0;
> 
> -       err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, &status);
> +       err = card_poll_until_tran(card, MMC_BLK_TIMEOUT_MS, &status);
> 
>          /*
>           * Do not assume data transferred correctly if there are any error bits
> --
> 2.31.1
> Hyperstone GmbH | Line-Eid-Strasse 3 | 78467 Konstanz
> Managing Directors: Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782