From 955f7e7474f4d5ebf690f7d2fe30dcc0da3a1d09 Mon Sep 17 00:00:00 2001 From: "Kenneth D. Merry" Date: Sun, 20 Feb 2000 04:42:44 +0000 Subject: [PATCH] Fix 'camcontrol inquiry'. The inquiry data structure changes (increased to 256 bytes) caused it to break on many devices. The SCSI spec says that for commands with 8-bit length fields, a value of 0 means 256 bytes. As it turns out, many devices don't deal with that properly. Some interpret the 0 as 0, and return no data. Others return more than 256 bytes of data, and cause an overrun. The fix is to tell the device we've only allocated SHORT_INQUIRY_LENGTH (36 bytes) of inquiry data, instead of sizeof(struct scsi_inquiry_data). camcontrol.c: Change inq_len in the call to scsi_inquiry() to SHORT_INQUIRY_LENGTH, and add a long comment explaining the reason for the change. scsi_all.h: Add a comment above the definitinon of SHORT_INQUIRY_LENGTH alerting people that it is both the initial probe inquiry length, and the minimum amount of data needed for scsi_print_inquiry() to function. scsi_all.c: Add a comment about SHORT_INQUIRY_LENGTH being the minimum amount of data needed for scsi_print_inquiry() to function. Reviewed by: gibbs Approved by: jkh Reported by: "John W. DeBoskey" --- sbin/camcontrol/camcontrol.c | 37 ++++++++++++++++++++++++++++++++++-- sys/cam/scsi/scsi_all.c | 7 +++++++ sys/cam/scsi/scsi_all.h | 7 ++++++- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/sbin/camcontrol/camcontrol.c b/sbin/camcontrol/camcontrol.c index 1d204f148b3..f415c2b6ab1 100644 --- a/sbin/camcontrol/camcontrol.c +++ b/sbin/camcontrol/camcontrol.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 1998, 1999 Kenneth D. Merry + * Copyright (c) 1997, 1998, 1999, 2000 Kenneth D. Merry * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -645,12 +645,45 @@ scsiinquiry(struct cam_device *device, int retry_count, int timeout) } bzero(inq_buf, sizeof(*inq_buf)); + /* + * Note that although the size of the inquiry buffer is the full + * 256 bytes specified in the SCSI spec, we only tell the device + * that we have allocated SHORT_INQUIRY_LENGTH bytes. There are + * two reasons for this: + * + * - The SCSI spec says that when a length field is only 1 byte, + * a value of 0 will be interpreted as 256. Therefore + * scsi_inquiry() will convert an inq_len (which is passed in as + * a u_int32_t, but the field in the CDB is only 1 byte) of 256 + * to 0. Evidently, very few devices meet the spec in that + * regard. Some devices, like many Seagate disks, take the 0 as + * 0, and don't return any data. One Pioneer DVD-R drive + * returns more data than the command asked for. + * + * So, since there are numerous devices that just don't work + * right with the full inquiry size, we don't send the full size. + * + * - The second reason not to use the full inquiry data length is + * that we don't need it here. The only reason we issue a + * standard inquiry is to get the vendor name, device name, + * and revision so scsi_print_inquiry() can print them. + * + * If, at some point in the future, more inquiry data is needed for + * some reason, this code should use a procedure similar to the + * probe code. i.e., issue a short inquiry, and determine from + * the additional length passed back from the device how much + * inquiry data the device supports. Once the amount the device + * supports is determined, issue an inquiry for that amount and no + * more. + * + * KDM, 2/18/2000 + */ scsi_inquiry(&ccb->csio, /* retries */ retry_count, /* cbfcnp */ NULL, /* tag_action */ MSG_SIMPLE_Q_TAG, /* inq_buf */ (u_int8_t *)inq_buf, - /* inq_len */ sizeof(struct scsi_inquiry_data), + /* inq_len */ SHORT_INQUIRY_LENGTH, /* evpd */ 0, /* page_code */ 0, /* sense_len */ SSD_FULL_SIZE, diff --git a/sys/cam/scsi/scsi_all.c b/sys/cam/scsi/scsi_all.c index 10d4cc8595b..2844ac47f0e 100644 --- a/sys/cam/scsi/scsi_all.c +++ b/sys/cam/scsi/scsi_all.c @@ -2290,6 +2290,13 @@ scsi_interpret_sense(struct cam_device *device, union ccb *ccb, return (error); } +/* + * This function currently requires at least 36 bytes, or + * SHORT_INQUIRY_LENGTH, worth of data to function properly. If this + * function needs more or less data in the future, another length should be + * defined in scsi_all.h to indicate the minimum amount of data necessary + * for this routine to function properly. + */ void scsi_print_inquiry(struct scsi_inquiry_data *inq_data) { diff --git a/sys/cam/scsi/scsi_all.h b/sys/cam/scsi/scsi_all.h index 37caa19e6f5..083b83686ba 100644 --- a/sys/cam/scsi/scsi_all.h +++ b/sys/cam/scsi/scsi_all.h @@ -477,7 +477,12 @@ struct scsi_start_stop_unit #define T_REMOV 1 #define T_FIXED 0 - +/* + * This length is the initial inquiry length used by the probe code, as + * well as the legnth necessary for scsi_print_inquiry() to function + * correctly. If either use requires a different length in the future, + * the two values should be de-coupled. + */ #define SHORT_INQUIRY_LENGTH 36 struct scsi_inquiry_data