All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: "Schaefer, Brandon" <brandon.schaefer@flukenetworks.com>
Cc: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"Bean Huo (beanhuo)" <beanhuo@micron.com>,
	Shawn Lin <shawn.lin@rock-chips.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: Enabling RPMB overwrites setting the boot partition via ioctl
Date: Tue, 23 May 2017 12:17:40 +0200	[thread overview]
Message-ID: <CAPDyKFq3tgYgWcO3w_7_1-3CgDWhKq2RWqKygcgBk2CfR8tsHA@mail.gmail.com> (raw)
In-Reply-To: <BN6PR12MB1442FDD4A02698E52489183490E50@BN6PR12MB1442.namprd12.prod.outlook.com>

+ Bean Huo, Shawn Lin, Bartlomiej Zolnierkiewicz,

On 20 May 2017 at 00:03, Schaefer, Brandon
<brandon.schaefer@flukenetworks.com> wrote:
> Linux kernel: 4.1.15 (imx_4.1.15_1.0.0_ga branch)
>
> I've noticed that writing to the boot partition is overwritten by RPMB related functions as it stores this byte (card->ext_csd.part_conf). It is necessary to update this reference when issuing CMD6 SWITCH through ioctl. See below patch for above kernel version:

Thanks for sharing the information!

>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index be776fa..cbd0533 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -60,7 +60,9 @@ MODULE_ALIAS("mmc:block");
> #define INAND_CMD38_ARG_SECTRIM2 0x88
> #define MMC_BLK_TIMEOUT_MS  (10 * 60 * 1000)        /* 10 minute timeout */
> #define MMC_SANITIZE_REQ_TIMEOUT 240000
> -#define MMC_EXTRACT_INDEX_FROM_ARG(x) ((x & 0x00FF0000) >> 16)
> +#define MMC_EXTRACT_SWITCH_MODE_FROM_ARG(x)  (((x) & 0x03000000) >> 24)
> +#define MMC_EXTRACT_INDEX_FROM_ARG(x)        (((x) & 0x00FF0000) >> 16)
> +#define MMC_EXTRACT_VALUE_FROM_ARG(x)        (((x) & 0x0000FF00) >> 8)
>
>  #define mmc_req_rel_wr(req)   (((req->cmd_flags & REQ_FUA) || \
>                                 (req->cmd_flags & REQ_META)) && \
> @@ -572,6 +574,24 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>               goto cmd_rel_host;
>        }
>
> +       /* If writing part config register, update struct */
> +       if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_PART_CONFIG) &&
> +           (cmd.opcode == MMC_SWITCH)) {
> +              u8 mode = MMC_EXTRACT_SWITCH_MODE_FROM_ARG(cmd.arg);
> +              u8 value = MMC_EXTRACT_VALUE_FROM_ARG(cmd.arg);
> +              switch (mode) {
> +              case MMC_SWITCH_MODE_SET_BITS:
> +                      card->ext_csd.part_config |= value;
> +                      break;
> +              case MMC_SWITCH_MODE_CLEAR_BITS:
> +                      card->ext_csd.part_config &= ~value;
> +                      break;
> +              case MMC_SWITCH_MODE_WRITE_BYTE:
> +                      card->ext_csd.part_config = value;
> +                      break;
> +              }
> +       }
> +
>        /*
>         * According to the SD specs, some commands require a delay after
>         * issuing the command.

I am not sure how to go with this, especially since you are referring
to an old kernel, 4.1.15. Can you confirm that the problem is still
present in the my next branch of my mmc git tree? That would be more
relevant information to start with.

So to move forward, I decide to investigate the problem a bit. Here's
my summary:

1) There are problems regarding switching between partitions in
general, because the (e)MMC partitions isn't correctly set up.
2) The are problems related to when userspace utilizes the mmc
back-door (mmc-ioctl) to change ext csd, because the mmc core don't
get informed about these updates. For example, the mmc core may then
take wrong decisions in the suspend/resume sequence.

It would really be nice to fix these problems, however I need good
quality patches, based on my mmc next branch confirming to fix
problems.

Here are some interesting samples from my investigation.
https://patchwork.kernel.org/patch/9632365/
https://community.nxp.com/thread/340088
http://www.spinics.net/lists/linux-mmc/msg13516.html


And here is an interesting commit, which really puzzles me as issuing
a CMD0 should always re-set the eMMC device to the user partition. I
don't get why this is needed and nor how it can fix a problem.

commit 41e2a4893566ced3c46af15df5b727326881e47d
Author: Philip Rakity <prakity@marvell.com>
Date:   Sat Mar 19 14:10:33 2011 -0400

    mmc: Ensure linux starts in eMMC user partition

    uBoot sometimes leaves eMMC pointing to the private boot partition.
    Ensure we always start looking at the user partition.


Kind regards
Uffe

  reply	other threads:[~2017-05-23 10:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-19 22:03 Enabling RPMB overwrites setting the boot partition via ioctl Schaefer, Brandon
2017-05-23 10:17 ` Ulf Hansson [this message]
2017-05-24  0:33   ` Shawn Lin
2017-05-24 19:03   ` Schaefer, Brandon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPDyKFq3tgYgWcO3w_7_1-3CgDWhKq2RWqKygcgBk2CfR8tsHA@mail.gmail.com \
    --to=ulf.hansson@linaro.org \
    --cc=b.zolnierkie@samsung.com \
    --cc=beanhuo@micron.com \
    --cc=brandon.schaefer@flukenetworks.com \
    --cc=linux-mmc@vger.kernel.org \
    --cc=shawn.lin@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.