| Message ID | 1667233524-13715-5-git-send-email-Arthur.Simchaev@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ufs: core: Always read the descriptors with max length | expand |
Hi Arthur,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20221031]
[cannot apply to jejb-scsi/for-next linus/master v6.1-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/1667233524-13715-5-git-send-email-Arthur.Simchaev%40wdc.com
patch subject: [PATCH 4/4] ufs: core: Remove ufshcd_map_desc_id_to_length function
config: loongarch-allyesconfig
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
git checkout 5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/ufs/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/ufs/ufshcd.h:22,
from drivers/ufs/core/ufshcd-priv.h:7,
from drivers/ufs/core/ufshpb.c:18:
drivers/ufs/core/ufshpb.c: In function 'ufshpb_get_lu_info':
>> include/ufs/ufs.h:19:35: warning: passing argument 7 of 'ufshcd_query_descriptor_retry' makes pointer from integer without a cast [-Wint-conversion]
19 | #define QUERY_DESC_MAX_SIZE 255
| ^~~
| |
| int
drivers/ufs/core/ufshpb.c:2391:55: note: in expansion of macro 'QUERY_DESC_MAX_SIZE'
2391 | desc_buf, QUERY_DESC_MAX_SIZE);
| ^~~~~~~~~~~~~~~~~~~
drivers/ufs/core/ufshcd-priv.h:49:54: note: expected 'int *' but argument is of type 'int'
49 | u8 *desc_buf, int *buf_len);
| ~~~~~^~~~~~~
vim +/ufshcd_query_descriptor_retry +19 include/ufs/ufs.h
5a0b0cb9bee767 drivers/scsi/ufs/ufs.h Sujit Reddy Thumma 2013-07-30 17
220d17a69de432 drivers/scsi/ufs/ufs.h Avri Altman 2018-10-07 18 #define GENERAL_UPIU_REQUEST_SIZE (sizeof(struct utp_upiu_req))
d44a5f98bb49b2 drivers/scsi/ufs/ufs.h Dolev Raviv 2014-06-29 @19 #define QUERY_DESC_MAX_SIZE 255
d44a5f98bb49b2 drivers/scsi/ufs/ufs.h Dolev Raviv 2014-06-29 20 #define QUERY_DESC_MIN_SIZE 2
b573d484e4ff33 drivers/scsi/ufs/ufs.h Yaniv Gardi 2016-03-10 21 #define QUERY_DESC_HDR_SIZE 2
68078d5cc1a59b drivers/scsi/ufs/ufs.h Dolev Raviv 2013-07-30 22 #define QUERY_OSF_SIZE (GENERAL_UPIU_REQUEST_SIZE - \
68078d5cc1a59b drivers/scsi/ufs/ufs.h Dolev Raviv 2013-07-30 23 (sizeof(struct utp_upiu_header)))
09a5a24ff36f90 drivers/scsi/ufs/ufs.h Avri Altman 2018-11-22 24 #define UFS_SENSE_SIZE 18
7a3e97b0dc4bba drivers/scsi/ufs/ufs.h Santosh Yaraganavi 2012-02-29 25
Hi Arthur,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20221101]
[cannot apply to jejb-scsi/for-next linus/master v6.1-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/1667233524-13715-5-git-send-email-Arthur.Simchaev%40wdc.com
patch subject: [PATCH 4/4] ufs: core: Remove ufshcd_map_desc_id_to_length function
config: loongarch-randconfig-s051-20221031
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
git checkout 5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/ufs/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/ufs/core/ufshpb.c:2391:55: sparse: sparse: incorrect type in argument 7 (different base types) @@ expected int *buf_len @@ got int @@
drivers/ufs/core/ufshpb.c:2391:55: sparse: expected int *buf_len
drivers/ufs/core/ufshpb.c:2391:55: sparse: got int
vim +2391 drivers/ufs/core/ufshpb.c
2379
2380 static int ufshpb_get_lu_info(struct ufs_hba *hba, int lun,
2381 struct ufshpb_lu_info *hpb_lu_info)
2382 {
2383 u16 max_active_rgns;
2384 u8 lu_enable;
2385 int ret;
2386 char desc_buf[QUERY_DESC_MAX_SIZE];
2387
2388 ufshcd_rpm_get_sync(hba);
2389 ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
2390 QUERY_DESC_IDN_UNIT, lun, 0,
> 2391 desc_buf, QUERY_DESC_MAX_SIZE);
2392 ufshcd_rpm_put_sync(hba);
2393
2394 if (ret) {
2395 dev_err(hba->dev,
2396 "%s: idn: %d lun: %d query request failed",
2397 __func__, QUERY_DESC_IDN_UNIT, lun);
2398 return ret;
2399 }
2400
2401 lu_enable = desc_buf[UNIT_DESC_PARAM_LU_ENABLE];
2402 if (lu_enable != LU_ENABLED_HPB_FUNC)
2403 return -ENODEV;
2404
2405 max_active_rgns = get_unaligned_be16(
2406 desc_buf + UNIT_DESC_PARAM_HPB_LU_MAX_ACTIVE_RGNS);
2407 if (!max_active_rgns) {
2408 dev_err(hba->dev,
2409 "lun %d wrong number of max active regions\n", lun);
2410 return -ENODEV;
2411 }
2412
2413 hpb_lu_info->num_blocks = get_unaligned_be64(
2414 desc_buf + UNIT_DESC_PARAM_LOGICAL_BLK_COUNT);
2415 hpb_lu_info->pinned_start = get_unaligned_be16(
2416 desc_buf + UNIT_DESC_PARAM_HPB_PIN_RGN_START_OFF);
2417 hpb_lu_info->num_pinned = get_unaligned_be16(
2418 desc_buf + UNIT_DESC_PARAM_HPB_NUM_PIN_RGNS);
2419 hpb_lu_info->max_active_rgns = max_active_rgns;
2420
2421 return 0;
2422 }
2423
diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c index b99e3f3..dc441ac 100644 --- a/drivers/ufs/core/ufs_bsg.c +++ b/drivers/ufs/core/ufs_bsg.c @@ -16,16 +16,11 @@ 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); + *desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size); return 0; } diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h index a9e8e1f..c52e2f3 100644 --- a/drivers/ufs/core/ufshcd-priv.h +++ b/drivers/ufs/core/ufshcd-priv.h @@ -70,9 +70,6 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index, int ufshcd_hold(struct ufs_hba *hba, bool async); void ufshcd_release(struct ufs_hba *hba); -void ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id, - int *desc_length); - int ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd); int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba, diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 9cc3abd..fc0695f 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -3369,20 +3369,6 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba, } /** - * ufshcd_map_desc_id_to_length - map descriptor IDN to its length - * @hba: Pointer to adapter instance - * @desc_id: descriptor idn value - * @desc_len: mapped desc length (out) - */ -void ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id, - int *desc_len) -{ - /*Always returns QUERY_DESC_MAX_SIZE*/ - *desc_len = QUERY_DESC_MAX_SIZE; -} -EXPORT_SYMBOL(ufshcd_map_desc_id_to_length); - -/** * ufshcd_read_desc_param - read the specified descriptor parameter * @hba: Pointer to adapter instance * @desc_id: descriptor idn value @@ -3402,26 +3388,13 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, { int ret; u8 *desc_buf; - int buff_len; + int buff_len = QUERY_DESC_MAX_SIZE; bool is_kmalloc = true; /* Safety check */ if (desc_id >= QUERY_DESC_IDN_MAX || !param_size) return -EINVAL; - /* Get the length of descriptor */ - ufshcd_map_desc_id_to_length(hba, desc_id, &buff_len); - if (!buff_len) { - dev_err(hba->dev, "%s: Failed to get desc length\n", __func__); - return -EINVAL; - } - - if (param_offset >= buff_len) { - dev_err(hba->dev, "%s: Invalid offset 0x%x in descriptor IDN 0x%x, length 0x%x\n", - __func__, param_offset, desc_id, buff_len); - return -EINVAL; - } - /* Check whether we need temp memory */ if (param_offset != 0 || param_size < buff_len) { desc_buf = kzalloc(buff_len, GFP_KERNEL); @@ -3434,15 +3407,23 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, /* Request for full descriptor */ ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC, - desc_id, desc_index, 0, - desc_buf, &buff_len); - + desc_id, desc_index, 0, + desc_buf, &buff_len); if (ret) { dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n", __func__, desc_id, desc_index, param_offset, ret); goto out; } + /* Update descriptor length */ + buff_len = desc_buf[QUERY_DESC_LENGTH_OFFSET]; + + if (param_offset >= buff_len) { + dev_err(hba->dev, "%s: Invalid offset 0x%x in descriptor IDN 0x%x, length 0x%x\n", + __func__, param_offset, desc_id, buff_len); + return -EINVAL; + } + /* Sanity check */ if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id) { dev_err(hba->dev, "%s: invalid desc_id %d in descriptor header\n", @@ -3451,9 +3432,6 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, goto out; } - /* Update descriptor length */ - buff_len = desc_buf[QUERY_DESC_LENGTH_OFFSET]; - if (is_kmalloc) { /* Make sure we don't copy more data than available */ if (param_offset >= buff_len) diff --git a/drivers/ufs/core/ufshpb.c b/drivers/ufs/core/ufshpb.c index 3d69a81c..053fd66 100644 --- a/drivers/ufs/core/ufshpb.c +++ b/drivers/ufs/core/ufshpb.c @@ -2382,16 +2382,13 @@ static int ufshpb_get_lu_info(struct ufs_hba *hba, int lun, { u16 max_active_rgns; u8 lu_enable; - int size; int ret; char desc_buf[QUERY_DESC_MAX_SIZE]; - ufshcd_map_desc_id_to_length(hba, QUERY_DESC_IDN_UNIT, &size); - ufshcd_rpm_get_sync(hba); ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC, QUERY_DESC_IDN_UNIT, lun, 0, - desc_buf, &size); + desc_buf, QUERY_DESC_MAX_SIZE); ufshcd_rpm_put_sync(hba); if (ret) {
ufshcd_map_desc_id_to_length function is redundant. According to the spec the host can send any descriptor size, the device will return the actual size. Therefore always read the descriptors with QUERY_DESC_MAX_SIZE size Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com> --- drivers/ufs/core/ufs_bsg.c | 7 +------ drivers/ufs/core/ufshcd-priv.h | 3 --- drivers/ufs/core/ufshcd.c | 46 +++++++++++------------------------------- drivers/ufs/core/ufshpb.c | 5 +---- 4 files changed, 14 insertions(+), 47 deletions(-)