On Sun, Mar 24, 2024 at 8:17 PM Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
> Patches 1-4 update existing users of esp_fifo_pop_buf() and esp_fifo_pop()
> with
> the internal cmdfifo to use the underlying Fifo8 device directly. The aim is
> to ensure that all the esp_fifo_*() functions only operate on the ESP's
> hardware
> FIFO.
>
> Patches 5-6 update esp_fifo_push() and esp_fifo_pop() to take ESPState
> directly
> as a parameter to prevent any usage that doesn't reference the ESP hardware
> FIFO.
>
> Patch 7 ensures that any usage of fifo8_push() for the ESP hardware FIFO is
> updated to use esp_fifo_push() instead.
>
> Patch 8 updates esp_fifo_pop_buf() to take ESPState directly as a parameter
> to prevent any usage that doesn't reference the ESP hardware FIFO.
>
> Patch 9 introduces the esp_fifo_push_buf() function for pushing multiple bytes
> to the ESP hardware FIFO, and updates callers to use it accordingly.
>
> Patches 10-12 incorporate additional protection to prevent FIFO assert()s and
> a
> cmdfifo buffer overflow discovered via fuzzing.
>
> Patch 13 is just code movement which avoids the use of a forward declaration
> whilst
> also making it easier to locate the mapping between ESP SCSI phases and their
> names.
>
> Patches 14 introduce a new esp_update_drq() function that implements the above
> DRQ logic which is called by both esp_fifo_{push,pop}_buf().
>
> Patch 15 updates esp_fifo_push() and esp_fifo_pop() to use the new
> esp_update_drq()
> function. At this point all reads/writes to the ESP FIFO use the esp_fifo_*()
> functions and will set DRQ correctly.
>
> Patch 16 is a small update to the logic in esp_pdma_write() to ensure that
> esp_fifo_push() is always called for PDMA writes to the FIFO, thereby ensuring
> that esp_update_drq() remains correct even in the case of FIFO overflow.
>
> Finally patch 17 removes all manual calls to esp_raise_drq() and
> esp_lower_drq()
> since the DRQ signal is now updated correctly upon each FIFO read/write
> access.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
> v3:
> - Rebase onto master
> - Add patch 1 to move the internals of esp_fifo_pop_buf() to a new
> esp_fifo8_pop_buf() function. This allows the FIFO wrap logic to still be
> used for managing cmdfifo
> - Rework esp_cdb_length() into esp_cdb_ready() as suggested by Paolo in patch
> 11
> - Update the logic in patch 12 to use the new esp_cdb_ready() function
>
> v2:
> - Rebase onto master
> - Add patches 9-12 to handle FIFO assert()s and cmdfifo overflow as reported
> by
> Chuhong Yuan <hsleste...@gmail.com>
>
>
> Mark Cave-Ayland (17):
> esp.c: move esp_fifo_pop_buf() internals to new esp_fifo8_pop_buf()
> function
> esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in
> do_command_phase()
> esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in
> do_message_phase()
> esp.c: replace cmdfifo use of esp_fifo_pop() in do_message_phase()
> esp.c: change esp_fifo_push() to take ESPState
> esp.c: change esp_fifo_pop() to take ESPState
> esp.c: use esp_fifo_push() instead of fifo8_push()
> esp.c: change esp_fifo_pop_buf() to take ESPState
> esp.c: introduce esp_fifo_push_buf() function for pushing to the FIFO
> esp.c: don't assert() if FIFO empty when executing non-DMA SELATNS
> esp.c: rework esp_cdb_length() into esp_cdb_ready()
> esp.c: prevent cmdfifo overflow in esp_cdb_ready()
> esp.c: move esp_set_phase() and esp_get_phase() towards the beginning
> of the file
> esp.c: introduce esp_update_drq() and update esp_fifo_{push,pop}_buf()
> to use it
> esp.c: update esp_fifo_{push,pop}() to call esp_update_drq()
> esp.c: ensure esp_pdma_write() always calls esp_fifo_push()
> esp.c: remove explicit setting of DRQ within ESP state machine
>
> hw/scsi/esp.c | 223 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 142 insertions(+), 81 deletions(-)
>
> --
> 2.39.2
>
