- 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
This commit is contained in:
Stephen McConnell 2016-06-20 18:14:51 +00:00
parent d304f3093a
commit f4e69c98fd
6 changed files with 35 additions and 29 deletions

View File

@ -1922,7 +1922,8 @@ 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 =

View File

@ -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 "

View File

@ -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)

View File

@ -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

View File

@ -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;
}

View File

@ -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