| 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: <20210305003525epcms2p5f99e8decce000d81e811875b1a495523@epcms2p5>
Date: Fri, 05 Mar 2021 09:35:25 +0900
From: Daejun Park <daejun7.park@...sung.com>
To: Bean Huo <huobean@...il.com>,
Daejun Park <daejun7.park@...sung.com>,
Greg KH <gregkh@...uxfoundation.org>,
"avri.altman@....com" <avri.altman@....com>,
"jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
"asutoshd@...eaurora.org" <asutoshd@...eaurora.org>,
"stanley.chu@...iatek.com" <stanley.chu@...iatek.com>,
"cang@...eaurora.org" <cang@...eaurora.org>,
"bvanassche@....org" <bvanassche@....org>,
ALIM AKHTAR <alim.akhtar@...sung.com>
CC: "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-kernel@...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: Re: [PATCH v26 4/4] scsi: ufs: Add HPB 2.0 support
Hi Bean,
> > +
> > +static inline int ufshpb_get_read_id(struct ufshpb_lu *hpb)
> > +{
> > + if (++hpb->cur_read_id >= MAX_HPB_READ_ID)
> > + hpb->cur_read_id = 0;
> > + return hpb->cur_read_id;
> > +}
> > +
> > +static int ufshpb_execute_pre_req(struct ufshpb_lu *hpb, struct
> > scsi_cmnd *cmd,
> > + struct ufshpb_req *pre_req, int
> > read_id)
> > +{
> > + struct scsi_device *sdev = cmd->device;
> > + struct request_queue *q = sdev->request_queue;
> > + struct request *req;
> > + struct scsi_request *rq;
> > + struct bio *bio = pre_req->bio;
> > +
> > + pre_req->hpb = hpb;
> > + pre_req->wb.lpn = sectors_to_logical(cmd->device,
> > + blk_rq_pos(cmd-
> > >request));
> > + pre_req->wb.len = sectors_to_logical(cmd->device,
> > + blk_rq_sectors(cmd-
> > >request));
> > + if (ufshpb_pre_req_add_bio_page(hpb, q, pre_req))
> > + return -ENOMEM;
> > +
> > + req = pre_req->req;
> > +
> > + /* 1. request setup */
> > + blk_rq_append_bio(req, &bio);
> > + req->rq_disk = NULL;
> > + req->end_io_data = (void *)pre_req;
> > + req->end_io = ufshpb_pre_req_compl_fn;
> > +
> > + /* 2. scsi_request setup */
> > + rq = scsi_req(req);
> > + rq->retries = 1;
> > +
> > + ufshpb_set_write_buf_cmd(rq->cmd, pre_req->wb.lpn, pre_req-
> > >wb.len,
> > + read_id);
> > + rq->cmd_len = scsi_command_size(rq->cmd);
> > +
> > + if (blk_insert_cloned_request(q, req) != BLK_STS_OK)
> > + return -EAGAIN;
> > +
> > + hpb->stats.pre_req_cnt++;
> > +
> > + return 0;
> > +}
> > +
> > +static int ufshpb_issue_pre_req(struct ufshpb_lu *hpb, struct
> > scsi_cmnd *cmd,
> > + int *read_id)
> > +{
> > + struct ufshpb_req *pre_req;
> > + struct request *req = NULL;
> > + struct bio *bio = NULL;
> > + unsigned long flags;
> > + int _read_id;
> > + int ret = 0;
> > +
> > + req = blk_get_request(cmd->device->request_queue,
> > + REQ_OP_SCSI_OUT | REQ_SYNC,
> > BLK_MQ_REQ_NOWAIT);
> > + if (IS_ERR(req))
> > + return -EAGAIN;
> > +
> > + bio = bio_alloc(GFP_ATOMIC, 1);
> > + if (!bio) {
> > + blk_put_request(req);
> > + return -EAGAIN;
> > + }
> > +
> > + spin_lock_irqsave(&hpb->rgn_state_lock, flags);
> > + pre_req = ufshpb_get_pre_req(hpb);
> > + if (!pre_req) {
> > + ret = -EAGAIN;
> > + goto unlock_out;
> > + }
> > + _read_id = ufshpb_get_read_id(hpb);
> > + spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
> > +
> > + pre_req->req = req;
> > + pre_req->bio = bio;
> > +
> > + ret = ufshpb_execute_pre_req(hpb, cmd, pre_req, _read_id);
> > + if (ret)
> > + goto free_pre_req;
> > +
> > + *read_id = _read_id;
> > +
> > + return ret;
> > +free_pre_req:
> > + spin_lock_irqsave(&hpb->rgn_state_lock, flags);
> > + ufshpb_put_pre_req(hpb, pre_req);
> > +unlock_out:
> > + spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
> > + bio_put(bio);
> > + blk_put_request(req);
> > + return ret;
> > +}
> > +
> > /*
> > * 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)
> > +int ufshpb_prep(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> > {
> > struct ufshpb_lu *hpb;
> > struct ufshpb_region *rgn;
> > @@ -291,26 +560,27 @@ void ufshpb_prep(struct ufs_hba *hba, struct
> > ufshcd_lrb *lrbp)
> > u64 ppn;
> > unsigned long flags;
> > int transfer_len, rgn_idx, srgn_idx, srgn_offset;
> > + int read_id = 0;
> > int err = 0;
> >
> > hpb = ufshpb_get_hpb_data(cmd->device);
> > if (!hpb)
> > - return;
> > + return -ENODEV;
> >
> > if (ufshpb_get_state(hpb) != HPB_PRESENT) {
> > dev_notice(&hpb->sdev_ufs_lu->sdev_dev,
> > "%s: ufshpb state is not PRESENT",
> > __func__);
> > - return;
> > + return -ENODEV;
> > }
> >
> > if (!ufshpb_is_write_or_discard_cmd(cmd) &&
> > !ufshpb_is_read_cmd(cmd))
> > - return;
> > + return 0;
> >
> > transfer_len = sectors_to_logical(cmd->device,
> > blk_rq_sectors(cmd-
> > >request));
> > if (unlikely(!transfer_len))
> > - return;
> > + return 0;
> >
> > lpn = sectors_to_logical(cmd->device, blk_rq_pos(cmd-
> > >request));
> > ufshpb_get_pos_from_lpn(hpb, lpn, &rgn_idx, &srgn_idx,
> > &srgn_offset);
> > @@ -323,18 +593,19 @@ void ufshpb_prep(struct ufs_hba *hba, struct
> > ufshcd_lrb *lrbp)
> > ufshpb_set_ppn_dirty(hpb, rgn_idx, srgn_idx,
> > srgn_offset,
> > transfer_len);
> > spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
> > - return;
> > + return 0;
> > }
> >
> > - if (!ufshpb_is_support_chunk(transfer_len))
> > - return;
> > + if (!ufshpb_is_support_chunk(hpb, transfer_len) &&
> > + (ufshpb_is_legacy(hba) && (transfer_len !=
> > HPB_LEGACY_CHUNK_HIGH)))
> > + return 0;
> >
> > spin_lock_irqsave(&hpb->rgn_state_lock, flags);
> > if (ufshpb_test_ppn_dirty(hpb, rgn_idx, srgn_idx,
> > srgn_offset,
> > transfer_len)) {
> > hpb->stats.miss_cnt++;
> > spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
> > - return;
> > + return 0;
> > }
> >
> > err = ufshpb_fill_ppn_from_page(hpb, srgn->mctx, srgn_offset,
> > 1, &ppn);
> > @@ -347,28 +618,46 @@ void ufshpb_prep(struct ufs_hba *hba, struct
> > ufshcd_lrb *lrbp)
> > * active state.
> > */
> > dev_err(hba->dev, "get ppn failed. err %d\n", err);
> > - return;
> > + return err;
> > + }
> > +
> > + if (!ufshpb_is_legacy(hba) &&
> > + ufshpb_is_required_wb(hpb, transfer_len)) {
> > + err = ufshpb_issue_pre_req(hpb, cmd, &read_id);
> > + if (err) {
> > + unsigned long timeout;
> > +
> > + timeout = cmd->jiffies_at_alloc +
> > msecs_to_jiffies(
> > + hpb->params.requeue_timeout_ms);
> > +
> > + if (time_before(jiffies, timeout))
> > + return -EAGAIN;
> > +
> > + hpb->stats.miss_cnt++;
> > + return 0;
> > + }
> > }
> >
> > - ufshpb_set_hpb_read_to_upiu(hpb, lrbp, lpn, ppn,
> > transfer_len);
> > + ufshpb_set_hpb_read_to_upiu(hpb, lrbp, lpn, ppn,
> > transfer_len, read_id);
> >
> > hpb->stats.hit_cnt++;
> > + return 0;
> > }
>
>
>
> BUG!!!
>
>
> Please read HPB 2.0 Spec carefully, and check how to correctly use HPB
> READ ID. you are assigning 0 for HPB write buffer. how can you expect
> the HPB READ be paired???
>
I fixed as follows,
static inline int ufshpb_get_read_id(struct ufshpb_lu *hpb)
{
if (hpb->cur_read_id >= MAX_HPB_READ_ID)
hpb->cur_read_id = 0;
return ++hpb->cur_read_id;
}
Thanks for comments, I will check carefully.
Thanks,
Daejun
Powered by blists - more mailing lists