Re: [Qemu-devel] [PATCH v6 05/10] esp: add pseudo-DMA as used by Macintosh

[Laurent Vivier <laurent@vivier.eu>]

On 25/01/2019 06:48, Thomas Huth wrote:
> On 2018-11-02 16:22, Mark Cave-Ayland wrote:
>> From: Laurent Vivier <laurent@vivier.eu>
> 
> I'd suggest to add a patch description that contains the text that
> Laurent provided as a reply to this patch in v5:
> 
> ---------------------------- 8< --------------------------------------
> There is no DMA in Quadra 800, so the CPU reads/writes the data from the
> PDMA register (offset 0x100, ESP_PDMA in hw/m68k/q800.c) and copies them
> to/from the memory.
> 
> There is a nice assembly loop in the kernel to do that, see
> linux/drivers/scsi/mac_esp.c:MAC_ESP_PDMA_LOOP().
> 
> The start of the transfer is triggered by the DREQ interrupt (see linux
> mac_esp_send_pdma_cmd()), the CPU polls on the IRQ flag to start the
> transfer after a SCSI command has been sent (in Quadra 800 it goes
> through the VIA2, the via2-irq line and the vIFR register)
> 
> The Macintosh hardware includes hardware handshaking to prevent the CPU
> from reading invalid data or writing data faster than the peripheral
> device can accept it.
> 
> This is the "blind mode", and from the doc:
> "Approximate maximum SCSI transfer rates within a blocks are 1.4 MB per
> second for blind transfers in the Macintosh II"
> 
> Some references can be found in:
>    Apple Macintosh Family Hardware Reference, ISBN 0-201-19255-1
>    Guide to the Macintosh Family Hardware, ISBN-0-201-52405-8
> ---------------------------- >8 --------------------------------------
> 
> ?
> 
>> Co-developed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   hw/scsi/esp.c         | 291 +++++++++++++++++++++++++++++++++++++++++++++-----
>>   include/hw/scsi/esp.h |   7 ++
>>   2 files changed, 269 insertions(+), 29 deletions(-)
>>
>> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
>> index 630d923623..8e9e27e479 100644
>> --- a/hw/scsi/esp.c
>> +++ b/hw/scsi/esp.c
> [...]
>> @@ -356,8 +511,7 @@ static void handle_ti(ESPState *s)
>>           s->dma_left = minlen;
>>           s->rregs[ESP_RSTAT] &= ~STAT_TC;
>>           esp_do_dma(s);
>> -    }
>> -    if (s->do_cmd) {
>> +    } else if (s->do_cmd) {
> 
> I'm not sure about this change... is it required? It could also change
> the behavior of the other users of this device...?
> 

The "else" is needed because this code has been duplicated inside 
esp_do_dma() to be executed only in the case of "real" dma and not for 
pseudo-dma.

Thanks,
Laurent