SCSI changes - PR58452 for review

[Nathanial Sloss <nathanialsloss%yahoo.com.au@localhost>]

HI,

I am looking to get the following patch reviewed.

It is also documented in PR/58452.

It does the following:

Abort current transfer on scsi bus reset.  This one seems logical as the state 
of the transfer is unknown after a bus reset.

Uses retry path for medium errors.  Bluescsi-v2 would through medium errors 
that would succeed if the transfer was retried.

Biggest change is for ncr5380 which is to stop scsipi_done from being 
reentrant in 5380sbc_done,  This helps with aborting transfers something which 
is common with dse(4). which previously if nested to deep in recursive calls 
to scsipi_done would cause a panic.

Finally to downgrade the Aborting 0/0/0 messages to a debug only option as 
they are harmless and no longer a warning that requires user intervention or 
concern.

I'll wait another two weeks before commiting anything should issues arise.

Best regards,

Nat

diff -r b6eba4068585 sys/dev/ic/ncr5380sbc.c
--- a/sys/dev/ic/ncr5380sbc.c	Fri Aug 02 11:45:52 2024 +0000
+++ b/sys/dev/ic/ncr5380sbc.c	Sat Aug 03 09:45:54 2024 +1000
@@ -393,6 +393,7 @@ ncr5380_init(struct ncr5380_softc *sc)
 static void
 ncr5380_reset_scsibus(struct ncr5380_softc *sc)
 {
+	struct sci_req *sr;
 
 	NCR_TRACE("reset_scsibus, cur=0x%x\n",
 			  (long) sc->sc_current);
@@ -409,6 +410,9 @@ ncr5380_reset_scsibus(struct ncr5380_sof
 	delay(100000);
 
 	/* XXX - Need to cancel disconnected requests. */
+	sr = sc->sc_current;
+	if (sr)
+		ncr5380_abort(sc);
 }
 
 
@@ -617,9 +621,11 @@ ncr5380_scsipi_request(struct scsipi_cha
 			/* Terminate any current command. */
 			sr = sc->sc_current;
 			if (sr) {
+#ifdef	NCR5380_DEBUG
 				printf("%s: polled request aborting %d/%d\n",
 				    device_xname(sc->sc_dev),
 				    sr->sr_target, sr->sr_lun);
+#endif
 				ncr5380_abort(sc);
 			}
 			if (sc->sc_state != NCR_IDLE) {
@@ -802,7 +808,7 @@ finish:
 	sc->sc_ncmds--;
 
 	/* Tell common SCSI code it is done. */
-	scsipi_done(xs);
+	scsipi_done_once(xs);
 
 	sc->sc_state = NCR_IDLE;
 	/* Now ncr5380_sched() may be called again. */
diff -r b6eba4068585 sys/dev/scsipi/scsipi_base.c
--- a/sys/dev/scsipi/scsipi_base.c	Fri Aug 02 11:45:52 2024 +0000
+++ b/sys/dev/scsipi/scsipi_base.c	Sat Aug 03 09:45:54 2024 +1000
@@ -96,6 +96,8 @@ SDT_PROBE_DEFINE1(scsi, base, xfer, rest
 SDT_PROBE_DEFINE1(scsi, base, xfer, free,  "struct scsipi_xfer *"/*xs*/);
 
 static int	scsipi_complete(struct scsipi_xfer *);
+static struct scsipi_channel*
+		scsipi_done_internal(struct scsipi_xfer *, bool);
 static void	scsipi_request_sense(struct scsipi_xfer *);
 static int	scsipi_enqueue(struct scsipi_xfer *);
 static void	scsipi_run_queue(struct scsipi_channel *chan);
@@ -1056,6 +1058,13 @@ scsipi_interpret_sense(struct scsipi_xfe
 		case SKEY_VOLUME_OVERFLOW:
 			error = ENOSPC;
 			break;
+		case SKEY_MEDIUM_ERROR:
+			if (xs->xs_retries != 0) {
+				xs->xs_retries--;
+				error = ERESTART;
+			} else
+				error = EIO;
+			break;
 		default:
 			error = EIO;
 			break;
@@ -1597,6 +1606,28 @@ scsipi_free_opcodeinfo(struct scsipi_per
 void
 scsipi_done(struct scsipi_xfer *xs)
 {
+	struct scsipi_channel *chan;
+	/*
+	 * If there are more xfers on the channel's queue, attempt to
+	 * run them.
+	 */
+	if ((chan = scsipi_done_internal(xs, true)) != NULL)
+		scsipi_run_queue(chan);
+}
+
+/*
+ * Just like scsipi_done(), but no recursion.  Useful if aborting the current
+ * transfer.
+ */
+void
+scsipi_done_once(struct scsipi_xfer *xs)
+{
+	(void)scsipi_done_internal(xs, false);
+}
+
+static struct scsipi_channel*
+scsipi_done_internal(struct scsipi_xfer *xs, bool more)
+{
 	struct scsipi_periph *periph = xs->xs_periph;
 	struct scsipi_channel *chan = periph->periph_channel;
 	int freezecnt;
@@ -1685,7 +1716,7 @@ scsipi_done(struct scsipi_xfer *xs)
 		 */
 		if (xs->xs_control & XS_CTL_POLL) {
 			mutex_exit(chan_mtx(chan));
-			return;
+			return NULL;
 		}
 		cv_broadcast(xs_cv(xs));
 		mutex_exit(chan_mtx(chan));
@@ -1697,7 +1728,7 @@ scsipi_done(struct scsipi_xfer *xs)
 	 * without error; no use in taking a context switch
 	 * if we can handle it in interrupt context.
 	 */
-	if (xs->error == XS_NOERROR) {
+	if (xs->error == XS_NOERROR && more == true) {
 		mutex_exit(chan_mtx(chan));
 		(void) scsipi_complete(xs);
 		goto out;
@@ -1712,11 +1743,7 @@ scsipi_done(struct scsipi_xfer *xs)
 	mutex_exit(chan_mtx(chan));
 
  out:
-	/*
-	 * If there are more xfers on the channel's queue, attempt to
-	 * run them.
-	 */
-	scsipi_run_queue(chan);
+	return chan;
 }
 
 /*
diff -r b6eba4068585 sys/dev/scsipi/scsipiconf.h
--- a/sys/dev/scsipi/scsipiconf.h	Fri Aug 02 11:45:52 2024 +0000
+++ b/sys/dev/scsipi/scsipiconf.h	Sat Aug 03 09:45:54 2024 +1000
@@ -702,6 +702,7 @@ int	scsipi_mode_sense_big(struct scsipi_
 	    struct scsi_mode_parameter_header_10 *, int, int, int, int);
 int	scsipi_start(struct scsipi_periph *, int, int);
 void	scsipi_done(struct scsipi_xfer *);
+void	scsipi_done_once(struct scsipi_xfer *);
 void	scsipi_user_done(struct scsipi_xfer *);
 int	scsipi_interpret_sense(struct scsipi_xfer *);
 void	scsipi_wait_drain(struct scsipi_periph *);