- No log bit in IOCStatus and endian-safe changes.

Use MPI2_IOCSTATUS_MASK when checking IOCStatus to mask off the log bit, and
make a few more things endian-safe.

- Fix possible use of invalid pointer.

It was possible to use an invalid pointer to get the target ID value. To fix
this, initialize a local Target ID variable to an invalid value and change that
variable to a valid value only if the pointer to the Target ID is not NULL.

- No need to set the MPSSAS_SHUTDOWN flag because it's never used.

- done_ccb pointer can be used if it is NULL.

To prevent this, move check for done_ccb == NULL to before done_ccb is used in
mpssas_stop_unit_done().

- Disks can go missing until a reboot is done in some cases.

This is due to the DevHandle not being released, which causes the Firmware to
not allow that disk to be re-added.

Reviewed by:	ken
Approved by:	re (gjb), ken, scottl, ambrisko (mentors)
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D6872

6 files changed, 35 insertions, 29 deletions

diff --git a/sys/dev/mps/mps.c b/sys/dev/mps/mps.c
index b564b33..3f75989 100644
--- a/sys/dev/mps/mps.c
+++ b/sys/dev/mps/mps.c
@@ -1922,9 +1922,10 @@ mps_intr_locked(void *data)
 					 */
 					rel_rep =
 					    (MPI2_DIAG_RELEASE_REPLY *)reply;
-					if (le16toh(rel_rep->IOCStatus) ==
+					if ((le16toh(rel_rep->IOCStatus) &
+					    MPI2_IOCSTATUS_MASK) ==
 					    MPI2_IOCSTATUS_DIAGNOSTIC_RELEASED)
-					    {
+					{
 						pBuffer =
 						    &sc->fw_diag_buffer_list[
 						    rel_rep->BufferType];
diff --git a/sys/dev/mps/mps_config.c b/sys/dev/mps/mps_config.c
index c21506d..e435d4f 100644
--- a/sys/dev/mps/mps_config.c
+++ b/sys/dev/mps/mps_config.c
@@ -499,7 +499,8 @@ mps_wd_config_pages(struct mps_softc *sc)
 		 */
 		if (mps_config_get_raid_volume_pg0(sc, &mpi_reply,
 		    raid_vol_pg0, (u32)raid_vol_pg0->DevHandle)) {
-			if (mpi_reply.IOCStatus !=
+			if ((le16toh(mpi_reply.IOCStatus) &
+			    MPI2_IOCSTATUS_MASK) !=
 			    MPI2_IOCSTATUS_CONFIG_INVALID_PAGE) {
 				mps_dprint(sc, MPS_FAULT,
 				    "Multiple RAID Volume Page0! Direct Drive "
diff --git a/sys/dev/mps/mps_sas.c b/sys/dev/mps/mps_sas.c
index 3790e8f..6a34c38 100644
--- a/sys/dev/mps/mps_sas.c
+++ b/sys/dev/mps/mps_sas.c
@@ -241,6 +241,8 @@ mpssas_alloc_tm(struct mps_softc *sc)
 void
 mpssas_free_tm(struct mps_softc *sc, struct mps_command *tm)
 {
+	int target_id = 0xFFFFFFFF;
+ 
 	if (tm == NULL)
 		return;
 
@@ -251,10 +253,11 @@ mpssas_free_tm(struct mps_softc *sc, struct mps_command *tm)
 	 */
 	if (tm->cm_targ != NULL) {
 		tm->cm_targ->flags &= ~MPSSAS_TARGET_INRESET;
+		target_id = tm->cm_targ->tid;
 	}
 	if (tm->cm_ccb) {
 		mps_dprint(sc, MPS_INFO, "Unfreezing devq for target ID %d\n",
-		    tm->cm_targ->tid);
+		    target_id);
 		xpt_release_devq(tm->cm_ccb->ccb_h.path, 1, TRUE);
 		xpt_free_path(tm->cm_ccb->ccb_h.path);
 		xpt_free_ccb(tm->cm_ccb);
@@ -372,12 +375,11 @@ mpssas_remove_volume(struct mps_softc *sc, struct mps_command *tm)
 		return;
 	}
 
-	if (reply->IOCStatus != MPI2_IOCSTATUS_SUCCESS) {
-		mps_dprint(sc, MPS_FAULT,
+	if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) !=
+	    MPI2_IOCSTATUS_SUCCESS) {
+		mps_dprint(sc, MPS_ERROR,
 		   "IOCStatus = 0x%x while resetting device 0x%x\n",
-		   reply->IOCStatus, handle);
-		mpssas_free_tm(sc, tm);
-		return;
+		   le16toh(reply->IOCStatus), handle);
 	}
 
 	mps_dprint(sc, MPS_XINFO,
@@ -394,7 +396,8 @@ mpssas_remove_volume(struct mps_softc *sc, struct mps_command *tm)
 	 * this target id if possible, and so we can assign the same target id
 	 * to this device if it comes back in the future.
 	 */
-	if (reply->IOCStatus == MPI2_IOCSTATUS_SUCCESS) {
+	if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) ==
+	    MPI2_IOCSTATUS_SUCCESS) {
 		targ = tm->cm_targ;
 		targ->handle = 0x0;
 		targ->encl_handle = 0x0;
@@ -567,24 +570,22 @@ mpssas_remove_device(struct mps_softc *sc, struct mps_command *tm)
 		    "%s: cm_flags = %#x for remove of handle %#04x! "
 		    "This should not happen!\n", __func__, tm->cm_flags,
 		    handle);
-		mpssas_free_tm(sc, tm);
-		return;
 	}
 
 	if (reply == NULL) {
 		/* XXX retry the remove after the diag reset completes? */
 		mps_dprint(sc, MPS_FAULT,
-		    "%s NULL reply reseting device 0x%04x\n", __func__, handle);
+		    "%s NULL reply resetting device 0x%04x\n", __func__,
+		    handle);
 		mpssas_free_tm(sc, tm);
 		return;
 	}
 
-	if (le16toh(reply->IOCStatus) != MPI2_IOCSTATUS_SUCCESS) {
-		mps_dprint(sc, MPS_FAULT,
+	if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) !=
+	    MPI2_IOCSTATUS_SUCCESS) {
+		mps_dprint(sc, MPS_ERROR,
 		   "IOCStatus = 0x%x while resetting device 0x%x\n",
 		   le16toh(reply->IOCStatus), handle);
-		mpssas_free_tm(sc, tm);
-		return;
 	}
 
 	mps_dprint(sc, MPS_XINFO, "Reset aborted %u commands\n",
@@ -662,7 +663,8 @@ mpssas_remove_complete(struct mps_softc *sc, struct mps_command *tm)
 	 * this target id if possible, and so we can assign the same target id
 	 * to this device if it comes back in the future.
 	 */
-	if (le16toh(reply->IOCStatus) == MPI2_IOCSTATUS_SUCCESS) {
+	if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) ==
+	    MPI2_IOCSTATUS_SUCCESS) {
 		targ = tm->cm_targ;
 		targ->handle = 0x0;
 		targ->encl_handle = 0x0;
@@ -880,7 +882,6 @@ mps_detach_sas(struct mps_softc *sc)
 		cam_sim_free(sassc->sim, FALSE);
 	}
 
-	sassc->flags |= MPSSAS_SHUTDOWN;
 	mps_unlock(sc);
 
 	if (sassc->devq != NULL)
diff --git a/sys/dev/mps/mps_sas_lsi.c b/sys/dev/mps/mps_sas_lsi.c
index e9fa06b..c04a78a 100644
--- a/sys/dev/mps/mps_sas_lsi.c
+++ b/sys/dev/mps/mps_sas_lsi.c
@@ -1161,15 +1161,15 @@ mpssas_stop_unit_done(struct cam_periph *periph, union ccb *done_ccb)
 	struct mpssas_softc *sassc;
 	char path_str[64];
 
+	if (done_ccb == NULL)
+		return;
+
 	sassc = (struct mpssas_softc *)done_ccb->ccb_h.ppriv_ptr1;
 
 	xpt_path_string(done_ccb->ccb_h.path, path_str, sizeof(path_str));
 	mps_dprint(sassc->sc, MPS_INFO, "Completing stop unit for %s\n",
 	    path_str);
 
-	if (done_ccb == NULL)
-		return;
-
 	/*
 	 * Nothing more to do except free the CCB and path.  If the command
 	 * timed out, an abort reset, then target reset will be issued during
diff --git a/sys/dev/mps/mps_user.c b/sys/dev/mps/mps_user.c
index 3fe0554..3638ac6 100644
--- a/sys/dev/mps/mps_user.c
+++ b/sys/dev/mps/mps_user.c
@@ -1220,12 +1220,14 @@ mps_post_fw_diag_buffer(struct mps_softc *sc,
 	 * Process POST reply.
 	 */
 	reply = (MPI2_DIAG_BUFFER_POST_REPLY *)cm->cm_reply;
-	if (reply->IOCStatus != MPI2_IOCSTATUS_SUCCESS) {
+	if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) !=
+	    MPI2_IOCSTATUS_SUCCESS) {
 		status = MPS_DIAG_FAILURE;
 		mps_dprint(sc, MPS_FAULT, "%s: post of FW  Diag Buffer failed "
 		    "with IOCStatus = 0x%x, IOCLogInfo = 0x%x and "
-		    "TransferLength = 0x%x\n", __func__, reply->IOCStatus,
-		    reply->IOCLogInfo, reply->TransferLength);
+		    "TransferLength = 0x%x\n", __func__,
+		    le16toh(reply->IOCStatus), le32toh(reply->IOCLogInfo),
+		    le32toh(reply->TransferLength));
 		goto done;
 	}
 
@@ -1304,12 +1306,13 @@ mps_release_fw_diag_buffer(struct mps_softc *sc,
 	 * Process RELEASE reply.
 	 */
 	reply = (MPI2_DIAG_RELEASE_REPLY *)cm->cm_reply;
-	if ((reply->IOCStatus != MPI2_IOCSTATUS_SUCCESS) ||
-	    pBuffer->owned_by_firmware) {
+	if (((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) !=
+	    MPI2_IOCSTATUS_SUCCESS) || pBuffer->owned_by_firmware) {
 		status = MPS_DIAG_FAILURE;
 		mps_dprint(sc, MPS_FAULT, "%s: release of FW Diag Buffer "
 		    "failed with IOCStatus = 0x%x and IOCLogInfo = 0x%x\n",
-		    __func__, reply->IOCStatus, reply->IOCLogInfo);
+		    __func__, le16toh(reply->IOCStatus),
+		    le32toh(reply->IOCLogInfo));
 		goto done;
 	}
 
diff --git a/sys/dev/mps/mpsvar.h b/sys/dev/mps/mpsvar.h
index 0cb51a0..74c5e55 100644
--- a/sys/dev/mps/mpsvar.h
+++ b/sys/dev/mps/mpsvar.h
@@ -33,7 +33,7 @@
 #ifndef _MPSVAR_H
 #define _MPSVAR_H
 
-#define MPS_DRIVER_VERSION	"20.00.00.00-fbsd"
+#define MPS_DRIVER_VERSION	"21.00.00.00-fbsd"
 
 #define MPS_DB_MAX_WAIT		2500