Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 9 Sep 2019 17:36:29 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r352082 - head/sbin/camcontrol
Message-ID:  <201909091736.x89HaTrf083664@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Mon Sep  9 17:36:29 2019
New Revision: 352082
URL: https://svnweb.freebsd.org/changeset/base/352082

Log:
  Fix number of problems found while testing on SAT devices.
  
   - Remove incomplete and dangerous ata_res decoding from ata_do_cmd().
  Instead switch all functions that need the result to use get_ata_status(),
  doing the same, but more careful, also reducing code duplication.
   - Made get_ata_status() to also decode fixed format sense.  In many cases
  it is still not enough to make it useful, since it can only report results
  of 28-bit command, but it is slightly better then nothing.
   - Organize error reporting in ata_do_cmd(), so that if caller specified
  AP_FLAG_CHK_COND, it is responsible for command errors (non-ioctl ones).
   - Make HPA/AMA errors not fatal for `identify` subcommand.
   - Fix reprobe() not being called on HPA/AMA when in quiet mode.
   - Remove not very useful messages from `format` and `sanitize` commands
  with -y flag.  Once they started, they often can't be stopped any way.
  
  MFC after:	5 days
  Sponsored by:	iXsystems, Inc.

Modified:
  head/sbin/camcontrol/camcontrol.c

Modified: head/sbin/camcontrol/camcontrol.c
==============================================================================
--- head/sbin/camcontrol/camcontrol.c	Mon Sep  9 17:34:18 2019	(r352081)
+++ head/sbin/camcontrol/camcontrol.c	Mon Sep  9 17:36:29 2019	(r352082)
@@ -1751,7 +1751,7 @@ atacapprint(struct ata_params *parm)
 }
 
 static int
-scsi_cam_pass_16_send(struct cam_device *device, union ccb *ccb, int quiet)
+scsi_cam_pass_16_send(struct cam_device *device, union ccb *ccb)
 {
 	struct ata_pass_16 *ata_pass_16;
 	struct ata_cmd ata_cmd;
@@ -1774,24 +1774,21 @@ scsi_cam_pass_16_send(struct cam_device *device, union
 		ccb->ccb_h.flags |= CAM_PASS_ERR_RECOVER;
 
 	if (cam_send_ccb(device, ccb) < 0) {
-		if (quiet != 1 || arglist & CAM_ARG_VERBOSE) {
-			warn("error sending ATA %s via pass_16",
-			     ata_op_string(&ata_cmd));
-		}
+		warn("error sending ATA %s via pass_16", ata_op_string(&ata_cmd));
 		return (1);
 	}
 
+	/*
+	 * Consider any non-CAM_REQ_CMP status as error and report it here,
+	 * unless caller set AP_FLAG_CHK_COND, in which case it is reponsible.
+	 */
 	if (!(ata_pass_16->flags & AP_FLAG_CHK_COND) &&
 	    (ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
-		if (quiet != 1 || arglist & CAM_ARG_VERBOSE) {
-			warnx("ATA %s via pass_16 failed",
-			      ata_op_string(&ata_cmd));
-		}
+		warnx("ATA %s via pass_16 failed", ata_op_string(&ata_cmd));
 		if (arglist & CAM_ARG_VERBOSE) {
 			cam_error_print(device, ccb, CAM_ESF_ALL,
 					CAM_EPF_ALL, stderr);
 		}
-
 		return (1);
 	}
 
@@ -1800,7 +1797,7 @@ scsi_cam_pass_16_send(struct cam_device *device, union
 
 
 static int
-ata_cam_send(struct cam_device *device, union ccb *ccb, int quiet)
+ata_cam_send(struct cam_device *device, union ccb *ccb)
 {
 	if (arglist & CAM_ARG_VERBOSE) {
 		warnx("sending ATA %s with timeout of %u msecs",
@@ -1815,24 +1812,21 @@ ata_cam_send(struct cam_device *device, union ccb *ccb
 		ccb->ccb_h.flags |= CAM_PASS_ERR_RECOVER;
 
 	if (cam_send_ccb(device, ccb) < 0) {
-		if (quiet != 1 || arglist & CAM_ARG_VERBOSE) {
-			warn("error sending ATA %s",
-			     ata_op_string(&(ccb->ataio.cmd)));
-		}
+		warn("error sending ATA %s", ata_op_string(&(ccb->ataio.cmd)));
 		return (1);
 	}
 
-	if ((ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
-		if (quiet != 1 || arglist & CAM_ARG_VERBOSE) {
-			warnx("ATA %s failed: %d",
-			      ata_op_string(&(ccb->ataio.cmd)), quiet);
-		}
-
+	/*
+	 * Consider any non-CAM_REQ_CMP status as error and report it here,
+	 * unless caller set AP_FLAG_CHK_COND, in which case it is reponsible.
+	 */
+	if (!(ccb->ataio.cmd.flags & CAM_ATAIO_NEEDRESULT) &&
+	    (ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
+		warnx("ATA %s failed", ata_op_string(&(ccb->ataio.cmd)));
 		if (arglist & CAM_ARG_VERBOSE) {
 			cam_error_print(device, ccb, CAM_ESF_ALL,
 					CAM_EPF_ALL, stderr);
 		}
-
 		return (1);
 	}
 
@@ -1844,7 +1838,7 @@ ata_do_pass_16(struct cam_device *device, union ccb *c
 	       u_int32_t flags, u_int8_t protocol, u_int8_t ata_flags,
 	       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)
+	       u_int16_t dxfer_len, int timeout)
 {
 	if (data_ptr != NULL) {
 		if (flags & CAM_DIR_OUT)
@@ -1874,7 +1868,7 @@ ata_do_pass_16(struct cam_device *device, union ccb *c
 			 /*sense_len*/SSD_FULL_SIZE,
 			 timeout);
 
-	return scsi_cam_pass_16_send(device, ccb, quiet);
+	return scsi_cam_pass_16_send(device, ccb);
 }
 
 static int
@@ -1910,50 +1904,10 @@ ata_do_cmd(struct cam_device *device, union ccb *ccb, 
 		return (1);
 
 	if (retval == 1) {
-		int error;
-
-		/* Try using SCSI Passthrough */
-		error = ata_do_pass_16(device, ccb, retries, flags, protocol,
+		return (ata_do_pass_16(device, ccb, retries, flags, protocol,
 				      ata_flags, tag_action, command, features,
 				      lba, sector_count, data_ptr, dxfer_len,
-				      timeout, 0);
-
-		if (ata_flags & AP_FLAG_CHK_COND) {
-			/* Decode ata_res from sense data */
-			struct ata_res_pass16 *res_pass16;
-			struct ata_res *res;
-			u_int i;
-			u_int16_t *ptr;
-
-			/* sense_data is 4 byte aligned */
-			ptr = (uint16_t*)(uintptr_t)&ccb->csio.sense_data;
-			for (i = 0; i < sizeof(*res_pass16) / 2; i++)
-				ptr[i] = le16toh(ptr[i]);
-
-			/* sense_data is 4 byte aligned */
-			res_pass16 = (struct ata_res_pass16 *)(uintptr_t)
-			    &ccb->csio.sense_data;
-			res = &ccb->ataio.res;
-			res->flags = res_pass16->flags;
-			res->status = res_pass16->status;
-			res->error = res_pass16->error;
-			res->lba_low = res_pass16->lba_low;
-			res->lba_mid = res_pass16->lba_mid;
-			res->lba_high = res_pass16->lba_high;
-			res->device = res_pass16->device;
-			res->lba_low_exp = res_pass16->lba_low_exp;
-			res->lba_mid_exp = res_pass16->lba_mid_exp;
-			res->lba_high_exp = res_pass16->lba_high_exp;
-			res->sector_count = res_pass16->sector_count;
-			res->sector_count_exp = res_pass16->sector_count_exp;
-			ccb->ccb_h.status &= ~CAM_STATUS_MASK;
-			if (res->status & ATA_STATUS_ERROR)
-				ccb->ccb_h.status |= CAM_ATA_STATUS_ERROR;
-			else
-				ccb->ccb_h.status |= CAM_REQ_CMP;
-		}
-
-		return (error);
+				      timeout));
 	}
 
 	CCB_CLEAR_ALL_EXCEPT_HDR(&ccb->ataio);
@@ -1974,7 +1928,7 @@ ata_do_cmd(struct cam_device *device, union ccb *ccb, 
 	if (ata_flags & AP_FLAG_CHK_COND)
 		ccb->ataio.cmd.flags |= CAM_ATAIO_NEEDRESULT;
 
-	return ata_cam_send(device, ccb, 0);
+	return ata_cam_send(device, ccb);
 }
 
 static void
@@ -1994,23 +1948,31 @@ dump_data(uint16_t *ptr, uint32_t len)
 }
 
 static int
-atahpa_proc_resp(struct cam_device *device, union ccb *ccb,
-		 int is48bit, u_int64_t *hpasize)
+atahpa_proc_resp(struct cam_device *device, union ccb *ccb, u_int64_t *hpasize)
 {
-	struct ata_res *res;
+	uint8_t error = 0, ata_device = 0, status = 0;
+	uint16_t count = 0;
+	uint64_t lba = 0;
+	int retval;
 
-	res = &ccb->ataio.res;
-	if (res->status & ATA_STATUS_ERROR) {
+	retval = get_ata_status(device, ccb, &error, &count, &lba, &ata_device,
+	    &status);
+	if (retval == 1) {
 		if (arglist & CAM_ARG_VERBOSE) {
 			cam_error_print(device, ccb, CAM_ESF_ALL,
 					CAM_EPF_ALL, stderr);
-			printf("error = 0x%02x, sector_count = 0x%04x, "
-			       "device = 0x%02x, status = 0x%02x\n",
-			       res->error, res->sector_count,
-			       res->device, res->status);
 		}
+		warnx("Can't get ATA command status");
+		return (retval);
+	}
 
-		if (res->error & ATA_ERROR_ID_NOT_FOUND) {
+	if (status & ATA_STATUS_ERROR) {
+		if (arglist & CAM_ARG_VERBOSE) {
+			cam_error_print(device, ccb, CAM_ESF_ALL,
+					CAM_EPF_ALL, stderr);
+		}
+
+		if (error & ATA_ERROR_ID_NOT_FOUND) {
 			warnx("Max address has already been set since "
 			      "last power-on or hardware reset");
 		}
@@ -2018,28 +1980,10 @@ atahpa_proc_resp(struct cam_device *device, union ccb 
 		return (1);
 	}
 
-	if (arglist & CAM_ARG_VERBOSE) {
-		fprintf(stdout, "%s%d: Raw native max data:\n",
-			device->device_name, device->dev_unit_num);
-		/* res is 4 byte aligned */
-		dump_data((uint16_t*)(uintptr_t)res, sizeof(struct ata_res));
-
-		printf("error = 0x%02x, sector_count = 0x%04x, device = 0x%02x, "
-		       "status = 0x%02x\n", res->error, res->sector_count,
-		       res->device, res->status);
-	}
-
 	if (hpasize != NULL) {
-		if (is48bit) {
-			*hpasize = (((u_int64_t)((res->lba_high_exp << 16) |
-			    (res->lba_mid_exp << 8) | res->lba_low_exp) << 24) |
-			    ((res->lba_high << 16) | (res->lba_mid << 8) |
-			    res->lba_low)) + 1;
-		} else {
-			*hpasize = (((res->device & 0x0f) << 24) |
-			    (res->lba_high << 16) | (res->lba_mid << 8) |
-			    res->lba_low) + 1;
-		}
+		if (retval == 2 || retval == 6)
+			return (1);
+		*hpasize = lba;
 	}
 
 	return (0);
@@ -2083,7 +2027,7 @@ ata_read_native_max(struct cam_device *device, int ret
 	if (error)
 		return (error);
 
-	return atahpa_proc_resp(device, ccb, is48bit, hpasize);
+	return atahpa_proc_resp(device, ccb, hpasize);
 }
 
 static int
@@ -2127,7 +2071,7 @@ atahpa_set_max(struct cam_device *device, int retry_co
 	if (error)
 		return (error);
 
-	return atahpa_proc_resp(device, ccb, is48bit, NULL);
+	return atahpa_proc_resp(device, ccb, NULL);
 }
 
 static int
@@ -2135,20 +2079,19 @@ atahpa_password(struct cam_device *device, int retry_c
 		u_int32_t timeout, union ccb *ccb,
 		int is48bit, struct ata_set_max_pwd *pwd)
 {
-	int error;
 	u_int cmd;
 	u_int8_t protocol;
 
 	protocol = AP_PROTO_PIO_OUT;
 	cmd = (is48bit) ? ATA_SET_MAX_ADDRESS48 : ATA_SET_MAX_ADDRESS;
 
-	error = ata_do_cmd(device,
+	return (ata_do_cmd(device,
 			   ccb,
 			   retry_count,
 			   /*flags*/CAM_DIR_OUT,
 			   /*protocol*/protocol,
 			   /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS |
-			    AP_FLAG_TLEN_SECT_CNT | AP_FLAG_CHK_COND,
+			    AP_FLAG_TLEN_SECT_CNT,
 			   /*tag_action*/MSG_SIMPLE_Q_TAG,
 			   /*command*/cmd,
 			   /*features*/ATA_HPA_FEAT_SET_PWD,
@@ -2157,31 +2100,25 @@ atahpa_password(struct cam_device *device, int retry_c
 			   /*data_ptr*/(u_int8_t*)pwd,
 			   /*dxfer_len*/sizeof(*pwd),
 			   timeout ? timeout : 1000,
-			   is48bit);
-
-	if (error)
-		return (error);
-
-	return atahpa_proc_resp(device, ccb, is48bit, NULL);
+			   is48bit));
 }
 
 static int
 atahpa_lock(struct cam_device *device, int retry_count,
 	    u_int32_t timeout, union ccb *ccb, int is48bit)
 {
-	int error;
 	u_int cmd;
 	u_int8_t protocol;
 
 	protocol = AP_PROTO_NON_DATA;
 	cmd = (is48bit) ? ATA_SET_MAX_ADDRESS48 : ATA_SET_MAX_ADDRESS;
 
-	error = ata_do_cmd(device,
+	return (ata_do_cmd(device,
 			   ccb,
 			   retry_count,
 			   /*flags*/CAM_DIR_NONE,
 			   /*protocol*/protocol,
-			   /*ata_flags*/AP_FLAG_CHK_COND,
+			   /*ata_flags*/0,
 			   /*tag_action*/MSG_SIMPLE_Q_TAG,
 			   /*command*/cmd,
 			   /*features*/ATA_HPA_FEAT_LOCK,
@@ -2190,12 +2127,7 @@ atahpa_lock(struct cam_device *device, int retry_count
 			   /*data_ptr*/NULL,
 			   /*dxfer_len*/0,
 			   timeout ? timeout : 1000,
-			   is48bit);
-
-	if (error)
-		return (error);
-
-	return atahpa_proc_resp(device, ccb, is48bit, NULL);
+			   is48bit));
 }
 
 static int
@@ -2203,20 +2135,19 @@ atahpa_unlock(struct cam_device *device, int retry_cou
 	      u_int32_t timeout, union ccb *ccb,
 	      int is48bit, struct ata_set_max_pwd *pwd)
 {
-	int error;
 	u_int cmd;
 	u_int8_t protocol;
 
 	protocol = AP_PROTO_PIO_OUT;
 	cmd = (is48bit) ? ATA_SET_MAX_ADDRESS48 : ATA_SET_MAX_ADDRESS;
 
-	error = ata_do_cmd(device,
+	return (ata_do_cmd(device,
 			   ccb,
 			   retry_count,
 			   /*flags*/CAM_DIR_OUT,
 			   /*protocol*/protocol,
 			   /*ata_flags*/AP_FLAG_BYT_BLOK_BLOCKS |
-			    AP_FLAG_TLEN_SECT_CNT | AP_FLAG_CHK_COND,
+			    AP_FLAG_TLEN_SECT_CNT,
 			   /*tag_action*/MSG_SIMPLE_Q_TAG,
 			   /*command*/cmd,
 			   /*features*/ATA_HPA_FEAT_UNLOCK,
@@ -2225,31 +2156,25 @@ atahpa_unlock(struct cam_device *device, int retry_cou
 			   /*data_ptr*/(u_int8_t*)pwd,
 			   /*dxfer_len*/sizeof(*pwd),
 			   timeout ? timeout : 1000,
-			   is48bit);
-
-	if (error)
-		return (error);
-
-	return atahpa_proc_resp(device, ccb, is48bit, NULL);
+			   is48bit));
 }
 
 static int
 atahpa_freeze_lock(struct cam_device *device, int retry_count,
 		   u_int32_t timeout, union ccb *ccb, int is48bit)
 {
-	int error;
 	u_int cmd;
 	u_int8_t protocol;
 
 	protocol = AP_PROTO_NON_DATA;
 	cmd = (is48bit) ? ATA_SET_MAX_ADDRESS48 : ATA_SET_MAX_ADDRESS;
 
-	error = ata_do_cmd(device,
+	return (ata_do_cmd(device,
 			   ccb,
 			   retry_count,
 			   /*flags*/CAM_DIR_NONE,
 			   /*protocol*/protocol,
-			   /*ata_flags*/AP_FLAG_CHK_COND,
+			   /*ata_flags*/0,
 			   /*tag_action*/MSG_SIMPLE_Q_TAG,
 			   /*command*/cmd,
 			   /*features*/ATA_HPA_FEAT_FREEZE,
@@ -2258,12 +2183,7 @@ atahpa_freeze_lock(struct cam_device *device, int retr
 			   /*data_ptr*/NULL,
 			   /*dxfer_len*/0,
 			   timeout ? timeout : 1000,
-			   is48bit);
-
-	if (error)
-		return (error);
-
-	return atahpa_proc_resp(device, ccb, is48bit, NULL);
+			   is48bit));
 }
 
 static int
@@ -2292,7 +2212,7 @@ ata_get_native_max(struct cam_device *device, int retr
 	if (error)
 		return (error);
 
-	return atahpa_proc_resp(device, ccb, /*is48bit*/1, nativesize);
+	return atahpa_proc_resp(device, ccb, nativesize);
 }
 
 static int
@@ -2324,21 +2244,20 @@ ataama_set(struct cam_device *device, int retry_count,
 	if (error)
 		return (error);
 
-	return atahpa_proc_resp(device, ccb, /*is48bit*/1, NULL);
+	return atahpa_proc_resp(device, ccb, NULL);
 }
 
 static int
 ataama_freeze(struct cam_device *device, int retry_count,
 		   u_int32_t timeout, union ccb *ccb)
 {
-	int error;
 
-	error = ata_do_cmd(device,
+	return (ata_do_cmd(device,
 			   ccb,
 			   retry_count,
 			   /*flags*/CAM_DIR_NONE,
 			   /*protocol*/AP_PROTO_NON_DATA | AP_EXTEND,
-			   /*ata_flags*/AP_FLAG_CHK_COND,
+			   /*ata_flags*/0,
 			   /*tag_action*/MSG_SIMPLE_Q_TAG,
 			   /*command*/ATA_AMAX_ADDR,
 			   /*features*/ATA_AMAX_ADDR_FREEZE,
@@ -2347,12 +2266,7 @@ ataama_freeze(struct cam_device *device, int retry_cou
 			   /*data_ptr*/NULL,
 			   /*dxfer_len*/0,
 			   timeout ? timeout : 30 * 1000,
-			   /*force48bit*/1);
-
-	if (error)
-		return (error);
-
-	return atahpa_proc_resp(device, ccb, /*is48bit*/1, NULL);
+			   /*force48bit*/1));
 }
 
 int
@@ -2448,7 +2362,7 @@ ataidentify(struct cam_device *device, int retry_count
 {
 	union ccb *ccb;
 	struct ata_params *ident_buf;
-	u_int64_t hpasize, nativesize;
+	u_int64_t hpasize = 0, nativesize = 0;
 
 	if ((ccb = cam_getccb(device)) == NULL) {
 		warnx("couldn't allocate CCB");
@@ -2467,22 +2381,12 @@ ataidentify(struct cam_device *device, int retry_count
 	}
 
 	if (ident_buf->support.command1 & ATA_SUPPORT_PROTECTED) {
-		if (ata_read_native_max(device, retry_count, timeout, ccb,
-					ident_buf, &hpasize) != 0) {
-			cam_freeccb(ccb);
-			return (1);
-		}
-	} else {
-		hpasize = 0;
+		ata_read_native_max(device, retry_count, timeout, ccb,
+				    ident_buf, &hpasize);
 	}
 	if (ident_buf->support2 & ATA_SUPPORT_AMAX_ADDR) {
-		if (ata_get_native_max(device, retry_count, timeout, ccb,
-					&nativesize) != 0) {
-			cam_freeccb(ccb);
-			return (1);
-		}
-	} else {
-		nativesize = 0;
+		ata_get_native_max(device, retry_count, timeout, ccb,
+				   &nativesize);
 	}
 
 	printf("%s%d: ", device->device_name, device->dev_unit_num);
@@ -3003,10 +2907,11 @@ atahpa(struct cam_device *device, int retry_count, int
 	}
 
 	if (action == ATA_HPA_ACTION_PRINT) {
-		error = ata_read_native_max(device, retry_count, timeout, ccb,
-					    ident_buf, &hpasize);
-		if (error == 0)
-			atahpa_print(ident_buf, hpasize, 1);
+		hpasize = 0;
+		if (ident_buf->support.command1 & ATA_SUPPORT_PROTECTED)
+			ata_read_native_max(device, retry_count, timeout, ccb,
+				    ident_buf, &hpasize);
+		atahpa_print(ident_buf, hpasize, 1);
 
 		cam_freeccb(ccb);
 		free(ident_buf);
@@ -3049,12 +2954,14 @@ atahpa(struct cam_device *device, int retry_count, int
 		if (error == 0) {
 			error = atahpa_set_max(device, retry_count, timeout,
 					       ccb, is48bit, maxsize, persist);
-			if (error == 0 && quiet == 0) {
-				/* redo identify to get new lba values */
-				error = ata_do_identify(device, retry_count,
-							timeout, ccb,
-							&ident_buf);
-				atahpa_print(ident_buf, hpasize, 1);
+			if (error == 0) {
+				if (quiet == 0) {
+					/* redo identify to get new values */
+					error = ata_do_identify(device,
+					    retry_count, timeout, ccb,
+					    &ident_buf);
+					atahpa_print(ident_buf, hpasize, 1);
+				}
 				/* Hint CAM to reprobe the device. */
 				reprobe(device);
 			}
@@ -3170,10 +3077,11 @@ ataama(struct cam_device *device, int retry_count, int
 	}
 
 	if (action == ATA_AMA_ACTION_PRINT) {
-		error = ata_get_native_max(device, retry_count, timeout, ccb,
+		nativesize = 0;
+		if (ident_buf->support2 & ATA_SUPPORT_AMAX_ADDR)
+			ata_get_native_max(device, retry_count, timeout, ccb,
 					   &nativesize);
-		if (error == 0)
-			ataama_print(ident_buf, nativesize, 1);
+		ataama_print(ident_buf, nativesize, 1);
 
 		cam_freeccb(ccb);
 		free(ident_buf);
@@ -3194,11 +3102,14 @@ ataama(struct cam_device *device, int retry_count, int
 		if (error == 0) {
 			error = ataama_set(device, retry_count, timeout,
 				       ccb, maxsize);
-			if (error == 0 && quiet == 0) {
-				/* redo identify to get new lba values */
-				error = ata_do_identify(device, retry_count,
-				    timeout, ccb, &ident_buf);
-				ataama_print(ident_buf, nativesize, 1);
+			if (error == 0) {
+				if (quiet == 0) {
+					/* redo identify to get new values */
+					error = ata_do_identify(device,
+					    retry_count, timeout, ccb,
+					    &ident_buf);
+					ataama_print(ident_buf, nativesize, 1);
+				}
 				/* Hint CAM to reprobe the device. */
 				reprobe(device);
 			}
@@ -5729,16 +5640,21 @@ build_ata_cmd(union ccb *ccb, uint32_t retry_count, ui
 	return (retval);
 }
 
+/*
+ * Returns: 0 -- success, 1 -- error, 2 -- lba truncated,
+ *	    4 -- count truncated, 6 -- lba and count truncated.
+ */
 int
 get_ata_status(struct cam_device *dev, union ccb *ccb, uint8_t *error,
 	       uint16_t *count, uint64_t *lba, uint8_t *device, uint8_t *status)
 {
-	int retval = 0;
+	int retval;
 
 	switch (ccb->ccb_h.func_code) {
 	case XPT_SCSI_IO: {
 		uint8_t opcode;
 		int error_code = 0, sense_key = 0, asc = 0, ascq = 0;
+		u_int sense_len;
 
 		/*
 		 * In this case, we have SCSI ATA PASS-THROUGH command, 12
@@ -5750,20 +5666,17 @@ get_ata_status(struct cam_device *dev, union ccb *ccb,
 			opcode = ccb->csio.cdb_io.cdb_bytes[0];
 		if ((opcode != ATA_PASS_12)
 		 && (opcode != ATA_PASS_16)) {
-			retval = 1;
 			warnx("%s: unsupported opcode %02x", __func__, opcode);
-			goto bailout;
+			return (1);
 		}
 
 		retval = scsi_extract_sense_ccb(ccb, &error_code, &sense_key,
 						&asc, &ascq);
 		/* Note: the _ccb() variant returns 0 for an error */
-		if (retval == 0) {
-			retval = 1;
-			goto bailout;
-		} else
-			retval = 0;
+		if (retval == 0)
+			return (1);
 
+		sense_len = ccb->csio.sense_len - ccb->csio.sense_resid;
 		switch (error_code) {
 		case SSD_DESC_CURRENT_ERROR:
 		case SSD_DESC_DEFERRED_ERROR: {
@@ -5774,13 +5687,12 @@ get_ata_status(struct cam_device *dev, union ccb *ccb,
 			sense = (struct scsi_sense_data_desc *)
 			    &ccb->csio.sense_data;
 
-			desc_ptr = scsi_find_desc(sense, ccb->csio.sense_len -
-			    ccb->csio.sense_resid, SSD_DESC_ATA);
+			desc_ptr = scsi_find_desc(sense, sense_len,
+			    SSD_DESC_ATA);
 			if (desc_ptr == NULL) {
 				cam_error_print(dev, ccb, CAM_ESF_ALL,
 				    CAM_EPF_ALL, stderr);
-				retval = 1;
-				goto bailout;
+				return (1);
 			}
 			desc = (struct scsi_sense_ata_ret_desc *)desc_ptr;
 
@@ -5812,22 +5724,47 @@ get_ata_status(struct cam_device *dev, union ccb *ccb,
 		}
 		case SSD_CURRENT_ERROR:
 		case SSD_DEFERRED_ERROR: {
-#if 0
-			struct scsi_sense_data_fixed *sense;
-#endif
+			uint64_t val;
+
 			/*
-			 * XXX KDM need to support fixed sense data.
+			 * In my understanding of SAT-5 specification, saying:
+			 * "without interpreting the contents of the STATUS",
+			 * this should not happen if CK_COND was set, but it
+			 * does at least for some devices, so try to revert.
 			 */
-			warnx("%s: Fixed sense data not supported yet",
-			    __func__);
-			retval = 1;
-			goto bailout;
-			break; /*NOTREACHED*/
+			if ((sense_key == SSD_KEY_ABORTED_COMMAND) &&
+			    (asc == 0) && (ascq == 0)) {
+				*status = ATA_STATUS_ERROR;
+				*error = ATA_ERROR_ABORT;
+				*device = 0;
+				*count = 0;
+				*lba = 0;
+				return (0);
+			}
+
+			if ((sense_key != SSD_KEY_RECOVERED_ERROR) ||
+			    (asc != 0x00) || (ascq != 0x1d))
+				return (1);
+
+			val = 0;
+			scsi_get_sense_info(&ccb->csio.sense_data, sense_len,
+			    SSD_DESC_INFO, &val, NULL);
+			*error = (val >> 24) & 0xff;
+			*status = (val >> 16) & 0xff;
+			*device = (val >> 8) & 0xff;
+			*count = val & 0xff;
+
+			val = 0;
+			scsi_get_sense_info(&ccb->csio.sense_data, sense_len,
+			    SSD_DESC_COMMAND, &val, NULL);
+			*lba = ((val >> 16) & 0xff) | (val & 0xff00) |
+				((val & 0xff) << 16);
+
+			/* Report UPPER NONZERO bits as errors 2, 4 and 6. */
+			return ((val >> 28) & 0x06);
 		}
 		default:
-			retval = 1;
-			goto bailout;
-			break;
+			return (1);
 		}
 
 		break;
@@ -5835,11 +5772,11 @@ get_ata_status(struct cam_device *dev, union ccb *ccb,
 	case XPT_ATA_IO: {
 		struct ata_res *res;
 
-		/*
-		 * In this case, we have an ATA command, and we need to
-		 * fill in the requested values from the result register
-		 * set.
-		 */
+		/* Only some statuses return ATA result register set. */
+		if (cam_ccb_status(ccb) != CAM_REQ_CMP &&
+		    cam_ccb_status(ccb) != CAM_ATA_STATUS_ERROR)
+			return (1);
+
 		res = &ccb->ataio.res;
 		*error = res->error;
 		*status = res->status;
@@ -5848,7 +5785,7 @@ get_ata_status(struct cam_device *dev, union ccb *ccb,
 		*lba = (res->lba_high << 16) |
 		       (res->lba_mid << 8) |
 		       (res->lba_low);
-		if (res->flags & CAM_ATAIO_48BIT) {
+		if (ccb->ataio.cmd.flags & CAM_ATAIO_48BIT) {
 			*count |= (res->sector_count_exp << 8);
 			*lba |= ((uint64_t)res->lba_low_exp << 24) |
 				((uint64_t)res->lba_mid_exp << 32) |
@@ -5859,11 +5796,9 @@ get_ata_status(struct cam_device *dev, union ccb *ccb,
 		break;
 	}
 	default:
-		retval = 1;
-		break;
+		return (1);
 	}
-bailout:
-	return (retval);
+	return (0);
 }
 
 static void
@@ -6461,7 +6396,7 @@ scsiformat(struct cam_device *device, int argc, char *
 	if (reportonly)
 		goto doreport;
 
-	if (quiet == 0) {
+	if (quiet == 0 && ycount == 0) {
 		fprintf(stdout, "You are about to REMOVE ALL DATA from the "
 			"following device:\n");
 
@@ -6702,66 +6637,78 @@ scsiformat_bailout:
 }
 
 static int
-sanitize_wait_ata(struct cam_device *device, union ccb *ccb, int quiet)
+sanitize_wait_ata(struct cam_device *device, union ccb *ccb, int quiet,
+    camcontrol_devtype devtype)
 {
-	struct ata_res *res;
 	int retval;
-	cam_status status;
+	uint8_t error = 0, ata_device = 0, status = 0;
+	uint16_t count = 0;
+	uint64_t lba = 0;
 	u_int val, perc;
 
 	do {
-		retval = ata_do_cmd(device,
-				   ccb,
-				   /*retries*/1,
-				   /*flags*/CAM_DIR_NONE,
-				   /*protocol*/AP_PROTO_NON_DATA | AP_EXTEND,
-				   /*ata_flags*/AP_FLAG_CHK_COND,
-				   /*tag_action*/MSG_SIMPLE_Q_TAG,
-				   /*command*/ATA_SANITIZE,
-				   /*features*/0x00, /* SANITIZE STATUS EXT */
-				   /*lba*/0,
-				   /*sector_count*/0,
-				   /*data_ptr*/NULL,
-				   /*dxfer_len*/0,
-				   /*timeout*/10000,
-				   /*is48bit*/1);
-		if (retval < 0) {
-			warn("error sending CAMIOCOMMAND ioctl");
-			if (arglist & CAM_ARG_VERBOSE) {
-				cam_error_print(device, ccb, CAM_ESF_ALL,
-						CAM_EPF_ALL, stderr);
-			}
+		retval = build_ata_cmd(ccb,
+			     /*retries*/ 0,
+			     /*flags*/ CAM_DIR_NONE,
+			     /*tag_action*/ MSG_SIMPLE_Q_TAG,
+			     /*protocol*/ AP_PROTO_NON_DATA,
+			     /*ata_flags*/ AP_FLAG_CHK_COND,
+			     /*features*/ 0x00, /* SANITIZE STATUS EXT */
+			     /*sector_count*/ 0,
+			     /*lba*/ 0,
+			     /*command*/ ATA_SANITIZE,
+			     /*auxiliary*/ 0,
+			     /*data_ptr*/ NULL,
+			     /*dxfer_len*/ 0,
+			     /*cdb_storage*/ NULL,
+			     /*cdb_storage_len*/ 0,
+			     /*sense_len*/ SSD_FULL_SIZE,
+			     /*timeout*/ 10000,
+			     /*is48bit*/ 1,
+			     /*devtype*/ devtype);
+		if (retval != 0) {
+			warnx("%s: build_ata_cmd() failed, likely "
+			    "programmer error", __func__);
 			return (1);
 		}
 
-		status = ccb->ccb_h.status & CAM_STATUS_MASK;
-		if (status == CAM_REQ_CMP) {
-			res = &ccb->ataio.res;
-			if (res->sector_count_exp & 0x40) {
-				if (quiet == 0) {
-					val = (res->lba_mid << 8) + res->lba_low;
-					perc = 10000 * val;
-					fprintf(stdout,
-					    "Sanitizing: %u.%02u%% (%d/%d)\r",
-					    (perc / (0x10000 * 100)),
-					    ((perc / 0x10000) % 100),
-					    val, 0x10000);
-					fflush(stdout);
-				}
-				sleep(1);
-			} else if ((res->sector_count_exp & 0x80) == 0) {
-				warnx("Sanitize complete with an error.     ");
-				return (1);
-			} else
-				break;
+		ccb->ccb_h.flags |= CAM_DEV_QFRZDIS;
+		ccb->ccb_h.flags |= CAM_PASS_ERR_RECOVER;
+		retval = cam_send_ccb(device, ccb);
+		if (retval != 0) {
+			warn("error sending SANITIZE STATUS EXT command");
+			return (1);
+		}
 
-		} else if (status != CAM_REQ_CMP && status != CAM_REQUEUE_REQ) {
-			warnx("Unexpected CAM status %#x", status);
-			if (arglist & CAM_ARG_VERBOSE)
-				cam_error_print(device, ccb, CAM_ESF_ALL,
-						CAM_EPF_ALL, stderr);
+		retval = get_ata_status(device, ccb, &error, &count, &lba,
+		    &ata_device, &status);
+		if (retval != 0) {
+			warnx("Can't get SANITIZE STATUS EXT status, "
+			    "sanitize may still run.");
+			return (retval);
+		}
+		if (status & ATA_STATUS_ERROR) {
+			warnx("SANITIZE STATUS EXT failed, "
+			    "sanitize may still run.");
 			return (1);
 		}
+		if (count & 0x4000) {
+			if (quiet == 0) {
+				val = lba & 0xffff;
+				perc = 10000 * val;
+				fprintf(stdout,
+				    "Sanitizing: %u.%02u%% (%d/%d)\r",
+				    (perc / (0x10000 * 100)),
+				    ((perc / 0x10000) % 100),
+				    val, 0x10000);
+				fflush(stdout);
+			}
+			sleep(1);
+		} else if ((count & 0x8000) == 0) {
+			warnx("Sanitize complete with an error.     ");
+			return (1);
+		} else
+			break;
 	} while (1);
 	return (0);
 }
@@ -7041,7 +6988,7 @@ sanitize(struct cam_device *device, int argc, char **a
 		}
 	}
 
-	if (quiet == 0) {
+	if (quiet == 0 && ycount == 0) {
 		fprintf(stdout, "You are about to REMOVE ALL DATA from the "
 			"following device:\n");
 
@@ -7169,7 +7116,7 @@ sanitize(struct cam_device *device, int argc, char **a
 				   retry_count,
 				   /*flags*/CAM_DIR_NONE,
 				   /*protocol*/AP_PROTO_NON_DATA | AP_EXTEND,
-				   /*ata_flags*/AP_FLAG_CHK_COND,
+				   /*ata_flags*/0,
 				   /*tag_action*/MSG_SIMPLE_Q_TAG,
 				   /*command*/ATA_SANITIZE,
 				   /*features*/feature,
@@ -7226,7 +7173,7 @@ doreport:
 	if (dt == CC_DT_SCSI) {
 		error = sanitize_wait_scsi(device, ccb, task_attr, quiet);
 	} else if (dt == CC_DT_ATA || dt == CC_DT_SATL) {
-		error = sanitize_wait_ata(device, ccb, quiet);
+		error = sanitize_wait_ata(device, ccb, quiet, dt);
 	} else
 		error = 1;
 	if (error == 0 && quiet == 0)
@@ -9199,56 +9146,63 @@ bailout:
 static int
 atapm_proc_resp(struct cam_device *device, union ccb *ccb)
 {
-    struct ata_res *res;
+	uint8_t error = 0, ata_device = 0, status = 0;
+	uint16_t count = 0;
+	uint64_t lba = 0;
+	int retval;
 
-    res = &ccb->ataio.res;
-    if (res->status & ATA_STATUS_ERROR) {
-        if (arglist & CAM_ARG_VERBOSE) {
-            cam_error_print(device, ccb, CAM_ESF_ALL,
-                    CAM_EPF_ALL, stderr);
-            printf("error = 0x%02x, sector_count = 0x%04x, "
-                   "device = 0x%02x, status = 0x%02x\n",
-                   res->error, res->sector_count,
-                   res->device, res->status);
-        }
+	retval = get_ata_status(device, ccb, &error, &count, &lba, &ata_device,
+	    &status);
+	if (retval == 1) {
+		if (arglist & CAM_ARG_VERBOSE) {
+			cam_error_print(device, ccb, CAM_ESF_ALL,
+					CAM_EPF_ALL, stderr);
+		}
+		warnx("Can't get ATA command status");
+		return (retval);
+	}
 
-        return (1);
-    }
+	if (status & ATA_STATUS_ERROR) {
+		cam_error_print(device, ccb, CAM_ESF_ALL,
+		    CAM_EPF_ALL, stderr);
+	        return (1);
+	}
 
-    if (arglist & CAM_ARG_VERBOSE) {
-        fprintf(stdout, "%s%d: Raw native check power data:\n",
-            device->device_name, device->dev_unit_num);
-        /* res is 4 byte aligned */
-        dump_data((uint16_t*)(uintptr_t)res, sizeof(struct ata_res));
+	printf("%s%d: ", device->device_name, device->dev_unit_num);
+	switch (count) {
+	case 0x00:
+		printf("Standby mode\n");
+		break;
+	case 0x01:
+		printf("Standby_y mode\n");
+		break;
+	case 0x40:
+		printf("NV Cache Power Mode and the spindle is spun down or spinning down\n");
+		break;
+	case 0x41:
+		printf("NV Cache Power Mode and the spindle is spun up or spinning up\n");
+		break;
+	case 0x80:
+		printf("Idle mode\n");
+		break;
+	case 0x81:
+		printf("Idle_a mode\n");
+		break;
+	case 0x82:
+		printf("Idle_b mode\n");
+		break;
+	case 0x83:
+		printf("Idle_c mode\n");
+		break;
+	case 0xff:
+		printf("Active or Idle mode\n");
+		break;
+	default:
+		printf("Unknown mode 0x%02x\n", count);
+		break;
+	}
 
-        printf("error = 0x%02x, sector_count = 0x%04x, device = 0x%02x, "
-               "status = 0x%02x\n", res->error, res->sector_count,
-               res->device, res->status);
-    }
-
-    printf("%s%d: ", device->device_name, device->dev_unit_num);
-    switch (res->sector_count) {
-    case 0x00:
-       printf("Standby mode\n");
-       break;
-    case 0x40:
-       printf("NV Cache Power Mode and the spindle is spun down or spinning down\n");
-       break;
-    case 0x41:
-       printf("NV Cache Power Mode and the spindle is spun up or spinning up\n");
-       break;
-    case 0x80:
-       printf("Idle mode\n");
-       break;
-    case 0xff:
-       printf("Active or Idle mode\n");
-       break;
-    default:
-       printf("Unknown mode 0x%02x\n", res->sector_count);
-       break;
-    }
-
-    return (0);
+	return (0);
 }
 
 static int



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201909091736.x89HaTrf083664>