[Cc: +Paolo as original author of the driver]

Dear Mark,


Thank you for your patch.

Am 29.07.23 um 15:04 schrieb Mark Cave-Ayland:
The ESP SELATN command used to send SCSI commands from the ESP to thes SCSI bus

s/thes/the/

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.

I’d appreciated a link to these QEMU patches, describing the problem in QEMU.

According to the NCR datasheet the INTR_BS/INTR_FC bits are set when the SELATN

Could you please mention the full name and revision of the datasheet?

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.

It’d be great, if you added the QEMU commands how to test your patch.

Lastly, will SeaBIOS built with your patches still work with older QEMU versions without your QEMU patches?

Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
  src/hw/esp-scsi.c | 36 ++++++++++++++++++++++--------------
  1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c
index e4815aa..2d2d915 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,22 +118,27 @@ 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;
+                }
+           }
          }
/* At end of DMA TC is set again -> complete command. */

Reviewed-by: Paul Menzel <pmen...@molgen.mpg.de>


Kind regards,

Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

Reply via email to