[REPOST PATCH v4 0/3] scsi: ufs-bsg: Add read descriptor

[REPOST PATCH v4 0/3] scsi: ufs-bsg: Add read descriptor

[Avri Altman]

UFS Protocol Information Units (UPIU) are UFS packets that travel
between the host and the device on the UniPro bus. Our previous series
added the capability to send UPIUs to the ufs driver. It does not cover
all the possible UPIU types - we are mainly focused on device management,
provisioning, testing and validation, so it covers UPIUs that falls in
that box.

Our intension is to publish ufs-utils soon - an open source user space
utility that relies on that infrastructure to perform those tasks.
This short series is adding one last functionality needed by ufs-utils
that was somehow left behind - allowing reading descriptors as well.

Repost of v4 and adds tags already received.
While at it, it turns out that quite a few people are waiting for
ufs-utils, e.g. https://www.spinics.net/lists/linux-scsi/msg127807.html.
So I expect that the small, but vibrant community of ufs users, will try
to expand this infrastructure even further.

V3->v4:
Improve code readability in ufs-bsg: Allow reading descriptors
Update Reviewed-by tag.

V2->v3:
Add a prep patch with write descriptor calling convention changes.
Elaborate the commit log of ufs-bsg: Allow reading descriptors
Add Reviewed-by tag.

v1->v2:
Withdraw from the attempt to change the reply buffer, instead place the
descriptor being read in the actual data buffer in the bio.

Avri Altman (3):
  scsi: ufs-bsg: Change the calling convention for write descriptor
  scsi: ufs: Allow reading descriptor via raw upiu
  scsi: ufs-bsg: Allow reading descriptors

 Documentation/scsi/ufs.txt | 11 ++++++++
 drivers/scsi/ufs/ufs_bsg.c | 63 ++++++++++++++++++++++++++--------------------
 drivers/scsi/ufs/ufshcd.c  | 20 ++++++++++-----
 3 files changed, 61 insertions(+), 33 deletions(-)

-- 
1.9.1

[REPOST PATCH v4 1/3] scsi: ufs-bsg: Change the calling convention for write descriptor

[Avri Altman]

When we had a write descriptor query upiu, we appended the descriptor
right after the bsg request.  This was fine as the bsg driver allows to
allocate whatever buffer we needed in its job request.

Still, the proper way to deliver payload, however small (we only write
config descriptors of 144 bytes), is by using the job request payload
data buffer.

So change this ABI now, while ufs-bsg is still new, and nobody is
actually using it.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---
 Documentation/scsi/ufs.txt |  6 ++++++
 drivers/scsi/ufs/ufs_bsg.c | 47 +++++++++++++++++++++++++---------------------
 2 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/Documentation/scsi/ufs.txt b/Documentation/scsi/ufs.txt
index 520b5b0..78fe7cb 100644
--- a/Documentation/scsi/ufs.txt
+++ b/Documentation/scsi/ufs.txt
@@ -147,6 +147,12 @@ send SG_IO with the applicable sg_io_v4:
 	io_hdr_v4.max_response_len = reply_len;
 	io_hdr_v4.request_len = request_len;
 	io_hdr_v4.request = (__u64)request_upiu;
+	if (dir == SG_DXFER_TO_DEV) {
+		io_hdr_v4.dout_xfer_len = (uint32_t)byte_cnt;
+		io_hdr_v4.dout_xferp = (uintptr_t)(__u64)buff;
+	}
+
+If you wish to write a descriptor, use the dout_xferp sg_io_v4.
 
 UFS Specifications can be found at,
 UFS - http://www.jedec.org/sites/default/files/docs/JESD220.pdf
diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index 775bb4e..2fd0769 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -27,15 +27,11 @@ static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
 
 static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
 				     unsigned int request_len,
-				     unsigned int reply_len,
-				     int desc_len, enum query_opcode desc_op)
+				     unsigned int reply_len)
 {
 	int min_req_len = sizeof(struct ufs_bsg_request);
 	int min_rsp_len = sizeof(struct ufs_bsg_reply);
 
-	if (desc_op == UPIU_QUERY_OPCODE_WRITE_DESC)
-		min_req_len += desc_len;
-
 	if (min_req_len > request_len || min_rsp_len > reply_len) {
 		dev_err(hba->dev, "not enough space assigned\n");
 		return -EINVAL;
@@ -44,14 +40,13 @@ static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
 	return 0;
 }
 
-static int ufs_bsg_verify_query_params(struct ufs_hba *hba,
-				       struct ufs_bsg_request *bsg_request,
-				       unsigned int request_len,
-				       unsigned int reply_len,
-				       uint8_t *desc_buff, int *desc_len,
-				       enum query_opcode desc_op)
+static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
+				     uint8_t **desc_buff, int *desc_len,
+				     enum query_opcode desc_op)
 {
+	struct ufs_bsg_request *bsg_request = job->request;
 	struct utp_upiu_query *qr;
+	u8 *descp;
 
 	if (desc_op == UPIU_QUERY_OPCODE_READ_DESC) {
 		dev_err(hba->dev, "unsupported opcode %d\n", desc_op);
@@ -67,11 +62,19 @@ static int ufs_bsg_verify_query_params(struct ufs_hba *hba,
 		return -EINVAL;
 	}
 
-	if (ufs_bsg_verify_query_size(hba, request_len, reply_len, *desc_len,
-				      desc_op))
+	if (*desc_len > job->request_payload.payload_len) {
+		dev_err(hba->dev, "Illegal desc size\n");
 		return -EINVAL;
+	}
+
+	descp = kzalloc(*desc_len, GFP_KERNEL);
+	if (!descp)
+		return -ENOMEM;
 
-	desc_buff = (uint8_t *)(bsg_request + 1);
+	sg_copy_to_buffer(job->request_payload.sg_list,
+			  job->request_payload.sg_cnt, descp, *desc_len);
+
+	*desc_buff = descp;
 
 out:
 	return 0;
@@ -91,7 +94,7 @@ static int ufs_bsg_request(struct bsg_job *job)
 	enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
 	int ret;
 
-	ret = ufs_bsg_verify_query_size(hba, req_len, reply_len, 0, desc_op);
+	ret = ufs_bsg_verify_query_size(hba, req_len, reply_len);
 	if (ret)
 		goto out;
 
@@ -101,9 +104,8 @@ static int ufs_bsg_request(struct bsg_job *job)
 	switch (msgcode) {
 	case UPIU_TRANSACTION_QUERY_REQ:
 		desc_op = bsg_request->upiu_req.qr.opcode;
-		ret = ufs_bsg_verify_query_params(hba, bsg_request, req_len,
-						  reply_len, desc_buff,
-						  &desc_len, desc_op);
+		ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
+						&desc_len, desc_op);
 		if (ret)
 			goto out;
 
@@ -135,11 +137,14 @@ static int ufs_bsg_request(struct bsg_job *job)
 		break;
 	}
 
+	if (!desc_buff)
+		goto out;
+
+	kfree(desc_buff);
+
 out:
 	bsg_reply->result = ret;
-	job->reply_len = sizeof(struct ufs_bsg_reply) +
-			 bsg_reply->reply_payload_rcv_len;
-
+	job->reply_len = sizeof(struct ufs_bsg_reply);
 	bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
 
 	return ret;
-- 
1.9.1

[REPOST PATCH v4 2/3] scsi: ufs: Allow reading descriptor via raw upiu

[Avri Altman]

Allow to read descriptors via raw upiu. This in fact was forbidden just
as a precaution, as ufs-bsg actually enforces which functionality is
supported.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufshcd.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5ca4581..7b5b601 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5780,6 +5780,20 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 
 	/* just copy the upiu response as it is */
 	memcpy(rsp_upiu, lrbp->ucd_rsp_ptr, sizeof(*rsp_upiu));
+	if (desc_buff && desc_op == UPIU_QUERY_OPCODE_READ_DESC) {
+		u8 *descp = (u8 *)lrbp->ucd_rsp_ptr + sizeof(*rsp_upiu);
+		u16 resp_len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) &
+			       MASK_QUERY_DATA_SEG_LEN;
+
+		if (*buff_len >= resp_len) {
+			memcpy(desc_buff, descp, resp_len);
+			*buff_len = resp_len;
+		} else {
+			dev_warn(hba->dev, "rsp size is bigger than buffer");
+			*buff_len = 0;
+			err = -EINVAL;
+		}
+	}
 
 	ufshcd_put_dev_cmd_tag(hba, tag);
 	wake_up(&hba->dev_cmd.tag_wq);
@@ -5815,11 +5829,6 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 	int ocs_value;
 	u8 tm_f = be32_to_cpu(req_upiu->header.dword_1) >> 16 & MASK_TM_FUNC;
 
-	if (desc_buff && desc_op != UPIU_QUERY_OPCODE_WRITE_DESC) {
-		err = -ENOTSUPP;
-		goto out;
-	}
-
 	switch (msgcode) {
 	case UPIU_TRANSACTION_NOP_OUT:
 		cmd_type = DEV_CMD_TYPE_NOP;
@@ -5860,7 +5869,6 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 		break;
 	}
 
-out:
 	return err;
 }
 
-- 
1.9.1

[REPOST PATCH v4 3/3] scsi: ufs-bsg: Allow reading descriptors

[Avri Altman]

Add this functionality, placing the descriptor being read in the actual
data buffer in the bio.

That is, for both read and write descriptors query upiu, we are using
the job's request_payload.  This in turn, is mapped back in user land to
the applicable sg_io_v4 xferp: dout_xferp for write descriptor,
and din_xferp for read descriptor.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---
 Documentation/scsi/ufs.txt |  7 ++++++-
 drivers/scsi/ufs/ufs_bsg.c | 20 ++++++++++++--------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/Documentation/scsi/ufs.txt b/Documentation/scsi/ufs.txt
index 78fe7cb..1769f71 100644
--- a/Documentation/scsi/ufs.txt
+++ b/Documentation/scsi/ufs.txt
@@ -150,9 +150,14 @@ send SG_IO with the applicable sg_io_v4:
 	if (dir == SG_DXFER_TO_DEV) {
 		io_hdr_v4.dout_xfer_len = (uint32_t)byte_cnt;
 		io_hdr_v4.dout_xferp = (uintptr_t)(__u64)buff;
+	} else {
+		io_hdr_v4.din_xfer_len = (uint32_t)byte_cnt;
+		io_hdr_v4.din_xferp = (uintptr_t)(__u64)buff;
 	}
 
-If you wish to write a descriptor, use the dout_xferp sg_io_v4.
+If you wish to read or write a descriptor, use the appropriate xferp of
+sg_io_v4.
+
 
 UFS Specifications can be found at,
 UFS - http://www.jedec.org/sites/default/files/docs/JESD220.pdf
diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index 2fd0769..869e71f 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -48,12 +48,8 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 	struct utp_upiu_query *qr;
 	u8 *descp;
 
-	if (desc_op == UPIU_QUERY_OPCODE_READ_DESC) {
-		dev_err(hba->dev, "unsupported opcode %d\n", desc_op);
-		return -ENOTSUPP;
-	}
-
-	if (desc_op != UPIU_QUERY_OPCODE_WRITE_DESC)
+	if (desc_op != UPIU_QUERY_OPCODE_WRITE_DESC &&
+	    desc_op != UPIU_QUERY_OPCODE_READ_DESC)
 		goto out;
 
 	qr = &bsg_request->upiu_req.qr;
@@ -71,8 +67,10 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 	if (!descp)
 		return -ENOMEM;
 
-	sg_copy_to_buffer(job->request_payload.sg_list,
-			  job->request_payload.sg_cnt, descp, *desc_len);
+	if (desc_op == UPIU_QUERY_OPCODE_WRITE_DESC)
+		sg_copy_to_buffer(job->request_payload.sg_list,
+				  job->request_payload.sg_cnt, descp,
+				  *desc_len);
 
 	*desc_buff = descp;
 
@@ -140,6 +138,12 @@ static int ufs_bsg_request(struct bsg_job *job)
 	if (!desc_buff)
 		goto out;
 
+	if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len)
+		bsg_reply->reply_payload_rcv_len =
+			sg_copy_from_buffer(job->request_payload.sg_list,
+					    job->request_payload.sg_cnt,
+					    desc_buff, desc_len);
+
 	kfree(desc_buff);
 
 out:
-- 
1.9.1

RE: [REPOST PATCH v4 0/3] scsi: ufs-bsg: Add read descriptor

[Avri Altman]

Martin,
Any further comments concerning this series?

Thanks,
Avri

> 
> UFS Protocol Information Units (UPIU) are UFS packets that travel
> between the host and the device on the UniPro bus. Our previous series
> added the capability to send UPIUs to the ufs driver. It does not cover
> all the possible UPIU types - we are mainly focused on device management,
> provisioning, testing and validation, so it covers UPIUs that falls in
> that box.
> 
> Our intension is to publish ufs-utils soon - an open source user space
> utility that relies on that infrastructure to perform those tasks.
> This short series is adding one last functionality needed by ufs-utils
> that was somehow left behind - allowing reading descriptors as well.
> 
> Repost of v4 and adds tags already received.
> While at it, it turns out that quite a few people are waiting for
> ufs-utils, e.g. https://www.spinics.net/lists/linux-scsi/msg127807.html.
> So I expect that the small, but vibrant community of ufs users, will try
> to expand this infrastructure even further.
> 
> V3->v4:
> Improve code readability in ufs-bsg: Allow reading descriptors
> Update Reviewed-by tag.
> 
> V2->v3:
> Add a prep patch with write descriptor calling convention changes.
> Elaborate the commit log of ufs-bsg: Allow reading descriptors
> Add Reviewed-by tag.
> 
> v1->v2:
> Withdraw from the attempt to change the reply buffer, instead place the
> descriptor being read in the actual data buffer in the bio.
> 
> Avri Altman (3):
>   scsi: ufs-bsg: Change the calling convention for write descriptor
>   scsi: ufs: Allow reading descriptor via raw upiu
>   scsi: ufs-bsg: Allow reading descriptors
> 
>  Documentation/scsi/ufs.txt | 11 ++++++++
>  drivers/scsi/ufs/ufs_bsg.c | 63 ++++++++++++++++++++++++++------------------
> --
>  drivers/scsi/ufs/ufshcd.c  | 20 ++++++++++-----
>  3 files changed, 61 insertions(+), 33 deletions(-)
> 
> --
> 1.9.1

Re: [REPOST PATCH v4 0/3] scsi: ufs-bsg: Add read descriptor

[Martin K. Petersen]

Avri,

> UFS Protocol Information Units (UPIU) are UFS packets that travel
> between the host and the device on the UniPro bus. Our previous series
> added the capability to send UPIUs to the ufs driver. It does not
> cover all the possible UPIU types - we are mainly focused on device
> management, provisioning, testing and validation, so it covers UPIUs
> that falls in that box.

Applied to 5.1/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering