On 22/07/26 10:31:04, [email protected] wrote:
> Hi,
>
> On 7/22/26 08:07, 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.
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> > ---
> > 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 | 124 +++++++++++++++++++++++
>
> Why not rpmb_ufs ?
because in the future (and IMO) rpmb.c should support UFS and EMMC using
the new probe interface (eventually removing rpmb_legacy).
>
> It's fine to have 2 backend implementations, the emmc is implementing the
> legacy optee API, just add a comment in the eMMC file about that.
>
> Neil
>
> > drivers/tee/optee/supplicant.c | 9 ++
> > 5 files changed, 183 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..9c3e0f031ad
> > --- /dev/null
> > +++ b/drivers/tee/optee/rpmb.c
> > @@ -0,0 +1,124 @@
> > +// 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
>
> Can this be a config option instead which defaults to 0 ?
um, I dont think so - this assumes there is only one controller. If there
are more than one, we would need a mechanism to select which one from
the enumerated values (maybe a DT alias (ie ufs-rpmb ?) I think the binding
order
changes as we enable/disable devices)
