On Fri, 2021-01-29 at 14:30 +0900, Daejun Park wrote:

+static void ufshpb_set_read_buf_cmd(unsigned char *cdb, int rgn_idx,
+                                          int srgn_idx, int
srgn_mem_size)
+{
+       cdb[0] = UFSHPB_READ_BUFFER;
+       cdb[1] = UFSHPB_READ_BUFFER_ID;
+
+       put_unaligned_be16(rgn_idx, &cdb[2]);
+       put_unaligned_be16(srgn_idx, &cdb[4]);
+       put_unaligned_be24(srgn_mem_size, &cdb[6]);
+
+       cdb[9] = 0x00;
+}
+
+static int ufshpb_execute_map_req(struct ufshpb_lu *hpb,
+                                 struct ufshpb_req *map_req)
+{
+       struct request_queue *q;
+       struct request *req;
+       struct scsi_request *rq;
+       int ret = 0;
+       int i;
+
+       q = hpb->sdev_ufs_lu->request_queue;
+       for (i = 0; i < hpb->pages_per_srgn; i++) {
+               ret = bio_add_pc_page(q, map_req->bio, map_req->mctx-

quoted

m_page[i],

+                                     PAGE_SIZE, 0);
+               if (ret != PAGE_SIZE) {
+                       dev_err(&hpb->sdev_ufs_lu->sdev_dev,
+                                  "bio_add_pc_page fail %d - %d\n",
+                                  map_req->rgn_idx, map_req-

quoted

srgn_idx);

+                       return ret;
+               }
+       }
+
+       req = map_req->req;
+
+       blk_rq_append_bio(req, &map_req->bio);
+
+       req->end_io_data = map_req;
+
+       rq = scsi_req(req);
+       ufshpb_set_read_buf_cmd(rq->cmd, map_req->rgn_idx,
+                               map_req->srgn_idx, hpb-

quoted

srgn_mem_size);

Hi Daejun

Thanks for your hard-working on the HPB driver.

I found you didn't take into account of allocation Length of the last
sub-region of the last region.

UFS HPB spec: 

"If the requested field of the HPB Region or HPB Sub-Region is out of
range, then the device shall terminate the command by sending RESPONSE
UPIU with CHECK CONDITION status, with the SENSE KEY set to ILLEGAL
REQUEST, and the additional sense code set to INVALID FIELD IN CDB"


Below codes are from my RFC HPB patchset:

https://patchwork.kernel.org/project/linux-scsi/patch/20200504142032.16619-6-beanhuo@micron.com/

+	alloc_len = hpb->hba->hpb_geo.subregion_entry_sz;
+	/*
+	 * HPB Sub-Regions are equally sized except for the last one
which is
+	 * smaller if the last hpb Region is not an integer multiple of
+	 * bHPBSubRegionSize.
+	 */
+	if (map_req->region == (hpb->lu_region_cnt - 1) &&
+	    map_req->subregion == (hpb->subregions_in_last_region - 1))
+		alloc_len = hpb->last_subregion_entry_size;
+
+	ufshpb_prepare_read_buf_cmd(ureq->cmd, map_req->region,
+				    map_req->subregion, alloc_len);
+	if (!ureq->req) {
+		ureq->req = blk_get_request(q, REQ_OP_SCSI_IN, 0);
+		if (IS_ERR(ureq->req)) {
+			ret =  PTR_ERR(ureq->req);
+			goto free_mem;
+		}
+	}

please fix it in your next version patch. thanks.


Kind regards,
Bean