| 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: <25d5e6fc76bbfcfdd326540cbfc706e8@codeaurora.org>
Date: Fri, 12 Mar 2021 09:23:55 +0800
From: Can Guo <cang@...eaurora.org>
To: daejun7.park@...sung.com
Cc: Greg KH <gregkh@...uxfoundation.org>, avri.altman@....com,
jejb@...ux.ibm.com, martin.petersen@...cle.com,
asutoshd@...eaurora.org, stanley.chu@...iatek.com,
bvanassche@....org, huobean@...il.com,
ALIM AKHTAR <alim.akhtar@...sung.com>,
linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
JinHwan Park <jh.i.park@...sung.com>,
Javier Gonzalez <javier.gonz@...sung.com>,
SEUNGUK SHIN <seunguk.shin@...sung.com>,
Sung-Jun Park <sungjun07.park@...sung.com>,
Jinyoung CHOI <j-young.choi@...sung.com>,
BoRam Shin <boram.shin@...sung.com>
Subject: Re: [PATCH v26 3/4] scsi: ufs: Prepare HPB read for cached sub-region
On 2021-03-03 14:28, Daejun Park wrote:
> This patch changes the read I/O to the HPB read I/O.
>
> If the logical address of the read I/O belongs to active sub-region,
> the
> HPB driver modifies the read I/O command to HPB read. It modifies the
> UPIU
> command of UFS instead of modifying the existing SCSI command.
>
> In the HPB version 1.0, the maximum read I/O size that can be converted
> to
> HPB read is 4KB.
>
> The dirty map of the active sub-region prevents an incorrect HPB read
> that
> has stale physical page number which is updated by previous write I/O.
>
> Reviewed-by: Can Guo <cang@...eaurora.org>
> Reviewed-by: Bart Van Assche <bvanassche@....org>
> Acked-by: Avri Altman <Avri.Altman@....com>
> Tested-by: Bean Huo <beanhuo@...ron.com>
> Signed-off-by: Daejun Park <daejun7.park@...sung.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 2 +
> drivers/scsi/ufs/ufshpb.c | 253 +++++++++++++++++++++++++++++++++++++-
> drivers/scsi/ufs/ufshpb.h | 2 +
> 3 files changed, 254 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 5852ff44c3cc..851c01a26207 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2656,6 +2656,8 @@ static int ufshcd_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *cmd)
>
> lrbp->req_abort_skip = false;
>
> + ufshpb_prep(hba, lrbp);
> +
> ufshcd_comp_scsi_upiu(hba, lrbp);
>
> err = ufshcd_map_sg(hba, lrbp);
> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> index 8abadb0e010a..c75a6816a03f 100644
> --- a/drivers/scsi/ufs/ufshpb.c
> +++ b/drivers/scsi/ufs/ufshpb.c
> @@ -46,6 +46,29 @@ static void ufshpb_set_state(struct ufshpb_lu *hpb,
> int state)
> atomic_set(&hpb->hpb_state, state);
> }
>
> +static int ufshpb_is_valid_srgn(struct ufshpb_region *rgn,
> + struct ufshpb_subregion *srgn)
> +{
> + return rgn->rgn_state != HPB_RGN_INACTIVE &&
> + srgn->srgn_state == HPB_SRGN_VALID;
> +}
> +
> +static bool ufshpb_is_read_cmd(struct scsi_cmnd *cmd)
> +{
> + return req_op(cmd->request) == REQ_OP_READ;
> +}
> +
> +static bool ufshpb_is_write_or_discard_cmd(struct scsi_cmnd *cmd)
> +{
> + return op_is_write(req_op(cmd->request)) ||
> + op_is_discard(req_op(cmd->request));
> +}
> +
> +static bool ufshpb_is_support_chunk(int transfer_len)
> +{
> + return transfer_len <= HPB_MULTI_CHUNK_HIGH;
> +}
> +
> static bool ufshpb_is_general_lun(int lun)
> {
> return lun < UFS_UPIU_MAX_UNIT_NUM_ID;
> @@ -80,8 +103,8 @@ static void ufshpb_kick_map_work(struct ufshpb_lu
> *hpb)
> }
>
> static bool ufshpb_is_hpb_rsp_valid(struct ufs_hba *hba,
> - struct ufshcd_lrb *lrbp,
> - struct utp_hpb_rsp *rsp_field)
> + struct ufshcd_lrb *lrbp,
> + struct utp_hpb_rsp *rsp_field)
> {
> /* Check HPB_UPDATE_ALERT */
> if (!(lrbp->ucd_rsp_ptr->header.dword_2 &
> @@ -107,6 +130,230 @@ static bool ufshpb_is_hpb_rsp_valid(struct
> ufs_hba *hba,
> return true;
> }
>
> +static void ufshpb_set_ppn_dirty(struct ufshpb_lu *hpb, int rgn_idx,
> + int srgn_idx, int srgn_offset, int cnt)
> +{
> + struct ufshpb_region *rgn;
> + struct ufshpb_subregion *srgn;
> + int set_bit_len;
> + int bitmap_len;
> +
> +next_srgn:
> + rgn = hpb->rgn_tbl + rgn_idx;
> + srgn = rgn->srgn_tbl + srgn_idx;
> +
> + if (likely(!srgn->is_last))
> + bitmap_len = hpb->entries_per_srgn;
> + else
> + bitmap_len = hpb->last_srgn_entries;
> +
> + if ((srgn_offset + cnt) > bitmap_len)
> + set_bit_len = bitmap_len - srgn_offset;
> + else
> + set_bit_len = cnt;
> +
> + if (rgn->rgn_state != HPB_RGN_INACTIVE &&
> + srgn->srgn_state == HPB_SRGN_VALID)
> + bitmap_set(srgn->mctx->ppn_dirty, srgn_offset, set_bit_len);
> +
> + srgn_offset = 0;
> + if (++srgn_idx == hpb->srgns_per_rgn) {
> + srgn_idx = 0;
> + rgn_idx++;
> + }
> +
> + cnt -= set_bit_len;
> + if (cnt > 0)
> + goto next_srgn;
> +}
> +
> +static bool ufshpb_test_ppn_dirty(struct ufshpb_lu *hpb, int rgn_idx,
> + int srgn_idx, int srgn_offset, int cnt)
> +{
> + struct ufshpb_region *rgn;
> + struct ufshpb_subregion *srgn;
> + int bitmap_len;
> + int bit_len;
> +
> +next_srgn:
> + rgn = hpb->rgn_tbl + rgn_idx;
> + srgn = rgn->srgn_tbl + srgn_idx;
> +
> + if (likely(!srgn->is_last))
> + bitmap_len = hpb->entries_per_srgn;
> + else
> + bitmap_len = hpb->last_srgn_entries;
> +
> + if (!ufshpb_is_valid_srgn(rgn, srgn))
> + return true;
> +
> + /*
> + * If the region state is active, mctx must be allocated.
> + * In this case, check whether the region is evicted or
> + * mctx allcation fail.
> + */
> + if (unlikely(!srgn->mctx)) {
> + dev_err(&hpb->sdev_ufs_lu->sdev_dev,
> + "no mctx in region %d subregion %d.\n",
> + srgn->rgn_idx, srgn->srgn_idx);
> + return true;
> + }
> +
> + if ((srgn_offset + cnt) > bitmap_len)
> + bit_len = bitmap_len - srgn_offset;
> + else
> + bit_len = cnt;
> +
> + if (find_next_bit(srgn->mctx->ppn_dirty,
> + bit_len, srgn_offset) >= srgn_offset)
> + return true;
> +
> + srgn_offset = 0;
> + if (++srgn_idx == hpb->srgns_per_rgn) {
> + srgn_idx = 0;
> + rgn_idx++;
> + }
> +
> + cnt -= bit_len;
> + if (cnt > 0)
> + goto next_srgn;
> +
> + return false;
> +}
> +
> +static int ufshpb_fill_ppn_from_page(struct ufshpb_lu *hpb,
> + struct ufshpb_map_ctx *mctx, int pos,
> + int len, u64 *ppn_buf)
> +{
> + struct page *page;
> + int index, offset;
> + int copied;
> +
> + index = pos / (PAGE_SIZE / HPB_ENTRY_SIZE);
> + offset = pos % (PAGE_SIZE / HPB_ENTRY_SIZE);
> +
> + if ((offset + len) <= (PAGE_SIZE / HPB_ENTRY_SIZE))
> + copied = len;
> + else
> + copied = (PAGE_SIZE / HPB_ENTRY_SIZE) - offset;
> +
> + page = mctx->m_page[index];
> + if (unlikely(!page)) {
> + dev_err(&hpb->sdev_ufs_lu->sdev_dev,
> + "error. cannot find page in mctx\n");
> + return -ENOMEM;
> + }
> +
> + memcpy(ppn_buf, page_address(page) + (offset * HPB_ENTRY_SIZE),
> + copied * HPB_ENTRY_SIZE);
> +
> + return copied;
> +}
> +
> +static void
> +ufshpb_get_pos_from_lpn(struct ufshpb_lu *hpb, unsigned long lpn, int
> *rgn_idx,
> + int *srgn_idx, int *offset)
> +{
> + int rgn_offset;
> +
> + *rgn_idx = lpn >> hpb->entries_per_rgn_shift;
> + rgn_offset = lpn & hpb->entries_per_rgn_mask;
> + *srgn_idx = rgn_offset >> hpb->entries_per_srgn_shift;
> + *offset = rgn_offset & hpb->entries_per_srgn_mask;
> +}
> +
> +static void
> +ufshpb_set_hpb_read_to_upiu(struct ufshpb_lu *hpb, struct ufshcd_lrb
> *lrbp,
> + u32 lpn, u64 ppn, unsigned int transfer_len)
> +{
> + unsigned char *cdb = lrbp->cmd->cmnd;
> +
> + cdb[0] = UFSHPB_READ;
> +
> + /* ppn value is stored as big-endian in the host memory */
> + memcpy(&cdb[6], &ppn, sizeof(u64));
> + cdb[14] = transfer_len;
> +
> + lrbp->cmd->cmd_len = UFS_CDB_SIZE;
> +}
> +
> +/*
> + * This function will set up HPB read command using host-side L2P map
> data.
> + * In HPB v1.0, maximum size of HPB read command is 4KB.
> + */
> +void ufshpb_prep(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> +{
> + struct ufshpb_lu *hpb;
> + struct ufshpb_region *rgn;
> + struct ufshpb_subregion *srgn;
> + struct scsi_cmnd *cmd = lrbp->cmd;
> + u32 lpn;
> + u64 ppn;
> + unsigned long flags;
> + int transfer_len, rgn_idx, srgn_idx, srgn_offset;
> + int err = 0;
> +
> + hpb = ufshpb_get_hpb_data(cmd->device);
> + if (!hpb)
> + return;
> +
> + if (ufshpb_get_state(hpb) != HPB_PRESENT) {
> + dev_notice(&hpb->sdev_ufs_lu->sdev_dev,
> + "%s: ufshpb state is not PRESENT", __func__);
> + return;
> + }
> +
Same here, please mute these prints before hpb is fully initialized.
Thanks,
Can Guo.
Powered by blists - more mailing lists