All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
@ 2022-06-20 12:26 Arthur Simchaev
  2022-07-17 11:28 ` Arthur Simchaev
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Arthur Simchaev @ 2022-06-20 12:26 UTC (permalink / raw)
  To: James, E.J.Bottomley, jejb, Martin, K.Petersen, martin.petersen
  Cc: linux-scsi, linux-kernel, Bean, Huo, beanhuo, Arthur Simchaev

The bsg driver allows user space to send device management commands.
As such, it is often used by field application engineers to debug various problems,
and as a test bed for new features as well.

Let's not bound ourself to hard coded descriptor sizes, as the new
Descriptors that supports new features are not defined yet.

Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
---
 drivers/scsi/ufs/ufs_bsg.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index 39bf204..7c56eba 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -6,24 +6,6 @@
  */
 #include "ufs_bsg.h"
 
-static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
-				       struct utp_upiu_query *qr)
-{
-	int desc_size = be16_to_cpu(qr->length);
-	int desc_id = qr->idn;
-
-	if (desc_size <= 0)
-		return -EINVAL;
-
-	ufshcd_map_desc_id_to_length(hba, desc_id, desc_len);
-	if (!*desc_len)
-		return -EINVAL;
-
-	*desc_len = min_t(int, *desc_len, desc_size);
-
-	return 0;
-}
-
 static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
 				     unsigned int request_len,
 				     unsigned int reply_len)
@@ -52,13 +34,11 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 		goto out;
 
 	qr = &bsg_request->upiu_req.qr;
-	if (ufs_bsg_get_query_desc_size(hba, desc_len, qr)) {
-		dev_err(hba->dev, "Illegal desc size\n");
-		return -EINVAL;
-	}
+	*desc_len = be16_to_cpu(qr->length);
 
-	if (*desc_len > job->request_payload.payload_len) {
-		dev_err(hba->dev, "Illegal desc size\n");
+	if (*desc_len <= 0 || *desc_len > QUERY_DESC_MAX_SIZE ||
+	    *desc_len > job->request_payload.payload_len) {
+		dev_err(hba->dev, "Illegal desc size %d\n", *desc_len);
 		return -EINVAL;
 	}
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-06-20 12:26 [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function Arthur Simchaev
@ 2022-07-17 11:28 ` Arthur Simchaev
  2022-07-19  2:53   ` Martin K. Petersen
  2022-08-07 23:30 ` Daniil Lunev
  2022-09-20  9:38 ` Bean Huo
  2 siblings, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-07-17 11:28 UTC (permalink / raw)
  To: Arthur Simchaev, martin.petersen
  Cc: linux-scsi, linux-kernel, beanhuo, Avi Shchislowski

Hi Martin

The bsg driver allows user space to send device management commands.
As such, it is often used by field application engineers to debug various problems, and as a test bed for new features as well.

Let's not bound ourself to hard coded descriptor sizes, as the new Descriptors that supports new features are not defined yet.

Please consider this patch series for kernel v5.20

Regards
Arthur

> -----Original Message-----
> From: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> Sent: Monday, June 20, 2022 3:26 PM
> To: James; E.J.Bottomley; jejb@linux.vnet.ibm.com; Martin; K.Petersen;
> martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Bean; Huo;
> beanhuo@micron.com; Arthur Simchaev <Arthur.Simchaev@wdc.com>
> Subject: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
> 
> The bsg driver allows user space to send device management commands.
> As such, it is often used by field application engineers to debug various
> problems,
> and as a test bed for new features as well.
> 
> Let's not bound ourself to hard coded descriptor sizes, as the new
> Descriptors that supports new features are not defined yet.
> 
> Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> ---
>  drivers/scsi/ufs/ufs_bsg.c | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> index 39bf204..7c56eba 100644
> --- a/drivers/scsi/ufs/ufs_bsg.c
> +++ b/drivers/scsi/ufs/ufs_bsg.c
> @@ -6,24 +6,6 @@
>   */
>  #include "ufs_bsg.h"
> 
> -static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
> -				       struct utp_upiu_query *qr)
> -{
> -	int desc_size = be16_to_cpu(qr->length);
> -	int desc_id = qr->idn;
> -
> -	if (desc_size <= 0)
> -		return -EINVAL;
> -
> -	ufshcd_map_desc_id_to_length(hba, desc_id, desc_len);
> -	if (!*desc_len)
> -		return -EINVAL;
> -
> -	*desc_len = min_t(int, *desc_len, desc_size);
> -
> -	return 0;
> -}
> -
>  static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
>  				     unsigned int request_len,
>  				     unsigned int reply_len)
> @@ -52,13 +34,11 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba
> *hba, struct bsg_job *job,
>  		goto out;
> 
>  	qr = &bsg_request->upiu_req.qr;
> -	if (ufs_bsg_get_query_desc_size(hba, desc_len, qr)) {
> -		dev_err(hba->dev, "Illegal desc size\n");
> -		return -EINVAL;
> -	}
> +	*desc_len = be16_to_cpu(qr->length);
> 
> -	if (*desc_len > job->request_payload.payload_len) {
> -		dev_err(hba->dev, "Illegal desc size\n");
> +	if (*desc_len <= 0 || *desc_len > QUERY_DESC_MAX_SIZE ||
> +	    *desc_len > job->request_payload.payload_len) {
> +		dev_err(hba->dev, "Illegal desc size %d\n", *desc_len);
>  		return -EINVAL;
>  	}
> 
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-07-17 11:28 ` Arthur Simchaev
@ 2022-07-19  2:53   ` Martin K. Petersen
  0 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2022-07-19  2:53 UTC (permalink / raw)
  To: Arthur Simchaev
  Cc: martin.petersen, linux-scsi, linux-kernel, beanhuo, Avi Shchislowski


Arthur,

> The bsg driver allows user space to send device management commands.
> As such, it is often used by field application engineers to debug
> various problems, and as a test bed for new features as well.

I would like a review from UFS stakeholders.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-06-20 12:26 [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function Arthur Simchaev
  2022-07-17 11:28 ` Arthur Simchaev
@ 2022-08-07 23:30 ` Daniil Lunev
  2022-08-16 14:32   ` Arthur Simchaev
  2022-08-16 14:44   ` Arthur Simchaev
  2022-09-20  9:38 ` Bean Huo
  2 siblings, 2 replies; 16+ messages in thread
From: Daniil Lunev @ 2022-08-07 23:30 UTC (permalink / raw)
  To: Arthur Simchaev
  Cc: James, E.J.Bottomley, jejb, Martin, K.Petersen, martin.petersen,
	linux-scsi, linux-kernel, Bean, Huo, beanhuo

On Mon, Jun 20, 2022 at 03:26:06PM +0300, Arthur Simchaev wrote:
> The bsg driver allows user space to send device management commands.
> As such, it is often used by field application engineers to debug various problems,
> and as a test bed for new features as well.
> 
> Let's not bound ourself to hard coded descriptor sizes, as the new
> Descriptors that supports new features are not defined yet.
Can you clarify what you mean "hard-coded"? The descriptor size is initialized
as QUERY_DESC_MAX_SIZE, and updated in `ufshcd_update_desc_length`, which is
called with the actual size upon finishing `ufshcd_read_desc_param`.

The function you are removing - `ufs_bsg_get_query_desc_size` - doesn't seem to
reject requests on incompatible size, only to restrict the size of the query.
The way the code is written - if I read it right - will lead to truncation of
the response if the size of the requested response is less than the actual
descriptor size, but otherwise you should get full descriptor back.

Can you provide a specific example where you see the problem to arise?

Thanks,
Daniil

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-08-07 23:30 ` Daniil Lunev
@ 2022-08-16 14:32   ` Arthur Simchaev
  2022-08-16 14:44   ` Arthur Simchaev
  1 sibling, 0 replies; 16+ messages in thread
From: Arthur Simchaev @ 2022-08-16 14:32 UTC (permalink / raw)
  To: Daniil Lunev
  Cc: James, E.J.Bottomley, jejb, Martin, K.Petersen, martin.petersen,
	linux-scsi, linux-kernel, Bean, Huo, beanhuo

Hi Daniil,
Thanks a lot for your review.

> Can you clarify what you mean "hard-coded"? The descriptor size is initialized
> as QUERY_DESC_MAX_SIZE, and updated in `ufshcd_update_desc_length`,
> which is
> called with the actual size upon finishing `ufshcd_read_desc_param`.
> 
> The function you are removing - `ufs_bsg_get_query_desc_size` - doesn't seem
> to
> reject requests on incompatible size, only to restrict the size of the query.
> The way the code is written - if I read it right - will lead to truncation of
> the response if the size of the requested response is less than the actual
> descriptor size, but otherwise you should get full descriptor back.
> 
> Can you provide a specific example where you see the problem to arise?

Reading a new UFS descriptors will be rejected due to hard coded descriptor definitions
in ufshcd_map_desc_id_to_length invoked from ufs_bsg_get_query_desc_size. 
For example FBO descriptor published in Jedec UFS 4.0 spec and already exist in some UFS devices.
Or others reserved descriptors which can be used as vendor's descriptor.
We should be able to read any UFS descriptor of any size (up to QUERY_DESC_MAX_SIZE).
According to the spec, the device will return the actual size.

The ufs bsg driver should not impose any such limitation. It's one of its design guidelines.
E.g. as done for the attributes, flags the kernel doesn't check it size(for attributes is the max - 4 bytes), 
nor access (read/write).
And just returns an appropriate error code should an error occur.


Regards
Arthur


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-08-07 23:30 ` Daniil Lunev
  2022-08-16 14:32   ` Arthur Simchaev
@ 2022-08-16 14:44   ` Arthur Simchaev
  2022-08-24  9:36     ` Daniil Lunev
  1 sibling, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-08-16 14:44 UTC (permalink / raw)
  To: Daniil Lunev
  Cc: martin.petersen, linux-scsi, linux-kernel, beanhuo, Avi Shchislowski

Hi Daniil,
Thanks a lot for your review.

> Can you clarify what you mean "hard-coded"? The descriptor size is initialized
> as QUERY_DESC_MAX_SIZE, and updated in `ufshcd_update_desc_length`,
> which is
> called with the actual size upon finishing `ufshcd_read_desc_param`.
> 
> The function you are removing - `ufs_bsg_get_query_desc_size` - doesn't seem
> to
> reject requests on incompatible size, only to restrict the size of the query.
> The way the code is written - if I read it right - will lead to truncation of
> the response if the size of the requested response is less than the actual
> descriptor size, but otherwise you should get full descriptor back.
> 
> Can you provide a specific example where you see the problem to arise?

Reading a new UFS descriptors will be rejected due to hard coded descriptor definitions
in ufshcd_map_desc_id_to_length invoked from ufs_bsg_get_query_desc_size. 
For example FBO descriptor published in Jedec UFS 4.0 spec and already exist in some UFS devices.
Or others reserved descriptors which can be used as vendor's descriptor.
We should be able to read any UFS descriptor of any size (up to QUERY_DESC_MAX_SIZE).
According to the spec, the device will return the actual size.

The ufs bsg driver should not impose any such limitation. It's one of its design guidelines.
E.g. as done for the attributes, flags the kernel doesn't check it size(for attributes is the max - 4 bytes), 
nor access (read/write).
And just returns an appropriate error code should an error occur.


Regards
Arthur

Regards
Arthur


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-08-16 14:44   ` Arthur Simchaev
@ 2022-08-24  9:36     ` Daniil Lunev
  2022-09-11 10:35       ` Arthur Simchaev
  0 siblings, 1 reply; 16+ messages in thread
From: Daniil Lunev @ 2022-08-24  9:36 UTC (permalink / raw)
  To: Arthur Simchaev
  Cc: martin.petersen, linux-scsi, linux-kernel, beanhuo, Avi Shchislowski

Reviewed-by: Daniil Lunev <dlunev@chromium.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-08-24  9:36     ` Daniil Lunev
@ 2022-09-11 10:35       ` Arthur Simchaev
  2022-09-19  8:33         ` Arthur Simchaev
  0 siblings, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-09-11 10:35 UTC (permalink / raw)
  To: martin.petersen
  Cc: linux-scsi, linux-kernel, beanhuo, Avi Shchislowski, Daniil Lunev

Hi Martin

Please consider applying this patch to the kernel.

Regards
Arthur

> -----Original Message-----
> From: Daniil Lunev <dlunev@chromium.org>
> Sent: Wednesday, August 24, 2022 12:36 PM
> To: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> Cc: martin.petersen@oracle.com; linux-scsi@vger.kernel.org; linux-
> kernel@vger.kernel.org; beanhuo@micron.com; Avi Shchislowski
> <Avi.Shchislowski@wdc.com>
> Subject: Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> function
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
> 
> 
> Reviewed-by: Daniil Lunev <dlunev@chromium.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-11 10:35       ` Arthur Simchaev
@ 2022-09-19  8:33         ` Arthur Simchaev
  2022-09-20  3:16           ` Martin K. Petersen
  0 siblings, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-09-19  8:33 UTC (permalink / raw)
  To: Arthur Simchaev, martin.petersen
  Cc: linux-scsi, linux-kernel, beanhuo, Avi Shchislowski, Daniil Lunev

Martin - a kind reminder.

Regards
Arthur

> -----Original Message-----
> From: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> Sent: Sunday, September 11, 2022 1:35 PM
> To: martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> beanhuo@micron.com; Avi Shchislowski <Avi.Shchislowski@wdc.com>; Daniil
> Lunev <dlunev@chromium.org>
> Subject: RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> function
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
> 
> 
> Hi Martin
> 
> Please consider applying this patch to the kernel.
> 
> Regards
> Arthur
> 
> > -----Original Message-----
> > From: Daniil Lunev <dlunev@chromium.org>
> > Sent: Wednesday, August 24, 2022 12:36 PM
> > To: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> > Cc: martin.petersen@oracle.com; linux-scsi@vger.kernel.org; linux-
> > kernel@vger.kernel.org; beanhuo@micron.com; Avi Shchislowski
> > <Avi.Shchislowski@wdc.com>
> > Subject: Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> > function
> >
> > CAUTION: This email originated from outside of Western Digital. Do not click
> > on links or open attachments unless you recognize the sender and know
> that
> > the content is safe.
> >
> >
> > Reviewed-by: Daniil Lunev <dlunev@chromium.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-19  8:33         ` Arthur Simchaev
@ 2022-09-20  3:16           ` Martin K. Petersen
  0 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2022-09-20  3:16 UTC (permalink / raw)
  To: Arthur Simchaev
  Cc: martin.petersen, linux-scsi, linux-kernel, beanhuo,
	Avi Shchislowski, Daniil Lunev


Arthur,

> Martin - a kind reminder.

I have been waiting for some of the other UFS contributors to chime in.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-06-20 12:26 [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function Arthur Simchaev
  2022-07-17 11:28 ` Arthur Simchaev
  2022-08-07 23:30 ` Daniil Lunev
@ 2022-09-20  9:38 ` Bean Huo
  2022-09-21  9:53   ` Arthur Simchaev
  2 siblings, 1 reply; 16+ messages in thread
From: Bean Huo @ 2022-09-20  9:38 UTC (permalink / raw)
  To: Arthur Simchaev, James, E.J.Bottomley, jejb, Martin, K.Petersen,
	martin.petersen
  Cc: linux-scsi, linux-kernel, Bean, Huo, beanhuo

Hi Arthur,


On Mon, 2022-06-20 at 15:26 +0300, Arthur Simchaev wrote:
> The bsg driver allows user space to send device management commands.
> As such, it is often used by field application engineers to debug
> various problems,
> and as a test bed for new features as well.
> 
> Let's not bound ourself to hard coded descriptor sizes, as the new

UFS descriptor size is no longer hardcoded (defined in the driver), the
size of the descriptor is reported by UFS itself, check the latest
kernel.


> Descriptors that supports new features are not defined yet.
> 
> Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> ---
>  drivers/scsi/ufs/ufs_bsg.c | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c

This UFS driver is in the wrong location, I assume you are using an
older kernel version?

Kind regards,
Bean


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-20  9:38 ` Bean Huo
@ 2022-09-21  9:53   ` Arthur Simchaev
  2022-09-28  8:33     ` Arthur Simchaev
  0 siblings, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-09-21  9:53 UTC (permalink / raw)
  To: Bean Huo, James, E.J.Bottomley, jejb, Martin, K.Petersen,
	martin.petersen
  Cc: linux-scsi, linux-kernel, Bean, Huo, beanhuo

Thank you Bean

> UFS descriptor size is no longer hardcoded (defined in the driver), the
> size of the descriptor is reported by UFS itself, check the latest
> kernel.
>
Invokes ufshcd_map_desc_id_to_length from bsg code, still problematic
also in the latest kernel. The function limited the ufs bsg functionality.
For example FBO descriptor published in Jedec UFS 4.0 spec and already exist in some UFS devices.
Or others reserved descriptors (RFU_0/1) which can be used as vendor's descriptor. The function returns len =0
We should be able to read any UFS descriptor of any size (up to QUERY_DESC_MAX_SIZE) or idn.
According to the spec, the device will return the actual size.

The ufs bsg driver should not impose any such limitation. It's one of its design guidelines.
E.g. as done for the attributes, flags the kernel doesn't check it size(for attributes is the max - 4 bytes),
nor access (read/write).
And just returns an appropriate error code should an error occur.

> This UFS driver is in the wrong location, I assume you are using an
> older kernel version?
Done

Regards
Arthur

> -----Original Message-----
> From: Bean Huo <huobean@gmail.com>
> Sent: Tuesday, September 20, 2022 12:38 PM
> To: Arthur Simchaev <Arthur.Simchaev@wdc.com>; James@vger.kernel.org;
> E.J.Bottomley@vger.kernel.org; jejb@linux.vnet.ibm.com;
> Martin@vger.kernel.org; K.Petersen@vger.kernel.org;
> martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> Bean@vger.kernel.org; Huo@vger.kernel.org; beanhuo@micron.com
> Subject: Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> function
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
> 
> 
> Hi Arthur,
> 
> 
> On Mon, 2022-06-20 at 15:26 +0300, Arthur Simchaev wrote:
> > The bsg driver allows user space to send device management commands.
> > As such, it is often used by field application engineers to debug
> > various problems,
> > and as a test bed for new features as well.
> >
> > Let's not bound ourself to hard coded descriptor sizes, as the new
> 
> UFS descriptor size is no longer hardcoded (defined in the driver), the
> size of the descriptor is reported by UFS itself, check the latest
> kernel.
> 
> 
> > Descriptors that supports new features are not defined yet.
> >
> > Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> > ---
> >  drivers/scsi/ufs/ufs_bsg.c | 28 ++++------------------------
> >  1 file changed, 4 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> 
> This UFS driver is in the wrong location, I assume you are using an
> older kernel version?
> 
> Kind regards,
> Bean


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-21  9:53   ` Arthur Simchaev
@ 2022-09-28  8:33     ` Arthur Simchaev
  2022-09-28 10:36       ` Bean Huo
  0 siblings, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-09-28  8:33 UTC (permalink / raw)
  To: martin.petersen, beanhuo
  Cc: linux-scsi, linux-kernel, Daniil Lunev, Avri Altman, Avi Shchislowski

Hi Bean

In case you don't have any comments I will appreciate if you will add "reviewed by" to the patch.

Regards
Arthur

> -----Original Message-----
> From: Arthur Simchaev
> Sent: Wednesday, September 21, 2022 12:53 PM
> To: Bean Huo <huobean@gmail.com>; James@vger.kernel.org;
> E.J.Bottomley@vger.kernel.org; jejb@linux.vnet.ibm.com;
> Martin@vger.kernel.org; K.Petersen@vger.kernel.org;
> martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> Bean@vger.kernel.org; Huo@vger.kernel.org; beanhuo@micron.com
> Subject: RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> function
> 
> Thank you Bean
> 
> > UFS descriptor size is no longer hardcoded (defined in the driver), the
> > size of the descriptor is reported by UFS itself, check the latest
> > kernel.
> >
> Invokes ufshcd_map_desc_id_to_length from bsg code, still problematic
> also in the latest kernel. The function limited the ufs bsg functionality.
> For example FBO descriptor published in Jedec UFS 4.0 spec and already exist
> in some UFS devices.
> Or others reserved descriptors (RFU_0/1) which can be used as vendor's
> descriptor. The function returns len =0
> We should be able to read any UFS descriptor of any size (up to
> QUERY_DESC_MAX_SIZE) or idn.
> According to the spec, the device will return the actual size.
> 
> The ufs bsg driver should not impose any such limitation. It's one of its design
> guidelines.
> E.g. as done for the attributes, flags the kernel doesn't check it size(for
> attributes is the max - 4 bytes),
> nor access (read/write).
> And just returns an appropriate error code should an error occur.
> 
> > This UFS driver is in the wrong location, I assume you are using an
> > older kernel version?
> Done
> 
> Regards
> Arthur
> 
> > -----Original Message-----
> > From: Bean Huo <huobean@gmail.com>
> > Sent: Tuesday, September 20, 2022 12:38 PM
> > To: Arthur Simchaev <Arthur.Simchaev@wdc.com>; James@vger.kernel.org;
> > E.J.Bottomley@vger.kernel.org; jejb@linux.vnet.ibm.com;
> > Martin@vger.kernel.org; K.Petersen@vger.kernel.org;
> > martin.petersen@oracle.com
> > Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> > Bean@vger.kernel.org; Huo@vger.kernel.org; beanhuo@micron.com
> > Subject: Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> > function
> >
> > CAUTION: This email originated from outside of Western Digital. Do not click
> > on links or open attachments unless you recognize the sender and know
> that
> > the content is safe.
> >
> >
> > Hi Arthur,
> >
> >
> > On Mon, 2022-06-20 at 15:26 +0300, Arthur Simchaev wrote:
> > > The bsg driver allows user space to send device management commands.
> > > As such, it is often used by field application engineers to debug
> > > various problems,
> > > and as a test bed for new features as well.
> > >
> > > Let's not bound ourself to hard coded descriptor sizes, as the new
> >
> > UFS descriptor size is no longer hardcoded (defined in the driver), the
> > size of the descriptor is reported by UFS itself, check the latest
> > kernel.
> >
> >
> > > Descriptors that supports new features are not defined yet.
> > >
> > > Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> > > ---
> > >  drivers/scsi/ufs/ufs_bsg.c | 28 ++++------------------------
> > >  1 file changed, 4 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> >
> > This UFS driver is in the wrong location, I assume you are using an
> > older kernel version?
> >
> > Kind regards,
> > Bean


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-28  8:33     ` Arthur Simchaev
@ 2022-09-28 10:36       ` Bean Huo
  2022-09-28 14:42         ` Arthur Simchaev
  0 siblings, 1 reply; 16+ messages in thread
From: Bean Huo @ 2022-09-28 10:36 UTC (permalink / raw)
  To: Arthur Simchaev, martin.petersen, beanhuo
  Cc: linux-scsi, linux-kernel, Daniil Lunev, Avri Altman, Avi Shchislowski

On Wed, 2022-09-28 at 08:33 +0000, Arthur Simchaev wrote:
> Hi Bean
> 
> In case you don't have any comments I will appreciate if you will add
> "reviewed by" to the patch.
> 
> Regards
> Arthur


Hi Arthur,

I'm thinking we should remove the desc size check in ufshcd.c entirely.
Just read any descriptor with a maximum size of QUERY_DESC_MAX_SIZE .
For user space queries, ufs_bsg reads data of the maximum length and
returns the requested length data. Thus can improve code readability
and save CPU cycles, also can fix your concern.

I don't know how about others' opinion?

Kind regards,
Bean




^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-28 10:36       ` Bean Huo
@ 2022-09-28 14:42         ` Arthur Simchaev
  2022-09-28 17:01           ` Arthur Simchaev
  0 siblings, 1 reply; 16+ messages in thread
From: Arthur Simchaev @ 2022-09-28 14:42 UTC (permalink / raw)
  To: Bean Huo, martin.petersen, beanhuo
  Cc: linux-scsi, linux-kernel, Daniil Lunev, Avri Altman, Avi Shchislowski

Agree with you. Will change & send the patch.

Regards
Arthur

> -----Original Message-----
> From: Bean Huo <huobean@gmail.com>
> Sent: Wednesday, September 28, 2022 1:36 PM
> To: Arthur Simchaev <Arthur.Simchaev@wdc.com>;
> martin.petersen@oracle.com; beanhuo@micron.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Daniil Lunev
> <dlunev@chromium.org>; Avri Altman <Avri.Altman@wdc.com>; Avi
> Shchislowski <Avi.Shchislowski@wdc.com>
> Subject: Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> function
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
> 
> 
> On Wed, 2022-09-28 at 08:33 +0000, Arthur Simchaev wrote:
> > Hi Bean
> >
> > In case you don't have any comments I will appreciate if you will add
> > "reviewed by" to the patch.
> >
> > Regards
> > Arthur
> 
> 
> Hi Arthur,
> 
> I'm thinking we should remove the desc size check in ufshcd.c entirely.
> Just read any descriptor with a maximum size of QUERY_DESC_MAX_SIZE .
> For user space queries, ufs_bsg reads data of the maximum length and
> returns the requested length data. Thus can improve code readability
> and save CPU cycles, also can fix your concern.
> 
> I don't know how about others' opinion?
> 
> Kind regards,
> Bean
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
  2022-09-28 14:42         ` Arthur Simchaev
@ 2022-09-28 17:01           ` Arthur Simchaev
  0 siblings, 0 replies; 16+ messages in thread
From: Arthur Simchaev @ 2022-09-28 17:01 UTC (permalink / raw)
  To: Arthur Simchaev, Bean Huo, martin.petersen, beanhuo
  Cc: linux-scsi, linux-kernel, Daniil Lunev, Avri Altman, Avi Shchislowski

Hi Bean. 

I think in any case we need remove the redundant  ufs_bsg_get_query_desc_size
function from ufs_bsg. As done in this patch and submit the another one in order to remove 
the desc size check in ufshcd.c entirely. 
Are you agree?  

Regards
Arthur

> -----Original Message-----
> From: Arthur Simchaev <Arthur.Simchaev@wdc.com>
> Sent: Wednesday, September 28, 2022 5:42 PM
> To: Bean Huo <huobean@gmail.com>; martin.petersen@oracle.com;
> beanhuo@micron.com
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Daniil Lunev
> <dlunev@chromium.org>; Avri Altman <Avri.Altman@wdc.com>; Avi
> Shchislowski <Avi.Shchislowski@wdc.com>
> Subject: RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> function
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
> 
> 
> Agree with you. Will change & send the patch.
> 
> Regards
> Arthur
> 
> > -----Original Message-----
> > From: Bean Huo <huobean@gmail.com>
> > Sent: Wednesday, September 28, 2022 1:36 PM
> > To: Arthur Simchaev <Arthur.Simchaev@wdc.com>;
> > martin.petersen@oracle.com; beanhuo@micron.com
> > Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Daniil Lunev
> > <dlunev@chromium.org>; Avri Altman <Avri.Altman@wdc.com>; Avi
> > Shchislowski <Avi.Shchislowski@wdc.com>
> > Subject: Re: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size
> > function
> >
> > CAUTION: This email originated from outside of Western Digital. Do not click
> > on links or open attachments unless you recognize the sender and know
> that
> > the content is safe.
> >
> >
> > On Wed, 2022-09-28 at 08:33 +0000, Arthur Simchaev wrote:
> > > Hi Bean
> > >
> > > In case you don't have any comments I will appreciate if you will add
> > > "reviewed by" to the patch.
> > >
> > > Regards
> > > Arthur
> >
> >
> > Hi Arthur,
> >
> > I'm thinking we should remove the desc size check in ufshcd.c entirely.
> > Just read any descriptor with a maximum size of QUERY_DESC_MAX_SIZE .
> > For user space queries, ufs_bsg reads data of the maximum length and
> > returns the requested length data. Thus can improve code readability
> > and save CPU cycles, also can fix your concern.
> >
> > I don't know how about others' opinion?
> >
> > Kind regards,
> > Bean
> >
> >


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-09-28 17:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-20 12:26 [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function Arthur Simchaev
2022-07-17 11:28 ` Arthur Simchaev
2022-07-19  2:53   ` Martin K. Petersen
2022-08-07 23:30 ` Daniil Lunev
2022-08-16 14:32   ` Arthur Simchaev
2022-08-16 14:44   ` Arthur Simchaev
2022-08-24  9:36     ` Daniil Lunev
2022-09-11 10:35       ` Arthur Simchaev
2022-09-19  8:33         ` Arthur Simchaev
2022-09-20  3:16           ` Martin K. Petersen
2022-09-20  9:38 ` Bean Huo
2022-09-21  9:53   ` Arthur Simchaev
2022-09-28  8:33     ` Arthur Simchaev
2022-09-28 10:36       ` Bean Huo
2022-09-28 14:42         ` Arthur Simchaev
2022-09-28 17:01           ` Arthur Simchaev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.