| Message ID | 20181018180549.20868-1-peron.clem@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] mmc-utils: RPMB fails with status 0x0001 on system eMMC chips | expand |
Hi, > Use MMC_IOC_MULTI_CMD for RPMB access Actually, this is much more appropriate title for this patch > > On some systems which use MMC as a main storage device > it is possible that RPMB commands are mixed with > generic MMC access commands which invalidates RPMB. > This patch uses MMC_IOC_MULTI_CMD. > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > --- > > v3: > move set_single_cmd in mmc_cmds.c > add MMC_IOC_MULTI_CMD check > set blocks in set_single_cmd > fix frame_ptr used > > mmc_cmds.c | 98 +++++++++++++++++++++++++++++------------------------- > 1 file changed, 53 insertions(+), 45 deletions(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index 44623fe..c8bfc09 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -1825,6 +1825,8 @@ int do_sanitize(int nargs, char **argv) > ret; > \ > }) > > +#define RMPB_MULTI_CMD_MAX_CMDS 3 > + > enum rpmb_op_type { > MMC_RPMB_WRITE_KEY = 0x01, > MMC_RPMB_READ_CNT = 0x02, > @@ -1847,6 +1849,17 @@ struct rpmb_frame { > u_int16_t req_resp; > }; > > +static inline void set_single_cmd(struct mmc_ioc_cmd *ioc, __u32 opcode, > + int write_flag) > +{ > + ioc->opcode = opcode; > + ioc->write_flag = write_flag; > + ioc->arg = 0x0; > + ioc->blksz = 512; > + ioc->blocks = 1; blocks Should be an input > + ioc->flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; > +} > + > /* Performs RPMB operation. > * > * @fd: RPMB device on which we should perform ioctl command > @@ -1861,22 +1874,27 @@ static int do_rpmb_op(int fd, > struct rpmb_frame *frame_out, > unsigned int out_cnt) > { > +#ifndef MMC_IOC_MULTI_CMD > + fprintf(stderr, "mmc-utils has been compiled without > MMC_IOC_MULTI_CMD" > + " support, needed by RPMB operation.\n"); > + exit(1); > +#else > int err; > u_int16_t rpmb_type; > - > - struct mmc_ioc_cmd ioc = { > - .arg = 0x0, > - .blksz = 512, > - .blocks = 1, > - .write_flag = 1, > - .opcode = MMC_WRITE_MULTIPLE_BLOCK, > - .flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | > MMC_CMD_ADTC, > - .data_ptr = (uintptr_t)frame_in > - }; > + struct mmc_ioc_multi_cmd *mioc; > + struct mmc_ioc_cmd *ioc; > > if (!frame_in || !frame_out || !out_cnt) > return -EINVAL; > > + /* prepare arguments for MMC_IOC_MUTLI_CMD ioctl */ > + mioc = (struct mmc_ioc_multi_cmd *) > + malloc(sizeof (struct mmc_ioc_multi_cmd) + > + RMPB_MULTI_CMD_MAX_CMDS * sizeof (struct > mmc_ioc_cmd)); > + if (!mioc) { > + return -ENOMEM; > + } > + > rpmb_type = be16toh(frame_in->req_resp); > > switch(rpmb_type) { > @@ -1887,33 +1905,24 @@ static int do_rpmb_op(int fd, > goto out; > } > > + mioc->num_of_cmds = 3; > + > /* Write request */ > - ioc.write_flag |= (1<<31); > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > - if (err < 0) { > - err = -errno; > - goto out; > - } > + ioc = &mioc->cmds[0]; > + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, (1 << 31) > | 1); > + mmc_ioc_cmd_set_data((*ioc), frame_in); > > /* Result request */ > + ioc = &mioc->cmds[1]; > memset(frame_out, 0, sizeof(*frame_out)); You need to save frame_out to carry the response - use a fresh frame here, e.g.: u_int8_t get_status_command[512] ={0}; > frame_out->req_resp = htobe16(MMC_RPMB_READ_RESP); get_status_command[511] = MMC_RPMB_READ_RESP; > - ioc.write_flag = 1; > - ioc.data_ptr = (uintptr_t)frame_out; > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > - if (err < 0) { > - err = -errno; > - goto out; > - } > + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, 1); > + mmc_ioc_cmd_set_data((*ioc), frame_out); mmc_ioc_cmd_set_data((*ioc), get_status_command); > > /* Get response */ > - ioc.write_flag = 0; > - ioc.opcode = MMC_READ_MULTIPLE_BLOCK; > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > - if (err < 0) { > - err = -errno; > - goto out; > - } > + ioc = &mioc->cmds[2]; > + set_single_cmd(ioc, MMC_READ_MULTIPLE_BLOCK, 0); > + mmc_ioc_cmd_set_data((*ioc), frame_in); mmc_ioc_cmd_set_data((*ioc), frame_out); > > break; > case MMC_RPMB_READ_CNT: > @@ -1924,23 +1933,18 @@ static int do_rpmb_op(int fd, > /* fall through */ > > case MMC_RPMB_READ: > - /* Request */ > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > - if (err < 0) { > - err = -errno; > - goto out; > - } > + mioc->num_of_cmds = 2; > + > + /* Read request */ > + ioc = &mioc->cmds[0]; > + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, 1); > + mmc_ioc_cmd_set_data((*ioc), frame_in); > > /* Get response */ > - ioc.write_flag = 0; > - ioc.opcode = MMC_READ_MULTIPLE_BLOCK; > - ioc.blocks = out_cnt; > - ioc.data_ptr = (uintptr_t)frame_out; > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > - if (err < 0) { > - err = -errno; > - goto out; > - } > + ioc = &mioc->cmds[1]; > + set_single_cmd(ioc, MMC_READ_MULTIPLE_BLOCK, 0); > + ioc->blocks = out_cnt; Not needed if out_cnt is an input to set_single_cmd > + mmc_ioc_cmd_set_data((*ioc), frame_out); > > break; > default: > @@ -1948,8 +1952,12 @@ static int do_rpmb_op(int fd, > goto out; > } > > + err = ioctl(fd, MMC_IOC_MULTI_CMD, mioc); > + > out: > + free(mioc); > return err; > +#endif /* !MMC_IOC_MULTI_CMD */ > } > > int do_rpmb_write_key(int nargs, char **argv) > -- > 2.17.1
Hi Avri, On Fri, 19 Oct 2018 at 13:41, Avri Altman <Avri.Altman@wdc.com> wrote: > > Hi, > > > Use MMC_IOC_MULTI_CMD for RPMB access > Actually, this is much more appropriate title for this patch > > > > > On some systems which use MMC as a main storage device > > it is possible that RPMB commands are mixed with > > generic MMC access commands which invalidates RPMB. > > This patch uses MMC_IOC_MULTI_CMD. > > > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > --- > > > > v3: > > move set_single_cmd in mmc_cmds.c > > add MMC_IOC_MULTI_CMD check > > set blocks in set_single_cmd > > fix frame_ptr used > > > > mmc_cmds.c | 98 +++++++++++++++++++++++++++++------------------------- > > 1 file changed, 53 insertions(+), 45 deletions(-) > > > > diff --git a/mmc_cmds.c b/mmc_cmds.c > > index 44623fe..c8bfc09 100644 > > --- a/mmc_cmds.c > > +++ b/mmc_cmds.c > > @@ -1825,6 +1825,8 @@ int do_sanitize(int nargs, char **argv) > > ret; > > \ > > }) > > > > +#define RMPB_MULTI_CMD_MAX_CMDS 3 > > + > > enum rpmb_op_type { > > MMC_RPMB_WRITE_KEY = 0x01, > > MMC_RPMB_READ_CNT = 0x02, > > @@ -1847,6 +1849,17 @@ struct rpmb_frame { > > u_int16_t req_resp; > > }; > > > > +static inline void set_single_cmd(struct mmc_ioc_cmd *ioc, __u32 opcode, > > + int write_flag) > > +{ > > + ioc->opcode = opcode; > > + ioc->write_flag = write_flag; > > + ioc->arg = 0x0; > > + ioc->blksz = 512; > > + ioc->blocks = 1; > blocks Should be an input > > > + ioc->flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; > > +} > > + > > /* Performs RPMB operation. > > * > > * @fd: RPMB device on which we should perform ioctl command > > @@ -1861,22 +1874,27 @@ static int do_rpmb_op(int fd, > > struct rpmb_frame *frame_out, > > unsigned int out_cnt) > > { > > +#ifndef MMC_IOC_MULTI_CMD > > + fprintf(stderr, "mmc-utils has been compiled without > > MMC_IOC_MULTI_CMD" > > + " support, needed by RPMB operation.\n"); > > + exit(1); > > +#else > > int err; > > u_int16_t rpmb_type; > > - > > - struct mmc_ioc_cmd ioc = { > > - .arg = 0x0, > > - .blksz = 512, > > - .blocks = 1, > > - .write_flag = 1, > > - .opcode = MMC_WRITE_MULTIPLE_BLOCK, > > - .flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | > > MMC_CMD_ADTC, > > - .data_ptr = (uintptr_t)frame_in > > - }; > > + struct mmc_ioc_multi_cmd *mioc; > > + struct mmc_ioc_cmd *ioc; > > > > if (!frame_in || !frame_out || !out_cnt) > > return -EINVAL; > > > > + /* prepare arguments for MMC_IOC_MUTLI_CMD ioctl */ > > + mioc = (struct mmc_ioc_multi_cmd *) > > + malloc(sizeof (struct mmc_ioc_multi_cmd) + > > + RMPB_MULTI_CMD_MAX_CMDS * sizeof (struct > > mmc_ioc_cmd)); > > + if (!mioc) { > > + return -ENOMEM; > > + } > > + > > rpmb_type = be16toh(frame_in->req_resp); > > > > switch(rpmb_type) { > > @@ -1887,33 +1905,24 @@ static int do_rpmb_op(int fd, > > goto out; > > } > > > > + mioc->num_of_cmds = 3; > > + > > /* Write request */ > > - ioc.write_flag |= (1<<31); > > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > > - if (err < 0) { > > - err = -errno; > > - goto out; > > - } > > + ioc = &mioc->cmds[0]; > > + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, (1 << 31) > > | 1); > > + mmc_ioc_cmd_set_data((*ioc), frame_in); > > > > /* Result request */ > > + ioc = &mioc->cmds[1]; > > memset(frame_out, 0, sizeof(*frame_out)); > You need to save frame_out to carry the response - use a fresh frame here, e.g.: > u_int8_t get_status_command[512] ={0}; Thanks for the explanation, I prefer to declare a new rpmb_frame. Will send a new patch. Thanks for your review, Clement > > > frame_out->req_resp = htobe16(MMC_RPMB_READ_RESP); > get_status_command[511] = MMC_RPMB_READ_RESP; > > > - ioc.write_flag = 1; > > - ioc.data_ptr = (uintptr_t)frame_out; > > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > > - if (err < 0) { > > - err = -errno; > > - goto out; > > - } > > + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, 1); > > + mmc_ioc_cmd_set_data((*ioc), frame_out); > mmc_ioc_cmd_set_data((*ioc), get_status_command); > > > > > /* Get response */ > > - ioc.write_flag = 0; > > - ioc.opcode = MMC_READ_MULTIPLE_BLOCK; > > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > > - if (err < 0) { > > - err = -errno; > > - goto out; > > - } > > + ioc = &mioc->cmds[2]; > > + set_single_cmd(ioc, MMC_READ_MULTIPLE_BLOCK, 0); > > + mmc_ioc_cmd_set_data((*ioc), frame_in); > mmc_ioc_cmd_set_data((*ioc), frame_out); > > > > > break; > > case MMC_RPMB_READ_CNT: > > @@ -1924,23 +1933,18 @@ static int do_rpmb_op(int fd, > > /* fall through */ > > > > case MMC_RPMB_READ: > > - /* Request */ > > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > > - if (err < 0) { > > - err = -errno; > > - goto out; > > - } > > + mioc->num_of_cmds = 2; > > + > > + /* Read request */ > > + ioc = &mioc->cmds[0]; > > + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, 1); > > + mmc_ioc_cmd_set_data((*ioc), frame_in); > > > > /* Get response */ > > - ioc.write_flag = 0; > > - ioc.opcode = MMC_READ_MULTIPLE_BLOCK; > > - ioc.blocks = out_cnt; > > - ioc.data_ptr = (uintptr_t)frame_out; > > - err = ioctl(fd, MMC_IOC_CMD, &ioc); > > - if (err < 0) { > > - err = -errno; > > - goto out; > > - } > > + ioc = &mioc->cmds[1]; > > + set_single_cmd(ioc, MMC_READ_MULTIPLE_BLOCK, 0); > > + ioc->blocks = out_cnt; > Not needed if out_cnt is an input to set_single_cmd > > > + mmc_ioc_cmd_set_data((*ioc), frame_out); > > > > break; > > default: > > @@ -1948,8 +1952,12 @@ static int do_rpmb_op(int fd, > > goto out; > > } > > > > + err = ioctl(fd, MMC_IOC_MULTI_CMD, mioc); > > + > > out: > > + free(mioc); > > return err; > > +#endif /* !MMC_IOC_MULTI_CMD */ > > } > > > > int do_rpmb_write_key(int nargs, char **argv) > > -- > > 2.17.1 >
diff --git a/mmc_cmds.c b/mmc_cmds.c index 44623fe..c8bfc09 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -1825,6 +1825,8 @@ int do_sanitize(int nargs, char **argv) ret; \ }) +#define RMPB_MULTI_CMD_MAX_CMDS 3 + enum rpmb_op_type { MMC_RPMB_WRITE_KEY = 0x01, MMC_RPMB_READ_CNT = 0x02, @@ -1847,6 +1849,17 @@ struct rpmb_frame { u_int16_t req_resp; }; +static inline void set_single_cmd(struct mmc_ioc_cmd *ioc, __u32 opcode, + int write_flag) +{ + ioc->opcode = opcode; + ioc->write_flag = write_flag; + ioc->arg = 0x0; + ioc->blksz = 512; + ioc->blocks = 1; + ioc->flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; +} + /* Performs RPMB operation. * * @fd: RPMB device on which we should perform ioctl command @@ -1861,22 +1874,27 @@ static int do_rpmb_op(int fd, struct rpmb_frame *frame_out, unsigned int out_cnt) { +#ifndef MMC_IOC_MULTI_CMD + fprintf(stderr, "mmc-utils has been compiled without MMC_IOC_MULTI_CMD" + " support, needed by RPMB operation.\n"); + exit(1); +#else int err; u_int16_t rpmb_type; - - struct mmc_ioc_cmd ioc = { - .arg = 0x0, - .blksz = 512, - .blocks = 1, - .write_flag = 1, - .opcode = MMC_WRITE_MULTIPLE_BLOCK, - .flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC, - .data_ptr = (uintptr_t)frame_in - }; + struct mmc_ioc_multi_cmd *mioc; + struct mmc_ioc_cmd *ioc; if (!frame_in || !frame_out || !out_cnt) return -EINVAL; + /* prepare arguments for MMC_IOC_MUTLI_CMD ioctl */ + mioc = (struct mmc_ioc_multi_cmd *) + malloc(sizeof (struct mmc_ioc_multi_cmd) + + RMPB_MULTI_CMD_MAX_CMDS * sizeof (struct mmc_ioc_cmd)); + if (!mioc) { + return -ENOMEM; + } + rpmb_type = be16toh(frame_in->req_resp); switch(rpmb_type) { @@ -1887,33 +1905,24 @@ static int do_rpmb_op(int fd, goto out; } + mioc->num_of_cmds = 3; + /* Write request */ - ioc.write_flag |= (1<<31); - err = ioctl(fd, MMC_IOC_CMD, &ioc); - if (err < 0) { - err = -errno; - goto out; - } + ioc = &mioc->cmds[0]; + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, (1 << 31) | 1); + mmc_ioc_cmd_set_data((*ioc), frame_in); /* Result request */ + ioc = &mioc->cmds[1]; memset(frame_out, 0, sizeof(*frame_out)); frame_out->req_resp = htobe16(MMC_RPMB_READ_RESP); - ioc.write_flag = 1; - ioc.data_ptr = (uintptr_t)frame_out; - err = ioctl(fd, MMC_IOC_CMD, &ioc); - if (err < 0) { - err = -errno; - goto out; - } + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, 1); + mmc_ioc_cmd_set_data((*ioc), frame_out); /* Get response */ - ioc.write_flag = 0; - ioc.opcode = MMC_READ_MULTIPLE_BLOCK; - err = ioctl(fd, MMC_IOC_CMD, &ioc); - if (err < 0) { - err = -errno; - goto out; - } + ioc = &mioc->cmds[2]; + set_single_cmd(ioc, MMC_READ_MULTIPLE_BLOCK, 0); + mmc_ioc_cmd_set_data((*ioc), frame_in); break; case MMC_RPMB_READ_CNT: @@ -1924,23 +1933,18 @@ static int do_rpmb_op(int fd, /* fall through */ case MMC_RPMB_READ: - /* Request */ - err = ioctl(fd, MMC_IOC_CMD, &ioc); - if (err < 0) { - err = -errno; - goto out; - } + mioc->num_of_cmds = 2; + + /* Read request */ + ioc = &mioc->cmds[0]; + set_single_cmd(ioc, MMC_WRITE_MULTIPLE_BLOCK, 1); + mmc_ioc_cmd_set_data((*ioc), frame_in); /* Get response */ - ioc.write_flag = 0; - ioc.opcode = MMC_READ_MULTIPLE_BLOCK; - ioc.blocks = out_cnt; - ioc.data_ptr = (uintptr_t)frame_out; - err = ioctl(fd, MMC_IOC_CMD, &ioc); - if (err < 0) { - err = -errno; - goto out; - } + ioc = &mioc->cmds[1]; + set_single_cmd(ioc, MMC_READ_MULTIPLE_BLOCK, 0); + ioc->blocks = out_cnt; + mmc_ioc_cmd_set_data((*ioc), frame_out); break; default: @@ -1948,8 +1952,12 @@ static int do_rpmb_op(int fd, goto out; } + err = ioctl(fd, MMC_IOC_MULTI_CMD, mioc); + out: + free(mioc); return err; +#endif /* !MMC_IOC_MULTI_CMD */ } int do_rpmb_write_key(int nargs, char **argv)