On 2/11/21 9:01 AM, Mark Cave-Ayland wrote:
> 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-ayl...@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);
>>> +
>>
>> Ditto.
>>
>>> + buf[s->pdma_cur++] = val;
>>> +}
>>
>> Otherwise:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4...@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.
OK great! I planed to review half of it (21/42) but was too tired so
stopped at this one :D My bad for missing in the cover:
- 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.
Regards,
Phil.
