On 7/23/26 9:38 AM, Jorge Ramirez-Ortiz wrote:
> OP-TEE computes its RPMB authentication key from a device identifier, so
> U-Boot must hand it the exact same identifier the Linux kernel uses;
> otherwise the derived key differs and OP-TEE cannot access RPMB secure
> storage provisioned under Linux, and vice versa. The OP-TEE RPMB probe
> also needs each region's size and reliable-write count to discover which
> regions exist and how they may be written. Provide both so OP-TEE secure
> storage stays interoperable across U-Boot and Linux on the same device.
>
> Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> ---
> drivers/ufs/Kconfig | 7 +-
> drivers/ufs/ufs-rpmb.c | 148 +++++++++++++++++++++++++++++++++++++++++
> include/ufs.h | 6 ++
> 3 files changed, 159 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
> index 99160a0819b..d39fcda42dc 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 fb4cd42ad75..ed5ecc0a5e6 100644
> --- a/drivers/ufs/ufs-rpmb.c
> +++ b/drivers/ufs/ufs-rpmb.c
> @@ -1,10 +1,14 @@
> // SPDX-License-Identifier: GPL-2.0+
> #include <dm.h>
> +#include <hexdump.h>
> #include <log.h>
> #include <malloc.h>
> #include <scsi.h>
> #include <ufs.h>
> +#include <vsprintf.h>
> +#include <u-boot/blake2.h>
These are mostly in alphabetical order, so would make sense to put
<u-boot/... after <linux/...
> #include <asm/cache.h>
> +#include <asm/unaligned.h>
> #include <linux/errno.h>
> #include <linux/string.h>
> #include "ufs.h"
> @@ -25,6 +29,15 @@
>
> #define GEOMETRY_DESC_RPMB_RW_SIZE 0x17
>
> +#define RPMB_UNIT_DESC_LOGICAL_BLK_SIZE 0x0A
> +#define RPMB_UNIT_DESC_LOGICAL_BLK_COUNT 0x0B
> +#define RPMB_UNIT_DESC_REGION0_SIZE 0x13
> +#define RPMB_UNIT_DESC_REGION1_SIZE 0x14
> +#define RPMB_UNIT_DESC_REGION2_SIZE 0x15
> +#define RPMB_UNIT_DESC_REGION3_SIZE 0x16
> +#define UFS_RPMB_LEGACY_SPEC_VER 0x0220
> +#define UFS_RPMB_REGION_UNIT_SHIFT 17
> +
> static u16 rpmb_frame_request(const void *frame)
> {
> const u8 *p = frame;
> @@ -171,3 +184,138 @@ static int ufs_rpmb_read_geometry(struct udevice
> *scsi_dev, u8 *rpmb_rw_size)
>
> return 0;
> }
> +
> +static int ufs_rpmb_read_region_sizes(struct ufs_hba *hba, u16 spec_ver,
> + u8 sizes[UFS_RPMB_NUM_REGIONS])
> +{
> + u8 unit[QUERY_DESC_UNIT_DEF_SIZE] = { };
> + int ret;
> +
> + ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT,
> + UFS_UPIU_RPMB_WLUN, 0, unit, sizeof(unit));
> + if (ret)
> + return ret;
> +
> + memset(sizes, 0, UFS_RPMB_NUM_REGIONS);
> +
> + if (spec_ver > UFS_RPMB_LEGACY_SPEC_VER) {
> + sizes[0] = unit[RPMB_UNIT_DESC_REGION0_SIZE];
> + sizes[1] = unit[RPMB_UNIT_DESC_REGION1_SIZE];
> + sizes[2] = unit[RPMB_UNIT_DESC_REGION2_SIZE];
> + sizes[3] = unit[RPMB_UNIT_DESC_REGION3_SIZE];
> + } else {
> + u64 region = (get_unaligned_be64(unit +
> + RPMB_UNIT_DESC_LOGICAL_BLK_COUNT)
> + << unit[RPMB_UNIT_DESC_LOGICAL_BLK_SIZE])
> + >> UFS_RPMB_REGION_UNIT_SHIFT;
A bit hard to read. Maybe like this?
u64 region;
region = get_unaligned_be64(unit +
RPMB_UNIT_DESC_LOGICAL_BLK_COUNT);
region <<= unit[RPMB_UNIT_DESC_LOGICAL_BLK_SIZE];
region >>= UFS_RPMB_REGION_UNIT_SHIFT;
Also probably worth checking value of unit[RPMB_UNIT_DESC_LOGICAL_BLK_SIZE]
before
using it if comes from a potentially untrusted source.
> +
> + 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';
> +}
This looks similar to ufshcd_remove_non_printable(). Should we try
to share code more?
> +
> +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);
> + bin2hex(serial_hex, serial, blen);
Why isn't this also blen - QUERY_DESC_HDR_SIZE? Is the 00 padding intentional?
> + 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;
> +}
> +
