Date: Sat, 3 Nov 2012 11:11:44 -0700 From: Doug Ambrisko <ambrisko@ambrisko.com> To: Xin LI <delphij@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r242497 - head/sys/dev/mfi Message-ID: <20121103181144.GA60979@ambrisko.com> In-Reply-To: <201211022207.qA2M7jU2012024@svn.freebsd.org> References: <201211022207.qA2M7jU2012024@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
FWIW, I also have a fix for this and use the same function for mfi_tbolt.c. It also has a bunch of other fixes. However, I need to merge up to this code. It could use some testing on various cards etc. Thanks, Doug A. On Fri, Nov 02, 2012 at 10:07:45PM +0000, Xin LI wrote: | Author: delphij | Date: Fri Nov 2 22:07:45 2012 | New Revision: 242497 | URL: http://svn.freebsd.org/changeset/base/242497 | | Log: | Copy code from scsi_read_write() as mfi_build_syspd_cdb() to build SCSI | command properly. Without this change, mfi(4) always sends 10 byte READ | and WRITE commands, which will cause data corruption when device is | larger than 2^32 sectors. | | PR: kern/173291 | Submitted by: Steven Hartland <steven.hartland multiplay.co.uk> | Reviewed by: mav | MFC after: 2 weeks | | Modified: | head/sys/dev/mfi/mfi.c | | Modified: head/sys/dev/mfi/mfi.c | ============================================================================== | --- head/sys/dev/mfi/mfi.c Fri Nov 2 22:03:39 2012 (r242496) | +++ head/sys/dev/mfi/mfi.c Fri Nov 2 22:07:45 2012 (r242497) | @@ -106,6 +106,8 @@ static void mfi_add_sys_pd_complete(stru | static struct mfi_command * mfi_bio_command(struct mfi_softc *); | static void mfi_bio_complete(struct mfi_command *); | static struct mfi_command *mfi_build_ldio(struct mfi_softc *,struct bio*); | +static int mfi_build_syspd_cdb(struct mfi_pass_frame *pass, uint32_t block_count, | + uint64_t lba, uint8_t byte2, int readop); | static struct mfi_command *mfi_build_syspdio(struct mfi_softc *,struct bio*); | static int mfi_send_frame(struct mfi_softc *, struct mfi_command *); | static int mfi_abort(struct mfi_softc *, struct mfi_command *); | @@ -1982,13 +1984,78 @@ mfi_bio_command(struct mfi_softc *sc) | mfi_enqueue_bio(sc, bio); | return cm; | } | + | +static int | +mfi_build_syspd_cdb(struct mfi_pass_frame *pass, uint32_t block_count, | + uint64_t lba, uint8_t byte2, int readop) | +{ | + int cdb_len; | + | + if (((lba & 0x1fffff) == lba) | + && ((block_count & 0xff) == block_count) | + && (byte2 == 0)) { | + /* We can fit in a 6 byte cdb */ | + struct scsi_rw_6 *scsi_cmd; | + | + scsi_cmd = (struct scsi_rw_6 *)&pass->cdb; | + scsi_cmd->opcode = readop ? READ_6 : WRITE_6; | + scsi_ulto3b(lba, scsi_cmd->addr); | + scsi_cmd->length = block_count & 0xff; | + scsi_cmd->control = 0; | + cdb_len = sizeof(*scsi_cmd); | + } else if (((block_count & 0xffff) == block_count) && ((lba & 0xffffffff) == lba)) { | + /* Need a 10 byte CDB */ | + struct scsi_rw_10 *scsi_cmd; | + | + scsi_cmd = (struct scsi_rw_10 *)&pass->cdb; | + scsi_cmd->opcode = readop ? READ_10 : WRITE_10; | + scsi_cmd->byte2 = byte2; | + scsi_ulto4b(lba, scsi_cmd->addr); | + scsi_cmd->reserved = 0; | + scsi_ulto2b(block_count, scsi_cmd->length); | + scsi_cmd->control = 0; | + cdb_len = sizeof(*scsi_cmd); | + } else if (((block_count & 0xffffffff) == block_count) && | + ((lba & 0xffffffff) == lba)) { | + /* Block count is too big for 10 byte CDB use a 12 byte CDB */ | + struct scsi_rw_12 *scsi_cmd; | + | + scsi_cmd = (struct scsi_rw_12 *)&pass->cdb; | + scsi_cmd->opcode = readop ? READ_12 : WRITE_12; | + scsi_cmd->byte2 = byte2; | + scsi_ulto4b(lba, scsi_cmd->addr); | + scsi_cmd->reserved = 0; | + scsi_ulto4b(block_count, scsi_cmd->length); | + scsi_cmd->control = 0; | + cdb_len = sizeof(*scsi_cmd); | + } else { | + /* | + * 16 byte CDB. We'll only get here if the LBA is larger | + * than 2^32 | + */ | + struct scsi_rw_16 *scsi_cmd; | + | + scsi_cmd = (struct scsi_rw_16 *)&pass->cdb; | + scsi_cmd->opcode = readop ? READ_16 : WRITE_16; | + scsi_cmd->byte2 = byte2; | + scsi_u64to8b(lba, scsi_cmd->addr); | + scsi_cmd->reserved = 0; | + scsi_ulto4b(block_count, scsi_cmd->length); | + scsi_cmd->control = 0; | + cdb_len = sizeof(*scsi_cmd); | + } | + | + return cdb_len; | +} | + | static struct mfi_command * | mfi_build_syspdio(struct mfi_softc *sc, struct bio *bio) | { | struct mfi_command *cm; | struct mfi_pass_frame *pass; | - int flags = 0, blkcount = 0; | - uint32_t context = 0; | + int flags = 0; | + uint8_t cdb_len; | + uint32_t block_count, context = 0; | | if ((cm = mfi_dequeue_free(sc)) == NULL) | return (NULL); | @@ -2002,35 +2069,29 @@ mfi_build_syspdio(struct mfi_softc *sc, | pass->header.cmd = MFI_CMD_PD_SCSI_IO; | switch (bio->bio_cmd & 0x03) { | case BIO_READ: | -#define SCSI_READ 0x28 | - pass->cdb[0] = SCSI_READ; | flags = MFI_CMD_DATAIN; | break; | case BIO_WRITE: | -#define SCSI_WRITE 0x2a | - pass->cdb[0] = SCSI_WRITE; | flags = MFI_CMD_DATAOUT; | break; | default: | - panic("Invalid bio command"); | + /* TODO: what about BIO_DELETE??? */ | + panic("Unsupported bio command"); | } | | /* Cheat with the sector length to avoid a non-constant division */ | - blkcount = (bio->bio_bcount + MFI_SECTOR_LEN - 1) / MFI_SECTOR_LEN; | + block_count = (bio->bio_bcount + MFI_SECTOR_LEN - 1) / MFI_SECTOR_LEN; | /* Fill the LBA and Transfer length in CDB */ | - pass->cdb[2] = (bio->bio_pblkno & 0xff000000) >> 24; | - pass->cdb[3] = (bio->bio_pblkno & 0x00ff0000) >> 16; | - pass->cdb[4] = (bio->bio_pblkno & 0x0000ff00) >> 8; | - pass->cdb[5] = bio->bio_pblkno & 0x000000ff; | - pass->cdb[7] = (blkcount & 0xff00) >> 8; | - pass->cdb[8] = (blkcount & 0x00ff); | + cdb_len = mfi_build_syspd_cdb(pass, block_count, bio->bio_pblkno, 0, | + flags == MFI_CMD_DATAIN); | + | pass->header.target_id = (uintptr_t)bio->bio_driver1; | pass->header.timeout = 0; | pass->header.flags = 0; | pass->header.scsi_status = 0; | pass->header.sense_len = MFI_SENSE_LEN; | pass->header.data_len = bio->bio_bcount; | - pass->header.cdb_len = 10; | + pass->header.cdb_len = cdb_len; | pass->sense_addr_lo = (uint32_t)cm->cm_sense_busaddr; | pass->sense_addr_hi = (uint32_t)((uint64_t)cm->cm_sense_busaddr >> 32); | cm->cm_complete = mfi_bio_complete; | @@ -2048,7 +2109,8 @@ mfi_build_ldio(struct mfi_softc *sc, str | { | struct mfi_io_frame *io; | struct mfi_command *cm; | - int flags, blkcount; | + int flags; | + uint32_t blkcount; | uint32_t context = 0; | | if ((cm = mfi_dequeue_free(sc)) == NULL) | @@ -2069,7 +2131,8 @@ mfi_build_ldio(struct mfi_softc *sc, str | flags = MFI_CMD_DATAOUT; | break; | default: | - panic("Invalid bio command"); | + /* TODO: what about BIO_DELETE??? */ | + panic("Unsupported bio command"); | } | | /* Cheat with the sector length to avoid a non-constant division */ | @@ -2460,7 +2523,7 @@ mfi_dump_syspd_blocks(struct mfi_softc * | struct mfi_command *cm; | struct mfi_pass_frame *pass; | int error; | - int blkcount = 0; | + uint32_t blkcount; | | if ((cm = mfi_dequeue_free(sc)) == NULL) | return (EBUSY); | @@ -2468,21 +2531,14 @@ mfi_dump_syspd_blocks(struct mfi_softc * | pass = &cm->cm_frame->pass; | bzero(pass->cdb, 16); | pass->header.cmd = MFI_CMD_PD_SCSI_IO; | - pass->cdb[0] = SCSI_WRITE; | - pass->cdb[2] = (lba & 0xff000000) >> 24; | - pass->cdb[3] = (lba & 0x00ff0000) >> 16; | - pass->cdb[4] = (lba & 0x0000ff00) >> 8; | - pass->cdb[5] = (lba & 0x000000ff); | blkcount = (len + MFI_SECTOR_LEN - 1) / MFI_SECTOR_LEN; | - pass->cdb[7] = (blkcount & 0xff00) >> 8; | - pass->cdb[8] = (blkcount & 0x00ff); | pass->header.target_id = id; | pass->header.timeout = 0; | pass->header.flags = 0; | pass->header.scsi_status = 0; | pass->header.sense_len = MFI_SENSE_LEN; | pass->header.data_len = len; | - pass->header.cdb_len = 10; | + pass->header.cdb_len = mfi_build_syspd_cdb(pass, blkcount, lba, 0, 0); | pass->sense_addr_lo = (uint32_t)cm->cm_sense_busaddr; | pass->sense_addr_hi = (uint32_t)((uint64_t)cm->cm_sense_busaddr >> 32); | cm->cm_data = virt;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121103181144.GA60979>