[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 15/42] esp: introduce esp_pdma_read() and esp_pdma_write()

From: Mark Cave-Ayland
Subject: Re: [PATCH v2 15/42] esp: introduce esp_pdma_read() and esp_pdma_write() functions
Date: Thu, 11 Feb 2021 08:01:50 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

On 10/02/2021 22:51, Philippe Mathieu-Daudé wrote:

Hi Mark,

On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
  hw/scsi/esp.c | 28 ++++++++++++++++++++--------
  1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index e7cf36f4b8..b0cba889a9 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -151,6 +151,20 @@ static uint8_t *get_pdma_buf(ESPState *s)
      return NULL;

Can you add get_pdma_len() (similar to get_pdma_buf) and ...

+static uint8_t esp_pdma_read(ESPState *s)
+    uint8_t *buf = get_pdma_buf(s);

        assert(s->pdma_cur < get_pdma_len(s));

+    return buf[s->pdma_cur++];
+static void esp_pdma_write(ESPState *s, uint8_t val)
+    uint8_t *buf = get_pdma_buf(s);


+    buf[s->pdma_cur++] = val;


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

One of the main purposes of this patchset is actually to completely remove all of these pdma_* variables and integrate everything into the existing FIFO and cmd buffers, so if you continue reading through the patchset you'll see that this soon disappears.

Even better towards the end you can see that both of these buffers are eventually replaced with QEMU's Fifo8 which has in-built assert()s to protect from underflow and overflow which should protect against memory corruption.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]