Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 3 Nov 2012 14:23:42 -0400
From:      Ryan Stone <rysto32@gmail.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:  <CAFMmRNxxtnExE5N5B0FsrPAJEtsZFc0Uc%2BuYLNtjCzhqarqdLw@mail.gmail.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
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 <delphij@freebsd.org> 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?CAFMmRNxxtnExE5N5B0FsrPAJEtsZFc0Uc%2BuYLNtjCzhqarqdLw>