| 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: <20210324033104.5kms7pez6arnkaoz@shaphisprc48410>
Date: Wed, 24 Mar 2021 03:31:04 +0000
From: Zang Leigang <zangleigang@...ilicon.com>
To: Avri Altman <avri.altman@....com>
CC: "James E . J . Bottomley" <jejb@...ux.vnet.ibm.com>,
"Martin K . Petersen" <martin.petersen@...cle.com>,
<linux-scsi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<gregkh@...uxfoundation.org>, Bart Van Assche <bvanassche@....org>,
yongmyung lee <ymhungry.lee@...sung.com>,
Daejun Park <daejun7.park@...sung.com>,
<alim.akhtar@...sung.com>, <asutoshd@...eaurora.org>,
Avi Shchislowski <avi.shchislowski@....com>,
Bean Huo <beanhuo@...ron.com>, <cang@...eaurora.org>,
<stanley.chu@...iatek.com>
Subject: Re: [PATCH v6 02/10] scsi: ufshpb: Add host control mode support to
rsp_upiu
On Mon, Mar 22, 2021 at 10:10:36AM +0200, Avri Altman wrote:
> In device control mode, the device may recommend the host to either
> activate or inactivate a region, and the host should follow. Meaning
> those are not actually recommendations, but more of instructions.
>
> On the contrary, in host control mode, the recommendation protocol is
> slightly changed:
> a) The device may only recommend the host to update a subregion of an
> already-active region. And,
> b) The device may *not* recommend to inactivate a region.
>
> Furthermore, in host control mode, the host may choose not to follow any
> of the device's recommendations. However, in case of a recommendation to
> update an active and clean subregion, it is better to follow those
> recommendation because otherwise the host has no other way to know that
> some internal relocation took place.
>
> Signed-off-by: Avri Altman <avri.altman@....com>
> ---
> drivers/scsi/ufs/ufshpb.c | 34 +++++++++++++++++++++++++++++++++-
> drivers/scsi/ufs/ufshpb.h | 2 ++
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> index fb10afcbb49f..d4f0bb6d8fa1 100644
> --- a/drivers/scsi/ufs/ufshpb.c
> +++ b/drivers/scsi/ufs/ufshpb.c
> @@ -166,6 +166,8 @@ static void ufshpb_set_ppn_dirty(struct ufshpb_lu *hpb, int rgn_idx,
> else
> set_bit_len = cnt;
>
> + set_bit(RGN_FLAG_DIRTY, &rgn->rgn_flags);
> +
> if (rgn->rgn_state != HPB_RGN_INACTIVE &&
> srgn->srgn_state == HPB_SRGN_VALID)
> bitmap_set(srgn->mctx->ppn_dirty, srgn_offset, set_bit_len);
> @@ -235,6 +237,11 @@ static bool ufshpb_test_ppn_dirty(struct ufshpb_lu *hpb, int rgn_idx,
> return false;
> }
>
> +static inline bool is_rgn_dirty(struct ufshpb_region *rgn)
> +{
> + return test_bit(RGN_FLAG_DIRTY, &rgn->rgn_flags);
> +}
> +
> static int ufshpb_fill_ppn_from_page(struct ufshpb_lu *hpb,
> struct ufshpb_map_ctx *mctx, int pos,
> int len, u64 *ppn_buf)
> @@ -713,6 +720,7 @@ static void ufshpb_put_map_req(struct ufshpb_lu *hpb,
> static int ufshpb_clear_dirty_bitmap(struct ufshpb_lu *hpb,
> struct ufshpb_subregion *srgn)
> {
> + struct ufshpb_region *rgn;
> u32 num_entries = hpb->entries_per_srgn;
>
> if (!srgn->mctx) {
> @@ -726,6 +734,10 @@ static int ufshpb_clear_dirty_bitmap(struct ufshpb_lu *hpb,
> num_entries = hpb->last_srgn_entries;
>
> bitmap_zero(srgn->mctx->ppn_dirty, num_entries);
> +
> + rgn = hpb->rgn_tbl + srgn->rgn_idx;
> + clear_bit(RGN_FLAG_DIRTY, &rgn->rgn_flags);
> +
> return 0;
> }
>
> @@ -1245,6 +1257,18 @@ static void ufshpb_rsp_req_region_update(struct ufshpb_lu *hpb,
> srgn_i =
> be16_to_cpu(rsp_field->hpb_active_field[i].active_srgn);
>
> + rgn = hpb->rgn_tbl + rgn_i;
> + if (hpb->is_hcm &&
> + (rgn->rgn_state != HPB_RGN_ACTIVE || is_rgn_dirty(rgn))) {
> + /*
> + * in host control mode, subregion activation
> + * recommendations are only allowed to active regions.
> + * Also, ignore recommendations for dirty regions - the
> + * host will make decisions concerning those by himself
> + */
> + continue;
> + }
> +
Hi Avri, host control mode also need the recommendations from device,
because the bkops would make the ppn invalid, is that right?
> dev_dbg(&hpb->sdev_ufs_lu->sdev_dev,
> "activate(%d) region %d - %d\n", i, rgn_i, srgn_i);
>
> @@ -1252,7 +1276,6 @@ static void ufshpb_rsp_req_region_update(struct ufshpb_lu *hpb,
> ufshpb_update_active_info(hpb, rgn_i, srgn_i);
> spin_unlock(&hpb->rsp_list_lock);
>
> - rgn = hpb->rgn_tbl + rgn_i;
> srgn = rgn->srgn_tbl + srgn_i;
>
> /* blocking HPB_READ */
> @@ -1263,6 +1286,14 @@ static void ufshpb_rsp_req_region_update(struct ufshpb_lu *hpb,
> hpb->stats.rb_active_cnt++;
> }
>
> + if (hpb->is_hcm) {
> + /*
> + * in host control mode the device is not allowed to inactivate
> + * regions
> + */
> + goto out;
> + }
> +
> 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,
> @@ -1287,6 +1318,7 @@ static void ufshpb_rsp_req_region_update(struct ufshpb_lu *hpb,
> hpb->stats.rb_inactive_cnt++;
> }
>
> +out:
> dev_dbg(&hpb->sdev_ufs_lu->sdev_dev, "Noti: #ACT %u #INACT %u\n",
> rsp_field->active_rgn_cnt, rsp_field->inactive_rgn_cnt);
>
> diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h
> index 7df30340386a..032672114881 100644
> --- a/drivers/scsi/ufs/ufshpb.h
> +++ b/drivers/scsi/ufs/ufshpb.h
> @@ -121,6 +121,8 @@ struct ufshpb_region {
>
> /* below information is used by lru */
> struct list_head list_lru_rgn;
> + unsigned long rgn_flags;
> +#define RGN_FLAG_DIRTY 0
> };
>
> #define for_each_sub_region(rgn, i, srgn) \
> --
> 2.25.1
>
Powered by blists - more mailing lists