[PATCH v3 0/3] esp: fixups to correctly terminate successful SCSI commands

[Mark Cave-Ayland]

This series is an extension of the v2 patch with a couple of modifications to
clear the FIFO at the start of the command, and to poll the ESP interrupt
register to detect the end of the transfer.

Signed-off-by: Mark Cave-Ayland

v3:
- Clear FIFO before issuing ESP command
- Poll ESP interrupt register to detect end of transfer

v2:
- Fix typo in commit message spotted by Zoltan

Mark Cave-Ayland (3):
  esp: clear FIFO before issuing ESP command
  esp: use ESP rather than DMA interrupt to detect end of transfer
  esp: correctly terminate successful SCSI commands

drivers/esp.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

[PATCH v3 1/3] esp: clear FIFO before issuing ESP command

[Mark Cave-Ayland]

This ensures that any partially transferred data is removed from the FIFO
before issuing the next ESP command.

Signed-off-by: Mark Cave-Ayland 
---
 drivers/esp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/esp.c b/drivers/esp.c
index 0880ab2..95e969f 100644
--- a/drivers/esp.c
+++ b/drivers/esp.c
@@ -97,6 +97,8 @@ do_command(esp_private_t *esp, sd_private_t *sd, int cmdlen, int replylen)
     esp->ll->regs[ESP_TCMED] = (cmdlen >> 8) & 0xff;
     // Set DMA direction and enable DMA
     esp->espdma.regs->cond_reg = DMA_ENABLE;
+    // Clear FIFO
+    esp->ll->regs[ESP_CMD] = ESP_CMD_FLUSH;
     // Set ATN, issue command
     esp->ll->regs[ESP_CMD] = ESP_CMD_SELA | ESP_CMD_DMA;
     // Wait for DMA to complete. Can this fail?

[PATCH v3 2/3] esp: use ESP rather than DMA interrupt to detect end of transfer

[Mark Cave-Ayland]

Rather than rely on the DMA interrupt to determine that an ESP interrupt has
occurred, poll the ESP interrupt register directly. This enables us to poll
until one of the specified interrupts occurs.

Since an ESP interrupt can still be in a pending state before issuing an ESP
command, read the ESP interrupt register beforehand to clear any pending
interrupts.

Signed-off-by: Mark Cave-Ayland 
---
 drivers/esp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/esp.c b/drivers/esp.c
index 95e969f..5c7ddb5 100644
--- a/drivers/esp.c
+++ b/drivers/esp.c
@@ -99,14 +99,14 @@ do_command(esp_private_t *esp, sd_private_t *sd, int cmdlen, int replylen)
     esp->espdma.regs->cond_reg = DMA_ENABLE;
     // Clear FIFO
     esp->ll->regs[ESP_CMD] = ESP_CMD_FLUSH;
+    // Clear pending interrupts
+    (void)esp->ll->regs[ESP_INTRPT];
     // Set ATN, issue command
     esp->ll->regs[ESP_CMD] = ESP_CMD_SELA | ESP_CMD_DMA;
-    // Wait for DMA to complete. Can this fail?
-    while ((esp->espdma.regs->cond_reg & DMA_HNDL_INTR) == 0) /* no-op */;
+    // Wait for ESP to complete
+    while (!(esp->ll->regs[ESP_INTRPT] & (ESP_INTR_BSERV | ESP_INTR_FDONE | ESP_INTR_DC)));
     // Check status
     status = esp->ll->regs[ESP_STATUS];
-    // Clear interrupts to avoid guests seeing spurious interrupts
-    (void)esp->ll->regs[ESP_INTRPT];

DPRINTF("do_command: id %d, cmd[0] 0x%x, status 0x%x\n", sd->id, esp->buffer[1], status);

@@ -130,14 +130,14 @@ do_command(esp_private_t *esp, sd_private_t *sd, int cmdlen, int replylen)
     esp->ll->regs[ESP_TCMED] = (replylen >> 8) & 0xff;
     // Set DMA direction
     esp->espdma.regs->cond_reg = DMA_ST_WRITE | DMA_ENABLE;
+    // Clear pending interrupts
+    (void)esp->ll->regs[ESP_INTRPT];
     // Transfer
     esp->ll->regs[ESP_CMD] = ESP_CMD_TI | ESP_CMD_DMA;
-    // Wait for DMA to complete
-    while ((esp->espdma.regs->cond_reg & DMA_HNDL_INTR) == 0) /* no-op */;
+    // Wait for ESP to complete
+    while (!(esp->ll->regs[ESP_INTRPT] & (ESP_INTR_BSERV | ESP_INTR_FDONE | ESP_INTR_DC)));
     // Check status
     status = esp->ll->regs[ESP_STATUS];
-    // Clear interrupts to avoid guests seeing spurious interrupts
-    (void)esp->ll->regs[ESP_INTRPT];

DPRINTF("do_command_reply: status 0x%x\n", status);

[PATCH v3 3/3] esp: correctly terminate successful SCSI commands

[Mark Cave-Ayland]

An upcoming series for QEMU's ESP emulation will check that ESP commands are
only issued according to their valid states as listed in the datasheet, and
otherwise generate an illegal command interrupt.

Currently if a SCSI command is expected to return no data or transfers data
successfully, the ASC is not returned to its disconnected state. This means
that any subsequent ESP initiator commands should be rejected until the SCSI
bus transaction is terminated.

Update the ESP driver so that if a SCSI command is successful then the ICCS
and MSGACC commands are issued by the host to complete the SCSI transaction.
This ensures that the ASC is returned to the disconnected state which allows
the ESP to accept subsequent SCSI commands without generating an illegal
command interrupt.

Signed-off-by: Mark Cave-Ayland 
---
 drivers/esp.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/esp.c b/drivers/esp.c
index 5c7ddb5..92f28cc 100644
--- a/drivers/esp.c
+++ b/drivers/esp.c
@@ -115,11 +115,12 @@ do_command(esp_private_t *esp, sd_private_t *sd, int cmdlen, int replylen)
         return status;
     }
     if (replylen == 0) {
-        return 0;
+        /* No reply expected, ICCS and MSGACC to complete */
+        goto done;
     }
     /* Target went to status phase instead of data phase? */
     if ((status & ESP_STAT_PMASK) == ESP_STATP) {
-        return status;
+        goto done;
     }

// Get reply
@@ -141,10 +142,18 @@ do_command(esp_private_t *esp, sd_private_t *sd, int cmdlen, int replylen)

DPRINTF("do_command_reply: status 0x%x\n", status);

-    if ((status & ESP_STAT_TCNT) != ESP_STAT_TCNT)
+    if ((status & ESP_STAT_TCNT) != ESP_STAT_TCNT) {
         return status;
-    else
-        return 0; // OK
+    }
+
+done:
+    // ICCS and MSGACC sequence to complete
+    esp->ll->regs[ESP_CMD] = ESP_CMD_ICCSEQ;
+    (void)esp->ll->regs[ESP_FDATA];  // Ignore 1st byte for now
+    (void)esp->ll->regs[ESP_FDATA];  // Ignore 2nd byte for now
+    esp->ll->regs[ESP_CMD] = ESP_CMD_MOK;
+
+    return 0; // OK
 }

// offset is in sectors

[Mark Cave-Ayland]

On 13/09/2024 23:05, Mark Cave-Ayland wrote:
> This series is an extension of the v2 patch with a couple of modifications to
> clear the FIFO at the start of the command, and to poll the ESP interrupt
> register to detect the end of the transfer.
>
> Signed-off-by: Mark Cave-Ayland
>
> v3:
> Clear FIFO before issuing ESP command
> Poll ESP interrupt register to detect end of transfer
>
> v2:
> Fix typo in commit message spotted by Zoltan
>
> Mark Cave-Ayland (3):
>    esp: clear FIFO before issuing ESP command
>    esp: use ESP rather than DMA interrupt to detect end of transfer
>    esp: correctly terminate successful SCSI commands
>
> drivers/esp.c | 37 ++++++++++++++++++++++++-------------
>   1 file changed, 24 insertions(+), 13 deletions(-)

Applied to master.

ATB,

Mark.