Re: [PATCH v2 11/42] esp: apply transfer length adjustment when STC is zero at TC load time

[Mark Cave-Ayland]

On 16/02/2021 07:33, Philippe Mathieu-Daudé wrote:

> On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
> > Perform the length adjustment whereby a value of 0 in the STC represents
> > a transfer length of 0x10000 at the point where the TC is loaded at the
>
> 0x10000 -> 64 KiB?

I'd prefer to keep these as they are, since TC is described in the documentation as 
16-bit counter: it is the number of bits that is relevant here as opposed to the 
absolute size.

There is a slight bit of trickery here in that the ESP emulation already handles a 
later variant of the chip which has a 24-bit counter which is why we can get away 
with setting its value to 0x10000 - guests that don't check for this will simply 
ignore the register containing the MSB.

> > start of a DMA command rather than just when a TI (Transfer Information)
> > command is executed. This better matches the description as given in the
> > datasheet.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/scsi/esp.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> > index a1acc2c9bd..02b7876394 100644
> > --- a/hw/scsi/esp.c
> > +++ b/hw/scsi/esp.c
> > @@ -562,9 +562,6 @@ static void handle_ti(ESPState *s)
> >       }
> >   
> >       dmalen = esp_get_tc(s);
> > -    if (dmalen == 0) {
> > -        dmalen = 0x10000;
> > -    }
> >       s->dma_counter = dmalen;
> >   
> >       if (s->do_cmd) {
> > @@ -699,7 +696,11 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> > val)
> >           if (val & CMD_DMA) {
> >               s->dma = 1;
> >               /* Reload DMA counter.  */
> > -            esp_set_tc(s, esp_get_stc(s));
> > +            if (esp_get_stc(s) == 0) {
> > +                esp_set_tc(s, 0x10000);
>
> 0x10000 -> 64 * KiB

And same here too.

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


ATB,

Mark.