On Tue, 3 Nov 2026 at 00:21, Igor Opaniuk <[email protected]> wrote: > > Hi Ilias, > > On Sun, Nov 1, 2026 at 12:18 PM Ilias Apalodimas > <[email protected]> wrote: > > > > Hi Igor, > > > > On Thu, 29 Oct 2026 at 17:45, Igor Opaniuk > > <[email protected]> wrote: > > > > > > The supplicant switched the eMMC to the RPMB hardware partition when the > > > device was first used and only switched back when the OP-TEE session was > > > released. > > > Between two RPMB requests other users of the same device switch > > > it back to the user area: libavb does so for every partition read while > > > verifying a chain, and the Android bootmeth reads boot/vendor_boot right > > > after verification. > > > > I am not sure I understand the bug here. do_call_with_arg() calls > > optee_suppl_rpmb_release() everytime we return from optee which calls > > release_mmc(). Jan introduced a nested call on that that would explain > > the bug? > > You're right, and the commit message is wrong. The partition is > restored every time a call into OP-TEE returns, not when the session is > released. And tbh I re-tested both bootmeth with AVB TA (+ also with > RPMB wiped) and > separate avb cmds like: avb read_rb, read_part, verify, write_rb and mmc > read, > without this patch, with only patch 2 on top of master, on QEMU with the AVB > TA > and all of it works. > > I can no longer reproduce the timeouts and I/O errors > from the commit message. Seems that they came from a tree with several fixes > in > flight at once, the sdhci status-timeout one among them (which was > accepted recently), > and I wrongly attributed them this patch, so I apologize for the noise.
No worries > > So please feel free to drop this patch. Patch 2 applies to master on > its own, and I > can resend it alone as v5 if you prefer and you don't have objections. I'll have a look at patch #2 and pick it up if it's ok Cheers /Ilias > > Thanks > > > > > > > > With RPMB left selected those reads hit the RPMB partition, and the next > > > RPMB frames were routed to whatever partition the other user left > > > selected, which ends in status timeouts and I/O errors (seen with > > > the AVB TA on QEMU's emulated eMMC). > > > > > > Select the RPMB partition around every data request and restore the > > > previously selected partition afterwards. > > > > > > Signed-off-by: Igor Opaniuk <[email protected]> > > > --- > > > drivers/tee/optee/rpmb_emmc.c | 88 > > > +++++++++++++++++++++++++------------------ > > > 1 file changed, 52 insertions(+), 36 deletions(-) > > > > > > diff --git a/drivers/tee/optee/rpmb_emmc.c b/drivers/tee/optee/rpmb_emmc.c > > > index 7ae6045511b..d4abc48f764 100644 > > > --- a/drivers/tee/optee/rpmb_emmc.c > > > +++ b/drivers/tee/optee/rpmb_emmc.c > > > @@ -42,51 +42,55 @@ struct rpmb_dev_info { > > > > > > static void release_mmc(struct optee_private *priv) > > > { > > > - int rc; > > > - > > > if (!priv->rpmb_mmc) > > > return; > > > > > > - rc = blk_select_hwpart_devnum(UCLASS_MMC, priv->rpmb_dev_id, > > > - priv->rpmb_original_part); > > > - if (rc) > > > - debug("%s: blk_select_hwpart_devnum() failed: %d\n", > > > - __func__, rc); > > > - > > > priv->rpmb_mmc = NULL; > > > } > > > > > > +/* > > > + * The RPMB hardware partition is selected for the duration of a single > > > + * request only. Between two requests other users of the same eMMC, for > > > + * instance libavb reading the user area while verifying a chain of > > > + * partitions, switch the device back to the user area. Keeping RPMB > > > + * selected across requests would make those reads, and the following > > > RPMB > > > + * frames, hit the wrong partition. get_mmc() therefore selects the RPMB > > > + * partition on every call, and restore_part() puts the previous one back > > > + * once the frames have been routed. > > > + */ > > > static struct mmc *get_mmc(struct optee_private *priv, int dev_id) > > > { > > > struct mmc *mmc; > > > int rc; > > > > > > - if (priv->rpmb_mmc && priv->rpmb_dev_id == dev_id) > > > - return priv->rpmb_mmc; > > > + if (!priv->rpmb_mmc || priv->rpmb_dev_id != dev_id) { > > > + release_mmc(priv); > > > > > > - release_mmc(priv); > > > + mmc = find_mmc_device(dev_id); > > > + if (!mmc) { > > > + debug("Cannot find RPMB device\n"); > > > + return NULL; > > > + } > > > + if (mmc_init(mmc)) { > > > + log(LOGC_BOARD, LOGL_ERR, "%s:MMC device %d init > > > failed\n", > > > + __func__, dev_id); > > > + return NULL; > > > + } > > > + if (!(mmc->version & MMC_VERSION_MMC)) { > > > + debug("Device id %d is not an eMMC device\n", > > > dev_id); > > > + return NULL; > > > + } > > > + if (mmc->version < MMC_VERSION_4_41) { > > > + debug("Device id %d: RPMB not supported before > > > version 4.41\n", > > > + dev_id); > > > + return NULL; > > > + } > > > > > > - mmc = find_mmc_device(dev_id); > > > - if (!mmc) { > > > - debug("Cannot find RPMB device\n"); > > > - return NULL; > > > - } > > > - if (mmc_init(mmc)) { > > > - log(LOGC_BOARD, LOGL_ERR, "%s:MMC device %d init > > > failed\n", __func__, dev_id); > > > - return NULL; > > > - } > > > - if (!(mmc->version & MMC_VERSION_MMC)) { > > > - debug("Device id %d is not an eMMC device\n", dev_id); > > > - return NULL; > > > - } > > > - if (mmc->version < MMC_VERSION_4_41) { > > > - debug("Device id %d: RPMB not supported before version > > > 4.41\n", > > > - dev_id); > > > - return NULL; > > > + priv->rpmb_mmc = mmc; > > > + priv->rpmb_dev_id = dev_id; > > > } > > > > > > - priv->rpmb_original_part = mmc_get_blk_desc(mmc)->hwpart; > > > - > > > + priv->rpmb_original_part = > > > mmc_get_blk_desc(priv->rpmb_mmc)->hwpart; > > > rc = blk_select_hwpart_devnum(UCLASS_MMC, dev_id, MMC_PART_RPMB); > > > if (rc) { > > > debug("Device id %d: cannot select RPMB partition: %d\n", > > > @@ -94,9 +98,18 @@ static struct mmc *get_mmc(struct optee_private *priv, > > > int dev_id) > > > return NULL; > > > } > > > > > > - priv->rpmb_mmc = mmc; > > > - priv->rpmb_dev_id = dev_id; > > > - return mmc; > > > + return priv->rpmb_mmc; > > > +} > > > + > > > +static void restore_part(struct optee_private *priv) > > > +{ > > > + int rc; > > > + > > > + rc = blk_select_hwpart_devnum(UCLASS_MMC, priv->rpmb_dev_id, > > > + priv->rpmb_original_part); > > > + if (rc) > > > + debug("%s: blk_select_hwpart_devnum() failed: %d\n", > > > + __func__, rc); > > > } > > > > > > static u32 rpmb_get_dev_info(u16 dev_id, struct rpmb_dev_info *info) > > > @@ -130,6 +143,7 @@ static u32 rpmb_process_request(struct optee_private > > > *priv, void *req, > > > { > > > struct rpmb_req *sreq = req; > > > struct mmc *mmc; > > > + int rc; > > > > > > if (req_size < sizeof(*sreq)) > > > return TEE_ERROR_BAD_PARAMETERS; > > > @@ -139,9 +153,11 @@ 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)) > > > + rc = mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req), > > > + req_size - sizeof(struct > > > rpmb_req), > > > + rsp, rsp_size); > > > + restore_part(priv); > > > + if (rc) > > > return TEE_ERROR_BAD_PARAMETERS; > > > return TEE_SUCCESS; > > > > > > > > > -- > > > 2.53.0 > > >
