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 (林智斌) <[email protected]> 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/[email protected]/ > 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
