MFC r314998: Fix FC target mode in mpt(4), broken in multiple ways.

 - Not set BufferLength caused receive of empty ATIOs.
 - CDB length guessing was broken at least for RC16.
 - mpt_req_untimeout() was called with wrong req parameter.
 - Sense data reporting was broken in several ways.

With this change my LSI7204EP-LC can pass at least basic tests as target.
The code is still far from perfect, but finally I found second hw/driver
after isp(4) that really can work in CAM target mode.

2 files changed, 51 insertions, 92 deletions

diff --git a/sys/dev/mpt/mpt.h b/sys/dev/mpt/mpt.h
index 20f2477..b2a936d 100644
--- a/sys/dev/mpt/mpt.h
+++ b/sys/dev/mpt/mpt.h
@@ -949,24 +949,6 @@ void mpt_prtc(struct mpt_softc *, const char *, ...)
 	__printflike(2, 3);
 
 /**************************** Target Mode Related ***************************/
-static __inline int mpt_cdblen(uint8_t, int);
-static __inline int
-mpt_cdblen(uint8_t cdb0, int maxlen)
-{
-	int group = cdb0 >> 5;
-	switch (group) {
-	case 0:
-		return (6);
-	case 1:
-		return (10);
-	case 4:
-	case 5:
-		return (12);
-	default:
-		return (16);
-	}
-}
-
 #ifdef	INVARIANTS
 static __inline request_t * mpt_tag_2_req(struct mpt_softc *, uint32_t);
 static __inline request_t *
diff --git a/sys/dev/mpt/mpt_cam.c b/sys/dev/mpt/mpt_cam.c
index d2b853c..28367a7 100644
--- a/sys/dev/mpt/mpt_cam.c
+++ b/sys/dev/mpt/mpt_cam.c
@@ -145,7 +145,7 @@ static void mpt_target_start_io(struct mpt_softc *, union ccb *);
 static cam_status mpt_abort_target_ccb(struct mpt_softc *, union ccb *);
 static int mpt_abort_target_cmd(struct mpt_softc *, request_t *);
 static void mpt_scsi_tgt_status(struct mpt_softc *, union ccb *, request_t *,
-    uint8_t, uint8_t const *);
+    uint8_t, uint8_t const *, u_int);
 static void
 mpt_scsi_tgt_tsk_mgmt(struct mpt_softc *, request_t *, mpt_task_mgmt_t,
     tgt_resource_t *, int);
@@ -4168,6 +4168,7 @@ mpt_post_target_command(struct mpt_softc *mpt, request_t *req, int ioindex)
 	fc = req->req_vbuf;
 	fc->BufferCount = 1;
 	fc->Function = MPI_FUNCTION_TARGET_CMD_BUFFER_POST;
+	fc->BufferLength = MIN(MPT_REQUEST_AREA - MPT_RQSL(mpt), UINT8_MAX);
 	fc->MsgContext = htole32(req->index | mpt->scsi_tgt_handler_id);
 
 	cb = &fc->Buffer[0];
@@ -4458,8 +4459,6 @@ mpt_target_start_io(struct mpt_softc *mpt, union ccb *ccb)
 			ccb->ccb_h.status |= CAM_RELEASE_SIMQ;
 		}
 	} else {
-		uint8_t *sp = NULL, sense[MPT_SENSE_SIZE];
-
 		/*
 		 * XXX: I don't know why this seems to happen, but
 		 * XXX: completing the CCB seems to make things happy.
@@ -4476,12 +4475,10 @@ mpt_target_start_io(struct mpt_softc *mpt, union ccb *ccb)
 			xpt_done(ccb);
 			return;
 		}
-		if (ccb->ccb_h.flags & CAM_SEND_SENSE) {
-			sp = sense;
-			memcpy(sp, &csio->sense_data,
-			   min(csio->sense_len, MPT_SENSE_SIZE));
-		}
-		mpt_scsi_tgt_status(mpt, ccb, cmd_req, csio->scsi_status, sp);
+		mpt_scsi_tgt_status(mpt, ccb, cmd_req, csio->scsi_status,
+		    (void *)&csio->sense_data,
+		    (ccb->ccb_h.flags & CAM_SEND_SENSE) ?
+		     csio->sense_len : 0);
 	}
 }
 
@@ -4503,7 +4500,7 @@ mpt_scsi_tgt_local(struct mpt_softc *mpt, request_t *cmd_req,
 	tgt = MPT_TGT_STATE(mpt, cmd_req);
 	if (length == 0 || tgt->resid == 0) {
 		tgt->resid = 0;
-		mpt_scsi_tgt_status(mpt, NULL, cmd_req, 0, NULL);
+		mpt_scsi_tgt_status(mpt, NULL, cmd_req, 0, NULL, 0);
 		return;
 	}
 
@@ -4656,7 +4653,7 @@ mpt_abort_target_cmd(struct mpt_softc *mpt, request_t *cmd_req)
 
 static void
 mpt_scsi_tgt_status(struct mpt_softc *mpt, union ccb *ccb, request_t *cmd_req,
-    uint8_t status, uint8_t const *sense_data)
+    uint8_t status, uint8_t const *sense_data, u_int sense_len)
 {
 	uint8_t *cmd_vbuf;
 	mpt_tgt_state_t *tgt;
@@ -4730,37 +4727,22 @@ mpt_scsi_tgt_status(struct mpt_softc *mpt, union ccb *ccb, request_t *cmd_req,
 		 */
 		memset(rsp, 0, sizeof (MPI_TARGET_FCP_RSP_BUFFER));
 
-		rsp[2] = status;
+		rsp[2] = htobe32(status);
+#define	MIN_FCP_RESPONSE_SIZE	24
+#ifndef	WE_TRUST_AUTO_GOOD_STATUS
+		resplen = MIN_FCP_RESPONSE_SIZE;
+#endif
 		if (tgt->resid) {
-			rsp[2] |= 0x800;	/* XXXX NEED MNEMONIC!!!! */
+			rsp[2] |= htobe32(0x800); /* XXXX NEED MNEMONIC!!!! */
 			rsp[3] = htobe32(tgt->resid);
-#ifdef	WE_TRUST_AUTO_GOOD_STATUS
-			resplen = sizeof (MPI_TARGET_FCP_RSP_BUFFER);
-#endif
+			resplen = MIN_FCP_RESPONSE_SIZE;
 		}
-		if (status == SCSI_STATUS_CHECK_COND) {
-			int i;
-
-			rsp[2] |= 0x200;	/* XXXX NEED MNEMONIC!!!! */
-			rsp[4] = htobe32(MPT_SENSE_SIZE);
-			if (sense_data) {
-				memcpy(&rsp[8], sense_data, MPT_SENSE_SIZE);
-			} else {
-				mpt_prt(mpt, "mpt_scsi_tgt_status: CHECK CONDI"
-				    "TION but no sense data?\n");
-				memset(&rsp, 0, MPT_SENSE_SIZE);
-			}
-			for (i = 8; i < (8 + (MPT_SENSE_SIZE >> 2)); i++) {
-				rsp[i] = htobe32(rsp[i]);
-			}
-#ifdef	WE_TRUST_AUTO_GOOD_STATUS
-			resplen = sizeof (MPI_TARGET_FCP_RSP_BUFFER);
-#endif
+		if (sense_len > 0) {
+			rsp[2] |= htobe32(0x200); /* XXXX NEED MNEMONIC!!!! */
+			rsp[4] = htobe32(sense_len);
+			memcpy(&rsp[6], sense_data, sense_len);
+			resplen = MIN_FCP_RESPONSE_SIZE + sense_len;
 		}
-#ifndef	WE_TRUST_AUTO_GOOD_STATUS
-		resplen = sizeof (MPI_TARGET_FCP_RSP_BUFFER);
-#endif
-		rsp[2] = htobe32(rsp[2]);
 	} else if (mpt->is_sas) {
 		PTR_MPI_TARGET_SSP_CMD_BUFFER ssp =
 		    (PTR_MPI_TARGET_SSP_CMD_BUFFER) cmd_vbuf;
@@ -4795,9 +4777,9 @@ mpt_scsi_tgt_status(struct mpt_softc *mpt, union ccb *ccb, request_t *cmd_req,
 	}
 
 	mpt_lprt(mpt, MPT_PRT_DEBUG, 
-	    "STATUS_CCB %p (wit%s sense) tag %x req %p:%u resid %u\n",
-	    ccb, sense_data?"h" : "hout", ccb? ccb->csio.tag_id : -1, req,
-	    req->serno, tgt->resid);
+	    "STATUS_CCB %p (with%s sense) tag %x req %p:%u resid %u\n",
+	    ccb, sense_len > 0 ? "" : "out", ccb ? ccb->csio.tag_id : -1,
+	    req, req->serno, tgt->resid);
 	if (ccb) {
 		ccb->ccb_h.status = CAM_SIM_QUEUED | CAM_REQ_INPROG;
 		mpt_req_timeout(req, SBT_1S * 60, mpt_timeout, ccb);
@@ -4816,7 +4798,7 @@ mpt_scsi_tgt_tsk_mgmt(struct mpt_softc *mpt, request_t *req, mpt_task_mgmt_t fc,
 	inot = (struct ccb_immediate_notify *) STAILQ_FIRST(&trtp->inots);
 	if (inot == NULL) {
 		mpt_lprt(mpt, MPT_PRT_WARN, "no INOTSs- sending back BSY\n");
-		mpt_scsi_tgt_status(mpt, NULL, req, SCSI_STATUS_BUSY, NULL);
+		mpt_scsi_tgt_status(mpt, NULL, req, SCSI_STATUS_BUSY, NULL, 0);
 		return;
 	}
 	STAILQ_REMOVE_HEAD(&trtp->inots, sim_links.stqe);
@@ -4930,8 +4912,8 @@ mpt_scsi_tgt_atio(struct mpt_softc *mpt, request_t *req, uint32_t reply_desc)
 			default:
 				mpt_prt(mpt, "CORRUPTED TASK MGMT BITS: 0x%x\n",
 				    fc->FcpCntl[2]);
-				mpt_scsi_tgt_status(mpt, 0, req,
-				    SCSI_STATUS_OK, 0);
+				mpt_scsi_tgt_status(mpt, NULL, req,
+				    SCSI_STATUS_OK, NULL, 0);
 				return;
 			}
 		} else {
@@ -5005,23 +4987,21 @@ mpt_scsi_tgt_atio(struct mpt_softc *mpt, request_t *req, uint32_t reply_desc)
 			 * REPORT LUNS gets illegal command.
 			 * All other commands get 'no such device'.
 			 */
-			uint8_t *sp, cond, buf[MPT_SENSE_SIZE];
+			uint8_t sense[MPT_SENSE_SIZE];
 			size_t len;
 
-			memset(buf, 0, MPT_SENSE_SIZE);
-			cond = SCSI_STATUS_CHECK_COND;
-			buf[0] = 0xf0;
-			buf[2] = 0x5;
-			buf[7] = 0x8;
-			sp = buf;
+			memset(sense, 0, sizeof(sense));
+			sense[0] = 0xf0;
+			sense[2] = 0x5;
+			sense[7] = 0x8;
 			tgt->tag_id = MPT_MAKE_TAGID(mpt, req, ioindex);
 
 			switch (cdbp[0]) {
 			case INQUIRY:
 			{
 				if (cdbp[1] != 0) {
-					buf[12] = 0x26;
-					buf[13] = 0x01;
+					sense[12] = 0x26;
+					sense[13] = 0x01;
 					break;
 				}
 				len = min(tgt->resid, cdbp[4]);
@@ -5034,27 +5014,28 @@ mpt_scsi_tgt_atio(struct mpt_softc *mpt, request_t *req, uint32_t reply_desc)
 			}
 			case REQUEST_SENSE:
 			{
-				buf[2] = 0x0;
+				sense[2] = 0x0;
 				len = min(tgt->resid, cdbp[4]);
-				len = min(len, sizeof (buf));
+				len = min(len, sizeof (sense));
 				mpt_lprt(mpt, MPT_PRT_DEBUG,
 				    "local reqsense %ld bytes\n", (long) len);
 				mpt_scsi_tgt_local(mpt, req, lun, 1,
-				    buf, len);
+				    sense, len);
 				return;
 			}
 			case REPORT_LUNS:
 				mpt_lprt(mpt, MPT_PRT_DEBUG, "REPORT LUNS\n");
-				buf[12] = 0x26;
+				sense[12] = 0x26;
 				return;
 			default:
 				mpt_lprt(mpt, MPT_PRT_DEBUG,
 				    "CMD 0x%x to unmanaged lun %jx\n",
 				    cdbp[0], (uintmax_t)lun);
-				buf[12] = 0x25;
+				sense[12] = 0x25;
 				break;
 			}
-			mpt_scsi_tgt_status(mpt, NULL, req, cond, sp);
+			mpt_scsi_tgt_status(mpt, NULL, req,
+			    SCSI_STATUS_CHECK_COND, sense, sizeof(sense));
 			return;
 		}
 		/* otherwise, leave trtp NULL */
@@ -5069,8 +5050,8 @@ mpt_scsi_tgt_atio(struct mpt_softc *mpt, request_t *req, uint32_t reply_desc)
 		if (trtp == NULL) {
 			mpt_prt(mpt, "task mgmt function %x but no listener\n",
 			    fct);
-			mpt_scsi_tgt_status(mpt, 0, req,
-			    SCSI_STATUS_OK, 0);
+			mpt_scsi_tgt_status(mpt, NULL, req,
+			    SCSI_STATUS_OK, NULL, 0);
 		} else {
 			mpt_scsi_tgt_tsk_mgmt(mpt, req, fct, trtp,
 			    GET_INITIATOR_INDEX(reply_desc));
@@ -5086,7 +5067,7 @@ mpt_scsi_tgt_atio(struct mpt_softc *mpt, request_t *req, uint32_t reply_desc)
 		    mpt->tenabled? "QUEUE FULL" : "BUSY");
 		mpt_scsi_tgt_status(mpt, NULL, req,
 		    mpt->tenabled? SCSI_STATUS_QUEUE_FULL : SCSI_STATUS_BUSY,
-		    NULL);
+		    NULL, 0);
 		return;
 	}
 	STAILQ_REMOVE_HEAD(&trtp->atios, sim_links.stqe);
@@ -5098,7 +5079,7 @@ mpt_scsi_tgt_atio(struct mpt_softc *mpt, request_t *req, uint32_t reply_desc)
 	atiop->ccb_h.target_lun = lun;
 	atiop->sense_len = 0;
 	atiop->init_id = GET_INITIATOR_INDEX(reply_desc);
-	atiop->cdb_len = mpt_cdblen(cdbp[0], 16);
+	atiop->cdb_len = 16;
 	memcpy(atiop->cdb_io.cdb_bytes, cdbp, atiop->cdb_len);
 
 	/*
@@ -5179,8 +5160,6 @@ mpt_scsi_tgt_reply_handler(struct mpt_softc *mpt, request_t *req,
 			break;
 		case TGT_STATE_MOVING_DATA:
 		{
-			uint8_t *sp = NULL, sense[MPT_SENSE_SIZE];
-
 			ccb = tgt->ccb;
 			if (tgt->req == NULL) {
 				panic("mpt: turbo target reply with null "
@@ -5200,12 +5179,12 @@ mpt_scsi_tgt_reply_handler(struct mpt_softc *mpt, request_t *req,
 				mpt_free_request(mpt, tgt->req);
 				tgt->req = NULL;
 				mpt_scsi_tgt_status(mpt, NULL, req,
-				    0, NULL);
+				    0, NULL, 0);
 				return (TRUE);
 			}
 			tgt->ccb = NULL;
 			tgt->nxfers++;
-			mpt_req_untimeout(req, mpt_timeout, ccb);
+			mpt_req_untimeout(tgt->req, mpt_timeout, ccb);
 			mpt_lprt(mpt, MPT_PRT_DEBUG,
 			    "TARGET_ASSIST %p (req %p:%u) done tag 0x%x\n",
 			    ccb, tgt->req, tgt->req->serno, ccb->csio.tag_id);
@@ -5241,13 +5220,11 @@ mpt_scsi_tgt_reply_handler(struct mpt_softc *mpt, request_t *req,
 			/*
 			 * Otherwise, send status (and sense)
 			 */
-			if (ccb->ccb_h.flags & CAM_SEND_SENSE) {
-				sp = sense;
-				memcpy(sp, &ccb->csio.sense_data,
-				   min(ccb->csio.sense_len, MPT_SENSE_SIZE));
-			}
 			mpt_scsi_tgt_status(mpt, ccb, req,
-			    ccb->csio.scsi_status, sp);
+			    ccb->csio.scsi_status,
+			    (void *)&ccb->csio.sense_data,
+			    (ccb->ccb_h.flags & CAM_SEND_SENSE) ?
+			     ccb->csio.sense_len : 0);
 			break;
 		}
 		case TGT_STATE_SENDING_STATUS:
@@ -5268,7 +5245,7 @@ mpt_scsi_tgt_reply_handler(struct mpt_softc *mpt, request_t *req,
 				    TGT_STATE_MOVING_DATA_AND_STATUS) {
 					tgt->nxfers++;
 				}
-				mpt_req_untimeout(req, mpt_timeout, ccb);
+				mpt_req_untimeout(tgt->req, mpt_timeout, ccb);
 				if (ccb->ccb_h.flags & CAM_SEND_SENSE) {
 					ccb->ccb_h.status |= CAM_SENT_SENSE;
 				}