mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Finn Thain <fthain@linux-m68k.org>
To: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Hannes Reinecke <hare@suse.com>,
	Michael Schmitz <schmitzmic@gmail.com>,
	Ondrej Zary <linux@zary.sk>, Stan Johnson <userm57@yahoo.com>,
	stable@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 03/11] scsi: mac_scsi: Disallow bus errors during PDMA send
Date: Wed, 07 Aug 2024 13:36:28 +1000	[thread overview]
Message-ID: <cc38df687ace2c4ffc375a683b2502fc476b600d.1723001788.git.fthain@linux-m68k.org> (raw)
In-Reply-To: <cover.1723001788.git.fthain@linux-m68k.org>

SD cards can produce write latency spikes on the order of a hundred
milliseconds. If the target firmware does not hide that latency during
DATA IN and OUT phases it can cause the PDMA circuitry to raise a
processor bus fault which in turn leads to an unreliable byte count
and a DMA overrun.

The Last Byte Sent flag is used to detect the overrun but this mechanism
is unreliable on some systems. Instead, set a DID_ERROR result whenever
there is a bus fault during a PDMA send, unless the cause was a phase
mismatch.

Cc: stable@vger.kernel.org # 5.15+
Reported-and-tested-by: Stan Johnson <userm57@yahoo.com>
Fixes: 7c1f3e3447a1 ("scsi: mac_scsi: Treat Last Byte Sent time-out as failure")
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 drivers/scsi/mac_scsi.c | 44 ++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c
index 39814c841113..2e9fad1e3069 100644
--- a/drivers/scsi/mac_scsi.c
+++ b/drivers/scsi/mac_scsi.c
@@ -102,11 +102,15 @@ __setup("mac5380=", mac_scsi_setup);
  * Linux SCSI drivers lack knowledge of the timing behaviour of SCSI targets
  * so bus errors are unavoidable.
  *
- * If a MOVE.B instruction faults, we assume that zero bytes were transferred
- * and simply retry. That assumption probably depends on target behaviour but
- * seems to hold up okay. The NOP provides synchronization: without it the
- * fault can sometimes occur after the program counter has moved past the
- * offending instruction. Post-increment addressing can't be used.
+ * If a MOVE.B instruction faults during a receive operation, we assume the
+ * target sent nothing and try again. That assumption probably depends on
+ * target firmware but it seems to hold up okay. If a fault happens during a
+ * send operation, the target may or may not have seen /ACK and got the byte.
+ * It's uncertain so the whole SCSI command gets retried.
+ *
+ * The NOP is needed for synchronization because the fault address in the
+ * exception stack frame may or may not be the instruction that actually
+ * caused the bus error. Post-increment addressing can't be used.
  */
 
 #define MOVE_BYTE(operands) \
@@ -243,22 +247,21 @@ static inline int mac_pdma_send(unsigned char *start, void __iomem *io, int n)
 	if (n >= 1) {
 		MOVE_BYTE("%0@,%3@");
 		if (result)
-			goto out;
+			return -1;
 	}
 	if (n >= 1 && ((unsigned long)addr & 1)) {
 		MOVE_BYTE("%0@,%3@");
 		if (result)
-			goto out;
+			return -2;
 	}
 	while (n >= 32)
 		MOVE_16_WORDS("%0@+,%3@");
 	while (n >= 2)
 		MOVE_WORD("%0@+,%3@");
 	if (result)
-		return start - addr; /* Negated to indicate uncertain length */
+		return start - addr - 1; /* Negated to indicate uncertain length */
 	if (n == 1)
 		MOVE_BYTE("%0@,%3@");
-out:
 	return addr - start;
 }
 
@@ -307,7 +310,6 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
 {
 	u8 __iomem *s = hostdata->pdma_io + (INPUT_DATA_REG << 4);
 	unsigned char *d = dst;
-	int result = 0;
 
 	hostdata->pdma_residual = len;
 
@@ -343,11 +345,12 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
 		if (bytes == 0)
 			continue;
 
-		result = -1;
+		if (macscsi_wait_for_drq(hostdata) <= 0)
+			set_host_byte(hostdata->connected, DID_ERROR);
 		break;
 	}
 
-	return result;
+	return 0;
 }
 
 static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
@@ -355,7 +358,6 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
 {
 	unsigned char *s = src;
 	u8 __iomem *d = hostdata->pdma_io + (OUTPUT_DATA_REG << 4);
-	int result = 0;
 
 	hostdata->pdma_residual = len;
 
@@ -377,17 +379,8 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
 			hostdata->pdma_residual -= bytes;
 		}
 
-		if (hostdata->pdma_residual == 0) {
-			if (NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG,
-			                          TCR_LAST_BYTE_SENT,
-			                          TCR_LAST_BYTE_SENT,
-			                          0) < 0) {
-				scmd_printk(KERN_ERR, hostdata->connected,
-				            "%s: Last Byte Sent timeout\n", __func__);
-				result = -1;
-			}
+		if (hostdata->pdma_residual == 0)
 			break;
-		}
 
 		if (bytes > 0)
 			continue;
@@ -400,11 +393,12 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
 		if (bytes == 0)
 			continue;
 
-		result = -1;
+		if (macscsi_wait_for_drq(hostdata) <= 0)
+			set_host_byte(hostdata->connected, DID_ERROR);
 		break;
 	}
 
-	return result;
+	return 0;
 }
 
 static int macscsi_dma_xfer_len(struct NCR5380_hostdata *hostdata,
-- 
2.39.5


  parent reply	other threads:[~2024-08-07  3:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-07  3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
2024-08-07  3:36 ` [PATCH 09/11] scsi: NCR5380: Remove redundant result calculation from NCR5380_transfer_pio() Finn Thain
2024-08-07  3:36 ` [PATCH 01/11] scsi: mac_scsi: Revise printk(KERN_DEBUG ...) messages Finn Thain
2024-08-07  3:36 ` [PATCH 04/11] scsi: NCR5380: Check for phase match during PDMA fixup Finn Thain
2024-08-07  3:36 ` [PATCH 11/11] scsi: NCR5380: Clean up indentation Finn Thain
2024-08-07  3:36 ` [PATCH 08/11] scsi: NCR5380: Drop redundant member from struct NCR5380_cmd Finn Thain
2024-08-07  3:36 ` [PATCH 06/11] scsi: NCR5380: Initialize buffer for MSG IN and STATUS transfers Finn Thain
2024-08-07  3:36 ` [PATCH 02/11] scsi: mac_scsi: Refactor polling loop Finn Thain
2024-08-07  3:36 ` [PATCH 07/11] scsi: NCR5380: Handle BSY signal loss during information transfer phases Finn Thain
2024-08-07  3:36 ` Finn Thain [this message]
2024-08-07  3:36 ` [PATCH 05/11] scsi: mac_scsi: Enable scatter/gather by default Finn Thain
2024-08-07  3:36 ` [PATCH 10/11] scsi: NCR5380: Remove obsolete comment Finn Thain
2024-08-13  2:07 ` [PATCH 00/11] NCR5380: Bug fixes and other improvements Martin K. Petersen
2024-08-17  1:34 ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc38df687ace2c4ffc375a683b2502fc476b600d.1723001788.git.fthain@linux-m68k.org \
    --to=fthain@linux-m68k.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=hare@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux@zary.sk \
    --cc=martin.petersen@oracle.com \
    --cc=schmitzmic@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=userm57@yahoo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®