[PATCH 011/117] Introduce the scsi_status union

Bart Van Assche
2021-04-20T00:09:10Z

Introduce the scsi_status union, a data structure that will be used in the
next patches to replace SCSI status codes represented as an integer. Define
that data structure as follows:

union scsi_status {
int32_t combined;
struct {
#if defined(__BIG_ENDIAN)
enum driver_status driver;
enum host_status host;
enum msg_byte msg;
enum sam_status status;
#elif defined(__LITTLE_ENDIAN)
enum sam_status status;
enum msg_byte msg;
enum host_status host;
enum driver_status driver;
#else
#error Endianness?
#endif
} b;
};

The 'combined' member makes it easy to convert existing SCSI code. The
'status', 'msg', 'host' and 'driver' enable access of individual SCSI
status fields in a type-safe fashion.

Change 'int result;' into the following to enable converting one driver at
a time:

union {
int result;
union scsi_status status;
};

A later patch will remove the outer union and 'int result;'.

Also to enable converting one driver at a time, make scsi_status_is_good(),
status_byte(), msg_byte(), host_byte() and driver_byte() accept an int or
union scsi_status as argument. A later patch will make this function and
these macros accept the scsi_status union only.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Can Guo <cang@codeaurora.org>
Cc: James Smart <james.smart@broadcom.com>
Cc: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/scsi.c | 9 +++++++++
include/linux/bsg-lib.h | 6 +++++-
include/scsi/scsi.h | 24 +++++++++++++++++++-----
include/scsi/scsi_bsg_iscsi.h | 6 +++++-
include/scsi/scsi_cmnd.h | 14 +++++++++-----
include/scsi/scsi_eh.h | 7 ++++++-
include/scsi/scsi_request.h | 6 +++++-
include/scsi/scsi_status.h | 29 +++++++++++++++++++++++++++++
include/uapi/scsi/scsi_bsg_fc.h | 10 ++++++++++
include/uapi/scsi/scsi_bsg_ufs.h | 11 +++++++++++
10 files changed, 108 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index e9e2f0e15ac8..4f71f2005be4 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -763,10 +763,19 @@ MODULE_LICENSE("GPL");
module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");

+#define TEST_STATUS ((union scsi_status){.combined = 0x01020308})
+
static int __init init_scsi(void)
{
int error;

+ BUILD_BUG_ON(sizeof(union scsi_status) != 4);
+ BUILD_BUG_ON(TEST_STATUS.combined != 0x01020308);
+ BUILD_BUG_ON(driver_byte(TEST_STATUS) != 1);
+ BUILD_BUG_ON(host_byte(TEST_STATUS) != 2);
+ BUILD_BUG_ON(msg_byte(TEST_STATUS) != 3);
+ BUILD_BUG_ON(status_byte(TEST_STATUS) != 4);
+
error = scsi_init_procfs();
if (error)
goto cleanup_queue;
diff --git a/include/linux/bsg-lib.h b/include/linux/bsg-lib.h
index 960988d42f77..f934afc45760 100644
--- a/include/linux/bsg-lib.h
+++ b/include/linux/bsg-lib.h
@@ -11,6 +11,7 @@

#include <linux/blkdev.h>
#include <scsi/scsi_request.h>
+#include <scsi/scsi_status.h>

struct request;
struct device;
@@ -52,7 +53,10 @@ struct bsg_job {
struct bsg_buffer request_payload;
struct bsg_buffer reply_payload;

- int result;
+ union {
+ int result; /* do not use in new code */
+ union scsi_status status;
+ };
unsigned int reply_payload_rcv_len;

/* BIDI support */
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index c9ccb6b45b76..18bb1fb2458f 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -39,7 +39,7 @@ enum scsi_timeouts {
* This returns true for known good conditions that may be treated as
* command completed normally
*/
-static inline int scsi_status_is_good(int status)
+static inline bool __scsi_status_is_good(int status)
{
/*
* FIXME: bit0 is listed as reserved in SCSI-2, but is
@@ -56,6 +56,20 @@ static inline int scsi_status_is_good(int status)
(status == SAM_STAT_COMMAND_TERMINATED));
}

+/*
+ * If the 'status' argument has type int, unsigned int or union scsi_status,
+ * return the combined SCSI status. If the 'status' argument has another type,
+ * trigger a compiler error by passing a struct to a context where an integer
+ * is expected.
+ */
+#define scsi_status_to_int(status) \
+ __builtin_choose_expr(sizeof(status) == 4, \
+ *(int32_t *)&(status), \
+ (union scsi_status){})
+
+#define scsi_status_is_good(status) \
+ __scsi_status_is_good(scsi_status_to_int(status))
+

/*
* standard mode-select header prepended to all mode-select commands
@@ -134,10 +148,10 @@ enum scsi_disposition {
* driver_byte = set by mid-level.
*/
#define status_byte(result) ((enum sam_status_divided_by_two) \
- (((result) >> 1) & 0x7f))
-#define msg_byte(result) (((result) >> 8) & 0xff)
-#define host_byte(result) (((result) >> 16) & 0xff)
-#define driver_byte(result) (((result) >> 24) & 0xff)
+ ((scsi_status_to_int((result)) >> 1) & 0x7f))
+#define msg_byte(result) ((scsi_status_to_int((result)) >> 8) & 0xff)
+#define host_byte(result) ((scsi_status_to_int((result)) >> 16) & 0xff)
+#define driver_byte(result) ((scsi_status_to_int((result)) >> 24) & 0xff)

#define sense_class(sense) (((sense) >> 4) & 0x7)
#define sense_error(sense) ((sense) & 0xf)
diff --git a/include/scsi/scsi_bsg_iscsi.h b/include/scsi/scsi_bsg_iscsi.h
index 6b8128005af8..d18e7e061927 100644
--- a/include/scsi/scsi_bsg_iscsi.h
+++ b/include/scsi/scsi_bsg_iscsi.h
@@ -13,6 +13,7 @@
*/

#include <scsi/scsi.h>
+#include <scsi/scsi_status.h>

/*
* iSCSI Transport SGIO v4 BSG Message Support
@@ -82,7 +83,10 @@ struct iscsi_bsg_reply {
* msg and status fields. The per-msgcode reply structure
* will contain valid data.
*/
- uint32_t result;
+ union {
+ uint32_t result; /* do not use in new code */
+ union scsi_status status;
+ };

/* If there was reply_payload, how much was recevied ? */
uint32_t reply_payload_rcv_len;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 202106e7c814..539be97b0a7d 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -140,7 +140,11 @@ struct scsi_cmnd {
* obtained by scsi_malloc is guaranteed
* to be at an address < 16Mb). */

- int result; /* Status code from lower level driver */
+ /* Status code from lower level driver */
+ union {
+ int result; /* do not use in new code. */
+ union scsi_status status;
+ };
int flags; /* Command flags */
unsigned long state; /* Command completion state */

@@ -317,23 +321,23 @@ static inline struct scsi_data_buffer *scsi_prot(struct scsi_cmnd *cmd)
static inline void set_status_byte(struct scsi_cmnd *cmd,
enum sam_status status)
{
- cmd->result = (cmd->result & 0xffffff00) | status;
+ cmd->status.b.status = status;
}

static inline void set_msg_byte(struct scsi_cmnd *cmd, enum msg_byte status)
{
- cmd->result = (cmd->result & 0xffff00ff) | (status << 8);
+ cmd->status.b.msg = status;
}

static inline void set_host_byte(struct scsi_cmnd *cmd, enum host_status status)
{
- cmd->result = (cmd->result & 0xff00ffff) | (status << 16);
+ cmd->status.b.host = status;
}

static inline void set_driver_byte(struct scsi_cmnd *cmd,
enum driver_status status)
{
- cmd->result = (cmd->result & 0x00ffffff) | (status << 24);
+ cmd->status.b.driver = status;
}

static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 468094254b3c..dcd66bb9a1ba 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -6,6 +6,8 @@

#include <scsi/scsi_cmnd.h>
#include <scsi/scsi_common.h>
+#include <scsi/scsi_status.h>
+
struct scsi_device;
struct Scsi_Host;

@@ -31,7 +33,10 @@ extern int scsi_ioctl_reset(struct scsi_device *, int __user *);

struct scsi_eh_save {
/* saved state */
- int result;
+ union {
+ int result; /* do not use in new code */
+ union scsi_status status;
+ };
unsigned int resid_len;
int eh_eflags;
enum dma_data_direction data_direction;
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
index b06f28c74908..83f5549cc74c 100644
--- a/include/scsi/scsi_request.h
+++ b/include/scsi/scsi_request.h
@@ -3,6 +3,7 @@
#define _SCSI_SCSI_REQUEST_H

#include <linux/blk-mq.h>
+#include <scsi/scsi_status.h>

#define BLK_MAX_CDB 16

@@ -10,7 +11,10 @@ struct scsi_request {
unsigned char __cmd[BLK_MAX_CDB];
unsigned char *cmd;
unsigned short cmd_len;
- int result;
+ union {
+ int result; /* do not use in new code */
+ union scsi_status status;
+ };
unsigned int sense_len;
unsigned int resid_len; /* residual count */
int retries;
diff --git a/include/scsi/scsi_status.h b/include/scsi/scsi_status.h
index da2ba825f981..120f5a43d2ed 100644
--- a/include/scsi/scsi_status.h
+++ b/include/scsi/scsi_status.h
@@ -3,6 +3,7 @@

#include <linux/types.h>
#include <linux/compiler_attributes.h>
+#include <asm/byteorder.h>
#include <scsi/scsi_proto.h>

/*
@@ -88,4 +89,32 @@ enum driver_status {
DRIVER_SENSE = 0x08,
} __packed;

+/**
+ * SCSI status passed by LLDs to the midlayer.
+ * @combined: One of the following:
+ * - A (driver, host, msg, status) quadruplet encoded as a 32-bit integer.
+ * - A negative Unix error code.
+ * - In the IDE code, a positive value that represents an error code, an
+ * error counter or a bitfield.
+ * @b: SCSI status code.
+ */
+union scsi_status {
+ int32_t combined;
+ struct {
+#if defined(__BIG_ENDIAN)
+ enum driver_status driver;
+ enum host_status host;
+ enum msg_byte msg;
+ enum sam_status status;
+#elif defined(__LITTLE_ENDIAN)
+ enum sam_status status;
+ enum msg_byte msg;
+ enum host_status host;
+ enum driver_status driver;
+#else
+#error Endianness?
+#endif
+ } b;
+};
+
#endif /* _SCSI_SCSI_STATUS_H */
diff --git a/include/uapi/scsi/scsi_bsg_fc.h b/include/uapi/scsi/scsi_bsg_fc.h
index 3ae65e93235c..419db719fe8e 100644
--- a/include/uapi/scsi/scsi_bsg_fc.h
+++ b/include/uapi/scsi/scsi_bsg_fc.h
@@ -9,6 +9,9 @@
#define SCSI_BSG_FC_H

#include <linux/types.h>
+#ifdef __KERNEL__
+#include <scsi/scsi_status.h>
+#endif

/*
* This file intended to be included by both kernel and user space
@@ -291,7 +294,14 @@ struct fc_bsg_reply {
* msg and status fields. The per-msgcode reply structure
* will contain valid data.
*/
+#ifdef __KERNEL__
+ union {
+ __u32 result; /* do not use in new kernel code */
+ union scsi_status status;
+ };
+#else
__u32 result;
+#endif

/* If there was reply_payload, how much was recevied ? */
__u32 reply_payload_rcv_len;
diff --git a/include/uapi/scsi/scsi_bsg_ufs.h b/include/uapi/scsi/scsi_bsg_ufs.h
index d55f2176dfd4..3dfe5a5a0842 100644
--- a/include/uapi/scsi/scsi_bsg_ufs.h
+++ b/include/uapi/scsi/scsi_bsg_ufs.h
@@ -9,6 +9,10 @@
#define SCSI_BSG_UFS_H

#include <linux/types.h>
+#ifdef __KERNEL__
+#include <scsi/scsi_status.h>
+#endif
+
/*
* This file intended to be included by both kernel and user space
*/
@@ -95,7 +99,14 @@ struct ufs_bsg_reply {
* msg and status fields. The per-msgcode reply structure
* will contain valid data.
*/
+#ifdef __KERNEL__
+ union {
+ __u32 result; /* do not use in new kernel code */
+ union scsi_status status;
+ };
+#else
__u32 result;
+#endif

/* If there was reply_payload, how much was received? */
__u32 reply_payload_rcv_len;