| Message ID | 20210209193018.31339-12-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | esp: consolidate PDMA transfer buffers and other fixes | expand |
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? > 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 Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
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.
Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit : > 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 > 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); > + } else { > + esp_set_tc(s, esp_get_stc(s)); > + } More fun? esp_set_tc(s, esp_get_stc(s) ?: 0x10000); > } else { > s->dma = 0; > } > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On 01/03/2021 21:35, Laurent Vivier wrote: > Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit : >> 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 >> 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); >> + } else { >> + esp_set_tc(s, esp_get_stc(s)); >> + } > > More fun? > > esp_set_tc(s, esp_get_stc(s) ?: 0x10000); I've just tried this, and it feels less intuitive to read when skimming over the code. I'd prefer to keep this in its current form and just accept that my coding style is more functional than artistic :/ >> } else { >> s->dma = 0; >> } >> > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> ATB, Mark.
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); + } else { + esp_set_tc(s, esp_get_stc(s)); + } } else { s->dma = 0; }
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 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(-)