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.
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
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 <[email protected]>
Reviewed-by: Evan Green <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Evan Green <[email protected]>
---
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 2ddf244..6b9ed21 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5775,6 +5775,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);
@@ -5810,11 +5824,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;
@@ -5855,7 +5864,6 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
break;
}
-out:
return err;
}
--
1.9.1
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 <[email protected]>
---
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
On Sat, Jan 26, 2019 at 11:08 PM Avri Altman <[email protected]> wrote:
>
> 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 <[email protected]>
Reviewed-by: Evan Green <[email protected]>
>
>Signed-off-by: Avri Altman <[email protected]>
>Reviewed-by: Evan Green <[email protected]>
Reviewed-by: Bean Huo <[email protected]>
>-----Original Message-----
>From: [email protected] [mailto:linux-scsi-
>[email protected]] On Behalf Of Avri Altman
>Sent: Sunday, January 27, 2019 8:08 AM
>To: James E.J. Bottomley <[email protected]>; Martin K. Petersen
><[email protected]>; [email protected]; linux-
>[email protected]; Evan Green <[email protected]>
>Cc: Avi Shchislowski <[email protected]>; Alex Lemberg
><[email protected]>; Avri Altman <[email protected]>
>Subject: [EXT] [PATCH v4 3/3] scsi: ufs-bsg: Allow reading descriptors
>
>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 <[email protected]>
Reviewed-by: Bean Huo <[email protected]>
Martin,
Any further comments?
Thanks,
Avri
> -----Original Message-----
> From: Evan Green <[email protected]>
> Sent: Monday, January 28, 2019 8:42 PM
> To: Avri Altman <[email protected]>
> Cc: James E.J. Bottomley <[email protected]>; Martin K. Petersen
> <[email protected]>; SCSI <[email protected]>; LKML
> <[email protected]>; Avi Shchislowski <[email protected]>;
> Alex Lemberg <[email protected]>
> Subject: Re: [PATCH v4 3/3] scsi: ufs-bsg: Allow reading descriptors
>
> On Sat, Jan 26, 2019 at 11:08 PM Avri Altman <[email protected]> wrote:
> >
> > 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 <[email protected]>
>
> Reviewed-by: Evan Green <[email protected]>
Martin,
Is there any reason why this sires is not applied for 5.1?
Thanks,
Avri
> -----Original Message-----
> From: Avri Altman
> Sent: Tuesday, February 05, 2019 2:07 PM
> To: Martin K. Petersen <[email protected]>
> Cc: James E.J. Bottomley <[email protected]>; SCSI <linux-
> [email protected]>; LKML <[email protected]>; Avi
> Shchislowski <[email protected]>; Alex Lemberg
> <[email protected]>; 'Evan Green' <[email protected]>
> Subject: RE: [PATCH v4 3/3] scsi: ufs-bsg: Allow reading descriptors
>
> Martin,
> Any further comments?
>
> Thanks,
> Avri
>
>
> > -----Original Message-----
> > From: Evan Green <[email protected]>
> > Sent: Monday, January 28, 2019 8:42 PM
> > To: Avri Altman <[email protected]>
> > Cc: James E.J. Bottomley <[email protected]>; Martin K. Petersen
> > <[email protected]>; SCSI <[email protected]>; LKML
> > <[email protected]>; Avi Shchislowski
> <[email protected]>;
> > Alex Lemberg <[email protected]>
> > Subject: Re: [PATCH v4 3/3] scsi: ufs-bsg: Allow reading descriptors
> >
> > On Sat, Jan 26, 2019 at 11:08 PM Avri Altman <[email protected]>
> wrote:
> > >
> > > 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 <[email protected]>
> >
> > Reviewed-by: Evan Green <[email protected]>
Avri,
> Is there any reason why this sires is not applied for 5.1?
I haven't had time to review it yet. I'll get there...
--
Martin K. Petersen Oracle Linux Engineering
> Avri,
>
> > Is there any reason why this sires is not applied for 5.1?
>
> I haven't had time to review it yet. I'll get there...
A kind reminder.
Thanks,
Avri
>
> --
> Martin K. Petersen Oracle Linux Engineering
Hi Avri,
> A kind reminder.
It's expired from patchwork so now is an excellent time to repost.
Generally speaking, resubmitting patches is superior to pinging. While I
personally dig through the archives to revisit older patches, I can
almost guarantee that no other potential reviewers will do the same. The
best way to get a patch back on people's radar is to repost it.
--
Martin K. Petersen Oracle Linux Engineering