esp-scsi: check for INTR_BS/INTR_FC instead of STAT_TC for command completion

The ESP SELATN command used to send SCSI commands from the ESP to the SCSI bus
is not a DMA command and therefore does not affect the STAT_TC bit. The only
reason this works at all is due to a bug in QEMU which (currently) always
updates the STAT_TC bit in ESP_RSTAT regardless of the state of the ESP_CMD_DMA
bit.

According to the NCR datasheet [1] the INTR_BS/INTR_FC bits are set when the
SELATN command has completed, so update the existing logic to check for these
bits in ESP_RINTR instead. Note that the read of ESP_RINTR needs to be
restricted to state == 0 as reading ESP_RINTR resets the ESP_RSTAT register
which breaks the STAT_TC check when state == 1.

This commit also includes an extra read of ESP_INTR to clear all the interrupt
bits before submitting the SELATN command to ensure that we don't accidentally
immediately progress to the data phase handling logic where ESP_RINTR bits have
already been set by a previous ESP command.

[1] "NCR 53C94, 53C95, 53C96 Advanced SCSI Controller"
    NCR_53C94_53C95_53C96_Data_Sheet_Feb90.pdf

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <20230807065300.366070-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

1 files changed, 22 insertions, 14 deletions

diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c
index e4815aa..ebdbd02 100644
--- a/src/hw/esp-scsi.c
+++ b/src/hw/esp-scsi.c
@@ -57,6 +57,8 @@
 #define ESP_STAT_MSG     0x04
 #define ESP_STAT_TC      0x10
 
+#define ESP_INTR_FC      0x08
+#define ESP_INTR_BS      0x10
 #define ESP_INTR_DC      0x20
 
 struct esp_lun_s {
@@ -97,8 +99,9 @@ esp_scsi_process_op(struct disk_op_s *op)
 
     outb(target, iobase + ESP_WBUSID);
 
-    /* Clear FIFO before sending command.  */
+    /* Clear FIFO and interrupts before sending command.  */
     outb(ESP_CMD_FLUSH, iobase + ESP_CMD);
+    inb(iobase + ESP_RINTR);
 
     /*
      * We need to pass the LUN at the beginning of the command, and the FIFO
@@ -115,21 +118,26 @@ esp_scsi_process_op(struct disk_op_s *op)
 
     for (state = 0;;) {
         u8 stat = inb(iobase + ESP_RSTAT);
+        u8 intr;
 
-        /* Detect disconnected device.  */
-        if (state == 0 && (inb(iobase + ESP_RINTR) & ESP_INTR_DC)) {
-            return DISK_RET_ENOTREADY;
-        }
+        if (state == 0) {
+            intr = inb(iobase + ESP_RINTR);
 
-        /* HBA reads command, clears CD, sets TC -> do DMA if needed.  */
-        if (state == 0 && (stat & ESP_STAT_TC)) {
-            state++;
-            if (op->count && blocksize) {
-                /* Data phase.  */
-                u32 count = (u32)op->count * blocksize;
-                esp_scsi_dma(iobase, (u32)op->buf_fl, count, scsi_is_read(op));
-                outb(ESP_CMD_TI | ESP_CMD_DMA, iobase + ESP_CMD);
-                continue;
+            /* Detect disconnected device.  */
+            if (intr & ESP_INTR_DC) {
+                return DISK_RET_ENOTREADY;
+            }
+
+            /* HBA reads command, executes it, sets BS/FC -> do DMA if needed.  */
+            if (intr & (ESP_INTR_BS | ESP_INTR_FC)) {
+                state++;
+                if (op->count && blocksize) {
+                    /* Data phase.  */
+                    u32 count = (u32)op->count * blocksize;
+                    esp_scsi_dma(iobase, (u32)op->buf_fl, count, scsi_is_read(op));
+                    outb(ESP_CMD_TI | ESP_CMD_DMA, iobase + ESP_CMD);
+                    continue;
+                }
             }
         }