Re: [PATCH v1 7/7] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez via U-Boot <u-boot@lists.u-boot-project.org>]

On 21/07/26 09:34:45, neil.armstrong@linaro.org wrote:
> On 7/20/26 10:51, Jorge Ramirez-Ortiz wrote:
> > OP-TEE's legacy RPMB supplicant interface assumes the RPMB device is eMMC
> > and derives the key from an eMMC-shaped device ID, so it can never
> > reproduce the key Linux derives for a UFS device; secure storage on
> > UFS-only platforms instead needs the transport-agnostic RPMB subsystem
> > interface, through which the normal world describes the real RPMB device
> > to OP-TEE. Add a UFS backend for it so OP-TEE derives a key matching Linux
> > and can use RPMB secure storage on UFS, leaving the legacy eMMC path
> > untouched.
> > 
> > This subsystem backend currently supports UFS drives only; eMMC still uses
> > the legacy interface. As the legacy interface is dropped in favour of the
> > subsystem one, an eMMC backend should be added here so both transports are
> > served through a single RPMB path.
> 
> It's kind of sad eMMC RPMB support is not implemented in the new interface,
> how hard would it be to add the support ?

I dont disagree - it makes the encapsulation more obvious as well - but
I currently dont have the cycles/hardware for it. if you delegate the
prototype to some LLM we are probably talking about a couple of hours
tops - any lesser model will do; then  a day or two of tidying up.

The issue is always verifying in real hardware - ie deciding to write a
key that you will have to remember forever. In my case I burnt the key
in my Qualcomm RB8 which means that I cant use QC's OP-TEE HUK driver
(since the value it reports changes when secure boot is enabled). So on
this hardware, my HUK will always be 0 (easy to remember)

> 
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > ---
> >   drivers/tee/optee/Makefile               |   1 +
> >   drivers/tee/optee/optee_msg_supplicant.h |   8 ++
> >   drivers/tee/optee/optee_private.h        |  41 ++++++++
> >   drivers/tee/optee/rpmb.c                 | 126 +++++++++++++++++++++++
> >   drivers/tee/optee/supplicant.c           |   9 ++
> >   5 files changed, 185 insertions(+)
> >   create mode 100644 drivers/tee/optee/rpmb.c
> > 
> > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > index 8321cf53a19..7af19834c00 100644
> > --- a/drivers/tee/optee/Makefile
> > +++ b/drivers/tee/optee/Makefile
> > @@ -4,3 +4,4 @@ obj-$(CONFIG_OPTEE) += core.o
> >   obj-y += supplicant.o
> >   obj-$(CONFIG_DM_I2C) += i2c.o
> >   obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
> > +obj-$(CONFIG_SUPPORT_UFS_RPMB) += rpmb.o
> > diff --git a/drivers/tee/optee/optee_msg_supplicant.h b/drivers/tee/optee/optee_msg_supplicant.h
> > index 963cfd47824..b720d5d7b3f 100644
> > --- a/drivers/tee/optee/optee_msg_supplicant.h
> > +++ b/drivers/tee/optee/optee_msg_supplicant.h
> > @@ -152,6 +152,14 @@
> >    */
> >   #define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> > +#define OPTEE_MSG_RPC_CMD_RPMB_PROBE_RESET	22
> > +#define OPTEE_MSG_RPC_CMD_RPMB_PROBE_NEXT	23
> > +#define OPTEE_MSG_RPC_CMD_RPMB_FRAMES		24
> > +
> > +#define OPTEE_RPC_SHM_TYPE_APPL		0
> > +
> > +#define OPTEE_RPC_RPMB_UFS		1
> > +
> >   /*
> >    * Was OPTEE_MSG_RPC_CMD_SQL_FS, which isn't supported any longer
> >    */
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index 1f07a27ee4b..2888257aefe 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -9,6 +9,17 @@
> >   #include <tee.h>
> >   #include <log.h>
> > +#ifdef CONFIG_SUPPORT_UFS_RPMB
> > +/**
> > + * struct optee_private - OP-TEE driver private data
> > + * @rpmb_next_region:	next UFS RPMB region to report on PROBE_NEXT
> > + * @rpmb_cur_region:	UFS RPMB region selected by the last PROBE_NEXT
> > + */
> > +struct optee_private {
> > +	unsigned int rpmb_next_region;
> > +	unsigned int rpmb_cur_region;
> > +};
> > +#else
> >   /**
> >    * struct optee_private - OP-TEE driver private data
> >    * @rpmb_mmc:		mmc device for the RPMB partition
> > @@ -22,6 +33,7 @@ struct optee_private {
> >   	int rpmb_dev_id;
> >   	int rpmb_original_part;
> >   };
> > +#endif
> >   struct optee_msg_arg;
> > @@ -60,6 +72,35 @@ static inline void optee_suppl_rpmb_release(struct udevice *dev)
> >   }
> >   #endif
> > +#ifdef CONFIG_SUPPORT_UFS_RPMB
> > +void optee_suppl_cmd_rpmb_probe_reset(struct udevice *dev,
> > +				      struct optee_msg_arg *arg);
> > +
> > +void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev,
> > +				     struct optee_msg_arg *arg);
> > +
> > +void optee_suppl_cmd_rpmb_frames(struct udevice *dev,
> > +				 struct optee_msg_arg *arg);
> > +#else
> > +static inline void optee_suppl_cmd_rpmb_probe_reset(struct udevice *dev,
> > +						    struct optee_msg_arg *arg)
> > +{
> > +	arg->ret = TEE_ERROR_NOT_IMPLEMENTED;
> > +}
> > +
> > +static inline void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev,
> > +						   struct optee_msg_arg *arg)
> > +{
> > +	arg->ret = TEE_ERROR_NOT_IMPLEMENTED;
> > +}
> > +
> > +static inline void optee_suppl_cmd_rpmb_frames(struct udevice *dev,
> > +					       struct optee_msg_arg *arg)
> > +{
> > +	arg->ret = TEE_ERROR_NOT_IMPLEMENTED;
> > +}
> > +#endif
> > +
> >   #ifdef CONFIG_DM_I2C
> >   /**
> >    * optee_suppl_cmd_i2c_transfer() - route I2C requests to an I2C chip
> > diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c
> > new file mode 100644
> > index 00000000000..3b3bd0ac12e
> > --- /dev/null
> > +++ b/drivers/tee/optee/rpmb.c
> > @@ -0,0 +1,126 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <dm.h>
> > +#include <tee.h>
> > +#include <ufs.h>
> > +
> > +#include "optee_msg.h"
> > +#include "optee_msg_supplicant.h"
> > +#include "optee_private.h"
> > +
> > +#define UFS_RPMB_CONTROLLER	0
> > +
> > +void optee_suppl_cmd_rpmb_probe_reset(struct udevice *dev,
> > +				      struct optee_msg_arg *arg)
> > +{
> > +	struct optee_private *priv = dev_get_priv(dev);
> > +
> > +	if (arg->num_params != 1 ||
> > +	    arg->params[0].attr != OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT) {
> > +		arg->ret = TEE_ERROR_BAD_PARAMETERS;
> > +		return;
> > +	}
> > +
> > +	priv->rpmb_next_region = 0;
> > +	priv->rpmb_cur_region = 0;
> > +
> > +	arg->params[0].u.value.a = OPTEE_RPC_SHM_TYPE_APPL;
> > +	arg->ret = TEE_SUCCESS;
> > +}
> > +
> > +void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev,
> > +				     struct optee_msg_arg *arg)
> > +{
> > +	struct optee_private *priv = dev_get_priv(dev);
> > +	struct udevice *scsi_dev;
> > +	struct tee_shm *cid_shm;
> > +	u8 size_mult = 0;
> > +	u8 rel_wr = 0;
> > +	void *cid_buf;
> > +	ulong cid_size;
> > +	int ret;
> > +
> > +	if (arg->num_params != 2 ||
> > +	    arg->params[0].attr != OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT ||
> > +	    arg->params[1].attr != OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT) {
> > +		arg->ret = TEE_ERROR_BAD_PARAMETERS;
> > +		return;
> > +	}
> > +
> > +	cid_shm = (struct tee_shm *)(ulong)arg->params[1].u.rmem.shm_ref;
> > +	cid_buf = (u8 *)cid_shm->addr + arg->params[1].u.rmem.offs;
> > +	cid_size = arg->params[1].u.rmem.size;
> > +	if (cid_size < UFS_RPMB_CID_SIZE) {
> > +		arg->ret = TEE_ERROR_SHORT_BUFFER;
> > +		return;
> > +	}
> > +
> > +	scsi_dev = ufs_rpmb_get_scsi_dev(UFS_RPMB_CONTROLLER);
> > +	if (!scsi_dev) {
> > +		arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
> > +		return;
> > +	}
> > +
> > +	while (priv->rpmb_next_region < UFS_RPMB_NUM_REGIONS) {
> > +		unsigned int region = priv->rpmb_next_region++;
> > +
> > +		ret = ufs_rpmb_get_region_info(scsi_dev, region, &size_mult,
> > +					       &rel_wr, cid_buf);
> > +		if (ret < 0) {
> > +			arg->ret = TEE_ERROR_GENERIC;
> > +			return;
> > +		}
> > +		if (!ret)
> > +			continue;
> > +
> > +		priv->rpmb_cur_region = region;
> > +		arg->params[0].u.value.a = OPTEE_RPC_RPMB_UFS;
> > +		arg->params[0].u.value.b = size_mult;
> > +		arg->params[0].u.value.c = rel_wr;
> > +		arg->params[1].u.rmem.size = UFS_RPMB_CID_SIZE;
> > +		arg->ret = TEE_SUCCESS;
> > +		return;
> > +	}
> > +
> > +	arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
> > +}
> > +
> > +void optee_suppl_cmd_rpmb_frames(struct udevice *dev,
> > +				 struct optee_msg_arg *arg)
> > +{
> > +	struct optee_private *priv = dev_get_priv(dev);
> > +	struct tee_shm *req_shm;
> > +	struct tee_shm *rsp_shm;
> > +	struct udevice *scsi_dev;
> > +	void *req_buf;
> > +	void *rsp_buf;
> > +	ulong req_size;
> > +	ulong rsp_size;
> > +
> > +	if (arg->num_params != 2 ||
> > +	    arg->params[0].attr != OPTEE_MSG_ATTR_TYPE_RMEM_INPUT ||
> > +	    arg->params[1].attr != OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT) {
> > +		arg->ret = TEE_ERROR_BAD_PARAMETERS;
> > +		return;
> > +	}
> > +
> > +	scsi_dev = ufs_rpmb_get_scsi_dev(UFS_RPMB_CONTROLLER);
> > +	if (!scsi_dev) {
> > +		arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
> > +		return;
> > +	}
> > +
> > +	req_shm = (struct tee_shm *)(ulong)arg->params[0].u.rmem.shm_ref;
> > +	req_buf = (u8 *)req_shm->addr + arg->params[0].u.rmem.offs;
> > +	req_size = arg->params[0].u.rmem.size;
> > +
> > +	rsp_shm = (struct tee_shm *)(ulong)arg->params[1].u.rmem.shm_ref;
> > +	rsp_buf = (u8 *)rsp_shm->addr + arg->params[1].u.rmem.offs;
> > +	rsp_size = arg->params[1].u.rmem.size;
> > +
> > +	if (ufs_rpmb_route_frames(scsi_dev, priv->rpmb_cur_region, req_buf,
> > +				  req_size, rsp_buf, rsp_size))
> > +		arg->ret = TEE_ERROR_BAD_PARAMETERS;
> > +	else
> > +		arg->ret = TEE_SUCCESS;
> > +}
> > diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c
> > index 8a426f53ba8..50b780037fb 100644
> > --- a/drivers/tee/optee/supplicant.c
> > +++ b/drivers/tee/optee/supplicant.c
> > @@ -89,6 +89,15 @@ void optee_suppl_cmd(struct udevice *dev, struct tee_shm *shm_arg,
> >   	case OPTEE_MSG_RPC_CMD_RPMB:
> >   		optee_suppl_cmd_rpmb(dev, arg);
> >   		break;
> > +	case OPTEE_MSG_RPC_CMD_RPMB_PROBE_RESET:
> > +		optee_suppl_cmd_rpmb_probe_reset(dev, arg);
> > +		break;
> > +	case OPTEE_MSG_RPC_CMD_RPMB_PROBE_NEXT:
> > +		optee_suppl_cmd_rpmb_probe_next(dev, arg);
> > +		break;
> > +	case OPTEE_MSG_RPC_CMD_RPMB_FRAMES:
> > +		optee_suppl_cmd_rpmb_frames(dev, arg);
> > +		break;
> >   	case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
> >   		optee_suppl_cmd_i2c_transfer(arg);
> >   		break;
>