Date: Fri, 13 Sep 2019 14:42:37 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r352285 - in stable/12: sbin/camcontrol sys/cam/scsi Message-ID: <201909131442.x8DEgb84026689@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Fri Sep 13 14:42:37 2019 New Revision: 352285 URL: https://svnweb.freebsd.org/changeset/base/352285 Log: MFC r352011: Supply SAT layer with valid transfer sizes. This is a rework of r344701, that noticed that number of bytes passes to 8 bit sector count field gets truncated. First decision was to not pass anything, since ATA specs define the field as N/A. But it appeared to be a problem for some SAT devices, that require information about data transfer to operate properly. Some additional investigation shown that it is quite a common practice to set unused fields of ATA commands (fortunately ATA specs formally allow it) to supply the information to SAT layer. I have found SAS-SATA interposer that does not allow pass-through without it. As side effect, reduce code duplication by removing ata_do_28bit_cmd() function, replacing it with more universal ata_do_cmd(). Modified: stable/12/sbin/camcontrol/camcontrol.c stable/12/sbin/camcontrol/fwdownload.c stable/12/sys/cam/scsi/scsi_all.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sbin/camcontrol/camcontrol.c ============================================================================== --- stable/12/sbin/camcontrol/camcontrol.c Fri Sep 13 08:14:46 2019 (r352284) +++ stable/12/sbin/camcontrol/camcontrol.c Fri Sep 13 14:42:37 2019 (r352285) @@ -1897,13 +1897,11 @@ ata_cam_send(struct cam_device *device, union ccb *ccb static int ata_do_pass_16(struct cam_device *device, union ccb *ccb, int retries, u_int32_t flags, u_int8_t protocol, u_int8_t ata_flags, - u_int8_t tag_action, u_int8_t command, u_int8_t features, - u_int64_t lba, u_int8_t sector_count, u_int8_t *data_ptr, + u_int8_t tag_action, u_int8_t command, u_int16_t features, + u_int64_t lba, u_int16_t sector_count, u_int8_t *data_ptr, u_int16_t dxfer_len, int timeout, int quiet) { if (data_ptr != NULL) { - ata_flags |= AP_FLAG_BYT_BLOK_BYTES | - AP_FLAG_TLEN_SECT_CNT; if (flags & CAM_DIR_OUT) ata_flags |= AP_FLAG_TDIR_TO_DEV; else @@ -1954,44 +1952,10 @@ ata_try_pass_16(struct cam_device *device) } static int -ata_do_28bit_cmd(struct cam_device *device, union ccb *ccb, int retries, - u_int32_t flags, u_int8_t protocol, u_int8_t tag_action, - u_int8_t command, u_int8_t features, u_int32_t lba, - u_int8_t sector_count, u_int8_t *data_ptr, u_int16_t dxfer_len, - int timeout, int quiet) -{ - - - switch (ata_try_pass_16(device)) { - case -1: - return (1); - case 1: - /* Try using SCSI Passthrough */ - return ata_do_pass_16(device, ccb, retries, flags, protocol, - 0, tag_action, command, features, lba, - sector_count, data_ptr, dxfer_len, - timeout, quiet); - } - - CCB_CLEAR_ALL_EXCEPT_HDR(&ccb->ataio); - cam_fill_ataio(&ccb->ataio, - retries, - NULL, - flags, - tag_action, - data_ptr, - dxfer_len, - timeout); - - ata_28bit_cmd(&ccb->ataio, command, features, lba, sector_count); - return ata_cam_send(device, ccb, quiet); -} - -static int ata_do_cmd(struct cam_device *device, union ccb *ccb, int retries, u_int32_t flags, u_int8_t protocol, u_int8_t ata_flags, - u_int8_t tag_action, u_int8_t command, u_int8_t features, - u_int64_t lba, u_int8_t sector_count, u_int8_t *data_ptr, + u_int8_t tag_action, u_int8_t command, u_int16_t features, + u_int64_t lba, u_int16_t sector_count, u_int8_t *data_ptr, u_int16_t dxfer_len, int timeout, int force48bit) { int retval; @@ -2238,14 +2202,15 @@ atahpa_password(struct cam_device *device, int retry_c retry_count, /*flags*/CAM_DIR_OUT, /*protocol*/protocol, - /*ata_flags*/AP_FLAG_CHK_COND, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT | AP_FLAG_CHK_COND, /*tag_action*/MSG_SIMPLE_Q_TAG, /*command*/cmd, /*features*/ATA_HPA_FEAT_SET_PWD, /*lba*/0, - /*sector_count*/0, + /*sector_count*/sizeof(*pwd) / 512, /*data_ptr*/(u_int8_t*)pwd, - /*dxfer_len*/sizeof(struct ata_set_max_pwd), + /*dxfer_len*/sizeof(*pwd), timeout ? timeout : 1000, is48bit); @@ -2305,14 +2270,15 @@ atahpa_unlock(struct cam_device *device, int retry_cou retry_count, /*flags*/CAM_DIR_OUT, /*protocol*/protocol, - /*ata_flags*/AP_FLAG_CHK_COND, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT | AP_FLAG_CHK_COND, /*tag_action*/MSG_SIMPLE_Q_TAG, /*command*/cmd, /*features*/ATA_HPA_FEAT_UNLOCK, /*lba*/0, - /*sector_count*/0, + /*sector_count*/sizeof(*pwd) / 512, /*data_ptr*/(u_int8_t*)pwd, - /*dxfer_len*/sizeof(struct ata_set_max_pwd), + /*dxfer_len*/sizeof(*pwd), timeout ? timeout : 1000, is48bit); @@ -2482,45 +2448,32 @@ ata_do_identify(struct cam_device *device, int retry_c return (1); } - error = ata_do_28bit_cmd(device, - ccb, - /*retries*/retry_count, - /*flags*/CAM_DIR_IN, - /*protocol*/AP_PROTO_PIO_IN, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/command, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/(u_int8_t *)ptr, - /*dxfer_len*/sizeof(struct ata_params), - /*timeout*/timeout ? timeout : 30 * 1000, - /*quiet*/1); +retry: + error = ata_do_cmd(device, + ccb, + /*retries*/retry_count, + /*flags*/CAM_DIR_IN, + /*protocol*/AP_PROTO_PIO_IN, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/command, + /*features*/0, + /*lba*/0, + /*sector_count*/sizeof(struct ata_params) / 512, + /*data_ptr*/(u_int8_t *)ptr, + /*dxfer_len*/sizeof(struct ata_params), + /*timeout*/timeout ? timeout : 30 * 1000, + /*force48bit*/0); if (error != 0) { - if (retry_command == 0) { - free(ptr); - return (1); + if (retry_command != 0) { + command = retry_command; + retry_command = 0; + goto retry; } - error = ata_do_28bit_cmd(device, - ccb, - /*retries*/retry_count, - /*flags*/CAM_DIR_IN, - /*protocol*/AP_PROTO_PIO_IN, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/retry_command, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/(u_int8_t *)ptr, - /*dxfer_len*/sizeof(struct ata_params), - /*timeout*/timeout ? timeout : 30 * 1000, - /*quiet*/0); - - if (error != 0) { - free(ptr); - return (1); - } + free(ptr); + return (1); } ident_buf = (struct ata_params *)ptr; @@ -2708,20 +2661,21 @@ atasecurity_freeze(struct cam_device *device, union cc if (quiet == 0) atasecurity_notify(ATA_SECURITY_FREEZE_LOCK, NULL); - return ata_do_28bit_cmd(device, - ccb, - retry_count, - /*flags*/CAM_DIR_NONE, - /*protocol*/AP_PROTO_NON_DATA, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/ATA_SECURITY_FREEZE_LOCK, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/NULL, - /*dxfer_len*/0, - /*timeout*/timeout, - /*quiet*/0); + return ata_do_cmd(device, + ccb, + retry_count, + /*flags*/CAM_DIR_NONE, + /*protocol*/AP_PROTO_NON_DATA, + /*ata_flags*/0, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/ATA_SECURITY_FREEZE_LOCK, + /*features*/0, + /*lba*/0, + /*sector_count*/0, + /*data_ptr*/NULL, + /*dxfer_len*/0, + /*timeout*/timeout, + /*force48bit*/0); } static int @@ -2733,20 +2687,22 @@ atasecurity_unlock(struct cam_device *device, union cc if (quiet == 0) atasecurity_notify(ATA_SECURITY_UNLOCK, pwd); - return ata_do_28bit_cmd(device, - ccb, - retry_count, - /*flags*/CAM_DIR_OUT, - /*protocol*/AP_PROTO_PIO_OUT, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/ATA_SECURITY_UNLOCK, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/(u_int8_t *)pwd, - /*dxfer_len*/sizeof(*pwd), - /*timeout*/timeout, - /*quiet*/0); + return ata_do_cmd(device, + ccb, + retry_count, + /*flags*/CAM_DIR_OUT, + /*protocol*/AP_PROTO_PIO_OUT, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/ATA_SECURITY_UNLOCK, + /*features*/0, + /*lba*/0, + /*sector_count*/sizeof(*pwd) / 512, + /*data_ptr*/(u_int8_t *)pwd, + /*dxfer_len*/sizeof(*pwd), + /*timeout*/timeout, + /*force48bit*/0); } static int @@ -2757,20 +2713,22 @@ atasecurity_disable(struct cam_device *device, union c if (quiet == 0) atasecurity_notify(ATA_SECURITY_DISABLE_PASSWORD, pwd); - return ata_do_28bit_cmd(device, - ccb, - retry_count, - /*flags*/CAM_DIR_OUT, - /*protocol*/AP_PROTO_PIO_OUT, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/ATA_SECURITY_DISABLE_PASSWORD, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/(u_int8_t *)pwd, - /*dxfer_len*/sizeof(*pwd), - /*timeout*/timeout, - /*quiet*/0); + return ata_do_cmd(device, + ccb, + retry_count, + /*flags*/CAM_DIR_OUT, + /*protocol*/AP_PROTO_PIO_OUT, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/ATA_SECURITY_DISABLE_PASSWORD, + /*features*/0, + /*lba*/0, + /*sector_count*/sizeof(*pwd) / 512, + /*data_ptr*/(u_int8_t *)pwd, + /*dxfer_len*/sizeof(*pwd), + /*timeout*/timeout, + /*force48bit*/0); } @@ -2816,20 +2774,21 @@ atasecurity_erase(struct cam_device *device, union ccb if (quiet == 0) atasecurity_notify(ATA_SECURITY_ERASE_PREPARE, NULL); - error = ata_do_28bit_cmd(device, - ccb, - retry_count, - /*flags*/CAM_DIR_NONE, - /*protocol*/AP_PROTO_NON_DATA, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/ATA_SECURITY_ERASE_PREPARE, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/NULL, - /*dxfer_len*/0, - /*timeout*/timeout, - /*quiet*/0); + error = ata_do_cmd(device, + ccb, + retry_count, + /*flags*/CAM_DIR_NONE, + /*protocol*/AP_PROTO_NON_DATA, + /*ata_flags*/0, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/ATA_SECURITY_ERASE_PREPARE, + /*features*/0, + /*lba*/0, + /*sector_count*/0, + /*data_ptr*/NULL, + /*dxfer_len*/0, + /*timeout*/timeout, + /*force48bit*/0); if (error != 0) return error; @@ -2837,20 +2796,22 @@ atasecurity_erase(struct cam_device *device, union ccb if (quiet == 0) atasecurity_notify(ATA_SECURITY_ERASE_UNIT, pwd); - error = ata_do_28bit_cmd(device, - ccb, - retry_count, - /*flags*/CAM_DIR_OUT, - /*protocol*/AP_PROTO_PIO_OUT, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/ATA_SECURITY_ERASE_UNIT, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/(u_int8_t *)pwd, - /*dxfer_len*/sizeof(*pwd), - /*timeout*/erase_timeout, - /*quiet*/0); + error = ata_do_cmd(device, + ccb, + retry_count, + /*flags*/CAM_DIR_OUT, + /*protocol*/AP_PROTO_PIO_OUT, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/ATA_SECURITY_ERASE_UNIT, + /*features*/0, + /*lba*/0, + /*sector_count*/sizeof(*pwd) / 512, + /*data_ptr*/(u_int8_t *)pwd, + /*dxfer_len*/sizeof(*pwd), + /*timeout*/erase_timeout, + /*force48bit*/0); if (error == 0 && quiet == 0) printf("\nErase Complete\n"); @@ -2867,20 +2828,22 @@ atasecurity_set_password(struct cam_device *device, un if (quiet == 0) atasecurity_notify(ATA_SECURITY_SET_PASSWORD, pwd); - return ata_do_28bit_cmd(device, - ccb, - retry_count, - /*flags*/CAM_DIR_OUT, - /*protocol*/AP_PROTO_PIO_OUT, - /*tag_action*/MSG_SIMPLE_Q_TAG, - /*command*/ATA_SECURITY_SET_PASSWORD, - /*features*/0, - /*lba*/0, - /*sector_count*/0, - /*data_ptr*/(u_int8_t *)pwd, - /*dxfer_len*/sizeof(*pwd), - /*timeout*/timeout, - /*quiet*/0); + return ata_do_cmd(device, + ccb, + retry_count, + /*flags*/CAM_DIR_OUT, + /*protocol*/AP_PROTO_PIO_OUT, + /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS | + AP_FLAG_TLEN_SECT_CNT, + /*tag_action*/MSG_SIMPLE_Q_TAG, + /*command*/ATA_SECURITY_SET_PASSWORD, + /*features*/0, + /*lba*/0, + /*sector_count*/sizeof(*pwd) / 512, + /*data_ptr*/(u_int8_t *)pwd, + /*dxfer_len*/sizeof(*pwd), + /*timeout*/timeout, + /*force48bit*/0); } static void @@ -9481,7 +9444,7 @@ atapm(struct cam_device *device, int argc, char **argv /*data_ptr*/NULL, /*dxfer_len*/0, /*timeout*/timeout ? timeout : 30 * 1000, - /*quiet*/1); + /*force48bit*/0); cam_freeccb(ccb); @@ -9534,11 +9497,12 @@ ataaxm(struct cam_device *device, int argc, char **arg } } - retval = ata_do_28bit_cmd(device, + retval = ata_do_cmd(device, ccb, /*retries*/retry_count, /*flags*/CAM_DIR_NONE, /*protocol*/AP_PROTO_NON_DATA, + /*ata_flags*/0, /*tag_action*/MSG_SIMPLE_Q_TAG, /*command*/ATA_SETFEATURES, /*features*/cmd, @@ -9547,7 +9511,7 @@ ataaxm(struct cam_device *device, int argc, char **arg /*data_ptr*/NULL, /*dxfer_len*/0, /*timeout*/timeout ? timeout : 30 * 1000, - /*quiet*/1); + /*force48bit*/0); cam_freeccb(ccb); return (retval); Modified: stable/12/sbin/camcontrol/fwdownload.c ============================================================================== --- stable/12/sbin/camcontrol/fwdownload.c Fri Sep 13 08:14:46 2019 (r352284) +++ stable/12/sbin/camcontrol/fwdownload.c Fri Sep 13 14:42:37 2019 (r352285) @@ -701,11 +701,11 @@ fw_check_device_ready(struct cam_device *dev, camcontr /*flags*/ CAM_DIR_IN, /*tag_action*/ MSG_SIMPLE_Q_TAG, /*protocol*/ AP_PROTO_PIO_IN, - /*ata_flags*/ AP_FLAG_BYT_BLOK_BYTES | + /*ata_flags*/ AP_FLAG_BYT_BLOK_BLOCKS | AP_FLAG_TLEN_SECT_CNT | AP_FLAG_TDIR_FROM_DEV, /*features*/ 0, - /*sector_count*/ (uint8_t) dxfer_len, + /*sector_count*/ dxfer_len / 512, /*lba*/ 0, /*command*/ ATA_ATA_IDENTIFY, /*auxiliary*/ 0, Modified: stable/12/sys/cam/scsi/scsi_all.c ============================================================================== --- stable/12/sys/cam/scsi/scsi_all.c Fri Sep 13 08:14:46 2019 (r352284) +++ stable/12/sys/cam/scsi/scsi_all.c Fri Sep 13 14:42:37 2019 (r352285) @@ -8281,10 +8281,10 @@ scsi_ata_identify(struct ccb_scsiio *csio, u_int32_t r tag_action, /*protocol*/AP_PROTO_PIO_IN, /*ata_flags*/AP_FLAG_TDIR_FROM_DEV | - AP_FLAG_BYT_BLOK_BYTES | + AP_FLAG_BYT_BLOK_BLOCKS | AP_FLAG_TLEN_SECT_CNT, /*features*/0, - /*sector_count*/dxfer_len, + /*sector_count*/dxfer_len / 512, /*lba*/0, /*command*/ATA_ATA_IDENTIFY, /*device*/ 0,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201909131442.x8DEgb84026689>