From: Bean Huo <beanhuo@micron.com>
This patch adds OP-TEE based RPMB support for UFS devices. This enables secure RPMB operations on
UFS devices through OP-TEE, providing the same functionality available for eMMC devices and
extending kernel-based secure storage support to UFS-based systems.
Benefits of OP-TEE based RPMB implementation:
- Eliminates dependency on userspace supplicant for RPMB access
- Enables early boot secure storage access (e.g., fTPM, secure UEFI variables)
- Provides kernel-level RPMB access as soon as UFS driver is initialized
- Removes complex initramfs dependencies and boot ordering requirements
- Ensures reliable and deterministic secure storage operations
- Supports both built-in and modular fTPM configurations
Co-developed-by: Can Guo <can.guo@oss.qualcomm.com>
Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
drivers/misc/Kconfig | 2 +-
drivers/ufs/core/Makefile | 1 +
drivers/ufs/core/ufs-rpmb.c | 253 +++++++++++++++++++++++++++++++++
drivers/ufs/core/ufshcd-priv.h | 13 ++
drivers/ufs/core/ufshcd.c | 30 +++-
include/ufs/ufs.h | 4 +
include/ufs/ufshcd.h | 3 +
7 files changed, 301 insertions(+), 5 deletions(-)
create mode 100644 drivers/ufs/core/ufs-rpmb.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b9ca56930003..46ffa62eac6e 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -106,7 +106,7 @@ config PHANTOM
config RPMB
tristate "RPMB partition interface"
- depends on MMC
+ depends on MMC || SCSI_UFSHCD
help
Unified RPMB unit interface for RPMB capable devices such as eMMC and
UFS. Provides interface for in-kernel security controllers to access
diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
index cf820fa09a04..51e1867e524e 100644
--- a/drivers/ufs/core/Makefile
+++ b/drivers/ufs/core/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
ufshcd-core-y += ufshcd.o ufs-sysfs.o ufs-mcq.o
+ufshcd-core-$(CONFIG_RPMB) += ufs-rpmb.o
ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
new file mode 100644
index 000000000000..84cdcf8efeb3
--- /dev/null
+++ b/drivers/ufs/core/ufs-rpmb.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * UFS OP-TEE based RPMB Driver
+ *
+ * Copyright (C) 2025 Micron Technology, Inc.
+ * Copyright (C) 2025 Qualcomm Technologies, Inc.
+ *
+ * Authors:
+ * Bean Huo <beanhuo@micron.com>
+ * Can Guo <can.guo@oss.qualcomm.com>
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/rpmb.h>
+#include <linux/string.h>
+#include <linux/list.h>
+#include <ufs/ufshcd.h>
+#include <linux/unaligned.h>
+#include "ufshcd-priv.h"
+
+#define UFS_RPMB_SEC_PROTOCOL 0xEC /* JEDEC UFS application */
+#define UFS_RPMB_SEC_PROTOCOL_ID 0x01 /* JEDEC UFS RPMB protocol ID, CDB byte3 */
+
+static const struct bus_type ufs_rpmb_bus_type = {
+ .name = "ufs_rpmb",
+};
+
+/* UFS RPMB device structure */
+struct ufs_rpmb_dev {
+ u8 region_id;
+ struct device dev;
+ struct rpmb_dev *rdev;
+ struct ufs_hba *hba;
+ struct list_head node;
+};
+
+static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t len, bool send)
+{
+ struct scsi_device *sdev = hba->ufs_rpmb_wlun;
+ u8 cdb[12] = { };
+
+ cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
+ cdb[1] = UFS_RPMB_SEC_PROTOCOL;
+ put_unaligned_be16(spsp, &cdb[2]);
+ put_unaligned_be32(len, &cdb[6]);
+
+ return scsi_execute_cmd(sdev, cdb, send ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
+ buffer, len, /*timeout=*/30 * HZ, 0, NULL);
+}
+
+/* UFS RPMB route frames implementation */
+static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_len, u8 *resp,
+ unsigned int resp_len)
+{
+ struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
+ struct rpmb_frame *frm_out = (struct rpmb_frame *)req;
+ bool need_result_read = true;
+ u16 req_type, protocol_id;
+ struct ufs_hba *hba;
+ int ret;
+
+ if (!ufs_rpmb) {
+ dev_err(dev, "Missing driver data\n");
+ return -ENODEV;
+ }
+
+ if (!req || !req_len || !resp || !resp_len)
+ return -EINVAL;
+
+ hba = ufs_rpmb->hba;
+
+ req_type = be16_to_cpu(frm_out->req_resp);
+
+ switch (req_type) {
+ case RPMB_PROGRAM_KEY:
+ if (req_len != sizeof(struct rpmb_frame) || resp_len != sizeof(struct rpmb_frame))
+ return -EINVAL;
+ break;
+ case RPMB_GET_WRITE_COUNTER:
+ if (req_len != sizeof(struct rpmb_frame) || resp_len != sizeof(struct rpmb_frame))
+ return -EINVAL;
+ need_result_read = false;
+ break;
+ case RPMB_WRITE_DATA:
+ if (req_len % sizeof(struct rpmb_frame) || resp_len != sizeof(struct rpmb_frame))
+ return -EINVAL;
+ break;
+ case RPMB_READ_DATA:
+ if (req_len != sizeof(struct rpmb_frame) || resp_len % sizeof(struct rpmb_frame))
+ return -EINVAL;
+ need_result_read = false;
+ break;
+ default:
+ dev_err(dev, "Unknown request type=0x%04x\n", req_type);
+ return -EINVAL;
+ }
+
+ protocol_id = ufs_rpmb->region_id << 8 | UFS_RPMB_SEC_PROTOCOL_ID;
+
+ ret = ufs_sec_submit(hba, protocol_id, req, req_len, true);
+ if (ret) {
+ dev_err(dev, "Command failed with ret=%d\n", ret);
+ return ret;
+ }
+
+ if (need_result_read) {
+ struct rpmb_frame *frm_resp = (struct rpmb_frame *)resp;
+
+ memset(frm_resp, 0, sizeof(*frm_resp));
+ frm_resp->req_resp = cpu_to_be16(RPMB_RESULT_READ);
+ ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, true);
+ if (ret) {
+ dev_err(dev, "Result read request failed with ret=%d\n", ret);
+ return ret;
+ }
+ }
+
+ if (!ret) {
+ ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, false);
+ if (ret)
+ dev_err(dev, "Response read failed with ret=%d\n", ret);
+ }
+
+ return ret;
+}
+
+static void ufs_rpmb_device_release(struct device *dev)
+{
+ struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
+
+ if (ufs_rpmb->rdev)
+ rpmb_dev_unregister(ufs_rpmb->rdev);
+}
+
+/* UFS RPMB device registration */
+int ufs_rpmb_probe(struct ufs_hba *hba)
+{
+ struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
+ struct rpmb_dev *rdev;
+ u8 cid[16] = { };
+ int region;
+ u8 *sn;
+ u32 cap;
+ int ret;
+
+ if (!hba->ufs_rpmb_wlun) {
+ dev_info(hba->dev, "No RPMB LUN, skip RPMB registration\n");
+ return -ENODEV;
+ }
+
+ /* Get the UNICODE serial number data */
+ sn = hba->dev_info.serial_number;
+ if (!sn) {
+ dev_err(hba->dev, "Serial number not available\n");
+ return -EINVAL;
+ }
+
+ INIT_LIST_HEAD(&hba->rpmbs);
+
+ /* Copy serial number into device ID (max 15 chars + NUL). */
+ strscpy(cid, sn, sizeof(cid));
+
+ struct rpmb_descr descr = {
+ .type = RPMB_TYPE_UFS,
+ .route_frames = ufs_rpmb_route_frames,
+ .dev_id_len = sizeof(cid),
+ .reliable_wr_count = hba->dev_info.rpmb_io_size,
+ };
+
+ for (region = 0; region < 4; region++) {
+ cap = hba->dev_info.rpmb_region_size[region];
+ if (!cap)
+ continue;
+
+ ufs_rpmb = devm_kzalloc(hba->dev, sizeof(*ufs_rpmb), GFP_KERNEL);
+ if (!ufs_rpmb) {
+ ret = -ENOMEM;
+ goto err_out;
+ }
+
+ ufs_rpmb->hba = hba;
+ ufs_rpmb->dev.parent = &hba->ufs_rpmb_wlun->sdev_gendev;
+ ufs_rpmb->dev.bus = &ufs_rpmb_bus_type;
+ ufs_rpmb->dev.release = ufs_rpmb_device_release;
+ dev_set_name(&ufs_rpmb->dev, "ufs_rpmb%d", region);
+
+ /* Set driver data BEFORE device_register */
+ dev_set_drvdata(&ufs_rpmb->dev, ufs_rpmb);
+
+ ret = device_register(&ufs_rpmb->dev);
+ if (ret) {
+ dev_err(hba->dev, "Failed to register UFS RPMB device %d\n", region);
+ put_device(&ufs_rpmb->dev);
+ goto err_out;
+ }
+
+ /* Make CID unique for this region by appending region numbe */
+ cid[sizeof(cid) - 1] = region;
+ descr.dev_id = (void *)cid;
+ descr.capacity = cap;
+
+ /* Register RPMB device */
+ rdev = rpmb_dev_register(&ufs_rpmb->dev, &descr);
+ if (IS_ERR(rdev)) {
+ dev_err(hba->dev, "Failed to register UFS RPMB device.\n");
+ device_unregister(&ufs_rpmb->dev);
+ ret = PTR_ERR(rdev);
+ goto err_out;
+ }
+
+ ufs_rpmb->rdev = rdev;
+ ufs_rpmb->region_id = region;
+
+ list_add_tail(&ufs_rpmb->node, &hba->rpmbs);
+
+ dev_info(hba->dev, "UFS RPMB region %d registered (capacity=%u)\n", region, cap);
+ }
+
+ return 0;
+err_out:
+ list_for_each_entry_safe(it, tmp, &hba->rpmbs, node) {
+ list_del(&it->node);
+ device_unregister(&it->dev);
+ }
+
+ return ret;
+}
+
+/* UFS RPMB remove handler */
+void ufs_rpmb_remove(struct ufs_hba *hba)
+{
+ struct ufs_rpmb_dev *ufs_rpmb, *tmp;
+
+ if (list_empty(&hba->rpmbs))
+ return;
+
+ /* Remove all registered RPMB devices */
+ list_for_each_entry_safe(ufs_rpmb, tmp, &hba->rpmbs, node) {
+ dev_info(hba->dev, "Removing UFS RPMB region %d\n", ufs_rpmb->region_id);
+ /* Remove from list first */
+ list_del(&ufs_rpmb->node);
+ /* Unregister device */
+ device_unregister(&ufs_rpmb->dev);
+ }
+
+ dev_info(hba->dev, "All UFS RPMB devices unregistered\n");
+}
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("OP-TEE RPMB subsystem based UFS RPMB driver");
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index d0a2c963a27d..523828d6b1d5 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -411,4 +411,17 @@ static inline u32 ufshcd_mcq_get_sq_head_slot(struct ufs_hw_queue *q)
return val / sizeof(struct utp_transfer_req_desc);
}
+#ifdef CONFIG_RPMB
+int ufs_rpmb_probe(struct ufs_hba *hba);
+void ufs_rpmb_remove(struct ufs_hba *hba);
+#else
+static inline int ufs_rpmb_probe(struct ufs_hba *hba)
+{
+ return 0;
+}
+static inline void ufs_rpmb_remove(struct ufs_hba *hba)
+{
+}
+#endif
+
#endif /* _UFSHCD_PRIV_H_ */
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 79c7588be28a..ec1670d94946 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5240,10 +5240,15 @@ static void ufshcd_lu_init(struct ufs_hba *hba, struct scsi_device *sdev)
desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] == UFS_LU_POWER_ON_WP)
hba->dev_info.is_lu_power_on_wp = true;
- /* In case of RPMB LU, check if advanced RPMB mode is enabled */
- if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN &&
- desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
- hba->dev_info.b_advanced_rpmb_en = true;
+ /* In case of RPMB LU, check if advanced RPMB mode is enabled, and get region size */
+ if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN) {
+ if (desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
+ hba->dev_info.b_advanced_rpmb_en = true;
+ hba->dev_info.rpmb_region_size[0] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION0_SIZE];
+ hba->dev_info.rpmb_region_size[1] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION1_SIZE];
+ hba->dev_info.rpmb_region_size[2] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION2_SIZE];
+ hba->dev_info.rpmb_region_size[3] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION3_SIZE];
+ }
kfree(desc_buf);
@@ -8151,8 +8156,11 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_RPMB_WLUN), NULL);
if (IS_ERR(sdev_rpmb)) {
ret = PTR_ERR(sdev_rpmb);
+ hba->ufs_rpmb_wlun = NULL;
+ dev_err(hba->dev, "%s: RPMB WLUN not found\n", __func__);
goto remove_ufs_device_wlun;
}
+ hba->ufs_rpmb_wlun = sdev_rpmb;
ufshcd_blk_pm_runtime_init(sdev_rpmb);
scsi_device_put(sdev_rpmb);
@@ -8425,6 +8433,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
int err;
u8 model_index;
u8 *desc_buf;
+ u8 serial_index;
struct ufs_dev_info *dev_info = &hba->dev_info;
desc_buf = kzalloc(QUERY_DESC_MAX_SIZE, GFP_KERNEL);
@@ -8460,6 +8469,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
UFS_DEV_HID_SUPPORT;
model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
+ serial_index = desc_buf[DEVICE_DESC_PARAM_SN];
err = ufshcd_read_string_desc(hba, model_index,
&dev_info->model, SD_ASCII_STD);
@@ -8469,6 +8479,12 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
goto out;
}
+ err = ufshcd_read_string_desc(hba, serial_index, &dev_info->serial_number, SD_RAW);
+ if (err < 0) {
+ dev_err(hba->dev, "%s: Failed reading Serial Number. err = %d\n", __func__, err);
+ goto out;
+ }
+
hba->luns_avail = desc_buf[DEVICE_DESC_PARAM_NUM_LU] +
desc_buf[DEVICE_DESC_PARAM_NUM_WLU];
@@ -8504,6 +8520,8 @@ static void ufs_put_device_desc(struct ufs_hba *hba)
kfree(dev_info->model);
dev_info->model = NULL;
+ kfree(dev_info->serial_number);
+ dev_info->serial_number = NULL;
}
/**
@@ -8647,6 +8665,8 @@ static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
else if (desc_buf[GEOMETRY_DESC_PARAM_MAX_NUM_LUN] == 0)
hba->dev_info.max_lu_supported = 8;
+ hba->dev_info.rpmb_io_size = desc_buf[GEOMETRY_DESC_PARAM_RPMB_RW_SIZE];
+
out:
kfree(desc_buf);
return err;
@@ -8832,6 +8852,7 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
ufs_bsg_probe(hba);
scsi_scan_host(hba->host);
+ ufs_rpmb_probe(hba);
out:
return ret;
@@ -10391,6 +10412,7 @@ void ufshcd_remove(struct ufs_hba *hba)
ufshcd_rpm_get_sync(hba);
ufs_hwmon_remove(hba);
ufs_bsg_remove(hba);
+ ufs_rpmb_remove(hba);
ufs_sysfs_remove_nodes(hba->dev);
cancel_delayed_work_sync(&hba->ufs_rtc_update_work);
blk_mq_destroy_queue(hba->tmf_queue);
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 72fd385037a6..1d44e2b32253 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -651,6 +651,10 @@ struct ufs_dev_info {
u8 rtt_cap; /* bDeviceRTTCap */
bool hid_sup;
+
+ u8 *serial_number;
+ u8 rpmb_io_size;
+ u8 rpmb_region_size[4];
};
/*
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 1d3943777584..17e97a45ef71 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -984,6 +984,7 @@ struct ufs_hba {
struct Scsi_Host *host;
struct device *dev;
struct scsi_device *ufs_device_wlun;
+ struct scsi_device *ufs_rpmb_wlun;
#ifdef CONFIG_SCSI_UFS_HWMON
struct device *hwmon_device;
@@ -1140,6 +1141,8 @@ struct ufs_hba {
int critical_health_count;
atomic_t dev_lvl_exception_count;
u64 dev_lvl_exception_id;
+
+ struct list_head rpmbs;
};
/**
--
2.34.1On 9/30/25 11:08 PM, Bean Huo wrote:
> +MODULE_DESCRIPTION("OP-TEE RPMB subsystem based UFS RPMB driver");
Hmm ... "OP-TEE UFS RPMB driver" is probably more clear and still
unambiguous. Anyway:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On Wed, 2025-10-01 at 12:51 -0700, Bart Van Assche wrote:
> On 9/30/25 11:08 PM, Bean Huo wrote:
> > +MODULE_DESCRIPTION("OP-TEE RPMB subsystem based UFS RPMB driver");
>
> Hmm ... "OP-TEE UFS RPMB driver" is probably more clear and still
> unambiguous. Anyway:
Bart,
ok, I will change to it in next version.
Kind regards,
Bean
Hi Bean,
On Wed, Oct 1, 2025 at 8:08 AM Bean Huo <beanhuo@iokpp.de> wrote:
>
> From: Bean Huo <beanhuo@micron.com>
>
> This patch adds OP-TEE based RPMB support for UFS devices. This enables secure RPMB operations on
> UFS devices through OP-TEE, providing the same functionality available for eMMC devices and
> extending kernel-based secure storage support to UFS-based systems.
>
> Benefits of OP-TEE based RPMB implementation:
> - Eliminates dependency on userspace supplicant for RPMB access
> - Enables early boot secure storage access (e.g., fTPM, secure UEFI variables)
> - Provides kernel-level RPMB access as soon as UFS driver is initialized
> - Removes complex initramfs dependencies and boot ordering requirements
> - Ensures reliable and deterministic secure storage operations
> - Supports both built-in and modular fTPM configurations
>
> Co-developed-by: Can Guo <can.guo@oss.qualcomm.com>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
> drivers/misc/Kconfig | 2 +-
> drivers/ufs/core/Makefile | 1 +
> drivers/ufs/core/ufs-rpmb.c | 253 +++++++++++++++++++++++++++++++++
> drivers/ufs/core/ufshcd-priv.h | 13 ++
> drivers/ufs/core/ufshcd.c | 30 +++-
> include/ufs/ufs.h | 4 +
> include/ufs/ufshcd.h | 3 +
> 7 files changed, 301 insertions(+), 5 deletions(-)
> create mode 100644 drivers/ufs/core/ufs-rpmb.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index b9ca56930003..46ffa62eac6e 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -106,7 +106,7 @@ config PHANTOM
>
> config RPMB
> tristate "RPMB partition interface"
> - depends on MMC
> + depends on MMC || SCSI_UFSHCD
> help
> Unified RPMB unit interface for RPMB capable devices such as eMMC and
> UFS. Provides interface for in-kernel security controllers to access
> diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
> index cf820fa09a04..51e1867e524e 100644
> --- a/drivers/ufs/core/Makefile
> +++ b/drivers/ufs/core/Makefile
> @@ -2,6 +2,7 @@
>
> obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> ufshcd-core-y += ufshcd.o ufs-sysfs.o ufs-mcq.o
> +ufshcd-core-$(CONFIG_RPMB) += ufs-rpmb.o
SCSI_UFSHCD might need the same trick ("depends on RPMB || !RPMB") in
Kconfig as we have for MMC_BLOCK.
> ufshcd-core-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
> ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o
> ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> new file mode 100644
> index 000000000000..84cdcf8efeb3
> --- /dev/null
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UFS OP-TEE based RPMB Driver
> + *
> + * Copyright (C) 2025 Micron Technology, Inc.
> + * Copyright (C) 2025 Qualcomm Technologies, Inc.
> + *
> + * Authors:
> + * Bean Huo <beanhuo@micron.com>
> + * Can Guo <can.guo@oss.qualcomm.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/rpmb.h>
> +#include <linux/string.h>
> +#include <linux/list.h>
> +#include <ufs/ufshcd.h>
> +#include <linux/unaligned.h>
> +#include "ufshcd-priv.h"
> +
> +#define UFS_RPMB_SEC_PROTOCOL 0xEC /* JEDEC UFS application */
> +#define UFS_RPMB_SEC_PROTOCOL_ID 0x01 /* JEDEC UFS RPMB protocol ID, CDB byte3 */
> +
> +static const struct bus_type ufs_rpmb_bus_type = {
> + .name = "ufs_rpmb",
> +};
> +
> +/* UFS RPMB device structure */
> +struct ufs_rpmb_dev {
> + u8 region_id;
> + struct device dev;
> + struct rpmb_dev *rdev;
> + struct ufs_hba *hba;
> + struct list_head node;
> +};
> +
> +static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t len, bool send)
> +{
> + struct scsi_device *sdev = hba->ufs_rpmb_wlun;
> + u8 cdb[12] = { };
> +
> + cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
> + cdb[1] = UFS_RPMB_SEC_PROTOCOL;
> + put_unaligned_be16(spsp, &cdb[2]);
> + put_unaligned_be32(len, &cdb[6]);
> +
> + return scsi_execute_cmd(sdev, cdb, send ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
> + buffer, len, /*timeout=*/30 * HZ, 0, NULL);
> +}
> +
> +/* UFS RPMB route frames implementation */
> +static int ufs_rpmb_route_frames(struct device *dev, u8 *req, unsigned int req_len, u8 *resp,
> + unsigned int resp_len)
> +{
> + struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
> + struct rpmb_frame *frm_out = (struct rpmb_frame *)req;
> + bool need_result_read = true;
> + u16 req_type, protocol_id;
> + struct ufs_hba *hba;
> + int ret;
> +
> + if (!ufs_rpmb) {
> + dev_err(dev, "Missing driver data\n");
> + return -ENODEV;
> + }
> +
> + if (!req || !req_len || !resp || !resp_len)
> + return -EINVAL;
This function is only called indirectly from rpmb_route_frames(),
where this is already checked.
> +
> + hba = ufs_rpmb->hba;
> +
> + req_type = be16_to_cpu(frm_out->req_resp);
> +
> + switch (req_type) {
> + case RPMB_PROGRAM_KEY:
> + if (req_len != sizeof(struct rpmb_frame) || resp_len != sizeof(struct rpmb_frame))
> + return -EINVAL;
> + break;
> + case RPMB_GET_WRITE_COUNTER:
> + if (req_len != sizeof(struct rpmb_frame) || resp_len != sizeof(struct rpmb_frame))
> + return -EINVAL;
> + need_result_read = false;
> + break;
> + case RPMB_WRITE_DATA:
> + if (req_len % sizeof(struct rpmb_frame) || resp_len != sizeof(struct rpmb_frame))
> + return -EINVAL;
> + break;
> + case RPMB_READ_DATA:
> + if (req_len != sizeof(struct rpmb_frame) || resp_len % sizeof(struct rpmb_frame))
> + return -EINVAL;
> + need_result_read = false;
> + break;
> + default:
> + dev_err(dev, "Unknown request type=0x%04x\n", req_type);
> + return -EINVAL;
> + }
> +
> + protocol_id = ufs_rpmb->region_id << 8 | UFS_RPMB_SEC_PROTOCOL_ID;
> +
> + ret = ufs_sec_submit(hba, protocol_id, req, req_len, true);
> + if (ret) {
> + dev_err(dev, "Command failed with ret=%d\n", ret);
> + return ret;
> + }
> +
> + if (need_result_read) {
> + struct rpmb_frame *frm_resp = (struct rpmb_frame *)resp;
> +
> + memset(frm_resp, 0, sizeof(*frm_resp));
> + frm_resp->req_resp = cpu_to_be16(RPMB_RESULT_READ);
> + ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, true);
> + if (ret) {
> + dev_err(dev, "Result read request failed with ret=%d\n", ret);
> + return ret;
> + }
> + }
> +
> + if (!ret) {
> + ret = ufs_sec_submit(hba, protocol_id, resp, resp_len, false);
> + if (ret)
> + dev_err(dev, "Response read failed with ret=%d\n", ret);
> + }
> +
> + return ret;
> +}
> +
> +static void ufs_rpmb_device_release(struct device *dev)
> +{
> + struct ufs_rpmb_dev *ufs_rpmb = dev_get_drvdata(dev);
> +
> + if (ufs_rpmb->rdev)
rpmb_dev_unregister() already checks rdev for NULL.
> + rpmb_dev_unregister(ufs_rpmb->rdev);
> +}
> +
> +/* UFS RPMB device registration */
> +int ufs_rpmb_probe(struct ufs_hba *hba)
> +{
> + struct ufs_rpmb_dev *ufs_rpmb, *it, *tmp;
> + struct rpmb_dev *rdev;
> + u8 cid[16] = { };
> + int region;
> + u8 *sn;
> + u32 cap;
> + int ret;
> +
> + if (!hba->ufs_rpmb_wlun) {
> + dev_info(hba->dev, "No RPMB LUN, skip RPMB registration\n");
> + return -ENODEV;
> + }
> +
> + /* Get the UNICODE serial number data */
> + sn = hba->dev_info.serial_number;
> + if (!sn) {
> + dev_err(hba->dev, "Serial number not available\n");
> + return -EINVAL;
> + }
> +
> + INIT_LIST_HEAD(&hba->rpmbs);
> +
> + /* Copy serial number into device ID (max 15 chars + NUL). */
> + strscpy(cid, sn, sizeof(cid));
strscpy(cid, sn) is enough; strscpy() can determine the size itself
since cid is an array.
> +
> + struct rpmb_descr descr = {
> + .type = RPMB_TYPE_UFS,
We'll need another type if the device uses the extended RPMB frame
format. How about you clarify this, where RPMB_TYPE_UFS is defined to
avoid confusion?
> + .route_frames = ufs_rpmb_route_frames,
> + .dev_id_len = sizeof(cid),
> + .reliable_wr_count = hba->dev_info.rpmb_io_size,
> + };
> +
> + for (region = 0; region < 4; region++) {
Would it make sense to use ARRAY_SIZE(hba->dev_info.rpmb_region_size)
to avoid another magic number?
> + cap = hba->dev_info.rpmb_region_size[region];
> + if (!cap)
> + continue;
> +
> + ufs_rpmb = devm_kzalloc(hba->dev, sizeof(*ufs_rpmb), GFP_KERNEL);
> + if (!ufs_rpmb) {
> + ret = -ENOMEM;
> + goto err_out;
> + }
> +
> + ufs_rpmb->hba = hba;
> + ufs_rpmb->dev.parent = &hba->ufs_rpmb_wlun->sdev_gendev;
> + ufs_rpmb->dev.bus = &ufs_rpmb_bus_type;
> + ufs_rpmb->dev.release = ufs_rpmb_device_release;
> + dev_set_name(&ufs_rpmb->dev, "ufs_rpmb%d", region);
> +
> + /* Set driver data BEFORE device_register */
> + dev_set_drvdata(&ufs_rpmb->dev, ufs_rpmb);
> +
> + ret = device_register(&ufs_rpmb->dev);
> + if (ret) {
> + dev_err(hba->dev, "Failed to register UFS RPMB device %d\n", region);
> + put_device(&ufs_rpmb->dev);
> + goto err_out;
> + }
> +
> + /* Make CID unique for this region by appending region numbe */
> + cid[sizeof(cid) - 1] = region;
> + descr.dev_id = (void *)cid;
Casting isn't needed; descr.dev_id has the same type as cid.
Cheers,
Jens
> + descr.capacity = cap;
> +
> + /* Register RPMB device */
> + rdev = rpmb_dev_register(&ufs_rpmb->dev, &descr);
> + if (IS_ERR(rdev)) {
> + dev_err(hba->dev, "Failed to register UFS RPMB device.\n");
> + device_unregister(&ufs_rpmb->dev);
> + ret = PTR_ERR(rdev);
> + goto err_out;
> + }
> +
> + ufs_rpmb->rdev = rdev;
> + ufs_rpmb->region_id = region;
> +
> + list_add_tail(&ufs_rpmb->node, &hba->rpmbs);
> +
> + dev_info(hba->dev, "UFS RPMB region %d registered (capacity=%u)\n", region, cap);
> + }
> +
> + return 0;
> +err_out:
> + list_for_each_entry_safe(it, tmp, &hba->rpmbs, node) {
> + list_del(&it->node);
> + device_unregister(&it->dev);
> + }
> +
> + return ret;
> +}
> +
> +/* UFS RPMB remove handler */
> +void ufs_rpmb_remove(struct ufs_hba *hba)
> +{
> + struct ufs_rpmb_dev *ufs_rpmb, *tmp;
> +
> + if (list_empty(&hba->rpmbs))
> + return;
> +
> + /* Remove all registered RPMB devices */
> + list_for_each_entry_safe(ufs_rpmb, tmp, &hba->rpmbs, node) {
> + dev_info(hba->dev, "Removing UFS RPMB region %d\n", ufs_rpmb->region_id);
> + /* Remove from list first */
> + list_del(&ufs_rpmb->node);
> + /* Unregister device */
> + device_unregister(&ufs_rpmb->dev);
> + }
> +
> + dev_info(hba->dev, "All UFS RPMB devices unregistered\n");
> +}
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("OP-TEE RPMB subsystem based UFS RPMB driver");
> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> index d0a2c963a27d..523828d6b1d5 100644
> --- a/drivers/ufs/core/ufshcd-priv.h
> +++ b/drivers/ufs/core/ufshcd-priv.h
> @@ -411,4 +411,17 @@ static inline u32 ufshcd_mcq_get_sq_head_slot(struct ufs_hw_queue *q)
> return val / sizeof(struct utp_transfer_req_desc);
> }
>
> +#ifdef CONFIG_RPMB
> +int ufs_rpmb_probe(struct ufs_hba *hba);
> +void ufs_rpmb_remove(struct ufs_hba *hba);
> +#else
> +static inline int ufs_rpmb_probe(struct ufs_hba *hba)
> +{
> + return 0;
> +}
> +static inline void ufs_rpmb_remove(struct ufs_hba *hba)
> +{
> +}
> +#endif
> +
> #endif /* _UFSHCD_PRIV_H_ */
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 79c7588be28a..ec1670d94946 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5240,10 +5240,15 @@ static void ufshcd_lu_init(struct ufs_hba *hba, struct scsi_device *sdev)
> desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] == UFS_LU_POWER_ON_WP)
> hba->dev_info.is_lu_power_on_wp = true;
>
> - /* In case of RPMB LU, check if advanced RPMB mode is enabled */
> - if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN &&
> - desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
> - hba->dev_info.b_advanced_rpmb_en = true;
> + /* In case of RPMB LU, check if advanced RPMB mode is enabled, and get region size */
> + if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN) {
> + if (desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
> + hba->dev_info.b_advanced_rpmb_en = true;
Does this indicate that the other RPMB frame format is used?
> + hba->dev_info.rpmb_region_size[0] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION0_SIZE];
> + hba->dev_info.rpmb_region_size[1] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION1_SIZE];
> + hba->dev_info.rpmb_region_size[2] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION2_SIZE];
> + hba->dev_info.rpmb_region_size[3] = desc_buf[RPMB_UNIT_DESC_PARAM_REGION3_SIZE];
> + }
>
>
> kfree(desc_buf);
> @@ -8151,8 +8156,11 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
> ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_RPMB_WLUN), NULL);
> if (IS_ERR(sdev_rpmb)) {
> ret = PTR_ERR(sdev_rpmb);
> + hba->ufs_rpmb_wlun = NULL;
> + dev_err(hba->dev, "%s: RPMB WLUN not found\n", __func__);
> goto remove_ufs_device_wlun;
> }
> + hba->ufs_rpmb_wlun = sdev_rpmb;
> ufshcd_blk_pm_runtime_init(sdev_rpmb);
> scsi_device_put(sdev_rpmb);
>
> @@ -8425,6 +8433,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
> int err;
> u8 model_index;
> u8 *desc_buf;
> + u8 serial_index;
> struct ufs_dev_info *dev_info = &hba->dev_info;
>
> desc_buf = kzalloc(QUERY_DESC_MAX_SIZE, GFP_KERNEL);
> @@ -8460,6 +8469,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
> UFS_DEV_HID_SUPPORT;
>
> model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
> + serial_index = desc_buf[DEVICE_DESC_PARAM_SN];
>
> err = ufshcd_read_string_desc(hba, model_index,
> &dev_info->model, SD_ASCII_STD);
> @@ -8469,6 +8479,12 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
> goto out;
> }
>
> + err = ufshcd_read_string_desc(hba, serial_index, &dev_info->serial_number, SD_RAW);
> + if (err < 0) {
> + dev_err(hba->dev, "%s: Failed reading Serial Number. err = %d\n", __func__, err);
> + goto out;
> + }
> +
> hba->luns_avail = desc_buf[DEVICE_DESC_PARAM_NUM_LU] +
> desc_buf[DEVICE_DESC_PARAM_NUM_WLU];
>
> @@ -8504,6 +8520,8 @@ static void ufs_put_device_desc(struct ufs_hba *hba)
>
> kfree(dev_info->model);
> dev_info->model = NULL;
> + kfree(dev_info->serial_number);
> + dev_info->serial_number = NULL;
> }
>
> /**
> @@ -8647,6 +8665,8 @@ static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
> else if (desc_buf[GEOMETRY_DESC_PARAM_MAX_NUM_LUN] == 0)
> hba->dev_info.max_lu_supported = 8;
>
> + hba->dev_info.rpmb_io_size = desc_buf[GEOMETRY_DESC_PARAM_RPMB_RW_SIZE];
> +
> out:
> kfree(desc_buf);
> return err;
> @@ -8832,6 +8852,7 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
>
> ufs_bsg_probe(hba);
> scsi_scan_host(hba->host);
> + ufs_rpmb_probe(hba);
>
> out:
> return ret;
> @@ -10391,6 +10412,7 @@ void ufshcd_remove(struct ufs_hba *hba)
> ufshcd_rpm_get_sync(hba);
> ufs_hwmon_remove(hba);
> ufs_bsg_remove(hba);
> + ufs_rpmb_remove(hba);
> ufs_sysfs_remove_nodes(hba->dev);
> cancel_delayed_work_sync(&hba->ufs_rtc_update_work);
> blk_mq_destroy_queue(hba->tmf_queue);
> diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
> index 72fd385037a6..1d44e2b32253 100644
> --- a/include/ufs/ufs.h
> +++ b/include/ufs/ufs.h
> @@ -651,6 +651,10 @@ struct ufs_dev_info {
> u8 rtt_cap; /* bDeviceRTTCap */
>
> bool hid_sup;
> +
> + u8 *serial_number;
> + u8 rpmb_io_size;
> + u8 rpmb_region_size[4];
> };
>
> /*
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 1d3943777584..17e97a45ef71 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -984,6 +984,7 @@ struct ufs_hba {
> struct Scsi_Host *host;
> struct device *dev;
> struct scsi_device *ufs_device_wlun;
> + struct scsi_device *ufs_rpmb_wlun;
>
> #ifdef CONFIG_SCSI_UFS_HWMON
> struct device *hwmon_device;
> @@ -1140,6 +1141,8 @@ struct ufs_hba {
> int critical_health_count;
> atomic_t dev_lvl_exception_count;
> u64 dev_lvl_exception_id;
> +
> + struct list_head rpmbs;
> };
>
> /**
> --
> 2.34.1
>
Jens,
I incorporated your suggestions in my v3 excpet these two:
On Wed, 2025-10-01 at 09:50 +0200, Jens Wiklander wrote:
> > diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
> > index cf820fa09a04..51e1867e524e 100644
> > --- a/drivers/ufs/core/Makefile
> > +++ b/drivers/ufs/core/Makefile
> > @@ -2,6 +2,7 @@
> >
> > obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> > ufshcd-core-y += ufshcd.o ufs-sysfs.o ufs-mcq.o
> > +ufshcd-core-$(CONFIG_RPMB) += ufs-rpmb.o
>
> SCSI_UFSHCD might need the same trick ("depends on RPMB || !RPMB") in
> Kconfig as we have for MMC_BLOCK.
>
> >
When RPMB=m and SCSI_UFSHCD=y, the ufs-rpmb.o is compiled into the built-in
ufshcd-core, ufs-rpmb.c calls functions from the OP-TEE RPMB subsystem module,
The kernel allows built-in code to reference module symbols (they become runtime
dependencies, not link-time), please check, I tested.
> >
> >
>
> > +
> > + struct rpmb_descr descr = {
> > + .type = RPMB_TYPE_UFS,
>
> We'll need another type if the device uses the extended RPMB frame
> format. How about you clarify this, where RPMB_TYPE_UFS is defined to
> avoid confusion?
As ufs-bsg.c, we could use ARPMB_TYPE_UFS for UFS advanced RPMB frame, if it is
RPMB, we take it as normal RPMB, the frame should be the same as MMC RPMB.
Kind regards,
Bean
Hi Bean,
On Wed, Oct 8, 2025 at 5:07 PM Bean Huo <huobean@gmail.com> wrote:
>
> Jens,
>
> I incorporated your suggestions in my v3 excpet these two:
>
>
> On Wed, 2025-10-01 at 09:50 +0200, Jens Wiklander wrote:
> > > diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
> > > index cf820fa09a04..51e1867e524e 100644
> > > --- a/drivers/ufs/core/Makefile
> > > +++ b/drivers/ufs/core/Makefile
> > > @@ -2,6 +2,7 @@
> > >
> > > obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> > > ufshcd-core-y += ufshcd.o ufs-sysfs.o ufs-mcq.o
> > > +ufshcd-core-$(CONFIG_RPMB) += ufs-rpmb.o
> >
> > SCSI_UFSHCD might need the same trick ("depends on RPMB || !RPMB") in
> > Kconfig as we have for MMC_BLOCK.
> >
> > >
> When RPMB=m and SCSI_UFSHCD=y, the ufs-rpmb.o is compiled into the built-in
> ufshcd-core, ufs-rpmb.c calls functions from the OP-TEE RPMB subsystem module,
> The kernel allows built-in code to reference module symbols (they become runtime
> dependencies, not link-time), please check, I tested.
>
> > >
> > >
> >
> > > +
> > > + struct rpmb_descr descr = {
> > > + .type = RPMB_TYPE_UFS,
> >
> > We'll need another type if the device uses the extended RPMB frame
> > format. How about you clarify this, where RPMB_TYPE_UFS is defined to
> > avoid confusion?
>
> As ufs-bsg.c, we could use ARPMB_TYPE_UFS for UFS advanced RPMB frame, if it is
> RPMB, we take it as normal RPMB, the frame should be the same as MMC RPMB.
Isn't it a bit confusing to set the type to RPMB_TYPE_EMMC when it's
actually a UFS RPMB, even if it's supposedly compatible enough?
While the frame format works, I'm concerned about the CID. It's
essentially a namespace of its own for eMMC, and at least the OP-TEE
implementation makes assumptions about the format by masking out the
PRV (Product Revision) and CRC (CRC7 checksum) fields from the CID
when deriving the RPMB key. For this to work reliably, the CID must be
guaranteed to be unique per RPMB device.
From what I understand, for UFS, the serial number is only guaranteed
to be unique if the manufacturer and the product name are taken into
account. Combined, these fields can be much larger than 16 bytes, and
we also have the partition number to consider.
By using RPMB_TYPE_UFS we can define a device ID tailored for UFS with
all the fields we need. Thoughts?
Cheers,
Jens
On Mon, 2025-10-13 at 10:21 +0200, Jens Wiklander wrote:
> Hi Bean,
>
>
>
> On Wed, Oct 8, 2025 at 5:07 PM Bean Huo <huobean@gmail.com> wrote:
> >
> > Jens,
> >
> > I incorporated your suggestions in my v3 excpet these two:
> >
> >
> > On Wed, 2025-10-01 at 09:50 +0200, Jens Wiklander wrote:
> > > > diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
> > > > index cf820fa09a04..51e1867e524e 100644
> > > > --- a/drivers/ufs/core/Makefile
> > > > +++ b/drivers/ufs/core/Makefile
> > > > @@ -2,6 +2,7 @@
> > > >
> > > > obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> > > > ufshcd-core-y += ufshcd.o ufs-sysfs.o ufs-
> > > > mcq.o
> > > > +ufshcd-core-$(CONFIG_RPMB) += ufs-rpmb.o
> > >
> > > SCSI_UFSHCD might need the same trick ("depends on RPMB || !RPMB") in
> > > Kconfig as we have for MMC_BLOCK.
> > >
> > > >
> > When RPMB=m and SCSI_UFSHCD=y, the ufs-rpmb.o is compiled into the built-in
> > ufshcd-core, ufs-rpmb.c calls functions from the OP-TEE RPMB subsystem
> > module,
> > The kernel allows built-in code to reference module symbols (they become
> > runtime
> > dependencies, not link-time), please check, I tested.
> >
> > > >
> > > >
> > >
> > > > +
> > > > + struct rpmb_descr descr = {
> > > > + .type = RPMB_TYPE_UFS,
> > >
> > > We'll need another type if the device uses the extended RPMB frame
> > > format. How about you clarify this, where RPMB_TYPE_UFS is defined to
> > > avoid confusion?
> >
> > As ufs-bsg.c, we could use ARPMB_TYPE_UFS for UFS advanced RPMB frame, if it
> > is
> > RPMB, we take it as normal RPMB, the frame should be the same as MMC RPMB.
>
> Isn't it a bit confusing to set the type to RPMB_TYPE_EMMC when it's
> actually a UFS RPMB, even if it's supposedly compatible enough?
>
The RPMB data format is the same for both eMMC RPMB and standard UFS RPMB.
However, the application commands used to access RPMB differ — eMMC uses MMC
commands, while UFS uses SCSI commands.
Additionally, UFS RPMB supports more RPMB operations than eMMC RPMB. Therefore,
we need to distinguish between them:
RPMB_TYPE_EMMC for eMMC RPMB
RPMB_TYPE_UFS for standard UFS RPMB
ARPMB_TYPE_UFS for advanced UFS RPMB.
> While the frame format works, I'm concerned about the CID. It's
> essentially a namespace of its own for eMMC, and at least the OP-TEE
> implementation makes assumptions about the format by masking out the
> PRV (Product Revision) and CRC (CRC7 checksum) fields from the CID
> when deriving the RPMB key. For this to work reliably, the CID must be
> guaranteed to be unique per RPMB device.
>
> From what I understand, for UFS, the serial number is only guaranteed
> to be unique if the manufacturer and the product name are taken into
> account. Combined, these fields can be much larger than 16 bytes, and
> we also have the partition number to consider.
>
> By using RPMB_TYPE_UFS we can define a device ID tailored for UFS with
> all the fields we need. Thoughts?
>
For certain memory vendors, the serial number is guaranteed to be unique among
all devices.
For partitions or regions, we have appended the region number to the end of the
CID — please check the patch for details.
Regarding improving CID uniqueness, we could include the OEM ID or product
number. However, this would make the CID longer than 16 bytes.
Kind regards,
Bean
> Cheers,
> Jens
On Mon, Oct 13, 2025 at 2:04 PM Bean Huo <huobean@gmail.com> wrote:
>
> On Mon, 2025-10-13 at 10:21 +0200, Jens Wiklander wrote:
> > Hi Bean,
> >
> >
> >
> > On Wed, Oct 8, 2025 at 5:07 PM Bean Huo <huobean@gmail.com> wrote:
> > >
> > > Jens,
> > >
> > > I incorporated your suggestions in my v3 excpet these two:
> > >
> > >
> > > On Wed, 2025-10-01 at 09:50 +0200, Jens Wiklander wrote:
> > > > > diff --git a/drivers/ufs/core/Makefile b/drivers/ufs/core/Makefile
> > > > > index cf820fa09a04..51e1867e524e 100644
> > > > > --- a/drivers/ufs/core/Makefile
> > > > > +++ b/drivers/ufs/core/Makefile
> > > > > @@ -2,6 +2,7 @@
> > > > >
> > > > > obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> > > > > ufshcd-core-y += ufshcd.o ufs-sysfs.o ufs-
> > > > > mcq.o
> > > > > +ufshcd-core-$(CONFIG_RPMB) += ufs-rpmb.o
> > > >
> > > > SCSI_UFSHCD might need the same trick ("depends on RPMB || !RPMB") in
> > > > Kconfig as we have for MMC_BLOCK.
> > > >
> > > > >
> > > When RPMB=m and SCSI_UFSHCD=y, the ufs-rpmb.o is compiled into the built-in
> > > ufshcd-core, ufs-rpmb.c calls functions from the OP-TEE RPMB subsystem
> > > module,
> > > The kernel allows built-in code to reference module symbols (they become
> > > runtime
> > > dependencies, not link-time), please check, I tested.
> > >
> > > > >
> > > > >
> > > >
> > > > > +
> > > > > + struct rpmb_descr descr = {
> > > > > + .type = RPMB_TYPE_UFS,
> > > >
> > > > We'll need another type if the device uses the extended RPMB frame
> > > > format. How about you clarify this, where RPMB_TYPE_UFS is defined to
> > > > avoid confusion?
> > >
> > > As ufs-bsg.c, we could use ARPMB_TYPE_UFS for UFS advanced RPMB frame, if it
> > > is
> > > RPMB, we take it as normal RPMB, the frame should be the same as MMC RPMB.
> >
> > Isn't it a bit confusing to set the type to RPMB_TYPE_EMMC when it's
> > actually a UFS RPMB, even if it's supposedly compatible enough?
> >
>
> The RPMB data format is the same for both eMMC RPMB and standard UFS RPMB.
> However, the application commands used to access RPMB differ — eMMC uses MMC
> commands, while UFS uses SCSI commands.
>
> Additionally, UFS RPMB supports more RPMB operations than eMMC RPMB. Therefore,
> we need to distinguish between them:
>
> RPMB_TYPE_EMMC for eMMC RPMB
>
> RPMB_TYPE_UFS for standard UFS RPMB
>
> ARPMB_TYPE_UFS for advanced UFS RPMB.
Good. A distinction was what I was asking for.
>
>
> > While the frame format works, I'm concerned about the CID. It's
> > essentially a namespace of its own for eMMC, and at least the OP-TEE
> > implementation makes assumptions about the format by masking out the
> > PRV (Product Revision) and CRC (CRC7 checksum) fields from the CID
> > when deriving the RPMB key. For this to work reliably, the CID must be
> > guaranteed to be unique per RPMB device.
> >
> > From what I understand, for UFS, the serial number is only guaranteed
> > to be unique if the manufacturer and the product name are taken into
> > account. Combined, these fields can be much larger than 16 bytes, and
> > we also have the partition number to consider.
> >
> > By using RPMB_TYPE_UFS we can define a device ID tailored for UFS with
> > all the fields we need. Thoughts?
> >
>
> For certain memory vendors, the serial number is guaranteed to be unique among
> all devices.
This is supposed to be generic and not rely on the behavior of only
some vendors.
>
> For partitions or regions, we have appended the region number to the end of the
> CID — please check the patch for details.
Yes, but how do you know that you don't overwrite a part of the serial number?
>
> Regarding improving CID uniqueness, we could include the OEM ID or product
> number. However, this would make the CID longer than 16 bytes.
UFS doesn't have a CID, but there's no need for one either. struct
rpmb_descr has dev_id and dev_id_len. It can be any length, within
reason.
Cheers,
Jens
>
>
>
> Kind regards,
> Bean
>
> > Cheers,
> > Jens
>
On Mon, 2025-10-13 at 14:22 +0200, Jens Wiklander wrote: > > > > For certain memory vendors, the serial number is guaranteed to be unique > > among > > all devices. > > This is supposed to be generic and not rely on the behavior of only > some vendors. > > > > > For partitions or regions, we have appended the region number to the end of > > the > > CID — please check the patch for details. > > Yes, but how do you know that you don't overwrite a part of the serial number? > > > > > Regarding improving CID uniqueness, we could include the OEM ID or product > > number. However, this would make the CID longer than 16 bytes. > > UFS doesn't have a CID, but there's no need for one either. struct > rpmb_descr has dev_id and dev_id_len. It can be any length, within > reason. > > Cheers, > Jens Hi Jens, how about combining wManufacturerID, wManufactureDate, wDeviceVersion, Serial Number (in unicode), plus the region number? Kind regards, Bean
On Mon, Oct 13, 2025 at 5:43 PM Bean Huo <beanhuo@iokpp.de> wrote: > > On Mon, 2025-10-13 at 14:22 +0200, Jens Wiklander wrote: > > > > > > For certain memory vendors, the serial number is guaranteed to be unique > > > among > > > all devices. > > > > This is supposed to be generic and not rely on the behavior of only > > some vendors. > > > > > > > > For partitions or regions, we have appended the region number to the end of > > > the > > > CID — please check the patch for details. > > > > Yes, but how do you know that you don't overwrite a part of the serial number? > > > > > > > > Regarding improving CID uniqueness, we could include the OEM ID or product > > > number. However, this would make the CID longer than 16 bytes. > > > > UFS doesn't have a CID, but there's no need for one either. struct > > rpmb_descr has dev_id and dev_id_len. It can be any length, within > > reason. > > > > Cheers, > > Jens > > > Hi Jens, > > how about combining wManufacturerID, wManufactureDate, wDeviceVersion, Serial > Number (in unicode), plus the region number? Sounds good. Cheers, Jens > > > Kind regards, > Bean
On Mon, 2025-10-13 at 17:53 +0200, Jens Wiklander wrote: > > Hi Jens, > > > > how about combining wManufacturerID, wManufactureDate, wDeviceVersion, > > Serial > > Number (in unicode), plus the region number? > > Sounds good. > > Cheers, > Jens Jens, please check the new version patch v5 I have sent, in which, I addressed your concern. Kind regards, Bean
On Wed, 2025-10-01 at 09:50 +0200, Jens Wiklander wrote:
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -5240,10 +5240,15 @@ static void ufshcd_lu_init(struct ufs_hba *hba,
> > struct scsi_device *sdev)
> > desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] == UFS_LU_POWER_ON_WP)
> > hba->dev_info.is_lu_power_on_wp = true;
> >
> > - /* In case of RPMB LU, check if advanced RPMB mode is enabled */
> > - if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN &&
> > - desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
> > - hba->dev_info.b_advanced_rpmb_en = true;
> > + /* In case of RPMB LU, check if advanced RPMB mode is enabled, and
> > get region size */
> > + if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN) {
> > + if (desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
> > + hba->dev_info.b_advanced_rpmb_en = true;
>
> Does this indicate that the other RPMB frame format is used?
yes, if BIT4 is 1, means Advanced RPMB is enabled, from the Spec:
"There are two RPMB modes; Normal RPMB mode and Advanced RPMB mode using EHS.
The RPMB mode can be configured by setting Bit4 of bRPMBRegionEnable parameter
of the RPMB descriptor in the configuration stage. If the device receives an
RPMB operation request of a different mode than the configured RPMB mode, the
device shall respond with ILLEGAL REQUEST."
if it is in Advanced RPMB mode, and use sends normal RPMB frame, the device will
respond ILLEGAL REQUEST in response. but it is better to mute this noise, I
will add this in next version: if it is in advanced RPMB, we will not call RPMB
probe.
Regarding the advanced RPMB, its frame is quite different with normal RPMB,
since we need to pass EHS, not just RPMB frame as we use. It is better to have a
check you and us online, see which way is better to enalbe it in op-tee OS.
Kind regards,
Bean
© 2016 - 2026 Red Hat, Inc.