On Wed, 2008-10-29 at 15:29 -0700, Harvey Harrison wrote:
> Did you ever have some time to think about my suggestion regarding
> defining some common packed structs for the different command formats
> taht could be endian-annotated, then use the unaligned helpers against
> pointers to these structs...so sparse would actually spot endian
> mixups in scsi thereafter?
Here's a small patch to illustrate the kind of thing I'm suggesting in
a less hand-wavy way.
diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index afe1de9..451f097 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -40,6 +40,7 @@
#include <linux/compat.h>
#include <linux/blkdev.h>
#include <linux/mutex.h>
+#include <asm/unaligned.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -753,57 +754,41 @@ megasas_build_ldio(struct megasas_instance *instance, struct scsi_cmnd *scp,
ldio->start_lba_hi = 0;
ldio->access_byte = (scp->cmd_len != 6) ? scp->cmnd[1] : 0;
- /*
- * 6-byte READ(0x08) or WRITE(0x0A) cdb
- */
if (scp->cmd_len == 6) {
+ /*
+ * 6-byte READ(0x08) or WRITE(0x0A) cdb
+ */
ldio->lba_count = (u32) scp->cmnd[4];
ldio->start_lba_lo = ((u32) scp->cmnd[1] << 16) |
((u32) scp->cmnd[2] << 8) | (u32) scp->cmnd[3];
ldio->start_lba_lo &= 0x1FFFFF;
- }
-
- /*
- * 10-byte READ(0x28) or WRITE(0x2A) cdb
- */
- else if (scp->cmd_len == 10) {
- ldio->lba_count = (u32) scp->cmnd[8] |
- ((u32) scp->cmnd[7] << 8);
- ldio->start_lba_lo = ((u32) scp->cmnd[2] << 24) |
- ((u32) scp->cmnd[3] << 16) |
- ((u32) scp->cmnd[4] << 8) | (u32) scp->cmnd[5];
- }
-
- /*
- * 12-byte READ(0xA8) or WRITE(0xAA) cdb
- */
- else if (scp->cmd_len == 12) {
- ldio->lba_count = ((u32) scp->cmnd[6] << 24) |
- ((u32) scp->cmnd[7] << 16) |
- ((u32) scp->cmnd[8] << 8) | (u32) scp->cmnd[9];
-
- ldio->start_lba_lo = ((u32) scp->cmnd[2] << 24) |
- ((u32) scp->cmnd[3] << 16) |
- ((u32) scp->cmnd[4] << 8) | (u32) scp->cmnd[5];
- }
-
- /*
- * 16-byte READ(0x88) or WRITE(0x8A) cdb
- */
- else if (scp->cmd_len == 16) {
- ldio->lba_count = ((u32) scp->cmnd[10] << 24) |
- ((u32) scp->cmnd[11] << 16) |
- ((u32) scp->cmnd[12] << 8) | (u32) scp->cmnd[13];
+ } else if (scp->cmd_len == 10) {
+ /*
+ * 10-byte READ(0x28) or WRITE(0x2A) cdb
+ */
+ struct scsi_read10 *cmd = (struct scsi_read10 *)scp->cmnd;
- ldio->start_lba_lo = ((u32) scp->cmnd[6] << 24) |
- ((u32) scp->cmnd[7] << 16) |
- ((u32) scp->cmnd[8] << 8) | (u32) scp->cmnd[9];
+ ldio->lba_count = get_unaligned_be16(&cmd->length);
+ ldio->start_lba_lo = get_unaligned_be32(&cmd->lba);
+ } else if (scp->cmd_len == 12) {
+ /*
+ * 12-byte READ(0xA8) or WRITE(0xAA) cdb
+ */
+ struct scsi_read12 *cmd = (struct scsi_read12 *)scp->cmnd;
- ldio->start_lba_hi = ((u32) scp->cmnd[2] << 24) |
- ((u32) scp->cmnd[3] << 16) |
- ((u32) scp->cmnd[4] << 8) | (u32) scp->cmnd[5];
+ ldio->lba_count = get_unaligned_be32(&cmd->length);
+ ldio->start_lba_lo = get_unaligned_be32(&cmd->lba);
+ } else if (scp->cmd_len == 16) {
+ /*
+ * 16-byte READ(0x88) or WRITE(0x8A) cdb
+ */
+ struct scsi_read16 *cmd = (struct scsi_read16 *)scp->cmnd;
+ u64 lba = get_unaligned_be64(&cmd->lba);
+ ldio->lba_count = get_unaligned_be32(&cmd->length);
+ ldio->start_lba_lo = (u32)lba;
+ ldio->start_lba_hi = (u32)(lba >> 32);
}
/*
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 855bf95..184d83d 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -131,6 +131,42 @@ struct scsi_cmnd {
unsigned char tag; /* SCSI-II queued command tag */
};
+/*
+ * Opcode 0x28
+ */
+struct scsi_read10 {
+ u8 op;
+ u8 flags;
+ __be32 lba;
+ u8 pad; /* reserved */
+ __be16 length;
+ u8 control;
+} __packed;
+
+/*
+ * Opcode 0xa8
+ */
+struct scsi_read12 {
+ u8 op;
+ u8 flags;
+ __be32 lba;
+ __be32 length;
+ u8 pad; /* reserved */
+ u8 control;
+} __packed;
+
+/*
+ * Opcode 0x88
+ */
+struct scsi_read16 {
+ u8 op;
+ u8 flags;
+ __be64 lba;
+ __be32 length;
+ u8 pad; /* MMC4, reserved, group number */
+ u8 control;
+} __packed;
+
extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t);
extern struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *, gfp_t);
extern void scsi_put_command(struct scsi_cmnd *);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
