| Message ID | 20210615085255.24899-1-litchi.pi@protonmail.com |
|---|---|
| State | Accepted |
| Commit | a9f7be509af90fa5f2c308867ad3b0bd48532c6e |
| Delegated to: | Peng Fan |
| Headers | show |
| Series | mmc: rpmb: Fix driver routing memory alignment with tmp buffer | expand |
Hello, Does anyone have a feedback on that fix ? I think the location of the fix is important to be discussed too as it needs to be generic but not overlap with existing checks. Have a nice day, Timothée Cercueil ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Tuesday, June 15th, 2021 at 10:53 AM, Timothée Cercueil <litchi.pi@protonmail.com> wrote: > From: litchipi litchi.pi@protonmail.com > > Fix mmc_rpmb_route_frames() implementation to comply with most MMC > > drivers that expect some alignment of MMC data frames in memory. > > When called from drivers/tee/optee/rpmb.c, the address passed is not > > aligned properly. OP-TEE OS inserts a 6-byte header before a raw RPMB > > frame which makes RPMB data buffer not 32bit aligned. To prevent breaking > > ABI with OPTEE-OS RPC memrefs, allocate a temporary buffer to copy the > > data into an aligned memory. > > Many RPMB drivers implicitly expect 32bit alignment of the eMMC frame > > including arm_pl180_mmci.c, sandbox_mmc.c and stm32_sdmmc2.c > > Signed-off-by: Timothée Cercueil timothee.cercueil@st.com > > Signed-off-by: Timothée Cercueil litchi.pi@protonmail.com > > drivers/mmc/rpmb.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > Changes since v1: > > - Fixed the Signed-off-by errors from previous patch. > > - This patch follows the subject discussed at: https://lists.denx.de/pipermail/u-boot/2021-June/451687.html > > - Changed the commit log 1st line > > diff --git a/drivers/mmc/rpmb.c b/drivers/mmc/rpmb.c > > index ea7e506666..b68d98573c 100644 > > --- a/drivers/mmc/rpmb.c > > +++ b/drivers/mmc/rpmb.c > > @@ -480,10 +480,24 @@ int mmc_rpmb_route_frames(struct mmc *mmc, void *req, unsigned long reqlen, > > * and possibly just delay an eventual error which will be harder > > * to track down. > > */ > > - void *rpmb_data = NULL; > - int ret; > > 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)); > > > > - if (!IS_ALIGNED((uintptr_t)req, ARCH_DMA_MINALIGN)) { > - /* Memory alignment is required by MMC driver */ > > > - rpmb_data = malloc(reqlen); > > > - if (!rpmb_data) > > > - return -ENOMEM; > > > > - memcpy(rpmb_data, req, reqlen); > > > - req = rpmb_data; > > > - } > > - ret = rpmb_route_frames(mmc, req, reqlen / sizeof(struct s_rpmb), > - rsp, rsplen / sizeof(struct s_rpmb)); > > > - free(rpmb_data); > - return ret; > > } > > -- > > 2.17.1
Hi, On 6/22/21 4:24 PM, litchi.pi wrote: > Hello, > Does anyone have a feedback on that fix ? > I think the location of the fix is important to be discussed too as it needs to be generic but not overlap with existing checks. > Have a nice day, Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > Timothée Cercueil > > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > > On Tuesday, June 15th, 2021 at 10:53 AM, Timothée Cercueil <litchi.pi@protonmail.com> wrote: > >> From: litchipi litchi.pi@protonmail.com >> >> Fix mmc_rpmb_route_frames() implementation to comply with most MMC >> >> drivers that expect some alignment of MMC data frames in memory. >> >> When called from drivers/tee/optee/rpmb.c, the address passed is not >> >> aligned properly. OP-TEE OS inserts a 6-byte header before a raw RPMB >> >> frame which makes RPMB data buffer not 32bit aligned. To prevent breaking >> >> ABI with OPTEE-OS RPC memrefs, allocate a temporary buffer to copy the >> >> data into an aligned memory. >> >> Many RPMB drivers implicitly expect 32bit alignment of the eMMC frame >> >> including arm_pl180_mmci.c, sandbox_mmc.c and stm32_sdmmc2.c >> >> Signed-off-by: Timothée Cercueil timothee.cercueil@st.com >> >> Signed-off-by: Timothée Cercueil litchi.pi@protonmail.com >> >> drivers/mmc/rpmb.c | 18 ++++++++++++++++-- >> >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> Changes since v1: >> >> - Fixed the Signed-off-by errors from previous patch. >> >> - This patch follows the subject discussed at: https://protect2.fireeye.com/v1/url?k=f4826206-ab195acc-f483e949-0cc47a31bee8-fbfde87d1c121f8b&q=1&e=4fb97df5-13bf-4f21-9dd7-ad6f5a6ed028&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2021-June%2F451687.html >> >> - Changed the commit log 1st line >> >> diff --git a/drivers/mmc/rpmb.c b/drivers/mmc/rpmb.c >> >> index ea7e506666..b68d98573c 100644 >> >> --- a/drivers/mmc/rpmb.c >> >> +++ b/drivers/mmc/rpmb.c >> >> @@ -480,10 +480,24 @@ int mmc_rpmb_route_frames(struct mmc *mmc, void *req, unsigned long reqlen, >> >> * and possibly just delay an eventual error which will be harder >> >> * to track down. >> >> */ >> >> - void *rpmb_data = NULL; >> - int ret; >> >> 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)); >> >> >> >> - if (!IS_ALIGNED((uintptr_t)req, ARCH_DMA_MINALIGN)) { >> - /* Memory alignment is required by MMC driver */ >> >> >> - rpmb_data = malloc(reqlen); >> >> >> - if (!rpmb_data) >> >> >> - return -ENOMEM; >> >> >> >> - memcpy(rpmb_data, req, reqlen); >> >> >> - req = rpmb_data; >> >> >> - } >> >> - ret = rpmb_route_frames(mmc, req, reqlen / sizeof(struct s_rpmb), >> - rsp, rsplen / sizeof(struct s_rpmb)); >> >> >> - free(rpmb_data); >> - return ret; >> >> } >> >> -- >> >> 2.17.1 >
diff --git a/drivers/mmc/rpmb.c b/drivers/mmc/rpmb.c index ea7e506666..b68d98573c 100644 --- a/drivers/mmc/rpmb.c +++ b/drivers/mmc/rpmb.c @@ -480,10 +480,24 @@ int mmc_rpmb_route_frames(struct mmc *mmc, void *req, unsigned long reqlen, * and possibly just delay an eventual error which will be harder * to track down. */ + void *rpmb_data = NULL; + int ret; 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)); + if (!IS_ALIGNED((uintptr_t)req, ARCH_DMA_MINALIGN)) { + /* Memory alignment is required by MMC driver */ + rpmb_data = malloc(reqlen); + if (!rpmb_data) + return -ENOMEM; + + memcpy(rpmb_data, req, reqlen); + req = rpmb_data; + } + + ret = rpmb_route_frames(mmc, req, reqlen / sizeof(struct s_rpmb), + rsp, rsplen / sizeof(struct s_rpmb)); + free(rpmb_data); + return ret; }