QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: qemu-devel@nongnu.org, pbonzini@redhat.com, fam@euphon.net,
	laurent@vivier.eu
Subject: [PATCH v3 00/42] esp: consolidate PDMA transfer buffers and other fixes
Date: Thu,  4 Mar 2021 22:10:21 +0000
Message-ID: <20210304221103.6369-1-mark.cave-ayland@ilande.co.uk> (raw)

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.

- 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.

  
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.

Many thanks to Guenter Roeck <linux@roeck-us.net> who provided me a test image for
the deferred interrupt test case, and also confirmed the updated version still
worked fine in his tests.


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>


v3:
- Rebase onto master (fix up minor conflicts with Paolo's SCSI error handling changes)
- Add R-B tags from Philippe and Laurent
- Check for failure of qdev_realize() in patch 3
- Touch up the commit messages on patches 9 and 10
- Remove extra "& 0xff" in patch 9
- Add deferred command completion interrupt for PDMA in patch 33
- Remove ti_size assignment comment in patch 38
- Remove extra "& 0xff" in patch 39


v2:
- Rebase onto master
- Add R-B tags from Philippe
- Add QOMification, Fifo8 conversion, deferred interrupt for incoming data, message
  out phase fixes, non-DMA commands, and unaligned access support


Mark Cave-Ayland (42):
  esp: checkpatch fixes
  esp: rename existing ESP QOM type to SYSBUS_ESP
  esp: QOMify the internal ESP device state
  esp: add vmstate_esp version to embedded ESPState
  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 buf parameter from do_cmd()
  esp: remove the buf and buflen parameters from get_cmd()
  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
  esp: implement FIFO flush command
  esp: latch individual bits in ESP_RINTR register
  esp: defer command completion interrupt on incoming data transfers
  esp: remove old deferred command completion mechanism
  esp: raise interrupt after every non-DMA byte transferred to the FIFO
  esp: add maxlen parameter to get_cmd()
  esp: transition to message out phase after SATN and stop command
  esp: convert ti_buf from array to Fifo8
  esp: convert cmdbuf from array to Fifo8
  esp: add trivial implementation of the ESP_RFLAGS register
  esp: implement non-DMA transfers in PDMA mode
  esp: add support for unaligned accesses

 hw/dma/sparc32_dma.c  |   4 +-
 hw/m68k/q800.c        |   4 +-
 hw/mips/jazz.c        |   4 +-
 hw/scsi/esp-pci.c     |  53 ++-
 hw/scsi/esp.c         | 975 +++++++++++++++++++++++++++++-------------
 hw/scsi/trace-events  |   5 +
 hw/sparc/sun4m.c      |   2 +-
 include/hw/scsi/esp.h |  52 +--
 8 files changed, 748 insertions(+), 351 deletions(-)

-- 
2.20.1



             reply index

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-04 22:10 Mark Cave-Ayland [this message]
2021-03-04 22:10 ` [PATCH v3 01/42] esp: checkpatch fixes Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 02/42] esp: rename existing ESP QOM type to SYSBUS_ESP Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 03/42] esp: QOMify the internal ESP device state Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 04/42] esp: add vmstate_esp version to embedded ESPState Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 05/42] esp: add trace event when receiving a TI command Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 06/42] esp: fix esp_reg_read() trace event Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 07/42] esp: add PDMA trace events Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 08/42] esp: determine transfer direction directly from SCSI phase Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 09/42] esp: introduce esp_get_tc() and esp_set_tc() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 10/42] esp: introduce esp_get_stc() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 11/42] esp: apply transfer length adjustment when STC is zero at TC load time Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 12/42] esp: remove dma_counter from ESPState Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 13/42] esp: remove dma_left " Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 14/42] esp: remove minlen restriction in handle_ti Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 15/42] esp: introduce esp_pdma_read() and esp_pdma_write() functions Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 16/42] esp: use pdma_origin directly in esp_pdma_read()/esp_pdma_write() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 17/42] esp: move pdma_len and TC logic into esp_pdma_read()/esp_pdma_write() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 18/42] esp: accumulate SCSI commands for PDMA transfers in cmdbuf instead of pdma_buf Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 19/42] esp: remove buf parameter from do_cmd() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 20/42] esp: remove the buf and buflen parameters from get_cmd() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 21/42] esp: remove redundant pdma_start from ESPState Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 22/42] esp: move PDMA length adjustments into esp_pdma_read()/esp_pdma_write() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 23/42] esp: use ti_wptr/ti_rptr to manage the current FIFO position for PDMA Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 24/42] esp: use in-built TC to determine PDMA transfer length Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 25/42] esp: remove CMD pdma_origin Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 26/42] esp: rename get_cmd_cb() to esp_select() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 27/42] esp: fix PDMA target selection Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 28/42] esp: use FIFO for PDMA transfers between initiator and device Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 29/42] esp: remove pdma_origin from ESPState Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 30/42] esp: add 4 byte PDMA read and write transfers Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 31/42] esp: implement FIFO flush command Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 32/42] esp: latch individual bits in ESP_RINTR register Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 33/42] esp: defer command completion interrupt on incoming data transfers Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 34/42] esp: remove old deferred command completion mechanism Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 35/42] esp: raise interrupt after every non-DMA byte transferred to the FIFO Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 36/42] esp: add maxlen parameter to get_cmd() Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 37/42] esp: transition to message out phase after SATN and stop command Mark Cave-Ayland
2021-03-04 22:10 ` [PATCH v3 38/42] esp: convert ti_buf from array to Fifo8 Mark Cave-Ayland
2021-03-04 22:11 ` [PATCH v3 39/42] esp: convert cmdbuf " Mark Cave-Ayland
2021-03-04 22:11 ` [PATCH v3 40/42] esp: add trivial implementation of the ESP_RFLAGS register Mark Cave-Ayland
2021-03-04 22:11 ` [PATCH v3 41/42] esp: implement non-DMA transfers in PDMA mode Mark Cave-Ayland
2021-03-04 22:11 ` [PATCH v3 42/42] esp: add support for unaligned accesses Mark Cave-Ayland
2021-03-04 22:58 ` [PATCH v3 00/42] esp: consolidate PDMA transfer buffers and other fixes Philippe Mathieu-Daudé
2021-03-05  7:31   ` Mark Cave-Ayland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210304221103.6369-1-mark.cave-ayland@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=fam@euphon.net \
    --cc=laurent@vivier.eu \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git