| 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: <1796371666.21650859682732.JavaMail.epsvc@epcpadp4>
Date: Mon, 25 Apr 2022 12:54:04 +0900
From: Keoseong Park <keosung.park@...sung.com>
To: Bean Huo <huobean@...il.com>,
ALIM AKHTAR <alim.akhtar@...sung.com>,
"avri.altman@....com" <avri.altman@....com>,
"jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
"stanley.chu@...iatek.com" <stanley.chu@...iatek.com>,
"beanhuo@...ron.com" <beanhuo@...ron.com>,
"bvanassche@....org" <bvanassche@....org>,
"tomas.winkler@...el.com" <tomas.winkler@...el.com>,
Daejun Park <daejun7.park@...sung.com>,
Keoseong Park <keosung.park@...sung.com>,
"peter.wang@...iatek.com" <peter.wang@...iatek.com>,
"powen.kao@...iatek.com" <powen.kao@...iatek.com>,
cpgsproxy3 <cpgsproxy3@...sung.com>
CC: "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 5/6] scsi: ufshpb: Add handing of device reset HPB
regions Infos in HPB device mode
>From: Bean Huo <beanhuo@...ron.com>
>
>In UFS HPB Spec JESD220-3A,
>
>"5.8. Active and inactive information upon power cycle
>...
>When the device is powered off by the host, the device may restore L2P map data
>upon power up or build from the host’s HPB READ command. In case device powered
>up and lost HPB information, device can signal to the host through HPB Sense data,
>by setting HPB Operation as ‘2’ which will inform the host that device reset HPB
>information."
>
>Therefore, for HPB device control mode, if the UFS device is reset via the RST_N
>pin, the active region information in the device will be reset. If the host side
>receives this notification from the device side, it is recommended to inactivate
>all active regions in the host's HPB cache.
>
>Signed-off-by: Bean Huo <beanhuo@...ron.com>
>---
> drivers/scsi/ufs/ufshpb.c | 82 +++++++++++++++++++++++++++------------
> 1 file changed, 58 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
>index e7f311bb4401..7868412054bf 100644
>--- a/drivers/scsi/ufs/ufshpb.c
>+++ b/drivers/scsi/ufs/ufshpb.c
>@@ -1137,6 +1137,39 @@ static int ufshpb_add_region(struct ufshpb_lu *hpb, struct ufshpb_region *rgn)
> spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
> return ret;
> }
>+/**
>+ *ufshpb_submit_region_inactive() - submit a region to be inactivated later
>+ *@hpb: per-LU HPB instance
>+ *@...ion_index: the index associated with the region that will be inactivated later
>+ */
>+static void ufshpb_submit_region_inactive(struct ufshpb_lu *hpb, int region_index)
>+{
>+ int subregion_index;
>+ struct ufshpb_region *rgn;
>+ struct ufshpb_subregion *srgn;
>+
>+ /*
>+ * Remove this region from active region list and add it to inactive list
>+ */
>+ spin_lock(&hpb->rsp_list_lock);
>+ ufshpb_update_inactive_info(hpb, region_index);
>+ spin_unlock(&hpb->rsp_list_lock);
>+
>+ rgn = hpb->rgn_tbl + region_index;
>+
>+ /*
>+ * Set subregion state to be HPB_SRGN_INVALID, there will no HPB read on this subregion
>+ */
>+ spin_lock(&hpb->rgn_state_lock);
>+ if (rgn->rgn_state != HPB_RGN_INACTIVE) {
>+ for (subregion_index = 0; subregion_index < rgn->srgn_cnt; subregion_index++) {
>+ srgn = rgn->srgn_tbl + subregion_index;
>+ if (srgn->srgn_state == HPB_SRGN_VALID)
>+ srgn->srgn_state = HPB_SRGN_INVALID;
>+ }
>+ }
>+ spin_unlock(&hpb->rgn_state_lock);
>+}
>
> static void ufshpb_rsp_req_region_update(struct ufshpb_lu *hpb,
> struct utp_hpb_rsp *rsp_field)
>@@ -1196,25 +1229,8 @@ static void ufshpb_rsp_req_region_update(struct ufshpb_lu *hpb,
>
> for (i = 0; i < rsp_field->inactive_rgn_cnt; i++) {
> rgn_i = be16_to_cpu(rsp_field->hpb_inactive_field[i]);
>- dev_dbg(&hpb->sdev_ufs_lu->sdev_dev,
>- "inactivate(%d) region %d\n", i, rgn_i);
>-
>- spin_lock(&hpb->rsp_list_lock);
>- ufshpb_update_inactive_info(hpb, rgn_i);
>- spin_unlock(&hpb->rsp_list_lock);
>-
>- rgn = hpb->rgn_tbl + rgn_i;
>-
>- spin_lock(&hpb->rgn_state_lock);
>- if (rgn->rgn_state != HPB_RGN_INACTIVE) {
>- for (srgn_i = 0; srgn_i < rgn->srgn_cnt; srgn_i++) {
>- srgn = rgn->srgn_tbl + srgn_i;
>- if (srgn->srgn_state == HPB_SRGN_VALID)
>- srgn->srgn_state = HPB_SRGN_INVALID;
>- }
>- }
>- spin_unlock(&hpb->rgn_state_lock);
>-
>+ dev_dbg(&hpb->sdev_ufs_lu->sdev_dev, "inactivate(%d) region %d\n", i, rgn_i);
>+ ufshpb_submit_region_inactive(hpb, rgn_i);
> }
>
> out:
>@@ -1249,14 +1265,32 @@ static void ufshpb_dev_reset_handler(struct ufs_hba *hba)
>
> __shost_for_each_device(sdev, hba->host) {
> hpb = ufshpb_get_hpb_data(sdev);
>- if (hpb && hpb->is_hcm)
>+ if (!hpb)
>+ continue;
>+
>+ if (hpb->is_hcm) {
> /*
>- * For the HPB host mode, in case device powered up and lost HPB
>- * information, we will set the region flag to be RGN_FLAG_UPDATE,
>- * it will let host reload its L2P entries(re-activate the region
>- * in the UFS device).
>+ * For the HPB host control mode, in case device powered up and lost HPB
>+ * information, we will set the region flag to be RGN_FLAG_UPDATE, it will
>+ * let host reload its L2P entries(reactivate region in the UFS device).
> */
> ufshpb_set_regions_update(hpb);
>+ } else {
>+ /*
>+ * For the HPB device control mode, if host side receives 02h:HPB Operation
>+ * in UPIU response, which means device recommends the host side should
>+ * inactivate all active regions. Here we add all active regions to inactive
>+ * list, they will be inactivated later in ufshpb_map_work_handler().
>+ */
>+ struct victim_select_info *lru_info = &hpb->lru_info;
>+ struct ufshpb_region *rgn;
>+
>+ list_for_each_entry(rgn, &lru_info->lh_lru_rgn, list_lru_rgn)
>+ ufshpb_submit_region_inactive(hpb, rgn->rgn_idx);
>+
>+ if (ufshpb_get_state(hpb) == HPB_PRESENT)
>+ queue_work(ufshpb_wq, &hpb->map_work);
>+ }
> }
> }
>
>--
>2.34.1
>
>
Looks good to me.
Reviewed-by: Keoseong Park <keosung.park@...sung.com>
Best Regards,
Keoseong Park
Powered by blists - more mailing lists