diff mbox series

scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function

Message ID 1655727966-31584-1-git-send-email-Arthur.Simchaev@wdc.com
State New
Headers show
Series scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function | expand

Commit Message

Arthur Simchaev June 20, 2022, 12:26 p.m. UTC
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(-)

Comments

Arthur Simchaev July 17, 2022, 11:28 a.m. UTC | #1
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
Martin K. Petersen July 19, 2022, 2:53 a.m. UTC | #2
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.
Daniil Lunev Aug. 7, 2022, 11:30 p.m. UTC | #3
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
Arthur Simchaev Aug. 16, 2022, 2:32 p.m. UTC | #4
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
Arthur Simchaev Aug. 16, 2022, 2:44 p.m. UTC | #5
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
Daniil Lunev Aug. 24, 2022, 9:36 a.m. UTC | #6
Reviewed-by: Daniil Lunev <dlunev@chromium.org>
Arthur Simchaev Sept. 11, 2022, 10:35 a.m. UTC | #7
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>
Arthur Simchaev Sept. 19, 2022, 8:33 a.m. UTC | #8
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>
Martin K. Petersen Sept. 20, 2022, 3:16 a.m. UTC | #9
Arthur,

> Martin - a kind reminder.

I have been waiting for some of the other UFS contributors to chime in.
Bean Huo Sept. 20, 2022, 9:38 a.m. UTC | #10
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
Arthur Simchaev Sept. 21, 2022, 9:53 a.m. UTC | #11
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
Arthur Simchaev Sept. 28, 2022, 8:33 a.m. UTC | #12
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
Bean Huo Sept. 28, 2022, 10:36 a.m. UTC | #13
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
Arthur Simchaev Sept. 28, 2022, 2:42 p.m. UTC | #14
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
> 
>
Arthur Simchaev Sept. 28, 2022, 5:01 p.m. UTC | #15
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
> >
> >
diff mbox series

Patch

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;
 	}