In UFS 4.0, it introduced advanced RPMB, which can significantly improve RPMB's command
performance, enhancing its atomic operation. We don't know which implementation will please
everyone, mark this advanced RPMB patch as RFC. Any suggestions to make the patch a master
patch are welcome.
Based on suggestions and feedback from Hannes Reinecke and Bart, we can use job_bsg->request
and job_bsg->reply to pass EHS packets without changing the BSG V4 structure and BSG core. So
we push RFC patch just to start Advanced RPMB mainlining
Bean Huo (2):
ufs: core: Advanced RPMB detection
ufs: core: Add advanced RPMB support in ufs_bsg
drivers/ufs/core/ufs_bsg.c | 115 +++++++++++++---------
drivers/ufs/core/ufshcd.c | 161 ++++++++++++++++++++++++-------
include/uapi/scsi/scsi_bsg_ufs.h | 30 +++++-
include/ufs/ufs.h | 3 +
include/ufs/ufshcd.h | 5 +
5 files changed, 233 insertions(+), 81 deletions(-)
--
2.25.1
From: Bean Huo <[email protected]>
Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().
Signed-off-by: Bean Huo <[email protected]>
---
drivers/ufs/core/ufshcd.c | 4 ++++
include/ufs/ufs.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ee73d7036133..d49e7a0b82ca 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4940,6 +4940,10 @@ 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;
+ if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_RPMB_UNIT &&
+ desc_buf[UNIT_DESC_PARAM_RPMB_REGION_EN] & 1 << 4)
+ hba->dev_info.b_advanced_rpmb_en = true;
+
kfree(desc_buf);
set_qdepth:
/*
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 1bba3fead2ce..2e617ab87750 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -199,6 +199,7 @@ enum unit_desc_param {
UNIT_DESC_PARAM_PSA_SENSITIVE = 0x7,
UNIT_DESC_PARAM_MEM_TYPE = 0x8,
UNIT_DESC_PARAM_DATA_RELIABILITY = 0x9,
+ UNIT_DESC_PARAM_RPMB_REGION_EN = 0x9,
UNIT_DESC_PARAM_LOGICAL_BLK_SIZE = 0xA,
UNIT_DESC_PARAM_LOGICAL_BLK_COUNT = 0xB,
UNIT_DESC_PARAM_ERASE_BLK_SIZE = 0x13,
@@ -601,6 +602,8 @@ struct ufs_dev_info {
bool b_rpm_dev_flush_capable;
u8 b_presrv_uspc_en;
+
+ bool b_advanced_rpmb_en;
};
/*
--
2.25.1
Hi,
> In UFS 4.0, it introduced advanced RPMB, which can significantly improve
> RPMB's command performance, enhancing its atomic operation. We don't
> know which implementation will please everyone, mark this advanced RPMB
> patch as RFC. Any suggestions to make the patch a master patch are welcome.
>
> Based on suggestions and feedback from Hannes Reinecke and Bart, we can
> use job_bsg->request and job_bsg->reply to pass EHS packets without changing
> the BSG V4 structure and BSG core.
Can you share the reference to this mail thread, or was it a privet discussion?
Thanks,
Avri
>So we push RFC patch just to start
> Advanced RPMB mainlining
>
> Bean Huo (2):
> ufs: core: Advanced RPMB detection
> ufs: core: Add advanced RPMB support in ufs_bsg
>
> drivers/ufs/core/ufs_bsg.c | 115 +++++++++++++---------
> drivers/ufs/core/ufshcd.c | 161 ++++++++++++++++++++++++-------
> include/uapi/scsi/scsi_bsg_ufs.h | 30 +++++-
> include/ufs/ufs.h | 3 +
> include/ufs/ufshcd.h | 5 +
> 5 files changed, 233 insertions(+), 81 deletions(-)
>
> --
> 2.25.1
On Tue, 2022-11-08 at 12:37 +0000, Avri Altman wrote:
> Hi,
>
> > In UFS 4.0, it introduced advanced RPMB, which can significantly
> > improve
> > RPMB's command performance, enhancing its atomic operation. We
> > don't
> > know which implementation will please everyone, mark this advanced
> > RPMB
> > patch as RFC. Any suggestions to make the patch a master patch are
> > welcome.
> > Based on suggestions and feedback from Hannes Reinecke and Bart, we
> > can
> > use job_bsg->request and job_bsg->reply to pass EHS packets without
> > changing
> > the BSG V4 structure and BSG core.
>
> Can you share the reference to this mail thread, or was it a privet
> discussion?
>
>
>
> Thanks,
>
> Avri
Avri,
Yes, this is a private discussion during this year's Storage Summit wit
h Hannes Reinecke on the first two proposals below, and a private
discussion with Bart on the following three proposals
1. Use current BSG v4, and transmit EHS in sense_buffer, which is rejected.
2. The optional suggestion is to use ufs_bsg, which is the patch.
3. New RPMB framework, but we should enable UFS/eMMC RPMB driver as
well in ufs/emmc core, also, the command will be passed to kernel over
ioctl(). interested in this one, But Bart suggested using io_uing
framework. Since RPMB operation is atomic required, we found it is not
safe to use io_uring now, this need passthorugh support SCSI layer as
well.
Kind regards,
Bean
> From: Bean Huo <[email protected]>
>
> Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().
>
> Signed-off-by: Bean Huo <[email protected]>
> ---
> drivers/ufs/core/ufshcd.c | 4 ++++
> include/ufs/ufs.h | 3 +++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> ee73d7036133..d49e7a0b82ca 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4940,6 +4940,10 @@ 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;
>
> + if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_RPMB_UNIT &&
Please remind me why do we need both UFS_RPMB_UNIT and UFS_UPIU_RPMB_WLUN ?
> + desc_buf[UNIT_DESC_PARAM_RPMB_REGION_EN] & 1 << 4)
(1 << 4) or BIT(4) ?
> + hba->dev_info.b_advanced_rpmb_en = true;
> +
> kfree(desc_buf);
> set_qdepth:
> /*
> diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h index
> 1bba3fead2ce..2e617ab87750 100644
> --- a/include/ufs/ufs.h
> +++ b/include/ufs/ufs.h
> @@ -199,6 +199,7 @@ enum unit_desc_param {
> UNIT_DESC_PARAM_PSA_SENSITIVE = 0x7,
> UNIT_DESC_PARAM_MEM_TYPE = 0x8,
> UNIT_DESC_PARAM_DATA_RELIABILITY = 0x9,
> + UNIT_DESC_PARAM_RPMB_REGION_EN = 0x9,
This is awkward. Better to define it, or -
Maybe it's time for rpmb to have its own unit descriptor - it surely deserve it.
Thanks,
Avri
> UNIT_DESC_PARAM_LOGICAL_BLK_SIZE = 0xA,
> UNIT_DESC_PARAM_LOGICAL_BLK_COUNT = 0xB,
> UNIT_DESC_PARAM_ERASE_BLK_SIZE = 0x13,
> @@ -601,6 +602,8 @@ struct ufs_dev_info {
>
> bool b_rpm_dev_flush_capable;
> u8 b_presrv_uspc_en;
> +
> + bool b_advanced_rpmb_en;
> };
>
> /*
> --
> 2.25.1
Avri,
thanks for your review.
On Tue, 2022-11-08 at 13:40 +0000, Avri Altman wrote:
> > From: Bean Huo <[email protected]>
> >
> > Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().
> >
> > Signed-off-by: Bean Huo <[email protected]>
> > ---
> > drivers/ufs/core/ufshcd.c | 4 ++++
> > include/ufs/ufs.h | 3 +++
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index
> > ee73d7036133..d49e7a0b82ca 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -4940,6 +4940,10 @@ 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;
> >
> > + if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_RPMB_UNIT
> > &&
> Please remind me why do we need both UFS_RPMB_UNIT and
> UFS_UPIU_RPMB_WLUN ?
I see. they are the same value, we should remove one, will change it in
next version.
>
> > + desc_buf[UNIT_DESC_PARAM_RPMB_REGION_EN] & 1 << 4)
> (1 << 4) or BIT(4) ?
>
> > + hba->dev_info.b_advanced_rpmb_en = true;
> > +
> > kfree(desc_buf);
> > set_qdepth:
> > /*
> > diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h index
> > 1bba3fead2ce..2e617ab87750 100644
> > --- a/include/ufs/ufs.h
> > +++ b/include/ufs/ufs.h
> > @@ -199,6 +199,7 @@ enum unit_desc_param {
> > UNIT_DESC_PARAM_PSA_SENSITIVE = 0x7,
> > UNIT_DESC_PARAM_MEM_TYPE = 0x8,
> > UNIT_DESC_PARAM_DATA_RELIABILITY = 0x9,
> > + UNIT_DESC_PARAM_RPMB_REGION_EN = 0x9,
> This is awkward. Better to define it, or -
> Maybe it's time for rpmb to have its own unit descriptor - it surely
> deserve it.
>
no problem, let me think about it, will add in the next version.
Kind regards,
Bean
> Avri,
>
> thanks for your review.
>
> On Tue, 2022-11-08 at 13:40 +0000, Avri Altman wrote:
> > > From: Bean Huo <[email protected]>
> > >
> > > Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().
> > >
> > > Signed-off-by: Bean Huo <[email protected]>
> > > ---
> > > drivers/ufs/core/ufshcd.c | 4 ++++
> > > include/ufs/ufs.h | 3 +++
> > > 2 files changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > > index ee73d7036133..d49e7a0b82ca 100644
> > > --- a/drivers/ufs/core/ufshcd.c
> > > +++ b/drivers/ufs/core/ufshcd.c
> > > @@ -4940,6 +4940,10 @@ 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;
> > >
> > > + if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_RPMB_UNIT
> > > &&
> > Please remind me why do we need both UFS_RPMB_UNIT and
> > UFS_UPIU_RPMB_WLUN ?
>
> I see. they are the same value, we should remove one, will change it in next
> version.
> >
> > > + desc_buf[UNIT_DESC_PARAM_RPMB_REGION_EN] & 1 << 4)
> > (1 << 4) or BIT(4) ?
Not saying that testing bit 4 of bRPMBRegionEnable is wrong,
Have you considered using bit 10 of dExtendedUFSFeaturesSupport and decided otherwise?
Thanks,
Avri
> In UFS 4.0, it introduced advanced RPMB, which can significantly improve
> RPMB's command performance, enhancing its atomic operation. We don't
> know which implementation will please everyone, mark this advanced RPMB
> patch as RFC. Any suggestions to make the patch a master patch are welcome.
>
> Based on suggestions and feedback from Hannes Reinecke and Bart, we can
> use job_bsg->request and job_bsg->reply to pass EHS packets without changing
> the BSG V4 structure and BSG core. So we push RFC patch just to start
> Advanced RPMB mainlining
I concur with this approach.
The current limitations that the new spec imposes,
e.g. putting confidential data in a construct that lives in the ufs-driver,
practically gives no other alternative but ufs-bsg.
If no one else object, maybe you can leave out the rfc from the next version.
Thanks,
Avri
>
> Bean Huo (2):
> ufs: core: Advanced RPMB detection
> ufs: core: Add advanced RPMB support in ufs_bsg
>
> drivers/ufs/core/ufs_bsg.c | 115 +++++++++++++---------
> drivers/ufs/core/ufshcd.c | 161 ++++++++++++++++++++++++-------
> include/uapi/scsi/scsi_bsg_ufs.h | 30 +++++-
> include/ufs/ufs.h | 3 +
> include/ufs/ufshcd.h | 5 +
> 5 files changed, 233 insertions(+), 81 deletions(-)
>
> --
> 2.25.1
Avri,
Thanks for your suggetions and review
On Wed, 2022-11-09 at 08:18 +0000, Avri Altman wrote:
> > In UFS 4.0, it introduced advanced RPMB, which can significantly
> > improve
> > RPMB's command performance, enhancing its atomic operation. We
> > don't
> > know which implementation will please everyone, mark this advanced
> > RPMB
> > patch as RFC. Any suggestions to make the patch a master patch are
> > welcome.
> > Based on suggestions and feedback from Hannes Reinecke and Bart, we
> > can
> > use job_bsg->request and job_bsg->reply to pass EHS packets without
> > changing
> > the BSG V4 structure and BSG core. So we push RFC patch just to
> > start
> > Advanced RPMB mainlining
>
> I concur with this approach.
>
> The current limitations that the new spec imposes,
>
> e.g. putting confidential data in a construct that lives in the ufs-
> driver,
>
> practically gives no other alternative but ufs-bsg.
>
>
>
> If no one else object, maybe you can leave out the rfc from the next
> version.
>
>
I will prepare next version, and address your all questions in the next
version. thanks.
Kind regards,
Bean