[PATCH 0/2] scsi: target: Introduce the version descriptors

[PATCH 0/2] scsi: target: Introduce the version descriptors

[Konstantin Shelekhin]

This patch series adds the VERSION DESCRIPTOR fields to the standard INQUIRY
data. The SCSI primary command set standard bumped to SPC-4 for consistency
since the version descriptor values for SBC-3 and some other standards are not
defined in SPC-3.

While not essential for normal operation, the version descriptors values are
often queried as an additional sanity check. For example, Windows Server
requires proper SCSI transport protocol standard during the iSCSI qualification
tests.

The layout follows SPC-4:

  SCSI architecture standard
  SCSI transport protocol standard
  SCSI primary command set standard
  SCSI device type command set standard

The physical transport standard is not reported because it requires
driver-specific probing to be accurate and I'm not aware of any use-case
to justify the additional complexity.

The values are reported "unclaimed" because I faced several issues with the
specific ones during the interoperability testing. For example, Windows Server
2016 fails to parse recent iSCSI standards, libiscsi failed to recognize the
final SBC-3 standard (however this is fixed now) and so on. Plus both FreeBSD
and SCST use (or were using) the same values, so the chance of breaking
something is much smaller.

Konstantin Shelekhin (2):
  scsi: target: Bump INQUIRY VERSION to SPC-4
  scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

 drivers/target/target_core_spc.c | 36 ++++++++++++++++++++++++++++++--
 include/scsi/scsi_proto.h        | 12 +++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

-- 
2.31.1

[PATCH 1/2] scsi: target: Bump INQUIRY VERSION to SPC-4

[Konstantin Shelekhin]

Bump the SCSI primary command set standard to SPC-4. The upcoming
version descriptors will report newer SCSI standards (like SBC-3) that
are not defined in SPC-3.

Signed-off-by: Konstantin Shelekhin <k.shelekhin@yadro.com>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/target/target_core_spc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 70a661801cb9..c42a954ce641 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -58,7 +58,7 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
 	if (dev->transport->get_device_type(dev) == TYPE_TAPE)
 		buf[1] = 0x80;
 
-	buf[2] = 0x05; /* SPC-3 */
+	buf[2] = 0x06; /* SPC-4 */
 
 	/*
 	 * NORMACA and HISUP = 0, RESPONSE DATA FORMAT = 2
-- 
2.31.1

[PATCH 2/2] scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

[Konstantin Shelekhin]

Extend the standard INQUIRY data to 96 bytes and fill in the VERSION
DESCRIPTOR fields.

The layout follows SPC-4:

  SCSI architecture standard
  SCSI transport protocol standard
  SCSI primary command set standard
  SCSI device type command set standard

All version descriptor values are defined as "unclaimed" because some
initiators fail to recognize anything else.

Signed-off-by: Konstantin Shelekhin <k.shelekhin@yadro.com>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 drivers/target/target_core_spc.c | 34 +++++++++++++++++++++++++++++++-
 include/scsi/scsi_proto.h        | 12 +++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index c42a954ce641..d3b2f3d075bf 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -47,10 +47,32 @@ static void spc_fill_alua_data(struct se_lun *lun, unsigned char *buf)
 	spin_unlock(&lun->lun_tg_pt_gp_lock);
 }
 
+static u16
+spc_find_scsi_transport_vd(int proto_id)
+{
+	switch (proto_id) {
+	case SCSI_PROTOCOL_FCP:
+		return SCSI_VERSION_DESCRIPTOR_FCP4;
+	case SCSI_PROTOCOL_ISCSI:
+		return SCSI_VERSION_DESCRIPTOR_ISCSI;
+	case SCSI_PROTOCOL_SAS:
+		return SCSI_VERSION_DESCRIPTOR_SAS3;
+	case SCSI_PROTOCOL_SBP:
+		return SCSI_VERSION_DESCRIPTOR_SBP3;
+	case SCSI_PROTOCOL_SRP:
+		return SCSI_VERSION_DESCRIPTOR_SRP;
+	default:
+		pr_warn("Cannot find VERSION DESCRIPTOR value for unknown SCSI"
+		        " transport PROTOCOL IDENTIFIER %#x\n", proto_id);
+		return 0;
+	}
+}
+
 sense_reason_t
 spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
 {
 	struct se_lun *lun = cmd->se_lun;
+	struct se_portal_group *tpg = lun->lun_tpg;
 	struct se_device *dev = cmd->se_dev;
 	struct se_session *sess = cmd->se_sess;
 
@@ -108,7 +130,17 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
 	       strnlen(dev->t10_wwn.model, INQUIRY_MODEL_LEN));
 	memcpy(&buf[32], dev->t10_wwn.revision,
 	       strnlen(dev->t10_wwn.revision, INQUIRY_REVISION_LEN));
-	buf[4] = 31; /* Set additional length to 31 */
+
+	/*
+	 * Set the VERSION DESCRIPTOR fields
+	 */
+	put_unaligned_be16(SCSI_VERSION_DESCRIPTOR_SAM5, &buf[58]);
+	put_unaligned_be16(spc_find_scsi_transport_vd(tpg->proto_id), &buf[60]);
+	put_unaligned_be16(SCSI_VERSION_DESCRIPTOR_SPC4, &buf[62]);
+	if (cmd->se_dev->transport->get_device_type(dev) == TYPE_DISK)
+		put_unaligned_be16(SCSI_VERSION_DESCRIPTOR_SBC3, &buf[64]);
+
+	buf[4] = 91; /* Set additional length to 91 */
 
 	return 0;
 }
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index c36860111932..5c106c4f249e 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -341,4 +341,16 @@ enum zbc_zone_cond {
 	ZBC_ZONE_COND_OFFLINE		= 0xf,
 };
 
+/* Version descriptor values for INQUIRY */
+enum scsi_version_descriptor {
+	SCSI_VERSION_DESCRIPTOR_FCP4	= 0x0a40,
+	SCSI_VERSION_DESCRIPTOR_ISCSI	= 0x0960,
+	SCSI_VERSION_DESCRIPTOR_SAM5	= 0x00a0,
+	SCSI_VERSION_DESCRIPTOR_SAS3	= 0x0c60,
+	SCSI_VERSION_DESCRIPTOR_SBC3	= 0x04c0,
+	SCSI_VERSION_DESCRIPTOR_SBP3	= 0x0980,
+	SCSI_VERSION_DESCRIPTOR_SPC4	= 0x0460,
+	SCSI_VERSION_DESCRIPTOR_SRP	= 0x0940
+};
+
 #endif /* _SCSI_PROTO_H_ */
-- 
2.31.1

Re: [PATCH 2/2] scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

Hi Konstantin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.12-rc8 next-20210416]
[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]

url:    https://github.com/0day-ci/linux/commits/Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: ia64-randconfig-s031-20210419 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.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.3-330-g09ec74f6-dirty
        # https://github.com/0day-ci/linux/commit/88a1d08b46b716d2e970659571d353faa4475a88
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
        git checkout 88a1d08b46b716d2e970659571d353faa4475a88
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   drivers/target/target_core_spc.c: note: in included file (through arch/ia64/include/asm/unaligned.h):
   include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (460 becomes 60)
>> include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (4c0 becomes c0)

vim +26 include/linux/unaligned/be_byteshift.h

064106a91be5e7 Harvey Harrison 2008-04-29  22  
064106a91be5e7 Harvey Harrison 2008-04-29  23  static inline void __put_unaligned_be16(u16 val, u8 *p)
064106a91be5e7 Harvey Harrison 2008-04-29  24  {
064106a91be5e7 Harvey Harrison 2008-04-29  25  	*p++ = val >> 8;
064106a91be5e7 Harvey Harrison 2008-04-29 @26  	*p++ = val;
064106a91be5e7 Harvey Harrison 2008-04-29  27  }
064106a91be5e7 Harvey Harrison 2008-04-29  28  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28172 bytes --]

Re: [PATCH 2/2] scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]

Hi Konstantin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.12-rc8 next-20210416]
[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]

url:    https://github.com/0day-ci/linux/commits/Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: ia64-randconfig-s031-20210419 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.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.3-330-g09ec74f6-dirty
        # https://github.com/0day-ci/linux/commit/88a1d08b46b716d2e970659571d353faa4475a88
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
        git checkout 88a1d08b46b716d2e970659571d353faa4475a88
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   drivers/target/target_core_spc.c: note: in included file (through arch/ia64/include/asm/unaligned.h):
   include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (460 becomes 60)
>> include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (4c0 becomes c0)

vim +26 include/linux/unaligned/be_byteshift.h

064106a91be5e7 Harvey Harrison 2008-04-29  22  
064106a91be5e7 Harvey Harrison 2008-04-29  23  static inline void __put_unaligned_be16(u16 val, u8 *p)
064106a91be5e7 Harvey Harrison 2008-04-29  24  {
064106a91be5e7 Harvey Harrison 2008-04-29  25  	*p++ = val >> 8;
064106a91be5e7 Harvey Harrison 2008-04-29 @26  	*p++ = val;
064106a91be5e7 Harvey Harrison 2008-04-29  27  }
064106a91be5e7 Harvey Harrison 2008-04-29  28  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28172 bytes --]

Re: [PATCH 2/2] scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

[Konstantin Shelekhin]

On Mon, Apr 19, 2021 at 02:57:10PM +0800, kernel test robot wrote:
> Hi Konstantin,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on mkp-scsi/for-next]
> [also build test WARNING on scsi/for-next v5.12-rc8 next-20210416]
> [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]
> 
> url:    https://github.com/0day-ci/linux/commits/Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
> config: ia64-randconfig-s031-20210419 (attached as .config)
> compiler: ia64-linux-gcc (GCC) 9.3.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.3-330-g09ec74f6-dirty
>         # https://github.com/0day-ci/linux/commit/88a1d08b46b716d2e970659571d353faa4475a88
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
>         git checkout 88a1d08b46b716d2e970659571d353faa4475a88
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=ia64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> sparse warnings: (new ones prefixed by >>)
>    drivers/target/target_core_spc.c: note: in included file (through arch/ia64/include/asm/unaligned.h):
>    include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (460 becomes 60)
> >> include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (4c0 becomes c0)
> 
> vim +26 include/linux/unaligned/be_byteshift.h
> 
> 064106a91be5e7 Harvey Harrison 2008-04-29  22  
> 064106a91be5e7 Harvey Harrison 2008-04-29  23  static inline void __put_unaligned_be16(u16 val, u8 *p)
> 064106a91be5e7 Harvey Harrison 2008-04-29  24  {
> 064106a91be5e7 Harvey Harrison 2008-04-29  25  	*p++ = val >> 8;
> 064106a91be5e7 Harvey Harrison 2008-04-29 @26  	*p++ = val;
> 064106a91be5e7 Harvey Harrison 2008-04-29  27  }
> 064106a91be5e7 Harvey Harrison 2008-04-29  28  

This looks like a sparse issue to me, no?

Re: [PATCH 2/2] scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

[Konstantin Shelekhin]

[-- Attachment #1: Type: text/plain, Size: 2671 bytes --]

On Mon, Apr 19, 2021 at 02:57:10PM +0800, kernel test robot wrote:
> Hi Konstantin,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on mkp-scsi/for-next]
> [also build test WARNING on scsi/for-next v5.12-rc8 next-20210416]
> [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]
> 
> url:    https://github.com/0day-ci/linux/commits/Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
> config: ia64-randconfig-s031-20210419 (attached as .config)
> compiler: ia64-linux-gcc (GCC) 9.3.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.3-330-g09ec74f6-dirty
>         # https://github.com/0day-ci/linux/commit/88a1d08b46b716d2e970659571d353faa4475a88
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Konstantin-Shelekhin/scsi-target-Introduce-the-version-descriptors/20210413-064553
>         git checkout 88a1d08b46b716d2e970659571d353faa4475a88
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=ia64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> sparse warnings: (new ones prefixed by >>)
>    drivers/target/target_core_spc.c: note: in included file (through arch/ia64/include/asm/unaligned.h):
>    include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (460 becomes 60)
> >> include/linux/unaligned/be_byteshift.h:26:16: sparse: sparse: cast truncates bits from constant value (4c0 becomes c0)
> 
> vim +26 include/linux/unaligned/be_byteshift.h
> 
> 064106a91be5e7 Harvey Harrison 2008-04-29  22  
> 064106a91be5e7 Harvey Harrison 2008-04-29  23  static inline void __put_unaligned_be16(u16 val, u8 *p)
> 064106a91be5e7 Harvey Harrison 2008-04-29  24  {
> 064106a91be5e7 Harvey Harrison 2008-04-29  25  	*p++ = val >> 8;
> 064106a91be5e7 Harvey Harrison 2008-04-29 @26  	*p++ = val;
> 064106a91be5e7 Harvey Harrison 2008-04-29  27  }
> 064106a91be5e7 Harvey Harrison 2008-04-29  28  

This looks like a sparse issue to me, no?

Re: [PATCH 0/2] scsi: target: Introduce the version descriptors

[Martin K. Petersen]

On Thu, 13 May 2021 22:28:02 +0300, Konstantin Shelekhin wrote:

> This patch series adds the VERSION DESCRIPTOR fields to the standard INQUIRY
> data. The SCSI primary command set standard bumped to SPC-4 for consistency
> since the version descriptor values for SBC-3 and some other standards are not
> defined in SPC-3.
> 
> While not essential for normal operation, the version descriptors values are
> often queried as an additional sanity check. For example, Windows Server
> requires proper SCSI transport protocol standard during the iSCSI qualification
> tests.
> 
> [...]

Applied to 5.14/scsi-queue, thanks!

[1/2] scsi: target: Bump INQUIRY VERSION to SPC-4
      https://git.kernel.org/mkp/scsi/c/64ae33ef7486
[2/2] scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data
      https://git.kernel.org/mkp/scsi/c/b790a56d66ea

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/2] scsi: target: Introduce the version descriptors

[Martin K. Petersen]

Konstantin,

> This patch series adds the VERSION DESCRIPTOR fields to the standard
> INQUIRY data. The SCSI primary command set standard bumped to SPC-4
> for consistency since the version descriptor values for SBC-3 and some
> other standards are not defined in SPC-3.

Applied to 5.14/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 0/2] scsi: target: Introduce the version descriptors

[Konstantin Shelekhin]

This patch series adds the VERSION DESCRIPTOR fields to the standard INQUIRY
data. The SCSI primary command set standard bumped to SPC-4 for consistency
since the version descriptor values for SBC-3 and some other standards are not
defined in SPC-3.

While not essential for normal operation, the version descriptors values are
often queried as an additional sanity check. For example, Windows Server
requires proper SCSI transport protocol standard during the iSCSI qualification
tests.

The layout follows SPC-4:

  SCSI architecture standard
  SCSI transport protocol standard
  SCSI primary command set standard
  SCSI device type command set standard

The physical transport standard is not reported because it requires
driver-specific probing to be accurate and I'm not aware of any use-case
to justify the additional complexity.

The values are reported "unclaimed" because I faced several issues with the
specific ones during the interoperability testing. For example, Windows Server
2016 fails to parse recent iSCSI standards, libiscsi failed to recognize the
final SBC-3 standard (however this is fixed now) and so on. Plus both FreeBSD
and SCST use (or were using) the same values, so the chance of breaking
something is much smaller.

Konstantin Shelekhin (2):
  scsi: target: Bump INQUIRY VERSION to SPC-4
  scsi: target: Add the VERSION DESCRIPTOR fields to the INQUIRY data

 drivers/target/target_core_spc.c | 36 ++++++++++++++++++++++++++++++--
 include/scsi/scsi_proto.h        | 12 +++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

-- 
2.31.1