[PATCH v2 0/5] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[PATCH v2 0/5] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Jorge Ramirez-Ortiz via U-Boot]

OP-TEE secure storage (CFG_RPMB_FS) relies on an RPMB partition, but
U-Boot's OP-TEE RPMB supplicant only speaks the legacy single-command
interface, which is bound to eMMC. SoCs that are UFS-only and have no
eMMC (for example the Qualcomm SA8775P) therefore cannot back OP-TEE
secure storage from U-Boot today. This series adds that support.

It introduces the transport-agnostic OP-TEE RPMB "subsystem" interface
(PROBE_RESET / PROBE_NEXT / FRAMES), where the normal world enumerates
the RPMB device and reports its kind, size and CID, then carries the
signed frames. The legacy eMMC supplicant is preserved unchanged, only
renamed to rpmb_legacy.c; the two are mutually exclusive via Kconfig
(SUPPORT_UFS_RPMB depends on !SUPPORT_EMMC_RPMB) because the OP-TEE
supplicant handles a single RPMB transport. The subsystem interface is
UFS-only for now; eMMC can be migrated onto it later as the legacy path
is retired.

On top of that it adds a UFS RPMB transport that moves JEDEC RPMB frames
to and from the RPMB Well-Known LUN using SCSI SECURITY PROTOCOL IN/OUT.
The per-region 16-byte CID is derived by BLAKE2b-hashing the exact
device-id string the Linux kernel builds (ufshcd_create_device_id()
plus a "-R<region>" suffix), so OP-TEE derives an RPMB key that matches
the one Linux would use.

The first patch is a standalone UFS descriptor fix the RPMB path depends
on (UTF-16BE string decoding); the transport patches also include a
power-on UNIT ATTENTION retry and a DMA-alignment bounce for the RPMB
WLUN.

Note: reading UFS descriptors reliably also requires the descriptor
data-segment cache-invalidation fix, which has already been posted and
merged separately, so this series is based on top of it.

Tested on the Qualcomm IQ-9075-EVK (SA8775P): OP-TEE with CFG_RPMB_FS
programs the RPMB key through U-Boot and reads/writes secure-storage
objects, with the derived CID matching the Linux UFS device_id ABI.

Dependencies:
Linux kernel:
 https://lore.kernel.org/linux-scsi/20260716083728.2226422-1-jorge.ramirez@oss.qualcomm.com/
Op-tee
 https://github.com/OP-TEE/optee_os/pull/7881

v2:
 - Squashed the standalone "retry SECURITY PROTOCOL on power-on UNIT
   ATTENTION" and "bounce unaligned frames through a DMA-aligned buffer"
   patches into the UFS RPMB transport patch; the series is now 5 patches.
 - Reused the existing ufshcd_read_desc_param() (now exported) for all
   descriptor reads instead of adding a new ufshcd_read_descriptor()
   wrapper.
 - Moved the SECURITY PROTOCOL IN/OUT opcodes to the generic SCSI header
   as SCSI_SECURITY_PROTOCOL_IN/OUT instead of private UFS defines.
 - Factored the UTF-16BE string-descriptor byte-swap into a
   ufshcd_str_desc_to_cpu() helper.
 - Dropped the ufs_rpmb_get_scsi_dev() wrapper; callers now use
   uclass_get_device(UCLASS_SCSI, ...) directly.

Jorge Ramirez-Ortiz (5):
  ufs: decode string descriptors as UTF-16 big-endian
  ufs: add RPMB transport over SCSI SECURITY PROTOCOL
  ufs: derive the per-region RPMB CID and size for OP-TEE
  optee: rename rpmb.c to rpmb_legacy.c
  optee: implement the RPMB subsystem interface for UFS

 drivers/tee/optee/Makefile               |   3 +-
 drivers/tee/optee/optee_msg_supplicant.h |   8 +
 drivers/tee/optee/optee_private.h        |  41 +++
 drivers/tee/optee/rpmb.c                 | 215 ++++++----------
 drivers/tee/optee/rpmb_legacy.c          | 193 ++++++++++++++
 drivers/tee/optee/supplicant.c           |   9 +
 drivers/ufs/Kconfig                      |  13 +
 drivers/ufs/Makefile                     |   1 +
 drivers/ufs/ufs-rpmb.c                   | 309 +++++++++++++++++++++++
 drivers/ufs/ufs-uclass.c                 |  25 +-
 drivers/ufs/ufs.h                        |   9 +
 include/scsi.h                           |   2 +
 include/ufs.h                            |  12 +
 13 files changed, 692 insertions(+), 148 deletions(-)
 create mode 100644 drivers/tee/optee/rpmb_legacy.c
 create mode 100644 drivers/ufs/ufs-rpmb.c


base-commit: ece349ade2973e220f524ce59e59711cc919263f
-- 
2.54.0

[PATCH v2 1/5] ufs: decode string descriptors as UTF-16 big-endian

[Jorge Ramirez-Ortiz via U-Boot]

UFS string descriptors are UTF-16 big-endian (JESD220), but
ufshcd_read_string_desc() fed the raw bytes to utf16_to_utf8(), which reads
host-endian code units, leaving dev_desc->model blank. Byte-swap to host
order before decoding, matching the kernel.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/ufs-uclass.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index 6a51f337e47..8f120fbbee8 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -1533,6 +1533,19 @@ static inline void ufshcd_remove_non_printable(uint8_t *val)
 		*val = ' ';
 }
 
+static inline void ufshcd_str_desc_to_cpu(u8 *desc, u32 size)
+{
+	u16 *p = (u16 *)&desc[QUERY_DESC_HDR_SIZE];
+	u32 len = desc[QUERY_DESC_LENGTH_OFFSET];
+	u32 i;
+
+	if (len > size)
+		len = size;
+
+	for (i = QUERY_DESC_HDR_SIZE; i + 1 < len; i += 2, p++)
+		*p = be16_to_cpu(*p);
+}
+
 /**
  * ufshcd_uic_pwr_ctrl - executes UIC commands (which affects the link power
  * state) and waits for it to take effect.
@@ -1765,6 +1778,8 @@ static int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 			goto out;
 		}
 
+		ufshcd_str_desc_to_cpu(buf, size);
+
 		/*
 		 * the descriptor contains string in UTF16 format
 		 * we need to convert to utf-8 so it can be displayed
-- 
2.54.0

[PATCH v2 2/5] ufs: add RPMB transport over SCSI SECURITY PROTOCOL

[Jorge Ramirez-Ortiz via U-Boot]

Add a UFS RPMB transport that moves fully-formed JEDEC RPMB frames to and
from the RPMB Well-Known LUN using SCSI SECURITY PROTOCOL IN/OUT commands,
exposed through ufs_rpmb_route_frames() for the OP-TEE RPMB supplicant and
guarded by CONFIG_SUPPORT_UFS_RPMB.

The RPMB WLUN is untouched during boot, so its first SECURITY PROTOCOL
command after power-on returns a power-on UNIT ATTENTION, which
ufs_scsi_exec() surfaces as -EINVAL. U-Boot has no SCSI mid-layer to
auto-retry, so retry the command a few times to avoid losing the first
frame OP-TEE sends. Bounce unaligned frames through an ARCH_DMA_MINALIGN
buffer, since the supplicant may pass a frame the controller cannot DMA
(for example a status-result frame built on the stack).

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/Kconfig      |  13 +++
 drivers/ufs/Makefile     |   1 +
 drivers/ufs/ufs-rpmb.c   | 168 +++++++++++++++++++++++++++++++++++++++
 drivers/ufs/ufs-uclass.c |   6 +-
 drivers/ufs/ufs.h        |   7 ++
 include/scsi.h           |   2 +
 include/ufs.h            |   6 ++
 7 files changed, 200 insertions(+), 3 deletions(-)
 create mode 100644 drivers/ufs/ufs-rpmb.c

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index 49472933de3..d39fcda42dc 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -92,4 +92,17 @@ config UFS_TI_J721E
 	  This selects the glue layer driver for Cadence controller
 	  present on TI's J721E devices.
 
+config SUPPORT_UFS_RPMB
+	bool "Enable UFS RPMB (Replay Protected Memory Block) support"
+	depends on UFS && OPTEE && !SUPPORT_EMMC_RPMB
+	select BLAKE2
+	help
+	  Route OP-TEE RPMB requests to the UFS RPMB Well-Known LUN using
+	  SCSI SECURITY PROTOCOL IN/OUT commands. Required for OP-TEE secure
+	  storage (CFG_RPMB_FS) on UFS-based platforms that have no eMMC.
+	  BLAKE2 is used to derive the fixed-length RPMB CID that matches the
+	  Linux kernel UFS device_id ABI. The OP-TEE supplicant handles a
+	  single RPMB transport, so this is mutually exclusive with the eMMC
+	  RPMB supplicant (SUPPORT_EMMC_RPMB).
+
 endmenu
diff --git a/drivers/ufs/Makefile b/drivers/ufs/Makefile
index e7f3c1d30c4..d60440ca119 100644
--- a/drivers/ufs/Makefile
+++ b/drivers/ufs/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_UFS_RENESAS) += ufs-renesas.o
 obj-$(CONFIG_UFS_RENESAS_GEN5) += ufs-renesas-rcar-gen5.o
 obj-$(CONFIG_UFS_ROCKCHIP) += ufs-rockchip.o
 obj-$(CONFIG_UFS_TI_J721E) += ti-j721e-ufs.o
+obj-$(CONFIG_SUPPORT_UFS_RPMB) += ufs-rpmb.o
diff --git a/drivers/ufs/ufs-rpmb.c b/drivers/ufs/ufs-rpmb.c
new file mode 100644
index 00000000000..5417a4ec0bc
--- /dev/null
+++ b/drivers/ufs/ufs-rpmb.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include <dm.h>
+#include <log.h>
+#include <malloc.h>
+#include <scsi.h>
+#include <ufs.h>
+#include <asm/cache.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include "ufs.h"
+
+#define RPMB_REQ_KEY		1
+#define RPMB_REQ_WCOUNTER	2
+#define RPMB_REQ_WRITE_DATA	3
+#define RPMB_REQ_READ_DATA	4
+#define RPMB_REQ_STATUS		5
+
+#define RPMB_FRAME_SIZE		512
+#define RPMB_FRAME_REQ_OFFSET	510
+
+#define UFS_RPMB_UA_RETRIES	3
+
+#define SEC_PROTOCOL_UFS		0xEC
+#define UFS_RPMB_SEC_PROTOCOL_ID	0x01
+
+#define GEOMETRY_DESC_RPMB_RW_SIZE	0x17
+
+static u16 rpmb_frame_request(const void *frame)
+{
+	const u8 *p = frame;
+
+	return ((u16)p[RPMB_FRAME_REQ_OFFSET] << 8) |
+		p[RPMB_FRAME_REQ_OFFSET + 1];
+}
+
+static int ufs_rpmb_secprot(struct udevice *scsi_dev, unsigned int region,
+			    u8 opcode, void *buf, unsigned int nframes,
+			    enum dma_data_direction dir)
+{
+	struct scsi_cmd pccb;
+	u32 len = nframes * RPMB_FRAME_SIZE;
+	u16 spsp = (region << 8) | UFS_RPMB_SEC_PROTOCOL_ID;
+	void *dma_buf = buf;
+	void *bounce = NULL;
+	int retries;
+	int ret = 0;
+
+	if (!IS_ALIGNED((uintptr_t)buf, ARCH_DMA_MINALIGN)) {
+		bounce = memalign(ARCH_DMA_MINALIGN,
+				  ALIGN(len, ARCH_DMA_MINALIGN));
+		if (!bounce)
+			return -ENOMEM;
+		dma_buf = bounce;
+		if (dir == DMA_TO_DEVICE)
+			memcpy(bounce, buf, len);
+	}
+
+	memset(&pccb, 0, sizeof(pccb));
+	pccb.lun = UFS_UPIU_RPMB_WLUN;
+	pccb.cmd[0] = opcode;
+	pccb.cmd[1] = SEC_PROTOCOL_UFS;
+	pccb.cmd[2] = (spsp >> 8) & 0xff;
+	pccb.cmd[3] = spsp & 0xff;
+	pccb.cmd[4] = 0;
+	pccb.cmd[5] = 0;
+	pccb.cmd[6] = (len >> 24) & 0xff;
+	pccb.cmd[7] = (len >> 16) & 0xff;
+	pccb.cmd[8] = (len >> 8) & 0xff;
+	pccb.cmd[9] = len & 0xff;
+	pccb.cmd[10] = 0;
+	pccb.cmd[11] = 0;
+	pccb.cmdlen = 12;
+	pccb.pdata = dma_buf;
+	pccb.datalen = len;
+	pccb.dma_dir = dir;
+
+	for (retries = UFS_RPMB_UA_RETRIES; retries > 0; retries--) {
+		ret = scsi_exec(scsi_dev, &pccb);
+		if (!ret)
+			break;
+	}
+
+	if (bounce) {
+		if (!ret && dir == DMA_FROM_DEVICE)
+			memcpy(buf, bounce, len);
+		free(bounce);
+	}
+
+	return ret;
+}
+
+static int ufs_rpmb_send(struct udevice *scsi_dev, unsigned int region,
+			 void *frames, unsigned int nframes)
+{
+	return ufs_rpmb_secprot(scsi_dev, region, SCSI_SECURITY_PROTOCOL_OUT,
+				frames, nframes, DMA_TO_DEVICE);
+}
+
+static int ufs_rpmb_recv(struct udevice *scsi_dev, unsigned int region,
+			 void *frames, unsigned int nframes)
+{
+	return ufs_rpmb_secprot(scsi_dev, region, SCSI_SECURITY_PROTOCOL_IN,
+				frames, nframes, DMA_FROM_DEVICE);
+}
+
+int ufs_rpmb_route_frames(struct udevice *scsi_dev, unsigned int region,
+			  void *req, unsigned long reqlen, void *rsp,
+			  unsigned long rsplen)
+{
+	unsigned int n_req = reqlen / RPMB_FRAME_SIZE;
+	unsigned int n_rsp = rsplen / RPMB_FRAME_SIZE;
+	u16 request;
+	int ret;
+
+	if (!scsi_dev || reqlen % RPMB_FRAME_SIZE ||
+	    rsplen % RPMB_FRAME_SIZE || !n_req)
+		return -EINVAL;
+
+	request = rpmb_frame_request(req);
+
+	switch (request) {
+	case RPMB_REQ_KEY:
+	case RPMB_REQ_WRITE_DATA: {
+		u8 status_frame[RPMB_FRAME_SIZE];
+
+		ret = ufs_rpmb_send(scsi_dev, region, req, n_req);
+		if (ret)
+			return ret;
+
+		memset(status_frame, 0, sizeof(status_frame));
+		status_frame[RPMB_FRAME_REQ_OFFSET] = RPMB_REQ_STATUS >> 8;
+		status_frame[RPMB_FRAME_REQ_OFFSET + 1] = RPMB_REQ_STATUS & 0xff;
+		ret = ufs_rpmb_send(scsi_dev, region, status_frame, 1);
+		if (ret)
+			return ret;
+
+		if (n_rsp < 1)
+			return -EINVAL;
+		return ufs_rpmb_recv(scsi_dev, region, rsp, 1);
+	}
+	case RPMB_REQ_WCOUNTER:
+	case RPMB_REQ_READ_DATA:
+		ret = ufs_rpmb_send(scsi_dev, region, req, 1);
+		if (ret)
+			return ret;
+		if (n_rsp < 1)
+			return -EINVAL;
+		return ufs_rpmb_recv(scsi_dev, region, rsp, n_rsp);
+	default:
+		debug("ufs-rpmb: unsupported request 0x%x\n", request);
+		return -EINVAL;
+	}
+}
+
+int ufs_rpmb_read_geometry(struct udevice *scsi_dev, u8 *rpmb_rw_size)
+{
+	struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
+	u8 desc[QUERY_DESC_GEOMETRY_DEF_SIZE];
+	int ret;
+
+	ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_GEOMETRY, 0, 0,
+				     desc, sizeof(desc));
+	if (ret)
+		return ret;
+
+	*rpmb_rw_size = desc[GEOMETRY_DESC_RPMB_RW_SIZE];
+	return 0;
+}
diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index 8f120fbbee8..e320a6dcfbc 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -1457,9 +1457,9 @@ static int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_
  * ufshcd_read_desc_param - read the specified descriptor parameter
  *
  */
-static int ufshcd_read_desc_param(struct ufs_hba *hba, enum desc_idn desc_id,
-				  int desc_index, u8 param_offset,
-				  u8 *param_read_buf, u8 param_size)
+int ufshcd_read_desc_param(struct ufs_hba *hba, enum desc_idn desc_id,
+			   int desc_index, u8 param_offset,
+			   u8 *param_read_buf, u8 param_size)
 {
 	int ret;
 	u8 *desc_buf;
diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index 0f6c93fbce7..9f55735602b 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -10,6 +10,9 @@
 struct udevice;
 
 #define UFS_CDB_SIZE	16
+
+#define UFS_UPIU_RPMB_WLUN		0xC4
+
 #define UPIU_TRANSACTION_UIC_CMD 0x1F
 #define UIC_CMD_SIZE (sizeof(u32) * 4)
 #define RESPONSE_UPIU_SENSE_DATA_LENGTH	18
@@ -809,4 +812,8 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
 
 int ufshcd_probe(struct udevice *dev, struct ufs_hba_ops *hba_ops);
 
+int ufshcd_read_desc_param(struct ufs_hba *hba, enum desc_idn desc_id,
+			   int desc_index, u8 param_offset, u8 *param_read_buf,
+			   u8 param_size);
+
 #endif
diff --git a/include/scsi.h b/include/scsi.h
index 2520a8b8fe6..cc79b6dd4c4 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -183,6 +183,8 @@ struct scsi_cmd {
 #define SCSI_WRITE_LONG	0x3F		/* Write Long (O) */
 #define SCSI_WRITE_SAME	0x41		/* Write Same (O) */
 #define SCSI_UNMAP	0x42		/* Write 10-Byte (MANDATORY) */
+#define SCSI_SECURITY_PROTOCOL_IN	0xA2	/* Security Protocol In (O) */
+#define SCSI_SECURITY_PROTOCOL_OUT	0xB5	/* Security Protocol Out (O) */
 
 /**
  * enum scsi_cmd_phase - current phase of the SCSI protocol
diff --git a/include/ufs.h b/include/ufs.h
index f6e27d90e43..f2e46bc5fbf 100644
--- a/include/ufs.h
+++ b/include/ufs.h
@@ -20,4 +20,10 @@ int ufs_probe(void);
  */
 int ufs_probe_dev(int index);
 
+int ufs_rpmb_route_frames(struct udevice *scsi_dev, unsigned int region,
+			  void *req, unsigned long reqlen, void *rsp,
+			  unsigned long rsplen);
+
+int ufs_rpmb_read_geometry(struct udevice *scsi_dev, u8 *rpmb_rw_size);
+
 #endif
-- 
2.54.0

[PATCH v2 3/5] ufs: derive the per-region RPMB CID and size for OP-TEE

[Jorge Ramirez-Ortiz via U-Boot]

Read the UFS device, unit and geometry descriptors to report each RPMB
region's size and reliable-write count, and build a 16-byte CID by
BLAKE2b-hashing the same device-id string the Linux kernel derives
(ufshcd_create_device_id() plus a "-R<region>" suffix), so OP-TEE derives
an RPMB key that matches Linux. Exposed via ufs_rpmb_get_region_info().

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/ufs-rpmb.c   | 141 +++++++++++++++++++++++++++++++++++++++
 drivers/ufs/ufs-uclass.c |   4 +-
 drivers/ufs/ufs.h        |   2 +
 include/ufs.h            |   6 ++
 4 files changed, 151 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/ufs-rpmb.c b/drivers/ufs/ufs-rpmb.c
index 5417a4ec0bc..49ecb38816b 100644
--- a/drivers/ufs/ufs-rpmb.c
+++ b/drivers/ufs/ufs-rpmb.c
@@ -1,10 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0+
 #include <dm.h>
+#include <hexdump.h>
 #include <log.h>
 #include <malloc.h>
 #include <scsi.h>
 #include <ufs.h>
+#include <vsprintf.h>
+#include <u-boot/blake2.h>
 #include <asm/cache.h>
+#include <asm/unaligned.h>
 #include <linux/errno.h>
 #include <linux/string.h>
 #include "ufs.h"
@@ -25,6 +29,15 @@
 
 #define GEOMETRY_DESC_RPMB_RW_SIZE	0x17
 
+#define RPMB_UNIT_DESC_LOGICAL_BLK_SIZE		0x0A
+#define RPMB_UNIT_DESC_LOGICAL_BLK_COUNT	0x0B
+#define RPMB_UNIT_DESC_REGION0_SIZE		0x13
+#define RPMB_UNIT_DESC_REGION1_SIZE		0x14
+#define RPMB_UNIT_DESC_REGION2_SIZE		0x15
+#define RPMB_UNIT_DESC_REGION3_SIZE		0x16
+#define UFS_RPMB_LEGACY_SPEC_VER		0x0220
+#define UFS_RPMB_REGION_UNIT_SHIFT		17
+
 static u16 rpmb_frame_request(const void *frame)
 {
 	const u8 *p = frame;
@@ -166,3 +179,131 @@ int ufs_rpmb_read_geometry(struct udevice *scsi_dev, u8 *rpmb_rw_size)
 	*rpmb_rw_size = desc[GEOMETRY_DESC_RPMB_RW_SIZE];
 	return 0;
 }
+
+static int ufs_rpmb_read_region_sizes(struct ufs_hba *hba, u16 spec_ver,
+				      u8 sizes[UFS_RPMB_NUM_REGIONS])
+{
+	u8 unit[QUERY_DESC_UNIT_DEF_SIZE] = { };
+	int ret;
+
+	ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT,
+				     UFS_UPIU_RPMB_WLUN, 0, unit, sizeof(unit));
+	if (ret)
+		return ret;
+
+	memset(sizes, 0, UFS_RPMB_NUM_REGIONS);
+
+	if (spec_ver > UFS_RPMB_LEGACY_SPEC_VER) {
+		sizes[0] = unit[RPMB_UNIT_DESC_REGION0_SIZE];
+		sizes[1] = unit[RPMB_UNIT_DESC_REGION1_SIZE];
+		sizes[2] = unit[RPMB_UNIT_DESC_REGION2_SIZE];
+		sizes[3] = unit[RPMB_UNIT_DESC_REGION3_SIZE];
+	} else {
+		u64 region = (get_unaligned_be64(unit +
+					RPMB_UNIT_DESC_LOGICAL_BLK_COUNT)
+			      << unit[RPMB_UNIT_DESC_LOGICAL_BLK_SIZE])
+			     >> UFS_RPMB_REGION_UNIT_SHIFT;
+
+		sizes[0] = region > 0xff ? 0xff : region;
+	}
+
+	return 0;
+}
+
+static void ufs_rpmb_string_to_ascii(const u8 *raw, char *out, size_t outsz)
+{
+	int nchars = ((int)raw[QUERY_DESC_LENGTH_OFFSET] - QUERY_DESC_HDR_SIZE);
+	int i, n = 0;
+
+	nchars = nchars > 0 ? nchars / 2 : 0;
+	for (i = 0; i < nchars && n < (int)outsz - 1; i++) {
+		u16 c = get_unaligned_be16(raw + QUERY_DESC_HDR_SIZE + i * 2);
+
+		out[n++] = (c >= 0x20 && c <= 0x7e) ? (char)c : ' ';
+	}
+	out[n] = '\0';
+}
+
+static int ufs_rpmb_build_cid(struct ufs_hba *hba, const u8 *dev_desc,
+			      unsigned int region, u8 *cid)
+{
+	u8 raw[QUERY_DESC_MAX_SIZE];
+	char model[MAX_MODEL_LEN * 8];
+	char serial_hex[QUERY_DESC_MAX_SIZE * 2 + 1];
+	char idstr[QUERY_DESC_MAX_SIZE * 3];
+	u8 serial[QUERY_DESC_MAX_SIZE] = { };
+	u16 manf_id, spec_ver, dev_ver, manf_date;
+	u8 blen;
+	int ret;
+
+	manf_id = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_MANF_ID);
+	spec_ver = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_SPEC_VER);
+	dev_ver = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_DEV_VER);
+	manf_date = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_MANF_DATE);
+
+	ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_STRING,
+				     dev_desc[DEVICE_DESC_PARAM_PRDCT_NAME], 0,
+				     raw, sizeof(raw));
+	if (ret)
+		return ret;
+	ufs_rpmb_string_to_ascii(raw, model, sizeof(model));
+
+	ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_STRING,
+				     dev_desc[DEVICE_DESC_PARAM_SN], 0,
+				     raw, sizeof(raw));
+	if (ret)
+		return ret;
+	blen = raw[QUERY_DESC_LENGTH_OFFSET];
+	if (blen < QUERY_DESC_HDR_SIZE)
+		return -EINVAL;
+	memcpy(serial, raw + QUERY_DESC_HDR_SIZE, blen - QUERY_DESC_HDR_SIZE);
+	bin2hex(serial_hex, serial, blen);
+	serial_hex[blen * 2] = '\0';
+
+	snprintf(idstr, sizeof(idstr), "%04X-%04X-%s-%s-%04X-%04X-R%u",
+		 manf_id, spec_ver, model, serial_hex, dev_ver, manf_date,
+		 region);
+
+	if (blake2b(cid, UFS_RPMB_CID_SIZE, idstr, strlen(idstr), NULL, 0))
+		return -EIO;
+
+	return 0;
+}
+
+int ufs_rpmb_get_region_info(struct udevice *scsi_dev, unsigned int region,
+			     u8 *size_mult, u8 *rel_wr, u8 *cid)
+{
+	struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
+	u8 dev_desc[QUERY_DESC_DEVICE_DEF_SIZE] = { };
+	u8 sizes[UFS_RPMB_NUM_REGIONS];
+	u16 spec_ver;
+	int ret;
+
+	if (region >= UFS_RPMB_NUM_REGIONS)
+		return 0;
+
+	ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_DEVICE, 0, 0,
+				     dev_desc, sizeof(dev_desc));
+	if (ret)
+		return ret;
+	spec_ver = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_SPEC_VER);
+
+	ret = ufs_rpmb_read_region_sizes(hba, spec_ver, sizes);
+	if (ret)
+		return ret;
+	if (!sizes[region])
+		return 0;
+
+	ret = ufs_rpmb_read_geometry(scsi_dev, rel_wr);
+	if (ret)
+		return ret;
+	if (!*rel_wr)
+		*rel_wr = 1;
+
+	ret = ufs_rpmb_build_cid(hba, dev_desc, region, cid);
+	if (ret)
+		return ret;
+
+	*size_mult = sizes[region];
+	return 1;
+}
diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index e320a6dcfbc..3a0bcd287ea 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -1801,8 +1801,8 @@ out:
 	return err;
 }
 
-static int ufs_get_device_desc(struct ufs_hba *hba,
-			       struct ufs_dev_desc *dev_desc)
+int ufs_get_device_desc(struct ufs_hba *hba,
+			struct ufs_dev_desc *dev_desc)
 {
 	int err;
 	size_t buff_len;
diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index 9f55735602b..5dae8dbef72 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -816,4 +816,6 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, enum desc_idn desc_id,
 			   int desc_index, u8 param_offset, u8 *param_read_buf,
 			   u8 param_size);
 
+int ufs_get_device_desc(struct ufs_hba *hba, struct ufs_dev_desc *dev_desc);
+
 #endif
diff --git a/include/ufs.h b/include/ufs.h
index f2e46bc5fbf..8cdd4724d66 100644
--- a/include/ufs.h
+++ b/include/ufs.h
@@ -20,10 +20,16 @@ int ufs_probe(void);
  */
 int ufs_probe_dev(int index);
 
+#define UFS_RPMB_CID_SIZE	16
+#define UFS_RPMB_NUM_REGIONS	4
+
 int ufs_rpmb_route_frames(struct udevice *scsi_dev, unsigned int region,
 			  void *req, unsigned long reqlen, void *rsp,
 			  unsigned long rsplen);
 
+int ufs_rpmb_get_region_info(struct udevice *scsi_dev, unsigned int region,
+			     u8 *size_mult, u8 *rel_wr, u8 *cid);
+
 int ufs_rpmb_read_geometry(struct udevice *scsi_dev, u8 *rpmb_rw_size);
 
 #endif
-- 
2.54.0

[PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Jorge Ramirez-Ortiz via U-Boot]

This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
name is free for the RPMB subsystem interface added next; pure rename plus
its Makefile object, no functional change.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/tee/optee/Makefile                  | 2 +-
 drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)

diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
index 36ac085ef42..8321cf53a19 100644
--- a/drivers/tee/optee/Makefile
+++ b/drivers/tee/optee/Makefile
@@ -3,4 +3,4 @@
 obj-$(CONFIG_OPTEE) += core.o
 obj-y += supplicant.o
 obj-$(CONFIG_DM_I2C) += i2c.o
-obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
+obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
similarity index 100%
rename from drivers/tee/optee/rpmb.c
rename to drivers/tee/optee/rpmb_legacy.c
-- 
2.54.0

[PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez-Ortiz via U-Boot]

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 <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                 | 124 +++++++++++++++++++++++
 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
+
+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;
+	}
+
+	if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &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;
+	}
+
+	if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &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;
-- 
2.54.0

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Neil Armstrong (Linaro) via U-Boot]

On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
> supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
> name is free for the RPMB subsystem interface added next; pure rename plus
> its Makefile object, no functional change.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---
>   drivers/tee/optee/Makefile                  | 2 +-
>   drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
>   2 files changed, 1 insertion(+), 1 deletion(-)
>   rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
> 
> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> index 36ac085ef42..8321cf53a19 100644
> --- a/drivers/tee/optee/Makefile
> +++ b/drivers/tee/optee/Makefile
> @@ -3,4 +3,4 @@
>   obj-$(CONFIG_OPTEE) += core.o
>   obj-y += supplicant.o
>   obj-$(CONFIG_DM_I2C) += i2c.o
> -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
> +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
> similarity index 100%
> rename from drivers/tee/optee/rpmb.c
> rename to drivers/tee/optee/rpmb_legacy.c

Why not rpmb_emmc ?

Neil

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Neil Armstrong (Linaro) via U-Boot]

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 <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                 | 124 +++++++++++++++++++++++

Why not rpmb_ufs ?

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 ?

> +
> +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;
> +	}
> +
> +	if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &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;
> +	}
> +
> +	if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &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;

Thanks,
Neil

Re: [PATCH v2 3/5] ufs: derive the per-region RPMB CID and size for OP-TEE

[Neil Armstrong (Linaro) via U-Boot]

Hi,

On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> Read the UFS device, unit and geometry descriptors to report each RPMB
> region's size and reliable-write count, and build a 16-byte CID by
> BLAKE2b-hashing the same device-id string the Linux kernel derives
> (ufshcd_create_device_id() plus a "-R<region>" suffix), so OP-TEE derives
> an RPMB key that matches Linux. Exposed via ufs_rpmb_get_region_info().
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---
>   drivers/ufs/ufs-rpmb.c   | 141 +++++++++++++++++++++++++++++++++++++++
>   drivers/ufs/ufs-uclass.c |   4 +-
>   drivers/ufs/ufs.h        |   2 +
>   include/ufs.h            |   6 ++
>   4 files changed, 151 insertions(+), 2 deletions(-)
> 

<snip>

> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> index e320a6dcfbc..3a0bcd287ea 100644
> --- a/drivers/ufs/ufs-uclass.c
> +++ b/drivers/ufs/ufs-uclass.c
> @@ -1801,8 +1801,8 @@ out:
>   	return err;
>   }
>   
> -static int ufs_get_device_desc(struct ufs_hba *hba,
> -			       struct ufs_dev_desc *dev_desc)
> +int ufs_get_device_desc(struct ufs_hba *hba,
> +			struct ufs_dev_desc *dev_desc)

I still don't find where ufs_get_device_desc() is used.

>   {
>   	int err;
>   	size_t buff_len;
> diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
> index 9f55735602b..5dae8dbef72 100644
<snip>

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Jorge Ramirez via U-Boot]

On 22/07/26 10:27:22, neil.armstrong@linaro.org wrote:
> On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> > This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
> > supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
> > name is free for the RPMB subsystem interface added next; pure rename plus
> > its Makefile object, no functional change.
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > ---
> >   drivers/tee/optee/Makefile                  | 2 +-
> >   drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
> >   2 files changed, 1 insertion(+), 1 deletion(-)
> >   rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
> > 
> > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > index 36ac085ef42..8321cf53a19 100644
> > --- a/drivers/tee/optee/Makefile
> > +++ b/drivers/tee/optee/Makefile
> > @@ -3,4 +3,4 @@
> >   obj-$(CONFIG_OPTEE) += core.o
> >   obj-y += supplicant.o
> >   obj-$(CONFIG_DM_I2C) += i2c.o
> > -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
> > +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
> > diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
> > similarity index 100%
> > rename from drivers/tee/optee/rpmb.c
> > rename to drivers/tee/optee/rpmb_legacy.c
> 
> Why not rpmb_emmc ?
> 
> Neil

right this is what is confusing about the current abstraction; op-tee has two modes:

  1. Legacy mode (legacy_operation = true)

  - Uses the original RPC command OPTEE_RPC_CMD_RPMB.
  - No device enumeration — assumes a single, fixed eMMC RPMB device at
  - CFG_RPMB_FS_DEV_ID (dev_type = OPTEE_RPC_RPMB_EMMC).
  - This is the fallback for older supplicants that don't understand the
    probe protocol    
  
  2. Probe / new mode (legacy_operation = false)

  - Uses OPTEE_RPC_CMD_RPMB_PROBE_RESET + OPTEE_RPC_CMD_RPMB_PROBE_NEXT
    to enumerate RPMB devices, then OPTEE_RPC_CMD_RPMB_FRAMES for the
    actual frame I/O.
  - Supports multiple RPMB devices
  - OP-TEE probes all devices first, deferring key provisioning until
    every device is examined.

This PR adds supports for UFS using the new mode of operation that
enumerates devices instead of the legacy mode.

In the meantime eMMC continues to use the legacy mode.

So the super-sets are legacy (for emmc) and probe/default (for UFS and
eventually eMMC/NVMe and so on).

Re: [PATCH v2 3/5] ufs: derive the per-region RPMB CID and size for OP-TEE

[Jorge Ramirez via U-Boot]

On 22/07/26 10:33:29, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> > Read the UFS device, unit and geometry descriptors to report each RPMB
> > region's size and reliable-write count, and build a 16-byte CID by
> > BLAKE2b-hashing the same device-id string the Linux kernel derives
> > (ufshcd_create_device_id() plus a "-R<region>" suffix), so OP-TEE derives
> > an RPMB key that matches Linux. Exposed via ufs_rpmb_get_region_info().
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > ---
> >   drivers/ufs/ufs-rpmb.c   | 141 +++++++++++++++++++++++++++++++++++++++
> >   drivers/ufs/ufs-uclass.c |   4 +-
> >   drivers/ufs/ufs.h        |   2 +
> >   include/ufs.h            |   6 ++
> >   4 files changed, 151 insertions(+), 2 deletions(-)
> > 
> 
> <snip>
> 
> > diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> > index e320a6dcfbc..3a0bcd287ea 100644
> > --- a/drivers/ufs/ufs-uclass.c
> > +++ b/drivers/ufs/ufs-uclass.c
> > @@ -1801,8 +1801,8 @@ out:
> >   	return err;
> >   }
> > -static int ufs_get_device_desc(struct ufs_hba *hba,
> > -			       struct ufs_dev_desc *dev_desc)
> > +int ufs_get_device_desc(struct ufs_hba *hba,
> > +			struct ufs_dev_desc *dev_desc)
> 
> I still don't find where ufs_get_device_desc() is used.

you right, missed it. will fix in v3

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Jorge Ramirez via U-Boot]

On 22/07/26 11:46:00, Jorge Ramirez wrote:
> On 22/07/26 10:27:22, neil.armstrong@linaro.org wrote:
> > On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> > > This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
> > > supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
> > > name is free for the RPMB subsystem interface added next; pure rename plus
> > > its Makefile object, no functional change.
> > > 
> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > ---
> > >   drivers/tee/optee/Makefile                  | 2 +-
> > >   drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
> > >   2 files changed, 1 insertion(+), 1 deletion(-)
> > >   rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
> > > 
> > > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > > index 36ac085ef42..8321cf53a19 100644
> > > --- a/drivers/tee/optee/Makefile
> > > +++ b/drivers/tee/optee/Makefile
> > > @@ -3,4 +3,4 @@
> > >   obj-$(CONFIG_OPTEE) += core.o
> > >   obj-y += supplicant.o
> > >   obj-$(CONFIG_DM_I2C) += i2c.o
> > > -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
> > > +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
> > > diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
> > > similarity index 100%
> > > rename from drivers/tee/optee/rpmb.c
> > > rename to drivers/tee/optee/rpmb_legacy.c
> > 
> > Why not rpmb_emmc ?
> > 
> > Neil
> 
> right this is what is confusing about the current abstraction; op-tee has two modes:
> 
>   1. Legacy mode (legacy_operation = true)
> 
>   - Uses the original RPC command OPTEE_RPC_CMD_RPMB.
>   - No device enumeration — assumes a single, fixed eMMC RPMB device at
>   - CFG_RPMB_FS_DEV_ID (dev_type = OPTEE_RPC_RPMB_EMMC).
>   - This is the fallback for older supplicants that don't understand the
>     probe protocol    
>   
>   2. Probe / new mode (legacy_operation = false)
> 
>   - Uses OPTEE_RPC_CMD_RPMB_PROBE_RESET + OPTEE_RPC_CMD_RPMB_PROBE_NEXT
>     to enumerate RPMB devices, then OPTEE_RPC_CMD_RPMB_FRAMES for the
>     actual frame I/O.
>   - Supports multiple RPMB devices
>   - OP-TEE probes all devices first, deferring key provisioning until
>     every device is examined.
> 
> This PR adds supports for UFS using the new mode of operation that
> enumerates devices instead of the legacy mode.
> 
> In the meantime eMMC continues to use the legacy mode.
> 
> So the super-sets are legacy (for emmc) and probe/default (for UFS and
> eventually eMMC/NVMe and so on).
> 

What the patch tries to say (my intention) is that rpmb_legacy.c should
not change moving forward and rpmb.c and tee should be extended with
eMMC support on the probe path.

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Peter Robinson via U-Boot]

On Wed, 22 Jul 2026 at 10:46, Jorge Ramirez via U-Boot
<u-boot@lists.u-boot-project.org> wrote:
>
> On 22/07/26 10:27:22, neil.armstrong@linaro.org wrote:
> > On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> > > This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
> > > supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
> > > name is free for the RPMB subsystem interface added next; pure rename plus
> > > its Makefile object, no functional change.
> > >
> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > ---
> > >   drivers/tee/optee/Makefile                  | 2 +-
> > >   drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
> > >   2 files changed, 1 insertion(+), 1 deletion(-)
> > >   rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
> > >
> > > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > > index 36ac085ef42..8321cf53a19 100644
> > > --- a/drivers/tee/optee/Makefile
> > > +++ b/drivers/tee/optee/Makefile
> > > @@ -3,4 +3,4 @@
> > >   obj-$(CONFIG_OPTEE) += core.o
> > >   obj-y += supplicant.o
> > >   obj-$(CONFIG_DM_I2C) += i2c.o
> > > -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
> > > +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
> > > diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
> > > similarity index 100%
> > > rename from drivers/tee/optee/rpmb.c
> > > rename to drivers/tee/optee/rpmb_legacy.c
> >
> > Why not rpmb_emmc ?
> >
> > Neil
>
> right this is what is confusing about the current abstraction; op-tee has two modes:
>
>   1. Legacy mode (legacy_operation = true)

Does the spec refer to this as legacy mode? Presumably it's RPMB on
eMMC vs RPMB on UFS (or presumably also NVME) or the revisions have
actual version numbers. Maybe name it after the interface that
supports the revision, or the actual revision of the RPMB spec, as
opposed to just legacy. What happens when next version comes along, to
we rename legacy to old_legacy so we can have new, legacy and
old_legacy?

>   - Uses the original RPC command OPTEE_RPC_CMD_RPMB.
>   - No device enumeration — assumes a single, fixed eMMC RPMB device at
>   - CFG_RPMB_FS_DEV_ID (dev_type = OPTEE_RPC_RPMB_EMMC).
>   - This is the fallback for older supplicants that don't understand the
>     probe protocol
>
>   2. Probe / new mode (legacy_operation = false)
>
>   - Uses OPTEE_RPC_CMD_RPMB_PROBE_RESET + OPTEE_RPC_CMD_RPMB_PROBE_NEXT
>     to enumerate RPMB devices, then OPTEE_RPC_CMD_RPMB_FRAMES for the
>     actual frame I/O.
>   - Supports multiple RPMB devices
>   - OP-TEE probes all devices first, deferring key provisioning until
>     every device is examined.
>
> This PR adds supports for UFS using the new mode of operation that
> enumerates devices instead of the legacy mode.
>
> In the meantime eMMC continues to use the legacy mode.
>
> So the super-sets are legacy (for emmc) and probe/default (for UFS and
> eventually eMMC/NVMe and so on).
>

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Neil Armstrong via U-Boot]

Hi,

On 7/22/26 14:33, Peter Robinson wrote:
> On Wed, 22 Jul 2026 at 10:46, Jorge Ramirez via U-Boot
> <u-boot@lists.u-boot-project.org> wrote:
>>
>> On 22/07/26 10:27:22, neil.armstrong@linaro.org wrote:
>>> On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
>>>> This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
>>>> supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
>>>> name is free for the RPMB subsystem interface added next; pure rename plus
>>>> its Makefile object, no functional change.
>>>>
>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>>> ---
>>>>    drivers/tee/optee/Makefile                  | 2 +-
>>>>    drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
>>>>    2 files changed, 1 insertion(+), 1 deletion(-)
>>>>    rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
>>>>
>>>> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
>>>> index 36ac085ef42..8321cf53a19 100644
>>>> --- a/drivers/tee/optee/Makefile
>>>> +++ b/drivers/tee/optee/Makefile
>>>> @@ -3,4 +3,4 @@
>>>>    obj-$(CONFIG_OPTEE) += core.o
>>>>    obj-y += supplicant.o
>>>>    obj-$(CONFIG_DM_I2C) += i2c.o
>>>> -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>>>> +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
>>>> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
>>>> similarity index 100%
>>>> rename from drivers/tee/optee/rpmb.c
>>>> rename to drivers/tee/optee/rpmb_legacy.c
>>>
>>> Why not rpmb_emmc ?
>>>
>>> Neil
>>
>> right this is what is confusing about the current abstraction; op-tee has two modes:
>>
>>    1. Legacy mode (legacy_operation = true)
> 
> Does the spec refer to this as legacy mode? Presumably it's RPMB on
> eMMC vs RPMB on UFS (or presumably also NVME) or the revisions have
> actual version numbers. Maybe name it after the interface that
> supports the revision, or the actual revision of the RPMB spec, as
> opposed to just legacy. What happens when next version comes along, to
> we rename legacy to old_legacy so we can have new, legacy and
> old_legacy?

So I did a small dig and both commands are still referenced in OP-TEE
and the TEE supplicant.

But indeed the OPTEE_RPC_CMD_RPMB command is only used in the userspace
TEE supplicant _and_ U-Boot, while Linux uses the other RPMB Frames
command like this implementation.

In OPTEE the usage of OPTEE_RPC_CMD_RPMB it's explicited as "legacy" in
https://github.com/OP-TEE/optee_os/blob/991587c721a603e831cad228626078289adad159/core/tee/tee_rpmb_fs.c#L486
so we can describe it as "Legacy", even if it's not described as legacy
on in the OP-TEE headers nor in the TEE supplicant.

But, I'll stick to my proposal to rename as rpmb_emmc and rpmb_ufs and
add a comment in the rpmb_emmc explaining it uses the OPTEE_RPC_CMD_RPMB
and may need to be add support for the FRAMES commands.
And the other way around add a small comment in the UFS implementation
explaining it only supports the FRAMES commands.

Neil

> 
>>    - Uses the original RPC command OPTEE_RPC_CMD_RPMB.
>>    - No device enumeration — assumes a single, fixed eMMC RPMB device at
>>    - CFG_RPMB_FS_DEV_ID (dev_type = OPTEE_RPC_RPMB_EMMC).
>>    - This is the fallback for older supplicants that don't understand the
>>      probe protocol
>>
>>    2. Probe / new mode (legacy_operation = false)
>>
>>    - Uses OPTEE_RPC_CMD_RPMB_PROBE_RESET + OPTEE_RPC_CMD_RPMB_PROBE_NEXT
>>      to enumerate RPMB devices, then OPTEE_RPC_CMD_RPMB_FRAMES for the
>>      actual frame I/O.
>>    - Supports multiple RPMB devices
>>    - OP-TEE probes all devices first, deferring key provisioning until
>>      every device is examined.
>>
>> This PR adds supports for UFS using the new mode of operation that
>> enumerates devices instead of the legacy mode.
>>
>> In the meantime eMMC continues to use the legacy mode.
>>
>> So the super-sets are legacy (for emmc) and probe/default (for UFS and
>> eventually eMMC/NVMe and so on).
>>

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez via U-Boot]

On 22/07/26 10:31:04, neil.armstrong@linaro.org 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 <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                 | 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)

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Jorge Ramirez via U-Boot]

On 22/07/26 15:10:16, Neil Armstrong wrote:
> Hi,
> 
> On 7/22/26 14:33, Peter Robinson wrote:
> > On Wed, 22 Jul 2026 at 10:46, Jorge Ramirez via U-Boot
> > <u-boot@lists.u-boot-project.org> wrote:
> > > 
> > > On 22/07/26 10:27:22, neil.armstrong@linaro.org wrote:
> > > > On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
> > > > > This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
> > > > > supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
> > > > > name is free for the RPMB subsystem interface added next; pure rename plus
> > > > > its Makefile object, no functional change.
> > > > > 
> > > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > > > ---
> > > > >    drivers/tee/optee/Makefile                  | 2 +-
> > > > >    drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
> > > > >    2 files changed, 1 insertion(+), 1 deletion(-)
> > > > >    rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
> > > > > 
> > > > > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > > > > index 36ac085ef42..8321cf53a19 100644
> > > > > --- a/drivers/tee/optee/Makefile
> > > > > +++ b/drivers/tee/optee/Makefile
> > > > > @@ -3,4 +3,4 @@
> > > > >    obj-$(CONFIG_OPTEE) += core.o
> > > > >    obj-y += supplicant.o
> > > > >    obj-$(CONFIG_DM_I2C) += i2c.o
> > > > > -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
> > > > > +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
> > > > > diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
> > > > > similarity index 100%
> > > > > rename from drivers/tee/optee/rpmb.c
> > > > > rename to drivers/tee/optee/rpmb_legacy.c
> > > > 
> > > > Why not rpmb_emmc ?
> > > > 
> > > > Neil
> > > 
> > > right this is what is confusing about the current abstraction; op-tee has two modes:
> > > 
> > >    1. Legacy mode (legacy_operation = true)
> > 
> > Does the spec refer to this as legacy mode? Presumably it's RPMB on
> > eMMC vs RPMB on UFS (or presumably also NVME) or the revisions have
> > actual version numbers. Maybe name it after the interface that
> > supports the revision, or the actual revision of the RPMB spec, as
> > opposed to just legacy. What happens when next version comes along, to
> > we rename legacy to old_legacy so we can have new, legacy and
> > old_legacy?

yeah, Neil addressed this below. I agree with you Peter but sometimes
legacy can be appropiately used (I believed that this to be one of
those binary instances).

> 
> So I did a small dig and both commands are still referenced in OP-TEE
> and the TEE supplicant.
> 
> But indeed the OPTEE_RPC_CMD_RPMB command is only used in the userspace
> TEE supplicant _and_ U-Boot, while Linux uses the other RPMB Frames
> command like this implementation.
> 
> In OPTEE the usage of OPTEE_RPC_CMD_RPMB it's explicited as "legacy" in
> https://github.com/OP-TEE/optee_os/blob/991587c721a603e831cad228626078289adad159/core/tee/tee_rpmb_fs.c#L486
> so we can describe it as "Legacy", even if it's not described as legacy
> on in the OP-TEE headers nor in the TEE supplicant.
> 
> But, I'll stick to my proposal to rename as rpmb_emmc and rpmb_ufs and
> add a comment in the rpmb_emmc explaining it uses the OPTEE_RPC_CMD_RPMB
> and may need to be add support for the FRAMES commands.
> And the other way around add a small comment in the UFS implementation
> explaining it only supports the FRAMES commands.


I'll push back (one last time), my point being that I expect rpmb_legacy
to disappear where as rpmb_mmc or rpmb_ufs will not (and I see no reason
to keep them in separate files): hence the pursue of a single rpmb.c

if after this you still want to separate rpmb_ufs and rpmb_mmc I'll go
ahead (but we will lose that temporary hint to the op-tee dependency)

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Neil Armstrong via U-Boot]

On 7/22/26 16:12, Jorge Ramirez wrote:
> On 22/07/26 10:31:04, neil.armstrong@linaro.org 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 <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                 | 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).

Let's see in the future, if somehow there's need for a common "new"
and emmc only legacy some rework will be needed anyway. The eMMC RPMB
hasn't been updated for a very long time...

Using the driver model to register an rpmb device attached to an eMMC
or UFS controller would the cleanest way to handle that, but we're not here now.

> 
>>
>> 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)

It could collide with other SCSI devices like SATA devices over PCIe.

Usually we try not to hardcode the devices ids.

Neil


> 
> 
> 

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez via U-Boot]

On 22/07/26 17:43:16, Neil Armstrong wrote:
> On 7/22/26 16:12, Jorge Ramirez wrote:
> > On 22/07/26 10:31:04, neil.armstrong@linaro.org 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 <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                 | 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).
> 
> Let's see in the future, if somehow there's need for a common "new"
> and emmc only legacy some rework will be needed anyway. The eMMC RPMB
> hasn't been updated for a very long time...
> 
> Using the driver model to register an rpmb device attached to an eMMC
> or UFS controller would the cleanest way to handle that, but we're not here now.
> 
> > 
> > > 
> > > 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)
> 
> It could collide with other SCSI devices like SATA devices over PCIe.
> 
> Usually we try not to hardcode the devices ids.


right...shall I go ahead with a DT alias approach?

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez via U-Boot]

On 22/07/26 20:12:19, Jorge Ramirez wrote:
> On 22/07/26 17:43:16, Neil Armstrong wrote:
> > On 7/22/26 16:12, Jorge Ramirez wrote:
> > > On 22/07/26 10:31:04, neil.armstrong@linaro.org 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 <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                 | 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).
> > 
> > Let's see in the future, if somehow there's need for a common "new"
> > and emmc only legacy some rework will be needed anyway. The eMMC RPMB
> > hasn't been updated for a very long time...
> > 
> > Using the driver model to register an rpmb device attached to an eMMC
> > or UFS controller would the cleanest way to handle that, but we're not here now.
> > 
> > > 
> > > > 
> > > > 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)
> > 
> > It could collide with other SCSI devices like SATA devices over PCIe.
> > 
> > Usually we try not to hardcode the devices ids.
> 
> 
> right...shall I go ahead with a DT alias approach?


something like this?


  8 ⋮  8 │ #include "optee_msg_supplicant.h"
  9 ⋮  9 │ #include "optee_private.h"
 10 ⋮ 10 │ 
 11 ⋮    │-#define UFS_RPMB_CONTROLLER        0
    ⋮ 11 │+static int optee_rpmb_get_scsi_dev(struct udevice **scsi_devp)
    ⋮ 12 │+{
    ⋮ 13 │+        struct udevice *ufs_dev, *scsi_dev;
    ⋮ 14 │+        ofnode node;
    ⋮ 15 │+        int ret;
    ⋮ 16 │+
    ⋮ 17 │+        node = ofnode_get_aliases_node("ufs-rpmb");
    ⋮ 18 │+        if (ofnode_valid(node))
    ⋮ 19 │+                ret = uclass_get_device_by_ofnode(UCLASS_UFS, node, &ufs_dev);
    ⋮ 20 │+        else
    ⋮ 21 │+                ret = uclass_get_device(UCLASS_UFS, 0, &ufs_dev);
    ⋮ 22 │+        if (ret)
    ⋮ 23 │+                return ret;
    ⋮ 24 │+
    ⋮ 25 │+        ret = device_get_child(ufs_dev, 0, &scsi_dev);
    ⋮ 26 │+        if (ret)
    ⋮ 27 │+                return ret;
    ⋮ 28 │+
    ⋮ 29 │+        *scsi_devp = scsi_dev;
    ⋮ 30 │+
    ⋮ 31 │+        return 0;
    ⋮ 32 │+}
 12 ⋮ 33 │ 
 13 ⋮ 34 │ void optee_suppl_cmd_rpmb_probe_reset(struct udevice *dev,
 14 ⋮ 35 │                                       struct optee_msg_arg *arg)

────────────────────────────────────────────────────────────────┐
• 76: void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev, │
────────────────────────────────────────────────────────────────┘
 55 ⋮ 76 │                 return;
 56 ⋮ 77 │         }
 57 ⋮ 78 │ 
 58 ⋮    │-        if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &scsi_dev)) {
    ⋮ 79 │+        if (optee_rpmb_get_scsi_dev(&scsi_dev)) {
 59 ⋮ 80 │                 arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
 60 ⋮ 81 │                 return;
 61 ⋮ 82 │         }

─────────────────────────────────────────────────────────────┐
• 124: void optee_suppl_cmd_rpmb_frames(struct udevice *dev, │
─────────────────────────────────────────────────────────────┘
 103⋮ 124│                 return;
 104⋮ 125│         }
 105⋮ 126│ 
 106⋮    │-        if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &scsi_dev)) {
    ⋮ 127│+        if (optee_rpmb_get_scsi_dev(&scsi_dev)) {
 107⋮ 128│                 arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
 108⋮ 129│                 return;
 109⋮ 130│         }

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Neil Armstrong]

On 7/22/26 20:44, Jorge Ramirez wrote:
> On 22/07/26 20:12:19, Jorge Ramirez wrote:
>> On 22/07/26 17:43:16, Neil Armstrong wrote:
>>> On 7/22/26 16:12, Jorge Ramirez wrote:
>>>> On 22/07/26 10:31:04, neil.armstrong@linaro.org 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 <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                 | 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).
>>>
>>> Let's see in the future, if somehow there's need for a common "new"
>>> and emmc only legacy some rework will be needed anyway. The eMMC RPMB
>>> hasn't been updated for a very long time...
>>>
>>> Using the driver model to register an rpmb device attached to an eMMC
>>> or UFS controller would the cleanest way to handle that, but we're not here now.
>>>
>>>>
>>>>>
>>>>> 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)
>>>
>>> It could collide with other SCSI devices like SATA devices over PCIe.
>>>
>>> Usually we try not to hardcode the devices ids.
>>
>>
>> right...shall I go ahead with a DT alias approach?
> 
> 
> something like this?
> 
> 
>    8 ⋮  8 │ #include "optee_msg_supplicant.h"
>    9 ⋮  9 │ #include "optee_private.h"
>   10 ⋮ 10 │
>   11 ⋮    │-#define UFS_RPMB_CONTROLLER        0
>      ⋮ 11 │+static int optee_rpmb_get_scsi_dev(struct udevice **scsi_devp)
>      ⋮ 12 │+{
>      ⋮ 13 │+        struct udevice *ufs_dev, *scsi_dev;
>      ⋮ 14 │+        ofnode node;
>      ⋮ 15 │+        int ret;
>      ⋮ 16 │+
>      ⋮ 17 │+        node = ofnode_get_aliases_node("ufs-rpmb");
>      ⋮ 18 │+        if (ofnode_valid(node))
>      ⋮ 19 │+                ret = uclass_get_device_by_ofnode(UCLASS_UFS, node, &ufs_dev);
>      ⋮ 20 │+        else
>      ⋮ 21 │+                ret = uclass_get_device(UCLASS_UFS, 0, &ufs_dev);
>      ⋮ 22 │+        if (ret)
>      ⋮ 23 │+                return ret;
>      ⋮ 24 │+
>      ⋮ 25 │+        ret = device_get_child(ufs_dev, 0, &scsi_dev);
>      ⋮ 26 │+        if (ret)
>      ⋮ 27 │+                return ret;
>      ⋮ 28 │+
>      ⋮ 29 │+        *scsi_devp = scsi_dev;
>      ⋮ 30 │+
>      ⋮ 31 │+        return 0;
>      ⋮ 32 │+}
>   12 ⋮ 33 │
>   13 ⋮ 34 │ void optee_suppl_cmd_rpmb_probe_reset(struct udevice *dev,
>   14 ⋮ 35 │                                       struct optee_msg_arg *arg)
> 
> ────────────────────────────────────────────────────────────────┐
> • 76: void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev, │
> ────────────────────────────────────────────────────────────────┘
>   55 ⋮ 76 │                 return;
>   56 ⋮ 77 │         }
>   57 ⋮ 78 │
>   58 ⋮    │-        if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &scsi_dev)) {
>      ⋮ 79 │+        if (optee_rpmb_get_scsi_dev(&scsi_dev)) {
>   59 ⋮ 80 │                 arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
>   60 ⋮ 81 │                 return;
>   61 ⋮ 82 │         }
> 
> ─────────────────────────────────────────────────────────────┐
> • 124: void optee_suppl_cmd_rpmb_frames(struct udevice *dev, │
> ─────────────────────────────────────────────────────────────┘
>   103⋮ 124│                 return;
>   104⋮ 125│         }
>   105⋮ 126│
>   106⋮    │-        if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &scsi_dev)) {
>      ⋮ 127│+        if (optee_rpmb_get_scsi_dev(&scsi_dev)) {
>   107⋮ 128│                 arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
>   108⋮ 129│                 return;
>   109⋮ 130│         }
> 

No I was just thinking as a Kconfig option along CONFIG_SUPPORT_UFS_RPMB but
I just didn't find where you added the CONFIG_SUPPORT_UFS_RPMB option...

Neil

Re: [PATCH v2 5/5] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez via U-Boot]

On 23/07/26 09:33:39, Neil Armstrong wrote:
> On 7/22/26 20:44, Jorge Ramirez wrote:
> > On 22/07/26 20:12:19, Jorge Ramirez wrote:
> > > On 22/07/26 17:43:16, Neil Armstrong wrote:
> > > > On 7/22/26 16:12, Jorge Ramirez wrote:
> > > > > On 22/07/26 10:31:04, neil.armstrong@linaro.org 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 <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                 | 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).
> > > > 
> > > > Let's see in the future, if somehow there's need for a common "new"
> > > > and emmc only legacy some rework will be needed anyway. The eMMC RPMB
> > > > hasn't been updated for a very long time...
> > > > 
> > > > Using the driver model to register an rpmb device attached to an eMMC
> > > > or UFS controller would the cleanest way to handle that, but we're not here now.
> > > > 
> > > > > 
> > > > > > 
> > > > > > 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)
> > > > 
> > > > It could collide with other SCSI devices like SATA devices over PCIe.
> > > > 
> > > > Usually we try not to hardcode the devices ids.
> > > 
> > > 
> > > right...shall I go ahead with a DT alias approach?
> > 
> > 
> > something like this?
> > 
> > 
> >    8 ⋮  8 │ #include "optee_msg_supplicant.h"
> >    9 ⋮  9 │ #include "optee_private.h"
> >   10 ⋮ 10 │
> >   11 ⋮    │-#define UFS_RPMB_CONTROLLER        0
> >      ⋮ 11 │+static int optee_rpmb_get_scsi_dev(struct udevice **scsi_devp)
> >      ⋮ 12 │+{
> >      ⋮ 13 │+        struct udevice *ufs_dev, *scsi_dev;
> >      ⋮ 14 │+        ofnode node;
> >      ⋮ 15 │+        int ret;
> >      ⋮ 16 │+
> >      ⋮ 17 │+        node = ofnode_get_aliases_node("ufs-rpmb");
> >      ⋮ 18 │+        if (ofnode_valid(node))
> >      ⋮ 19 │+                ret = uclass_get_device_by_ofnode(UCLASS_UFS, node, &ufs_dev);
> >      ⋮ 20 │+        else
> >      ⋮ 21 │+                ret = uclass_get_device(UCLASS_UFS, 0, &ufs_dev);
> >      ⋮ 22 │+        if (ret)
> >      ⋮ 23 │+                return ret;
> >      ⋮ 24 │+
> >      ⋮ 25 │+        ret = device_get_child(ufs_dev, 0, &scsi_dev);
> >      ⋮ 26 │+        if (ret)
> >      ⋮ 27 │+                return ret;
> >      ⋮ 28 │+
> >      ⋮ 29 │+        *scsi_devp = scsi_dev;
> >      ⋮ 30 │+
> >      ⋮ 31 │+        return 0;
> >      ⋮ 32 │+}
> >   12 ⋮ 33 │
> >   13 ⋮ 34 │ void optee_suppl_cmd_rpmb_probe_reset(struct udevice *dev,
> >   14 ⋮ 35 │                                       struct optee_msg_arg *arg)
> > 
> > ────────────────────────────────────────────────────────────────┐
> > • 76: void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev, │
> > ────────────────────────────────────────────────────────────────┘
> >   55 ⋮ 76 │                 return;
> >   56 ⋮ 77 │         }
> >   57 ⋮ 78 │
> >   58 ⋮    │-        if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &scsi_dev)) {
> >      ⋮ 79 │+        if (optee_rpmb_get_scsi_dev(&scsi_dev)) {
> >   59 ⋮ 80 │                 arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
> >   60 ⋮ 81 │                 return;
> >   61 ⋮ 82 │         }
> > 
> > ─────────────────────────────────────────────────────────────┐
> > • 124: void optee_suppl_cmd_rpmb_frames(struct udevice *dev, │
> > ─────────────────────────────────────────────────────────────┘
> >   103⋮ 124│                 return;
> >   104⋮ 125│         }
> >   105⋮ 126│
> >   106⋮    │-        if (uclass_get_device(UCLASS_SCSI, UFS_RPMB_CONTROLLER, &scsi_dev)) {
> >      ⋮ 127│+        if (optee_rpmb_get_scsi_dev(&scsi_dev)) {
> >   107⋮ 128│                 arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
> >   108⋮ 129│                 return;
> >   109⋮ 130│         }
> > 
> 
> No I was just thinking as a Kconfig option along CONFIG_SUPPORT_UFS_RPMB but
> I just didn't find where you added the CONFIG_SUPPORT_UFS_RPMB option...
> 
> Neil

well ok, sure I can do the Kconfig change.

I havent enabled RPMB just yet in configs/qcom_lemans_defconfig until
the OP-TEE support is merged.

This is what I have on my development branch:

# Route OP-TEE RPMB secure storage over the UFS RPMB (no eMMC on this SoC).                                        
CONFIG_SUPPORT_UFS_RPMB=y

Re: [PATCH v2 4/5] optee: rename rpmb.c to rpmb_legacy.c

[Neil Armstrong]

On 7/22/26 16:35, Jorge Ramirez wrote:
> On 22/07/26 15:10:16, Neil Armstrong wrote:
>> Hi,
>>
>> On 7/22/26 14:33, Peter Robinson wrote:
>>> On Wed, 22 Jul 2026 at 10:46, Jorge Ramirez via U-Boot
>>> <u-boot@lists.u-boot-project.org> wrote:
>>>>
>>>> On 22/07/26 10:27:22, neil.armstrong@linaro.org wrote:
>>>>> On 7/22/26 08:07, Jorge Ramirez-Ortiz wrote:
>>>>>> This file implements the legacy single-command OPTEE_MSG_RPC_CMD_RPMB
>>>>>> supplicant (eMMC only). Rename it to rpmb_legacy.c so the primary rpmb.c
>>>>>> name is free for the RPMB subsystem interface added next; pure rename plus
>>>>>> its Makefile object, no functional change.
>>>>>>
>>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>>>>> ---
>>>>>>     drivers/tee/optee/Makefile                  | 2 +-
>>>>>>     drivers/tee/optee/{rpmb.c => rpmb_legacy.c} | 0
>>>>>>     2 files changed, 1 insertion(+), 1 deletion(-)
>>>>>>     rename drivers/tee/optee/{rpmb.c => rpmb_legacy.c} (100%)
>>>>>>
>>>>>> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
>>>>>> index 36ac085ef42..8321cf53a19 100644
>>>>>> --- a/drivers/tee/optee/Makefile
>>>>>> +++ b/drivers/tee/optee/Makefile
>>>>>> @@ -3,4 +3,4 @@
>>>>>>     obj-$(CONFIG_OPTEE) += core.o
>>>>>>     obj-y += supplicant.o
>>>>>>     obj-$(CONFIG_DM_I2C) += i2c.o
>>>>>> -obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>>>>>> +obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb_legacy.o
>>>>>> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_legacy.c
>>>>>> similarity index 100%
>>>>>> rename from drivers/tee/optee/rpmb.c
>>>>>> rename to drivers/tee/optee/rpmb_legacy.c
>>>>>
>>>>> Why not rpmb_emmc ?
>>>>>
>>>>> Neil
>>>>
>>>> right this is what is confusing about the current abstraction; op-tee has two modes:
>>>>
>>>>     1. Legacy mode (legacy_operation = true)
>>>
>>> Does the spec refer to this as legacy mode? Presumably it's RPMB on
>>> eMMC vs RPMB on UFS (or presumably also NVME) or the revisions have
>>> actual version numbers. Maybe name it after the interface that
>>> supports the revision, or the actual revision of the RPMB spec, as
>>> opposed to just legacy. What happens when next version comes along, to
>>> we rename legacy to old_legacy so we can have new, legacy and
>>> old_legacy?
> 
> yeah, Neil addressed this below. I agree with you Peter but sometimes
> legacy can be appropiately used (I believed that this to be one of
> those binary instances).
> 
>>
>> So I did a small dig and both commands are still referenced in OP-TEE
>> and the TEE supplicant.
>>
>> But indeed the OPTEE_RPC_CMD_RPMB command is only used in the userspace
>> TEE supplicant _and_ U-Boot, while Linux uses the other RPMB Frames
>> command like this implementation.
>>
>> In OPTEE the usage of OPTEE_RPC_CMD_RPMB it's explicited as "legacy" in
>> https://github.com/OP-TEE/optee_os/blob/991587c721a603e831cad228626078289adad159/core/tee/tee_rpmb_fs.c#L486
>> so we can describe it as "Legacy", even if it's not described as legacy
>> on in the OP-TEE headers nor in the TEE supplicant.
>>
>> But, I'll stick to my proposal to rename as rpmb_emmc and rpmb_ufs and
>> add a comment in the rpmb_emmc explaining it uses the OPTEE_RPC_CMD_RPMB
>> and may need to be add support for the FRAMES commands.
>> And the other way around add a small comment in the UFS implementation
>> explaining it only supports the FRAMES commands.
> 
> 
> I'll push back (one last time), my point being that I expect rpmb_legacy
> to disappear where as rpmb_mmc or rpmb_ufs will not (and I see no reason
> to keep them in separate files): hence the pursue of a single rpmb.c
> 
> if after this you still want to separate rpmb_ufs and rpmb_mmc I'll go
> ahead (but we will lose that temporary hint to the op-tee dependency)
> 

Well I'm not sure it's "legacy" as we speak because it's very probable
some OP-TEE binaries used in production right now uses the OPTEE_RPC_CMD_RPMB,
so ultimately we need to support both on any storages to support any OP-TEE
versions.

My point is after your changeset, one file is hardcoding eMMC support and
the other is hardcoding UFS and there's no dynamic Device Model way to
dynamically register an RPMB partition for any storage.

So ultimately, this should be implemented using Driver Model and the
eMMC code should live in eMMC subsystem, same for UFS, by registering
an RPMB subdevice like this is done on the Linux side.

But for now, rename the file relative to it's content to avoid any confusion.

Neil