mbox series

[00/25] esp: consolidate PDMA transfer buffers

Message ID 20201230153745.30241-1-mark.cave-ayland@ilande.co.uk
Headers show
Series esp: consolidate PDMA transfer buffers | expand

Message

Mark Cave-Ayland Dec. 30, 2020, 3:37 p.m. UTC
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-4 are minor patches which make esp.c checkpatch friendly whilst also
  fixing up some trace events ready for later patches in the series
  
- Patches 5-11 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
  12-20 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.

- Patches 21 and 22 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 23 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 24 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).
  
- Finally patch 25 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).


Testing
=======

I've tested this on my SPARC32 OpenBIOS images which include Linux, OpenBSD, NetBSD,
and Solaris and all of these continue to boot as before.

Similarly the q800 m68k Linux test image still boots as before with these patches
applied. It is possible with lots of hacks to load Laurent's EMILE bootloader using
a MacOS toolbox ROM - the hope is to try and start upstreaming more of these changes
as time allows.


Migration
=========

The patchset ensures that ESP devices without PDMA (i.e. everything except the q800
machine) will migrate successfully. This is fairly simple: the only change required
here is to copy the old synthetic dma_left value over into the TC.

Unfortunately migrating the PDMA subsection is a lot harder due to the change in the
way that the DMA TC and changes to the point at which transfer counters are updated.
For this reason the patchset will not migrate from older q800 snapshots: I don't
believe this to be a problem since some devices are still missing VMStateDescription
plus there are likely to be more breaking changes as the q800 machine matures.


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


Mark Cave-Ayland (25):
  esp: checkpatch fixes
  esp: add trace event when receiving a TI command
  esp: fix esp_reg_read() trace event
  esp: add PDMA trace events
  esp: determine transfer direction directly from SCSI phase
  esp: introduce esp_get_tc() and esp_set_tc()
  esp: introduce esp_get_stc()
  esp: apply transfer length adjustment when STC is zero at TC load time
  esp: remove dma_counter from ESPState
  esp: remove dma_left from ESPState
  esp: remove minlen restriction in handle_ti
  esp: introduce esp_pdma_read() and esp_pdma_write() functions
  esp: use pdma_origin directly in esp_pdma_read()/esp_pdma_write()
  esp: move pdma_len and TC logic into esp_pdma_read()/esp_pdma_write()
  esp: accumulate SCSI commands for PDMA transfers in cmdbuf instead of
    pdma_buf
  esp: remove redundant pdma_start from ESPState
  esp: move PDMA length adjustments into
    esp_pdma_read()/esp_pdma_write()
  esp: use ti_wptr/ti_rptr to manage the current FIFO position for PDMA
  esp: use in-built TC to determine PDMA transfer length
  esp: remove CMD pdma_origin
  esp: rename get_cmd_cb() to esp_select()
  esp: fix PDMA target selection
  esp: use FIFO for PDMA transfers between initiator and device
  esp: remove pdma_origin from ESPState
  esp: add 4 byte PDMA read and write transfers

 hw/scsi/esp.c         | 461 +++++++++++++++++++++++++-----------------
 hw/scsi/trace-events  |   5 +
 include/hw/scsi/esp.h |  20 +-
 3 files changed, 279 insertions(+), 207 deletions(-)

Comments

Mark Cave-Ayland Jan. 6, 2021, 3:18 p.m. UTC | #1
On 30/12/2020 15:37, 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-4 are minor patches which make esp.c checkpatch friendly whilst also
>    fixing up some trace events ready for later patches in the series
>    
> - Patches 5-11 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
>    12-20 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.
> 
> - Patches 21 and 22 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 23 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 24 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).
>    
> - Finally patch 25 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).
> 
> 
> Testing
> =======
> 
> I've tested this on my SPARC32 OpenBIOS images which include Linux, OpenBSD, NetBSD,
> and Solaris and all of these continue to boot as before.
> 
> Similarly the q800 m68k Linux test image still boots as before with these patches
> applied. It is possible with lots of hacks to load Laurent's EMILE bootloader using
> a MacOS toolbox ROM - the hope is to try and start upstreaming more of these changes
> as time allows.
> 
> 
> Migration
> =========
> 
> The patchset ensures that ESP devices without PDMA (i.e. everything except the q800
> machine) will migrate successfully. This is fairly simple: the only change required
> here is to copy the old synthetic dma_left value over into the TC.
> 
> Unfortunately migrating the PDMA subsection is a lot harder due to the change in the
> way that the DMA TC and changes to the point at which transfer counters are updated.
> For this reason the patchset will not migrate from older q800 snapshots: I don't
> believe this to be a problem since some devices are still missing VMStateDescription
> plus there are likely to be more breaking changes as the q800 machine matures.
> 
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> 
> Mark Cave-Ayland (25):
>    esp: checkpatch fixes
>    esp: add trace event when receiving a TI command
>    esp: fix esp_reg_read() trace event
>    esp: add PDMA trace events
>    esp: determine transfer direction directly from SCSI phase
>    esp: introduce esp_get_tc() and esp_set_tc()
>    esp: introduce esp_get_stc()
>    esp: apply transfer length adjustment when STC is zero at TC load time
>    esp: remove dma_counter from ESPState
>    esp: remove dma_left from ESPState
>    esp: remove minlen restriction in handle_ti
>    esp: introduce esp_pdma_read() and esp_pdma_write() functions
>    esp: use pdma_origin directly in esp_pdma_read()/esp_pdma_write()
>    esp: move pdma_len and TC logic into esp_pdma_read()/esp_pdma_write()
>    esp: accumulate SCSI commands for PDMA transfers in cmdbuf instead of
>      pdma_buf
>    esp: remove redundant pdma_start from ESPState
>    esp: move PDMA length adjustments into
>      esp_pdma_read()/esp_pdma_write()
>    esp: use ti_wptr/ti_rptr to manage the current FIFO position for PDMA
>    esp: use in-built TC to determine PDMA transfer length
>    esp: remove CMD pdma_origin
>    esp: rename get_cmd_cb() to esp_select()
>    esp: fix PDMA target selection
>    esp: use FIFO for PDMA transfers between initiator and device
>    esp: remove pdma_origin from ESPState
>    esp: add 4 byte PDMA read and write transfers
> 
>   hw/scsi/esp.c         | 461 +++++++++++++++++++++++++-----------------
>   hw/scsi/trace-events  |   5 +
>   include/hw/scsi/esp.h |  20 +-
>   3 files changed, 279 insertions(+), 207 deletions(-)

Please ignore this patchset for now - whilst the changes allow mixed PDMA and FIFO 
requests, they assume that the FIFO request sizes have the same alignment as the PDMA 
request sizes. I've just found a case where this isn't true in the MacOS toolbox ROM, 
so I'll investigate further and then resubmit.


ATB,

Mark.
Paolo Bonzini Jan. 13, 2021, 2:39 p.m. UTC | #2
On 30/12/20 16:37, 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.

This is all esp.c, so if Laurent and you are fine just send a pull request.

Thanks!

Paolo



> The patchset is split into several sections:
> 
> - Patches 1-4 are minor patches which make esp.c checkpatch friendly whilst also
>    fixing up some trace events ready for later patches in the series
>    
> - Patches 5-11 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
>    12-20 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.
> 
> - Patches 21 and 22 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 23 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 24 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).
>    
> - Finally patch 25 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).
> 
> 
> Testing
> =======
> 
> I've tested this on my SPARC32 OpenBIOS images which include Linux, OpenBSD, NetBSD,
> and Solaris and all of these continue to boot as before.
> 
> Similarly the q800 m68k Linux test image still boots as before with these patches
> applied. It is possible with lots of hacks to load Laurent's EMILE bootloader using
> a MacOS toolbox ROM - the hope is to try and start upstreaming more of these changes
> as time allows.
> 
> 
> Migration
> =========
> 
> The patchset ensures that ESP devices without PDMA (i.e. everything except the q800
> machine) will migrate successfully. This is fairly simple: the only change required
> here is to copy the old synthetic dma_left value over into the TC.
> 
> Unfortunately migrating the PDMA subsection is a lot harder due to the change in the
> way that the DMA TC and changes to the point at which transfer counters are updated.
> For this reason the patchset will not migrate from older q800 snapshots: I don't
> believe this to be a problem since some devices are still missing VMStateDescription
> plus there are likely to be more breaking changes as the q800 machine matures.
> 
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> 
> Mark Cave-Ayland (25):
>    esp: checkpatch fixes
>    esp: add trace event when receiving a TI command
>    esp: fix esp_reg_read() trace event
>    esp: add PDMA trace events
>    esp: determine transfer direction directly from SCSI phase
>    esp: introduce esp_get_tc() and esp_set_tc()
>    esp: introduce esp_get_stc()
>    esp: apply transfer length adjustment when STC is zero at TC load time
>    esp: remove dma_counter from ESPState
>    esp: remove dma_left from ESPState
>    esp: remove minlen restriction in handle_ti
>    esp: introduce esp_pdma_read() and esp_pdma_write() functions
>    esp: use pdma_origin directly in esp_pdma_read()/esp_pdma_write()
>    esp: move pdma_len and TC logic into esp_pdma_read()/esp_pdma_write()
>    esp: accumulate SCSI commands for PDMA transfers in cmdbuf instead of
>      pdma_buf
>    esp: remove redundant pdma_start from ESPState
>    esp: move PDMA length adjustments into
>      esp_pdma_read()/esp_pdma_write()
>    esp: use ti_wptr/ti_rptr to manage the current FIFO position for PDMA
>    esp: use in-built TC to determine PDMA transfer length
>    esp: remove CMD pdma_origin
>    esp: rename get_cmd_cb() to esp_select()
>    esp: fix PDMA target selection
>    esp: use FIFO for PDMA transfers between initiator and device
>    esp: remove pdma_origin from ESPState
>    esp: add 4 byte PDMA read and write transfers
> 
>   hw/scsi/esp.c         | 461 +++++++++++++++++++++++++-----------------
>   hw/scsi/trace-events  |   5 +
>   include/hw/scsi/esp.h |  20 +-
>   3 files changed, 279 insertions(+), 207 deletions(-)
>
Mark Cave-Ayland Jan. 13, 2021, 7:29 p.m. UTC | #3
On 13/01/2021 14:39, Paolo Bonzini wrote:

> On 30/12/20 16:37, 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.
> 
> This is all esp.c, so if Laurent and you are fine just send a pull request.
> 
> Thanks!
> 
> Paolo

I sent a self-nak a little while back because I found an issue with address alignment 
in some requests coming from the MacOS toolbox ROM. I think I now understand what the 
issue is, so I hope to post a v2 with this fixed soon.


ATB,

Mark.