[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHUa44HYNgiRU5yOrVq8gBwHEEAxGz6TyT0PrBpVOiFfKxYhOQ@mail.gmail.com>
Date: Mon, 13 Oct 2025 14:22:26 +0200
From: Jens Wiklander <jens.wiklander@...aro.org>
To: Bean Huo <huobean@...il.com>
Cc: avri.altman@....com, bvanassche@....org, alim.akhtar@...sung.com,
jejb@...ux.ibm.com, martin.petersen@...cle.com, can.guo@....qualcomm.com,
ulf.hansson@...aro.org, beanhuo@...ron.com, linux-scsi@...r.kernel.org,
linux-kernel@...r.kernel.org, beanhuo@...pp.de
Subject: Re: [PATCH v2 3/3] scsi: ufs: core: Add OP-TEE based RPMB driver for
UFS devices
On Mon, Oct 13, 2025 at 2:04 PM Bean Huo <huobean@...il.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@...il.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
>
Powered by blists - more mailing lists