Date: Mon, 15 Jul 2013 18:02:26 +0000 (UTC) From: "Kenneth D. Merry" <ken@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r253369 - stable/9/sys/cam/scsi Message-ID: <201307151802.r6FI2QMF093694@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ken Date: Mon Jul 15 18:02:26 2013 New Revision: 253369 URL: http://svnweb.freebsd.org/changeset/base/253369 Log: MFC r253274 and r253368: r253274 | ken | 2013-07-12 11:09:50 -0600 (Fri, 12 Jul 2013) | 40 lines Fix a problem with READ ELEMENT STATUS that occurs on some changers that don't support the DVCID and CURDATA bits that were introduced in the SMC spec. These changers will return an Illegal Request type error if the bits are set. This causes "chio status" to fail. The fix is two-fold. First, for changers that claim to be SCSI-2 or older, don't set the DVCID and CURDATA bits for READ ELEMENT STATUS. For newer changers (SCSI-3 and newer), we default to setting the new bits, but back off and try the READ ELEMENT STATUS without the bits if we get an Illegal Request type error. This has been tested on a Qualstar TLS-8211, which is a SCSI-2 changer that does not support the new bits, and a Spectra T-380, which is a SCSI-3 changer that does support the new bits. In the absence of a SCSI-3 changer that does not support the bits, I tested that with some error injection code. (The SMC spec says that support for CURDATA is mandatory, and DVCID is optional.) scsi_ch.c: Add a new quirk, CH_Q_NO_DVCID that gets set for SCSI-2 and older libraries, or newer libraries that report errors when the DVCID/CURDATA bits are set. In chgetelemstatus(), use the new quirk to determine whether or not to set DVCID and CURDATA. If we get an error with the bits set, back off and try without the bits. Set the quirk flag if the read element status succeeds without the bits set. Increase the READ ELEMENT STATUS timeout to 60 seconds after testing with a Spectra T-380. The previous value was 10 seconds, and too short for the T-380. This may be decreased later after some additional testing and investigation. Tested by: Andre Albsmeier <Andre.Albsmeier@siemens.com> Sponsored by: Spectra Logic ------------------------------------------------------------------------ r253368 | ken | 2013-07-15 10:38:48 -0600 (Mon, 15 Jul 2013) | 5 lines Fix an argument reversal in calls to scsi_read_element_status(). Reported by: Ulrich Spoerlein <uqs@FreeBSD.org> Approved by: re (kib) Modified: stable/9/sys/cam/scsi/scsi_ch.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/cam/scsi/scsi_ch.c ============================================================================== --- stable/9/sys/cam/scsi/scsi_ch.c Mon Jul 15 16:38:48 2013 (r253368) +++ stable/9/sys/cam/scsi/scsi_ch.c Mon Jul 15 18:02:26 2013 (r253369) @@ -102,7 +102,7 @@ static const u_int32_t CH_TIMEOUT_MODE_S static const u_int32_t CH_TIMEOUT_MOVE_MEDIUM = 100000; static const u_int32_t CH_TIMEOUT_EXCHANGE_MEDIUM = 100000; static const u_int32_t CH_TIMEOUT_POSITION_TO_ELEMENT = 100000; -static const u_int32_t CH_TIMEOUT_READ_ELEMENT_STATUS = 10000; +static const u_int32_t CH_TIMEOUT_READ_ELEMENT_STATUS = 60000; static const u_int32_t CH_TIMEOUT_SEND_VOLTAG = 10000; static const u_int32_t CH_TIMEOUT_INITIALIZE_ELEMENT_STATUS = 500000; @@ -122,12 +122,14 @@ typedef enum { typedef enum { CH_Q_NONE = 0x00, - CH_Q_NO_DBD = 0x01 + CH_Q_NO_DBD = 0x01, + CH_Q_NO_DVCID = 0x02 } ch_quirks; #define CH_Q_BIT_STRING \ "\020" \ - "\001NO_DBD" + "\001NO_DBD" \ + "\002NO_DVCID" #define ccb_state ppriv_field0 #define ccb_bp ppriv_ptr1 @@ -396,6 +398,14 @@ chregister(struct cam_periph *periph, vo periph->softc = softc; softc->quirks = CH_Q_NONE; + /* + * The DVCID and CURDATA bits were not introduced until the SMC + * spec. If this device claims SCSI-2 or earlier support, then it + * very likely does not support these bits. + */ + if (cgd->inq_data.version <= SCSI_REV_2) + softc->quirks |= CH_Q_NO_DVCID; + bzero(&cpi, sizeof(cpi)); xpt_setup_ccb(&cpi.ccb_h, periph->path, CAM_PRIORITY_NORMAL); cpi.ccb_h.func_code = XPT_PATH_INQ; @@ -1208,6 +1218,8 @@ chgetelemstatus(struct cam_periph *perip caddr_t data = NULL; size_t size, desclen; int avail, i, error = 0; + int curdata, dvcid, sense_flags; + int try_no_dvcid = 0; struct changer_element_status *user_data = NULL; struct ch_softc *softc; union ccb *ccb; @@ -1239,14 +1251,31 @@ chgetelemstatus(struct cam_periph *perip cam_periph_lock(periph); ccb = cam_periph_getccb(periph, CAM_PRIORITY_NORMAL); + sense_flags = SF_RETRY_UA; + if (softc->quirks & CH_Q_NO_DVCID) { + dvcid = 0; + curdata = 0; + } else { + dvcid = 1; + curdata = 1; + /* + * Don't print anything for an Illegal Request, because + * these flags can cause some changers to complain. We'll + * retry without them if we get an error. + */ + sense_flags |= SF_QUIET_IR; + } + +retry_einval: + scsi_read_element_status(&ccb->csio, /* retries */ 1, /* cbfcnp */ chdone, /* tag_action */ MSG_SIMPLE_Q_TAG, /* voltag */ want_voltags, /* sea */ softc->sc_firsts[chet], - /* dvcid */ 1, - /* curdata */ 1, + /* curdata */ curdata, + /* dvcid */ dvcid, /* count */ 1, /* data_ptr */ data, /* dxfer_len */ 1024, @@ -1254,9 +1283,38 @@ chgetelemstatus(struct cam_periph *perip /* timeout */ CH_TIMEOUT_READ_ELEMENT_STATUS); error = cam_periph_runccb(ccb, cherror, /*cam_flags*/ CAM_RETRY_SELTO, - /*sense_flags*/ SF_RETRY_UA, + /*sense_flags*/ sense_flags, softc->device_stats); + /* + * An Illegal Request sense key (only used if there is no asc/ascq) + * or 0x24,0x00 for an ASC/ASCQ both map to EINVAL. If dvcid or + * curdata are set (we set both or neither), try turning them off + * and see if the command is successful. + */ + if ((error == EINVAL) + && (dvcid || curdata)) { + dvcid = 0; + curdata = 0; + error = 0; + /* At this point we want to report any Illegal Request */ + sense_flags &= ~SF_QUIET_IR; + try_no_dvcid = 1; + goto retry_einval; + } + + /* + * In this case, we tried a read element status with dvcid and + * curdata set, and it failed. We retried without those bits, and + * it succeeded. Suggest to the user that he set a quirk, so we + * don't go through the retry process the first time in the future. + * This should only happen on changers that claim SCSI-3 or higher, + * but don't support these bits. + */ + if ((try_no_dvcid != 0) + && (error == 0)) + softc->quirks |= CH_Q_NO_DVCID; + if (error) goto done; cam_periph_unlock(periph); @@ -1284,8 +1342,8 @@ chgetelemstatus(struct cam_periph *perip /* voltag */ want_voltags, /* sea */ softc->sc_firsts[chet] + cesr->cesr_element_base, - /* dvcid */ 1, - /* curdata */ 1, + /* curdata */ curdata, + /* dvcid */ dvcid, /* count */ cesr->cesr_element_count, /* data_ptr */ data, /* dxfer_len */ size,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307151802.r6FI2QMF093694>