All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arthur Simchaev <Arthur.Simchaev@wdc.com>
To: Daniil Lunev <dlunev@chromium.org>
Cc: "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"beanhuo@micron.com" <beanhuo@micron.com>,
	Avi Shchislowski <Avi.Shchislowski@wdc.com>
Subject: RE: [PATCH] scsi: ufs-bsg: Remove ufs_bsg_get_query_desc_size function
Date: Tue, 16 Aug 2022 14:44:14 +0000	[thread overview]
Message-ID: <BY5PR04MB6327431615BFFFD735EB2502ED6B9@BY5PR04MB6327.namprd04.prod.outlook.com> (raw)
In-Reply-To: <YvBK/8yeohLhu2Za@google.com>

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


  parent reply	other threads:[~2022-08-16 14:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BY5PR04MB6327431615BFFFD735EB2502ED6B9@BY5PR04MB6327.namprd04.prod.outlook.com \
    --to=arthur.simchaev@wdc.com \
    --cc=Avi.Shchislowski@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=dlunev@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.