From owner-svn-src-all@freebsd.org Tue Sep 3 14:08:09 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AEFD9DD76C; Tue, 3 Sep 2019 14:07:15 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2610:1c1:1:6074::16:84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46N80M0tVCz4QQ6; Tue, 3 Sep 2019 14:07:15 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 1452) id B564E1B358; Tue, 3 Sep 2019 14:06:33 +0000 (UTC) X-Original-To: yuripv@localmail.freebsd.org Delivered-To: yuripv@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id BB1EA128; Sun, 21 Apr 2019 19:07:27 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2610:1c1:1:6074::16:84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9F3C884963; Sun, 21 Apr 2019 19:07:11 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 538) id BA19B1FEAE; Sun, 21 Apr 2019 19:07:04 +0000 (UTC) Delivered-To: src-committers@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id A0BD41FEA7 for ; Sun, 21 Apr 2019 19:07:04 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0C0B384890; Sun, 21 Apr 2019 19:07:04 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id DB5881971; Sun, 21 Apr 2019 19:07:03 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x3LJ739r010296; Sun, 21 Apr 2019 19:07:03 GMT (envelope-from mav@FreeBSD.org) Received: (from mav@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x3LJ73vc010293; Sun, 21 Apr 2019 19:07:03 GMT (envelope-from mav@FreeBSD.org) Message-Id: <201904211907.x3LJ73vc010293@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mav set sender to mav@FreeBSD.org using -f From: Alexander Motin To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r346491 - head/sys/cam/scsi X-SVN-Group: head X-SVN-Commit-Author: mav X-SVN-Commit-Paths: head/sys/cam/scsi X-SVN-Commit-Revision: 346491 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk X-Loop: FreeBSD.org Sender: owner-src-committers@freebsd.org X-Rspamd-Queue-Id: 9F3C884963 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.96 / 15.00]; local_wl_from(0.00)[freebsd.org]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.96)[-0.958,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Status: O X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Tue, 03 Sep 2019 14:08:10 -0000 X-Original-Date: Sun, 21 Apr 2019 19:07:03 +0000 (UTC) X-List-Received-Date: Tue, 03 Sep 2019 14:08:10 -0000 Author: mav Date: Sun Apr 21 19:07:03 2019 New Revision: 346491 URL: https://svnweb.freebsd.org/changeset/base/346491 Log: Polish SCSI sense data validity checks. According to specs and common sense, all sense data reported in descriptor format should be valid. But practice shows different, some devices return descriptors with invalid data, resulting in error messages looking worse. Decouple block/stream commands sense data and information field printing. Looking on present specs, there are much more cases when those fields are not related, and incomplete old code was not printing valid sense data and leaving empty lines for invalid. MFC after: 2 weeks Modified: head/sys/cam/scsi/scsi_all.c head/sys/cam/scsi/scsi_all.h Modified: head/sys/cam/scsi/scsi_all.c ============================================================================== --- head/sys/cam/scsi/scsi_all.c Sun Apr 21 18:27:13 2019 (r346490) +++ head/sys/cam/scsi/scsi_all.c Sun Apr 21 19:07:03 2019 (r346491) @@ -4061,6 +4061,10 @@ scsi_get_sense_info(struct scsi_sense_data *sense_data struct scsi_sense_info *info_desc; info_desc = (struct scsi_sense_info *)desc; + + if ((info_desc->byte2 & SSD_INFO_VALID) == 0) + goto bailout; + *info = scsi_8btou64(info_desc->info); if (signed_info != NULL) *signed_info = *info; @@ -4081,6 +4085,9 @@ scsi_get_sense_info(struct scsi_sense_data *sense_data fru_desc = (struct scsi_sense_fru *)desc; + if (fru_desc->fru == 0) + goto bailout; + *info = fru_desc->fru; if (signed_info != NULL) *signed_info = (int8_t)fru_desc->fru; @@ -4181,10 +4188,9 @@ scsi_get_sks(struct scsi_sense_data *sense_data, u_int if (desc == NULL) goto bailout; - /* - * No need to check the SKS valid bit for descriptor sense. - * If the descriptor is present, it is valid. - */ + if ((desc->sense_key_spec[0] & SSD_SKS_VALID) == 0) + goto bailout; + bcopy(desc->sense_key_spec, sks, sizeof(desc->sense_key_spec)); break; } @@ -4261,9 +4267,6 @@ scsi_get_block_info(struct scsi_sense_data *sense_data if (SSD_FIXED_IS_PRESENT(sense, sense_len, flags) == 0) goto bailout; - if ((sense->flags & SSD_ILI) == 0) - goto bailout; - *block_bits = sense->flags & SSD_ILI; break; } @@ -4317,9 +4320,6 @@ scsi_get_stream_info(struct scsi_sense_data *sense_dat if (SSD_FIXED_IS_PRESENT(sense, sense_len, flags) == 0) goto bailout; - if ((sense->flags & (SSD_ILI|SSD_EOM|SSD_FILEMARK)) == 0) - goto bailout; - *stream_bits = sense->flags & (SSD_ILI|SSD_EOM|SSD_FILEMARK); break; } @@ -4361,8 +4361,6 @@ scsi_progress_sbuf(struct sbuf *sb, uint16_t progress) int scsi_sks_sbuf(struct sbuf *sb, int sense_key, uint8_t *sks) { - if ((sks[0] & SSD_SKS_VALID) == 0) - return (1); switch (sense_key) { case SSD_KEY_ILLEGAL_REQUEST: { @@ -4459,7 +4457,7 @@ scsi_fru_sbuf(struct sbuf *sb, uint64_t fru) } void -scsi_stream_sbuf(struct sbuf *sb, uint8_t stream_bits, uint64_t info) +scsi_stream_sbuf(struct sbuf *sb, uint8_t stream_bits) { int need_comma; @@ -4467,6 +4465,7 @@ scsi_stream_sbuf(struct sbuf *sb, uint8_t stream_bits, /* * XXX KDM this needs more descriptive decoding. */ + sbuf_printf(sb, "Stream Command Sense Data: "); if (stream_bits & SSD_DESC_STREAM_FM) { sbuf_printf(sb, "Filemark"); need_comma = 1; @@ -4479,15 +4478,15 @@ scsi_stream_sbuf(struct sbuf *sb, uint8_t stream_bits, if (stream_bits & SSD_DESC_STREAM_ILI) sbuf_printf(sb, "%sILI", (need_comma) ? "," : ""); - - sbuf_printf(sb, ": Info: %#jx", (uintmax_t) info); } void -scsi_block_sbuf(struct sbuf *sb, uint8_t block_bits, uint64_t info) +scsi_block_sbuf(struct sbuf *sb, uint8_t block_bits) { + + sbuf_printf(sb, "Block Command Sense Data: "); if (block_bits & SSD_DESC_BLOCK_ILI) - sbuf_printf(sb, "ILI: residue %#jx", (uintmax_t) info); + sbuf_printf(sb, "ILI"); } void @@ -4500,6 +4499,9 @@ scsi_sense_info_sbuf(struct sbuf *sb, struct scsi_sens info = (struct scsi_sense_info *)header; + if ((info->byte2 & SSD_INFO_VALID) == 0) + return; + scsi_info_sbuf(sb, cdb, cdb_len, inq_data, scsi_8btou64(info->info)); } @@ -4528,6 +4530,9 @@ scsi_sense_sks_sbuf(struct sbuf *sb, struct scsi_sense sks = (struct scsi_sense_sks *)header; + if ((sks->sense_key_spec[0] & SSD_SKS_VALID) == 0) + return; + scsi_extract_sense_len(sense, sense_len, &error_code, &sense_key, &asc, &ascq, /*show_errors*/ 1); @@ -4544,6 +4549,9 @@ scsi_sense_fru_sbuf(struct sbuf *sb, struct scsi_sense fru = (struct scsi_sense_fru *)header; + if (fru->fru == 0) + return; + scsi_fru_sbuf(sb, (uint64_t)fru->fru); } @@ -4554,14 +4562,9 @@ scsi_sense_stream_sbuf(struct sbuf *sb, struct scsi_se struct scsi_sense_desc_header *header) { struct scsi_sense_stream *stream; - uint64_t info; stream = (struct scsi_sense_stream *)header; - info = 0; - - scsi_get_sense_info(sense, sense_len, SSD_DESC_INFO, &info, NULL); - - scsi_stream_sbuf(sb, stream->byte3, info); + scsi_stream_sbuf(sb, stream->byte3); } void @@ -4571,14 +4574,9 @@ scsi_sense_block_sbuf(struct sbuf *sb, struct scsi_sen struct scsi_sense_desc_header *header) { struct scsi_sense_block *block; - uint64_t info; block = (struct scsi_sense_block *)header; - info = 0; - - scsi_get_sense_info(sense, sense_len, SSD_DESC_INFO, &info, NULL); - - scsi_block_sbuf(sb, block->byte3, info); + scsi_block_sbuf(sb, block->byte3); } void @@ -4863,7 +4861,7 @@ scsi_sense_only_sbuf(struct scsi_sense_data *sense, u_ const char *asc_desc; uint8_t sks[3]; uint64_t val; - int info_valid; + uint8_t bits; /* * Get descriptions for the sense key, ASC, and ASCQ. If @@ -4882,42 +4880,28 @@ scsi_sense_only_sbuf(struct scsi_sense_data *sense, u_ sbuf_printf(sb, " asc:%x,%x (%s)\n", asc, ascq, asc_desc); /* - * Get the info field if it is valid. + * Print any block or stream device-specific information. */ - if (scsi_get_sense_info(sense, sense_len, SSD_DESC_INFO, - &val, NULL) == 0) - info_valid = 1; - else - info_valid = 0; + if (scsi_get_block_info(sense, sense_len, inq_data, + &bits) == 0 && bits != 0) { + sbuf_cat(sb, path_str); + scsi_block_sbuf(sb, bits); + sbuf_printf(sb, "\n"); + } else if (scsi_get_stream_info(sense, sense_len, inq_data, + &bits) == 0 && bits != 0) { + sbuf_cat(sb, path_str); + scsi_stream_sbuf(sb, bits); + sbuf_printf(sb, "\n"); + } - if (info_valid != 0) { - uint8_t bits; - - /* - * Determine whether we have any block or stream - * device-specific information. - */ - if (scsi_get_block_info(sense, sense_len, inq_data, - &bits) == 0) { - sbuf_cat(sb, path_str); - scsi_block_sbuf(sb, bits, val); - sbuf_printf(sb, "\n"); - } else if (scsi_get_stream_info(sense, sense_len, - inq_data, &bits) == 0) { - sbuf_cat(sb, path_str); - scsi_stream_sbuf(sb, bits, val); - sbuf_printf(sb, "\n"); - } else if (val != 0) { - /* - * The information field can be valid but 0. - * If the block or stream bits aren't set, - * and this is 0, it isn't terribly useful - * to print it out. - */ - sbuf_cat(sb, path_str); - scsi_info_sbuf(sb, cdb, cdb_len, inq_data, val); - sbuf_printf(sb, "\n"); - } + /* + * Print the info field. + */ + if (scsi_get_sense_info(sense, sense_len, SSD_DESC_INFO, + &val, NULL) == 0) { + sbuf_cat(sb, path_str); + scsi_info_sbuf(sb, cdb, cdb_len, inq_data, val); + sbuf_printf(sb, "\n"); } /* Modified: head/sys/cam/scsi/scsi_all.h ============================================================================== --- head/sys/cam/scsi/scsi_all.h Sun Apr 21 18:27:13 2019 (r346490) +++ head/sys/cam/scsi/scsi_all.h Sun Apr 21 19:07:03 2019 (r346491) @@ -3749,8 +3749,8 @@ void scsi_command_sbuf(struct sbuf *sb, uint8_t *cdb, void scsi_progress_sbuf(struct sbuf *sb, uint16_t progress); int scsi_sks_sbuf(struct sbuf *sb, int sense_key, uint8_t *sks); void scsi_fru_sbuf(struct sbuf *sb, uint64_t fru); -void scsi_stream_sbuf(struct sbuf *sb, uint8_t stream_bits, uint64_t info); -void scsi_block_sbuf(struct sbuf *sb, uint8_t block_bits, uint64_t info); +void scsi_stream_sbuf(struct sbuf *sb, uint8_t stream_bits); +void scsi_block_sbuf(struct sbuf *sb, uint8_t block_bits); void scsi_sense_info_sbuf(struct sbuf *sb, struct scsi_sense_data *sense, u_int sense_len, uint8_t *cdb, int cdb_len, struct scsi_inquiry_data *inq_data,