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-ayl...@ilande.co.uk>
Reviewed-by: Guenter Roeck <li...@roeck-us.net>
Tested-by: Guenter Roeck <li...@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
>
