From owner-svn-src-all@FreeBSD.ORG Sat Nov 3 18:23:44 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id F3638E16; Sat, 3 Nov 2012 18:23:43 +0000 (UTC) (envelope-from rysto32@gmail.com) Received: from mail-vc0-f182.google.com (mail-vc0-f182.google.com [209.85.220.182]) by mx1.freebsd.org (Postfix) with ESMTP id 68E658FC08; Sat, 3 Nov 2012 18:23:43 +0000 (UTC) Received: by mail-vc0-f182.google.com with SMTP id fw7so6244493vcb.13 for ; Sat, 03 Nov 2012 11:23:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=nerh7rUrJXYQ+b3CUMzk3aVv/lwxzfXHFgj0Wvt79NA=; b=PAra1f8urjKpB71F7Wmapdy//134oP64qwT6bdQ6SmYYSas/Q6r3Xk2BiDAJAjY2rP yTV5Lv7FzmzobPvbHfWkiujWGoYx6GcK8enAk/o9WUboXmEKqYY7kQeO+BHpocNAfESn lrhTJwtKMFaePxGqbmGCBax4+fSjN+PcR4p/omUbqaf9CD+Mui+cOEfap7WsIpOjcMsX lRC5Mu0OrG6ERyGFZknEKZoqVP0nLw78lMrKKMoioqwCP53nq88HCndeUY8WLR/UCpod UYQ5+AW2eAqw9tvg99AQx+DE6VCGOefC+ZHrysLvxM+h8VS+nQkhJC0PkUWK4KKWV8u/ cmww== MIME-Version: 1.0 Received: by 10.220.148.134 with SMTP id p6mr5054858vcv.3.1351967022469; Sat, 03 Nov 2012 11:23:42 -0700 (PDT) Received: by 10.58.207.114 with HTTP; Sat, 3 Nov 2012 11:23:42 -0700 (PDT) In-Reply-To: <201211022207.qA2M7jU2012024@svn.freebsd.org> References: <201211022207.qA2M7jU2012024@svn.freebsd.org> Date: Sat, 3 Nov 2012 14:23:42 -0400 Message-ID: Subject: Re: svn commit: r242497 - head/sys/dev/mfi From: Ryan Stone To: Xin LI Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Nov 2012 18:23:44 -0000 Does this apply to only to JBOD, or are RAID arrays > 2TB also affected? On Fri, Nov 2, 2012 at 6:07 PM, 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 > 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; >