Overhaul error handling in the IPS driver. Don't use a magic value for

driver-induced errors, instead be better about propagating error status
upwards.  Add more error definitions, courtesy of the linux driver.  Fix
a command leak in the ioctl handler.  Re-arrange some of the command handlers
to localize error handling.

MFC After: 3 days

sys/dev/ips/ips.c

@@ -308,7 +308,7 @@ static void ips_timeout(void *arg)
 				sc->state |= IPS_TIMEOUT;
 				device_printf(sc->dev, "WARNING: command timeout. Adapter is in toaster mode, resetting to known state\n");
 			}
-			command[i].status.value = IPS_ERROR_STATUS;
+			ips_set_error(&command[i], ETIMEDOUT);
 			command[i].callback(&command[i]);
 			/* hmm, this should be enough cleanup */
 		} else
@@ -554,7 +554,7 @@ void ips_issue_morpheus_cmd(ips_command_t *command)
 {
 	/* hmmm, is there a cleaner way to do this? */
 	if(command->sc->state & IPS_OFFLINE){
-		command->status.value = IPS_ERROR_STATUS;
+		ips_set_error(command, EINVAL);
 		command->callback(command);
 		return;
 	}
@@ -722,7 +722,7 @@ void ips_issue_copperhead_cmd(ips_command_t *command)
 	int i;
 	/* hmmm, is there a cleaner way to do this? */
 	if(command->sc->state & IPS_OFFLINE){
-		command->status.value = IPS_ERROR_STATUS;
+		ips_set_error(command, EINVAL);
 		command->callback(command);
 		return;
 	}

sys/dev/ips/ips.h

@@ -150,9 +150,29 @@ MALLOC_DECLARE(M_IPSBUF);
 #define IPS_RW_NVRAM_CMD		0xBC
 #define IPS_FFDC_CMD			0xD7
 
-/* error information returned by the adapter */
+/* basic_status information returned by the adapter */
 #define IPS_MIN_ERROR			0x02
-#define IPS_ERROR_STATUS		0x13000200 /* ahh, magic numbers */
+#define IPS_BASIC_STATUS_MASK		0xFF
+#define IPS_GSC_STATUS_MASK		0x0F
+#define IPS_CMD_SUCCESS			0x00
+#define IPS_CMD_RECOVERED_ERROR		0x01
+#define IPS_DRV_ERROR			0x02	/* Driver supplied error */
+#define IPS_INVAL_OPCO			0x03
+#define IPS_INVAL_CMD_BLK		0x04
+#define IPS_INVAL_PARM_BLK		0x05
+#define IPS_BUSY			0x08
+#define IPS_CMD_CMPLT_WERROR		0x0C
+#define IPS_LD_ERROR			0x0D
+#define IPS_CMD_TIMEOUT			0x0E
+#define IPS_PHYS_DRV_ERROR		0x0F
+
+/* extended_status information returned by the adapter */
+#define IPS_ERR_SEL_TO			0xF0
+#define IPS_ERR_OU_RUN			0xF2
+#define IPS_ERR_HOST_RESET		0xF7
+#define IPS_ERR_DEV_RESET		0xF8
+#define IPS_ERR_RECOVERY		0xFC
+#define IPS_ERR_CKCOND			0xFF
 
 #define IPS_OS_FREEBSD			8
 #define IPS_VERSION_MAJOR		"0.90"
@@ -207,7 +227,12 @@ MALLOC_DECLARE(M_IPSBUF);
 
 #define ips_read_request(iobuf)		((iobuf)->bio_cmd == BIO_READ)
 
-#define COMMAND_ERROR(status)		(((status)->fields.basic_status & 0x0f) >= IPS_MIN_ERROR)
+#define COMMAND_ERROR(command)		(((command)->status.fields.basic_status & IPS_GSC_STATUS_MASK) >= IPS_MIN_ERROR)
+
+#define ips_set_error(command, error)	do {				\
+	(command)->status.fields.basic_status = IPS_DRV_ERROR;		\
+	(command)->status.fields.reserved = ((error) & 0x0f);		\
+} while (0);
 
 #ifndef IPS_DEBUG
 #define DEVICE_PRINTF(x...)
@@ -216,6 +241,7 @@ MALLOC_DECLARE(M_IPSBUF);
 #define DEVICE_PRINTF(level,x...)	if(IPS_DEBUG >= level)device_printf(x)
 #define PRINTF(level,x...)		if(IPS_DEBUG >= level)printf(x)
 #endif
+
 /*
  *   IPS STRUCTS
  */

sys/dev/ips/ips_commands.c

@@ -59,9 +59,10 @@ static void ips_io_request_finish(ips_command_t *command)
 				BUS_DMASYNC_POSTWRITE);
 	}
 	bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		iobuf->bio_flags |=BIO_ERROR;
 		iobuf->bio_error = EIO;
+		printf("ips: io error, status= %d\n", command->status.value);
 	}
 	ips_insert_free_cmd(command->sc, command);
 	ipsd_finish(iobuf);
@@ -174,8 +175,7 @@ static void ips_adapter_info_callback(void *cmdptr, bus_dma_segment_t *segments,
 	ips_adapter_info_cmd *command_struct;
 	sc = command->sc;
 	if(error){
-		command->status.value = IPS_ERROR_STATUS; /* a lovely error value */
-		ips_insert_free_cmd(sc, command);
+		ips_set_error(command, error);
 		printf("ips: error = %d in ips_get_adapter_info\n", error);
 		return;
 	}
@@ -189,6 +189,14 @@ static void ips_adapter_info_callback(void *cmdptr, bus_dma_segment_t *segments,
 	bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
 			BUS_DMASYNC_PREREAD);
 	sc->ips_issue_cmd(command);
+	if (sema_timedwait(&sc->cmd_sema, 30*hz) != 0) {
+		ips_set_error(command, ETIMEDOUT);
+		return;
+	}
+
+	bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
+			BUS_DMASYNC_POSTREAD);
+	memcpy(&(sc->adapter_info), command->data_buffer, IPS_ADAPTER_INFO_LEN);
 }
 
 
@@ -222,21 +230,13 @@ static int ips_send_adapter_info_cmd(ips_command_t *command)
 		goto exit;
 	}
 	command->callback = ips_wakeup_callback;
-	bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
-			command->data_buffer,IPS_ADAPTER_INFO_LEN, 
-			ips_adapter_info_callback, command, BUS_DMA_NOWAIT);
+	error = bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
+				command->data_buffer,IPS_ADAPTER_INFO_LEN, 
+				ips_adapter_info_callback, command,
+				BUS_DMA_NOWAIT);
 
-	if ((command->status.value == IPS_ERROR_STATUS) ||
-	    (sema_timedwait(&sc->cmd_sema, 30*hz) != 0))
-		error = ETIMEDOUT;
-
-	if (error == 0) {
-		bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
-				BUS_DMASYNC_POSTREAD);
-		memcpy(&(sc->adapter_info), command->data_buffer, 
-			IPS_ADAPTER_INFO_LEN);
-	}
-	bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
+	if (error == 0)
+		bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
 
 exit:
 	/* I suppose I should clean up my memory allocations */
@@ -257,7 +257,7 @@ int ips_get_adapter_info(ips_softc_t *sc)
 		return ENXIO;
 	}
 	ips_send_adapter_info_cmd(command);
-	if (COMMAND_ERROR(&command->status)){
+	if (COMMAND_ERROR(command)){
 		error = ENXIO;
 	}
 	return error;
@@ -272,10 +272,11 @@ static void ips_drive_info_callback(void *cmdptr, bus_dma_segment_t *segments,in
 	ips_softc_t *sc;
 	ips_command_t *command = cmdptr;
 	ips_drive_cmd *command_struct;
+	ips_drive_info_t *driveinfo;
+
 	sc = command->sc;
 	if(error){
-                command->status.value = IPS_ERROR_STATUS;
-		ips_insert_free_cmd(sc, command);
+		ips_set_error(command, error);
 		printf("ips: error = %d in ips_get_drive_info\n", error);
 		return;
 	}
@@ -289,13 +290,23 @@ static void ips_drive_info_callback(void *cmdptr, bus_dma_segment_t *segments,in
 	bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
 			BUS_DMASYNC_PREREAD);
 	sc->ips_issue_cmd(command);
+	if (sema_timedwait(&sc->cmd_sema, 10*hz) != 0) {
+		ips_set_error(command, ETIMEDOUT);
+		return;
+	}
+
+	bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
+			BUS_DMASYNC_POSTREAD);
+	driveinfo = command->data_buffer;
+	memcpy(sc->drives, driveinfo->drives, sizeof(ips_drive_t) * 8);	
+	sc->drivecount = driveinfo->drivecount;
+	device_printf(sc->dev, "logical drives: %d\n",sc->drivecount);
 }
 
 static int ips_send_drive_info_cmd(ips_command_t *command)
 {
 	int error = 0;
 	ips_softc_t *sc = command->sc;
-	ips_drive_info_t *driveinfo;
 
 	if (bus_dma_tag_create(	/* parent    */	sc->adapter_dmatag,
 				/* alignemnt */	1,
@@ -321,22 +332,12 @@ static int ips_send_drive_info_cmd(ips_command_t *command)
 		goto exit;
 	}
 	command->callback = ips_wakeup_callback;
-	bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
-			command->data_buffer,IPS_DRIVE_INFO_LEN, 
-			ips_drive_info_callback, command, BUS_DMA_NOWAIT);
-	if ((command->status.value == IPS_ERROR_STATUS) ||
-	    (sema_timedwait(&sc->cmd_sema, 10*hz) != 0))
-		error = ETIMEDOUT;
-
-	if (error == 0) {
-		bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
-				BUS_DMASYNC_POSTREAD);
-		driveinfo = command->data_buffer;
-		memcpy(sc->drives, driveinfo->drives, sizeof(ips_drive_t) * 8);	
-		sc->drivecount = driveinfo->drivecount;
-		device_printf(sc->dev, "logical drives: %d\n",sc->drivecount);
-	}
-	bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
+	error = bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
+				command->data_buffer,IPS_DRIVE_INFO_LEN, 
+				ips_drive_info_callback, command,
+				BUS_DMA_NOWAIT);
+	if (error == 0)
+		bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
 
 exit:
 	/* I suppose I should clean up my memory allocations */
@@ -357,7 +358,7 @@ int ips_get_drive_info(ips_softc_t *sc)
 		return ENXIO;
 	}
 	ips_send_drive_info_cmd(command);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		error = ENXIO;
 	}
 	return error;
@@ -378,7 +379,7 @@ static int ips_send_flush_cache_cmd(ips_command_t *command)
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_PREWRITE);
 	sc->ips_issue_cmd(command);
-	if (command->status.value != IPS_ERROR_STATUS)
+	if (COMMAND_ERROR(command) == 0)
 		sema_wait(&sc->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
@@ -393,7 +394,7 @@ int ips_flush_cache(ips_softc_t *sc)
 		device_printf(sc->dev, "ERROR: unable to get a command! can't flush cache!\n");
 	}
 	ips_send_flush_cache_cmd(command);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		device_printf(sc->dev, "ERROR: cache flush command failed!\n");
 	}
 	return 0;
@@ -459,7 +460,7 @@ static int ips_send_ffdc_reset_cmd(ips_command_t *command)
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap,
 			BUS_DMASYNC_PREWRITE);
 	sc->ips_issue_cmd(command);
-	if (command->status.value != IPS_ERROR_STATUS)
+	if (COMMAND_ERROR(command) == 0)
 		sema_wait(&sc->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
@@ -473,7 +474,7 @@ int ips_ffdc_reset(ips_softc_t *sc)
 		device_printf(sc->dev, "ERROR: unable to get a command! can't send ffdc reset!\n");
 	}
 	ips_send_ffdc_reset_cmd(command);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		device_printf(sc->dev, "ERROR: ffdc reset command failed!\n");
 	}
 	return 0;
@@ -512,8 +513,7 @@ static void ips_read_nvram_callback(void *cmdptr, bus_dma_segment_t *segments,in
 	ips_rw_nvram_cmd *command_struct;
 	sc = command->sc;
 	if(error){
-                command->status.value = IPS_ERROR_STATUS;
-		ips_insert_free_cmd(sc, command);
+		ips_set_error(command, error);
 		printf("ips: error = %d in ips_read_nvram_callback\n", error);
 		return;
 	}
@@ -529,6 +529,12 @@ static void ips_read_nvram_callback(void *cmdptr, bus_dma_segment_t *segments,in
 	bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
 			BUS_DMASYNC_PREREAD);
 	sc->ips_issue_cmd(command);
+	if (sema_timedwait(&sc->cmd_sema, 30*hz) != 0) {
+		ips_set_error(command, ETIMEDOUT);
+		return;
+	}
+	bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
+			BUS_DMASYNC_POSTWRITE);
 }
 
 static int ips_read_nvram(ips_command_t *command)
@@ -560,18 +566,12 @@ static int ips_read_nvram(ips_command_t *command)
 		goto exit;
 	}
 	command->callback = ips_write_nvram;
-	bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
-			command->data_buffer,IPS_NVRAM_PAGE_SIZE, 
-			ips_read_nvram_callback, command, BUS_DMA_NOWAIT);
-	if ((command->status.value == IPS_ERROR_STATUS) ||
-	    (sema_timedwait(&sc->cmd_sema, 30*hz) != 0))
-		error = ETIMEDOUT;
-
-	if (error == 0) {
-		bus_dmamap_sync(command->data_dmatag, command->data_dmamap, 
-				BUS_DMASYNC_POSTWRITE);
-	}
-	bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
+	error = bus_dmamap_load(command->data_dmatag, command->data_dmamap, 
+				command->data_buffer,IPS_NVRAM_PAGE_SIZE, 
+				ips_read_nvram_callback, command,
+				BUS_DMA_NOWAIT);
+	if (error == 0)
+		bus_dmamap_unload(command->data_dmatag, command->data_dmamap);
 
 exit:
 	bus_dmamem_free(command->data_dmatag, command->data_buffer, 
@@ -590,7 +590,7 @@ int ips_update_nvram(ips_softc_t *sc)
 		return 1;
 	}
 	ips_read_nvram(command);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		device_printf(sc->dev, "ERROR: nvram update command failed!\n");
 	}
 	return 0;
@@ -613,7 +613,7 @@ static int ips_send_config_sync_cmd(ips_command_t *command)
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_PREWRITE);
 	sc->ips_issue_cmd(command);
-	if (command->status.value != IPS_ERROR_STATUS)
+	if (COMMAND_ERROR(command) == 0)
 		sema_wait(&sc->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
@@ -633,7 +633,7 @@ static int ips_send_error_table_cmd(ips_command_t *command)
 	bus_dmamap_sync(sc->command_dmatag, command->command_dmamap, 
 			BUS_DMASYNC_PREWRITE);
 	sc->ips_issue_cmd(command);
-	if (command->status.value != IPS_ERROR_STATUS)
+	if (COMMAND_ERROR(command) == 0)
 		sema_wait(&sc->cmd_sema);
 	ips_insert_free_cmd(sc, command);
 	return 0;
@@ -650,7 +650,7 @@ int ips_clear_adapter(ips_softc_t *sc)
 		return 1;
 	}
 	ips_send_config_sync_cmd(command);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		device_printf(sc->dev, "ERROR: cache sync command failed!\n");
 		return 1;
 	}
@@ -661,7 +661,7 @@ int ips_clear_adapter(ips_softc_t *sc)
 		return 1;
 	}
 	ips_send_error_table_cmd(command);
-	if(COMMAND_ERROR(&command->status)){
+	if(COMMAND_ERROR(command)){
 		device_printf(sc->dev, "ERROR: etable command failed!\n");
 		return 1;
 	}

sys/dev/ips/ips_disk.c

@@ -222,7 +222,7 @@ ipsd_dump(void *arg, void *virtual, vm_offset_t physical, off_t offset,
 			error = EIO;
 			break;
 		}
-		if (COMMAND_ERROR(&command->status)) {
+		if (COMMAND_ERROR(command)) {
 			error = EIO;
 			break;
 		}
@@ -251,7 +251,7 @@ ipsd_dump_map_sg(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
 
 	if (error) {
 		printf("ipsd_dump_map_sg: error %d\n", error);
-		command->status.value = IPS_ERROR_STATUS;
+		ips_set_error(command, error);
 		return;
 	}
 
@@ -292,7 +292,7 @@ static void
 ipsd_dump_block_complete(ips_command_t *command)
 {
 
-	if (COMMAND_ERROR(&command->status))
+	if (COMMAND_ERROR(command))
 		printf("ipsd_dump completion error= 0x%x\n",
 		    command->status.value);
 

sys/dev/ips/ips_ioctl.c

@@ -54,8 +54,7 @@ static void ips_ioctl_callback(void *cmdptr, bus_dma_segment_t *segments,int seg
 	ips_ioctl_t *ioctl_cmd = command->arg;
 	ips_generic_cmd *command_buffer = command->command_buffer;
 	if(error){
-		ioctl_cmd->status.value = IPS_ERROR_STATUS;
-		ips_insert_free_cmd(command->sc, command);
+		ips_set_error(command, error);
 		return;
 	}
 	command_buffer->id = command->id;
@@ -123,7 +122,7 @@ static int ips_ioctl_cmd(ips_softc_t *sc, ips_ioctl_t *ioctl_cmd, ips_user_reque
 	ips_ioctl_start(command);
 	while( ioctl_cmd->status.value == 0xffffffff)
 		msleep(ioctl_cmd, &sc->queue_mtx, 0, "ips", hz/10);
-	if(COMMAND_ERROR(&ioctl_cmd->status))
+	if(COMMAND_ERROR(ioctl_cmd))
 		error = EIO;
 	else
 		error = 0;
@@ -131,6 +130,10 @@ static int ips_ioctl_cmd(ips_softc_t *sc, ips_ioctl_t *ioctl_cmd, ips_user_reque
 	if(copyout(ioctl_cmd->data_buffer, user_request->data_buffer,
 	    ioctl_cmd->datasize))
 		error = EINVAL;
+	mtx_lock(&sc->queue_mtx);
+	ips_insert_free_cmd(sc, command);
+	mtx_unlock(&sc->queue_mtx);
+
 exit:	bus_dmamem_free(ioctl_cmd->dmatag, ioctl_cmd->data_buffer,
 			ioctl_cmd->dmamap);
 	bus_dma_tag_destroy(ioctl_cmd->dmatag);