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

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

[Jorge Ramirez-Ortiz]

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_emmc.c, with the new UFS backend added as a separate
rpmb_ufs.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). A small preparatory patch surfaces the
RESPONSE UPIU sense data from ufs_scsi_exec() so the transport can retry
only on a power-on UNIT ATTENTION; the transport patch also bounces
unaligned frames through a DMA-aligned buffer 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

v5:
 - ufs: decode string descriptors: added a Fixes: tag; the EFI
   file_open() callers now pass UTF16_LITTLE_ENDIAN (UEFI CHAR16 / FAT
   paths are little-endian by origin) instead of UTF16_HOST_ENDIAN.
   UTF16_HOST_ENDIAN is kept in the enum to mirror the kernel's
   utf16s_to_utf8s().
 - ufs: new patch "surface sense data from ufs_scsi_exec()": copy the
   RESPONSE UPIU sense data into the SCSI command so callers can read the
   sense key. No current caller inspects it, so behaviour is unchanged.
 - ufs: RPMB transport: retry the SECURITY PROTOCOL only on a UNIT
   ATTENTION sense key (using the surfaced sense data) rather than on any
   error; build the status frame with put_unaligned_be16(); move
   GEOMETRY_DESC_RPMB_RW_SIZE to the per-region CID patch that first uses
   it.
 - Series is now 6 patches (was 5).

v4:
 - ufs: decode string descriptors: dropped the in-place
   ufshcd_str_desc_to_cpu() byte-swap helper; instead added an endian
   argument to utf16_to_utf8() (UTF16_HOST/LITTLE/BIG_ENDIAN) and decode
   with UTF16_BIG_ENDIAN, mirroring the kernel's utf16s_to_utf8s().
   Existing EFI callers pass UTF16_HOST_ENDIAN.
 - ufs: RPMB transport: build the SECURITY PROTOCOL CDB with
   put_unaligned_be16()/put_unaligned_be32(); drop the
   rpmb_frame_request() helper in favour of get_unaligned_be16(); move
   ufs_rpmb_read_geometry() to the patch that first uses it so it is not
   an unused static function during git bisect.
 - ufs: per-region CID/size: reject an out-of-range device-reported
   logical block size before shifting and split the size computation into
   separate statements for readability; order <u-boot/...> after
   <linux/...>; note that the serial hex encoding matches the kernel
   device-id ABI.

v3:
 - Renamed the legacy eMMC supplicant to rpmb_emmc.c (was rpmb_legacy.c)
   and kept it as a pure 100% rename: the UFS RPMB subsystem backend now
   lives in its own rpmb_ufs.c instead of being folded into rpmb.c, so
   the eMMC path is left byte-for-byte unchanged.
 - Added CONFIG_UFS_RPMB_CONTROLLER to select the RPMB-owning UFS
   controller on boards with more than one UFS controller.
 - Reworked the commit messages to lead with the motivation rather than
   the implementation detail.

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 (6):
  ufs: decode string descriptors as UTF-16 big-endian
  ufs: surface sense data from ufs_scsi_exec()
  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_emmc.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 => rpmb_emmc.c} |   0
 drivers/tee/optee/rpmb_ufs.c              | 141 +++++++++
 drivers/tee/optee/supplicant.c            |   9 +
 drivers/ufs/Kconfig                       |  21 ++
 drivers/ufs/Makefile                      |   1 +
 drivers/ufs/ufs-rpmb.c                    | 347 ++++++++++++++++++++++
 drivers/ufs/ufs-uclass.c                  |  38 ++-
 drivers/ufs/ufs.h                         |   7 +
 include/charset.h                         |  17 +-
 include/scsi.h                            |   2 +
 include/ufs.h                             |  10 +
 lib/charset.c                             |  15 +-
 lib/efi_loader/efi_file.c                 |   4 +-
 16 files changed, 652 insertions(+), 12 deletions(-)
 rename drivers/tee/optee/{rpmb.c => rpmb_emmc.c} (100%)
 create mode 100644 drivers/tee/optee/rpmb_ufs.c
 create mode 100644 drivers/ufs/ufs-rpmb.c


base-commit: b8e0086b585791a771416ce51b58ce0097bc0b28
-- 
2.54.0

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

[Jorge Ramirez-Ortiz]

UFS string descriptors are UTF-16 big-endian (JESD220), but
ufshcd_read_string_desc() decoded them as host-endian and so left
dev_desc->model blank.

Give utf16_to_utf8() the source byte order and decode UFS descriptors as
big-endian, matching the kernel's utf16s_to_utf8s(). The EFI file-path
callers consume little-endian UEFI strings and are marked accordingly.

Fixes: 067c1b033282 ("ufs: Call ufs_scsi_bind() from uclass .post_bind")
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/ufs-uclass.c  |  7 ++++---
 include/charset.h         | 17 ++++++++++++++++-
 lib/charset.c             | 15 ++++++++++++++-
 lib/efi_loader/efi_file.c |  4 ++--
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index 6ef01f8e4aa..21aed9f97bd 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -1767,11 +1767,12 @@ static int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 		}
 
 		/*
-		 * the descriptor contains string in UTF16 format
-		 * we need to convert to utf-8 so it can be displayed
+		 * the descriptor contains a big-endian UTF-16 string, convert
+		 * it to utf-8 so it can be displayed
 		 */
 		utf16_to_utf8(buff_ascii,
-			      (uint16_t *)&buf[QUERY_DESC_HDR_SIZE], ascii_len);
+			      (uint16_t *)&buf[QUERY_DESC_HDR_SIZE], ascii_len,
+			      UTF16_BIG_ENDIAN);
 
 		/* replace non-printable or non-ASCII characters with spaces */
 		for (i = 0; i < ascii_len; i++)
diff --git a/include/charset.h b/include/charset.h
index 348bad5883a..79d0ae1ef49 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -13,6 +13,19 @@
 
 #define MAX_UTF8_PER_UTF16 3
 
+/**
+ * enum utf16_endian - byte order of a UTF-16 string
+ *
+ * @UTF16_HOST_ENDIAN:	code units are in host byte order
+ * @UTF16_LITTLE_ENDIAN:	code units are little-endian
+ * @UTF16_BIG_ENDIAN:	code units are big-endian
+ */
+enum utf16_endian {
+	UTF16_HOST_ENDIAN,
+	UTF16_LITTLE_ENDIAN,
+	UTF16_BIG_ENDIAN,
+};
+
 /*
  * codepage_437 - Unicode to codepage 437 translation table
  */
@@ -299,9 +312,11 @@ size_t u16_strlcat(u16 *dest, const u16 *src, size_t count);
  * @dest:	the destination buffer to write the utf8 characters
  * @src:	the source utf16 string
  * @size:	the number of utf16 characters to convert
+ * @endian:	byte order of the code units in 'src'
  * Return:	the pointer to the first unwritten byte in 'dest'
  */
-uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size);
+u8 *utf16_to_utf8(u8 *dest, const u16 *src, size_t size,
+		  enum utf16_endian endian);
 
 /**
  * utf_to_cp() - translate Unicode code point to 8bit codepage
diff --git a/lib/charset.c b/lib/charset.c
index 182c92a50c4..8ccac4048c0 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -5,6 +5,7 @@
  *  Copyright (c) 2017 Rob Clark
  */
 
+#include <asm/byteorder.h>
 #include <charset.h>
 #include <capitalization.h>
 #include <cp437.h>
@@ -458,13 +459,25 @@ size_t u16_strlcat(u16 *dest, const u16 *src, size_t count)
 }
 
 /* Convert UTF-16 to UTF-8.  */
-uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size)
+u8 *utf16_to_utf8(u8 *dest, const u16 *src, size_t size,
+		  enum utf16_endian endian)
 {
 	uint32_t code_high = 0;
 
 	while (size--) {
 		uint32_t code = *src++;
 
+		switch (endian) {
+		case UTF16_LITTLE_ENDIAN:
+			code = le16_to_cpu(code);
+			break;
+		case UTF16_BIG_ENDIAN:
+			code = be16_to_cpu(code);
+			break;
+		case UTF16_HOST_ENDIAN:
+			break;
+		}
+
 		if (code_high) {
 			if (code >= 0xDC00 && code <= 0xDFFF) {
 				/* Surrogate pair.  */
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index 19b43c4a625..7dee57d594a 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -184,7 +184,7 @@ static struct efi_file_handle *file_open(struct file_system *fs,
 	int flen = 0;
 
 	if (file_name) {
-		utf16_to_utf8((u8 *)f0, file_name, 1);
+		utf16_to_utf8((u8 *)f0, file_name, 1, UTF16_LITTLE_ENDIAN);
 		flen = u16_strlen(file_name);
 	}
 
@@ -216,7 +216,7 @@ static struct efi_file_handle *file_open(struct file_system *fs,
 			*p++ = '/';
 		}
 
-		utf16_to_utf8((u8 *)p, file_name, flen);
+		utf16_to_utf8((u8 *)p, file_name, flen, UTF16_LITTLE_ENDIAN);
 
 		if (sanitize_path(fh->path))
 			goto error;

base-commit: b8e0086b585791a771416ce51b58ce0097bc0b28
-- 
2.54.0

[PATCH v5 2/6] ufs: surface sense data from ufs_scsi_exec()

[Jorge Ramirez-Ortiz]

On a CHECK CONDITION the UFS device returns SCSI sense data inline in the
RESPONSE UPIU, but ufs_scsi_exec() discarded it and reported every
failure as a bare -EINVAL, leaving callers unable to tell a transient
condition such as a power-on UNIT ATTENTION from a hard error.

Preserve the sense data and SCSI status in the scsi_cmd. No caller
inspects them yet, so behaviour is unchanged.

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

diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index 21aed9f97bd..3d96a05281c 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -1657,6 +1657,22 @@ static void prepare_prdt_table(struct ufs_hba *hba, struct scsi_cmd *pccb)
 	ufshcd_cache_flush(req_desc, sizeof(*req_desc));
 }
 
+/*
+ * On a CHECK CONDITION the sense data is returned inline in the RESPONSE
+ * UPIU; copy it into the SCSI command so callers can inspect the sense key
+ * (for example to tell a transient UNIT ATTENTION from a hard error).
+ */
+static void ufs_scsi_copy_sense(struct ufs_hba *hba, struct scsi_cmd *pccb)
+{
+	u16 len = be16_to_cpu(hba->ucd_rsp_ptr->sr.sense_data_len);
+
+	if (len > RESPONSE_UPIU_SENSE_DATA_LENGTH)
+		len = RESPONSE_UPIU_SENSE_DATA_LENGTH;
+
+	memcpy(pccb->sense_buf, hba->ucd_rsp_ptr->sr.sense_data, len);
+	pccb->sensedatalen = len;
+}
+
 static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
 {
 	struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
@@ -1664,6 +1680,9 @@ static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
 	int ocs, result = 0;
 	u8 scsi_status;
 
+	pccb->status = 0;
+	pccb->sensedatalen = 0;
+
 	ufshcd_prepare_req_desc_hdr(hba, &upiu_flags, pccb->dma_dir);
 	ufshcd_prepare_utp_scsi_cmd_upiu(hba, pccb, upiu_flags);
 	prepare_prdt_table(hba, pccb);
@@ -1683,8 +1702,12 @@ static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
 			result = ufshcd_get_rsp_upiu_result(hba->ucd_rsp_ptr);
 
 			scsi_status = result & MASK_SCSI_STATUS;
-			if (scsi_status)
+			if (scsi_status) {
+				pccb->status = scsi_status;
+				if (scsi_status == S_CHECK_COND)
+					ufs_scsi_copy_sense(hba, pccb);
 				return -EINVAL;
+			}
 
 			break;
 		case UPIU_TRANSACTION_REJECT_UPIU:
-- 
2.54.0

[PATCH v5 3/6] ufs: add RPMB transport over SCSI SECURITY PROTOCOL

[Jorge Ramirez-Ortiz]

OP-TEE secure storage (CFG_RPMB_FS) needs RPMB, but on UFS-only platforms
with no eMMC the OP-TEE RPMB supplicant cannot reach the device's RPMB
Well-Known LUN: UFS exposes RPMB through SCSI SECURITY PROTOCOL IN/OUT
rather than an eMMC-style partition. Add that transport so RPMB-backed
secure storage works on UFS platforms.

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

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index 6dffdc00b32..a0067b1f203 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -92,4 +92,14 @@ 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
+	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.
+	  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..e6e0b2aec33
--- /dev/null
+++ b/drivers/ufs/ufs-rpmb.c
@@ -0,0 +1,167 @@
+// 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 <asm/unaligned.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
+
+static u8 ufs_rpmb_sense_key(const struct scsi_cmd *pccb)
+{
+	const u8 *sense = pccb->sense_buf;
+
+	if (!pccb->sensedatalen)
+		return SENSE_NO_SENSE;
+
+	/* fixed (0x70/0x71) vs descriptor (0x72/0x73) sense format */
+	if ((sense[0] & 0x7f) == 0x72 || (sense[0] & 0x7f) == 0x73)
+		return sense[1] & 0xf;
+
+	return sense[2] & 0xf;
+}
+
+static int ufs_rpmb_secprot(struct udevice *scsi_dev, unsigned int region,
+			    u8 opcode, void *buf, unsigned int nframes,
+			    enum dma_data_direction dir)
+{
+	u16 spsp = (region << 8) | UFS_RPMB_SEC_PROTOCOL_ID;
+	u32 len = nframes * RPMB_FRAME_SIZE;
+	struct scsi_cmd pccb;
+	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;
+	put_unaligned_be16(spsp, &pccb.cmd[2]);
+	pccb.cmd[4] = 0;
+	pccb.cmd[5] = 0;
+	put_unaligned_be32(len, &pccb.cmd[6]);
+	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;
+
+		/*
+		 * A power-on or reset UNIT ATTENTION is a transient
+		 * condition the device clears once it has been reported,
+		 * so retry it. Any other failure is final: stop and
+		 * report it.
+		 */
+		if (ufs_rpmb_sense_key(&pccb) != SENSE_UNIT_ATTENTION)
+			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 = get_unaligned_be16((u8 *)req + RPMB_FRAME_REQ_OFFSET);
+
+	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));
+		put_unaligned_be16(RPMB_REQ_STATUS,
+				   &status_frame[RPMB_FRAME_REQ_OFFSET]);
+		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;
+	}
+}
diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
index 3d96a05281c..2ca4fb2a020 100644
--- a/drivers/ufs/ufs-uclass.c
+++ b/drivers/ufs/ufs-uclass.c
@@ -1458,9 +1458,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..1bec3ce73a4 100644
--- a/include/ufs.h
+++ b/include/ufs.h
@@ -20,4 +20,8 @@ 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);
+
 #endif
-- 
2.54.0

[PATCH v5 4/6] ufs: derive the per-region RPMB CID and size for OP-TEE

[Jorge Ramirez-Ortiz]

OP-TEE derives its RPMB authentication key from a device identifier, so
U-Boot must supply the exact identifier the Linux kernel uses; otherwise
the keys differ and OP-TEE cannot access RPMB storage provisioned under
Linux, or vice versa. The RPMB probe also needs each region's size and
reliable-write count to enumerate the regions. Provide both so secure
storage stays interoperable between U-Boot and Linux on the same device.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
 drivers/ufs/Kconfig    |   7 +-
 drivers/ufs/ufs-rpmb.c | 181 +++++++++++++++++++++++++++++++++++++++++
 include/ufs.h          |   6 ++
 3 files changed, 192 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index a0067b1f203..648b58b46e4 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -95,11 +95,14 @@ config UFS_TI_J721E
 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.
-	  The OP-TEE supplicant handles a single RPMB transport, so this is
-	  mutually exclusive with the eMMC RPMB supplicant (SUPPORT_EMMC_RPMB).
+	  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/ufs-rpmb.c b/drivers/ufs/ufs-rpmb.c
index e6e0b2aec33..18230786abe 100644
--- a/drivers/ufs/ufs-rpmb.c
+++ b/drivers/ufs/ufs-rpmb.c
@@ -1,13 +1,16 @@
 // 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 <asm/cache.h>
 #include <asm/unaligned.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <u-boot/blake2.h>
 #include "ufs.h"
 
 #define RPMB_REQ_KEY		1
@@ -24,6 +27,17 @@
 #define SEC_PROTOCOL_UFS		0xEC
 #define UFS_RPMB_SEC_PROTOCOL_ID	0x01
 
+#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 u8 ufs_rpmb_sense_key(const struct scsi_cmd *pccb)
 {
 	const u8 *sense = pccb->sense_buf;
@@ -165,3 +179,170 @@ int ufs_rpmb_route_frames(struct udevice *scsi_dev, unsigned int region,
 		return -EINVAL;
 	}
 }
+
+static 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;
+}
+
+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 {
+		u8 blk_shift = unit[RPMB_UNIT_DESC_LOGICAL_BLK_SIZE];
+		u64 region;
+
+		/*
+		 * bLogicalBlockSize is a device-supplied log2 block size;
+		 * reject values that would make the shift below undefined.
+		 */
+		if (blk_shift >= 64)
+			return -EINVAL;
+
+		region = get_unaligned_be64(unit +
+					    RPMB_UNIT_DESC_LOGICAL_BLK_COUNT);
+		region <<= blk_shift;
+		region >>= 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)
+{
+	char serial_hex[QUERY_DESC_MAX_SIZE * 2 + 1];
+	u16 manf_id, spec_ver, dev_ver, manf_date;
+	u8 serial[QUERY_DESC_MAX_SIZE] = { };
+	char idstr[QUERY_DESC_MAX_SIZE * 3];
+	char model[MAX_MODEL_LEN * 8];
+	u8 raw[QUERY_DESC_MAX_SIZE];
+	u8 blen;
+	int ret;
+
+	manf_date = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_MANF_DATE);
+	spec_ver = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_SPEC_VER);
+	manf_id = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_MANF_ID);
+	dev_ver = get_unaligned_be16(dev_desc + DEVICE_DESC_PARAM_DEV_VER);
+
+	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);
+	/*
+	 * The identifier format is an ABI shared with the secure world. The
+	 * kernel's ufshcd_create_device_id() hex-encodes the full descriptor
+	 * length (header included), so the serial always carries two trailing
+	 * zero bytes; reproduce that here so the derived CID matches.
+	 */
+	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/include/ufs.h b/include/ufs.h
index 1bec3ce73a4..a8b7d370f78 100644
--- a/include/ufs.h
+++ b/include/ufs.h
@@ -20,8 +20,14 @@ 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);
+
 #endif
-- 
2.54.0

[PATCH v5 5/6] optee: rename rpmb.c to rpmb_emmc.c

[Jorge Ramirez-Ortiz]

rpmb.c implements the legacy single-command RPMB supplicant, which is
eMMC-only. Rename it to rpmb_emmc.c to make room for a UFS backend.

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_emmc.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/tee/optee/{rpmb.c => rpmb_emmc.c} (100%)

diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
index 36ac085ef42..d0c7f8dfb12 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_emmc.o
diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_emmc.c
similarity index 100%
rename from drivers/tee/optee/rpmb.c
rename to drivers/tee/optee/rpmb_emmc.c
-- 
2.54.0

[PATCH v5 6/6] optee: implement the RPMB subsystem interface for UFS

[Jorge Ramirez-Ortiz]

The legacy RPMB supplicant assumes an eMMC device 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. eMMC keeps using the legacy interface.

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_ufs.c             | 141 +++++++++++++++++++++++
 drivers/tee/optee/supplicant.c           |   9 ++
 drivers/ufs/Kconfig                      |   8 ++
 6 files changed, 208 insertions(+)
 create mode 100644 drivers/tee/optee/rpmb_ufs.c

diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
index d0c7f8dfb12..f2af10999a9 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_emmc.o
+obj-$(CONFIG_SUPPORT_UFS_RPMB) += rpmb_ufs.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_ufs.c b/drivers/tee/optee/rpmb_ufs.c
new file mode 100644
index 00000000000..3e54a1a37d7
--- /dev/null
+++ b/drivers/tee/optee/rpmb_ufs.c
@@ -0,0 +1,141 @@
+// 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"
+
+static int optee_rpmb_get_dev(struct udevice **scsi_devp)
+{
+	struct udevice *ufs_dev, *scsi_dev;
+	int ret;
+
+	ret = uclass_get_device(UCLASS_UFS, CONFIG_UFS_RPMB_CONTROLLER,
+				&ufs_dev);
+	if (ret)
+		return ret;
+
+	ret = device_get_child(ufs_dev, 0, &scsi_dev);
+	if (ret)
+		return ret;
+
+	*scsi_devp = scsi_dev;
+
+	return 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 (optee_rpmb_get_dev(&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 (optee_rpmb_get_dev(&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;
diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index 648b58b46e4..f174ca0841d 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -105,4 +105,12 @@ config SUPPORT_UFS_RPMB
 	  single RPMB transport, so this is mutually exclusive with the eMMC
 	  RPMB supplicant (SUPPORT_EMMC_RPMB).
 
+config UFS_RPMB_CONTROLLER
+	int "UFS controller index used for RPMB"
+	depends on SUPPORT_UFS_RPMB
+	default 0
+	help
+	  Index of the RPMB-owning UFS controller in the UCLASS_UFS device
+	  list. Leave at 0 unless the board has more than one UFS controller.
+
 endmenu
-- 
2.54.0

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Jorge Ramirez]

On 18/09/26 22:06:32, Jorge Ramirez-Ortiz wrote:
> 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_emmc.c, with the new UFS backend added as a separate
> rpmb_ufs.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). A small preparatory patch surfaces the
> RESPONSE UPIU sense data from ufs_scsi_exec() so the transport can retry
> only on a power-on UNIT ATTENTION; the transport patch also bounces
> unaligned frames through a DMA-aligned buffer 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/
accepted in v4 --> https://lkml.org/lkml/2026/8/27/369

> Op-tee
>  https://github.com/OP-TEE/optee_os/pull/7881
and also following Ilias review https://github.com/OP-TEE/optee_os/pull/8007 (both merged)

For reference, at this point Linux can access RPMB and just u-boot pending on this set.

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Macpaul Lin (林智斌)]

[-- Attachment #1: Type: text/plain, Size: 13858 bytes --]

On 18/09/26 22:06:32, Jorge Ramirez-Ortiz wrote:
> 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_emmc.c, with the new UFS backend added as a separate
> rpmb_ufs.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). A small preparatory patch surfaces the
> RESPONSE UPIU sense data from ufs_scsi_exec() so the transport can retry
> only on a power-on UNIT ATTENTION; the transport patch also bounces
> unaligned frames through a DMA-aligned buffer 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/
accepted in v4 --> https://lkml.org/lkml/2026/8/27/369

> Op-tee
>  https://urldefense.com/v3/__https://github.com/OP-TEE/optee_os/pull/7881__;!!CTRNKA9wMg0ARbw!lQ5D9IUE1D5erqbaXCklFua6JiijZYB6XN0q5LDFZZpiaGmrH_k10rLkcjU1nSzhsRDX--J49Gt22E8U73lx7Es567TUAxwX$
and also following Ilias review https://urldefense.com/v3/__https://github.com/OP-TEE/optee_os/pull/8007__;!!CTRNKA9wMg0ARbw!lQ5D9IUE1D5erqbaXCklFua6JiijZYB6XN0q5LDFZZpiaGmrH_k10rLkcjU1nSzhsRDX--J49Gt22E8U73lx7Es565JETGdQ$  (both merged)

> For reference, at this point Linux can access RPMB and just u-boot pending on this set.
I'd like to have some discussion in more detail as below, it will be quite long. However, my company's
email server and mail clients both have problem again as usual. I'm not sure if this reply could be send
out correctly and in correct format. Let's see. 
Moreover, I've invited Kernel's maintainers Stanley Jhu and Bean Huo to this mail thread to see if the opinion increases more compatibility. 
But, I'm not sure how to invite OPTEE's maintainers, I'll try to forward the following description to OPTEE later.

So.. let's start from patch 4/6 and then review the whole story.
4/6 — ufs: derive the per-region RPMB CID and size for OP-TEE

Patch 4/6 hard-codes both the hash algorithm (BLAKE2b) and the output
length (UFS_RPMB_CID_SIZE = 16) used to derive the RPMB CID for UFS
devices. Neither choice is mandated by any specification the series
builds on (UFS/JESD220, RPMB, or the OP-TEE/TEE Client API), and
hard-coding them creates a real compatibility risk for products that
have already shipped with a different, vendor-specific derivation. The
CID derivation should be factored out as a vendor-specific, build-time
selectable component rather than a single fixed algorithm baked into
common UFS core code.

1. Neither the UFS, RPMB, nor TEE specifications mandate a hash
   algorithm for CID derivation
- The UFS specification (JESD220) defines the Device/Unit/Geometry
  descriptors that patch 4/6 reads (manufacturer ID, spec version,
  model string, serial number, device version, manufacture date,
  region sizes) — it says nothing about combining them into a "CID",
  and certainly nothing about which hash function to use.
- The RPMB frame protocol itself (struct rpmb_frame in
  include/linux/rpmb.h, and the equivalent JEDEC RPMB frame layout)
  has no CID field at all — CID is purely a host-side/secure-world
  key management convention, not part of the wire protocol.
- The GlobalPlatform TEE Client API / OP-TEE's own RPMB subsystem RPC
  contract only defines that a fixed-size opaque byte buffer is passed
  as the device identifier — it does not specify how that buffer's
  contents are derived from the underlying storage device.
- Because no specification prescribes the algorithm, this is
  legitimately vendor-specific territory, and different vendors — or
  even the same vendor across product generations — may reasonably
  choose different KDFs/hash functions (BLAKE2b, SHA-256, a
  vendor-proprietary hash, or a raw non-hashed encoding, as seen in
  other real-world implementations of this same feature). The patch
  set should not force one specific vendor's choice into common UFS
  core code as the only option.

2. UFS RPMB does not mandate a 16-byte CID; the size should be variable

- The 16-byte length is not derived from any UFS or RPMB requirement.
  It comes from the eMMC CID register (JEDEC JESD84 Card/Device
  Identification Register), which is a fixed 128-bit hardware
  register — a purely eMMC-specific hardware fact.
- Linux kernel's own generic RPMB abstraction (struct rpmb_descr in
  include/linux/rpmb.h) already models this correctly and treats the
  device identifier as variable-length by design:

  struct rpmb_descr {
          enum rpmb_type type;
          int (*route_frames)(struct device *dev, u8 *req, unsigned int req_len,
                              u8 *resp, unsigned int resp_len);
          u8 *dev_id;
          size_t dev_id_len;   /* variable, not fixed at 16 */
          u16 reliable_wr_count;
          u16 capacity;
  };

  Likewise, the UFS-specific device identifier the Linux kernel builds
  in ufshcd_create_device_id() is a variable-length ASCII string
  ("%04X-%04X-%s-%s-%04X-%04X"), not a fixed 16-byte value.
- Patch 4/6 should follow the same pattern: UFS_RPMB_CID_SIZE should
  not be a hard #define 16 baked into include/ufs.h; the CID buffer
  size should be a property of the selected vendor backend (e.g.
  returned alongside the derived CID, or declared per-backend), so
  that a 16-byte output is one valid choice among others rather than
  an assumed universal constant.

3. The vendor-specific part should be factored out; 16 bytes is only
   there for eMMC compatibility, and UFS has no such hardware CID at all

- To restate the core problem plainly: UFS has no CID register.
  There is no piece of UFS hardware that patch 4/6's
  ufs_rpmb_build_cid() is "reading" — it is entirely a software
  construction. The only reason the result is squeezed into 16 bytes
  is to fit the pre-existing OP-TEE RPC buffer size that was
  originally designed around eMMC's real, hardware-defined 16-byte CID
  register.
- Conflating "the UFS-specific descriptor-reading/string-building
  logic" with "the specific hash function and output length choice"
  in a single non-configurable function means every downstream user of
  this series inherits one vendor's algorithm choice with no way to
  opt out, even though the two concerns are logically separable:
  - Reading UFS descriptors and building the input identifier string
    is common, spec-driven logic that can stay in shared UFS core
    code.
  - Turning that string into the actual CID bytes handed to OP-TEE is
    the vendor-specific part and should be pluggable.
- Suggested restructuring: keep ufshcd-side descriptor reads and
  string-building in common code, but expose the final
  string-to-CID step as a replaceable hook/weak function (or a small
  vendor ops table), so the BLAKE2b implementation in this series
  becomes the default backend rather than the only backend.

4. Hard-coding one algorithm creates real firmware-upgrade and
   key-compatibility risk for already-shipping products, so the
   derivation must be vendor-selectable at build time (via defconfig)

5. Since neither the algorithm (point 1) nor the output length (point 2)
  is specified by any standard, and CID is purely a software
  convention invented to reuse an eMMC-shaped OP-TEE ABI (point 3),
  different implementations that already exist today have made
  different, incompatible choices for the same problem:
  - This series (upstream v5, patch 4/6): builds an identifier string
    from Device Descriptor fields (manufacturer ID, spec version,
    model, serial, device version, manufacture date) and hashes it
    with BLAKE2b to a fixed 16-byte output.
  - At least one other real, shipping downstream implementation (a
    vendor's production U-Boot branch analyzed alongside this series)
    derives the CID by directly decoding/porting the device's
    tee-supplicant-side read_cid()-style logic — i.e. no hash at all,
    just a direct byte encoding of the serial-number string
    descriptor.
  - Both approaches are equally "valid" in the sense that neither
    violates any specification, because none exists to violate — but
    they are mutually incompatible: the same physical UFS device will
    produce a different 16-byte value under each scheme, and
    therefore OP-TEE will derive a different final RPMB authentication
    key under each scheme.
- The practical consequence: any product that has already shipped in
  the field with one CID-derivation scheme (whether that is this
  series's BLAKE2b approach or a vendor's own pre-existing scheme) has
  effectively burned that scheme into its provisioned RPMB key
  material. If a firmware update later replaces the bootloader/OP-TEE
  RPMB glue with a build that hard-codes a different CID derivation
  (for example, adopting this patch set's BLAKE2b implementation
  unconditionally on top of a product that shipped with a different
  vendor scheme), the newly-derived CID will not match the CID used
  when the device's RPMB key material was originally provisioned. The
  device will then either:
  - fail to authenticate against its own RPMB partition (OP-TEE
    derives a key that doesn't match what was provisioned), or
  - silently begin deriving a different key than before, which is
    just as bad if any secure-storage data was provisioned under the
    old key and must remain readable after the upgrade.

  Because CID derivation happens deep inside firmware/secure-world
  boot code with no user-visible error surface until the RPMB HMAC
  check actually fails, this class of bug is essentially undetectable
  in normal testing and only surfaces as field failures after an
  upgrade — exactly the "silent, undetectable until it fails" failure
  mode already called out in the CID appendix.
- Therefore, the fix is not merely a documentation/comment change:
  the patch set should ship this concern as a build-time (Kconfig /
  defconfig) selectable option, e.g.:

  config UFS_RPMB_CID_ALGO_BLAKE2B
          string "UFS RPMB CID derivation algorithm"
          depends on SUPPORT_UFS_RPMB
          default "blake2b"
          help
            Selects which algorithm derives the per-region RPMB CID
            from UFS device descriptors. This value is a software
            convention with no basis in the UFS or RPMB specification
            (UFS has no hardware CID register); it exists only to
            populate the fixed-size CID field inherited from the
            eMMC-oriented OP-TEE RPMB ABI. Products that have already
            shipped with a specific CID derivation MUST keep using
            that same derivation across firmware updates, or the
            derived RPMB authentication key will change and existing
            RPMB-backed secure storage will become unreadable.

  with the BLAKE2b implementation in this series wired in as one
  selectable backend (CONFIG_UFS_RPMB_CID_ALGO_BLAKE2B), so that a
  vendor already shipping a different scheme can select their own
  backend at defconfig time instead of being forced onto this
  series's default and breaking already-provisioned devices on
  upgrade.

Each vendor should be able to upstream their own algo and more.
And these algo should be configurable by the users or by board makers.
For example, MediaTek and Ubuntu are using different algo and CID length for
UFS RPMB.

Here we suggest to make some change to increase the compatibility.

1. Split ufs_rpmb_build_cid() into (a) a common helper that reads the
   UFS descriptors and builds the identifier string, and (b) a
   pluggable string-to-CID backend, with BLAKE2b as the default
   backend rather than the only one.
2. Make the CID buffer size a variable size, or at least property returned by the selected
   backend rather than a fixed UFS_RPMB_CID_SIZE = 16 constant in
   common code.
3. Add a Kconfig choice (e.g. UFS_RPMB_CID_ALGO_BLAKE2) so vendors can
   select their existing/legacy CID derivation at defconfig time.
4. Document explicitly, in code comments and Kconfig help text, that
   this value is a software ABI convention with no basis in the UFS
   or RPMB specification, and that changing it on an already-shipped
   product is a breaking change to that product's RPMB key material.

Thanks
Macpaul Lin

[-- Attachment #2: Type: text/html, Size: 24442 bytes --]

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Ilias Apalodimas]

Thanks for the detailed analysis.

I am pulling in Jens for OP-TEE and I'll have a closer look next week as well.

/Ilias

On Thu, 24 Sept 2026 at 20:50, Macpaul Lin (林智斌)
<Macpaul.Lin@mediatek.com> wrote:
>
> On 18/09/26 22:06:32, Jorge Ramirez-Ortiz wrote:
> > 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_emmc.c, with the new UFS backend added as a separate
> > rpmb_ufs.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). A small preparatory patch surfaces the
> > RESPONSE UPIU sense data from ufs_scsi_exec() so the transport can retry
> > only on a power-on UNIT ATTENTION; the transport patch also bounces
> > unaligned frames through a DMA-aligned buffer 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/
> accepted in v4 --> https://lkml.org/lkml/2026/8/27/369
>
> > Op-tee
> >  https://urldefense.com/v3/__https://github.com/OP-TEE/optee_os/pull/7881__;!!CTRNKA9wMg0ARbw!lQ5D9IUE1D5erqbaXCklFua6JiijZYB6XN0q5LDFZZpiaGmrH_k10rLkcjU1nSzhsRDX--J49Gt22E8U73lx7Es567TUAxwX$
> and also following Ilias review https://urldefense.com/v3/__https://github.com/OP-TEE/optee_os/pull/8007__;!!CTRNKA9wMg0ARbw!lQ5D9IUE1D5erqbaXCklFua6JiijZYB6XN0q5LDFZZpiaGmrH_k10rLkcjU1nSzhsRDX--J49Gt22E8U73lx7Es565JETGdQ$  (both merged)
>
> > For reference, at this point Linux can access RPMB and just u-boot pending on this set.
> I'd like to have some discussion in more detail as below, it will be quite long. However, my company's
> email server and mail clients both have problem again as usual. I'm not sure if this reply could be send
> out correctly and in correct format. Let's see.
> Moreover, I've invited Kernel's maintainers Stanley Jhu and Bean Huo to this mail thread to see if the opinion increases more compatibility.
> But, I'm not sure how to invite OPTEE's maintainers, I'll try to forward the following description to OPTEE later.
>
> So.. let's start from patch 4/6 and then review the whole story.
> 4/6 — ufs: derive the per-region RPMB CID and size for OP-TEE
>
> Patch 4/6 hard-codes both the hash algorithm (BLAKE2b) and the output
> length (UFS_RPMB_CID_SIZE = 16) used to derive the RPMB CID for UFS
> devices. Neither choice is mandated by any specification the series
> builds on (UFS/JESD220, RPMB, or the OP-TEE/TEE Client API), and
> hard-coding them creates a real compatibility risk for products that
> have already shipped with a different, vendor-specific derivation. The
> CID derivation should be factored out as a vendor-specific, build-time
> selectable component rather than a single fixed algorithm baked into
> common UFS core code.
>
> 1. Neither the UFS, RPMB, nor TEE specifications mandate a hash
>    algorithm for CID derivation
> - The UFS specification (JESD220) defines the Device/Unit/Geometry
>   descriptors that patch 4/6 reads (manufacturer ID, spec version,
>   model string, serial number, device version, manufacture date,
>   region sizes) — it says nothing about combining them into a "CID",
>   and certainly nothing about which hash function to use.
> - The RPMB frame protocol itself (struct rpmb_frame in
>   include/linux/rpmb.h, and the equivalent JEDEC RPMB frame layout)
>   has no CID field at all — CID is purely a host-side/secure-world
>   key management convention, not part of the wire protocol.
> - The GlobalPlatform TEE Client API / OP-TEE's own RPMB subsystem RPC
>   contract only defines that a fixed-size opaque byte buffer is passed
>   as the device identifier — it does not specify how that buffer's
>   contents are derived from the underlying storage device.
> - Because no specification prescribes the algorithm, this is
>   legitimately vendor-specific territory, and different vendors — or
>   even the same vendor across product generations — may reasonably
>   choose different KDFs/hash functions (BLAKE2b, SHA-256, a
>   vendor-proprietary hash, or a raw non-hashed encoding, as seen in
>   other real-world implementations of this same feature). The patch
>   set should not force one specific vendor's choice into common UFS
>   core code as the only option.
>
> 2. UFS RPMB does not mandate a 16-byte CID; the size should be variable
>
> - The 16-byte length is not derived from any UFS or RPMB requirement.
>   It comes from the eMMC CID register (JEDEC JESD84 Card/Device
>   Identification Register), which is a fixed 128-bit hardware
>   register — a purely eMMC-specific hardware fact.
> - Linux kernel's own generic RPMB abstraction (struct rpmb_descr in
>   include/linux/rpmb.h) already models this correctly and treats the
>   device identifier as variable-length by design:
>
>   struct rpmb_descr {
>           enum rpmb_type type;
>           int (*route_frames)(struct device *dev, u8 *req, unsigned int req_len,
>                               u8 *resp, unsigned int resp_len);
>           u8 *dev_id;
>           size_t dev_id_len;   /* variable, not fixed at 16 */
>           u16 reliable_wr_count;
>           u16 capacity;
>   };
>
>   Likewise, the UFS-specific device identifier the Linux kernel builds
>   in ufshcd_create_device_id() is a variable-length ASCII string
>   ("%04X-%04X-%s-%s-%04X-%04X"), not a fixed 16-byte value.
> - Patch 4/6 should follow the same pattern: UFS_RPMB_CID_SIZE should
>   not be a hard #define 16 baked into include/ufs.h; the CID buffer
>   size should be a property of the selected vendor backend (e.g.
>   returned alongside the derived CID, or declared per-backend), so
>   that a 16-byte output is one valid choice among others rather than
>   an assumed universal constant.
>
> 3. The vendor-specific part should be factored out; 16 bytes is only
>    there for eMMC compatibility, and UFS has no such hardware CID at all
>
> - To restate the core problem plainly: UFS has no CID register.
>   There is no piece of UFS hardware that patch 4/6's
>   ufs_rpmb_build_cid() is "reading" — it is entirely a software
>   construction. The only reason the result is squeezed into 16 bytes
>   is to fit the pre-existing OP-TEE RPC buffer size that was
>   originally designed around eMMC's real, hardware-defined 16-byte CID
>   register.
> - Conflating "the UFS-specific descriptor-reading/string-building
>   logic" with "the specific hash function and output length choice"
>   in a single non-configurable function means every downstream user of
>   this series inherits one vendor's algorithm choice with no way to
>   opt out, even though the two concerns are logically separable:
>   - Reading UFS descriptors and building the input identifier string
>     is common, spec-driven logic that can stay in shared UFS core
>     code.
>   - Turning that string into the actual CID bytes handed to OP-TEE is
>     the vendor-specific part and should be pluggable.
> - Suggested restructuring: keep ufshcd-side descriptor reads and
>   string-building in common code, but expose the final
>   string-to-CID step as a replaceable hook/weak function (or a small
>   vendor ops table), so the BLAKE2b implementation in this series
>   becomes the default backend rather than the only backend.
>
> 4. Hard-coding one algorithm creates real firmware-upgrade and
>    key-compatibility risk for already-shipping products, so the
>    derivation must be vendor-selectable at build time (via defconfig)
>
> 5. Since neither the algorithm (point 1) nor the output length (point 2)
>   is specified by any standard, and CID is purely a software
>   convention invented to reuse an eMMC-shaped OP-TEE ABI (point 3),
>   different implementations that already exist today have made
>   different, incompatible choices for the same problem:
>   - This series (upstream v5, patch 4/6): builds an identifier string
>     from Device Descriptor fields (manufacturer ID, spec version,
>     model, serial, device version, manufacture date) and hashes it
>     with BLAKE2b to a fixed 16-byte output.
>   - At least one other real, shipping downstream implementation (a
>     vendor's production U-Boot branch analyzed alongside this series)
>     derives the CID by directly decoding/porting the device's
>     tee-supplicant-side read_cid()-style logic — i.e. no hash at all,
>     just a direct byte encoding of the serial-number string
>     descriptor.
>   - Both approaches are equally "valid" in the sense that neither
>     violates any specification, because none exists to violate — but
>     they are mutually incompatible: the same physical UFS device will
>     produce a different 16-byte value under each scheme, and
>     therefore OP-TEE will derive a different final RPMB authentication
>     key under each scheme.
> - The practical consequence: any product that has already shipped in
>   the field with one CID-derivation scheme (whether that is this
>   series's BLAKE2b approach or a vendor's own pre-existing scheme) has
>   effectively burned that scheme into its provisioned RPMB key
>   material. If a firmware update later replaces the bootloader/OP-TEE
>   RPMB glue with a build that hard-codes a different CID derivation
>   (for example, adopting this patch set's BLAKE2b implementation
>   unconditionally on top of a product that shipped with a different
>   vendor scheme), the newly-derived CID will not match the CID used
>   when the device's RPMB key material was originally provisioned. The
>   device will then either:
>   - fail to authenticate against its own RPMB partition (OP-TEE
>     derives a key that doesn't match what was provisioned), or
>   - silently begin deriving a different key than before, which is
>     just as bad if any secure-storage data was provisioned under the
>     old key and must remain readable after the upgrade.
>
>   Because CID derivation happens deep inside firmware/secure-world
>   boot code with no user-visible error surface until the RPMB HMAC
>   check actually fails, this class of bug is essentially undetectable
>   in normal testing and only surfaces as field failures after an
>   upgrade — exactly the "silent, undetectable until it fails" failure
>   mode already called out in the CID appendix.
> - Therefore, the fix is not merely a documentation/comment change:
>   the patch set should ship this concern as a build-time (Kconfig /
>   defconfig) selectable option, e.g.:
>
>   config UFS_RPMB_CID_ALGO_BLAKE2B
>           string "UFS RPMB CID derivation algorithm"
>           depends on SUPPORT_UFS_RPMB
>           default "blake2b"
>           help
>             Selects which algorithm derives the per-region RPMB CID
>             from UFS device descriptors. This value is a software
>             convention with no basis in the UFS or RPMB specification
>             (UFS has no hardware CID register); it exists only to
>             populate the fixed-size CID field inherited from the
>             eMMC-oriented OP-TEE RPMB ABI. Products that have already
>             shipped with a specific CID derivation MUST keep using
>             that same derivation across firmware updates, or the
>             derived RPMB authentication key will change and existing
>             RPMB-backed secure storage will become unreadable.
>
>   with the BLAKE2b implementation in this series wired in as one
>   selectable backend (CONFIG_UFS_RPMB_CID_ALGO_BLAKE2B), so that a
>   vendor already shipping a different scheme can select their own
>   backend at defconfig time instead of being forced onto this
>   series's default and breaking already-provisioned devices on
>   upgrade.
>
> Each vendor should be able to upstream their own algo and more.
> And these algo should be configurable by the users or by board makers.
> For example, MediaTek and Ubuntu are using different algo and CID length for
> UFS RPMB.
>
> Here we suggest to make some change to increase the compatibility.
>
> 1. Split ufs_rpmb_build_cid() into (a) a common helper that reads the
>    UFS descriptors and builds the identifier string, and (b) a
>    pluggable string-to-CID backend, with BLAKE2b as the default
>    backend rather than the only one.
> 2. Make the CID buffer size a variable size, or at least property returned by the selected
>    backend rather than a fixed UFS_RPMB_CID_SIZE = 16 constant in
>    common code.
> 3. Add a Kconfig choice (e.g. UFS_RPMB_CID_ALGO_BLAKE2) so vendors can
>    select their existing/legacy CID derivation at defconfig time.
> 4. Document explicitly, in code comments and Kconfig help text, that
>    this value is a software ABI convention with no basis in the UFS
>    or RPMB specification, and that changing it on an already-shipped
>    product is a breaking change to that product's RPMB key material.
>
> Thanks
> Macpaul Lin

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Jorge Ramirez]

On 25/09/26 12:59:28, Ilias Apalodimas wrote:
> Thanks for the detailed analysis.
>
> I am pulling in Jens for OP-TEE and I'll have a closer look next week as well.

the concernt I read - please correct me if I am wrong - is that this code affects
downstream projects.

This is just not acceptable to me - downstream implementations should have been
posted to prevent exactly these kind issues.

We spent  time and effort doing an implementation and sharing it with the
community - it wouldnt be fair to us to block this merge on the basis of
downstream projects being impacted.


>
> /Ilias
>
> On Thu, 24 Sept 2026 at 20:50, Macpaul Lin (林智斌)
> <Macpaul.Lin@mediatek.com> wrote:
> >
> > On 18/09/26 22:06:32, Jorge Ramirez-Ortiz wrote:
> > > 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_emmc.c, with the new UFS backend added as a separate
> > > rpmb_ufs.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). A small preparatory patch surfaces the
> > > RESPONSE UPIU sense data from ufs_scsi_exec() so the transport can retry
> > > only on a power-on UNIT ATTENTION; the transport patch also bounces
> > > unaligned frames through a DMA-aligned buffer 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/
> > accepted in v4 --> https://lkml.org/lkml/2026/8/27/369
> >
> > > Op-tee
> > >  https://urldefense.com/v3/__https://github.com/OP-TEE/optee_os/pull/7881__;!!CTRNKA9wMg0ARbw!lQ5D9IUE1D5erqbaXCklFua6JiijZYB6XN0q5LDFZZpiaGmrH_k10rLkcjU1nSzhsRDX--J49Gt22E8U73lx7Es567TUAxwX$
> > and also following Ilias review https://urldefense.com/v3/__https://github.com/OP-TEE/optee_os/pull/8007__;!!CTRNKA9wMg0ARbw!lQ5D9IUE1D5erqbaXCklFua6JiijZYB6XN0q5LDFZZpiaGmrH_k10rLkcjU1nSzhsRDX--J49Gt22E8U73lx7Es565JETGdQ$  (both merged)
> >
> > > For reference, at this point Linux can access RPMB and just u-boot pending on this set.
> > I'd like to have some discussion in more detail as below, it will be quite long. However, my company's
> > email server and mail clients both have problem again as usual. I'm not sure if this reply could be send
> > out correctly and in correct format. Let's see.
> > Moreover, I've invited Kernel's maintainers Stanley Jhu and Bean Huo to this mail thread to see if the opinion increases more compatibility.
> > But, I'm not sure how to invite OPTEE's maintainers, I'll try to forward the following description to OPTEE later.
> >
> > So.. let's start from patch 4/6 and then review the whole story.
> > 4/6 — ufs: derive the per-region RPMB CID and size for OP-TEE
> >
> > Patch 4/6 hard-codes both the hash algorithm (BLAKE2b) and the output
> > length (UFS_RPMB_CID_SIZE = 16) used to derive the RPMB CID for UFS
> > devices. Neither choice is mandated by any specification the series
> > builds on (UFS/JESD220, RPMB, or the OP-TEE/TEE Client API), and
> > hard-coding them creates a real compatibility risk for products that
> > have already shipped with a different, vendor-specific derivation. The
> > CID derivation should be factored out as a vendor-specific, build-time
> > selectable component rather than a single fixed algorithm baked into
> > common UFS core code.

I am a bit at lost. Why didnt those products pushed their code upstream? If they
have an issue with this proposal, they can apply their needs on top, can they
not?

> >
> > 1. Neither the UFS, RPMB, nor TEE specifications mandate a hash
> >    algorithm for CID derivation
> > - The UFS specification (JESD220) defines the Device/Unit/Geometry
> >   descriptors that patch 4/6 reads (manufacturer ID, spec version,
> >   model string, serial number, device version, manufacture date,
> >   region sizes) — it says nothing about combining them into a "CID",
> >   and certainly nothing about which hash function to use.
> > - The RPMB frame protocol itself (struct rpmb_frame in
> >   include/linux/rpmb.h, and the equivalent JEDEC RPMB frame layout)
> >   has no CID field at all — CID is purely a host-side/secure-world
> >   key management convention, not part of the wire protocol.
> > - The GlobalPlatform TEE Client API / OP-TEE's own RPMB subsystem RPC
> >   contract only defines that a fixed-size opaque byte buffer is passed
> >   as the device identifier — it does not specify how that buffer's
> >   contents are derived from the underlying storage device.
> > - Because no specification prescribes the algorithm, this is
> >   legitimately vendor-specific territory, and different vendors — or
> >   even the same vendor across product generations — may reasonably
> >   choose different KDFs/hash functions (BLAKE2b, SHA-256, a
> >   vendor-proprietary hash, or a raw non-hashed encoding, as seen in
> >   other real-world implementations of this same feature). The patch
> >   set should not force one specific vendor's choice into common UFS
> >   core code as the only option.
> >
> > 2. UFS RPMB does not mandate a 16-byte CID; the size should be variable
> >
> > - The 16-byte length is not derived from any UFS or RPMB requirement.
> >   It comes from the eMMC CID register (JEDEC JESD84 Card/Device
> >   Identification Register), which is a fixed 128-bit hardware
> >   register — a purely eMMC-specific hardware fact.
> > - Linux kernel's own generic RPMB abstraction (struct rpmb_descr in
> >   include/linux/rpmb.h) already models this correctly and treats the
> >   device identifier as variable-length by design:
> >
> >   struct rpmb_descr {
> >           enum rpmb_type type;
> >           int (*route_frames)(struct device *dev, u8 *req, unsigned int req_len,
> >                               u8 *resp, unsigned int resp_len);
> >           u8 *dev_id;
> >           size_t dev_id_len;   /* variable, not fixed at 16 */
> >           u16 reliable_wr_count;
> >           u16 capacity;
> >   };
> >
> >   Likewise, the UFS-specific device identifier the Linux kernel builds
> >   in ufshcd_create_device_id() is a variable-length ASCII string
> >   ("%04X-%04X-%s-%s-%04X-%04X"), not a fixed 16-byte value.
> > - Patch 4/6 should follow the same pattern: UFS_RPMB_CID_SIZE should
> >   not be a hard #define 16 baked into include/ufs.h; the CID buffer
> >   size should be a property of the selected vendor backend (e.g.
> >   returned alongside the derived CID, or declared per-backend), so
> >   that a 16-byte output is one valid choice among others rather than
> >   an assumed universal constant.
> >
> > 3. The vendor-specific part should be factored out; 16 bytes is only
> >    there for eMMC compatibility, and UFS has no such hardware CID at all
> >
> > - To restate the core problem plainly: UFS has no CID register.
> >   There is no piece of UFS hardware that patch 4/6's
> >   ufs_rpmb_build_cid() is "reading" — it is entirely a software
> >   construction. The only reason the result is squeezed into 16 bytes
> >   is to fit the pre-existing OP-TEE RPC buffer size that was
> >   originally designed around eMMC's real, hardware-defined 16-byte CID
> >   register.
> > - Conflating "the UFS-specific descriptor-reading/string-building
> >   logic" with "the specific hash function and output length choice"
> >   in a single non-configurable function means every downstream user of
> >   this series inherits one vendor's algorithm choice with no way to
> >   opt out, even though the two concerns are logically separable:
> >   - Reading UFS descriptors and building the input identifier string
> >     is common, spec-driven logic that can stay in shared UFS core
> >     code.
> >   - Turning that string into the actual CID bytes handed to OP-TEE is
> >     the vendor-specific part and should be pluggable.

why cant you extend what is being proposed in this series?

> > - Suggested restructuring: keep ufshcd-side descriptor reads and
> >   string-building in common code, but expose the final
> >   string-to-CID step as a replaceable hook/weak function (or a small
> >   vendor ops table), so the BLAKE2b implementation in this series
> >   becomes the default backend rather than the only backend.

sure, send a patch on top of this one.

> >
> > 4. Hard-coding one algorithm creates real firmware-upgrade and
> >    key-compatibility risk for already-shipping products, so the
> >    derivation must be vendor-selectable at build time (via defconfig)

I dont see why since U-boot currently does not support UFS RPMB. IF downstream
has an issue, please send patches.

> >
> > 5. Since neither the algorithm (point 1) nor the output length (point 2)
> >   is specified by any standard, and CID is purely a software
> >   convention invented to reuse an eMMC-shaped OP-TEE ABI (point 3),
> >   different implementations that already exist today have made
> >   different, incompatible choices for the same problem:

I dont see this code upstream. How can this be a blocker?

> >   - This series (upstream v5, patch 4/6): builds an identifier string
> >     from Device Descriptor fields (manufacturer ID, spec version,
> >     model, serial, device version, manufacture date) and hashes it
> >     with BLAKE2b to a fixed 16-byte output.
> >   - At least one other real, shipping downstream implementation (a
> >     vendor's production U-Boot branch analyzed alongside this series)
> >     derives the CID by directly decoding/porting the device's
> >     tee-supplicant-side read_cid()-style logic — i.e. no hash at all,
> >     just a direct byte encoding of the serial-number string
> >     descriptor.

You keep on mentioning an impact to downstream and my reasoning is the same: why
didnt they push their implementations upstream?

> >   - Both approaches are equally "valid" in the sense that neither
> >     violates any specification, because none exists to violate — but
> >     they are mutually incompatible: the same physical UFS device will
> >     produce a different 16-byte value under each scheme, and
> >     therefore OP-TEE will derive a different final RPMB authentication
> >     key under each scheme.
> > - The practical consequence: any product that has already shipped in
> >   the field with one CID-derivation scheme (whether that is this
> >   series's BLAKE2b approach or a vendor's own pre-existing scheme) has
> >   effectively burned that scheme into its provisioned RPMB key
> >   material. If a firmware update later replaces the bootloader/OP-TEE
> >   RPMB glue with a build that hard-codes a different CID derivation
> >   (for example, adopting this patch set's BLAKE2b implementation
> >   unconditionally on top of a product that shipped with a different
> >   vendor scheme), the newly-derived CID will not match the CID used
> >   when the device's RPMB key material was originally provisioned. The
> >   device will then either:
> >   - fail to authenticate against its own RPMB partition (OP-TEE
> >     derives a key that doesn't match what was provisioned), or
> >   - silently begin deriving a different key than before, which is
> >     just as bad if any secure-storage data was provisioned under the
> >     old key and must remain readable after the upgrade.
> >
> >   Because CID derivation happens deep inside firmware/secure-world
> >   boot code with no user-visible error surface until the RPMB HMAC
> >   check actually fails, this class of bug is essentially undetectable
> >   in normal testing and only surfaces as field failures after an
> >   upgrade — exactly the "silent, undetectable until it fails" failure
> >   mode already called out in the CID appendix.
> > - Therefore, the fix is not merely a documentation/comment change:
> >   the patch set should ship this concern as a build-time (Kconfig /
> >   defconfig) selectable option, e.g.:
> >
> >   config UFS_RPMB_CID_ALGO_BLAKE2B
> >           string "UFS RPMB CID derivation algorithm"
> >           depends on SUPPORT_UFS_RPMB
> >           default "blake2b"
> >           help
> >             Selects which algorithm derives the per-region RPMB CID
> >             from UFS device descriptors. This value is a software
> >             convention with no basis in the UFS or RPMB specification
> >             (UFS has no hardware CID register); it exists only to
> >             populate the fixed-size CID field inherited from the
> >             eMMC-oriented OP-TEE RPMB ABI. Products that have already
> >             shipped with a specific CID derivation MUST keep using
> >             that same derivation across firmware updates, or the
> >             derived RPMB authentication key will change and existing
> >             RPMB-backed secure storage will become unreadable.
> >
> >   with the BLAKE2b implementation in this series wired in as one
> >   selectable backend (CONFIG_UFS_RPMB_CID_ALGO_BLAKE2B), so that a
> >   vendor already shipping a different scheme can select their own
> >   backend at defconfig time instead of being forced onto this
> >   series's default and breaking already-provisioned devices on
> >   upgrade.
> >
> > Each vendor should be able to upstream their own algo and more.

I dont see why but feel free to propose a patch on top of thise series.

> > And these algo should be configurable by the users or by board makers.
> > For example, MediaTek and Ubuntu are using different algo and CID length for
> > UFS RPMB.
> >
> > Here we suggest to make some change to increase the compatibility.
> >
> > 1. Split ufs_rpmb_build_cid() into (a) a common helper that reads the
> >    UFS descriptors and builds the identifier string, and (b) a
> >    pluggable string-to-CID backend, with BLAKE2b as the default
> >    backend rather than the only one.

That would break kernel compatibility - the kernel uses blake2b.

> > 2. Make the CID buffer size a variable size, or at least property returned by the selected
> >    backend rather than a fixed UFS_RPMB_CID_SIZE = 16 constant in
> >    common code.

What would be the benefit of this?

> > 3. Add a Kconfig choice (e.g. UFS_RPMB_CID_ALGO_BLAKE2) so vendors can
> >    select their existing/legacy CID derivation at defconfig time.

And the kernel? how are you going to handled that? This would be a source of
pain when doing software upgrades.

> > 4. Document explicitly, in code comments and Kconfig help text, that
> >    this value is a software ABI convention with no basis in the UFS
> >    or RPMB specification, and that changing it on an already-shipped
> >    product is a breaking change to that product's RPMB key material.
> >
> > Thanks
> > Macpaul Lin

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Jorge Ramirez]

On 26/09/26 22:41:00, Jorge Ramirez wrote:
> On 25/09/26 12:59:28, Ilias Apalodimas wrote:
> > Thanks for the detailed analysis.
> >
> > I am pulling in Jens for OP-TEE and I'll have a closer look next week as well.
>
> the concernt I read - please correct me if I am wrong - is that this code affects
> downstream projects.
>
> This is just not acceptable to me - downstream implementations should have been
> posted to prevent exactly these kind issues.
>
> We spent  time and effort doing an implementation and sharing it with the
> community - it wouldnt be fair to us to block this merge on the basis of
> downstream projects being impacted.
>
>

in summary and to be very clear

1- the upstream CID derivation is the u-boot/op-tee contract that
matches mainline linux's ufshcd_create_device_id(). The contract is an
OP-TEE-defined ABI, not a Global Platform spec.

2. Moreover CID-key is OP-TEE's own HUK-based derivation, not GP, not JEDEC.

3- Nothing here blocks anyone from overriding the static ufs_rpmb_build_cid()
downstream.

3- if downstream scheme differs, that's the cost of having kept it out of tree,
not a reason to hold this series for speculative plug-in infrastructure with a
single backend.

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Jens Wiklander]

Hi,

On Sat, Sep 26, 2026 at 11:09 PM Jorge Ramirez
<jorge.ramirez@oss.qualcomm.com> wrote:
>
> On 26/09/26 22:41:00, Jorge Ramirez wrote:
> > On 25/09/26 12:59:28, Ilias Apalodimas wrote:
> > > Thanks for the detailed analysis.
> > >
> > > I am pulling in Jens for OP-TEE and I'll have a closer look next week as well.
> >
> > the concernt I read - please correct me if I am wrong - is that this code affects
> > downstream projects.
> >
> > This is just not acceptable to me - downstream implementations should have been
> > posted to prevent exactly these kind issues.
> >
> > We spent  time and effort doing an implementation and sharing it with the
> > community - it wouldnt be fair to us to block this merge on the basis of
> > downstream projects being impacted.
> >
> >
>
> in summary and to be very clear
>
> 1- the upstream CID derivation is the u-boot/op-tee contract that
> matches mainline linux's ufshcd_create_device_id(). The contract is an
> OP-TEE-defined ABI, not a Global Platform spec.
>
> 2. Moreover CID-key is OP-TEE's own HUK-based derivation, not GP, not JEDEC.

This follows the OP-TEE ABI. Further extensions to the OP-TEE ABI are
best discussed in an issue or pull request at
https://github.com/OP-TEE/optee_os

Cheers,
Jens

Re: [PATCH v5 0/6] ufs: rpmb: route OP-TEE RPMB secure storage over UFS

[Ilias Apalodimas]

Hi Jorge,


On Sun, 27 Sept 2026 at 00:09, Jorge Ramirez
<jorge.ramirez@oss.qualcomm.com> wrote:
>
> On 26/09/26 22:41:00, Jorge Ramirez wrote:
> > On 25/09/26 12:59:28, Ilias Apalodimas wrote:
> > > Thanks for the detailed analysis.
> > >
> > > I am pulling in Jens for OP-TEE and I'll have a closer look next week as well.
> >
> > the concernt I read - please correct me if I am wrong - is that this code affects
> > downstream projects.
> >
> > This is just not acceptable to me - downstream implementations should have been
> > posted to prevent exactly these kind issues.
> >
> > We spent  time and effort doing an implementation and sharing it with the
> > community - it wouldnt be fair to us to block this merge on the basis of
> > downstream projects being impacted.
> >
> >
>
> in summary and to be very clear
>
> 1- the upstream CID derivation is the u-boot/op-tee contract that
> matches mainline linux's ufshcd_create_device_id(). The contract is an
> OP-TEE-defined ABI, not a Global Platform spec.
>
> 2. Moreover CID-key is OP-TEE's own HUK-based derivation, not GP, not JEDEC.
>
> 3- Nothing here blocks anyone from overriding the static ufs_rpmb_build_cid()
> downstream.
>
> 3- if downstream scheme differs, that's the cost of having kept it out of tree,
> not a reason to hold this series for speculative plug-in infrastructure with a
> single backend.

+1. Macpaul is there anything else you can review apart from the
downstream breakage. TBH since you didn;t bother upstreaming that,
objecting to code that adds real support doesn't really stand as an
arguement.

Jorge, I did a quick pass and I don't see any big issues with the
series. I'll do another pass later this week and send a PR if the CI
agrees.

Cheers
/Ilias

Re: [PATCH v5 3/6] ufs: add RPMB transport over SCSI SECURITY PROTOCOL

[Stanley Jhu]

On Fri, Sep 18, 2026 at 10:06:35PM +0200, Jorge Ramirez-Ortiz wrote:
> +		/*
> +		 * A power-on or reset UNIT ATTENTION is a transient
> +		 * condition the device clears once it has been reported,
> +		 * so retry it. Any other failure is final: stop and
> +		 * report it.
> +		 */
> +		if (ufs_rpmb_sense_key(&pccb) != SENSE_UNIT_ATTENTION)
> +			break;

This retries on any UNIT ATTENTION, e.g. 2Ah/01h (MODE PARAMETERS
CHANGED), not only the power-on/reset one the comment describes.
Would it be cleaner to also check for ASC 29h, as the Linux driver
does? Commit a8a34238e5e6 ("scsi: ufs: rpmb: Retry power-on UNIT
ATTENTION on the RPMB WLUN") in the SCSI tree retries on ASC 29h with
any ASCQ.

Re: [PATCH v5 6/6] optee: implement the RPMB subsystem interface for UFS

[Ilias Apalodimas]

Hi Jorge,


> 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

drivers/tee/optee/optee_msg.h already contains these afaict.

> +
> +#define OPTEE_RPC_RPMB_UFS             1
> +
>  /*

other than that

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v5 5/6] optee: rename rpmb.c to rpmb_emmc.c

[Ilias Apalodimas]

On Fri, 18 Sept 2026 at 23:06, Jorge Ramirez-Ortiz
<jorge.ramirez@oss.qualcomm.com> wrote:
>
> rpmb.c implements the legacy single-command RPMB supplicant, which is
> eMMC-only. Rename it to rpmb_emmc.c to make room for a UFS backend.
>
> No functional change.
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

>  drivers/tee/optee/Makefile                | 2 +-
>  drivers/tee/optee/{rpmb.c => rpmb_emmc.c} | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename drivers/tee/optee/{rpmb.c => rpmb_emmc.c} (100%)
>
> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> index 36ac085ef42..d0c7f8dfb12 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_emmc.o
> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb_emmc.c
> similarity index 100%
> rename from drivers/tee/optee/rpmb.c
> rename to drivers/tee/optee/rpmb_emmc.c
> --
> 2.54.0
>

Re: [PATCH v5 2/6] ufs: surface sense data from ufs_scsi_exec()

[Ilias Apalodimas]

Any chance we can get an ACK from UFS maintainers?

Thanks
/Ilias

On Fri, 18 Sept 2026 at 23:06, Jorge Ramirez-Ortiz
<jorge.ramirez@oss.qualcomm.com> wrote:
>
> On a CHECK CONDITION the UFS device returns SCSI sense data inline in the
> RESPONSE UPIU, but ufs_scsi_exec() discarded it and reported every
> failure as a bare -EINVAL, leaving callers unable to tell a transient
> condition such as a power-on UNIT ATTENTION from a hard error.
>
> Preserve the sense data and SCSI status in the scsi_cmd. No caller
> inspects them yet, so behaviour is unchanged.
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---
>  drivers/ufs/ufs-uclass.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> index 21aed9f97bd..3d96a05281c 100644
> --- a/drivers/ufs/ufs-uclass.c
> +++ b/drivers/ufs/ufs-uclass.c
> @@ -1657,6 +1657,22 @@ static void prepare_prdt_table(struct ufs_hba *hba, struct scsi_cmd *pccb)
>         ufshcd_cache_flush(req_desc, sizeof(*req_desc));
>  }
>
> +/*
> + * On a CHECK CONDITION the sense data is returned inline in the RESPONSE
> + * UPIU; copy it into the SCSI command so callers can inspect the sense key
> + * (for example to tell a transient UNIT ATTENTION from a hard error).
> + */
> +static void ufs_scsi_copy_sense(struct ufs_hba *hba, struct scsi_cmd *pccb)
> +{
> +       u16 len = be16_to_cpu(hba->ucd_rsp_ptr->sr.sense_data_len);
> +
> +       if (len > RESPONSE_UPIU_SENSE_DATA_LENGTH)
> +               len = RESPONSE_UPIU_SENSE_DATA_LENGTH;
> +
> +       memcpy(pccb->sense_buf, hba->ucd_rsp_ptr->sr.sense_data, len);
> +       pccb->sensedatalen = len;
> +}
> +
>  static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
>  {
>         struct ufs_hba *hba = dev_get_uclass_priv(scsi_dev->parent);
> @@ -1664,6 +1680,9 @@ static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
>         int ocs, result = 0;
>         u8 scsi_status;
>
> +       pccb->status = 0;
> +       pccb->sensedatalen = 0;
> +
>         ufshcd_prepare_req_desc_hdr(hba, &upiu_flags, pccb->dma_dir);
>         ufshcd_prepare_utp_scsi_cmd_upiu(hba, pccb, upiu_flags);
>         prepare_prdt_table(hba, pccb);
> @@ -1683,8 +1702,12 @@ static int ufs_scsi_exec(struct udevice *scsi_dev, struct scsi_cmd *pccb)
>                         result = ufshcd_get_rsp_upiu_result(hba->ucd_rsp_ptr);
>
>                         scsi_status = result & MASK_SCSI_STATUS;
> -                       if (scsi_status)
> +                       if (scsi_status) {
> +                               pccb->status = scsi_status;
> +                               if (scsi_status == S_CHECK_COND)
> +                                       ufs_scsi_copy_sense(hba, pccb);
>                                 return -EINVAL;
> +                       }
>
>                         break;
>                 case UPIU_TRANSACTION_REJECT_UPIU:
> --
> 2.54.0
>

Re: [PATCH v5 2/6] ufs: surface sense data from ufs_scsi_exec()

[Jorge Ramirez]

On 02/10/26 14:16:54, Ilias Apalodimas wrote:
> Any chance we can get an ACK from UFS maintainers?
>
> Thanks
> /Ilias


Ilias is it possible that the maintainers have silently quit?

Re: [PATCH v5 2/6] ufs: surface sense data from ufs_scsi_exec()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

On Tue, Oct 13, 2026 at 07:48:02PM +0200, Jorge Ramirez wrote:
> On 02/10/26 14:16:54, Ilias Apalodimas wrote:
> > Any chance we can get an ACK from UFS maintainers?
> >
> > Thanks
> > /Ilias
> 
> 
> Ilias is it possible that the maintainers have silently quit?

Neil was at all the conferences last week, and I think another
conference the week before.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]