| Message ID | 20221107131038.201724-1-beanhuo@iokpp.de |
|---|---|
| Headers | show |
| Series | UFS Advanced RPMB | expand |
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
Avri, thanks for your review. On Tue, 2022-11-08 at 13:40 +0000, Avri Altman wrote: > > From: Bean Huo <beanhuo@micron.com> > > > > Check UFS Advanced RPMB LU enablement during ufshcd_lu_init(). > > > > Signed-off-by: Bean Huo <beanhuo@micron.com> > > --- > > 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 <beanhuo@micron.com> > > > > > > Check UFS Advanced RPMB LU enablement during ufshcd_lu_init(). > > > > > > Signed-off-by: Bean Huo <beanhuo@micron.com> > > > --- > > > 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