expand information in the scsi_sense_hdr
RFC 1/1 expand information in the scsi_sense_hdr
Since the scsi_execute_req() call is trying to make this the standard way of passing back sense (mainly because scsi_execute_req() is designed for internal consumption, which has more simple sense parsing requirements) there needs to be enough information in the header for all sense uses within the kernel.
This patch adds the info field, the flags field, the sense key specific field and the command info field to the sense header. The disadvantage is that it adds nineteen bytes to the sense header.
James
drivers/scsi/scsi_error.c +80 -6
@@ -1919,7 +1919,14 @@ int scsi_normalize_sense(const u8 *senseif (!scsi_sense_valid (sshdr))return 0 ; -if (sshdr->response_code >=0x72 ) { +if (sshdr->response_code ==0x72 || sshdr->response_code ==0x73 ) + sshdr->descriptor =1 ; +if (sshdr->response_code <0x70 || sshdr->response_code >0x73 ) +return 0 ; + sshdr->deferred = (sshdr->response_code &0x01 ) ?1 :0 ; + +if (sshdr->descriptor) { +const u8 *desc;/* * descriptor format */
@@ -1931,19 +1938,86 @@ int scsi_normalize_sense(const u8 *sense sshdr->ascq = sense_buffer[3 ];if (sb_len >7 ) sshdr->additional_length = sense_buffer[7 ]; + +/* now get descriptor flags data */ + desc =scsi_sense_desc_find (sense_buffer, sb_len, +SCSI_SENSE_DESCRIPTOR_SSC ); +if (desc) + sshdr->flags = desc[3 ] &0xe0 ; + desc =scsi_sense_desc_find (sense_buffer, sb_len, +SCSI_SENSE_DESCRIPTOR_SBC ); +if (desc) + sshdr->flags = desc[3 ] &0x20 ;/* ILI */ + desc =scsi_sense_desc_find (sense_buffer, sb_len, +SCSI_SENSE_DESCRIPTOR_INFO ); +if (desc) { + sshdr->valid = (desc[2 ] &0x80 ) ?1 :0 ; + sshdr->info = + ((u64 )desc[4 ] <<56 ) | + ((u64 )desc[5 ] <<48 ) | + ((u64 )desc[6 ] <<40 ) | + ((u64 )desc[7 ] <<32 ) | + ((u64 )desc[8 ] <<24 ) | + ((u64 )desc[9 ] <<16 ) | + ((u64 )desc[10 ] <<8 ) | + (u64 )desc[11 ]; + } + desc =scsi_sense_desc_find (sense_buffer, sb_len, +SCSI_SENSE_DESCRIPTOR_CMD ); +if (desc) + sshdr->cmd_info = + ((u64 )desc[4 ] <<56 ) | + ((u64 )desc[5 ] <<48 ) | + ((u64 )desc[6 ] <<40 ) | + ((u64 )desc[7 ] <<32 ) | + ((u64 )desc[8 ] <<24 ) | + ((u64 )desc[9 ] <<16 ) | + ((u64 )desc[10 ] <<8 ) | + (u64 )desc[11 ]; + desc =scsi_sense_desc_find (sense_buffer, sb_len, +SCSI_SENSE_DESCRIPTOR_CMD ); +if (desc) { + sshdr->sksv = (desc[2 ] &0x80 ) ?1 :0 ; + sshdr->extra_sense_specific = desc[2 ] &0x7f ; + sshdr->sense_specific = + ((u16 )desc[3 ] <<8 ) | + (u16 )desc[4 ]; + } }else {/* * fixed format */ + sshdr->valid = (sense_buffer[0 ] &0x80 ) ?1 :0 ;if (sb_len >2 ) sshdr->sense_key = (sense_buffer[2 ] &0xf ); -if (sb_len >7 ) { +if (sb_len >3 ) + sshdr->flags = sense_buffer[2 ] &0xe0 ; +if (sb_len >6 && sshdr->valid) + sshdr->info = + ((u32 )sense_buffer[3 ] <<24 ) | + ((u32 )sense_buffer[4 ] <<16 ) | + ((u32 )sense_buffer[5 ] <<8 ) | + (u32 )sense_buffer[6 ]; +if (sb_len >7 ) sb_len = (sb_len < (sense_buffer[7 ] +8 )) ? sb_len : (sense_buffer[7 ] +8 ); -if (sb_len >12 ) - sshdr->asc = sense_buffer[12 ]; -if (sb_len >13 ) - sshdr->ascq = sense_buffer[13 ]; + +if (sb_len >11 ) + sshdr->cmd_info = + ((u32 )sense_buffer[8 ] <<24 ) | + ((u32 )sense_buffer[9 ] <<16 ) | + ((u32 )sense_buffer[10 ] <<8 ) | + (u32 )sense_buffer[11 ]; +if (sb_len >12 ) + sshdr->asc = sense_buffer[12 ]; +if (sb_len >13 ) + sshdr->ascq = sense_buffer[13 ]; +if (sb_len >17 ) { + sshdr->sksv = (sense_buffer[15 ] &0x80 ) ?1 :0 ; +if (sshdr->sksv) + sshdr->sense_specific = + ((u16 )sense_buffer[15 ] &0x7f ) <<8 | + (u16 )sense_buffer[16 ]; } }
include/scsi/scsi_eh.h +18 -2
@@ -16,8 +16,17 @@ struct Scsi_Host;* in which more information is required (e.g. the LBA of a MEDIUM ERROR). */ struct scsi_sense_hdr {/* See SPC-3 section 4.5 */ -u8 response_code;/* permit: 0x0, 0x70, 0x71, 0x72, 0x73 */ -u8 sense_key; +u64 info; +u64 cmd_info; +u16 sense_specific; +u8 extra_sense_specific;/* only valid for descriptor format */ +u8 valid:1 , + response_code:7 ;/* permit: 0x0, 0x70, 0x71, 0x72, 0x73 */ +u8 sksv:1 , + deferred:1 , + descriptor:1 , + sense_key:4 ; +u8 flags;/* FILEMARK, EOM or ILI */ u8 asc;u8 ascq;u8 byte4;
@@ -26,6 +35,13 @@ struct scsi_sense_hdr { /* See SPC-3 seu8 additional_length;/* always 0 for fixed sense format */ }; +/* Descriptor types */ +#define SCSI_SENSE_DESCRIPTOR_INFO 0x00 +#define SCSI_SENSE_DESCRIPTOR_CMD 0x01 +#define SCSI_SENSE_DESCRIPTOR_SKS 0x02 +#define SCSI_SENSE_DESCRIPTOR_SSC 0x04 +#define SCSI_SENSE_DESCRIPTOR_SBC 0x05 +static inline int scsi_sense_valid (struct scsi_sense_hdr *sshdr) {if (!sshdr)
Quoted 9 lines · James Bottomley
Since the
scsi_execute_req()call is trying to make this the standard way of passing back sense (mainly becausescsi_execute_req()is designed for internal consumption, which has more simple sense parsing requirements) there needs to be enough information in the header for all sense uses within the kernel.This patch adds the info field, the flags field, the sense key specific field and the command info field to the sense header. The disadvantage is that it adds nineteen bytes to the sense header.
Another disadvantage is that it doesn't cope with the newer descriptor fields that don't have equivalents in the older fixed format. OSD and SAT have their own sense descriptors. Vendors can (and do) have fixed sense data (defined beyond offset 18 and field replaceable unit) and can add their own sense descriptors [0x80 to 0xff]. Mapping extensible descriptor format (still limited to 252 bytes overall) back to a (different) fixed format could be self defeating.
My intention with struct scsi_sense_hdr was to give a small fixed size view of sense data (from either fixed or descriptor format) sufficient for drivers to do error processing. It is sufficient for many but not all cases in drivers that I converted. I was not trying to replace the sense data. Is the actual sense data still going to be available to the SG_IO ioctl and the sg driver?
BTW If you implement this change, I don't think field member "u8 byte4;" and friends are any longer relevant as that was to maintain the mapping between "struct scsi_sense_data" and the descriptor format sense data header (i.e. both 8 bytes long).
Doug Gilbert
Quoted 9 lines · Douglas Gilbert
Another disadvantage is that it doesn't cope with the newer descriptor fields that don't have equivalents in the older fixed format. OSD and SAT have their own sense descriptors. Vendors can (and do) have fixed sense data (defined beyond offset 18 and field replaceable unit) and can add their own sense descriptors [
0x80to0xff]. Mapping extensible descriptor format (still limited to 252 bytes overall) back to a (different) fixed format could be self defeating.
Remember that the only use is for internal, so if no-one's interested in the information it doesn't matter if we don't use it. If someone becomes interested in OSD or SAT descriptors, they can add them (currently OSD is rather expensive and rare and SAT is the province of SAS which is still under discussion). However, the only use currently is Filemark, ILI and EOM which are really only a property of SSC and SBC.
Quoted 8 lines
My intention with struct
scsi_sense_hdrwas to give a small fixed size view of sense data (from either fixed or descriptor format) sufficient for drivers to do error processing. It is sufficient for many but not all cases in drivers that I converted. I was not trying to replace the sense data. Is the actual sense data still going to be available to theSG_IOioctl and the sg driver?
All commands that report sense data back to the user still send the raw sense data. Like I said, the sense header is for internal kernel use.
Quoted 5 lines
BTW If you implement this change, I don't think field member "u8 byte4;" and friends are any longer relevant as that was to maintain the mapping between "struct
scsi_sense_data" and the descriptor format sense data header (i.e. both 8 bytes long).
OK, I'll junk those, thanks.
James