2023-05-25 10:11:29

by Christian Loehle

[permalink] [raw]
Subject: [PATCHv2 2/2] mmc: block: ioctl: Add PROG-error aggregation

Userspace currently has no way of checking for error bits of
detection mode X. These are error bits that are only detected by
the card when executing the command. For e.g. a sanitize operation
this may be minutes after the RSP was seen by the host.

Currently userspace programs cannot see these error bits reliably.
They could issue a multi ioctl cmd with a CMD13 immediately following
it, but since errors of detection mode X are automatically cleared
(they are all clear condition B).
mmc_poll_for_busy of the first ioctl may have already hidden such an
error flag.

In case of the security operations: sanitize, secure erases and
RPMB writes, this could lead to the operation not being performed
successfully by the card with the user not knowing.
If the user trusts that this operation is completed
(e.g. their data is sanitized), this could be a security issue.
An attacker could e.g. provoke a eMMC (VCC) flash fail, where a
successful sanitize of a card is not possible. A card may move out
of PROG state but issue a bit 19 R1 error.

This patch therefore will also have the consequence of a mmc-utils
patch, which enables the bit for the security-sensitive operations.

Signed-off-by: Christian Loehle <[email protected]>
---
drivers/mmc/core/block.c | 17 ++++++-----------
drivers/mmc/core/mmc_ops.c | 25 ++++++++++++++++++++++++-
drivers/mmc/core/mmc_ops.h | 3 +++
3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index e46330815484..44c1b2825032 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -470,7 +470,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
struct mmc_data data = {};
struct mmc_request mrq = {};
struct scatterlist sg;
- bool r1b_resp, use_r1b_resp = false;
+ bool r1b_resp;
unsigned int busy_timeout_ms;
int err;
unsigned int target_part;
@@ -551,8 +551,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
if (r1b_resp)
- use_r1b_resp = mmc_prepare_busy_cmd(card->host, &cmd,
- busy_timeout_ms);
+ mmc_prepare_busy_cmd(card->host, &cmd, busy_timeout_ms);

mmc_wait_for_req(card->host, &mrq);
memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp));
@@ -605,19 +604,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
if (idata->ic.postsleep_min_us)
usleep_range(idata->ic.postsleep_min_us, idata->ic.postsleep_max_us);

- /* No need to poll when using HW busy detection. */
- if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
- return 0;
-
if (mmc_host_is_spi(card->host)) {
if (idata->ic.write_flag || r1b_resp || cmd.flags & MMC_RSP_SPI_BUSY)
return mmc_spi_err_check(card);
return err;
}
- /* Ensure RPMB/R1B command has completed by polling with CMD13. */
- if (idata->rpmb || r1b_resp)
- err = mmc_poll_for_busy(card, busy_timeout_ms, false,
- MMC_BUSY_IO);
+ /* Poll for write/R1B execution errors */
+ if (idata->ic.write_flag || r1b_resp)
+ err = mmc_poll_for_busy_err_flags(card, busy_timeout_ms, false,
+ MMC_BUSY_IO, &idata->ic.response[0]);

return err;
}
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 3b3adbddf664..11e566ab719c 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -57,7 +57,9 @@ static const u8 tuning_blk_pattern_8bit[] = {
struct mmc_busy_data {
struct mmc_card *card;
bool retry_crc_err;
+ bool aggregate_err_flags;
enum mmc_busy_cmd busy_cmd;
+ u32 *status;
};

struct mmc_op_cond_busy_data {
@@ -464,7 +466,8 @@ static int mmc_busy_cb(void *cb_data, bool *busy)
u32 status = 0;
int err;

- if (data->busy_cmd != MMC_BUSY_IO && host->ops->card_busy) {
+ if (data->busy_cmd != MMC_BUSY_IO && host->ops->card_busy &&
+ !data->aggregate_err_flags) {
*busy = host->ops->card_busy(host);
return 0;
}
@@ -477,6 +480,9 @@ static int mmc_busy_cb(void *cb_data, bool *busy)
if (err)
return err;

+ if (data->aggregate_err_flags)
+ *data->status = R1_STATUS(*data->status) | status;
+
switch (data->busy_cmd) {
case MMC_BUSY_CMD6:
err = mmc_switch_status_error(host, status);
@@ -549,12 +555,29 @@ int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,

cb_data.card = card;
cb_data.retry_crc_err = retry_crc_err;
+ cb_data.aggregate_err_flags = false;
cb_data.busy_cmd = busy_cmd;

return __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_busy_cb, &cb_data);
}
EXPORT_SYMBOL_GPL(mmc_poll_for_busy);

+int mmc_poll_for_busy_err_flags(struct mmc_card *card, unsigned int timeout_ms,
+ bool retry_crc_err, enum mmc_busy_cmd busy_cmd, u32 *status)
+{
+ struct mmc_host *host = card->host;
+ struct mmc_busy_data cb_data;
+
+ cb_data.card = card;
+ cb_data.retry_crc_err = retry_crc_err;
+ cb_data.aggregate_err_flags = true;
+ cb_data.busy_cmd = busy_cmd;
+ cb_data.status = status;
+
+ return __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_busy_cb, &cb_data);
+}
+EXPORT_SYMBOL_GPL(mmc_poll_for_busy_err_flags);
+
bool mmc_prepare_busy_cmd(struct mmc_host *host, struct mmc_command *cmd,
unsigned int timeout_ms)
{
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 09ffbc00908b..fc7ec43a78dc 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -47,6 +47,9 @@ int __mmc_poll_for_busy(struct mmc_host *host, unsigned int period_us,
void *cb_data);
int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
bool retry_crc_err, enum mmc_busy_cmd busy_cmd);
+int mmc_poll_for_busy_err_flags(struct mmc_card *card, unsigned int timeout_ms,
+ bool retry_crc_err, enum mmc_busy_cmd busy_cmd,
+ u32 *status);
int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
unsigned int timeout_ms, unsigned char timing,
bool send_status, bool retry_crc_err, unsigned int retries);
--
2.37.3


Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782



2023-06-08 15:56:00

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCHv2 2/2] mmc: block: ioctl: Add PROG-error aggregation

On Thu, 25 May 2023 at 11:56, Christian Loehle <[email protected]> wrote:
>
> Userspace currently has no way of checking for error bits of
> detection mode X. These are error bits that are only detected by
> the card when executing the command. For e.g. a sanitize operation
> this may be minutes after the RSP was seen by the host.
>
> Currently userspace programs cannot see these error bits reliably.
> They could issue a multi ioctl cmd with a CMD13 immediately following
> it, but since errors of detection mode X are automatically cleared
> (they are all clear condition B).
> mmc_poll_for_busy of the first ioctl may have already hidden such an
> error flag.
>
> In case of the security operations: sanitize, secure erases and
> RPMB writes, this could lead to the operation not being performed
> successfully by the card with the user not knowing.
> If the user trusts that this operation is completed
> (e.g. their data is sanitized), this could be a security issue.
> An attacker could e.g. provoke a eMMC (VCC) flash fail, where a
> successful sanitize of a card is not possible. A card may move out
> of PROG state but issue a bit 19 R1 error.
>
> This patch therefore will also have the consequence of a mmc-utils
> patch, which enables the bit for the security-sensitive operations.
>
> Signed-off-by: Christian Loehle <[email protected]>
> ---
> drivers/mmc/core/block.c | 17 ++++++-----------
> drivers/mmc/core/mmc_ops.c | 25 ++++++++++++++++++++++++-
> drivers/mmc/core/mmc_ops.h | 3 +++
> 3 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index e46330815484..44c1b2825032 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -470,7 +470,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> struct mmc_data data = {};
> struct mmc_request mrq = {};
> struct scatterlist sg;
> - bool r1b_resp, use_r1b_resp = false;
> + bool r1b_resp;
> unsigned int busy_timeout_ms;
> int err;
> unsigned int target_part;
> @@ -551,8 +551,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
> r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
> if (r1b_resp)
> - use_r1b_resp = mmc_prepare_busy_cmd(card->host, &cmd,
> - busy_timeout_ms);
> + mmc_prepare_busy_cmd(card->host, &cmd, busy_timeout_ms);
>
> mmc_wait_for_req(card->host, &mrq);
> memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp));
> @@ -605,19 +604,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> if (idata->ic.postsleep_min_us)
> usleep_range(idata->ic.postsleep_min_us, idata->ic.postsleep_max_us);
>
> - /* No need to poll when using HW busy detection. */
> - if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
> - return 0;
> -
> if (mmc_host_is_spi(card->host)) {
> if (idata->ic.write_flag || r1b_resp || cmd.flags & MMC_RSP_SPI_BUSY)
> return mmc_spi_err_check(card);
> return err;
> }
> - /* Ensure RPMB/R1B command has completed by polling with CMD13. */
> - if (idata->rpmb || r1b_resp)
> - err = mmc_poll_for_busy(card, busy_timeout_ms, false,
> - MMC_BUSY_IO);
> + /* Poll for write/R1B execution errors */
> + if (idata->ic.write_flag || r1b_resp)

Earlier we polled for requests that were targeted to rpmb, no matter
if they were write or reads. Are you intentionally changing this? If
so, can you explain why?

> + err = mmc_poll_for_busy_err_flags(card, busy_timeout_ms, false,
> + MMC_BUSY_IO, &idata->ic.response[0]);

I think it's better to extend the mmc_blk_busy_cb, rather than
introducing an entirely new polling function.

Then you can call __mmc_poll_for_busy() here instead.

>
> return err;
> }
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 3b3adbddf664..11e566ab719c 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -57,7 +57,9 @@ static const u8 tuning_blk_pattern_8bit[] = {
> struct mmc_busy_data {
> struct mmc_card *card;
> bool retry_crc_err;
> + bool aggregate_err_flags;
> enum mmc_busy_cmd busy_cmd;
> + u32 *status;
> };
>
> struct mmc_op_cond_busy_data {
> @@ -464,7 +466,8 @@ static int mmc_busy_cb(void *cb_data, bool *busy)
> u32 status = 0;
> int err;
>
> - if (data->busy_cmd != MMC_BUSY_IO && host->ops->card_busy) {
> + if (data->busy_cmd != MMC_BUSY_IO && host->ops->card_busy &&
> + !data->aggregate_err_flags) {
> *busy = host->ops->card_busy(host);
> return 0;
> }
> @@ -477,6 +480,9 @@ static int mmc_busy_cb(void *cb_data, bool *busy)
> if (err)
> return err;
>
> + if (data->aggregate_err_flags)
> + *data->status = R1_STATUS(*data->status) | status;
> +
> switch (data->busy_cmd) {
> case MMC_BUSY_CMD6:
> err = mmc_switch_status_error(host, status);
> @@ -549,12 +555,29 @@ int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
>
> cb_data.card = card;
> cb_data.retry_crc_err = retry_crc_err;
> + cb_data.aggregate_err_flags = false;
> cb_data.busy_cmd = busy_cmd;
>
> return __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_busy_cb, &cb_data);
> }
> EXPORT_SYMBOL_GPL(mmc_poll_for_busy);
>
> +int mmc_poll_for_busy_err_flags(struct mmc_card *card, unsigned int timeout_ms,
> + bool retry_crc_err, enum mmc_busy_cmd busy_cmd, u32 *status)
> +{
> + struct mmc_host *host = card->host;
> + struct mmc_busy_data cb_data;
> +
> + cb_data.card = card;
> + cb_data.retry_crc_err = retry_crc_err;
> + cb_data.aggregate_err_flags = true;
> + cb_data.busy_cmd = busy_cmd;
> + cb_data.status = status;
> +
> + return __mmc_poll_for_busy(host, 0, timeout_ms, &mmc_busy_cb, &cb_data);
> +}
> +EXPORT_SYMBOL_GPL(mmc_poll_for_busy_err_flags);
> +
> bool mmc_prepare_busy_cmd(struct mmc_host *host, struct mmc_command *cmd,
> unsigned int timeout_ms)
> {
> diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
> index 09ffbc00908b..fc7ec43a78dc 100644
> --- a/drivers/mmc/core/mmc_ops.h
> +++ b/drivers/mmc/core/mmc_ops.h
> @@ -47,6 +47,9 @@ int __mmc_poll_for_busy(struct mmc_host *host, unsigned int period_us,
> void *cb_data);
> int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
> bool retry_crc_err, enum mmc_busy_cmd busy_cmd);
> +int mmc_poll_for_busy_err_flags(struct mmc_card *card, unsigned int timeout_ms,
> + bool retry_crc_err, enum mmc_busy_cmd busy_cmd,
> + u32 *status);
> int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> unsigned int timeout_ms, unsigned char timing,
> bool send_status, bool retry_crc_err, unsigned int retries);
> --

Kind regards
Uffe

2023-06-20 11:42:40

by Christian Loehle

[permalink] [raw]
Subject: RE: [PATCHv2 2/2] mmc: block: ioctl: Add PROG-error aggregation

>>
>> Userspace currently has no way of checking for error bits of detection
>> mode X. These are error bits that are only detected by the card when
>> executing the command. For e.g. a sanitize operation this may be
>> minutes after the RSP was seen by the host.
>>
>> Currently userspace programs cannot see these error bits reliably.
>> They could issue a multi ioctl cmd with a CMD13 immediately following
>> it, but since errors of detection mode X are automatically cleared
>> (they are all clear condition B).
>> mmc_poll_for_busy of the first ioctl may have already hidden such an
>> error flag.
>>
>> In case of the security operations: sanitize, secure erases and RPMB
>> writes, this could lead to the operation not being performed
>> successfully by the card with the user not knowing.
>> If the user trusts that this operation is completed (e.g. their data
>> is sanitized), this could be a security issue.
>> An attacker could e.g. provoke a eMMC (VCC) flash fail, where a
>> successful sanitize of a card is not possible. A card may move out of
>> PROG state but issue a bit 19 R1 error.
>>
>> This patch therefore will also have the consequence of a mmc-utils
>> patch, which enables the bit for the security-sensitive operations.
>>
>> Signed-off-by: Christian Loehle <[email protected]>
>> ---
>> drivers/mmc/core/block.c | 17 ++++++-----------
>> drivers/mmc/core/mmc_ops.c | 25 ++++++++++++++++++++++++-
>> drivers/mmc/core/mmc_ops.h | 3 +++
>> 3 files changed, 33 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
>> e46330815484..44c1b2825032 100644
>> --- a/drivers/mmc/core/block.c
>> +++ b/drivers/mmc/core/block.c
>> @@ -470,7 +470,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>> struct mmc_data data = {};
>> struct mmc_request mrq = {};
>> struct scatterlist sg;
>> - bool r1b_resp, use_r1b_resp = false;
>> + bool r1b_resp;
>> unsigned int busy_timeout_ms;
>> int err;
>> unsigned int target_part;
>> @@ -551,8 +551,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>> busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
>> r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
>> if (r1b_resp)
>> - use_r1b_resp = mmc_prepare_busy_cmd(card->host, &cmd,
>> - busy_timeout_ms);
>> + mmc_prepare_busy_cmd(card->host, &cmd,
>> + busy_timeout_ms);
>>
>> mmc_wait_for_req(card->host, &mrq);
>> memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp)); @@
>> -605,19 +604,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>> if (idata->ic.postsleep_min_us)
>> usleep_range(idata->ic.postsleep_min_us,
>> idata->ic.postsleep_max_us);
>>
>> - /* No need to poll when using HW busy detection. */
>> - if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
>> - return 0;
>> -
>> if (mmc_host_is_spi(card->host)) {
>> if (idata->ic.write_flag || r1b_resp || cmd.flags & MMC_RSP_SPI_BUSY)
>> return mmc_spi_err_check(card);
>> return err;
>> }
>> - /* Ensure RPMB/R1B command has completed by polling with CMD13. */
>> - if (idata->rpmb || r1b_resp)
>> - err = mmc_poll_for_busy(card, busy_timeout_ms, false,
>> - MMC_BUSY_IO);
>> + /* Poll for write/R1B execution errors */
>> + if (idata->ic.write_flag || r1b_resp)
>
> Earlier we polled for requests that were targeted to rpmb, no matter if they were write or reads. Are you intentionally changing this? If so, can you explain why?
>
Will re-introduce. I cant really think of a reason right now to do this after rpmb reads, but thats a different story.

>> + err = mmc_poll_for_busy_err_flags(card, busy_timeout_ms, false,
>> + MMC_BUSY_IO,
>> + &idata->ic.response[0]);
>
> I think it's better to extend the mmc_blk_busy_cb, rather than introducing an entirely new polling function.
>
> Then you can call __mmc_poll_for_busy() here instead.

Not sure if I understood you right, but I will send a new version with __mmc_poll_for_busy call directly.
It does feel a bit more awkward, at least to me, because both mmc_blk_busy_cb nor mmc_busy_data are currently only in mmc_ops.c

Anyway, both versions "extend the mmc_blk_busy_cb", so I'm not sure if I understood you correctly, we will see.
I may also just send both and you pick whichever you prefer.

Regards,
Christian


Attachments:
smime.p7s (10.06 kB)

2023-06-20 16:02:00

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCHv2 2/2] mmc: block: ioctl: Add PROG-error aggregation

On Tue, 20 Jun 2023 at 13:23, Christian Loehle <[email protected]> wrote:
>
> >>
> >> Userspace currently has no way of checking for error bits of detection
> >> mode X. These are error bits that are only detected by the card when
> >> executing the command. For e.g. a sanitize operation this may be
> >> minutes after the RSP was seen by the host.
> >>
> >> Currently userspace programs cannot see these error bits reliably.
> >> They could issue a multi ioctl cmd with a CMD13 immediately following
> >> it, but since errors of detection mode X are automatically cleared
> >> (they are all clear condition B).
> >> mmc_poll_for_busy of the first ioctl may have already hidden such an
> >> error flag.
> >>
> >> In case of the security operations: sanitize, secure erases and RPMB
> >> writes, this could lead to the operation not being performed
> >> successfully by the card with the user not knowing.
> >> If the user trusts that this operation is completed (e.g. their data
> >> is sanitized), this could be a security issue.
> >> An attacker could e.g. provoke a eMMC (VCC) flash fail, where a
> >> successful sanitize of a card is not possible. A card may move out of
> >> PROG state but issue a bit 19 R1 error.
> >>
> >> This patch therefore will also have the consequence of a mmc-utils
> >> patch, which enables the bit for the security-sensitive operations.
> >>
> >> Signed-off-by: Christian Loehle <[email protected]>
> >> ---
> >> drivers/mmc/core/block.c | 17 ++++++-----------
> >> drivers/mmc/core/mmc_ops.c | 25 ++++++++++++++++++++++++-
> >> drivers/mmc/core/mmc_ops.h | 3 +++
> >> 3 files changed, 33 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> >> e46330815484..44c1b2825032 100644
> >> --- a/drivers/mmc/core/block.c
> >> +++ b/drivers/mmc/core/block.c
> >> @@ -470,7 +470,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> >> struct mmc_data data = {};
> >> struct mmc_request mrq = {};
> >> struct scatterlist sg;
> >> - bool r1b_resp, use_r1b_resp = false;
> >> + bool r1b_resp;
> >> unsigned int busy_timeout_ms;
> >> int err;
> >> unsigned int target_part;
> >> @@ -551,8 +551,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> >> busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
> >> r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
> >> if (r1b_resp)
> >> - use_r1b_resp = mmc_prepare_busy_cmd(card->host, &cmd,
> >> - busy_timeout_ms);
> >> + mmc_prepare_busy_cmd(card->host, &cmd,
> >> + busy_timeout_ms);
> >>
> >> mmc_wait_for_req(card->host, &mrq);
> >> memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp)); @@
> >> -605,19 +604,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> >> if (idata->ic.postsleep_min_us)
> >> usleep_range(idata->ic.postsleep_min_us,
> >> idata->ic.postsleep_max_us);
> >>
> >> - /* No need to poll when using HW busy detection. */
> >> - if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
> >> - return 0;
> >> -
> >> if (mmc_host_is_spi(card->host)) {
> >> if (idata->ic.write_flag || r1b_resp || cmd.flags & MMC_RSP_SPI_BUSY)
> >> return mmc_spi_err_check(card);
> >> return err;
> >> }
> >> - /* Ensure RPMB/R1B command has completed by polling with CMD13. */
> >> - if (idata->rpmb || r1b_resp)
> >> - err = mmc_poll_for_busy(card, busy_timeout_ms, false,
> >> - MMC_BUSY_IO);
> >> + /* Poll for write/R1B execution errors */
> >> + if (idata->ic.write_flag || r1b_resp)
> >
> > Earlier we polled for requests that were targeted to rpmb, no matter if they were write or reads. Are you intentionally changing this? If so, can you explain why?
> >
> Will re-introduce. I cant really think of a reason right now to do this after rpmb reads, but thats a different story.

Okay, good.

My main point is, if we want to change that, let's do that as a separate patch.

>
> >> + err = mmc_poll_for_busy_err_flags(card, busy_timeout_ms, false,
> >> + MMC_BUSY_IO,
> >> + &idata->ic.response[0]);
> >
> > I think it's better to extend the mmc_blk_busy_cb, rather than introducing an entirely new polling function.
> >
> > Then you can call __mmc_poll_for_busy() here instead.
>
> Not sure if I understood you right, but I will send a new version with __mmc_poll_for_busy call directly.
> It does feel a bit more awkward, at least to me, because both mmc_blk_busy_cb nor mmc_busy_data are currently only in mmc_ops.c
>
> Anyway, both versions "extend the mmc_blk_busy_cb", so I'm not sure if I understood you correctly, we will see.
> I may also just send both and you pick whichever you prefer.

I was thinking that mmc_blk_card_busy() calls __mmc_poll_for_busy().
While doing that, it uses the mmc_blk_busy_cb() - which seems to be
almost what we want to do here too.

Did that make sense?

Kind regards
Uffe