Re: [PATCH v1 1/9] scsi: scsi_debug: move ASC and ASCQ definitions to scsi_proto.h

On 7/6/26 8:56 AM, Damien Le Moal wrote:
> The scsi_debug driver internally defines lots of SCSI additional sense
> codes (ASC) and additional sense code qualifiers (ASCQ). Move these
> definitions to include/scsi/scsi_proto.h so that they can be reused
> elsewhere in the SCSI and ATA code. This also makes the scsi_debug.c file
> a little smaller.
> 
> Signed-off-by: Damien Le Moal <[email protected]>
> ---
>   drivers/scsi/scsi_debug.c | 46 -----------------------------------
>   include/scsi/scsi_proto.h | 51 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 51 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 9d1c9c41d0f9..4a95e6bae38b 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -67,52 +67,6 @@ static const char *sdebug_version_date = "20210520";
>   
>   #define MY_NAME "scsi_debug"
>   
> -/* Additional Sense Code (ASC) */
> -#define NO_ADDITIONAL_SENSE 0x0
> -#define OVERLAP_ATOMIC_COMMAND_ASC 0x0
> -#define OVERLAP_ATOMIC_COMMAND_ASCQ 0x23
> -#define FILEMARK_DETECTED_ASCQ 0x1
> -#define EOP_EOM_DETECTED_ASCQ 0x2
> -#define BEGINNING_OF_P_M_DETECTED_ASCQ 0x4
> -#define EOD_DETECTED_ASCQ 0x5
> -#define LOGICAL_UNIT_NOT_READY 0x4
> -#define LOGICAL_UNIT_COMMUNICATION_FAILURE 0x8
> -#define UNRECOVERED_READ_ERR 0x11
> -#define PARAMETER_LIST_LENGTH_ERR 0x1a
> -#define INVALID_OPCODE 0x20
> -#define LBA_OUT_OF_RANGE 0x21
> -#define INVALID_FIELD_IN_CDB 0x24
> -#define INVALID_FIELD_IN_PARAM_LIST 0x26
> -#define WRITE_PROTECTED 0x27
> -#define UA_READY_ASC 0x28
> -#define UA_RESET_ASC 0x29
> -#define UA_CHANGED_ASC 0x2a
> -#define TOO_MANY_IN_PARTITION_ASC 0x3b
> -#define TARGET_CHANGED_ASC 0x3f
> -#define LUNS_CHANGED_ASCQ 0x0e
> -#define INSUFF_RES_ASC 0x55
> -#define INSUFF_RES_ASCQ 0x3
> -#define POWER_ON_RESET_ASCQ 0x0
> -#define POWER_ON_OCCURRED_ASCQ 0x1
> -#define BUS_RESET_ASCQ 0x2	/* scsi bus reset occurred */
> -#define MODE_CHANGED_ASCQ 0x1	/* mode parameters changed */
> -#define CAPACITY_CHANGED_ASCQ 0x9
> -#define SAVING_PARAMS_UNSUP 0x39
> -#define TRANSPORT_PROBLEM 0x4b
> -#define THRESHOLD_EXCEEDED 0x5d
> -#define LOW_POWER_COND_ON 0x5e
> -#define MISCOMPARE_VERIFY_ASC 0x1d
> -#define MICROCODE_CHANGED_ASCQ 0x1	/* with TARGET_CHANGED_ASC */
> -#define MICROCODE_CHANGED_WO_RESET_ASCQ 0x16
> -#define WRITE_ERROR_ASC 0xc
> -#define UNALIGNED_WRITE_ASCQ 0x4
> -#define WRITE_BOUNDARY_ASCQ 0x5
> -#define READ_INVDATA_ASCQ 0x6
> -#define READ_BOUNDARY_ASCQ 0x7
> -#define ATTEMPT_ACCESS_GAP 0x9
> -#define INSUFF_ZONE_ASCQ 0xe
> -/* see drivers/scsi/sense_codes.h */
> -
>   /* Additional Sense Code Qualifier (ASCQ) */
>   #define ACK_NAK_TO 0x3
>   
> diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
> index f64385cde5b9..965cde7ebc5b 100644
> --- a/include/scsi/scsi_proto.h
> +++ b/include/scsi/scsi_proto.h
> @@ -233,6 +233,57 @@ enum sam_status {
>   #define MISCOMPARE          0x0e
>   #define COMPLETED	    0x0f
>   
> +/*
> + * Additional Sense Codes (ASC).
> + */
> +#define NO_ADDITIONAL_SENSE		0x00
> +#define OVERLAP_ATOMIC_COMMAND_ASC	0x00
> +#define LOGICAL_UNIT_NOT_READY		0x04
> +#define LOGICAL_UNIT_COMMUNICATION_FAILURE 0x8
> +#define WRITE_ERROR_ASC			0x0c
> +#define UNRECOVERED_READ_ERR		0x11
> +#define PARAMETER_LIST_LENGTH_ERR	0x1a
> +#define MISCOMPARE_VERIFY_ASC		0x1d
> +#define INVALID_OPCODE			0x20
> +#define LBA_OUT_OF_RANGE		0x21
> +#define INVALID_FIELD_IN_CDB		0x24
> +#define INVALID_FIELD_IN_PARAM_LIST	0x26
> +#define WRITE_PROTECTED			0x27
> +#define UA_READY_ASC			0x28
> +#define UA_RESET_ASC			0x29
> +#define UA_CHANGED_ASC			0x2a
> +#define TOO_MANY_IN_PARTITION_ASC	0x3b
> +#define TARGET_CHANGED_ASC		0x3f
> +#define SAVING_PARAMS_UNSUP		0x39
> +#define TRANSPORT_PROBLEM		0x4b
> +#define INSUFF_RES_ASC			0x55
> +#define LOW_POWER_COND_ON		0x5e
> +#define THRESHOLD_EXCEEDED		0x5d
> +
> +/*
> + * Additional Sense Code Qualifiers (ASCQ).
> + */
> +#define POWER_ON_RESET_ASCQ		0x00
> +#define MODE_CHANGED_ASCQ		0x01	/* mode parameters changed */
> +#define FILEMARK_DETECTED_ASCQ		0x01
> +#define POWER_ON_OCCURRED_ASCQ		0x01
> +#define MICROCODE_CHANGED_ASCQ		0x01	/* with TARGET_CHANGED_ASC */
> +#define BUS_RESET_ASCQ			0x02	/* scsi bus reset occurred */
> +#define EOP_EOM_DETECTED_ASCQ		0x02
> +#define INSUFF_RES_ASCQ			0x03
> +#define BEGINNING_OF_P_M_DETECTED_ASCQ	0x04
> +#define UNALIGNED_WRITE_ASCQ		0x04
> +#define EOD_DETECTED_ASCQ		0x05
> +#define WRITE_BOUNDARY_ASCQ		0x05
> +#define READ_INVDATA_ASCQ		0x06
> +#define READ_BOUNDARY_ASCQ		0x07
> +#define CAPACITY_CHANGED_ASCQ		0x09
> +#define ATTEMPT_ACCESS_GAP		0x09
> +#define LUNS_CHANGED_ASCQ		0x0e
> +#define INSUFF_ZONE_ASCQ		0x0e
> +#define MICROCODE_CHANGED_WO_RESET_ASCQ 0x16
> +#define OVERLAP_ATOMIC_COMMAND_ASCQ	0x23
> +
>   /*
>    *  DEVICE TYPES
>    *  Please keep them in 0x%02x format for $MODALIAS to work

Weelll ... _technically_ the ASC/ASCQ codes have to be evaluated
together, and the individual definitions only make sense for a
combination of ASC/ASCQ codes.
EG SPC-5 defines the ASC/ASCQ 0x20/0x00 as 'Invalid command opcode',
but with this we would deocde it as 'INVALID_OPCODE'/'POWER_ON_RESET_ASCQ',
but 'POWER ON RESET OCCURRED' is ASC/ASCQ 0x29/0x00.
So if we were to define ASCQ codes we would need to define the
ASCQ codes for each ASC to avoid these issues.

Makes me wonder if we shouldn't introduce u16 for sense code
handling ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich