scsi: Do no try to probe for CDL on old drives

Quoted 8 lines · Damien Le Moal

drivers/ata/libata-scsi.c +0 -0

index 92ae4b4f30ac..7aa70af1fc07 100644
@@ -1828,6 +1828,9 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
  		hdr[2] = 0x7; /* claim SPC-5 version compatibility */
  	}
 

drivers/ata/libata-scsi.c static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) · line 1832

Quoted 2 lines

drivers/ata/libata-scsi.c +2 -0

+	if (args->dev->flags & ATA_DFLAG_CDL)
+		hdr[2] = 0xd; /* claim SPC-6 version compatibility */

How about using the symbolic name SCSI_SPC_6 - 1 instead of the literal constant 0xd?

drivers/ata/libata-scsi.c static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)

Quoted 6 lines

drivers/ata/libata-scsi.c +1 -1

-	sdev->scsi_level = inq_result[2] & 0x07;
+	sdev->scsi_level = inq_result[2] & 0x0f;
  	if (sdev->scsi_level >= 2 ||
  	    (sdev->scsi_level == 1 && (inq_result[3] & 0x0f) == 1))
  		sdev->scsi_level++;

Can support for inq_result[3] & 0x0f == 1 be dropped? From an SPC-2 draft from 2001: "A RESPONSE DATA FORMAT field value of two indicates that the data shall be in the format specified in this standard. Response data format values less than two are obsolete. Response data format values greater than two are reserved."

drivers/ata/libata-scsi.c enum scsi_disposition { · line 162

Quoted 8 lines

drivers/ata/libata-scsi.c +3 -0

@@ -157,6 +157,9 @@ enum scsi_disposition {
  #define SCSI_3          4        /* SPC */
  #define SCSI_SPC_2      5
  #define SCSI_SPC_3      6
+#define SCSI_SPC_4	7
+#define SCSI_SPC_5	8
+#define SCSI_SPC_6	14

Please consider changing the SCSI_SPC_* constants such that these match the SPC standard. Having numerical values that do not match the standard is confusing.

Thanks,

Bart.

In any case, such a change is outside the scope of this patch and should be a followup.

Some old drives (e.g. an Ultra320 SCSI disk as reported by John) do not seem to execute MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES commands correctly and hang when a non-zero service action is specified (one command format with service action case in scsi_report_opcode()).

Currently, CDL probing with scsi_cdl_check_cmd() is the only caller using a non zero service action for scsi_report_opcode(). To avoid issues with these old drives, do not attempt CDL probe if the device reports support for an SPC version lower than 5 (CDL was introduced in SPC-5). To keep things working with ATA devices which probe for the CDL T2A and T2B pages introduced with SPC-6, modify ata_scsiop_inq_std() to claim SPC-6 version compatibility for ATA drives supporting CDL.

SPC-6 standard version number is defined as Dh (= 13) in SPC-6 r09. Fix scsi_probe_lun() to correctly capture this value by changing the bit mask for the second byte of the INQUIRY response from 0x7 to 0xf. include/scsi/scsi.h is modified to add the definition SCSI_SPC_6 with the value 14 (Dh + 1). The missing definitions for the SCSI_SPC_4 and SCSI_SPC_5 versions are also added.

Reported-by John David Anglin <[email protected]>

Fixes 624885209f31 ("scsi: core: Detect support for command duration limits")

Cc [email protected]

Signed-off-by Damien Le Moal <[email protected]>

---

Some old drives (e.g. an Ultra320 SCSI disk as reported by John) do not seem to execute MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES commands correctly and hang when a non-zero service action is specified (one command format with service action case in scsi_report_opcode()).

Currently, CDL probing with scsi_cdl_check_cmd() is the only caller using a non zero service action for scsi_report_opcode(). To avoid issues with these old drives, do not attempt CDL probe if the device reports support for an SPC version lower than 5 (CDL was introduced in SPC-5). To keep things working with ATA devices which probe for the CDL T2A and T2B pages introduced with SPC-6, modify ata_scsiop_inq_std() to claim SPC-6 version compatibility for ATA drives supporting CDL.

SPC-6 standard version number is defined as Dh (= 13) in SPC-6 r09. Fix scsi_probe_lun() to correctly capture this value by changing the bit mask for the second byte of the INQUIRY response from 0x7 to 0xf. include/scsi/scsi.h is modified to add the definition SCSI_SPC_6 with the value 14 (Dh + 1). The missing definitions for the SCSI_SPC_4 and SCSI_SPC_5 versions are also added.

Reported-by John David Anglin <[email protected]>

Fixes 624885209f31 ("scsi: core: Detect support for command duration limits")

Cc [email protected]

Signed-off-by Damien Le Moal <[email protected]>

---
 drivers/ata/libata-scsi.c |  3 +++
 drivers/scsi/scsi.c       | 11 +++++++++++
 drivers/scsi/scsi_scan.c  |  2 +-
 include/scsi/scsi.h       |  3 +++
 4 files changed, 18 insertions(+), 1 deletion(-)

drivers/ata/libata-scsi.c +3 -0

index 92ae4b4f30ac..7aa70af1fc07 100644
@@ -1828,6 +1828,9 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 		hdr[2] = 0x7; /* claim SPC-5 version compatibility */
 	}
 
+	if (args->dev->flags & ATA_DFLAG_CDL)
+		hdr[2] = 0xd; /* claim SPC-6 version compatibility */
+
 	memcpy(rbuf, hdr, sizeof(hdr));
 	memcpy(&rbuf[8], "ATA     ", 8);
 	ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);

drivers/scsi/scsi.c +11 -0

index d0911bc28663..89367c4bf0ef 100644
@@ -613,6 +613,17 @@ void scsi_cdl_check(struct scsi_device *sdev)
 	bool cdl_supported;
 	unsigned char *buf;
 
+	/*
+	 * Support for CDL was defined in SPC-5. Ignore devices reporting an
+	 * lower SPC version. This also avoids problems with old drives choking
+	 * on MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES with a
+	 * service action specified, as done in scsi_cdl_check_cmd().
+	 */
+	if (sdev->scsi_level < SCSI_SPC_5) {
+		sdev->cdl_supported = 0;
+		return;
+	}
+
 	buf = kmalloc(SCSI_CDL_CHECK_BUF_LEN, GFP_KERNEL);
 	if (!buf) {
 		sdev->cdl_supported = 0;

drivers/scsi/scsi_scan.c +1 -1

index 6650f63afec9..37dd6bbcffd3 100644
@@ -822,7 +822,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	 * device is attached at LUN 0 (SCSI_SCAN_TARGET_PRESENT) so
 	 * non-zero LUNs can be scanned.
 	 */
-	sdev->scsi_level = inq_result[2] & 0x07;
+	sdev->scsi_level = inq_result[2] & 0x0f;
 	if (sdev->scsi_level >= 2 ||
 	    (sdev->scsi_level == 1 && (inq_result[3] & 0x0f) == 1))
 		sdev->scsi_level++;

include/scsi/scsi.h +3 -0

index ec093594ba53..4498f845b112 100644
@@ -157,6 +157,9 @@ enum scsi_disposition {
 #define SCSI_3          4        /* SPC */
 #define SCSI_SPC_2      5
 #define SCSI_SPC_3      6
+#define SCSI_SPC_4	7
+#define SCSI_SPC_5	8
+#define SCSI_SPC_6	14
 
 /*
  * INQ PERIPHERAL QUALIFIERS

Signature · 1 line

2.41.0

Some old drives (e.g. an Ultra320 SCSI disk as reported by John) do not seem to execute MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES commands correctly and hang when a non-zero service action is specified (one command format with service action case in scsi_report_opcode()).

Currently, CDL probing with scsi_cdl_check_cmd() is the only caller using a non zero service action for scsi_report_opcode(). To avoid issues with these old drives, do not attempt CDL probe if the device reports support for an SPC version lower than 5 (CDL was introduced in SPC-5). To keep things working with ATA devices which probe for the CDL T2A and T2B pages introduced with SPC-6, modify ata_scsiop_inq_std() to claim SPC-6 version compatibility for ATA drives supporting CDL.

[...]