Re: lsi53c895a assert with AmigaOS

[BALATON Zoltan]

On Sun, 3 Mar 2024, Sven Schnelle wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
>
> > AmigaOS4 also has a driver for this card so I've tried to test it but
> > it trips an assert. Does anybody have an idea why and how it could be
> > fixed? Sven's recent patches don't seem to have an effect on this, it
> > still happens shortly after it tries to access the SCSI device with
> > those patches applied. (Unfortunately AmigaOS is not freely available
> > so it's a bit hard to reproduce but I can do tests if needed.) I got
> > the following traces:
> > [..]
> > lsi_do_command Send command len=6
> > qemu-system-ppc: ../hw/scsi/lsi53c895a.c:863: lsi_do_command: Assertion 
> > `s->current == NULL' failed.
> >
> > Any idea what could it be and what could be done about it?
>
> I think the Host is resetting the SCSI controller while it still has
> some request pending. I made a hack to work around that bug, but so
> far i haven't spent the time to verify whether it's correct or whether
> there are additional changes required. Here it is:

This does avoid the assert and now it boots but then can't read the scsi 
device. (I've tried with a scsi-cd with an iso image and it thinks it's an 
audio CD and cannot read data from it). Maybe something else is needed but 
this seems to go one step further. However I don't see "lsi_reset Reset" 
traces other than once when the driver starts so not sure it's really 
related to reset. Could it be that the driver expects it to be able to 
send commands while another one is still processing so the pending one 
would need to be put back in the queue instead of cancelling ir? But I 
don't know how to do that so cannot try unless you can give me a patch.

Regards,
BALATON Zoltan

> From 6a807653679fde5e3e09a7f27576c673f335fef6 Mon Sep 17 00:00:00 2001
> From: Sven Schnelle <svens@stackframe.org>
> Date: Sat, 3 Feb 2024 19:46:07 +0100
> Subject: [PATCH] lsi53c895a: free pending requests on reset
>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
> hw/scsi/lsi53c895a.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index d607a5f9fb..c6bd801a7e 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -346,6 +346,8 @@ static lsi_request *get_pending_req(LSIState *s)
>
> static void lsi_soft_reset(LSIState *s)
> {
> +    lsi_request *p, *p_next;
> +
>     trace_lsi_reset();
>     s->carry = 0;
>
> @@ -413,8 +415,14 @@ static void lsi_soft_reset(LSIState *s)
>     s->sbc = 0;
>     s->csbc = 0;
>     s->sbr = 0;
> -    assert(QTAILQ_EMPTY(&s->queue));
> -    assert(!s->current);
> +
> +    QTAILQ_FOREACH_SAFE(p, &s->queue, next, p_next) {
> +        scsi_req_cancel(p->req);
> +    }
> +
> +    if (s->current)
> +        scsi_req_cancel(s->current->req);
> +    s->current = NULL;
> }
>
> static int lsi_dma_40bit(LSIState *s)
> @@ -860,7 +868,9 @@ static void lsi_do_command(LSIState *s)
>         return;
>     }
>
> -    assert(s->current == NULL);
> +    if (s->current)
> +        scsi_req_cancel(s->current->req);
> +
>     s->current = g_new0(lsi_request, 1);
>     s->current->tag = s->select_tag;
>     s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun, buf,