linux-kernel - RE: [<RFC PATCH v1> 1/2] scsi: ufs: add write booster feature support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <MN2PR04MB6991F821F0613DE77AA18A15FCED0@MN2PR04MB6991.namprd04.prod.outlook.com>
Date:   Tue, 25 Feb 2020 11:21:53 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Asutosh Das <asutoshd@...eaurora.org>,
        "subhashj@...eaurora.org" <subhashj@...eaurora.org>,
        "cang@...eaurora.org" <cang@...eaurora.org>,
        "rnayak@...eaurora.org" <rnayak@...eaurora.org>,
        "vinholikatti@...il.com" <vinholikatti@...il.com>,
        "jejb@...ux.vnet.ibm.com" <jejb@...ux.vnet.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
CC:     "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Stanley Chu <stanley.chu@...iatek.com>,
        Bean Huo <beanhuo@...ron.com>,
        Tomas Winkler <tomas.winkler@...el.com>,
        Colin Ian King <colin.king@...onical.com>,
        Bart Van Assche <bvanassche@....org>,
        Venkat Gopalakrishnan <venkatg@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: RE: [<RFC PATCH v1> 1/2] scsi: ufs: add write booster feature support

> > +       /* Enable WB only for UFS-3.1 OR if desc len >= 0x59 */
> You don't really check that the descriptor size is large enough
> 
> > +       if (dev_info->wspecversion >= 0x310) {
> > +               hba->dev_info.d_ext_ufs_feature_sup =
> > +                       desc_buf[DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP]
> > +                                                               << 24 |
> > +                       desc_buf[DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP +
> 1]
> > +                                                               << 16 |
> > +                       desc_buf[DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP +
> 2]
> > +                                                               << 8 |
> > +                       desc_buf[DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP +
> 3];
> > +
> > +               hba->dev_info.b_wb_buffer_type =
> > +                       desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
> > +
> > +               if (hba->dev_info.b_wb_buffer_type)
> > +                       goto skip_unit_desc;
> > +
> > +               hba->dev_info.wb_config_lun = false;
> > +               for (lun = 0; lun < UFS_UPIU_MAX_GENERAL_LUN; lun++) {
Also lun not defined

> You forgot to set
> u8 wb_buf[4] = {};
> 
> > +                       memset(wb_buf, 0, sizeof(wb_buf));
> Not needed
> 
> > +                       err = ufshcd_get_wb_alloc_units(hba, lun, wb_buf);
> > +                       if (err)
> > +                               break;
> > +
> > +                       res = wb_buf[0] << 24 | wb_buf[1] << 16 |
> > +                               wb_buf[2] << 8 | wb_buf[3];
And res not defined as well.

> > +                       if (res) {
> > +                               hba->dev_info.wb_config_lun = true;
> > +                               break;
> > +                       }
> > +               }
> > +       }
> > +
> 
> Thanks,
> Avri