[PATCH 0/4] esp-pci: fixes for Linux and MS-DOS

[PATCH 0/4] esp-pci: fixes for Linux and MS-DOS

[Mark Cave-Ayland]

This series contains fixes for the esp-pci device (am53c974 or dc390) for a
few issues spotted whilst testing the previous ESP series.

Patches 1-3 are fixes for issues found by Helge/Guenter whilst testing the
hppa C3700 machine with the amd53c974/dc390 devices under Linux, whilst patch
4 fixes an issue that was exposed by testing MS-DOS and Windows drivers.

With this series applied on top of the reworked ESP device, it is possible to
boot Linux under qemu-system-hppa without any errors and also boot and install
Win98SE from a DC390 PCI SCSI controller (no IDE!) using an MS-DOS boot floppy.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Based-on: 20240112125420.514425-1-mark.cave-ayland@ilande.co.uk


Mark Cave-Ayland (4):
  esp-pci.c: use correct address register for PCI DMA transfers
  esp-pci.c: generate PCI interrupt from separate ESP and PCI sources
  esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion
    interrupt
  esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued

 hw/scsi/esp-pci.c | 61 +++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 20 deletions(-)

-- 
2.39.2

[PATCH 1/4] esp-pci.c: use correct address register for PCI DMA transfers

[Mark Cave-Ayland]

The current code in esp_pci_dma_memory_rw() sets the DMA address to the value
of the DMA_SPA (Starting Physical Address) register which is incorrect: this
means that for each callback from the SCSI layer the DMA address is set back
to the starting address.

In the case where only a single SCSI callback occurs (currently for transfer
lengths < 128kB) this works fine, however for larger transfers the DMA address
wraps back to the initial starting address, corrupting the buffer holding the
data transferred to the guest.

Fix esp_pci_dma_memory_rw() to use the DMA_WAC (Working Address Counter) for
the DMA address which is correctly incremented across multiple SCSI layer
transfers.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/esp-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 93b3429e0f..7117725371 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -275,7 +275,7 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
         qemu_log_mask(LOG_UNIMP, "am53c974: MDL transfer not implemented\n");
     }
 
-    addr = pci->dma_regs[DMA_SPA];
+    addr = pci->dma_regs[DMA_WAC];
     if (pci->dma_regs[DMA_WBC] < len) {
         len = pci->dma_regs[DMA_WBC];
     }
-- 
2.39.2

Re: [PATCH 1/4] esp-pci.c: use correct address register for PCI DMA transfers

[Guenter Roeck]

On Fri, Jan 12, 2024 at 01:15:26PM +0000, Mark Cave-Ayland wrote:
> The current code in esp_pci_dma_memory_rw() sets the DMA address to the value
> of the DMA_SPA (Starting Physical Address) register which is incorrect: this
> means that for each callback from the SCSI layer the DMA address is set back
> to the starting address.
> 
> In the case where only a single SCSI callback occurs (currently for transfer
> lengths < 128kB) this works fine, however for larger transfers the DMA address
> wraps back to the initial starting address, corrupting the buffer holding the
> data transferred to the guest.
> 
> Fix esp_pci_dma_memory_rw() to use the DMA_WAC (Working Address Counter) for
> the DMA address which is correctly incremented across multiple SCSI layer
> transfers.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  hw/scsi/esp-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 93b3429e0f..7117725371 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -275,7 +275,7 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
>          qemu_log_mask(LOG_UNIMP, "am53c974: MDL transfer not implemented\n");
>      }
>  
> -    addr = pci->dma_regs[DMA_SPA];
> +    addr = pci->dma_regs[DMA_WAC];
>      if (pci->dma_regs[DMA_WBC] < len) {
>          len = pci->dma_regs[DMA_WBC];
>      }
> -- 
> 2.39.2
> 

[PATCH 2/4] esp-pci.c: generate PCI interrupt from separate ESP and PCI sources

[Mark Cave-Ayland]

The am53c974/dc390 PCI interrupt has two separate sources: the first is from the
internal ESP device, and the second is from the PCI DMA transfer logic.

Update the ESP interrupt handler so that it sets DMA_STAT_SCSIINT rather than
driving the PCI IRQ directly, and introduce a new esp_pci_update_irq() function
to generate the correct PCI IRQ level. In particular this fixes spurious interrupts
being generated by setting DMA_STAT_DONE at the end of a transfer if DMA_CMD_INTE_D
isn't set in the DMA_CMD register.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/esp-pci.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 7117725371..15dc3c004d 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -77,6 +77,29 @@ struct PCIESPState {
     ESPState esp;
 };
 
+static void esp_pci_update_irq(PCIESPState *pci)
+{
+    int scsi_level = !!(pci->dma_regs[DMA_STAT] & DMA_STAT_SCSIINT);
+    int dma_level = (pci->dma_regs[DMA_CMD] & DMA_CMD_INTE_D) ?
+                    !!(pci->dma_regs[DMA_STAT] & DMA_STAT_DONE) : 0;
+    int level = scsi_level || dma_level;
+
+    pci_set_irq(PCI_DEVICE(pci), level);
+}
+
+static void esp_irq_handler(void *opaque, int irq_num, int level)
+{
+    PCIESPState *pci = PCI_ESP(opaque);
+
+    if (level) {
+        pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT;
+    } else {
+        pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT;
+    }
+
+    esp_pci_update_irq(pci);
+}
+
 static void esp_pci_handle_idle(PCIESPState *pci, uint32_t val)
 {
     ESPState *s = &pci->esp;
@@ -151,6 +174,7 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
             /* clear some bits on write */
             uint32_t mask = DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE;
             pci->dma_regs[DMA_STAT] &= ~(val & mask);
+            esp_pci_update_irq(pci);
         }
         break;
     default:
@@ -161,17 +185,14 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
 
 static uint32_t esp_pci_dma_read(PCIESPState *pci, uint32_t saddr)
 {
-    ESPState *s = &pci->esp;
     uint32_t val;
 
     val = pci->dma_regs[saddr];
     if (saddr == DMA_STAT) {
-        if (s->rregs[ESP_RSTAT] & STAT_INT) {
-            val |= DMA_STAT_SCSIINT;
-        }
         if (!(pci->sbac & SBAC_STATUS)) {
             pci->dma_regs[DMA_STAT] &= ~(DMA_STAT_ERROR | DMA_STAT_ABORT |
                                          DMA_STAT_DONE);
+            esp_pci_update_irq(pci);
         }
     }
 
@@ -350,6 +371,7 @@ static void esp_pci_command_complete(SCSIRequest *req, size_t resid)
     esp_command_complete(req, resid);
     pci->dma_regs[DMA_WBC] = 0;
     pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
+    esp_pci_update_irq(pci);
 }
 
 static const struct SCSIBusInfo esp_pci_scsi_info = {
@@ -386,7 +408,7 @@ static void esp_pci_scsi_realize(PCIDevice *dev, Error **errp)
                           "esp-io", 0x80);
 
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io);
-    s->irq = pci_allocate_irq(dev);
+    s->irq = qemu_allocate_irq(esp_irq_handler, pci, 0);
 
     scsi_bus_init(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info);
 }
-- 
2.39.2

Re: [PATCH 2/4] esp-pci.c: generate PCI interrupt from separate ESP and PCI sources

[Guenter Roeck]

On Fri, Jan 12, 2024 at 01:15:27PM +0000, Mark Cave-Ayland wrote:
> The am53c974/dc390 PCI interrupt has two separate sources: the first is from the
> internal ESP device, and the second is from the PCI DMA transfer logic.
> 
> Update the ESP interrupt handler so that it sets DMA_STAT_SCSIINT rather than
> driving the PCI IRQ directly, and introduce a new esp_pci_update_irq() function
> to generate the correct PCI IRQ level. In particular this fixes spurious interrupts
> being generated by setting DMA_STAT_DONE at the end of a transfer if DMA_CMD_INTE_D
> isn't set in the DMA_CMD register.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  hw/scsi/esp-pci.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 7117725371..15dc3c004d 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -77,6 +77,29 @@ struct PCIESPState {
>      ESPState esp;
>  };
>  
> +static void esp_pci_update_irq(PCIESPState *pci)
> +{
> +    int scsi_level = !!(pci->dma_regs[DMA_STAT] & DMA_STAT_SCSIINT);
> +    int dma_level = (pci->dma_regs[DMA_CMD] & DMA_CMD_INTE_D) ?
> +                    !!(pci->dma_regs[DMA_STAT] & DMA_STAT_DONE) : 0;
> +    int level = scsi_level || dma_level;
> +
> +    pci_set_irq(PCI_DEVICE(pci), level);
> +}
> +
> +static void esp_irq_handler(void *opaque, int irq_num, int level)
> +{
> +    PCIESPState *pci = PCI_ESP(opaque);
> +
> +    if (level) {
> +        pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT;
> +    } else {
> +        pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT;
> +    }
> +
> +    esp_pci_update_irq(pci);
> +}
> +
>  static void esp_pci_handle_idle(PCIESPState *pci, uint32_t val)
>  {
>      ESPState *s = &pci->esp;
> @@ -151,6 +174,7 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
>              /* clear some bits on write */
>              uint32_t mask = DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE;
>              pci->dma_regs[DMA_STAT] &= ~(val & mask);
> +            esp_pci_update_irq(pci);
>          }
>          break;
>      default:
> @@ -161,17 +185,14 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
>  
>  static uint32_t esp_pci_dma_read(PCIESPState *pci, uint32_t saddr)
>  {
> -    ESPState *s = &pci->esp;
>      uint32_t val;
>  
>      val = pci->dma_regs[saddr];
>      if (saddr == DMA_STAT) {
> -        if (s->rregs[ESP_RSTAT] & STAT_INT) {
> -            val |= DMA_STAT_SCSIINT;
> -        }
>          if (!(pci->sbac & SBAC_STATUS)) {
>              pci->dma_regs[DMA_STAT] &= ~(DMA_STAT_ERROR | DMA_STAT_ABORT |
>                                           DMA_STAT_DONE);
> +            esp_pci_update_irq(pci);
>          }
>      }
>  
> @@ -350,6 +371,7 @@ static void esp_pci_command_complete(SCSIRequest *req, size_t resid)
>      esp_command_complete(req, resid);
>      pci->dma_regs[DMA_WBC] = 0;
>      pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
> +    esp_pci_update_irq(pci);
>  }
>  
>  static const struct SCSIBusInfo esp_pci_scsi_info = {
> @@ -386,7 +408,7 @@ static void esp_pci_scsi_realize(PCIDevice *dev, Error **errp)
>                            "esp-io", 0x80);
>  
>      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io);
> -    s->irq = pci_allocate_irq(dev);
> +    s->irq = qemu_allocate_irq(esp_irq_handler, pci, 0);
>  
>      scsi_bus_init(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info);
>  }
> -- 
> 2.39.2
> 

[PATCH 3/4] esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion interrupt

[Mark Cave-Ayland]

The setting of DMA_STAT_DONE at the end of a DMA transfer can be configured to
generate an interrupt, however the Linux driver manually checks for DMA_STAT_DONE
being set and if it is, considers that a DMA transfer has completed.

If DMA_STAT_DONE is set but the ESP device isn't indicating an interrupt then
the Linux driver considers this to be a spurious interrupt. However this can
occur in QEMU as there is a delay between the end of DMA transfer where
DMA_STAT_DONE is set, and the ESP device raising its completion interrupt.

This appears to be an incorrect assumption in the Linux driver as the ESP and
PCI DMA interrupt sources are separate (and may not be raised exactly
together), however we can work around this by synchronising the setting of
DMA_STAT_DONE at the end of a DMA transfer with the ESP completion interrupt.

In conjunction with the previous commit Linux is now able to correctly boot
from an am53c974 PCI SCSI device on the hppa C3700 machine without emitting
"iget: checksum invalid" and "Spurious irq, sreg=10" errors.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/esp-pci.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 15dc3c004d..875a49199d 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -93,6 +93,18 @@ static void esp_irq_handler(void *opaque, int irq_num, int level)
 
     if (level) {
         pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT;
+
+        /*
+         * If raising the ESP IRQ to indicate end of DMA transfer, set
+         * DMA_STAT_DONE at the same time. In theory this should be done in
+         * esp_pci_dma_memory_rw(), however there is a delay between setting
+         * DMA_STAT_DONE and the ESP IRQ arriving which is visible to the
+         * guest that can cause confusion e.g. Linux
+         */
+        if ((pci->dma_regs[DMA_CMD] & DMA_CMD_MASK) == 0x3 &&
+            pci->dma_regs[DMA_WBC] == 0) {
+                pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
+        }
     } else {
         pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT;
     }
@@ -306,9 +318,6 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
     /* update status registers */
     pci->dma_regs[DMA_WBC] -= len;
     pci->dma_regs[DMA_WAC] += len;
-    if (pci->dma_regs[DMA_WBC] == 0) {
-        pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
-    }
 }
 
 static void esp_pci_dma_memory_read(void *opaque, uint8_t *buf, int len)
@@ -363,24 +372,13 @@ static const VMStateDescription vmstate_esp_pci_scsi = {
     }
 };
 
-static void esp_pci_command_complete(SCSIRequest *req, size_t resid)
-{
-    ESPState *s = req->hba_private;
-    PCIESPState *pci = container_of(s, PCIESPState, esp);
-
-    esp_command_complete(req, resid);
-    pci->dma_regs[DMA_WBC] = 0;
-    pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
-    esp_pci_update_irq(pci);
-}
-
 static const struct SCSIBusInfo esp_pci_scsi_info = {
     .tcq = false,
     .max_target = ESP_MAX_DEVS,
     .max_lun = 7,
 
     .transfer_data = esp_transfer_data,
-    .complete = esp_pci_command_complete,
+    .complete = esp_command_complete,
     .cancel = esp_request_cancelled,
 };
 
-- 
2.39.2

Re: [PATCH 3/4] esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion interrupt

[Guenter Roeck]

On Fri, Jan 12, 2024 at 01:15:28PM +0000, Mark Cave-Ayland wrote:
> The setting of DMA_STAT_DONE at the end of a DMA transfer can be configured to
> generate an interrupt, however the Linux driver manually checks for DMA_STAT_DONE
> being set and if it is, considers that a DMA transfer has completed.
> 
> If DMA_STAT_DONE is set but the ESP device isn't indicating an interrupt then
> the Linux driver considers this to be a spurious interrupt. However this can
> occur in QEMU as there is a delay between the end of DMA transfer where
> DMA_STAT_DONE is set, and the ESP device raising its completion interrupt.
> 
> This appears to be an incorrect assumption in the Linux driver as the ESP and
> PCI DMA interrupt sources are separate (and may not be raised exactly
> together), however we can work around this by synchronising the setting of
> DMA_STAT_DONE at the end of a DMA transfer with the ESP completion interrupt.
> 
> In conjunction with the previous commit Linux is now able to correctly boot
> from an am53c974 PCI SCSI device on the hppa C3700 machine without emitting
> "iget: checksum invalid" and "Spurious irq, sreg=10" errors.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  hw/scsi/esp-pci.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 15dc3c004d..875a49199d 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -93,6 +93,18 @@ static void esp_irq_handler(void *opaque, int irq_num, int level)
>  
>      if (level) {
>          pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT;
> +
> +        /*
> +         * If raising the ESP IRQ to indicate end of DMA transfer, set
> +         * DMA_STAT_DONE at the same time. In theory this should be done in
> +         * esp_pci_dma_memory_rw(), however there is a delay between setting
> +         * DMA_STAT_DONE and the ESP IRQ arriving which is visible to the
> +         * guest that can cause confusion e.g. Linux
> +         */
> +        if ((pci->dma_regs[DMA_CMD] & DMA_CMD_MASK) == 0x3 &&
> +            pci->dma_regs[DMA_WBC] == 0) {
> +                pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
> +        }
>      } else {
>          pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT;
>      }
> @@ -306,9 +318,6 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
>      /* update status registers */
>      pci->dma_regs[DMA_WBC] -= len;
>      pci->dma_regs[DMA_WAC] += len;
> -    if (pci->dma_regs[DMA_WBC] == 0) {
> -        pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
> -    }
>  }
>  
>  static void esp_pci_dma_memory_read(void *opaque, uint8_t *buf, int len)
> @@ -363,24 +372,13 @@ static const VMStateDescription vmstate_esp_pci_scsi = {
>      }
>  };
>  
> -static void esp_pci_command_complete(SCSIRequest *req, size_t resid)
> -{
> -    ESPState *s = req->hba_private;
> -    PCIESPState *pci = container_of(s, PCIESPState, esp);
> -
> -    esp_command_complete(req, resid);
> -    pci->dma_regs[DMA_WBC] = 0;
> -    pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
> -    esp_pci_update_irq(pci);
> -}
> -
>  static const struct SCSIBusInfo esp_pci_scsi_info = {
>      .tcq = false,
>      .max_target = ESP_MAX_DEVS,
>      .max_lun = 7,
>  
>      .transfer_data = esp_transfer_data,
> -    .complete = esp_pci_command_complete,
> +    .complete = esp_command_complete,
>      .cancel = esp_request_cancelled,
>  };
>  
> -- 
> 2.39.2
> 

[PATCH 4/4] esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued

[Mark Cave-Ayland]

Even though the BLAST command isn't fully implemented in QEMU, the DMA_STAT_BCMBLT
bit should be set after the command has been issued to indicate that the command
has completed.

This fixes an issue with the DC390 DOS driver which issues the BLAST command as
part of its normal error recovery routine at startup, and otherwise sits in a
tight loop waiting for DMA_STAT_BCMBLT to be set before continuing.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/esp-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 875a49199d..42d9d2e483 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -124,6 +124,7 @@ static void esp_pci_handle_blast(PCIESPState *pci, uint32_t val)
 {
     trace_esp_pci_dma_blast(val);
     qemu_log_mask(LOG_UNIMP, "am53c974: cmd BLAST not implemented\n");
+    pci->dma_regs[DMA_STAT] |= DMA_STAT_BCMBLT;
 }
 
 static void esp_pci_handle_abort(PCIESPState *pci, uint32_t val)
-- 
2.39.2

Re: [PATCH 4/4] esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued

[Guenter Roeck]

On Fri, Jan 12, 2024 at 01:15:29PM +0000, Mark Cave-Ayland wrote:
> Even though the BLAST command isn't fully implemented in QEMU, the DMA_STAT_BCMBLT
> bit should be set after the command has been issued to indicate that the command
> has completed.
> 
> This fixes an issue with the DC390 DOS driver which issues the BLAST command as
> part of its normal error recovery routine at startup, and otherwise sits in a
> tight loop waiting for DMA_STAT_BCMBLT to be set before continuing.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  hw/scsi/esp-pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 875a49199d..42d9d2e483 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -124,6 +124,7 @@ static void esp_pci_handle_blast(PCIESPState *pci, uint32_t val)
>  {
>      trace_esp_pci_dma_blast(val);
>      qemu_log_mask(LOG_UNIMP, "am53c974: cmd BLAST not implemented\n");
> +    pci->dma_regs[DMA_STAT] |= DMA_STAT_BCMBLT;
>  }
>  
>  static void esp_pci_handle_abort(PCIESPState *pci, uint32_t val)
> -- 
> 2.39.2
> 

Re: [PATCH 0/4] esp-pci: fixes for Linux and MS-DOS

[Philippe Mathieu-Daudé]

On 12/1/24 14:15, Mark Cave-Ayland wrote:
> This series contains fixes for the esp-pci device (am53c974 or dc390) for a
> few issues spotted whilst testing the previous ESP series.
> 
> Patches 1-3 are fixes for issues found by Helge/Guenter whilst testing the
> hppa C3700 machine with the amd53c974/dc390 devices under Linux, whilst patch
> 4 fixes an issue that was exposed by testing MS-DOS and Windows drivers.
> 
> With this series applied on top of the reworked ESP device, it is possible to
> boot Linux under qemu-system-hppa without any errors and also boot and install
> Win98SE from a DC390 PCI SCSI controller (no IDE!) using an MS-DOS boot floppy.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Based-on: 20240112125420.514425-1-mark.cave-ayland@ilande.co.uk
> 
> 
> Mark Cave-Ayland (4):
>    esp-pci.c: use correct address register for PCI DMA transfers
>    esp-pci.c: generate PCI interrupt from separate ESP and PCI sources
>    esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion
>      interrupt
>    esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued
> 
>   hw/scsi/esp-pci.c | 61 +++++++++++++++++++++++++++++++----------------
>   1 file changed, 41 insertions(+), 20 deletions(-)

Series queued to my hw-misc tree, thanks!

Re: [PATCH 0/4] esp-pci: fixes for Linux and MS-DOS

[Michael Tokarev]

12.01.2024 16:15, Mark Cave-Ayland:
> This series contains fixes for the esp-pci device (am53c974 or dc390) for a
> few issues spotted whilst testing the previous ESP series.
> 
> Patches 1-3 are fixes for issues found by Helge/Guenter whilst testing the
> hppa C3700 machine with the amd53c974/dc390 devices under Linux, whilst patch
> 4 fixes an issue that was exposed by testing MS-DOS and Windows drivers.
> 
> With this series applied on top of the reworked ESP device, it is possible to
> boot Linux under qemu-system-hppa without any errors and also boot and install
> Win98SE from a DC390 PCI SCSI controller (no IDE!) using an MS-DOS boot floppy.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Based-on: 20240112125420.514425-1-mark.cave-ayland@ilande.co.uk
> 
> 
> Mark Cave-Ayland (4):
>    esp-pci.c: use correct address register for PCI DMA transfers
>    esp-pci.c: generate PCI interrupt from separate ESP and PCI sources
>    esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion
>      interrupt
>    esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued

Is it worth to pick up for stable?  Especially the first one.
It's interesting this bug is here for a very long time.. :)

Thanks,

/mjt

Re: [PATCH 0/4] esp-pci: fixes for Linux and MS-DOS

[Guenter Roeck]

On 1/20/24 05:09, Michael Tokarev wrote:
> 12.01.2024 16:15, Mark Cave-Ayland:
>> This series contains fixes for the esp-pci device (am53c974 or dc390) for a
>> few issues spotted whilst testing the previous ESP series.
>>
>> Patches 1-3 are fixes for issues found by Helge/Guenter whilst testing the
>> hppa C3700 machine with the amd53c974/dc390 devices under Linux, whilst patch
>> 4 fixes an issue that was exposed by testing MS-DOS and Windows drivers.
>>
>> With this series applied on top of the reworked ESP device, it is possible to
>> boot Linux under qemu-system-hppa without any errors and also boot and install
>> Win98SE from a DC390 PCI SCSI controller (no IDE!) using an MS-DOS boot floppy.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Based-on: 20240112125420.514425-1-mark.cave-ayland@ilande.co.uk
>>
>>
>> Mark Cave-Ayland (4):
>>    esp-pci.c: use correct address register for PCI DMA transfers
>>    esp-pci.c: generate PCI interrupt from separate ESP and PCI sources
>>    esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion
>>      interrupt
>>    esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued
> 
> Is it worth to pick up for stable?  Especially the first one.
> It's interesting this bug is here for a very long time.. :)
> 

FWIW, I never observed the first one with Linux. I had carried variants
of the other three in my tree for a long time, but they were never
in a shape to be sent upstream and I never bothered trying to find
the root cause. All those _can_ be observed when booting Linux. So,
if anything, I'd argue that they should all be taken into stable
releases.

Thanks,
Guenter

Re: [PATCH 0/4] esp-pci: fixes for Linux and MS-DOS

[Mark Cave-Ayland]

On 20/01/2024 13:09, Michael Tokarev wrote:

> 12.01.2024 16:15, Mark Cave-Ayland:
>> This series contains fixes for the esp-pci device (am53c974 or dc390) for a
>> few issues spotted whilst testing the previous ESP series.
>>
>> Patches 1-3 are fixes for issues found by Helge/Guenter whilst testing the
>> hppa C3700 machine with the amd53c974/dc390 devices under Linux, whilst patch
>> 4 fixes an issue that was exposed by testing MS-DOS and Windows drivers.
>>
>> With this series applied on top of the reworked ESP device, it is possible to
>> boot Linux under qemu-system-hppa without any errors and also boot and install
>> Win98SE from a DC390 PCI SCSI controller (no IDE!) using an MS-DOS boot floppy.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Based-on: 20240112125420.514425-1-mark.cave-ayland@ilande.co.uk
>>
>>
>> Mark Cave-Ayland (4):
>>    esp-pci.c: use correct address register for PCI DMA transfers
>>    esp-pci.c: generate PCI interrupt from separate ESP and PCI sources
>>    esp-pci.c: synchronise setting of DMA_STAT_DONE with ESP completion
>>      interrupt
>>    esp-pci.c: set DMA_STAT_BCMBLT when BLAST command issued
> 
> Is it worth to pick up for stable?  Especially the first one.
> It's interesting this bug is here for a very long time.. :)

Good question! I did my comprehensive boot tests with this series on top of the core 
ESP series so I can't say that I've tested this series on its own. Then again other 
than the DMA_STAT_DONE patch which is a timing change, the rest of the patches are 
fixing specific edge cases which were already broken so I would be surprised if 
anything broke.


ATB,

Mark.