Re: [PATCH v2 31/42] esp: implement FIFO flush command

[Mark Cave-Ayland]

On 03/03/2021 19:32, Laurent Vivier wrote:

> Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> > At this point it is now possible to properly implement the FIFO flush command
> > without causing guest errors.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/scsi/esp.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> > index 1d56c99527..0994673ff8 100644
> > --- a/hw/scsi/esp.c
> > +++ b/hw/scsi/esp.c
> > @@ -770,6 +770,8 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> > val)
> >           case CMD_FLUSH:
> >               trace_esp_mem_writeb_cmd_flush(val);
> >               /*s->ti_size = 0;*/
> > +            s->ti_wptr = 0;
> > +            s->ti_rptr = 0;
> >               s->rregs[ESP_RINTR] = INTR_FC;
> >               s->rregs[ESP_RSEQ] = 0;
> >               s->rregs[ESP_RFLAGS] = 0;
>
> Why don't  you set aso ti_size to 0?

I remember trying this and there was a sequence in one of my test images whereby the 
OS submitted a SCSI CDB followed by a flush (presumably to clear the CDB from the 
FIFO): this caused ti_size to be zeroed, losing the response reply size.

As Phil's original comment was against patch 38 "esp: convert ti_buf from array to 
Fifo8" that's where the comment is finally removed - it doesn't make sense to keep it 
after the conversion.

> Anyway:
>
> Reviwed-by: Laurent Vivier <laurent@vivier.eu>
>
> Thanks,
> Laurent


ATB,

Mark.