| 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
| ||
|
Message-ID: <8d618210-0f83-47cc-d0c4-cc4343e4d51c@codeaurora.org>
Date: Mon, 16 Mar 2020 16:52:08 -0700
From: "Asutosh Das (asd)" <asutoshd@...eaurora.org>
To: Avri Altman <Avri.Altman@....com>,
"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
On 2/25/2020 4:50 AM, Avri Altman wrote:
>> +/*
>> + * ufshcd_get_wb_alloc_units - returns
>> "dLUNumWriteBoosterBufferAllocUnits"
>> + * @hba: per-adapter instance
>> + * @lun: UFS device lun id
>> + * @d_lun_wbb_au: pointer to buffer to hold the LU's alloc units info
>> + *
>> + * Returns 0 in case of success and d_lun_wbb_au would be returned
>> + * Returns -ENOTSUPP if reading d_lun_wbb_au is not supported.
>> + * Returns -EINVAL in case of invalid parameters passed to this function.
>> + */
>> +static int ufshcd_get_wb_alloc_units(struct ufs_hba *hba,
>> + u8 lun,
>> + u8 *d_lun_wbb_au)
>> +{
>> + int ret;
>> +
>> + if (!d_lun_wbb_au)
>> + ret = -EINVAL;
>> +
>> + /* WB can be supported only from LU0..LU7 */
>> + else if (lun >= UFS_UPIU_MAX_GENERAL_LUN)
>> + ret = -ENOTSUPP;
>> + else
>> + ret = ufshcd_read_unit_desc_param(hba,
>> + lun,
>> + UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
>> + d_lun_wbb_au,
>> + sizeof(*d_lun_wbb_au));
> You are reading here a single byte, instead of 4
>
>> + return ret;
>> +}
>> +
>> /**
>> * ufshcd_get_lu_power_on_wp_status - get LU's power on write protect
>> * status
>> @@ -5194,6 +5267,165 @@ static void
>> ufshcd_bkops_exception_event_handler(struct ufs_hba *hba)
>> __func__, err);
>> }
>>
>> +static bool ufshcd_wb_sup(struct ufs_hba *hba)
>> +{
>> + return ((hba->dev_info.d_ext_ufs_feature_sup &
>> + UFS_DEV_WRITE_BOOSTER_SUP) &&
> Don't you want to have a vendor cap as well,
> to allow the platform vendor to control this feature?
I presume each platform vendor would provide a provisioning
script/method which would configure the WB properties.
It can be controlled through that.
>
>> + (hba->dev_info.b_wb_buffer_type
>> + || hba->dev_info.wb_config_lun));
>> +}
>> +
>> +
>
>
>
>> +static bool ufshcd_wb_is_buf_flush_needed(struct ufs_hba *hba)
>> +{
>> + int ret;
>> + u32 cur_buf, status, avail_buf;
>> +
>> + if (!ufshcd_wb_sup(hba))
>> + return false;
>> +
>> + ret = ufshcd_query_attr_retry(hba,
>> UPIU_QUERY_OPCODE_READ_ATTR,
>> + QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE,
>> + 0, 0, &avail_buf);
>> + if (ret) {
>> + dev_warn(hba->dev, "%s dAvailableWriteBoosterBufferSize read
>> failed %d\n",
>> + __func__, ret);
>> + return false;
>> + }
>> +
>> + ret = ufshcd_vops_get_user_cap_mode(hba);
>> + if (ret <= 0) {
>> + dev_dbg(hba->dev, "Get user-cap reduction mode: failed: %d\n",
>> + ret);
>> + /* Most commonly used */
>> + ret = UFS_WB_BUFF_PRESERVE_USER_SPACE;
>> + }
>> +
>> + hba->dev_info.keep_vcc_on = false;
>> + if (ret == UFS_WB_BUFF_USER_SPACE_RED_EN) {
>> + if (avail_buf <= UFS_WB_10_PERCENT_BUF_REMAIN) {
>> + hba->dev_info.keep_vcc_on = true;
>> + return true;
>> + }
>> + return false;
>> + } else if (ret == UFS_WB_BUFF_PRESERVE_USER_SPACE) {
>> + ret = ufshcd_query_attr_retry(hba,
>> UPIU_QUERY_OPCODE_READ_ATTR,
>> + QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE,
>> + 0, 0, &cur_buf);
>> + if (ret) {
>> + dev_err(hba->dev, "%s dCurWriteBoosterBufferSize read failed
>> %d\n",
>> + __func__, ret);
>> + return false;
>> + }
>> +
>> + if (!cur_buf) {
>> + dev_info(hba->dev, "dCurWBBuf: %d WB disabled until free-
>> space is available\n",
>> + cur_buf);
>> + return false;
>> + }
>> +
>> + ret = ufshcd_get_ee_status(hba, &status);
>> + if (ret) {
>> + dev_err(hba->dev, "%s: failed to get exception status %d\n",
>> + __func__, ret);
>> + if (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN) {
>> + hba->dev_info.keep_vcc_on = true;
>> + return true;
>> + }
>> + return false;
>> + }
>> +
>> + status &= hba->ee_ctrl_mask;
>> +
>> + if ((status & MASK_EE_URGENT_BKOPS) ||
> So you are getting the status, but not the bkops level.
> And what about WRITEBOOSTER_EVENT_EN? After all it was invented specifically for WB...
Yeah - WB exception event is not supported in this series. I will push
another change to support that.
With WB enabled, any BKOPS exception level warrants the vcc to be on.
This is to minimize performance impact. Please correct this
understanding from device perspective.
>
>> + (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN)) {
>> + hba->dev_info.keep_vcc_on = true;
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>
> Thanks,
> Avri
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
Powered by blists - more mailing lists