diff mbox series

RPMB operation with auto-cmd12 host controller

Message ID CAJiuCcf83f0BsUfc1qU_g1zcJ8iQ8DPZ6BT0mQgp6BKJM=ArFQ@mail.gmail.com
State New
Headers show
Series RPMB operation with auto-cmd12 host controller | expand

Commit Message

Clément Péron Oct. 17, 2018, 4:18 p.m. UTC
Hi,

I'm using the sdhci-iproc host controller on my board (running linux-4.9).
When I try to access to the RPMB partition I get the following message.

mmc0: Timeout waiting for hardware interrupt.
sdhci: =========== REGISTER DUMP (mmc0)===========
sdhci: Sys addr: 0x7e820000 | Version:  0x0000ac02
sdhci: Blk size: 0x00007200 | Blk cnt:  0x00000000
sdhci: Argument: 0x00000000 | Trn mode: 0x00000027
sdhci: Present:  0x01f70002 | Host ctl: 0x00000017
sdhci: Power:    0x0000000f | Blk gap:  0x00000000
sdhci: Wake-up:  0x00000000 | Clock:    0x00000207
sdhci: Timeout:  0x0000000e | Int stat: 0x00000000
sdhci: Int enab: 0x02ff008b | Sig enab: 0x02ff008b
sdhci: AC12 err: 0x00000002 | Slot int: 0x00000000
sdhci: Caps:     0x05e80000 | Caps_1:   0x03002075
sdhci: Cmd:      0x0000193a | Max curr: 0x00000001
sdhci: Host ctl2: 0x0000800c
sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x7f30c210
sdhci: ===========================================
sdhci-iproc 18041000.sdhci: __mmc_blk_ioctl_cmd: data error -110
RPMB ioctl failed: Connection timed out

The controller has the auto-cmd12 feature wich make the RPMB cmd incorrect.
Because the cmd23 is sent separately and an incorrect CMD12 is added
during the operation (from what I understand).

A dirty patch to make it works could be (on top of 4.9) :

        struct scatterlist sg;
@@ -530,10 +531,12 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
*card, struct mmc_blk_data *md,
        }

        if (is_rpmb) {
-               err = mmc_set_blockcount(card, data.blocks,
-                       idata->ic.write_flag & (1 << 31));
-               if (err)
-                       return err;
+               sbc.opcode = MMC_SET_BLOCK_COUNT;
+               sbc.arg = data.blocks & 0x0000FFFF;
+               if (idata->ic.write_flag & (1 << 31))
+                       sbc.arg |= 1 << 31;
+               sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+               mrq.sbc = &sbc;
        }

        if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&

But this patch will break other controller. Do you know a proper way
to fix this issue ?

Thanks,
Clement

Comments

Avri Altman Oct. 17, 2018, 7:59 p.m. UTC | #1
>I'm using the sdhci-iproc host controller on my board (running linux-4.9).
>When I try to access to the RPMB partition I get the following message.
Is this happening while you testing your multi-ioctl code?
Or are you using single ioctl?

Thanks,
Avri
Clément Péron Oct. 18, 2018, 8:57 a.m. UTC | #2
Hi Avri,

On Wed, 17 Oct 2018 at 22:00, Avri Altman <Avri.Altman@wdc.com> wrote:
>
>
> >I'm using the sdhci-iproc host controller on my board (running linux-4.9).
> >When I try to access to the RPMB partition I get the following message.
> Is this happening while you testing your multi-ioctl code?
> Or are you using single ioctl?

Yes I use a custom mmc-utils using MMC_IOC_MULTI_CMD ioctl.

This works fine with another board and a SDHCI controller that didn't
have the auto-cmd12 feature.

Thanks,
Clement

>
> Thanks,
> Avri
Clément Péron Nov. 6, 2018, 3:13 p.m. UTC | #3
Hi,

On Thu, 18 Oct 2018 at 10:57, Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi Avri,
>
> On Wed, 17 Oct 2018 at 22:00, Avri Altman <Avri.Altman@wdc.com> wrote:
> >
> >
> > >I'm using the sdhci-iproc host controller on my board (running linux-4.9).
> > >When I try to access to the RPMB partition I get the following message.
> > Is this happening while you testing your multi-ioctl code?
> > Or are you using single ioctl?
>
> Yes I use a custom mmc-utils using MMC_IOC_MULTI_CMD ioctl.
>
> This works fine with another board and a SDHCI controller that didn't
> have the auto-cmd12 feature.

Can confirm this i have disable the ACMD12 feature in the SDHCI controller.
And disable this CAP in the DT and RPMB are fine :).

Not sure how to fix it properly however :(

Regards,
Clement

>
> Thanks,
> Clement
>
> >
> > Thanks,
> > Avri
Ulf Hansson Nov. 19, 2018, 3 p.m. UTC | #4
On 6 November 2018 at 16:13, Clément Péron <peron.clem@gmail.com> wrote:
> Hi,
>
> On Thu, 18 Oct 2018 at 10:57, Clément Péron <peron.clem@gmail.com> wrote:
>>
>> Hi Avri,
>>
>> On Wed, 17 Oct 2018 at 22:00, Avri Altman <Avri.Altman@wdc.com> wrote:
>> >
>> >
>> > >I'm using the sdhci-iproc host controller on my board (running linux-4.9).
>> > >When I try to access to the RPMB partition I get the following message.
>> > Is this happening while you testing your multi-ioctl code?
>> > Or are you using single ioctl?
>>
>> Yes I use a custom mmc-utils using MMC_IOC_MULTI_CMD ioctl.
>>
>> This works fine with another board and a SDHCI controller that didn't
>> have the auto-cmd12 feature.
>
> Can confirm this i have disable the ACMD12 feature in the SDHCI controller.
> And disable this CAP in the DT and RPMB are fine :).
>
> Not sure how to fix it properly however :(

The RPMB partitions are registered only for controllers supporting
CMD23, which is indicated via MMC_CAP_CMD23. See mmc_decode_ext_csd().

In other words, the patch looks good to me!

Would you mind re-posting a proper patch, with updated changelog, a
good commit message header and add your sob tag.

This should be added tagged for stable as well, however I will look
into that once you re-posted a new version.

Kind regards
Ulf Hansson
Clément Péron Nov. 19, 2018, 5:34 p.m. UTC | #5
Hi Ulf,

On Mon, 19 Nov 2018 at 16:01, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On 6 November 2018 at 16:13, Clément Péron <peron.clem@gmail.com> wrote:
> > Hi,
> >
> > On Thu, 18 Oct 2018 at 10:57, Clément Péron <peron.clem@gmail.com> wrote:
> >>
> >> Hi Avri,
> >>
> >> On Wed, 17 Oct 2018 at 22:00, Avri Altman <Avri.Altman@wdc.com> wrote:
> >> >
> >> >
> >> > >I'm using the sdhci-iproc host controller on my board (running linux-4.9).
> >> > >When I try to access to the RPMB partition I get the following message.
> >> > Is this happening while you testing your multi-ioctl code?
> >> > Or are you using single ioctl?
> >>
> >> Yes I use a custom mmc-utils using MMC_IOC_MULTI_CMD ioctl.
> >>
> >> This works fine with another board and a SDHCI controller that didn't
> >> have the auto-cmd12 feature.
> >
> > Can confirm this i have disable the ACMD12 feature in the SDHCI controller.
> > And disable this CAP in the DT and RPMB are fine :).
> >
> > Not sure how to fix it properly however :(
>
> The RPMB partitions are registered only for controllers supporting
> CMD23, which is indicated via MMC_CAP_CMD23. See mmc_decode_ext_csd().

Yes you are right, and I check my kernel and I forget but I revert
this check in my kernel long time ago.
d0123ccac55088811bde4f76c4a3fdbd39c3cfba

The comment of the commit says "SET_BLOCK_COUNT CMD23 is needed for
all access to RPMB partition."

In the revert I wrote the comment saying : "regarding JEDEC doc, CMD23
isn't mandatory to talk with RPMB partition (only more reliable)".
Not sure if it's still true but my other cards running a SoCFPGA
CycloneV using DesignWare Platform drivers doesn't support CMD23 but
are dealing fine with RPMB.

>
> In other words, the patch looks good to me!
>
> Would you mind re-posting a proper patch, with updated changelog, a
> good commit message header and add your sob tag.

I also remember that I tested this fix with an i.MX board using i.MX
ESDHC controller which I think support the CMD23 but after this fix
the RPMB are no more usable...

Regards,
Clement

>
> This should be added tagged for stable as well, however I will look
> into that once you re-posted a new version.
>
> Kind regards
> Ulf Hansson
Ulf Hansson Nov. 19, 2018, 5:43 p.m. UTC | #6
On 19 November 2018 at 18:34, Clément Péron <peron.clem@gmail.com> wrote:
> Hi Ulf,
>
> On Mon, 19 Nov 2018 at 16:01, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On 6 November 2018 at 16:13, Clément Péron <peron.clem@gmail.com> wrote:
>> > Hi,
>> >
>> > On Thu, 18 Oct 2018 at 10:57, Clément Péron <peron.clem@gmail.com> wrote:
>> >>
>> >> Hi Avri,
>> >>
>> >> On Wed, 17 Oct 2018 at 22:00, Avri Altman <Avri.Altman@wdc.com> wrote:
>> >> >
>> >> >
>> >> > >I'm using the sdhci-iproc host controller on my board (running linux-4.9).
>> >> > >When I try to access to the RPMB partition I get the following message.
>> >> > Is this happening while you testing your multi-ioctl code?
>> >> > Or are you using single ioctl?
>> >>
>> >> Yes I use a custom mmc-utils using MMC_IOC_MULTI_CMD ioctl.
>> >>
>> >> This works fine with another board and a SDHCI controller that didn't
>> >> have the auto-cmd12 feature.
>> >
>> > Can confirm this i have disable the ACMD12 feature in the SDHCI controller.
>> > And disable this CAP in the DT and RPMB are fine :).
>> >
>> > Not sure how to fix it properly however :(
>>
>> The RPMB partitions are registered only for controllers supporting
>> CMD23, which is indicated via MMC_CAP_CMD23. See mmc_decode_ext_csd().
>
> Yes you are right, and I check my kernel and I forget but I revert
> this check in my kernel long time ago.
> d0123ccac55088811bde4f76c4a3fdbd39c3cfba
>
> The comment of the commit says "SET_BLOCK_COUNT CMD23 is needed for
> all access to RPMB partition."
>
> In the revert I wrote the comment saying : "regarding JEDEC doc, CMD23
> isn't mandatory to talk with RPMB partition (only more reliable)".
> Not sure if it's still true but my other cards running a SoCFPGA
> CycloneV using DesignWare Platform drivers doesn't support CMD23 but
> are dealing fine with RPMB.

Well, it works probably because the mmc core manually sends the CMD23
command. It shouldn't do that, simply because that isn't how we
designed the interface towards the mmc host drivers in regards to
this.

Another reason to why it could work, is maybe because of not using
"multi" requests, so then the host driver may just be "lucky".

>
>>
>> In other words, the patch looks good to me!
>>
>> Would you mind re-posting a proper patch, with updated changelog, a
>> good commit message header and add your sob tag.
>
> I also remember that I tested this fix with an i.MX board using i.MX
> ESDHC controller which I think support the CMD23 but after this fix
> the RPMB are no more usable...

Well, in such case there is likely a bug in the mmc host driver, which
we need to fix.

In either case, I think we should move forward with your suggested
patch. If any additional things are needed on top, then let's fix them
as well.

[...]

Kind regards
Uffe
Avri Altman Nov. 20, 2018, 6:48 a.m. UTC | #7
> Well, in such case there is likely a bug in the mmc host driver, which
> we need to fix.
> 
> In either case, I think we should move forward with your suggested
> patch. If any additional things are needed on top, then let's fix them
> as well.
I don't think that this could no longer happen on modern kernels (was observed on V4.9),
Because mmc_blk_alloc_rpmb_part will not be called for the reasons you specified,
So there is no mmcblk0rpmb, and idata->rpmb is null for all other devices.

Thanks,
Avri
Clément Péron Nov. 20, 2018, 2:21 p.m. UTC | #8
Hi Avri,

With On Tue, 20 Nov 2018 at 07:48, Avri Altman <Avri.Altman@wdc.com> wrote:
>
> > Well, in such case there is likely a bug in the mmc host driver, which
> > we need to fix.
> >
> > In either case, I think we should move forward with your suggested
> > patch. If any additional things are needed on top, then let's fix them
> > as well.
> I don't think that this could no longer happen on modern kernels (was observed on V4.9),
> Because mmc_blk_alloc_rpmb_part will not be called for the reasons you specified,
> So there is no mmcblk0rpmb, and idata->rpmb is null for all other devices.

I would like to confirm this and did some test on 4.14.81 with a
sdhci-iproc controller / MMC_MULTI_CMD in mmc_utils.

But the read-counter doesn't seems to work :
in 4.14 :
# uname -a
Linux 192.168.1.219 4.14.81 #1 SMP PREEMPT Tue Nov 20 15:14:36 CET
2018 armv7l GNU/Linux
# date
Tue Nov 20 14:19:28 UTC 2018
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x7085de01
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x70f56201
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x70a59301
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x70752301

whereas in 4.9:
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x000001d0
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x000001d0
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x000001d0
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x000001d0
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x000001d0
# mmc rpmb read-counter /dev/mmcblk0rpmb
Counter value: 0x000001d0

>
> Thanks,
> Avri
diff mbox series

Patch

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c411f7a68641..d0d1fd9cfad5 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -464,6 +464,7 @@  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 = {0};
+       struct mmc_command sbc = {0};
        struct mmc_data data = {0};
        struct mmc_request mrq = {NULL};