Re: [PATCH v2 3/3] scsi: ufs: core: Add OP-TEE based RPMB driver for UFS devices

[Bean Huo <huobean@gmail.com>]

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