scsi: mac_scsi: Disallow bus errors during PDMA send

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>
Link: https://lore.kernel.org/r/cc38df687ace2c4ffc375a683b2502fc476b600d.1723001788.git.fthain@linux-m68k.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c
index 99a2008f875287bbb055f649b325524e14753333..3958f7dc679f0a885b6b705244d44b201de35a3a 100644 (file)
--- 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,