RE:(2) [PATCH 2/3] scsi: ufs: Map the correct size to the rpmb unit descriptor

[Daejun Park <daejun7.park@samsung.com>]

Hi,

> > Hi Avri,
> > 
> > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index
> > > 579cf6f9e7a1..d0be8d4c8091 100644
> > > --- a/drivers/scsi/ufs/ufs.h
> > > +++ b/drivers/scsi/ufs/ufs.h
> > > @@ -167,6 +167,7 @@ enum desc_idn {
> > >          QUERY_DESC_IDN_GEOMETRY                = 0x7,
> > >          QUERY_DESC_IDN_POWER                = 0x8,
> > >          QUERY_DESC_IDN_HEALTH           = 0x9,
> > > +        QUERY_DESC_IDN_UNIT_RPMB        = 0xA,
> > >          QUERY_DESC_IDN_MAX,
> > 
> > By adding QUERY_DESC_IDN_UNIT_RPMB, the value of
> > QUERY_DESC_IDN_MAX is changed to 0xB.
> > ...
> Yes
>  
> > 
> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > index 74ccfd2b80ce..eec1bc95391b 100644
> > > --- a/drivers/scsi/ufs/ufshcd.c
> > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > @@ -3319,11 +3319,13 @@ int ufshcd_query_descriptor_retry(struct ufs_hba
> > *hba,
> > >   * @desc_len: mapped desc length (out)
> > >   */
> > >  void ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn
> > desc_id,
> > > -                                  int *desc_len)
> > > +                                  int desc_index, int *desc_len)
> > >  {
> > >          if (desc_id >= QUERY_DESC_IDN_MAX || desc_id ==
> > QUERY_DESC_IDN_RFU_0 ||
> > >              desc_id == QUERY_DESC_IDN_RFU_1)
> > >                  *desc_len = 0;
> > 
> > So, if user sending desc_id as 0xA, it can not be detected as invalid descriptor.
> Which user?
> Oh, you mean if someone uses the ufs-bsg with some upiu-based query app, like ufs-utils?
> Well, those apps are for developers and field engineers, expected to know what they are doing...

Yes, but checking QUERY_DESC_IDN_MAX can be useless because of adding entry in enum desc_idn.
 
> Alternatively, maybe its better to just remove the unit descriptor sysfs entries for wlun altogether?
> They really meant nothing and shouldn't be there in the first place.
> What do you think?

Although if they were removed, ufs-bsg can access unit descriptors of wlun.
But it can be OK because developers are expected to access unit descriptors of wlun correctly.
So, I think it can be a solution.

> Thanks,
> Avri 
> > 
> > > +        else if (desc_index == UFS_UPIU_RPMB_WLUN)
> > > +                *desc_len = hba->desc_size[QUERY_DESC_IDN_UNIT_RPMB];
> > >          else
> > >                  *desc_len = hba->desc_size[desc_id];  }
> > 
> > Thanks,
> > Daejun
>  
>  
>