All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] mmc: refactor RPMB block count handling
@ 2018-11-20 23:08 Wolfram Sang
  2018-11-20 23:08 ` [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count Wolfram Sang
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Wolfram Sang @ 2018-11-20 23:08 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Wolfram Sang

On Renesas R-Car SDHI hardware, we sometimes had timeouts accessing the RPMB.
This is because AutoCMD23/12 features needs a properly filled sbc to work
correctly. But RPMB sends an individual CMD23. I could have fixed the driver
but after some research concluded that fixing the core seems the proper thing
to do. I also added some sanity checking while here. Please let me know what
you think.

Tested on a R-Car M3-N. No timeouts showed up anymore. I'll try to improve the
testing, though, to check if I can make the occasional timeouts from before
more reproducible. And then confirm that this patchset improves the situation :)

Wolfram Sang (3):
  mmc: core: validate user input for RPMB block count
  mmc: core: use mrq->sbc when sending CMD23 for RPMB
  mmc: core: remove obsolete mmc_set_blockcount() function

 drivers/mmc/core/block.c | 14 +++++++++-----
 drivers/mmc/core/core.c  | 14 --------------
 drivers/mmc/core/core.h  |  2 --
 3 files changed, 9 insertions(+), 21 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count
  2018-11-20 23:08 [RFC PATCH 0/3] mmc: refactor RPMB block count handling Wolfram Sang
@ 2018-11-20 23:08 ` Wolfram Sang
  2018-11-22 12:15   ` Avri Altman
  2018-11-25 22:12   ` Clément Péron
  2018-11-20 23:08 ` [RFC PATCH 2/3] mmc: core: use mrq->sbc when sending CMD23 for RPMB Wolfram Sang
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 18+ messages in thread
From: Wolfram Sang @ 2018-11-20 23:08 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Wolfram Sang

For RPMB, block count is a non-zero 16 bit wide number. Reject invalid
values from userspace instead of just masking the unneeded bits. Tested
with a modified 'mmc-utils' package.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/core/block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index c35b5b08bb33..9e0f7e4aa8c6 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -550,6 +550,9 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	}
 
 	if (idata->rpmb) {
+		if (data.blocks > 65535 || !data.blocks)
+			return -EINVAL;
+
 		err = mmc_set_blockcount(card, data.blocks,
 			idata->ic.write_flag & (1 << 31));
 		if (err)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 2/3] mmc: core: use mrq->sbc when sending CMD23 for RPMB
  2018-11-20 23:08 [RFC PATCH 0/3] mmc: refactor RPMB block count handling Wolfram Sang
  2018-11-20 23:08 ` [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count Wolfram Sang
@ 2018-11-20 23:08 ` Wolfram Sang
  2018-11-25 22:13   ` Clément Péron
  2018-11-20 23:08 ` [RFC PATCH 3/3] mmc: core: remove obsolete mmc_set_blockcount() function Wolfram Sang
  2018-11-21  0:31 ` [RFC PATCH 0/3] mmc: refactor RPMB block count handling Ulf Hansson
  3 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2018-11-20 23:08 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Wolfram Sang, Masaharu Hayakawa

When sending out CMD23 in the blk preparation, the comment there
rightfully says:

	 * However, it is not sufficient to just send CMD23,
	 * and avoid the final CMD12, as on an error condition
	 * CMD12 (stop) needs to be sent anyway. This, coupled
	 * with Auto-CMD23 enhancements provided by some
	 * hosts, means that the complexity of dealing
	 * with this is best left to the host. If CMD23 is
	 * supported by card and host, we'll fill sbc in and let
	 * the host deal with handling it correctly.

Let's do this behaviour for RPMB as well, and not send CMD23
independently. Otherwise IP cores (like Renesas SDHI) may timeout
because of automatic CMD23/CMD12 handling.

Reported-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

I also noticed that write_flag is an int, and we are playing with the sign bit
on 32-bit systems. But sadly this is exported to userspace...

 drivers/mmc/core/block.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 9e0f7e4aa8c6..66fc425615cd 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -472,7 +472,7 @@ static int ioctl_do_sanitize(struct mmc_card *card)
 static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 			       struct mmc_blk_ioc_data *idata)
 {
-	struct mmc_command cmd = {};
+	struct mmc_command cmd = {}, sbc = {};
 	struct mmc_data data = {};
 	struct mmc_request mrq = {};
 	struct scatterlist sg;
@@ -553,10 +553,11 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 		if (data.blocks > 65535 || !data.blocks)
 			return -EINVAL;
 
-		err = mmc_set_blockcount(card, data.blocks,
-			idata->ic.write_flag & (1 << 31));
-		if (err)
-			return err;
+		sbc.opcode = MMC_SET_BLOCK_COUNT;
+		/* copy the 'Reliable Write' bit */
+		sbc.arg = data.blocks | (idata->ic.write_flag & BIT(31));
+		sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+		mrq.sbc = &sbc;
 	}
 
 	if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 3/3] mmc: core: remove obsolete mmc_set_blockcount() function
  2018-11-20 23:08 [RFC PATCH 0/3] mmc: refactor RPMB block count handling Wolfram Sang
  2018-11-20 23:08 ` [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count Wolfram Sang
  2018-11-20 23:08 ` [RFC PATCH 2/3] mmc: core: use mrq->sbc when sending CMD23 for RPMB Wolfram Sang
@ 2018-11-20 23:08 ` Wolfram Sang
  2018-11-25 22:13   ` Clément Péron
  2018-11-21  0:31 ` [RFC PATCH 0/3] mmc: refactor RPMB block count handling Ulf Hansson
  3 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2018-11-20 23:08 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Wolfram Sang

The only user was converted to fill a sbc command which is the proper
way to do it because of AutoCMD23 feature of some hosts.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/core/core.c | 14 --------------
 drivers/mmc/core/core.h |  2 --
 2 files changed, 16 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 50a5c340307b..d3085f70e9a4 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2413,20 +2413,6 @@ int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen)
 }
 EXPORT_SYMBOL(mmc_set_blocklen);
 
-int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
-			bool is_rel_write)
-{
-	struct mmc_command cmd = {};
-
-	cmd.opcode = MMC_SET_BLOCK_COUNT;
-	cmd.arg = blockcount & 0x0000FFFF;
-	if (is_rel_write)
-		cmd.arg |= 1 << 31;
-	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
-	return mmc_wait_for_cmd(card->host, &cmd, 5);
-}
-EXPORT_SYMBOL(mmc_set_blockcount);
-
 static void mmc_hw_reset_for_init(struct mmc_host *host)
 {
 	mmc_pwrseq_reset(host);
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 087ba68b2920..8fb6bc37f808 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -118,8 +118,6 @@ int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
 unsigned int mmc_calc_max_discard(struct mmc_card *card);
 
 int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
-int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
-			bool is_rel_write);
 
 int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
 		     atomic_t *abort);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-20 23:08 [RFC PATCH 0/3] mmc: refactor RPMB block count handling Wolfram Sang
                   ` (2 preceding siblings ...)
  2018-11-20 23:08 ` [RFC PATCH 3/3] mmc: core: remove obsolete mmc_set_blockcount() function Wolfram Sang
@ 2018-11-21  0:31 ` Ulf Hansson
  2018-11-21  2:35   ` Wolfram Sang
  3 siblings, 1 reply; 18+ messages in thread
From: Ulf Hansson @ 2018-11-21  0:31 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc, Linux-Renesas, Clément Péron, Avri Altman

+ Avri, Clément (due to recent related discussions)

On 21 November 2018 at 00:08, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> On Renesas R-Car SDHI hardware, we sometimes had timeouts accessing the RPMB.
> This is because AutoCMD23/12 features needs a properly filled sbc to work
> correctly. But RPMB sends an individual CMD23. I could have fixed the driver
> but after some research concluded that fixing the core seems the proper thing
> to do. I also added some sanity checking while here. Please let me know what
> you think.
>
> Tested on a R-Car M3-N. No timeouts showed up anymore. I'll try to improve the
> testing, though, to check if I can make the occasional timeouts from before
> more reproducible. And then confirm that this patchset improves the situation :)
>
> Wolfram Sang (3):
>   mmc: core: validate user input for RPMB block count
>   mmc: core: use mrq->sbc when sending CMD23 for RPMB
>   mmc: core: remove obsolete mmc_set_blockcount() function
>
>  drivers/mmc/core/block.c | 14 +++++++++-----
>  drivers/mmc/core/core.c  | 14 --------------
>  drivers/mmc/core/core.h  |  2 --
>  3 files changed, 9 insertions(+), 21 deletions(-)
>
> --
> 2.11.0
>

These changes makes perfect sense to me! I give it a day or two to
allow people to comment/test, then I will queue them up.

Perhaps we should add a suggested by tag from Clément for patch2, as
he kind of suggested this change already [1].

Kind regards
Uffe

[1]
https://patchwork.kernel.org/patch/10645847/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-21  0:31 ` [RFC PATCH 0/3] mmc: refactor RPMB block count handling Ulf Hansson
@ 2018-11-21  2:35   ` Wolfram Sang
  2018-11-22 10:35     ` Clément Péron
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2018-11-21  2:35 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Wolfram Sang, linux-mmc, Linux-Renesas, Clément Péron,
	Avri Altman

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

On Wed, Nov 21, 2018 at 01:31:35AM +0100, Ulf Hansson wrote:
> + Avri, Clément (due to recent related discussions)

Thanks!

> Perhaps we should add a suggested by tag from Clément for patch2, as
> he kind of suggested this change already [1].

No strong opinion on that, but technically my inspiration for patch 2
was Hayakawa-san. Add it as you see fit...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-21  2:35   ` Wolfram Sang
@ 2018-11-22 10:35     ` Clément Péron
  2018-11-24 14:26       ` Clément Péron
  0 siblings, 1 reply; 18+ messages in thread
From: Clément Péron @ 2018-11-22 10:35 UTC (permalink / raw)
  To: wsa; +Cc: Ulf Hansson, wsa+renesas, linux-mmc, linux-renesas-soc, Avri Altman

Hi Wolfram,

On Wed, 21 Nov 2018 at 03:35, Wolfram Sang <wsa@the-dreams.de> wrote:
>dispositifs
> On Wed, Nov 21, 2018 at 01:31:35AM +0100, Ulf Hansson wrote:
> > + Avri, Clément (due to recent related discussions)
>
> Thanks!
>
> > Perhaps we should add a suggested by tag from Clément for patch2, as
> > he kind of suggested this change already [1].
>
> No strong opinion on that, but technically my inspiration for patch 2
> was Hayakawa-san. Add it as you see fit...

Fine for me, so don't add it.
Let me just a day to test it.

Thanks,
Clement

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count
  2018-11-20 23:08 ` [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count Wolfram Sang
@ 2018-11-22 12:15   ` Avri Altman
  2018-11-22 14:05     ` Wolfram Sang
  2018-11-25 22:12   ` Clément Péron
  1 sibling, 1 reply; 18+ messages in thread
From: Avri Altman @ 2018-11-22 12:15 UTC (permalink / raw)
  To: Wolfram Sang, linux-mmc; +Cc: linux-renesas-soc

> 
> For RPMB, block count is a non-zero 16 bit wide number. Reject invalid
Actually this is not what limits the number of rpmb frames to be read,
As those are not indicated in the block_count field of the rpmb frame,
But in CMD23.
While it is true that by eMMCv52 RPMB_SIZE_MULT max value is 0x80,
Which limits the RPMB are to 16M, or 65535 rpmb frames,
I don't think that it is up to the host to validate the rpmb frame content - 
The device will return the appropriate error should such an error occur.
Also, specs are changing from time to time, so hard-coding 65535 is less appropriate.

> values from userspace instead of just masking the unneeded bits. Tested
> with a modified 'mmc-utils' package.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/mmc/core/block.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index c35b5b08bb33..9e0f7e4aa8c6 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -550,6 +550,9 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> *card, struct mmc_blk_data *md,
>  	}
> 
>  	if (idata->rpmb) {
> +		if (data.blocks > 65535 || !data.blocks)
> +			return -EINVAL;
> +

Other than my comment above, this series looks fine to me.

Thanks,
Avri

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count
  2018-11-22 12:15   ` Avri Altman
@ 2018-11-22 14:05     ` Wolfram Sang
  2018-11-22 15:57       ` Avri Altman
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2018-11-22 14:05 UTC (permalink / raw)
  To: Avri Altman; +Cc: Wolfram Sang, linux-mmc, linux-renesas-soc

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

Hi Avri,

thanks for your comments!

> > For RPMB, block count is a non-zero 16 bit wide number. Reject invalid
> Actually this is not what limits the number of rpmb frames to be read,
> As those are not indicated in the block_count field of the rpmb frame,
> But in CMD23.
> While it is true that by eMMCv52 RPMB_SIZE_MULT max value is 0x80,
> Which limits the RPMB are to 16M, or 65535 rpmb frames,
> I don't think that it is up to the host to validate the rpmb frame content - 

I wanted to fix the following issue:

Currently, in to-be-removed mmc_set_blockcount() we have:

	cmd.arg = blockcount & 0x0000FFFF;

So, sending a IOCTL with a value of 65537 will silently(!) produce a
block count of 1. I didn't like this.

I don't have the eMMC specs on this computer but I recall it is defined
somewhere that there are 2 bytes reserved for it in the packet frame?

But yes, standards may be extended, so I am not opposed to fill in
bigger numbers than 16 bit and let the card handle the error.

So you suggest dropping this patch and keep the others, did I get this
right? Would be fine with me.

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count
  2018-11-22 14:05     ` Wolfram Sang
@ 2018-11-22 15:57       ` Avri Altman
  2018-11-22 23:34         ` Wolfram Sang
  0 siblings, 1 reply; 18+ messages in thread
From: Avri Altman @ 2018-11-22 15:57 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Wolfram Sang, linux-mmc, linux-renesas-soc

> 
> Hi Avri,
> 
> thanks for your comments!
> 
> > > For RPMB, block count is a non-zero 16 bit wide number. Reject invalid
> > Actually this is not what limits the number of rpmb frames to be read,
> > As those are not indicated in the block_count field of the rpmb frame,
> > But in CMD23.
> > While it is true that by eMMCv52 RPMB_SIZE_MULT max value is 0x80,
> > Which limits the RPMB are to 16M, or 65535 rpmb frames,
> > I don't think that it is up to the host to validate the rpmb frame content -
> 
> I wanted to fix the following issue:
> 
> Currently, in to-be-removed mmc_set_blockcount() we have:
> 
> 	cmd.arg = blockcount & 0x0000FFFF;
> 
> So, sending a IOCTL with a value of 65537 will silently(!) produce a
> block count of 1. I didn't like this.
> 
> I don't have the eMMC specs on this computer but I recall it is defined
> somewhere that there are 2 bytes reserved for it in the packet frame?
Yes. But strangely as it may sound, this u16 in the rpmb frame should be set 
to 0x0 in all cases except for data write in which it may carry 1, 2, or 32.
The block count for all other operations is set by CMD23.
This might explain why the community people hate JEDEC guys so much.

I agree - the blockcount in mmc_set_blockcount carries not the frame
Content, but the mmc_ioc_cmd blocks field, which is unsigned int.
 So this line in mmc_set_blockcount performs an unnecessary input validation.

> 
> But yes, standards may be extended, so I am not opposed to fill in
> bigger numbers than 16 bit and let the card handle the error.
> 
> So you suggest dropping this patch and keep the others, did I get this
> right? Would be fine with me.
Yes. Thanks.

Cheers,
Avri

> 
> Regards,
> 
>    Wolfram

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count
  2018-11-22 15:57       ` Avri Altman
@ 2018-11-22 23:34         ` Wolfram Sang
  0 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2018-11-22 23:34 UTC (permalink / raw)
  To: Avri Altman; +Cc: Wolfram Sang, linux-mmc, linux-renesas-soc

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]


> This might explain why the community people hate JEDEC guys so much.

:D

Okay, this u16 problem of block count exists in mmc-tools, too.

And I'd like to resend this series with a comment added to explain the
situation. Because the specs seem to be easy to get wrong here.

>  So this line in mmc_set_blockcount performs an unnecessary input validation.

Even wrong input validation, as I understand it.

> > So you suggest dropping this patch and keep the others, did I get this
> > right? Would be fine with me.
> Yes. Thanks.

Good, let's do this then.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-22 10:35     ` Clément Péron
@ 2018-11-24 14:26       ` Clément Péron
  2018-11-24 21:37         ` Wolfram Sang
  0 siblings, 1 reply; 18+ messages in thread
From: Clément Péron @ 2018-11-24 14:26 UTC (permalink / raw)
  To: wsa; +Cc: Ulf Hansson, wsa+renesas, linux-mmc, linux-renesas-soc, Avri Altman

Hi

On Thu, 22 Nov 2018 at 11:35, Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi Wolfram,
>
> On Wed, 21 Nov 2018 at 03:35, Wolfram Sang <wsa@the-dreams.de> wrote:
> >dispositifs
> > On Wed, Nov 21, 2018 at 01:31:35AM +0100, Ulf Hansson wrote:
> > > + Avri, Clément (due to recent related discussions)
> >
> > Thanks!
> >
> > > Perhaps we should add a suggested by tag from Clément for patch2, as
> > > he kind of suggested this change already [1].
> >
> > No strong opinion on that, but technically my inspiration for patch 2
> > was Hayakawa-san. Add it as you see fit...
>
> Fine for me, so don't add it.
> Let me just a day to test it.

Tested fine with SDHCI Iproc controller on 4.9.137 and 4.19.2 with
Auto-CMD12 enable.
But there is something, it's not working not introduce with this patch on 4.14

Regards,
Clement

>
> Thanks,
> Clement

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-24 14:26       ` Clément Péron
@ 2018-11-24 21:37         ` Wolfram Sang
  2018-11-25 22:11           ` Clément Péron
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2018-11-24 21:37 UTC (permalink / raw)
  To: Clément Péron
  Cc: Ulf Hansson, wsa+renesas, linux-mmc, linux-renesas-soc, Avri Altman

[-- Attachment #1: Type: text/plain, Size: 240 bytes --]


> Tested fine with SDHCI Iproc controller on 4.9.137 and 4.19.2 with
> Auto-CMD12 enable.
> But there is something, it's not working not introduce with this patch on 4.14

I didn't get the last sentence. Could you explain in more detail?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-24 21:37         ` Wolfram Sang
@ 2018-11-25 22:11           ` Clément Péron
  2018-11-26  9:38             ` Wolfram Sang
  0 siblings, 1 reply; 18+ messages in thread
From: Clément Péron @ 2018-11-25 22:11 UTC (permalink / raw)
  To: wsa; +Cc: Ulf Hansson, wsa+renesas, linux-mmc, linux-renesas-soc, Avri Altman

On Sat, 24 Nov 2018 at 22:37, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>
> > Tested fine with SDHCI Iproc controller on 4.9.137 and 4.19.2 with
> > Auto-CMD12 enable.
> > But there is something, it's not working not introduce with this patch on 4.14
>
> I didn't get the last sentence. Could you explain in more detail?

Just want to point that RPMB with Iproc controller don't work on 4.14
but it's not due to your patch.

But it's fix the issue on 4.9 and 4.19.

Regards,
Clement

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count
  2018-11-20 23:08 ` [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count Wolfram Sang
  2018-11-22 12:15   ` Avri Altman
@ 2018-11-25 22:12   ` Clément Péron
  1 sibling, 0 replies; 18+ messages in thread
From: Clément Péron @ 2018-11-25 22:12 UTC (permalink / raw)
  To: wsa+renesas; +Cc: linux-mmc, linux-renesas-soc

On Wed, 21 Nov 2018 at 00:10, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> For RPMB, block count is a non-zero 16 bit wide number. Reject invalid
> values from userspace instead of just masking the unneeded bits. Tested
> with a modified 'mmc-utils' package.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Clément Péron <peron.clem@gmail.com>
> ---
>  drivers/mmc/core/block.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index c35b5b08bb33..9e0f7e4aa8c6 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -550,6 +550,9 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>         }
>
>         if (idata->rpmb) {
> +               if (data.blocks > 65535 || !data.blocks)
> +                       return -EINVAL;
> +
>                 err = mmc_set_blockcount(card, data.blocks,
>                         idata->ic.write_flag & (1 << 31));
>                 if (err)
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/3] mmc: core: use mrq->sbc when sending CMD23 for RPMB
  2018-11-20 23:08 ` [RFC PATCH 2/3] mmc: core: use mrq->sbc when sending CMD23 for RPMB Wolfram Sang
@ 2018-11-25 22:13   ` Clément Péron
  0 siblings, 0 replies; 18+ messages in thread
From: Clément Péron @ 2018-11-25 22:13 UTC (permalink / raw)
  To: wsa+renesas; +Cc: linux-mmc, linux-renesas-soc, masaharu.hayakawa.ry

On Wed, 21 Nov 2018 at 00:11, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> When sending out CMD23 in the blk preparation, the comment there
> rightfully says:
>
>          * However, it is not sufficient to just send CMD23,
>          * and avoid the final CMD12, as on an error condition
>          * CMD12 (stop) needs to be sent anyway. This, coupled
>          * with Auto-CMD23 enhancements provided by some
>          * hosts, means that the complexity of dealing
>          * with this is best left to the host. If CMD23 is
>          * supported by card and host, we'll fill sbc in and let
>          * the host deal with handling it correctly.
>
> Let's do this behaviour for RPMB as well, and not send CMD23
> independently. Otherwise IP cores (like Renesas SDHI) may timeout
> because of automatic CMD23/CMD12 handling.
>
> Reported-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Clément Péron <peron.clem@gmail.com>
> ---
>
> I also noticed that write_flag is an int, and we are playing with the sign bit
> on 32-bit systems. But sadly this is exported to userspace...
>
>  drivers/mmc/core/block.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 9e0f7e4aa8c6..66fc425615cd 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -472,7 +472,7 @@ static int ioctl_do_sanitize(struct mmc_card *card)
>  static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>                                struct mmc_blk_ioc_data *idata)
>  {
> -       struct mmc_command cmd = {};
> +       struct mmc_command cmd = {}, sbc = {};
>         struct mmc_data data = {};
>         struct mmc_request mrq = {};
>         struct scatterlist sg;
> @@ -553,10 +553,11 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>                 if (data.blocks > 65535 || !data.blocks)
>                         return -EINVAL;
>
> -               err = mmc_set_blockcount(card, data.blocks,
> -                       idata->ic.write_flag & (1 << 31));
> -               if (err)
> -                       return err;
> +               sbc.opcode = MMC_SET_BLOCK_COUNT;
> +               /* copy the 'Reliable Write' bit */
> +               sbc.arg = data.blocks | (idata->ic.write_flag & BIT(31));
> +               sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +               mrq.sbc = &sbc;
>         }
>
>         if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 3/3] mmc: core: remove obsolete mmc_set_blockcount() function
  2018-11-20 23:08 ` [RFC PATCH 3/3] mmc: core: remove obsolete mmc_set_blockcount() function Wolfram Sang
@ 2018-11-25 22:13   ` Clément Péron
  0 siblings, 0 replies; 18+ messages in thread
From: Clément Péron @ 2018-11-25 22:13 UTC (permalink / raw)
  To: wsa+renesas; +Cc: linux-mmc, linux-renesas-soc

On Wed, 21 Nov 2018 at 00:10, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> The only user was converted to fill a sbc command which is the proper
> way to do it because of AutoCMD23 feature of some hosts.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Clément Péron <peron.clem@gmail.com>
> ---
>  drivers/mmc/core/core.c | 14 --------------
>  drivers/mmc/core/core.h |  2 --
>  2 files changed, 16 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 50a5c340307b..d3085f70e9a4 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2413,20 +2413,6 @@ int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen)
>  }
>  EXPORT_SYMBOL(mmc_set_blocklen);
>
> -int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
> -                       bool is_rel_write)
> -{
> -       struct mmc_command cmd = {};
> -
> -       cmd.opcode = MMC_SET_BLOCK_COUNT;
> -       cmd.arg = blockcount & 0x0000FFFF;
> -       if (is_rel_write)
> -               cmd.arg |= 1 << 31;
> -       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> -       return mmc_wait_for_cmd(card->host, &cmd, 5);
> -}
> -EXPORT_SYMBOL(mmc_set_blockcount);
> -
>  static void mmc_hw_reset_for_init(struct mmc_host *host)
>  {
>         mmc_pwrseq_reset(host);
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 087ba68b2920..8fb6bc37f808 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -118,8 +118,6 @@ int mmc_erase_group_aligned(struct mmc_card *card, unsigned int from,
>  unsigned int mmc_calc_max_discard(struct mmc_card *card);
>
>  int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
> -int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
> -                       bool is_rel_write);
>
>  int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
>                      atomic_t *abort);
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/3] mmc: refactor RPMB block count handling
  2018-11-25 22:11           ` Clément Péron
@ 2018-11-26  9:38             ` Wolfram Sang
  0 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2018-11-26  9:38 UTC (permalink / raw)
  To: Clément Péron
  Cc: Ulf Hansson, wsa+renesas, linux-mmc, linux-renesas-soc, Avri Altman

[-- Attachment #1: Type: text/plain, Size: 131 bytes --]


> Just want to point that RPMB with Iproc controller don't work on 4.14
> but it's not due to your patch.

Thanks, now I got it!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-11-26 20:32 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-20 23:08 [RFC PATCH 0/3] mmc: refactor RPMB block count handling Wolfram Sang
2018-11-20 23:08 ` [RFC PATCH 1/3] mmc: core: validate user input for RPMB block count Wolfram Sang
2018-11-22 12:15   ` Avri Altman
2018-11-22 14:05     ` Wolfram Sang
2018-11-22 15:57       ` Avri Altman
2018-11-22 23:34         ` Wolfram Sang
2018-11-25 22:12   ` Clément Péron
2018-11-20 23:08 ` [RFC PATCH 2/3] mmc: core: use mrq->sbc when sending CMD23 for RPMB Wolfram Sang
2018-11-25 22:13   ` Clément Péron
2018-11-20 23:08 ` [RFC PATCH 3/3] mmc: core: remove obsolete mmc_set_blockcount() function Wolfram Sang
2018-11-25 22:13   ` Clément Péron
2018-11-21  0:31 ` [RFC PATCH 0/3] mmc: refactor RPMB block count handling Ulf Hansson
2018-11-21  2:35   ` Wolfram Sang
2018-11-22 10:35     ` Clément Péron
2018-11-24 14:26       ` Clément Péron
2018-11-24 21:37         ` Wolfram Sang
2018-11-25 22:11           ` Clément Péron
2018-11-26  9:38             ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.