Re: [PATCH v2 00/42] esp: consolidate PDMA transfer buffers and other fixes

[Philippe Mathieu-Daudé]

Hi Mark,

On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
> This patch series comes from an experimental branch that I've been working on
> to try and boot a MacOS toolbox ROM under the QEMU q800 machine. The effort is
> far from complete, but it seems worth submitting these patches separately 
> since
> they are limited to the ESP device and form a substantial part of the work to
> date.
> 
> As part of Laurent's recent q800 work so-called PDMA (pseudo-DMA) support was
> added to the ESP device. This is whereby the DREQ (DMA request) line is used
> to signal to the host CPU that it can transfer data to/from the device over
> the SCSI bus.
> 
> The existing PDMA tracks 4 separate transfer data sources as indicated by the
> ESP pdma_origin variable: PDMA, TI, CMD and ASYNC with an independent variable
> pdma_len to store the transfer length. This works well with Linux which uses a
> single PDMA request to transfer a number of sectors in a single request.
> 
> Unfortunately the MacOS toolbox ROM has other ideas here: it sends data to the
> ESP as a mixture of FIFO and PDMA transfers and then uses a mixture of the 
> FIFO
> and DMA counters to confirm that the correct number of bytes have been
> transferred. For this to work correctly the PDMA buffers and separate pdma_len
> transfer counter must be consolidated into the FIFO to allow mixing of both
> types of transfer within a single request.
> 
> The patchset is split into several sections:
> 
> - Patches 1-7 are minor patches which make esp.c checkpatch friendly, QOMify 
> ESPState,
>   and also fix up some trace events ready for later patches in the series
> 
> - Patches 8-13 unify the DMA transfer count. In particular there are 2 
> synthetic
>   variables dma_counter and dma_left within ESPState which do not need to 
> exist. 
>   DMA transfer lengths are programmed into the TC (transfer count) register 
> which is 
>   decremented for each byte transferred, generating an interrupt when it 
> reaches zero.
>   These patches add helper functions to read the TC and STC registers 
> directly and
>   remove these synthetic variables so that the DMA transfer length is now 
> tracked in
>   a single place.
> 
> - Now that the TC register represents the authoritative DMA transfer length, 
> patches
>   14-25 work to eliminate the separate PDMA variables pdma_start, pdma_cur, 
> pdma_len
>   and separate PDMA buffers PDMA and CMD. The PDMA position variables can be 
> replaced
>   by the existing ESP cmdlen and ti_wptr/ti_rptr, whilst the FIFO (TI) buffer 
> is used
>   for incoming data with commands being accumulated in cmdbuf as per standard 
> DMA
>   requests.

I tried to help reviewing up to this point.

The next parts are too specific to me.

> - Patches 26 and 27 fix the detection of missing SCSI targets by the MacOS 
> toolbox ROM
>   on startup at which point it will attempt to start reading information from 
> a CDROM
>   attached to the q800 machine.
> 
> - Patch 28 is the main rework of the PDMA buffer transfers: instead of 
> tracking the
>   SCSI transfers using a separate ASYNC pdma_origin, the contents of the 
> ESPState
>   async_buf are copied to the FIFO buffer in 16-byte chunks with the transfer 
> status
>   and IRQs being set accordingly.
> 
> - Patch 29 removes the last separate PDMA variable pdma_origin, including the 
> separate
>   PDMA migration subsection which is no longer required (see note below about 
> migration
>   compatibility).
>   
> - Patch 30 enables 4 byte PDMA reads/writes over the SCSI bus which are used 
> by MacOS
>   when reading the next stage bootloader from CDROM (this is an increase from
>   2 bytes currently implemented and used by Linux).
> 
> - Patches 31-34 fix an issue whereby the MacOS toolbox ROM tries to read 
> incoming data
>   from the target within a few instructions of receiving the command complete 
> interrupt.
>   Since IO is asynchronous in QEMU, it is necessary to delay the command 
> complete
>   interrupt for incoming data to avoid underflow.
> 
> - Patches 35-37 fix a problem with the SATN and stop command not changing the 
> SCSI bus
>   to message out phase. This actually first manifested itself after the Fifo8 
> conversion
>   with guests that mix DMA/non-DMA commands but it is moved forward to aid 
> bisection.
> 
> - Patches 38-39 convert ti_buf and cmdbuf from simple arrays to QEMU's Fifo8 
> type which
>   helped locate a handful of bugs around handling the buffer pointers which 
> are
>   incorpated within earlier patches within the series.
>   
> - Finally patches 40-42 add support for the FIFO count registers, non-DMA 
> transfers and
>   unaligned accesses which are required for the MacOS toolbox ROM to 
> successful read
>   files from disk.