PERFORCE change 163876 for review

Alexander Motin mav at FreeBSD.org
Tue Jun 9 09:47:28 UTC 2009


http://perforce.freebsd.org/chv.cgi?CH=163876

Change 163876 by mav at mav_mavbook on 2009/06/09 09:47:10

	Implement proper command ordering enforcement for the worst case of
	AHCI specification, when FIS based swhitching is not supported, but
	PM is present and NCQ commands to different devices are intermixed,
	and mixed with untagged commands. When collision condition
	detected, simq freezed and CCB saved for later, until all active
	requests processed or error detected. 
	I have decided not to do it on upper layers, as some of this
	limitations are AHCI specific, and better controllers able to
	do all of this in silicon.

Affected files ...

.. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#20 edit
.. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.h#8 edit

Differences ...

==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#20 (text+ko) ====

@@ -66,7 +66,7 @@
 static int ahci_ctlr_reset(device_t dev);
 static void ahci_begin_transaction(device_t dev, union ccb *ccb);
 static void ahci_dmasetprd(void *arg, bus_dma_segment_t *segs, int nsegs, int error);
-static void ahci_execute_command(struct ahci_slot *slot);
+static void ahci_execute_transaction(struct ahci_slot *slot);
 static void ahci_timeout(struct ahci_slot *slot);
 static void ahci_end_transaction(struct ahci_slot *slot, enum ahci_err_type et);
 static int ahci_hardreset(device_t dev, int port, uint32_t *signature);
@@ -708,7 +708,7 @@
 	    NULL, NULL,
 	    ch->dma.max_iosize * AHCI_MAX_SLOTS,
 	    AHCI_DMA_ENTRIES, ch->dma.segsize,
-	    0, NULL, NULL, &ch->dma.data_tag)) {
+	    0, busdma_lock_mutex, &ch->mtx, &ch->dma.data_tag)) {
 		goto error;
 	}
 	return;
@@ -882,6 +882,14 @@
 		res = ch->rslots & ~(cstatus | sstatus);
 		err = 0;
 	}
+	/* On error, requeue frozen command. */
+	if (err && ch->frozen) {
+		union ccb *fccb = ch->frozen;
+		ch->frozen = NULL;
+		xpt_release_simq(ch->sim, TRUE);
+		fccb->ccb_h.status = CAM_REQUEUE_REQ;
+		xpt_done(fccb);
+	}
 	/* Check all slots. */
 	for (i = 0; i < AHCI_MAX_SLOTS; i++) {
 		/* Do we have an event on slot? */
@@ -906,6 +914,36 @@
 	mtx_unlock(&ch->mtx);
 }
 
+/* Must be called with channel locked. */
+static int
+ahci_check_collision(device_t dev, union ccb *ccb)
+{
+	struct ahci_channel *ch = device_get_softc(dev);
+
+	if ((ccb->ccb_h.func_code == XPT_ATA_IO) &&
+	    (ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA)) {
+		/* Tagged command while untagged are active. */
+		if (ch->numrslots != 0 && ch->numtslots == 0)
+			return (1);
+		/* Tagged command while tagged to other target is active. */
+		if (ch->numtslots != 0 &&
+		    ch->taggedtarget != ccb->ccb_h.target_id)
+			return (1);
+	} else {
+		/* Untagged command while tagged are active. */
+		if (ch->numrslots != 0 && ch->numtslots != 0)
+			return (1);
+	}
+	if ((ccb->ccb_h.func_code == XPT_ATA_IO) &&
+	    (ccb->ataio.cmd.flags & CAM_ATAIO_CONTROL)) {
+		/* Control command while anything active. */
+		if (ch->numrslots != 0)
+			return (1);
+	}
+	return (0);
+}
+
+/* Must be called with channel locked. */
 static void
 ahci_begin_transaction(device_t dev, union ccb *ccb)
 {
@@ -913,6 +951,7 @@
 	struct ahci_slot *slot;
 	int tag;
 
+	/* Choose empty slot. */
 	tag = ch->lastslot;
 	do {
 		tag++;
@@ -924,15 +963,21 @@
 	if (tag == ch->lastslot)
 		device_printf(ch->dev, "ALL SLOTS BUSY!\n");
 	ch->lastslot = tag;
-
+	/* Occupy chosen slot. */
 	slot = &ch->slot[tag];
 //device_printf(slot->dev, "%s slot %d\n", __func__, slot->slot);
-	slot->state = AHCI_SLOT_TAKEN;
 	slot->ccb = ccb;
 	slot->dma.nsegs = 0;
-
-	/* if request moves data setup and load SG list */
+	/* Update channel stats. */
+	ch->numrslots++;
+	if ((slot->ccb->ccb_h.func_code == XPT_ATA_IO) &&
+	    (slot->ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA)) {
+		ch->numtslots++;
+		ch->taggedtarget = ccb->ccb_h.target_id;
+	}
+	/* If request moves data, setup and load SG list */
 	if ((ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE) {
+		slot->state = AHCI_SLOT_LOADING;
 		if (ccb->ccb_h.func_code == XPT_ATA_IO) {
 			if (bus_dmamap_load(ch->dma.data_tag, slot->dma.data_map,
 			    ccb->ataio.data_ptr, ccb->ataio.dxfer_len,
@@ -946,12 +991,11 @@
 				device_printf(dev, "FAILURE - load data\n");
 			}
 		}
-		return;
-	}
-
-	ahci_execute_command(slot);
+	} else
+		ahci_execute_transaction(slot);
 }
 
+/* Locked by busdma engine. */
 void
 ahci_dmasetprd(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
 {    
@@ -986,52 +1030,53 @@
 	    ((slot->ccb->ccb_h.flags & CAM_DIR_IN) ?
 	    BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE));
 
-	ahci_execute_command(slot);
+	ahci_execute_transaction(slot);
 }
 
+/* Must be called with channel locked. */
 static void
-ahci_execute_command(struct ahci_slot *slot)
+ahci_execute_transaction(struct ahci_slot *slot)
 {
-	struct ahci_channel *ch = device_get_softc(slot->dev);
+	device_t dev = slot->dev;
+	struct ahci_channel *ch = device_get_softc(dev);
 	struct ahci_cmd_tab *ctp;
 	struct ahci_cmd_list *clp;
-	int port = slot->ccb->ccb_h.target_id & 0x0f;
+	union ccb *ccb = slot->ccb;
+	int port = ccb->ccb_h.target_id & 0x0f;
 	int fis_size;
 
-//device_printf(slot->dev, "%s slot %d\n", __func__, slot->slot);
-	slot->state = AHCI_SLOT_LOADED;
+//device_printf(dev, "%s slot %d\n", __func__, slot->slot);
 	/* Get a piece of the workspace for this request */
 	ctp = (struct ahci_cmd_tab *)
 		(ch->dma.work + AHCI_CT_OFFSET + (AHCI_CT_SIZE * slot->slot));
 	/* Setup the FIS for this request */
-	if (!(fis_size = ahci_setup_fis(ctp, slot->ccb, slot->slot))) {
+	if (!(fis_size = ahci_setup_fis(ctp, ccb, slot->slot))) {
 		device_printf(ch->dev, "setting up SATA FIS failed\n");
-		slot->ccb->ccb_h.status = CAM_REQ_INVALID;
-		xpt_done(slot->ccb);
+		ccb->ccb_h.status = CAM_REQ_INVALID;
+		xpt_done(ccb);
 		return;
 	}
 	/* Setup the command list entry */
 	clp = (struct ahci_cmd_list *)
 	    (ch->dma.work + AHCI_CL_OFFSET + (AHCI_CL_SIZE * slot->slot));
 	clp->prd_length = slot->dma.nsegs;
-	clp->cmd_flags = (slot->ccb->ccb_h.flags & CAM_DIR_OUT ? AHCI_CMD_WRITE : 0) |
-		     (slot->ccb->ccb_h.func_code == XPT_SCSI_IO ?
+	clp->cmd_flags = (ccb->ccb_h.flags & CAM_DIR_OUT ? AHCI_CMD_WRITE : 0) |
+		     (ccb->ccb_h.func_code == XPT_SCSI_IO ?
 		      (AHCI_CMD_ATAPI | AHCI_CMD_PREFETCH) : 0) |
 		     (fis_size / sizeof(u_int32_t)) |
 		     (port << 12);
 	/* Special handling for Soft Reset command. */
-	if ((slot->ccb->ccb_h.func_code == XPT_ATA_IO) &&
-	    (slot->ccb->ataio.cmd.flags & CAM_ATAIO_CONTROL) &&
-	    slot->ccb->ataio.cmd.control & ATA_A_RESET) {
+	if ((ccb->ccb_h.func_code == XPT_ATA_IO) &&
+	    (ccb->ataio.cmd.flags & CAM_ATAIO_CONTROL) &&
+	    ccb->ataio.cmd.control & ATA_A_RESET) {
 		clp->cmd_flags |= AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY;
 	}
 	clp->bytecount = 0;
 	clp->cmd_table_phys = htole64(ch->dma.work_bus + AHCI_CT_OFFSET +
 				  (AHCI_CT_SIZE * slot->slot));
 	/* Set ACTIVE bit for NCQ commands. */
-	if ((slot->ccb->ccb_h.func_code == XPT_ATA_IO) &&
-	    (slot->ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA)) {
-		ch->aslots |= (1 << slot->slot);
+	if ((ccb->ccb_h.func_code == XPT_ATA_IO) &&
+	    (ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA)) {
 		ATA_OUTL(ch->r_mem, AHCI_P_SACT, 1 << slot->slot);
 	}
 	/* Issue command to the controller. */
@@ -1054,11 +1099,11 @@
     ATA_INL(ch->r_mem, AHCI_P_SIG));
 */
 	/* Device reset commands doesn't interrupt. Poll them. */
-	if (slot->ccb->ccb_h.func_code == XPT_ATA_IO &&
-	    (slot->ccb->ataio.cmd.command == ATA_DEVICE_RESET ||
-	    (slot->ccb->ataio.cmd.flags & CAM_ATAIO_CONTROL))) {
+	if (ccb->ccb_h.func_code == XPT_ATA_IO &&
+	    (ccb->ataio.cmd.command == ATA_DEVICE_RESET ||
+	    (ccb->ataio.cmd.flags & CAM_ATAIO_CONTROL))) {
 		u_int32_t status;
-		int count, timeout = slot->ccb->ccb_h.timeout;
+		int count, timeout = ccb->ccb_h.timeout;
 		enum ahci_err_type et = AHCI_ERR_NONE;
 
 		for (count = 0; count < timeout; count++) {
@@ -1080,7 +1125,7 @@
 	}
 
 	/* start the timeout */
-	callout_reset(&slot->timeout, (int)slot->ccb->ccb_h.timeout * hz / 1000,
+	callout_reset(&slot->timeout, (int)ccb->ccb_h.timeout * hz / 1000,
 	    (timeout_t*)ahci_timeout, slot);
 	return;
 }
@@ -1095,6 +1140,13 @@
 	int i;
 
 device_printf(dev, "Timeout on slot %d\n", slot->slot);
+	if (ch->frozen) {
+		union ccb *fccb = ch->frozen;
+		ch->frozen = NULL;
+		xpt_release_simq(ch->sim, TRUE);
+		fccb->ccb_h.status = CAM_SCSI_BUS_RESET;
+		xpt_done(fccb);
+	}
 	ahci_stop(dev);
 	for (i = 0; i < AHCI_MAX_SLOTS; i++) {
 		/* Do we have a running request on slot? */
@@ -1115,14 +1167,15 @@
 {
 	device_t dev = slot->dev;
 	struct ahci_channel *ch = device_get_softc(dev);
+	union ccb *ccb = slot->ccb;
 //	struct ahci_cmd_list *clp;
 
 //device_printf(dev, "%s slot %d\n", __func__, slot->slot);
 	/* Kill the timeout */
 	callout_stop(&slot->timeout);
 	/* Read registers to the result struct */
-	if (slot->ccb->ccb_h.func_code == XPT_ATA_IO) {
-		struct ata_res *res = &slot->ccb->ataio.res;
+	if (ccb->ccb_h.func_code == XPT_ATA_IO) {
+		struct ata_res *res = &ccb->ataio.res;
 		u_int8_t *fis = ch->dma.work + AHCI_FB_OFFSET + 0x40;
 
 		res->status = fis[2];
@@ -1143,45 +1196,58 @@
 	    (ch->dma.work + AHCI_CL_OFFSET + (AHCI_CL_SIZE * slot->slot));
 	request->donecount = clp->bytecount;
 #endif
-	if ((slot->ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE) {
+	if ((ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE) {
 		bus_dmamap_sync(slot->dma.sg_tag, slot->dma.sg_map,
 		    BUS_DMASYNC_POSTWRITE);
 		bus_dmamap_sync(ch->dma.data_tag, slot->dma.data_map,
-		    (slot->ccb->ccb_h.flags & CAM_DIR_IN) ?
+		    (ccb->ccb_h.flags & CAM_DIR_IN) ?
 		    BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
 		bus_dmamap_unload(ch->dma.data_tag, slot->dma.data_map);
 	}
 	/* Set proper result status. */
 	switch (et) {
 	case AHCI_ERR_NONE:
-		slot->ccb->ccb_h.status = CAM_REQ_CMP;
-		if (slot->ccb->ccb_h.func_code == XPT_SCSI_IO)
-			slot->ccb->csio.scsi_status = SCSI_STATUS_OK;
+		ccb->ccb_h.status = CAM_REQ_CMP;
+		if (ccb->ccb_h.func_code == XPT_SCSI_IO)
+			ccb->csio.scsi_status = SCSI_STATUS_OK;
 		break;
 	case AHCI_ERR_REAL:
-		if (slot->ccb->ccb_h.func_code == XPT_SCSI_IO) {
-			slot->ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR;
-			slot->ccb->csio.scsi_status = SCSI_STATUS_CHECK_COND;
+		if (ccb->ccb_h.func_code == XPT_SCSI_IO) {
+			ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR;
+			ccb->csio.scsi_status = SCSI_STATUS_CHECK_COND;
 		} else
-			slot->ccb->ccb_h.status = CAM_REQ_CMP_ERR;
+			ccb->ccb_h.status = CAM_REQ_CMP_ERR;
 		break;
 	case AHCI_ERR_BTW:
-		slot->ccb->ccb_h.status = CAM_REQUEUE_REQ;
+		ccb->ccb_h.status = CAM_REQUEUE_REQ;
 		break;
 	case AHCI_ERR_RESET:
-		slot->ccb->ccb_h.status = CAM_SCSI_BUS_RESET;
+		ccb->ccb_h.status = CAM_SCSI_BUS_RESET;
 		break;
 	case AHCI_ERR_TIMEOUT:
-		slot->ccb->ccb_h.status = CAM_CMD_TIMEOUT;
+		ccb->ccb_h.status = CAM_CMD_TIMEOUT;
 		break;
 	default:
-		slot->ccb->ccb_h.status = CAM_REQ_CMP_ERR;
+		ccb->ccb_h.status = CAM_REQ_CMP_ERR;
 	}
-	xpt_done(slot->ccb);
+	/* Free slot. */
 	ch->rslots &= ~(1 << slot->slot);
 	slot->state = AHCI_SLOT_EMPTY;
 	slot->ccb = NULL;
-	return;
+	/* Update channel stats. */
+	ch->numrslots--;
+	if ((ccb->ccb_h.func_code == XPT_ATA_IO) &&
+	    (ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA)) {
+		ch->numtslots--;
+	}
+	xpt_done(ccb);
+	if (ch->frozen && ch->numrslots == 0) {
+		union ccb *fccb = ch->frozen;
+//device_printf(dev, "Unfreeze\n");
+		ch->frozen = NULL;
+		ahci_begin_transaction(dev, fccb);
+		xpt_release_simq(ch->sim, TRUE);
+	}
 }
 
 static void
@@ -1464,12 +1530,14 @@
 static void
 ahciaction(struct cam_sim *sim, union ccb *ccb)
 {
+	device_t dev;
 	struct ahci_channel *ch;
 
-	CAM_DEBUG(ccb->ccb_h.path, CAM_DEBUG_TRACE, ("ahciaction func_codeL%d\n",
+	CAM_DEBUG(ccb->ccb_h.path, CAM_DEBUG_TRACE, ("ahciaction func_code=%x\n",
 	    ccb->ccb_h.func_code));
 
 	ch = (struct ahci_channel *)cam_sim_softc(sim);
+	dev = ch->dev;
 //device_printf(ch->dev, "ccb func 0x%x %d:%d\n", ccb->ccb_h.func_code,  ccb->ccb_h.target_id, ccb->ccb_h.target_lun);
 	switch (ccb->ccb_h.func_code) {
 	/* Common cases first */
@@ -1480,7 +1548,16 @@
 			xpt_done(ccb);
 			break;
 		}
-		ahci_begin_transaction(ch->dev, ccb);
+		/* Check for command collision. */
+		if (ahci_check_collision(dev, ccb)) {
+			/* Freeze command. */
+//device_printf(dev, "Freeze\n");
+			/* We have only one frozen slot, so freeze simq also. */
+			xpt_freeze_simq(ch->sim, 1);
+			ch->frozen = ccb;
+			return;
+		}
+		ahci_begin_transaction(dev, ccb);
 		break;
 	case XPT_EN_LUN:		/* Enable LUN as a target */
 	case XPT_TARGET_IO:		/* Execute target I/O request */
@@ -1555,7 +1632,7 @@
 #endif
 	case XPT_RESET_BUS:		/* Reset the specified SCSI bus */
 	case XPT_RESET_DEV:	/* Bus Device Reset the specified SCSI device */
-		ahci_reset(ch->dev);
+		ahci_reset(dev);
 		ccb->ccb_h.status = CAM_REQ_CMP;
 		xpt_done(ccb);
 		break;

==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.h#8 (text+ko) ====

@@ -329,8 +329,7 @@
 
 enum ahci_slot_states {
 	AHCI_SLOT_EMPTY,
-	AHCI_SLOT_TAKEN,
-	AHCI_SLOT_LOADED,
+	AHCI_SLOT_LOADING,
 	AHCI_SLOT_RUNNING,
 	AHCI_SLOT_WAITING
 };
@@ -377,8 +376,11 @@
 	
 	struct ahci_slot	slot[AHCI_MAX_SLOTS];
 	uint32_t		rslots;		/* Running slots */
-	uint32_t		aslots;		/* SACTIVE slots */
+	int			numrslots;	/* Number of running slots */
+	int			numtslots;	/* Number of tagged slots */
 	int			lastslot;	/* Last used slot */
+	int			taggedtarget;	/* Last tagged target */
+	union ccb		*frozen;	/* Frozen command */
 };
 
 /* structure describing a AHCI controller */


More information about the p4-projects mailing list