* [PATCH] optee/rpmb.c: Fix driver routing memory alignment using a temporary buffer
@ 2021-06-08 10:07 Timothée Cercueil
2021-06-08 12:20 ` Heinrich Schuchardt
0 siblings, 1 reply; 3+ messages in thread
From: Timothée Cercueil @ 2021-06-08 10:07 UTC (permalink / raw)
To: u-boot; +Cc: Jens Wiklander, Heinrich Schuchardt, Timothée Cercueil
From: Timothée Cercueil <timothee.cercueil@st.com>
OP-TEE OS inserts a 6-byte header before a raw RPMB frame which makes
RPMB data buffer not 32bit aligned. Many RPMB drivers implicitly expect
32bit alignment of the eMMC frame including arm_pl180_mmci.c, sandbox_mmc.c
and stm32_sdmmc2.c
To prevent breaking ABI with OPTEE-OS RPC memrefs, allocate a temporary
buffer to copy the data into an aligned memory.
Signed-off-by: Timothée Cercueil <timothee.cercueil@st.com>
Change-Id: Id78d638851a666a35af907ca8de71dae00946457
Signed-off-by: Timothée Cercueil <litchi.pi@protonmail.com>
---
drivers/tee/optee/rpmb.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c
index 0804fc963c..4713df711c 100644
--- a/drivers/tee/optee/rpmb.c
+++ b/drivers/tee/optee/rpmb.c
@@ -122,6 +122,10 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req,
{
struct rpmb_req *sreq = req;
struct mmc *mmc;
+ void *rpmb_data;
+ void *rpmb_frame_buffer = NULL;
+ size_t rpmb_data_sz;
+ int ret;
if (req_size < sizeof(*sreq))
return TEE_ERROR_BAD_PARAMETERS;
@@ -131,9 +135,23 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req,
mmc = get_mmc(priv, sreq->dev_id);
if (!mmc)
return TEE_ERROR_ITEM_NOT_FOUND;
- if (mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req),
- req_size - sizeof(struct rpmb_req),
- rsp, rsp_size))
+
+ rpmb_data = RPMB_REQ_DATA(req);
+ rpmb_data_sz = req_size - sizeof(struct rpmb_req);
+ if (!IS_ALIGNED((uintptr_t)rpmb_data, sizeof(u32))) {
+ /* 32bit data alignment is required by RPMB driver */
+ rpmb_frame_buffer = malloc(rpmb_data_sz);
+ if (!rpmb_frame_buffer)
+ return TEE_ERROR_OUT_OF_MEMORY;
+
+ memcpy(rpmb_frame_buffer, rpmb_data, rpmb_data_sz);
+ rpmb_data = rpmb_frame_buffer;
+ }
+
+ ret = mmc_rpmb_route_frames(mmc, rpmb_data, rpmb_data_sz,
+ rsp, rsp_size);
+ free(rpmb_frame_buffer);
+ if (ret)
return TEE_ERROR_BAD_PARAMETERS;
return TEE_SUCCESS;
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] optee/rpmb.c: Fix driver routing memory alignment using a temporary buffer
2021-06-08 10:07 [PATCH] optee/rpmb.c: Fix driver routing memory alignment using a temporary buffer Timothée Cercueil
@ 2021-06-08 12:20 ` Heinrich Schuchardt
2021-06-08 13:34 ` litchi.pi
0 siblings, 1 reply; 3+ messages in thread
From: Heinrich Schuchardt @ 2021-06-08 12:20 UTC (permalink / raw)
To: Timothée Cercueil
Cc: Jens Wiklander, Timothée Cercueil, Ilias Apalodimas,
Peng Fan, u-boot
On 08.06.21 12:07, Timothée Cercueil wrote:
> From: Timothée Cercueil <timothee.cercueil@st.com>
>
> OP-TEE OS inserts a 6-byte header before a raw RPMB frame which makes
> RPMB data buffer not 32bit aligned. Many RPMB drivers implicitly expect
> 32bit alignment of the eMMC frame including arm_pl180_mmci.c, sandbox_mmc.c
> and stm32_sdmmc2.c
Generally block device drivers expect ARCH_DMA_MINALIGN. Have a look at
mmc_rpmb_read(), mmc_rpmb_write().
Why would we care about alignment in two different modules:
drivers/tee/optee/rpmb.c and drivers/mmc/rpmb.c?
Shouldn't the fix be in drivers/mmc/rpmb.c?
Let't include the MMC maintainer in the discussion.
Best regards
Heinrich
>
> To prevent breaking ABI with OPTEE-OS RPC memrefs, allocate a temporary
> buffer to copy the data into an aligned memory.
>
> Signed-off-by: Timothée Cercueil <timothee.cercueil@st.com>
> Change-Id: Id78d638851a666a35af907ca8de71dae00946457
> Signed-off-by: Timothée Cercueil <litchi.pi@protonmail.com>
> ---
> drivers/tee/optee/rpmb.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c
> index 0804fc963c..4713df711c 100644
> --- a/drivers/tee/optee/rpmb.c
> +++ b/drivers/tee/optee/rpmb.c
> @@ -122,6 +122,10 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req,
> {
> struct rpmb_req *sreq = req;
> struct mmc *mmc;
> + void *rpmb_data;
> + void *rpmb_frame_buffer = NULL;
> + size_t rpmb_data_sz;
> + int ret;
>
> if (req_size < sizeof(*sreq))
> return TEE_ERROR_BAD_PARAMETERS;
> @@ -131,9 +135,23 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req,
> mmc = get_mmc(priv, sreq->dev_id);
> if (!mmc)
> return TEE_ERROR_ITEM_NOT_FOUND;
> - if (mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req),
> - req_size - sizeof(struct rpmb_req),
> - rsp, rsp_size))
> +
> + rpmb_data = RPMB_REQ_DATA(req);
> + rpmb_data_sz = req_size - sizeof(struct rpmb_req);
> + if (!IS_ALIGNED((uintptr_t)rpmb_data, sizeof(u32))) {
> + /* 32bit data alignment is required by RPMB driver */
> + rpmb_frame_buffer = malloc(rpmb_data_sz);
> + if (!rpmb_frame_buffer)
> + return TEE_ERROR_OUT_OF_MEMORY;
> +
> + memcpy(rpmb_frame_buffer, rpmb_data, rpmb_data_sz);
> + rpmb_data = rpmb_frame_buffer;
> + }
> +
> + ret = mmc_rpmb_route_frames(mmc, rpmb_data, rpmb_data_sz,
> + rsp, rsp_size);
> + free(rpmb_frame_buffer);
> + if (ret)
> return TEE_ERROR_BAD_PARAMETERS;
> return TEE_SUCCESS;
>
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] optee/rpmb.c: Fix driver routing memory alignment using a temporary buffer
2021-06-08 12:20 ` Heinrich Schuchardt
@ 2021-06-08 13:34 ` litchi.pi
0 siblings, 0 replies; 3+ messages in thread
From: litchi.pi @ 2021-06-08 13:34 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Jens Wiklander, Timothée Cercueil, Ilias Apalodimas,
Peng Fan, u-boot
I agree, the most generic way to fix this problem would be to ensure memory alignment inside the general RPMB driver.
the drivers/tee/optee/rpmb.c driver uses `mmc_rpmb_route_frames`, which is a parallel API used to drive RPMB frames directly into the MMC driver.
I thought that the MMC driver expecting aligned buffer (and panicking if not) was a normal behaviour, and the memory alignment had to be fixed before that.
The problem is that it comes from a shared memory allocation from OP-TEE, which is far less easy to fix using `ALLOC_CACHE_ALIGN_BUFFER` or so.
If the MMC driver has to handle unaligned addresses, then the fix should maybe be on the MMC driver itself indeed,
Regards,
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, June 8, 2021 2:20 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 08.06.21 12:07, Timothée Cercueil wrote:
>
> > From: Timothée Cercueil timothee.cercueil@st.com
> > OP-TEE OS inserts a 6-byte header before a raw RPMB frame which makes
> > RPMB data buffer not 32bit aligned. Many RPMB drivers implicitly expect
> > 32bit alignment of the eMMC frame including arm_pl180_mmci.c, sandbox_mmc.c
> > and stm32_sdmmc2.c
>
> Generally block device drivers expect ARCH_DMA_MINALIGN. Have a look at
> mmc_rpmb_read(), mmc_rpmb_write().
>
> Why would we care about alignment in two different modules:
> drivers/tee/optee/rpmb.c and drivers/mmc/rpmb.c?
>
> Shouldn't the fix be in drivers/mmc/rpmb.c?
>
> Let't include the MMC maintainer in the discussion.
>
> Best regards
>
> Heinrich
>
> > To prevent breaking ABI with OPTEE-OS RPC memrefs, allocate a temporary
> > buffer to copy the data into an aligned memory.
> >
> > Signed-off-by: Timothée Cercueil timothee.cercueil@st.com
> > Change-Id: Id78d638851a666a35af907ca8de71dae00946457
> > Signed-off-by: Timothée Cercueil litchi.pi@protonmail.com
> >
> > -------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> >
> > drivers/tee/optee/rpmb.c | 24 +++++++++++++++++++++---
> > 1 file changed, 21 insertions(+), 3 deletions(-)
> > diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c
> > index 0804fc963c..4713df711c 100644
> > --- a/drivers/tee/optee/rpmb.c
> > +++ b/drivers/tee/optee/rpmb.c
> > @@ -122,6 +122,10 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req,
> > {
> > struct rpmb_req *sreq = req;
> > struct mmc *mmc;
> >
> > - void *rpmb_data;
> >
> > - void *rpmb_frame_buffer = NULL;
> >
> > - size_t rpmb_data_sz;
> >
> > - int ret;
> > if (req_size < sizeof(*sreq))
> > return TEE_ERROR_BAD_PARAMETERS;
> > @@ -131,9 +135,23 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req,
> > mmc = get_mmc(priv, sreq->dev_id);
> > if (!mmc)
> > return TEE_ERROR_ITEM_NOT_FOUND;
> >
> >
> > - if (mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req),
> >
> >
> > - req_size - sizeof(struct rpmb_req),
> >
> >
> > - rsp, rsp_size))
> >
> >
> >
> > -
> > - rpmb_data = RPMB_REQ_DATA(req);
> >
> >
> > - rpmb_data_sz = req_size - sizeof(struct rpmb_req);
> >
> >
> > - if (!IS_ALIGNED((uintptr_t)rpmb_data, sizeof(u32))) {
> >
> >
> > - /* 32bit data alignment is required by RPMB driver */
> >
> >
> > - rpmb_frame_buffer = malloc(rpmb_data_sz);
> >
> >
> > - if (!rpmb_frame_buffer)
> >
> >
> > - return TEE_ERROR_OUT_OF_MEMORY;
> >
> >
> > -
> > - memcpy(rpmb_frame_buffer, rpmb_data, rpmb_data_sz);
> >
> >
> > - rpmb_data = rpmb_frame_buffer;
> >
> >
> > - }
> >
> >
> > -
> > - ret = mmc_rpmb_route_frames(mmc, rpmb_data, rpmb_data_sz,
> >
> >
> > - rsp, rsp_size);
> >
> >
> > - free(rpmb_frame_buffer);
> >
> >
> > - if (ret)
> > return TEE_ERROR_BAD_PARAMETERS;
> > return TEE_SUCCESS;
> >
> >
> >
> > --
> > 2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-09 3:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 10:07 [PATCH] optee/rpmb.c: Fix driver routing memory alignment using a temporary buffer Timothée Cercueil
2021-06-08 12:20 ` Heinrich Schuchardt
2021-06-08 13:34 ` litchi.pi
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.