[03/10] mmc: rpmb: add mmc_rpmb_route_frames()

Adds mmc_rpmb_route_frames() to route RPMB data frames from/to an
external entity.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

---
 drivers/mmc/rpmb.c | 160 +++++++++++++++++++++++++++++++++++++++++++++
 include/mmc.h      |   2 +
 2 files changed, 162 insertions(+)

-- 
2.17.1

As I didn't have any available Hikey board, tested this on Poplar:

Tested-by: Igor Opaniuk <igor.opaniuk@linaro.org>

BTW, we've had it up for discussion already, but just to clarify and summarize:
As ID of eMMC is hardcoded in the OP-TEE OS core (CFG_RPMB_FS_DEV_ID),
we will probably have issues on some platforms, where there is a
difference in the probe order of MMC controllers (for example, on
Poplar eMMC is 0 in U-boot, but in Linux it's 1, as SD is enumerated
as 0).
I guess it's unlikely that people will introduce changes to
U-boot/Linux to make this order conform to each other, so instead, we
should let the Normal World-part to decide what eMMC id to use from
these RPMB frames.

Added Joakim and Jerome so they can follow this thread.

Thanks

On 13 August 2018 at 18:53, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> Adds mmc_rpmb_route_frames() to route RPMB data frames from/to an
> external entity.
>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  drivers/mmc/rpmb.c | 160 +++++++++++++++++++++++++++++++++++++++++++++
>  include/mmc.h      |   2 +
>  2 files changed, 162 insertions(+)
>
> diff --git a/drivers/mmc/rpmb.c b/drivers/mmc/rpmb.c
> index dfbdb0deb107..908f19208955 100644
> --- a/drivers/mmc/rpmb.c
> +++ b/drivers/mmc/rpmb.c
> @@ -321,3 +321,163 @@ int mmc_rpmb_write(struct mmc *mmc, void *addr, unsigned short blk,
>         }
>         return i;
>  }
> +
> +static int send_write_mult_block(struct mmc *mmc, const struct s_rpmb *frm,
> +                                unsigned short cnt)
> +{
> +       struct mmc_cmd cmd = {
> +               .cmdidx = MMC_CMD_WRITE_MULTIPLE_BLOCK,
> +               .resp_type = MMC_RSP_R1b,
> +       };
> +       struct mmc_data data = {
> +               .src = (const void *)frm,
> +               .blocks = cnt,
> +               .blocksize = sizeof(*frm),
> +               .flags = MMC_DATA_WRITE,
> +       };
> +
> +       return mmc_send_cmd(mmc, &cmd, &data);
> +}
> +
> +static int send_read_mult_block(struct mmc *mmc, struct s_rpmb *frm,
> +                               unsigned short cnt)
> +{
> +       struct mmc_cmd cmd = {
> +               .cmdidx = MMC_CMD_READ_MULTIPLE_BLOCK,
> +               .resp_type = MMC_RSP_R1,
> +       };
> +       struct mmc_data data = {
> +               .dest = (void *)frm,
> +               .blocks = cnt,
> +               .blocksize = sizeof(*frm),
> +               .flags = MMC_DATA_READ,
> +       };
> +
> +       return mmc_send_cmd(mmc, &cmd, &data);
> +}
> +
> +static int rpmb_route_write_req(struct mmc *mmc, struct s_rpmb *req,
> +                               unsigned short req_cnt, struct s_rpmb *rsp,
> +                               unsigned short rsp_cnt)
> +{
> +       int ret;
> +
> +       /*
> +        * Send the write request.
> +        */
> +       ret = mmc_set_blockcount(mmc, req_cnt, true);
> +       if (ret)
> +               return ret;
> +
> +       ret = send_write_mult_block(mmc, req, req_cnt);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Read the result of the request.
> +        */
> +       ret = mmc_set_blockcount(mmc, 1, false);
> +       if (ret)
> +               return ret;
> +
> +       memset(rsp, 0, sizeof(*rsp));
> +       rsp->request = cpu_to_be16(RPMB_REQ_STATUS);
> +       ret = send_write_mult_block(mmc, rsp, 1);
> +       if (ret)
> +               return ret;
> +
> +       ret = mmc_set_blockcount(mmc, 1, false);
> +       if (ret)
> +               return ret;
> +
> +       return send_read_mult_block(mmc, rsp, 1);
> +}
> +
> +static int rpmb_route_read_req(struct mmc *mmc, struct s_rpmb *req,
> +                              unsigned short req_cnt, struct s_rpmb *rsp,
> +                              unsigned short rsp_cnt)
> +{
> +       int ret;
> +
> +       /*
> +        * Send the read request.
> +        */
> +       ret = mmc_set_blockcount(mmc, 1, false);
> +       if (ret)
> +               return ret;
> +
> +       ret = send_write_mult_block(mmc, req, 1);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Read the result of the request.
> +        */
> +
> +       ret = mmc_set_blockcount(mmc, rsp_cnt, false);
> +       if (ret)
> +               return ret;
> +
> +       return send_read_mult_block(mmc, rsp, rsp_cnt);
> +}
> +
> +static int rpmb_route_frames(struct mmc *mmc, struct s_rpmb *req,
> +                            unsigned short req_cnt, struct s_rpmb *rsp,
> +                            unsigned short rsp_cnt)
> +{
> +       unsigned short n;
> +
> +       /*
> +        * If multiple request frames are provided, make sure that all are
> +        * of the same type.
> +        */
> +       for (n = 1; n < req_cnt; n++)
> +               if (req[n].request != req->request)
> +                       return -EINVAL;
> +
> +       switch (be16_to_cpu(req->request)) {
> +       case RPMB_REQ_KEY:
> +               if (req_cnt != 1 || rsp_cnt != 1)
> +                       return -EINVAL;
> +               return rpmb_route_write_req(mmc, req, req_cnt, rsp, rsp_cnt);
> +
> +       case RPMB_REQ_WRITE_DATA:
> +               if (!req_cnt || rsp_cnt != 1)
> +                       return -EINVAL;
> +               return rpmb_route_write_req(mmc, req, req_cnt, rsp, rsp_cnt);
> +
> +       case RPMB_REQ_WCOUNTER:
> +               if (req_cnt != 1 || rsp_cnt != 1)
> +                       return -EINVAL;
> +               return rpmb_route_read_req(mmc, req, req_cnt, rsp, rsp_cnt);
> +
> +       case RPMB_REQ_READ_DATA:
> +               if (req_cnt != 1 || !req_cnt)
> +                       return -EINVAL;
> +               return rpmb_route_read_req(mmc, req, req_cnt, rsp, rsp_cnt);
> +
> +       default:
> +               debug("Unsupported message type: %d\n",
> +                     be16_to_cpu(req->request));
> +               return -EINVAL;
> +       }
> +}
> +
> +int mmc_rpmb_route_frames(struct mmc *mmc, void *req, unsigned long reqlen,
> +                         void *rsp, unsigned long rsplen)
> +{
> +       /*
> +        * Whoever crafted the data supplied to this function knows how to
> +        * format the PRMB frames and which response is expected. If
> +        * there's some unexpected mismatch it's more helpful to report an
> +        * error immediately than trying to guess what was the intention
> +        * and possibly just delay an eventual error which will be harder
> +        * to track down.
> +        */
> +
> +       if (reqlen % sizeof(struct s_rpmb) || rsplen % sizeof(struct s_rpmb))
> +               return -EINVAL;
> +
> +       return rpmb_route_frames(mmc, req, reqlen / sizeof(struct s_rpmb),
> +                                rsp, rsplen / sizeof(struct s_rpmb));
> +}
> diff --git a/include/mmc.h b/include/mmc.h
> index df4255b828a7..d6e02af4edea 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -748,6 +748,8 @@ int mmc_rpmb_read(struct mmc *mmc, void *addr, unsigned short blk,
>                   unsigned short cnt, unsigned char *key);
>  int mmc_rpmb_write(struct mmc *mmc, void *addr, unsigned short blk,
>                    unsigned short cnt, unsigned char *key);
> +int mmc_rpmb_route_frames(struct mmc *mmc, void *req, unsigned long reqlen,
> +                         void *rsp, unsigned long rsplen);
>  #ifdef CONFIG_CMD_BKOPS_ENABLE
>  int mmc_set_bkops_enable(struct mmc *mmc);
>  #endif
> --
> 2.17.1
>

Hi Igor,

On Thu, Aug 16, 2018 at 2:13 PM, Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
> As I didn't have any available Hikey board, tested this on Poplar:
>
> Tested-by: Igor Opaniuk <igor.opaniuk@linaro.org>
>
> BTW, we've had it up for discussion already, but just to clarify and summarize:
> As ID of eMMC is hardcoded in the OP-TEE OS core (CFG_RPMB_FS_DEV_ID),
> we will probably have issues on some platforms, where there is a
> difference in the probe order of MMC controllers (for example, on
> Poplar eMMC is 0 in U-boot, but in Linux it's 1, as SD is enumerated
> as 0).
> I guess it's unlikely that people will introduce changes to
> U-boot/Linux to make this order conform to each other, so instead, we
> should let the Normal World-part to decide what eMMC id to use from
> these RPMB frames.

Both U-boot and Linux are part of Normal World. I guess you're
suggesting to move the logic of selecting RPMB device based on the ID
from Secure World to tee-supplicant. For Linux that's a user space
daemon and in U-boot that's part of the OP-TEE driver. I think it
would be unfortunate to have this logic in user space, upgrades can
make a mess of this.

This is in one aspect a board specific problem which can be addressed
by defining the sequence number (what's indicated by
CFG_RPMB_FS_DEV_ID above) of relevant RPMB partitions on a specific
board. This is what we have been relying on indirectly so far.

Another way to deal with this problem is to let Secure World probe all
available RPMB partitions and use the one using the expected shared
secret for MACing. A drawback is that it's more complicated in Secure
World and will take a while before it's implemented.

Thanks,
Jens