system/freebsd-src
system/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src synced 2024-09-16 06:52:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Polish SCSI sense data validity checks.

Browse source 
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
This commit is contained in:
Alexander Motin 2019-04-21 19:07:03 +00:00
parent da343996fa
commit ed569aadca
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=346491
2 changed files with 50 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

112
sys/cam/scsi/scsi_all.c
View file

@ -4061,6 +4061,10 @@ scsi_get_sense_info(struct scsi_sense_data *sense_data, u_int sense_len,
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, u_int sense_len,
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 sense_len, uint8_t *sks)
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, u_int sense_len,
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_data, u_int sense_len,
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, uint64_t info)
/*
* 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, uint64_t info)
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_sense_data *sense,
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_data *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_data *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_sense_data *sense,
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_sense_data *sense,
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_int sense_len,
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_int sense_len,
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_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");
}
/*
* Print the info field.
*/
if (scsi_get_sense_info(sense, sense_len, SSD_DESC_INFO,
&val, NULL) == 0)
info_valid = 1;
else
info_valid = 0;
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");
}
&val, NULL) == 0) {
sbuf_cat(sb, path_str);
scsi_info_sbuf(sb, cdb, cdb_len, inq_data, val);
sbuf_printf(sb, "\n");
}
/*

4
sys/cam/scsi/scsi_all.h
View file

@ -3749,8 +3749,8 @@ void scsi_command_sbuf(struct sbuf *sb, uint8_t *cdb, int cdb_len,
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,